Exposed frameFD.flags (for bitrate switching)

[wdim0]

Hi,

I've made an important modification that enables users of node-can to do bitrate switching for CAN-FD frames. Until now it was impossible (nobody complained yet?) - there was no way how to perform bitrate switching (set BRS bit in CAN-FD frame). Bitrate switching is one of the main points of "FD" = "Flexible Data-rate".

frameFD.flags in SendFD method is now filled from new "fd_flags" symbol (feel free to rename if you come up with better name). In the original code, the frameFD.flags was hardcoded to 0. But we need to be able to influence bit 2..0 in this flags field in order to fully control the transmission behavior.

Let's see how official "cansend" tool handles it and see the meaning of the flags:

This is a FD frame without bitrate switching (captured during my initial testing)

And this is FD frame with bitrate switching after the modification. fd_flags / flags is set to 0x01 in the "caller" code. (same data as above)

Anyway, thanks for node-can!

Have a nice day

Martin

[wdim0]

Anybody alive there? This modification is rather important.

[sebi2k1]

Hi and thanks for the pull request. But the change can't work. You are reading the field an object which doesn't have the field. You have to extend socketcan.json:363.

Please also make sure the way you introduce it must be compatible with existing implementation and should cover error cases too.

[wdim0]

Hi @sebi2k1,

thank you for your answer.

Sebi, you write that it can't work, yet it DO WORK for me - tested. Please note that not all users of node-can use "DatabaseService" - some do call "sendFD(...)" directly. For example this: https://github.com/grodansparadis/node-red-contrib-socketcan which is the only up-to-date implementation of CAN bus handling for Node-RED (other similar cannot even send FD frames). So for "sendFD(...)" callers, my modification is enough - tested.

And to be fair with you, I don't feel to be familiar enough with DatabaseService to change the code there (yes, I could guess, but I cannot test) so better approach than ask to pull non-tested and possible buggy code is to hope that you are willing to fill in the missing things to complete it if needed. I really think that flags should be somehow passed to this: in because without it, node-can cannot be used to send CAN-FD messages with bitrate switching set on (which is the point of "FD" = "Flexible Data-rate").

node-can is awesome and gives the Node.js the ability to handle CAN bus. It knows CAN-FD. Let's give it possibility to use it fully.

Thanks, sorry for possible additional work for you and have a nice day @sebi2k1

Martin

[juleq]

@wdim0 I think you are misunderstanding the answer of @sebi2k1. His point was not that hardcoding in flags on the native side does not work technically. His point was that this PR cannot be accepted since adding support requires implementing the passing of these flags from js to native. And that prior validation, error handling and testing is required.

To the tone: This is an open source project and as far as I understand that, you are not entitled to allocate the time @sebi2k1 can spent.

If you want it fast, you could get into it and flesh out the PR as required.

If you cannot do that yourself, maybe you can ask @sebi2k1 to add a "help wanted" tag. I think this is a wonderful and straightforward task to get into open source contribution.

My best regards.

[wdim0]

Hi @juleq,

I'm sorry if what I wrote left an impression of a bad tone. I don't want to offend anybody and I don't want to demand anybody's time. I'm really grateful for node-can and for what all the contributors of node-can have achieved.

I don't need it fast. I have what I need working already. I just wanted to share this IMHO important improvement with others - to give to the community, not just take.

But yes, my modification is not visible to "DatabaseService". If the PR would be accepted, it will not break anything, it's backward compatible, but it exposes the new fd_flags only to users of native side (users like for example https://github.com/grodansparadis/node-red-contrib-socketcan is), not to the "DatabaseService" users. Maybe somebody sometime (not just @sebi2k1) will pick it up later and implement it for "DatabaseService" when needed. I cannot do the modifications you require (extend socketcan.js:363, error handling, ...), because I don't use it this way. I cannot test it - I cannot guarantee the code then. I don't use the "DatabaseService" and .kcd ecosystem.

@sebi2k1 - I'm sorry if I offended you. Please accept my apology in that case. If you think that giving to users of node-can a possibility to produce CAN-FD frames with bitrate switching is a good idea, could you please:

a) add a "help wanted" tag as @juleq suggests

or b) accept the PR and at least users of native side will be able to use it for now. The PR will not break anything and it's backward compatible - tested. If the new fd_flags is not filled/touched, it will have the same value of 0, as is hardcoded in the current official version: https://github.com/sebi2k1/node-can/blob/5f30eb69bc31157fb1b85830253cd8b56cebdc39/src/rawchannel.cc#L378

If you don't see any point in possibility to be able to produce CAN-FD frames with bitrate switching, then please close/discard this PR.

Thank you, all the good to you and have a nice day

Martin