Repository navigation
fix(mf4): read packed error-frame payloads without losing byte order - #2115
Open
timothyanderson096-ocdealcheck wants to merge 1 commit into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Thank you for the fix! |
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.
Summary of Changes
MF4 CAN error frames whose DataBytes field is a packed scalar currently raise
IndexError: invalid index to scalar variablewhen the reader slices that scalar.Serialize a one-record field slice before applying DataLength in bytes. The slice preserves the field's declared byte order and keeps extraction limited to the current record. Ordinary byte arrays and existing message metadata retain their meaning.
Add eight synthetic MF4 v4.10 save/reopen cases: the reported scalar value 16777216, little/big-endian scalars, zero/one-byte lengths, byte arrays and 12/64-byte CAN FD payloads. Each case contains two distinct records and checks payload, timestamp, ID, channel, direction and flags. Add the required news fragment.
Related Issues / Pull Requests
Type of Change
Checklist
tox).Additional Notes
Tested clean public head:
42ce8dbaeb46245b48ab10a0abfd44947496e00c, tree7018d81a940c34cb8f75d88e6e005c817e5598c2, on unchanged baseb4f82abede25ff83376be793a2935c41f81c3869.The public head has the identical source tree as the original candidate
ceb00b12cd640723aae7ac17b43a900980c95795; publication changed Git commit metadata only.Fresh checks on 26 October 2026, Linux / CPython 3.13.16, NumPy 2.5.4, asammdf 8.8.28:
tox -e py313 -- test/logformats_test.py::TestMF4FileFormat: 12 passed, one existing append-mode skip. Output and environments were kept outside the checkout; source remained clean and unchanged.Earlier configured checks on 25 October, on this same source: Black/Ruff/Pylint, five mypy static targets, strict Sphinx HTML and 39 doctests passed. The full suite had 511 passed, 29 subtests passed, 98 skipped and 28 multicast-initialization failures; the same 28 failure identities occurred on the unchanged base in that environment. Those full/static/docs checks were not rerun in this focused refresh. No assertions or skips were changed to claim all-green tox; the checkbox remains unchecked.
The fixtures are synthetic asammdf files, not the unavailable original Kvaser capture. Tested payload schemas are packed u1/u4 and u1 arrays; arbitrary malformed schemas and native big-endian hardware are not established. Returned data/dlc follow DataLength; the fixture's separate stored DLC field is not an additional behavior claim. The full OS/interpreter and physical-adapter matrix remains for CI/maintainers.
Prepared with AI assistance and independently inspected by another agent on 25 October; that inspection is not maintainer approval.
Timothy Anderson — Spotted Ridge Engineering / OC Labs · GDN (Global Decision Network)
timothyanderson096@gmail.com · Engineering evidence