Consume POLL body to ETX before replying to avoid RS-485 bus contention - #30
Open
brocci wants to merge 1 commit into
Open
Consume POLL body to ETX before replying to avoid RS-485 bus contention#30brocci wants to merge 1 commit into
brocci wants to merge 1 commit into
Conversation
brocci
force-pushed
the
fix-poll-answered-before-etx
branch
2 times, most recently
from
August 15, 2026 21:53
d7dedd1 to
8fe9ecb
Compare
brocci
added a commit
to brocci/ArduinoCMRI
that referenced
this pull request
Aug 16, 2026
# Conflicts: # test/test_cmri/test_main.cpp
brocci
added a commit
to brocci/ArduinoCMRI
that referenced
this pull request
Sep 10, 2026
PR madleech#30 consumes the POLL body to ETX before replying to avoid RS-485 bus contention. But POSTAMBLE_IGNORE is also reached when ignoring frames addressed to other nodes, and it decided to reply by checking _rx_packet_type. Since that field is only refreshed in DECODE_CMD, a node that had recently been polled (type == 'P') spuriously transmitted a GET reply every time it finished ignoring another node's frame. On a multi-node bus this ping-ponged between nodes, colliding with the addressed node's response and JMRI's polls. Fix the regression without changing PR madleech#30's design: clear _rx_packet_type when entering IGNORE_CMD, so finishing an ignored (foreign) frame can never look like a pending poll. Also initialize _rx_packet_type in the constructor.
A node now consumes the whole POLL frame (DLE-aware) through to ETX before replying, so it never transmits while the host is still on the wire. A node must only reply to a POLL addressed to it. _rx_packet_type is refreshed in DECODE_CMD and cleared when entering IGNORE_CMD, so finishing an ignored (foreign) frame can never look like a pending poll (this fixes a spurious GET reply on multi-node buses). _rx_packet_type is initialized in the constructor. Adds regression tests: reply waits for ETX, no reply to another node's POLL/GET, own POLL still answered after ignoring others.
brocci
force-pushed
the
fix-poll-answered-before-etx
branch
from
September 10, 2026 21:27
62c8fc1 to
8e230de
Compare
brocci
added a commit
to brocci/ArduinoCMRI
that referenced
this pull request
Sep 11, 2026
PR madleech#30 consumes the POLL body to ETX before replying to avoid RS-485 bus contention. But POSTAMBLE_IGNORE is also reached when ignoring frames addressed to other nodes, and it decided to reply by checking _rx_packet_type. Since that field is only refreshed in DECODE_CMD, a node that had recently been polled (type == 'P') spuriously transmitted a GET reply every time it finished ignoring another node's frame. On a multi-node bus this ping-ponged between nodes, colliding with the addressed node's response and JMRI's polls. Fix the regression without changing PR madleech#30's design: clear _rx_packet_type when entering IGNORE_CMD, so finishing an ignored (foreign) frame can never look like a pending poll. Also initialize _rx_packet_type in the constructor. Tests: regression tests for hearing another node's POLL/GET and still replying to one's own POLL, plus tests pinning process()'s returned packet type (PR madleech#27 semantics: POLL/SET reported, NOOP for ignored frames and INIT).
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
1 participant
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.
As reported in issue #28:
[SPEC-DEVIATION] Poll is answered without verifying the poll's ETX — now with real collision exposure
goto POSTAMBLE_POLLon thePbyte itself), src/CMRI.cpp:258-260, src/CMRI.cpp:147 (delayMicroseconds(50)).P, before the poll's ETX has been received or checked. A corrupted frame (FF FF 02 UA P <noise>) still gets a full response. In v1.5 the 50 msdelay()masked the timing race; in v1.7.0 the reply starts ~50 µs after thePbyte — while the host's ETX (~1.04 ms at 9600 bps) is still on the wire. On the spec's four-wire network the pairs are separate so no electrical collision occurs, but on the common 2-wire RS-485 setups this library is explicitly marketed for (Auto485 examples, examples/rs485_rx_and_tx/rs485_rx_and_tx.ino) the node's driver can be enabled while the host is still transmitting — a genuine bus-contention window of about one character time.POSTAMBLE_POLL_WAIT_ETXstate; reply only after ETX is received.This PR implements the required fix and adds corresponding tests.
The fix does not use a
POSTAMBLE_POLL_WAIT_ETXadditional state as suggested above, but reuses existingIGNORE_DATAand a small change inPOSTAMBLE_IGNORE.