Fix #9: DTC/freeze-frame parsing (phantom codes, Mode 02, hex frame index)
- parse_dtcs CAN branch is now message-aware: each ECU reply '<svc> <count> <pairs>' has its header stripped per-message, instead of flattening all lines and stripping svc+count once. With multiple ECUs the old code ate the second header as a DTC pair -> phantom codes. Critically, it does NOT blind-scan for svc (0x43 is a legal DTC first byte: C03xx) — a numbered ISO-TP continuation is distinguished by its 'N:' frame-index prefix, not by value. - _line_bytes strips hex frame indices A:-F: (ISO-TP index cycles 0-F), not just 0-9, so consecutive frames past the 10th aren't dropped. - read_freeze_frame sends the correct '020200' (svc 02, PID 02, frame 00) and skips SID+PID+frame (+3), fixing the off-by-one that mis-read the freeze DTC. - tests/test_dtc_parse.py: single-frame, multi-ECU (no phantom), numbered multiframe with a real C03xx continuation, hex index, non-CAN legacy. Closes #9 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yT89n4zR4qbrySoSiEyZs
This commit is contained in:
+26
-10
@@ -23,11 +23,14 @@ def decode_dtc(b1, b2):
|
|||||||
return f"{_LETTER[(b1 >> 6) & 3]}{(b1 >> 4) & 3}{b1 & 0xF:X}{b2:02X}"
|
return f"{_LETTER[(b1 >> 6) & 3]}{(b1 >> 4) & 3}{b1 & 0xF:X}{b2:02X}"
|
||||||
|
|
||||||
|
|
||||||
|
_HEX = "0123456789ABCDEFabcdef"
|
||||||
|
|
||||||
|
|
||||||
def _line_bytes(ln):
|
def _line_bytes(ln):
|
||||||
ln = ln.replace(" ", "")
|
ln = ln.replace(" ", "")
|
||||||
if len(ln) >= 2 and ln[1] == ":" and ln[0] in "0123456789":
|
if len(ln) >= 2 and ln[1] == ":" and ln[0] in _HEX:
|
||||||
ln = ln[2:] # drop CAN multiframe index "N:"
|
ln = ln[2:] # drop CAN multiframe index "N:" (0-F, cycles)
|
||||||
if not ln or any(c not in "0123456789ABCDEFabcdef" for c in ln):
|
if not ln or any(c not in _HEX for c in ln):
|
||||||
return []
|
return []
|
||||||
return [int(ln[i:i + 2], 16) for i in range(0, len(ln) - 1, 2)]
|
return [int(ln[i:i + 2], 16) for i in range(0, len(ln) - 1, 2)]
|
||||||
|
|
||||||
@@ -35,11 +38,24 @@ def _line_bytes(ln):
|
|||||||
def parse_dtcs(lines, svc, is_can):
|
def parse_dtcs(lines, svc, is_can):
|
||||||
pairs = []
|
pairs = []
|
||||||
if is_can:
|
if is_can:
|
||||||
data = [b for ln in lines for b in _line_bytes(ln)]
|
# Message-aware: multiple ECUs each reply "<svc> <count> <pairs...>", and
|
||||||
if svc in data:
|
# a DTC's own first byte can equal svc (0x43 == C03xx), so we must NOT
|
||||||
data = data[data.index(svc) + 1:]
|
# blind-scan the flattened stream for svc. A line whose payload starts
|
||||||
data = data[1:] if data else data
|
# with svc begins a new ECU message (drop svc + count byte); an ISO-TP
|
||||||
pairs = data
|
# numbered continuation "N:" (N>=1) appends raw pairs to the current one.
|
||||||
|
started = False
|
||||||
|
for ln in lines:
|
||||||
|
raw = ln.replace(" ", "")
|
||||||
|
cont = len(raw) >= 2 and raw[1] == ":" and raw[0] in _HEX and raw[0] != "0"
|
||||||
|
b = _line_bytes(ln)
|
||||||
|
if not b:
|
||||||
|
continue
|
||||||
|
if not cont and b[0] == svc: # header of an ECU message: svc + count
|
||||||
|
pairs.extend(b[2:])
|
||||||
|
started = True
|
||||||
|
elif started: # continuation / pairs of current message
|
||||||
|
pairs.extend(b)
|
||||||
|
# else: bytes before any header (ISO-TP length line, stray) -> ignore
|
||||||
else:
|
else:
|
||||||
for ln in lines:
|
for ln in lines:
|
||||||
data = _line_bytes(ln)
|
data = _line_bytes(ln)
|
||||||
@@ -211,9 +227,9 @@ class ElmLink:
|
|||||||
"""Mode 02: the DTC that set the freeze frame + the standard PID snapshot."""
|
"""Mode 02: the DTC that set the freeze frame + the standard PID snapshot."""
|
||||||
from . import obdservices as svc
|
from . import obdservices as svc
|
||||||
out = {"dtc": None, "values": []}
|
out = {"dtc": None, "values": []}
|
||||||
d = self._bytes(self.cmd("0202", timeout=timeout))
|
d = self._bytes(self.cmd("020200", timeout=timeout)) # svc 02, PID 02, frame 00
|
||||||
if 0x42 in d:
|
if 0x42 in d:
|
||||||
r = d[d.index(0x42) + 2:] # after '42 02'
|
r = d[d.index(0x42) + 3:] # skip 42 (SID) 02 (PID) 00 (frame)
|
||||||
if len(r) >= 2 and (r[0] or r[1]):
|
if len(r) >= 2 and (r[0] or r[1]):
|
||||||
out["dtc"] = decode_dtc(r[0], r[1])
|
out["dtc"] = decode_dtc(r[0], r[1])
|
||||||
for name, pid, nbytes, dec, unit in svc.FREEZE_PIDS:
|
for name, pid, nbytes, dec, unit in svc.FREEZE_PIDS:
|
||||||
|
|||||||
@@ -0,0 +1,63 @@
|
|||||||
|
"""DTC parsing correctness (CAN multi-ECU, ISO-TP frame indices, single-frame)."""
|
||||||
|
import os
|
||||||
|
import sys
|
||||||
|
|
||||||
|
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
|
||||||
|
|
||||||
|
from obdcore.link import parse_dtcs, _line_bytes, decode_dtc
|
||||||
|
|
||||||
|
|
||||||
|
def test_single_frame_can():
|
||||||
|
# 43 02 <P0133> <P0420>
|
||||||
|
lines = ["43 02 01 33 04 20"]
|
||||||
|
assert parse_dtcs(lines, 0x43, True) == ["P0133", "P0420"]
|
||||||
|
print(" single-frame CAN: OK")
|
||||||
|
|
||||||
|
|
||||||
|
def test_multi_ecu_no_phantom_codes():
|
||||||
|
# Two ECUs each reply "43 <count> ...". The old flatten-and-strip-once code
|
||||||
|
# ate the second header (43 01) as a DTC pair -> phantom code. Each header
|
||||||
|
# must be stripped per-message.
|
||||||
|
lines = ["43 01 01 33", "43 01 04 20"]
|
||||||
|
got = parse_dtcs(lines, 0x43, True)
|
||||||
|
assert got == ["P0133", "P0420"], got
|
||||||
|
assert not any(d.startswith("C0301") or "4301" in d for d in got)
|
||||||
|
print(f" multi-ECU CAN, no phantom codes: {got}: OK")
|
||||||
|
|
||||||
|
|
||||||
|
def test_iso_tp_numbered_frames_hex_index():
|
||||||
|
# Multiframe with numbered continuations, including hex indices A:..F:.
|
||||||
|
# First frame "0:" starts with 43 08; continuations carry raw pairs. A
|
||||||
|
# continuation starting with 0x43 (a real C03xx code) must NOT be mistaken
|
||||||
|
# for a new header because it is a numbered continuation.
|
||||||
|
lines = [
|
||||||
|
"0: 43 08 01 33 04 20",
|
||||||
|
"1: 05 67 07 E4 43 45", # ...last pair 43 45 == C0345 (real code, starts 0x43)
|
||||||
|
"2: 09 96 00 00 00 00",
|
||||||
|
]
|
||||||
|
got = parse_dtcs(lines, 0x43, True)
|
||||||
|
assert got == ["P0133", "P0420", "P0567", "P07E4", "C0345", "P0996"], got
|
||||||
|
print(f" numbered multiframe + C03xx continuation not misread: {got}: OK")
|
||||||
|
|
||||||
|
|
||||||
|
def test_hex_frame_index_stripped():
|
||||||
|
assert _line_bytes("A: 11 22") == [0x11, 0x22] # hex index A: now stripped
|
||||||
|
assert _line_bytes("F:1122") == [0x11, 0x22]
|
||||||
|
print(" _line_bytes strips hex ISO-TP frame indices A:-F:: OK")
|
||||||
|
|
||||||
|
|
||||||
|
def test_legacy_non_can_still_works():
|
||||||
|
# 43 header per line (non-CAN)
|
||||||
|
lines = ["43 01 33 00 00", "43 04 20 00 00"]
|
||||||
|
got = parse_dtcs(lines, 0x43, False)
|
||||||
|
assert "P0133" in got and "P0420" in got, got
|
||||||
|
print(f" non-CAN legacy parse intact: {got}: OK")
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
test_single_frame_can()
|
||||||
|
test_multi_ecu_no_phantom_codes()
|
||||||
|
test_iso_tp_numbered_frames_hex_index()
|
||||||
|
test_hex_frame_index_stripped()
|
||||||
|
test_legacy_non_can_still_works()
|
||||||
|
print("\nALL DTC PARSE TESTS PASS")
|
||||||
Reference in New Issue
Block a user