Repository navigation
fix: honest FCC/CE apply-success reporting + 4G allowlist contradiction - #39
Merged
Merged
Conversation
…ction sendFrames() returned true if ANY of the 42 writes (21 frames x 2 rounds) succeeded, so the app reported "FCC enabled" even when the link dropped after the first write. It now requires every write to succeed and treats an empty frame list as a failure, matching sendFramesUnix() (the 4G path) in the same file. The aggregation rule is extracted to allFramesSucceeded() and unit tested. No DUML frames, ports, timing, or ACK handling changed — this only makes success reporting truthful. Also removes wa140 (Mini 4 Pro) from MODELS_WITH_4G: the Mini series has no cellular module, and the comment above the set already said so while the set itself included wa140. Resolves that self-contradiction (issue #27). The sendFrames fix re-applies the approach from PR #17 (ther3ptil31987-prog), cleanly on current main.
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.
What this does
Two low-risk correctness fixes, both unit-testable and CI-validated. No DUML frames, ports, timing, or ACK handling are changed.
1. Honest apply-success reporting
sendFrames()OR-accumulated success across all 42 writes (21 frames × 2 rounds) and returnedtrueif any single write succeeded. If the link dropped after the first write, the app still reported "FCC mode enabled" on a mostly-failed transmission (the root of the "success is unverified/brittle" reports).It now requires every write to succeed and treats an empty frame list as a failure — matching
sendFramesUnix()(the 4G path) which already does this. The rule is extracted toallFramesSucceeded()and covered byDumlTransportSendFramesTest.Per-frame success is "TCP connect + write succeeded" (the ACK read is best-effort), so this does not wrongly fail on NO_ACK frames — it only stops false positives.
Re-applies the approach from #17 by @ther3ptil31987-prog, rebased cleanly on current main (that PR had drifted into conflict).
2. Fix the 4G allowlist self-contradiction (#27)
The comment above
MODELS_WITH_4Gstates the Mini series (incl.wa140) has no cellular module, yetwa140was in the set. Removedwa140so the code matches its own stated intent.What this intentionally does not touch
Deeper 4G work (the
serial.take(5)full-serial parsing bug, model-code detection) and a readback-based verified-status layer are separate follow-ups that need on-hardware testing before shipping. This PR is scoped to changes that are safe without a real RC2.Testing
DumlTransportSendFramesTest(6 cases covering all-success, partial-fail, single-fail, all-fail, empty).assembleRelease+testDebugUnitTest.Closes the false-success half of #13; addresses #27.