fix(uavcan): stop losing and reordering SocketCAN TX frames - #28363
Merged
dakejahl merged 2 commits intoAug 25, 2026
Merged
dakejahl merged 2 commits into
dakejahl merged 2 commits into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
🔎 FLASH Analysispx4_fmu-v5x [Total VM Diff: -16 byte (-0 %)] FILE SIZE VM SIZE
-------------- --------------
+0.0% +55 [ = ] 0 .debug_abbrev
-0.0% -2 [ = ] 0 .debug_info
-0.0% -21 [ = ] 0 .debug_line
-60.0% -3 [ = ] 0 [Unmapped]
-0.0% -18 [ = ] 0 [section .debug_line]
-0.0% -10 [ = ] 0 .debug_loclists
+0.0% +13 [ = ] 0 .debug_rnglists
+100% +1 [ = ] 0 [Unmapped]
+0.0% +12 [ = ] 0 [section .debug_rnglists]
+0.0% +17 [ = ] 0 .debug_str
+0.1% +16 [ = ] 0 [Unmapped]
-0.0% -16 -0.0% -16 .text
-3.6% -16 -3.6% -16 uavcan::CanIOManager::send()
+0.0% +52 -0.0% -16 TOTAL
FILE SIZE VM SIZE
-------------- --------------
+0.0% +55 [ = ] 0 .debug_abbrev
-0.0% -2 [ = ] 0 .debug_info
-0.0% -21 [ = ] 0 .debug_line
-50.0% -3 [ = ] 0 [Unmapped]
-0.0% -18 [ = ] 0 [section .debug_line]
-0.0% -10 [ = ] 0 .debug_loclists
+0.0% +13 [ = ] 0 .debug_rnglists
+100% +1 [ = ] 0 [Unmapped]
+0.0% +12 [ = ] 0 [section .debug_rnglists]
+0.0% +17 [ = ] 0 .debug_str
+0.2% +16 [ = ] 0 [Unmapped]
-0.0% -16 -0.0% -16 .text
-3.6% -16 -3.6% -16 uavcan::CanIOManager::send()
+0.0% +52 -0.0% -16 TOTAL
Updated: 2026-08-24T22:21:29 |
Sorry, something went wrong.
Copy link
Copy Markdown
Member
|
Awesome work @dakejahl , I did already some work on the TX transmission (PX4/NuttX@cb1d457) but got stuck on the behavior in combination with DroneCAN. But your NuttX patch looks quite lean and clean. I've got a colleague working with test setup for DroneCAN, I'll ask him tomorrow to verify on our end as well. |
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
Author
Great! Keep me posted |
Sorry, something went wrong.
CanIface::send() forwarded sendmsg()'s return straight to libuavcan. A frame the CAN driver had no free hardware mailbox for never reached the bus and NuttX does not buffer it, so returning negative there loses it -- and losing one frame destroys the whole multi-frame transfer it belonged to. libuavcan already distinguishes "queue full, retry" (0) from "error" (-1), so report 0 and let it re-queue the frame. A non-blocking send the driver could not take immediately surfaces as ETIMEDOUT from net_timedwait(), not the ENOBUFS or EAGAIN one would expect, which is why matching only those left the frame loss in place. receive() had the mirror problem: uc_can_io maps any negative onto -ErrDriver, so an empty read after a spurious POLLIN was reported as a driver failure. Assisted-by: Claude:claude-opus-5[1m] Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com>
CanIOManager::send() asks the TX queue whether it holds something of equal or higher priority and, if so, sends that first. But when sendFromTxQueue() returns 0 -- it had nothing left to send, e.g. the head expired -- the code fell through and sent the new frame directly, putting it on the wire ahead of the equal-priority frames still queued behind it. Every frame of a multi-frame transfer shares one CAN ID, so those are exactly equal priority: the bypass reorders a transfer that the queue was holding in the right order, and the receiver discards it on the toggle bit. Only send directly when the queue does not already own the ordering. Assisted-by: Claude:claude-opus-5[1m] Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com>
dakejahl
force-pushed
the
fix/dronecan-tx-frame-ordering
branch
from
afc6399 to
3070500
Compare
August 24, 2026 22:13
Copy link
Copy Markdown
|
Hello, I tested these fixes and I will share my status.
The update finishes successfully (3 minutes) but with some mentions: the file.Read responses are 38 frames long (1 multi-frame) and multiple requests from F9P are launched without waiting for answer, making RT1176 to TX multiple responses at the same time.
So worth mentioning that this fix is highly effective with increasing the uavcan pool memory.
|
Sorry, something went wrong.
bogdan-beju-nxp
pushed a commit
to bogdan-beju-nxp/PX4-Autopilot
that referenced
this pull request
Sep 8, 2026
* fix(uavcan): retry SocketCAN frames the driver could not take CanIface::send() forwarded sendmsg()'s return straight to libuavcan. A frame the CAN driver had no free hardware mailbox for never reached the bus and NuttX does not buffer it, so returning negative there loses it -- and losing one frame destroys the whole multi-frame transfer it belonged to. libuavcan already distinguishes "queue full, retry" (0) from "error" (-1), so report 0 and let it re-queue the frame. A non-blocking send the driver could not take immediately surfaces as ETIMEDOUT from net_timedwait(), not the ENOBUFS or EAGAIN one would expect, which is why matching only those left the frame loss in place. receive() had the mirror problem: uc_can_io maps any negative onto -ErrDriver, so an empty read after a spurious POLLIN was reported as a driver failure. Assisted-by: Claude:claude-opus-5[1m] Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com> * fix(uavcan): do not let a new frame overtake the TX queue CanIOManager::send() asks the TX queue whether it holds something of equal or higher priority and, if so, sends that first. But when sendFromTxQueue() returns 0 -- it had nothing left to send, e.g. the head expired -- the code fell through and sent the new frame directly, putting it on the wire ahead of the equal-priority frames still queued behind it. Every frame of a multi-frame transfer shares one CAN ID, so those are exactly equal priority: the bypass reorders a transfer that the queue was holding in the right order, and the receiver discards it on the toggle bit. Only send directly when the queue does not already own the ordering. Assisted-by: Claude:claude-opus-5[1m] Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com> --------- Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com> (cherry picked from commit e172022)
This was referenced Sep 8, 2026
mrpollo
pushed a commit
that referenced
this pull request
Sep 9, 2026
* fix(uavcan): retry SocketCAN frames the driver could not take CanIface::send() forwarded sendmsg()'s return straight to libuavcan. A frame the CAN driver had no free hardware mailbox for never reached the bus and NuttX does not buffer it, so returning negative there loses it -- and losing one frame destroys the whole multi-frame transfer it belonged to. libuavcan already distinguishes "queue full, retry" (0) from "error" (-1), so report 0 and let it re-queue the frame. A non-blocking send the driver could not take immediately surfaces as ETIMEDOUT from net_timedwait(), not the ENOBUFS or EAGAIN one would expect, which is why matching only those left the frame loss in place. receive() had the mirror problem: uc_can_io maps any negative onto -ErrDriver, so an empty read after a spurious POLLIN was reported as a driver failure. Assisted-by: Claude:claude-opus-5[1m] Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com> * fix(uavcan): do not let a new frame overtake the TX queue CanIOManager::send() asks the TX queue whether it holds something of equal or higher priority and, if so, sends that first. But when sendFromTxQueue() returns 0 -- it had nothing left to send, e.g. the head expired -- the code fell through and sent the new frame directly, putting it on the wire ahead of the equal-priority frames still queued behind it. Every frame of a multi-frame transfer shares one CAN ID, so those are exactly equal priority: the bypass reorders a transfer that the queue was holding in the right order, and the receiver discards it on the toggle bit. Only send directly when the queue does not already own the ordering. Assisted-by: Claude:claude-opus-5[1m] Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com> --------- Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com> (cherry picked from commit e172022)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two of the three causes of the corrupted DroneCAN traffic reported in #26868. The third is in the FlexCAN driver and is handled separately: apache/nuttx#19957, backported in PX4/NuttX#394. All three are needed for a clean bus on i.MX RT; these two stand alone and are safe to merge in any order.
Problem
On SocketCAN targets a multi-frame DroneCAN transfer from the FMU regularly arrived reordered or short, and the receiver discarded it on the toggle bit. Measured on the wire on an i.MX RT1176, a 6-frame message failed 81.7% of the time. Single-frame traffic was unaffected, and a GNSS node on the same bus was flawless over the same capture, which places the fault on the FMU transmit path.
CanIface::send() forwarded sendmsg()'s return straight to libuavcan. A frame the CAN driver had no free hardware mailbox for never reached the bus, and NuttX does not buffer it, so a negative return loses it — and losing one frame destroys the whole transfer it belonged to. libuavcan already distinguishes "queue full, retry" (0) from "error" (-1). Worth noting for anyone who tried this before: a non-blocking send the driver could not take surfaces as ETIMEDOUT out of net_timedwait(), not the ENOBUFS or EAGAIN one would expect, so matching only those leaves the loss in place.
receive() had the mirror problem — uc_can_io maps any negative onto -ErrDriver, so an empty read after a spurious POLLIN was reported as a driver failure.
Separately, CanIOManager::send() asks the TX queue whether it holds something of equal or higher priority and sends that first, but when sendFromTxQueue() returned 0 (nothing left to send, e.g. the head expired) it fell through and sent the new frame directly — ahead of the equal-priority frames still queued behind it. Every frame of a multi-frame transfer shares one CAN ID, so those are exactly equal priority, and the bypass reordered a transfer the queue was holding correctly.
Solution
Report 0 rather than a negative from send() when the driver could not take the frame, so libuavcan re-queues it, and treat an empty receive() as no data rather than a driver error. Only send directly when the TX queue does not already own the ordering.
Measured on an ARK FMU-v6XRT against an ARK X20 GNSS node, decoding CAN-ID and tail byte per frame off a separate USB-CAN analyser: with these two plus the FlexCAN patch, 0 corrupt transfers in 84 594 frames over 5 minutes, then 35 029 more, against an 81.7% baseline. uavcan status CAN1 IO errors 0, sensor rates unchanged. Builds checked on px4_fmu-v6xrt, px4_fmu-v6x and SITL; only v6XRT was run on hardware.