Configure CAN to preserve message order

[thomasonw]

Description

Ref issue #4090 this change will configure the STM32Fx CPUs to keep message order of CAN packets.

There are some higher level CAN protocols (specifically NMEA2000 fastpacket) which depend on the packet order of transmitted messages be preserved. The current initialization will not preserve message order, instead will utilize message priority to decide packets being sent - and at the same time not assure same order of same priority messages.

This change assures messages will be sent in true FIFO.

Status

READY

Migrations

No API changes are needed with this approach for keeping packet order.

NO

Related PRs

#4090

NOTE

If it is agreed that this is the approach for addressing issue #4090, other MBED targets should be reviewed and their code modified accordantly.

[0xc0170]

cc @bcostm @adustm @LMESTM @jeromecoutant

[sg-]

bump @bcostm @adustm @LMESTM @jeromecoutant

[bcostm]

Thanks @thomasonw for this PR.
LGTM but I don't know if this change can cause issues in other protocols ? Maybe for future improvement is to add the possibility to change this packets priority in the CAN api, at init for example or add a new method ?

[0xc0170]

I don't know if this change can cause issues in other protocols ?

Are there? As you asked in the issue, if we specify for CAN that it should be ordered, what would be the consequences?

[thomasonw]

All,
Thank you for chiming in! Unintended consequences is a great question, here is my take: The current state of prioritizing higher priority messages in the TX queue has the effect of allowing one Tx message to be sent out ahead of the other two, as opposed to waiting until the others ones have cleared.

In thinking of the use case for why one would want this, there are very few which come up. Perhaps the most obvious one being if somehow the existing messages are stuck, and there is a critical need to get a message out. One example might be: a life-critical application where the CAN bus is at such high utilization it is preventing the other messages from finding a Tx window, and we need to send out a safety critical (perhaps STOP moving something) message. I would offer that such a situation is unlikely for any MBED application - largely because if indeed such a critical application was being developed it is unlikely one would rely on the MBED libs for the basis (not trying to be disrespectfully here, but the quality of the MBED libs are perhaps not sufficient to use for the basis of any truly life / safety critical development).

And I might offer, any CAN which sees such high utilization as described above is in its self in of fundamental adjustment.

I might add that I suspect for most people would be surprised to learn that message order is not kept already. I could not find any MBED documentation on the subject, and I suspect most folks would assume order would be kept, just like any other API be it serial, files, etc...

Lastly I will point out that Linux has adopted a must-keep-message-order stance in their soketCAN protocol - myself, I think it would be better for MBED to take a like approach, explicitly state must keep Tx order and all drivers should be updated to implement that.

So, in short: I offer that keeping Tx order is in general the appropriate approach, and that if indeed there is a situation where one anticipates the need to jump the queue, it is unlikely MBED would be used in the 1st place, but even so perhaps there are existing alternatives (ala, re-initializing the CAN subsystem and then placing in the high priority message)

Does this make sense? Have I missed anything?

[adustm]

@bperry730, @ohagendorf : any opinion ?

[thomasonw]

And thoughts on this?

[theotherjimmy]

@bperry730 @ohagendorf Could you review?

[bperry730]

I'm not as much of an expert on CAN as @thomasonw, but here are my thoughts:

My understanding of CAN is that message priority is a key part of the CAN protocol, and it makes sense from a safety perspective. As thomasonw mentioned, if you have a safety critical message (like power-off or stop) then you want that message to leave the CAN queue as soon as possible. In one of my applications, we are communicating with a motor driver over CAN. The motor driver allows for high priority safety messages that will safely turn off the device. It's seems a little risky to me to reduce the efficacy of this mechanism. Having said that, I don't anticipate that we will be nearly saturating the CAN bus bandwidth to the point that this would become an issue.

Also, I don't necessarily agree with the argument that 'mbed is not the library to use in serious embedded development'. I believe that mbed wants to be THE embedded library for ARM cortex-M processors, so even if we're not there now, we should expect and produce quality of that caliber.

Having said that, I'm not particularly passionate about this change. I don't think it would affect any of the applications I've encountered. The one change I would consider is adding a separate member function or constructor to the CAN api that allows you to select whether you want a priority queue or regular queue for your application. This might not be necessary or might make the CAN api too bulky, but I think it's a valuable path to consider.

[thomasonw]

bperry you are correct the CAN protocol provides for backoff of lower priority messages between competing nodes as part of its fundamental operation, and most controllers will then attempt to automatically rebroadcast those backed off messages in the next frame timeslot. This pull request does not alter any of that.

I am not claiming to be an expert here, but I do understand that today’s Best Practices for CAN controllers is to run the internal Tx queue in Priority message mode (PQ), contrary to this pull-request which enabled FIFO mode. PQ is needed in high utilization CAN environments with currently state-of-art running reliably in the mid-80% as I understand, with FIFO being problematic in CAN deployments where utilization exceeds 40%. But to be able to archive high rates more than PQ would be needed - with potential API changes involved as well. (Example: How to recover from the current situation where if we want to send out a high priority message, but all Tx mailboxes are filled with lower-priority messages being blocked due to CAN bus utilization)

My approach here was to solve a known issue w/o requiring API changes, and I do believe for most apps likely to be supported with the MBED libs it will be sufficient. However, if the community is open to API changes there are several other ways to address the core issue – one could add a Tx_queue() status query, that would be perhaps the most direct. An optional FIFO/PQ mode selector in can_init(), or perhaps a FIFO/PQ option to can_mode() might be another option. Or I could simple direct code such changes in my app. Not sure how such high-level framing conversations such as this occur inside the MBED community.

Finally, I feel compelled to clarify my comments on MBED and its application – In no way was I claiming that MBED was inappropriate for ‘serious’ embedded applications, MBED is a great effort, and will improve over time. However issues such as #3829 illustrate my point; that such a significant error even existed is caution for using MBED for any direct human life-safety (Health, man-machine motion, etc) development. The risk / liability is just too high.

[theotherjimmy]

bump @ohagendorf do you have any comments?

[theotherjimmy]

bump @ohagendorf ?

[ohagendorf]

In our current projects we don't have the described problem. At the moment. But I'm sure both possibilities should be available/possible.
I would highly suggest to make this point configurable as @bcostm said. Why not use the mode method with an additional entry in the Mode enum for this?

[thomasonw]

I have made a mess of my local MBED copy, so have deleted it and will close this PR, then open a new one with suggested edits.

[thomasonw]

See #4359 for new submission