fix(_read_metadata): accumulate split serial chunks so calibration survives (#61)#64
Closed
jmdevita wants to merge 1 commit into
Closed
fix(_read_metadata): accumulate split serial chunks so calibration survives (#61)#64jmdevita wants to merge 1 commit into
jmdevita wants to merge 1 commit into
Conversation
…rvives _read_metadata() read the device metadata in a loop but returned as soon as a single serial read contained "END", discarding every earlier chunk. The PPK2 sends its calibration metadata as one multi-line ASCII string ending in "END", which the USB serial driver can split across multiple reads (observed on macOS). When that happened, calibration fields delivered in earlier chunks -- O, S, I, GS, GI, HW -- were silently dropped and left at their default of 0. With the range-0 zero-point offset O[0] missing, every low-uA reading gained a fixed offset of roughly ~1.8 uA, with no error raised. Accumulate each decoded chunk and only return once "END" appears in the combined string (matching the official nRF Connect Power Profiler). Bump the retry budget 5 -> 10 since a split stream needs several reads. Bumps the version to 0.9.3 to distinguish this build from the PyPI 0.9.2 that predates it. Same approach as upstream PR IRNAS#62; fixes IRNAS#61. Adds tests/test_read_metadata.py: feeds a split metadata stream through a fake serial port and asserts the full string survives and the parsed O[0] offset is preserved (would stay 0 under the old drop-chunks behaviour). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit c3e138e)
Author
|
Closing for now — opened prematurely; will re-open after review on my fork. |
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.
Problem
_read_metadata()returns as soon as a single serial read contains"END", without accumulating earlier reads. The PPK2 sends its calibration metadata as one multi-line ASCII string ending inEND, which the USB serial driver can split across multiple reads (reproducible on macOS). When that happens, calibration fields delivered in the earlier chunk —O,S,I,GS,GI,HW— are silently dropped and left at their default of0.With the range-0 zero-point offset
O[0]missing, every low-µA reading gains a fixed offset of roughly ~1.8 µA, with no error raised — so low-current measurements read systematically high versus the nRF Connect Power Profiler.Fix
Accumulate each decoded chunk and only return once
"END"appears in the combined string (matching the official Power Profiler). The retry budget is raised5 → 10since a split stream needs several reads.Test
Adds
tests/test_read_metadata.py: feeds a split metadata stream through a fake serial port and asserts the full string survives and the parsedO[0]offset is preserved (it stays0under the old drop-chunks behaviour).Fixes #61. This is the same root cause and approach as #62 — opening it with an accompanying regression test; happy to defer to whichever you prefer to merge.