fix(j1939): decode by source address (#13), split shared messages, stop 11-bit ID cross-matching - #14
Merged
Merged
Conversation
When a database defines a J1939 PGN at more than one source address (e.g. DM1_239 at SA 0xEF and DM1_243 at SA 0xF3), a frame of that PGN now only decodes into the message for its own SA. Frames from an SA the database does not define stay undecoded instead of borrowing the first node's message, which merged DM1 from every ECU into one signal. A PGN the database defines at a single SA keeps the any-SA fallback, since that SA is usually a placeholder. The candidate lookup moves to DBCDecoder.candidates_for(), which the vectorized decoder and the debug inspector now call instead of keeping their own copies of the rule. Refs #13 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…order A database often defines a J1939 message once at a placeholder source address while several ECUs send it. In a truck log, CCVS came from four nodes: two with real brake/clutch switch states and two sending 3 (not available). All four decoded into one series, and because the bulk loaders insert one frame ID at a time, the series held four back-to-back sweeps of the recording instead of one time-ordered trace. Zoomed out, the plot drew a solid box between 0 and 3; zoomed in, pyqtgraph's binary-search clipping picked one sweep and showed a flat 3. - A message that frames from more than one source address decode into now gets one series per sender, named "CCVS [SA 0x17]". A message with a single sender keeps its database name. - SignalStore flags any series whose bulk insert starts before its last sample, and sort_merged_series() reorders only those series once loading finishes. All three bulk load paths call it, so one sender on two priorities still loads as one time-ordered series. - The MF4 reader applies the same rules to asammdf's extraction. asammdf does its own J1939 matching and gives each frame ID its own group. The reader now reads the frame ID from each group's comment and drops groups our decoder would not match, which brings the #13 rule to MF4 files. It also splits multi-sender messages the same way instead of merging the groups back together. Refs #13 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The decoder indexed every message under its ID masked to 11 bits and looked every frame up the same way, so the two ID types cross-matched: extended J1939 frame 0x18FECA13 became a candidate for a standard message 0x213 (and, listed first, won over its own DM1 message), and a standard frame 0x2EF decoded as the extended message 0x18FECAEF. Messages are now indexed by ID type. A standard frame only matches standard messages and an extended frame only extended ones; an ID that still carries the DBC's extended flag bit (0x80000000) keeps matching. The per-frame candidate cache is keyed on the ID type too, so a standard and an extended frame with the same ID no longer share an entry. Co-Authored-By: Claude Opus 5.5 <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
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.
Fixes #13.
Problem
#13: when a DBC defines a J1939 PGN at several source addresses (DM1_239 at SA 0xEF, DM1_243 at SA 0xF3), the decoder still added every message with that PGN as a fallback candidate and used the first one. DM1 frames from every other ECU (VCU 0x13, OBC 0xE6, …) decoded into
xxx_239as well, so one node's DTC signal mixed in DTCs from other nodes. MF4 files had the same bug, because asammdf does its own PGN matching.Found while testing #13: when a DBC defines a message once at a placeholder SA and several ECUs send it, those ECUs were merged into one series. The bulk loaders insert one frame ID at a time, so the merged series held one back-to-back sweep of the recording per sender, out of time order. In a log,
xxx:Switchcomes from four nodes: two send real switch states and two send 3 (not available). Zoomed out it drew a solid box between 0 and 3. Zoomed in, pyqtgraph's binary-search clipping picked one sweep and showed a flat 3.Found in the same lookup: the decoder also indexed and looked up every ID masked to 11 bits, so standard and extended IDs cross-matched. Extended frame
0x18FECA13got the candidates[Std 0x213, DM1], and the loader used the standard message. Standard frame0x2EFdecoded as the extended message0x18FECAEF.Changes
DBCDecoder.candidates_for). If a PGN is defined at more than one SA, a frame only matches the message for its own SA. An SA the database doesn't define stays undecoded. A PGN defined at a single SA keeps the any-SA fallback, because that SA is usually a placeholder. The vectorized decoder and the debug inspector now call this one method instead of keeping their own copies of the rule.CxVS [SA 0x17]. A single sender keeps the database name.SignalStoreflags any series whose bulk insert starts before its last sample.sort_merged_series()reorders only those series after loading. All three bulk load paths call it, so one sender on two priorities still loads as one ordered series.0x80000000extended flag keeps matching. The candidate cache key now includes the ID type. The bulk ASC/BLF loader still infers the type from the ID (anything above0x7FFcounts as extended). That covers J1939, but an extended frame with an ID of0x7FFor below is still treated as standard there, as it was before this PR.Testing
tests/test_dbc_decoder.py: 8 decoder tests for [BUG] J1939 Decoding ignores Source Address (SA), causing signal overlap on PGN FECA (DM1) #13 (undefined SAs stay undecoded, a frame matches its own SA's message, priority is ignored, the single-SA fallback is kept, the vectorized decoder delegates).tests/test_j1939_senders.py(new): 8 end-to-endLoadWorkertests on generated ASC and MF4 files: split per sender, single sender keeps its name, [BUG] J1939 Decoding ignores Source Address (SA), causing signal overlap on PGN FECA (DM1) #13 on both formats, one sender on two priorities stays in time order, and the store sort.tests/test_dbc_decoder.py: 4 tests for ID-type matching. Three of them failed before the fix (extended matching standard on the low bits, standard matching extended, and one ID sent as both types). The fourth checks that each type still matches its own messages.J1939.dbc: now loads as[SA 0x00],[SA 0x17],[SA 0x21]and[SA 0x31], each with 1958 samples in time order.Note
In logs where a message now splits, saved configs or formulas that name the merged series will no longer find it. Nothing renames old references to the new names.
🤖 Generated with Claude Code