Request for feedback for CAN message structure

[Pavel Pisa]

Hello Prashanth S and others,

excuse me for lower activity last weeks. We have exams period
and I have lot of other duties and was even ill. I am at Embedded
World in Nuremberg now, so may it be there is some chance to meet
somebody other from RTEMS community.

As for the e-mail it is better to add one of my personal e-mails
(pisa at cmp.felk.cvut.cz or ppisa at pikron.com)
to notice me for something important, I do not check every
message comming through my list subscription. I go through
RTEMS, NuttX, CAN, etc. forums only when I have spare time.

On Monday 20 of June 2022 18:37:38 Prashanth S wrote:
> This is a request for suggestions and feedback for the CAN message
> structure.
>
> *CAN message structure that represents the can messages from application to
> driver:*
>
> struct can_message {
> uint32_t timestamp;
> uint32_t id;                 // 32 bits to support extended id (29 bits)
> uint16_t flags;            // RTR | BRS | EXT ...
> uint8_t dlc;                 // (0 - 8) | 12 | 16 | 20 | 24 | 32 | 48 | 64
> uint8_t res;                 // reserved for alignment.
> uint8_t data[64];         // For supporting data transfer up to 64 bytes.
> };
>
> This is the CAN messages structure created based on the suggestions in the
> mail chain and looking through other CAN solutions (Nuttx, GRCAN, LinCAN).

Yes I like solution with explicit and aligned fields.
I confirm that I think that length representing real data length is better
and it has been chosen for CAN FD by SocketCAN.

I confirm that flags implemented with explicit sized field seem to be more
safe to me. Bitfields are tricky due to endianing and have problem with 
initialization and copying when new field is added.

I am not sure if the field name "dlc" (data length code) is right
when the field represents real length, may be "len" is a better
match.

I think that fields order makes sense as well (alignment, purpose etc..).
Only think to consider is possibility to swap

  uint32_t timestamp;
  uint32_t id;

because id is the first and the most important and accessed field,
so when the pointer to structure is give it can be small win
on some targets that it can be accessed without offset addition.
But that is probably neglectable and there can be arguments against.

Another think to consider is a size of "dlc" and or "len" code.
There is ongoing standardization process for CAN XL and it
would allow CAN frames up to 2kB and the length will be with
byte granularity. But CAN XL adds datatype filed and much more
options so it is probably out of the actual RTEMS scope
and would require variable length read and write systemcalls
because 2kB overhead for some 1 or 2 byte message is too high.

So even 8 bit length field is OK for now and may be res field
can solve CAN XL compatibility one day.

So generally I agree with the message structure, I declare
minor weight vote to switch from "dlc" to "len".
I think that switch to common message structure based API
on RTEMS would be big win and that (at least for
now) keeping character device like API with IOCTLs
is simpler and matches better actual RTEMS CAN implementations. 

Best wishes,

Pavel