Repository navigation
Add a Robot 36 determinism guard for the SSTV line renderer - #169
Merged
Merged
Conversation
Robot 36 is excluded from the stateless exact-equality reuse test because it is stateful across lines (even lines carry R-Y, odd lines B-Y, paired across line boundaries) and keeps its own per-line arrays rather than reusing a shared scratch buffer. That left it without any exact-equality guard, only the tolerance-based round-trip tests. This adds a test that renders the same signal twice with two separate renderers and asserts the two full RGB buffers are byte-identical, confirming the rendering is deterministic and leaks no state between renderer instances.
There was a problem hiding this comment.
🟢 Approval recommended
The focused, test-only change matches its stated purpose and has no identified blocking issues.
No open issues 🎉
What changed in this PR
Adds an exact-equality determinism guard for Robot 36, complementing the existing SSTV renderer tests.
Changes:
- Renders the same signal across all transmitted lines using two separate renderer instances.
- Asserts byte-identical full RGB buffers without changing production code or existing tests.
| File | Description |
|---|---|
| OscarWatch.Tests/Sstv/SstvLineRendererAllocationTests.cs | Adds the Robot 36 determinism test and explanatory documentation. |
Review effort: Balanced
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.
Hi Peter,
This is a follow-up to #166 and the quietest-of-5 allocation fix on main (7093fb9).
While looking at that area I noticed a gap in the renderer's test coverage. The exact-equality reuse test (Reusing_scratch_buffers_matches_a_fresh_buffer_per_line) deliberately excludes Robot 36, because Robot 36 is stateful across lines and keeps its own per-line arrays rather than reusing a shared scratch buffer. That left Robot 36 with no exact-equality guard at all, only the tolerance-based round-trip tests.
This change adds a test that renders the same Robot 36 signal twice, using two separate SstvLineRenderer instances, across all transmitted lines, and asserts the two full RGB buffers are byte-identical. That confirms the rendering is deterministic and leaks no state between renderer instances, which is the invariant we rely on for the reused-buffer path.
The change is test-only. No production code was touched, and the existing tests are unchanged.
Verification, local SSTV subset:
Passed! - Failed: 0, Passed: 179, Skipped: 0, Total: 179, Duration: 2 m 10 s - OscarWatch.Tests.dll (net10.0)
The new test on its own:
Passed! - Failed: 0, Passed: 1, Skipped: 0, Total: 1, Duration: 40 ms - OscarWatch.Tests.dll (net10.0)