Repository navigation
Return from Can::send() whether the frame was handed to the driver - #370
Merged
Merged
Conversation
Can::send() drops a frame while the bus is not RUNNING and ignored the result of twai_transmit(), so a module waiting for an answer could not tell a dropped frame from an unanswered one. It now returns false in both cases. Every caller ignores the result so far.
…the result means A frame the driver rejects, e.g. because the TX queue is full, returned false silently, while a frame dropped because the bus is not RUNNING printed the rate-limited "CAN send dropped" echo. Both paths now share one helper, and the second one names the reason: "TX queue full" for ESP_ERR_TIMEOUT, the ESP-IDF error name otherwise. The header says that true means the driver accepted the frame, not that it was acknowledged on the bus, and the definitions carry the const of the declarations. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WgKVNX7roCP4Wj4t5CrsE9
RmdMotor::send() waited up to 3 ms for the motor's answer and retried twice, also when Can::send() had dropped the frame because the bus is not RUNNING or the TX queue is full. With a stopped bus that cost at least 9 ms and three warnings per motor and step, since step() sends 0x9c every time. Now it returns false right away, without waiting or retrying in the same call: the bus state and the queue do not change within microseconds, and the next step() sends again anyway. The three attempts stay for a frame that was queued but not answered within 3 ms. Callers see no new result: after three failed attempts they got false before as well. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WgKVNX7roCP4Wj4t5CrsE9
falkoschindler
approved these changes
Nov 3, 2026
falkoschindler
left a comment
Collaborator
There was a problem hiding this comment.
@JensOgorek The change does what it says, and the ESP32 build with ESP-IDF v5.3.6 is clean. Two commits on the branch pick up what the review turned up; details below so the decisions stay traceable.
Fixed on the branch
- A frame that
twai_transmit()rejects was dropped silently (e8ed213). The "bus not RUNNING" path printed its rate-limited echo, while a rejected frame, e.g. with a full TX queue, only returnedfalse. Both paths now go through one helper, and the second one names the reason: "TX queue full" forESP_ERR_TIMEOUT, the ESP-IDF error name otherwise. The existing message stays word for word. RmdMotor::send()kept waiting and retrying after a dropped frame (4cd0352). It is the pattern from the motivation: up to 3 ms of busy-wait and three attempts per frame, so a stopped bus cost at least 9 ms and three warnings per motor and step, sincestep()sends 0x9c every time. It now returnsfalseat once whenCan::send()does; the three attempts stay for a frame that was queued but not answered within 3 ms. Callers get the same result as before after three failed attempts. Not tested on hardware; your F30 plexus withcan.stop()would be the place.- The header did not say what
truemeans (e8ed213). Two comment lines on the overloads: the driver accepted the frame, which is not an acknowledgement on the bus. - The definitions dropped the
constof the declarations (e8ed213). Aligned, includingdlc.
Collaborator
Author
Bench retest of 4cd0352 passed, merging@falkoschindler Thanks for the two commits. On a bench ESP32 with an |
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.
Motivation
A module that waits for a motor's answer after each frame cannot tell from
Can::send()whether the frame went out. While the bus is notRUNNING,Can::send()drops the frame, and it ignores the result oftwai_transmit(), e.g. when the TX queue is full. Such a module either waits for an answer to a frame that was never sent or asks the TWAI driver itself, which makes a second place that decides when the bus is usable.Implementation
Can::send()overloads returnfalsewhen they drop the frame, i.e. while the bus is notRUNNINGor whentwai_transmit()fails, andtrueotherwise.twai_transmit()rejects prints the same rate-limited "CAN send dropped" echo as a frame dropped while the bus is notRUNNING, with the reason ("TX queue full" or the ESP-IDF error name). The header says thattruemeans the driver accepted the frame, not that it was acknowledged on the bus.RmdMotor::send()returnsfalseat once whenCan::send()dropped the frame, instead of waiting up to 3 ms and retrying twice. The three attempts stay for a frame that was queued but not answered. Callers ofRmdMotor::send()get the same result as before after three failed attempts.Test on F30's plexus 2 (ESP32-S3)
This commit on top of #365, with a downstream RMD motor module that returns at once when
Can::send()returnsfalse. Withcan.stop()for 6 s on the plexus console, the module printed no CAN timeout (975 lines in a build without both changes), Lizard printed its "CAN send dropped: bus not RUNNING" once per second, and both motors answered right aftercan.start(). Gains that the startup script writes to the motors were still applied after a restart.The two review commits came after this test and were tested on the bench, see below.
Bench test of the review commits (ESP32)
A classic ESP32 without a CAN transceiver, with
can = Can(32, 33, 1000000)andrmd = RmdMotor(can, 9, 6)without a motor behind it, before (773ac07) and after the review commits (4cd0352), both built with ESP-IDF v5.3.6. Without a transceiver the bus staysRUNNINGand the TX queue fills up; thencan.stop()for 6 s.RUNNING, TX queue fullcan.stop()A motor that answers takes the same path as before, since
RmdMotor::send()only returns earlier whenCan::send()drops the frame.Progress
※