fix: accumulate serial chunks in _read_metadata to prevent calibration data loss#62
Open
valentinkauf wants to merge 1 commit into
Open
Conversation
jmdevita
added a commit
to jmdevita/ppk2-api-python
that referenced
this pull request
Jul 1, 2026
…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>
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()reads the PPK2 metadata string in a loop but returnsas soon as it finds a single serial buffer chunk containing
"END",discarding all previously received chunks.
The PPK2 firmware sends its full calibration metadata as a multi-line
ASCII string ending with
END. Depending on the OS USB serial driverand buffer sizes, this string arrives split across multiple read buffers.
When that happens, calibration fields that arrived in earlier chunks —
O,S,I,GS,GI,HW, etc. — are silently discarded andremain at their default values of 0.
Concrete impact:
At the lowest measurement range (range 0, ~µA scale),
O[0]is thefactory-measured ADC zero-point offset (typically ~166 ADC counts). With
O[0] = 0(default), the calibration formula:applies no zero-point correction, adding a fixed ~1.8 µA offset to every
sample. This makes the library report systematically wrong values for any
target consuming in the low-µA range, with no warning or error.
The nRF Connect Power Profiler application (the official Nordic reference)
accumulates all chunks:
metadata += chunkuntil"END"appears. Itnever loses calibration data and always reads correctly.
Reproduction
Run on a factory-calibrated PPK2 with a low-current (< 5 µA) load.
Compare the average reading from this library to nRF Connect — the
library reads ~1.8 µA higher.
Enabling verbose logging of the raw metadata string reveals the issue:
fields like
HW:andO0:–O4:are absent from what the libraryreceived, even though the device sent them.
Fix
Accumulate all serial chunks before checking for
"END", matching nRFConnect behaviour. The number of loop iterations is increased from 5 to
10 to accommodate devices that may require more reads.
Tested on
Calibrated: 0)