Re: [PATCH] can-utils: Add exact CAN frame length calculation (including bitstuffing)

[Michal Sojka <sojkam1@fel.cvut.cz>]

Hello all,

On Sat, Jan 25 2014, Oliver Hartkopp wrote:
> Hello Michal,
>
> On 24.01.2014 14:59, Michal Sojka wrote:
>> This adds an algorithm for calculating the exact number of bits a CAN
>> frame occupies on the bus and uses this algorithm in canbusload.
>
>
>> +++ b/canframelen.h
>> @@ -0,0 +1,8 @@
>> +#ifndef CANFRAMELEN_H
>> +#define CANFRAMELEN_H
>> +
>> +#include <linux/can.h>
>> +
>> +unsigned can_frame_length(struct can_frame *frame);
>> +
>> +#endif
>> 
>
> I would suggest two things:
>
> 1. Add some comment about the function (e.g. that it returns the length in
> bits with/without inter frame gap).
>
> 2. I would prefer to add some calculation mode identifier here and provide all
> calculation modes in canframelen.c
>
> Including this stuff and it's comments:
>
> https://gitorious.org/linux-can/can-utils/source/canbusload.c#L375
>
> E.g. some
>
> enum {
> 	NO_BITSTUFFING, /* plain bit calculation without bitstuffing */
> 	BITSTUFFING, /* with bitstuffing following Tindle estimation */
> 	PRECISE, /* precise calculation including bitstuffing in CRC */
> }
>
> To be future proof with CAN I would suggest to add some CAN / CAN FD switch.
>
> E.g.
>
> unsigned can_frame_length(struct canfd_frame *cfd, int mode, int mtu);
>
> Where mtu is CAN_MTU or CANFD_MTU to distinguish the frame types.
> In the first step supporting CAN_MTU would be sufficient.
> As a return value of zero always shows a problem this can be used to indicated
> unsupported calculation modes/frame types.

The updated patch will follow this email.

Best regards,
-Michal