Conversation
Real-hardware testing over a noisy BLE ELM327 clone kept producing implausible trouble codes (e.g. P0D00) that would come and go across repeated re-reads on the same vehicle with no real fault behind them. parseMultiFrameMessage assembled ISO-TP consecutive frames in receive order with no check that the sequence was complete — a single dropped BLE notification mid-transfer silently shifted every byte after the gap, and extractDataFromFrame's short-data fallback returned the truncated result instead of failing. Both now throw instead of degrading silently: a gap in the 1,2,3.. consecutive-frame sequence, or a final assembly shorter than the length the first frame promised, is a corrupt read, not a partial one to make the best of. Also: `Status` (PID 0101 — MIL + confirmed DTC count + per-monitor readiness) had every field but `dtcCount` non-public despite the struct itself being public, and `StatusTest` wasn't public at all — so a consuming app could physically not read the check-engine-light state or monitor readiness it decoded. Made both fully public.
Broader audit pass after a real-hardware report of "sensors missing again" and a DTC scan failing with an opaque "OBDServiceError error 1" even over WiFi. elm327.swift: getSupportedPIDs' parseResponse took only the FIRST ECU's reply to each supported-PID bitmap request (0100/0120/...) on this vehicle's two-ECU bus. Any PID advertised only by the second ECU was silently absent from OBDInfo.supportedPIDs — and because "first" came from a Dictionary's iteration order, WHICH ecu won wasn't even stable connect to connect, so the missing set could differ each time. Now unions the bitmap across every ECU that answered. wifiManager.swift: processResponse dropped the entire last line whenever it contained the '>' prompt, instead of just the prompt character. A WiFi clone that appends '>' directly onto the last data line with no preceding newline (common on cheap ELM327 emulators) lost that whole line — including real DTC/measurement bytes — while BLE's equivalent path already stripped just the character. Also normalized per-line trimming before the "no data" check, which an untrimmed trailing \r could dodge. Root cause of the opaque error: OBDServiceError, ParserError, CommunicationError, DecodeError conformed to Error but not LocalizedError, and BLEManagerError's CustomStringConvertible.description was never wired to errorDescription — so .localizedDescription on any of them (or anything wrapping them, which is everything OBDService throws) produced only "TypeName error N", the exact opaque message this session hit. Every one of these now surfaces its real message, recursively through underlying errors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Label fix: PIDs 49-4B are the accelerator PEDAL sensor per SAE J1979
("Accelerator pedal position D/E/F"), not the throttle plate — the
library described them as "Absolute throttle position D/E/F" (copy-
pasted from 47/48, which really are throttle position). Same 0-100%
linear decode either way, so no vehicle ever saw a wrong number, only
a wrong label. Verified against the Wikipedia OBD-II PID reference
table rather than assumed.
Legacy protocols (J1850 PWM/VPW, ISO9141-2, ISO14230 KWP — used by
pre-CAN vehicles) had the exact same gap-blind assembly bug this
session already fixed for CAN: the generic multi-frame path checked
only that the lowest order byte was 1, not that the whole sequence
was contiguous, so a dropped frame produced a silently-truncated,
shifted response instead of a failure. Now checks every index.
Adapter init now sends ATAT1 (adaptive timing — Elm's own recommendation
for noisy links, growing the per-command timeout from observed bus
response time instead of a fixed one) and ATCAF1 (CAN auto-formatting —
makes explicit the framing assumption every CAN parser in this package
already depends on implicitly). Both best-effort (not `okResponse`):
an older/cheap clone that doesn't recognize either command must not
fail the whole connection over an optional reliability improvement.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Status/StatusDecoder only ever decoded bytes A and B of PID 0101 (MIL, DTC count, and the 3 "continuous" monitors: misfire/fuel system/ components). Bytes C and D — the 8 "non-continuous" monitors — were never even read, despite being present in every compliant 4-byte response. Those are exactly the ones an emissions/smog inspection readiness check actually depends on: catalyst, evaporative system, oxygen sensor, secondary air, EGR/VVT. Verified the exact bit layout against the SAE J1979-derived reference table (Wikipedia's OBD-II PIDs article) rather than guessing: byte C = availability (1 = available), byte D = completion (0 = complete), same polarity as the existing 3, just at bit offset 16-31 instead of 8-15. The 8 fields use spark- ignition (gasoline) semantics as their canonical meaning; the app layer relabels for compression-ignition (diesel) using the same bit positions' differing meaning where that's reliably documented. detectProtocolManually (the fallback sweep used only when the ELM327's own ATSP0 auto-search fails) tried protocols in raw enum-declaration order — 5 legacy protocols before ever reaching CAN. Since MY2008+ US / mid-2000s+ EU vehicles are essentially all CAN, that spent up to 5 full round-trips (ATSPn + 0100 + timeout each) on protocols that were never going to answer first. Now sweeps CAN (6-9) first, then legacy (1-5), then J1939/user CAN (A-C) last. Also: the generic legacy-protocol multi-frame assembler had the same frame-gap blindness the CAN parser did (fixed last commit) — checked only that the lowest order byte was 1, not that the sequence was contiguous. Same fix applied. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A background audit of the files this session hadn't yet reviewed found the real dominant cause of "sensors missing again" — bigger than the multi-ECU union fixed earlier today: extractSupportedPIDs always labeled bits as PID 01-20 (`index + 1`), regardless of which of the 6 supported-PID getters (0100/0120/0140/ 0160/0180/01A0) produced the bitmap. Every getter after the first reported its bits under the wrong (already-covered) PID numbers, so anything from PID 0x21 up — fuel level, ambient air temp, control module voltage, fuel type, fuel rate, throttle position B-F, and most of what makes a live-sensor dashboard interesting — could never be recognized as supported, on ANY vehicle, single- or multi-ECU. configureSensors' fallback probe never caught this either, since discovery wasn't empty (PIDs 01-20 alone are enough to populate several groups) — it just silently capped there. Fixed by deriving each getter's block offset from its own command string and adding it before formatting the hex PID label. Also fixed, in priority order: - twosComp(_:length:) masked to `length` bits but never subtracted 2^length for the top half of the range — structurally could never return a negative number. Affects EvapPressureDecoder (PID 0132) and every `signed: true` UAS entry (Mode 6 monitor test values). - getStatus()/requestVin() had the same non-deterministic "only .first ECU" bug the multi-ECU PID fix addressed elsewhere — now prefer the engine ECU like scanDTCs already does. - WiFi sendAndReceiveData treated a TCP EOF mid-response the same as a clean, prompt-terminated one — a dropped connection returned whatever partial bytes had arrived as if they were a complete response. New CommunicationError.connectionClosed distinguishes it. - OBDLogger's minimumLogLevel comparison used OSLogType's raw values directly, which aren't ordered by severity (debug=2 sorts above info=1) — inverted the filter so info/warning were dropped by default while debug passed. Added an explicit severity-rank map. - mockManager's (Simulator-only) multi-frame length calculation added raw hex-character count instead of byte count for the consecutive- frame portion, inflating the declared length ~2x — harmless until this session's stricter parser.swift bounds check started throwing on the now-detectably-wrong length for any multi-PID mock response. - protocol_legacy.swift's order-byte path could index frame.data[2] out of bounds and crash on a truncated frame; now throws instead. All 35 existing package tests still pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Real-hardware test against a 2016 Jeep Cherokee (ISO 15765-4, 29-bit ID, protocol 7) came back with zero sensors, no VIN, and every supported-PID query empty — a total regression from the Mustang (11-bit, protocol 6), which works fine. CANParser grouped frames by `Frame.txID`, which masks the address byte with `& 0x07`. That's only meaningful for the 11-bit SAE J1979 functional range (responses 0x7E8-0x7EF, where the low nibble IS the 0-7 ECU index by construction) — it has nothing to do with 29-bit extended addressing, where this vehicle's two ECUs answer from 0x10 and 0x18. Both mask to 0 and collapsed onto the same ECUID bucket, so two independent single-frame replies to one request got merged into a 2-frame group; `Message.init` then tried to decode that as an ISO-TP multi-frame sequence (no `.firstFrame` to anchor on), threw, and silently dropped both ECUs' data — for every request, not just one. Added `Frame.rawAddress` (the untouched address byte) and group by that instead. `txID`/`ECUID` is unchanged and still used for display labels — it just doesn't have to be correct to keep frames from different ECUs apart anymore. All 35 existing tests still pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… can silence this per-command line (a raw os.Logger call can't be gated).
Researched the real addressing conventions rather than assuming the 11-bit fix generalized on its own: - 11-bit (SAE J1979): responses are always 0x7E8-0x7EF — the low nibble directly IS a 0-7 ECU index by construction (ECU assigned 0x7E0-0x7E7 responds at assigned-ID+8). `& 0x07` happens to be exactly right here, and only here. - 29-bit (ISO 15765-4 extended): responses are 0x18DAF1XX where XX is a full, OEM-assigned byte with no fixed range — confirmed this is what broke the Jeep Cherokee (0x10 and 0x18 both mask to 0). - Legacy (ISO 9141-2 / ISO 14230 KWP): per SAE J2178, source address bytes are likewise OEM/tester-assigned, not a small fixed range — same collision risk as 29-bit CAN, just never hit yet on real hardware. `LegacyFrame`/`LegacyParcer` had the identical `txID`-based (`& 0x07`-masked) grouping as the CAN parser did before this session's fix, so applied the same one: added `LegacyFrame.rawAddress` (the untouched source byte) and group by that instead. Also brought the legacy parser's fault-tolerance up to parity with the CAN parser's (which already does this, per its own comment): a single malformed frame or one ECU's frames failing to assemble now drops just that piece instead of throwing and discarding the entire response — `try?` instead of `try` in both `compactMap`s. ECUID's small 4-case enum (engine/transmission/unknown/becm) is intentionally left as a best-effort *label* only, not touched here — OEMs choose their own 29-bit/legacy addresses freely (confirmed via research, not assumed), so there's no universal byte-to-name mapping to encode. Grouping no longer depends on the label being correct; only display does. All 35 tests still pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The actual root cause of the 2016 Jeep Cherokee (protocol 7) reading nothing at all — deeper than the ECU-grouping collision fixed in ecf6e46, which was necessary but not sufficient. Every CAN protocol class passed idBits: 11 to the parser, including the 29-bit variants (7, 9) and J1939 (A). Frame.init prepends "00000" padding for 11-bit frames, whose printed header is only 3 hex chars — applying that to an already-full 29-bit line ("18DAF118...", 8 header chars, 24 chars total) produces a 29-char odd-length hex string. hexBytes walks it two chars at a time from index 0, so every byte boundary lands half a nibble off: 12 real bytes become 14 garbage bytes, the 6...12 size guard rejects the frame, and compactMap silently drops it. Result: EVERY frame from a 29-bit vehicle discarded — no VIN, no supported PIDs, no sensors, no DTCs — while protocol detection still "succeeded" because testProtocol greps the raw text for "41 00" without parsing. Protocols 7/9/A now pass idBits: 29. Added a regression test built from the real capture in the Jeep's connection log (two ECUs, 0x10 and 0x18, single-frame replies to 0100) — it locks in both this fix and the rawAddress grouping fix, and documents the single-frame payload convention (PCI + mode echo dropped, trailing pad kept). 36 tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The parameter was declared and silently discarded (`retries _:`), making every BLE command exactly one 3-second attempt. Two real consequences, both protocol-wide: - One dropped BLE notification failed the whole read instead of re-asking — a per-read coin flip on a noisy in-car link, and a plausible contributor to the sensor flakiness chased earlier this session. - K-line protocol detection (ISO 9141 / KWP 5-baud init runs 5-10 s inside the ELM327 while it prints "SEARCHING...") could never fit a single 3 s window over BLE, while the WiFi transport honors its retries — the same vehicle would connect over WiFi and fail over BLE for no visible reason. Re-sending after a timeout is safe with the exactly-once completion gate: the timed-out attempt's completion was already consumed, and a late reply to attempt N carries the same payload attempt N+1 awaits. "NO DATA" is deliberately NOT retried — it's the adapter's well-formed "vehicle didn't answer", not a comm failure, and re-asking an unsupported PID three times would burn the live-polling cycle's budget for nothing. 36 tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Mode 02 returns the snapshot of live values the ECU stored at the moment an emissions DTC set — it stays in memory until codes are cleared, so it applies to codes already stored, not just ones that appear while connected. Which DTC owns the stored frame is already readable via Mode 01 PID 02 (Mode1.freezeDTC, present since forever). Request format is 02 <PID> <frame#>; the response payload matches the Mode 01 layout with one extra frame-number byte after the PID echo, so each PID reuses its own Mode 01 decoder on payload.dropFirst(2). Unsupported/uncaptured PIDs answer NO DATA (single attempt, no retries — that's a well-formed "not stored") and are omitted from the result rather than failing the whole snapshot. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Found by reading through icanhack.nl's ISO-TP reference rather than re-deriving from scratch: "if the CAN frame is less than 8 bytes, it can be padded — the spec calls for 0xCC, but 0xAA or 0x55 are common in practice." parseSingleFrameMessage returned everything after the PCI+mode-echo bytes with no regard for the PCI's own declared length, so a padded frame's trailing filler bytes rode along into the decoded data. Harmless for the ordinary Mode 01 measurement decoders — they only ever read fixed byte offsets from the front, so trailing bytes are never touched. Not harmless for DTCDecoder (Mode 03/07/0A), which walks the ENTIRE data length two bytes at a time: a non-zero pad byte pairs up with whatever follows it (another pad byte, or nothing, zero-extended) and decodes as a plausible-looking trouble code that was never actually reported by the vehicle. This is a strong candidate for at least some of the intermittent phantom-DTC reports from earlier this session (P0D00 appearing/disappearing across re-reads) — a separate, independent cause from the frame-sequence-gap bug already fixed, since this one doesn't require a dropped BLE packet at all, just an adapter that pads with anything other than zeros. Regression test built directly from this session's own capture: the 29-bit two-ECU test data (real bytes off a 2016 Jeep Cherokee) turns out to end in exactly this kind of non-zero padding (0xAA, 0x00) — updated its expected payloads to the correctly-truncated 5 bytes (PID-echo + 4-byte supported-PID bitmap, matching the 0100 spec exactly) instead of the 6 bytes the old buggy behavior produced. Added a second, minimal test isolating just the padding-strip behavior against a synthetic Mode 03 single-frame response. 37 tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Real-hardware test against the Jeep produced clearly-impossible readings (Engine Run Time = 1,124,073,472 s; Control Module Voltage = 237,559,786 V; Distance w/MIL = 11,184,810 km) alongside a wall of otherwise-normal PIDs timing out repeatedly. Decoding the garbage values to hex was the tell: 1124073472 = 0x43000000 — 0x43 is literally the Mode 3 (GET_DTC) response echo byte — and 11184810 = 0xAAAAAA, the exact non-zero CAN pad byte this session's previous commit just learned to strip from single-frame payloads. Root cause: BLEMessageProcessor's `buffer` was never cleared when a command timed out — only the completion handler slot was reset. A response (or partial response) that arrived just after we gave up waiting for it sat in `buffer` untouched, waiting to be silently prepended onto whichever command's response came next — a stale Mode 3 echo byte or leftover pad byte corrupting a completely unrelated PID's decode. Every command boundary must start from an empty buffer, timeout or not. Fixed by clearing it in the same cancellation handler that resets the completion, and moved every `buffer` touch behind the lock already used for the completion hand-off — the buffer is written from CoreBluetooth's delegate queue and cleared from a Task cancellation handler, two contexts Swift does not guarantee share a queue. Also added missing sanity-check bounds to PIDs this session's own garbage output happened to touch (intake/ambient/oil/manifold temp, catalyst temp x4, MAF, engine run time, distance w/MIL, warm-ups count, control module voltage, direct-inject fuel rail pressure) — they had no declared min/max at all, so the app's own clamp (which treats the exact default 0...100 as "no metadata, don't clamp") never had anything to check against and let any decoded value through regardless of magnitude. This is a safety net on top of the buffer fix, not a replacement for it — every other UAS/temperature PID in the table has the same latent gap and would show the same class of garbage if it hit a similar corruption. 37 tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
On a two-ECU vehicle (e.g. the 2016 Jeep Cherokee, source addresses 0x10 and 0x18), every single-answer read path — the app's per-PID live-sensor polling (OBDService.sendCommand), freeze frame, getStatus, requestVin — picked its response via `.first` on a Dictionary-ordered message list: a different module from one poll to the next. Live values could alternate between two ECUs' answers, and the earlier `.ecu == .engine` preference (getStatus/requestVin) didn't actually disambiguate on 29-bit buses, where the `& 0x07` label mask maps every module to "engine". New `preferredECUMessage(_:pidEcho:)`: 1. keeps only messages whose first payload byte echoes the requested PID (when given) — discards stale/foreign responses outright; 2. of those, takes the lowest raw source address — the primary engine ECM on BOTH addressing schemes (0x7E8 < 0x7E9... on 11-bit, and 0x10 < 0x18... on 29-bit per SAE J2178). MessageProtocol gains `sourceAddress` (the untouched address byte both Message and LegacyMessage already carried per-frame) to make that possible without leaning on the degenerate ECUID label. 37 tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ansport failure — keep it at debug so a parked car polling its ignition probe doesn't flood the console with error-level lines.
Pulled valexa's own continued work on develop since our fork point (a5badae): the getStatus() off-by-one (message.data is [PID,A,B,C,D]; decode() needs .dropFirst() to read A as the MIL byte, not the PID echo) and parseUDS19Data's 3rd DTC byte (keeps distinct ISO 14229 sub-faults of the same base code from collapsing together), plus routing the "NO DATA" case through OBDLogger at debug severity instead of raw error-level logging. One real conflict, in BLEManager.sendCommand: valexa's fix and our own retry-handling addition (this session, e469363) both touched the same catch block for different reasons. Combined rather than picked a side — our retry loop with the noData short-circuit, now also logging that case at debug severity per valexa's fix. elm327.swift's getStatus() auto-merged cleanly with both fixes intact (our preferredECUMessage ECU selection + valexa's dropFirst byte-align). 37 tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
….swift:363) sendCommand(_:retries:) previously discarded the parameter (retries _: Int) and only ever made one attempt. It now actually retries up to retries times, backing off by BLEConstants.retryDelay (0.5s — an existing constant that was already defined but never used, which is a good sign the retry loop was originally intended and just never got wired up). NO DATA still throws immediately without retrying, since that's a legitimate "unsupported PID" answer, not a dropped response — retrying it would just add latency for no benefit. Verified with swift build on the package directly (can't go through build_sim/the app scheme for this one, since per CLAUDE.md the app builds against the remote pinned revision, not this local clone — this change won't reach the app until it's committed, pushed to origin/develop, and the revision bumped in Package.resolved).
…r ATZ succeeded): elm327.swift — adapterInitialization()'s post-ATZ commands (ATE0/ATL0/ATS0/ATH1/ATSP0) now get retries: 3, matching the resilience ATZ already had. Previously only ATZ was retried; a single dropped frame on any command after it failed the whole connection. obd2service.swift — startConnection now retries the full connect+init sequence once automatically if it fails with .adapterConnectionFailed (the transient-transport class we saw), so a one-off USB/BLE hiccup no longer requires you to manually tap Connect again. .noAdapterFound (genuine BLE scan timeout) is deliberately excluded — retrying that would just double an already-full wait. 0100 timeout despite ignition on (attempt 3): the specific 0100 probe that timed out is the best-effort one right after ATSP0 — its result is discarded (try?) and ATDPN + a separately-retried testProtocol 0100 do the real work, so this alone likely wasn't fatal. But since it happened live, I bumped the post-ATSP0 settle delay 1s→2s and gave that probe 2 retries — cuts down the false-negative window where the ELM327 hasn't finished settling onto the bus yet.
…e Wi-Fi interface (requiredInterfaceType = .wifi — this is the fix for "no internet" routing), TCP keepalive detects a dead adapter in ~8 s, fatal socket errors publish a real disconnect, and the ATZ reconnect no longer emits a phantom disconnect mid-handshake.
Probe sends ATI (side-effect-free, also interrupts an in-progress SEARCHING) instead of bare CR, and validity now requires the > prompt on top of the printable-ASCII check. A command timeout sets a needsResync flag; the next send does tcflush(TCIFLUSH) right before writing, so a late reply can't be read as the next command's response. Read errors now log errno + strerror — next time a drop happens, the log will say whether it was a USB detach (ENXIO/EIO) or something else. Changes in elm327.swift: The supported-PID sweep aborts when the transport disconnects instead of burning a full timeout per remaining getter against a closed fd, and setupVehicle now throws rather than overwriting a disconnect with "Connected to Vehicle". The manual protocol sweep tries CAN protocols (6, 7, 8, 9) first with single attempts — worst case for a CAN car drops from minutes to ≤ ~20 s. Test fix: testSetupVehicle was silently skipping its assertion (its catch just printed); it now connects the mock adapter first — required by the new guard — and fails on error.
… with errorDescription = description. Fixes readability at every .localizedDescription site that can carry a BLEManagerError, in one edit.
elm327.swift:567 — per-getter progress dropped to .debug; the catch now logs BLEManagerError.noData ("not supported") at .debug, keeps the disconnect-abort branch at .error, and logs genuine errors via "\(error)" at .error. WiFi's "no data" already returns nil and is skipped silently, so the BLE-specific check is complete.
…nly when present (accepts bare digits), and treats A0/0 as noProtocolFound with an accurate log line instead of the misleading invalid ATDPN value. Failure behavior is unchanged (both still fall through to the manual sweep).
84 call sites used private Logger instances that OBDLogger.minimumLogLevel can't filter, so the consuming app's log-level preference only reached 56. Per-command traffic is now obdDebug, session milestones obdInfo, failures obdError. Warnings triaged one by one: OSLogType raw values are non-monotonic, so .error as a minimum also suppresses obdWarning (.default = 0).
From Xandir150, tested on a 2004 Mustang (11-bit) and a 2016 Jeep Cherokee (29-bit). 29-bit frames went through the 11-bit parser, so none parsed. ECU grouping collided on 29-bit addresses, multi-ECU reads picked a random ECU, supported PIDs above 0x20 never showed, twosComp never returned a negative, and CAN padding leaked into DTCs. Conflicts were the PR's logic against develop's OBDLogger calls. Kept develop's BLE retry loop, which already has the same fix (ac0d338). On top of the PR: EvapPressureDecoder now sign-folds A/B as one 16-bit value. Once twosComp works, folding B on its own skews readings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
isWorthRetrying(_:) splits transport drops (retry) from vehicle-answered-definitively (noProtocolFound, ignitionOff, …) which now propagate — no more double protocol sweep under the app's own attempts: 2 obd2service.swift ATDPN "A" stripped only when something follows, so manual-mode J1939 is detected elm327.swift Supported-PID matching scoped to the getter's own mode, so Mode 6 MID bitmaps stop vouching for Mode 1 PIDs elm327.swift try await Task.sleep in the retry backoff, so a disconnect mid-backoff stops the loop bleManager.swift Retry loop bails on a .cancelled/.failed socket instead of burning attempts against a dead NWConnection wifiManager.swift public init on Status / StatusTest decoders.swift strerror_r instead of strerror MacSerialManager.swift Verification: swift build clean with no new warnings (the two MacSerialManager Sendable warnings are pre-existing, at lines 150 and 210 — nowhere near my edit), swift test 37/37 passing, and build_sim on the App scheme succeeded with zero warnings. I used swift build for the package because XcodeBuildMCP's SwiftPM tools aren't enabled here and it isn't among the commands CLAUDE.md rules out; the app itself went through build_sim as normal. Left alone: the ELM327.connectionState cross-isolation read. The sink is .receive(on: .main) while the new guards read from an arbitrary executor, so the sweep's abort-on-disconnect is a hop late and the read races. Fixing it means deciding ELM327's isolation — bigger than a review follow-up, and documented as such. Fix doc at docs/fix/2026-09-17-bugfix-obd-logging-toggle-and-swiftobd2-pr6-review.md, with a matching ### Fixed bullet in CHANGELOG.md.
Without a restore identifier iOS never relaunched a suspended or terminated app when the dongle powered up, so a drive that started before the app was opened was never recorded. Creating the central manager last: with restoration the system delivers willRestoreState as the first delegate callback, and that handler reaches into peripheralManager, which the old ordering had not built yet. Co-Authored-By: Claude Opus 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
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.
What
25 commits of correctness fixes to the OBD transport, parser and decoder layers, plus a
logging cleanup. Every fix here came out of chasing a concrete misbehaviour on real
hardware — a 2016 Jeep Cherokee KL (ISO 15765-4 29-bit), and BLE/WiFi/USB-serial adapters
on a Jaguar I-Pace and XF.
Why it matters
Three of these were silently returning wrong data rather than failing:
ISO_15765_4_29bit_500k/250kandSAE_J1939passedidBits: 11to the parser, which prepends"00000"padding meant for3-hex-char 11-bit headers. On an already-full 8-char 29-bit header that makes the hex
string odd-length, shifts every byte boundary by half a nibble, and inflates a 12-byte
frame to 14 garbage bytes — which the size guard then rejects. No VIN, no supported PIDs,
no sensors. Protocol detection still reported success, because it greps the raw text for
41 00without parsing.txID, whose& 0x07mask onlymeans anything in the 11-bit SAE J1979 range
0x7E8-0x7EF. On a 29-bit bus, sourceaddresses
0x10and0x18both mask to0— two modules' single-frame replies mergedinto one group that
Message.initthen tried to decode as multi-frame ISO-TP, failingand discarding both.
BLEMessageProcessorreset its completion slot on timeout but never cleared
buffer, so a reply arriving justafter we gave up got prepended to the next command's response. Symptom: Engine Run Time
= 1,124,073,472 s (
0x43000000— a Mode 3 echo byte) and Distance w/MIL = 11,184,810 km(
0xAAAAAA— the CAN pad byte).Changes by area
Parser / protocols
txID(CAN and legacy).idBits: 29for the 29-bit CAN protocols and J1939.harmless for fixed-offset PID decoders, but
DTCDecoderwalks the full length in 2-bytestrides and decoded pad bytes as phantom trouble codes.
returning truncated-but-plausible data.
try?), a bounds guard before the order-byte access,and full order-byte sequence validation.
MessageProtocol.sourceAddress+preferredECUMessage(_:pidEcho:)for deterministicECU selection — Dictionary-order
.firstpicked a different module from one poll to thenext on a two-ECU vehicle, making values flicker.
Decoders / commands
must be added to the bit index. Without it every vehicle's sensor list was capped at the
first 32 standard PIDs — fuel level, ambient temp, control module voltage and fuel rate
could never be recognised.
Status/StatusTestfields public so consumers can build inspection-readiness UI.twosCompactually folds the sign now (value & maskwas a no-op);EvapPressureDecoderfolds over the 16-bit pair rather than per byte.
(they're a different physical sensor from throttle plate — decoded value was always right,
only the label was wrong).
requestFreezeFrame).Transport
retriesparameter (it was declared and discarded), clear the buffer ontimeout, lock every buffer access, and keep NO DATA at debug — it's a routine reply, not a
transport failure.
internet), TCP keepalive to detect a dead adapter in ~8s, publish a real disconnect on
fatal socket errors, treat mid-response EOF as
connectionClosedinstead of success, andstop the ATZ reconnect emitting a phantom disconnect mid-handshake.
ATIrather than a bare CR (a lone CR is "repeat last command" onan ELM327, so the probe re-ran whatever the previous session left in the buffer), require
the
>prompt, resync after a timeout so a late reply can't be read as the next command'sresponse, and report
errnoon read errors.Logging / diagnostics
OBDLoggerso the consuming app's log-level preference canactually gate it (raw
os.Loggercalls can't be).OBDLogger's level filter:OSLogTyperaw values aren't ordered by severity(
.default=0,.info=1,.debug=2), so comparing.rawValueinverted the filter.LocalizedErroronBLEManagerError,CommunicationError,DecodeError,ParserErrorso
.localizedDescriptionshows the real message instead of "… error 5."ECUID: Sendablefor consumers building with strict concurrency.Tests
Two regression tests in
test_protocol_can.swift: single-frame padding truncation, and areal two-ECU 29-bit capture from the Jeep that locks both the
idBitsand the ECU-groupingfixes.
elm327Testnow establishes the mock adapter connection first (matching the realflow) and fails loudly instead of printing.
Downstream note
OBDLogger's corrected level filter changes whatminimumLogLevelmeans. A consumersetting
.defaultto mean "show everything" was relying on the old inverted comparison andshould now set
.debug.