fix: correct cs-bit polarity in compound-wait and chained request logic - #63
Open
SoundMatt wants to merge 1 commit into
Open
fix: correct cs-bit polarity in compound-wait and chained request logic#63SoundMatt wants to merge 1 commit into
SoundMatt wants to merge 1 commit into
Conversation
compound_wait_check_of() and should_execute_chained() in include/rcp/request.hpp had the cs bit's polarity inverted relative to the CompoundWaitCheck enum's own documented mapping and the field's "conditional-start" naming. cs=false now maps to Immediate / always- execute, and cs=true maps to AfterChangeOnly / abort-on-predecessor- error, matching the enum comments and the OPEN Alliance TC18 Remote Control Protocol Specification v0.5.1_RC's field semantics. Updates the affected assertions in tests/test_request.cpp, which previously locked in the inverted behavior. Closes #58 Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com>
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.
Summary
compound_wait_check_of()andshould_execute_chained()ininclude/rcp/request.hpphad thecsbit's polarity backwards relative to theCompoundWaitCheckenum's own documented mapping and the field's "conditional-start" naming.should_execute_chained()is reachable from a real public API path (RequestLedger::finalize() -> propagate_chain_completion()), so this was a live bug for chained requests.cs=falsemaps to Immediate/always-execute andcs=truemaps to AfterChangeOnly/abort-on-predecessor-error, matching the enum comments and the field semantics described in the OPEN Alliance TC18 Remote Control Protocol Specification v0.5.1_RC.tests/test_request.cpp(including the two chained-successor propagation tests whosecsvalues encoded the old, wrong polarity) so the test suite now protects the correct behavior.Closes #58
Test plan
cmake --build build --parallel(full build, no warnings introduced)ctest --test-dir build -j4— 53/53 tests pass, includingrcp_request