Extending CAN mode to include FIFO #4359
Conversation
|
@thomasonw Have you look at other CAN implementations to make sure that this option is generic and possible (regular use case) ? If yes, please provide details, it would help. when changing order, if |
|
@0xc0170 Hello. I searched the MBED-OS files for "CanMode" to locate any other devices which might be affected. I found 10x, including the STM32 file. All of them contained a default: statement which would return 0 (failed) if called with MODE_FIFO. So the application will know the device can not be configured to assure packet order. Going forward these other devices could be extended - a simple (and commonly used) approach is to use only one Tx mailbox. Once modified, or confirmed they already do work in FIFO, the MODE_FIFO: should be added to device switch statement. Good point on reverting FIFO mode. NORMAL would be an obvious choice. I have updated the pull to reset the appropriate flags. Makes code a little larger, but to your point is perhaps more consistent with how other 'mode' states operate. One thing I do not know is how the controller will behave making changes in-flight, notable if there happens to be data in the Tx buffers. Without a way to see if the buffers are flushed seems it would be safer to RESET the controller to get back to the default mode. |
|
Can you please rebase instead of merging ? (to remove latest merge commit) |
|
cc @bcostm @LMESTM @adustm @jeromecoutant please review |
|
@0xc0170 "Can you please rebase instead of merging ? (to remove latest merge commit)" OK, need help here - I have never figured out the correct way to bring my copy up to current w/o causing some issues. Would you be so kind as to give me a pointer on how to rebase instead of merging? Will be happy to, just do not know the safe steps to take... Thank you, |
|
There are few options. I would just remove the last commit (could do with `git rebase -i HEAD~2 and remove the one commit from those two) and then github contains help regarding rebasing https://help.github.com/articles/using-git-rebase-on-the-command-line/, https://help.github.com/articles/resolving-merge-conflicts-after-a-git-rebase/ |
Allows optional configuration of CAN controller to transmit messages in FIFO order (chronologically) independent of their priority.
Feedback on pull request, have MODE_NORMAL restore the flag.
|
@0xc0170 -- ok, perhaps I have accomplished the git changes. Will you please take a look and see if things are now where they need to be? Thank you for the hints, still kind of driving by braille though |
There was a problem hiding this comment.
Looks sensinble.
Before we accept, we should make sure this option is generically accepted by targets. I checked quickly NXP kinetis series, their CAN is a bit complicated at first glanze but seems like their fifo can be set to this.
have you verified by any other mcu family other than STM32?
|
I am not sure how many CPUs explicitly have the ability to select transmission order. But a very common approach to force FIFO is simply use only one Tx mailbox - and make sure any software queue in the drivers also work in true FIFO mode. |
|
@sg- This involves API additions. Could you review? |
|
@sg- bump. |
|
@thomasonw How many devices support this? Is FIFO mode unique to STM32 targets? |
|
Hardware enforced FIFO mode does seem unique to the STM32 line of CPUs – though I have not studied all other CPUs in depth. Having said that, the most common approach to doing a simple FIFO implementation that is universal is to use only one Tx mainbox – it is the most common way socketCAN (linux) does their drivers. Of course with more work multi-Tx mailboxes can support FIFO mode, and the CPUs with multiple Tx queues are able to do a mixed FIFO / prioritized approach.
|
|
Thanks @thomasonw . Seems like this should be parked, and find more common approach. Thus I'll close this PR, you can create a tracking issue to describe the problem, reference this PR there. What do you think? |
|
I apologize for the delayed response, at times we have less than optimal internet connection. @0xc0170 , I am not sure I follow what you mean by a more common approach. Both of the proposals I have submitted here are applicable to any combination of CPU and driver. There is already opened a PR, here: #4090 Perhaps stepping back: This is not a CPU issue, but a lack of specificity in the MBED simple CAN APIs, specifically how message ordering is handled. The STMF0 CPUs happen to expose the lacking in such a way as to prevent the MBED simple CAN APi being usable for applications where message order is critical, NMEA2000 fact-packets being one example. It is likely other CPUs do not exhibit the behavior; hence an inconsistency in the MBED definition. It is not fully clear to me how these issues are resolved in the MBED community, so at this point I will await for a determined direction. My input to the MBED community is that my 1st solution is the correct one for the simple CAN API, that message order should be preserved. This is consistent with best practices for other simple CAN drivers in other environments, including other embedded libs as well as Linux’s soketCAN. |
We were unable to see that this new mode would get HAL support for other targets. Having new mode that is supported by minority is not ideal.
Can you reference directly docs or code for this? Do they support this fifo mode (order kept, no priority involved) ?
I missed this one, explains the issue more in detail. One of the solutions to this would be that HAL specifies the order of messages is kept (chronological order, as in here, fifo)? Anyone having any suggestions? |
|
@0xc0170 Hello. Myself, I am not a fan of adding the new Mode to the API, though it could be implemented on any CAN controller - the simplest way is to use only one Tx mailbox. My preferred solution for the simple MBED CAN drivers is to declare they keep message order, again that is often (though not always) simply implemented by using one Tx mailbox. For a few examples of a simple CAN APIs which retain message order see the following (note, it is most often necessary to examine the actual code to see how it operates): It is perhaps also helpful to step back a little. FIFO is not best-practice for high capacity CAN deployments, in fact anything above around 30-40% utilization FIFO brings a multitude of issues - the chief being excessive retries at nodes as well as some nodes delaying or blocking messages as a result of lower-priory messages in effect being 'stuck' in their Tx mailboxes, preventing higher priority messages from gaining access. (A form of priority inversion). This white paper provides an overview of the issue, as well as some of the approaches used to overcome it: https://pdfs.semanticscholar.org/36b3/b0b15b107ebb811a8cac4a733a6a248a6dde.pdf And with this I want to point out a key difference: The API needed to support advanced CAN queue management goes well beyond what MBED presents. It must include more direct access to Tx mailboxes, their quantity, their status. It must provide for the ability to cancel/recall a queued message if needed, the ability to understand how many Tx boxes are present (perhaps setting the quantity), and if the hardware supports multiple hardware queues as well. Callbacks for both Tx and Rx would be needed. Among other capabilities. None of this is presented by the MBED CAN API The MBED API presents a simple CAN interface, with message write function that in the case of the STM32Fxxx CPUs only indicates of a packet was placed in the Tx queue. One could easily introduce excessive delays to higher priority messages simply by filling all 3x Tx mailboxes in the STM32Fxxx CAN controller with lower priority ones on a highly utilized CAN - until one of those messages clear any additional message, regardless of its priority, will be blocked as the MBED API provides no ability (outside of direct hardware access and/or resetting the controller) to correct this. I have not examined other CPUs drivers but I suspect there may be other such situations in the current MBED libs. I offer that the current MBED CAN API is appropriate for low to moderate utilization situations, and that it is in this way that retaining message order is appropriate for reasons outlined above. I will also say it would be a good exercise to extend the MBED CAN API with more capable features; allowing for not only higher utilization, but also application control over this message ordering issue. However such an extended API would be a notable effort, esp to cover a wide range of controllers. |
|
Is there any update on this issue? |
|
I dont think there is any. The tracking issue was closed due to inactivity. You can comment there with providing the details for your interest in the issue |
|
Hello.
The conclusion of MBED was to make no changes, so we have dropped the request and have moved on to proprietary CAN drivers and a non-MBED solution.
|
|
@thomasonw could you give me an example what you ended up with please? |
|
never mind, thanks anyway |
Allows optional configuration of CAN controller to transmit messages in
FIFO order (chronologically) independent of their priority.
Description
This pull request is a continuation of #4121 using a clean copy of the MBED lib. The goal is to allow an ability for application to assure CAN message order it retained. In the conversations of #4121 it was suggested to extend the ::MODE() API to include a FIFO option.
This pull request does that for the STM32.. CPUs featuring a built in CAN controller.
Status
READY
Migrations
If this PR changes any APIs or behaviors, give a short description of what API users should do when this PR is merged.
YES -- ::MODE now has a new option: FIFO. If the controller as drivers have been modified to support FIFO mode 1 will be returned. It the controller is not able to assure message order, 0 (failure) will be returned.
,
Related PRs
#4090, #4121
Todos
Deploy notes
Notes regarding the deployment of this PR. These should note any
required changes in the build environment, tools, compilers, etc.
Steps to test or reproduce
Outline the steps to test or reproduce the PR here.