docs(v0.13.0): cycle plan — Real Reads, Real Long (6 slices, 2 Tier B) - #150
Conversation
Slice 1 of v0.13.0: cycle plan + ROADMAP cycle header.
The plan's key insight is that ISO-TP multi-frame is ALREADY
implemented (src-tauri/src/transport/isotp.rs, 430 lines + 10
unit tests) since v0.6.0 — what's been blocking F/G long reads
and E-series CAS multi-frame is the WIRE-UP into the production
connect() paths, not the implementation itself. v0.13.0 is
'plug it in' rather than 'implement it'.
Cycle shape (1 Tier B wire-up + 3 Tier A frontend + 1 Tier A
+ Tier B small fix + 1 docs):
- Slice 1 (Tier A docs): this PR — plan + ROADMAP header.
- Slice 2 (Tier B): wire IsoTpTransport into kdcan.rs::connect()
and enet.rs::connect(). The impl is done; the wire-up is small
(two connect() paths switch from bare Transport to
IsoTpTransport<RawBus>).
- Slice 3 (Tier A): VIN-via-ISO-TP regression test using the
simulator's multi_frame personality. Catches a missing wire-up
before the Tier B lands.
- Slice 4 (Tier A + Tier B): KWP2000 slow-module timeout fix.
Configurable deadline + per-target override table in
kdcan.rs::request. Small Rust change but the protected-path
exposure is real (per CLAUDE.md §1).
- Slice 5 (Tier A): E-series CAN broadcast frame decoder
(0x0AA RPM, 0x1D0 coolant, 0x545 oil temp). Pure frontend —
the bytes are already on the bus from the transport, the
renderer just doesn't decode them. Renders as a Live Gauges
panel using the existing src/js/gauges.js widget.
- Slice 6 (Tier A docs): docs/validation/multi-frame.md harness
doc, same shape as testplans.md / service-functions.md /
dtc-history.md.
Honest ceiling (what we will NOT do this cycle):
- Re-implement ISO-TP (the 10 existing unit tests cover it).
- ENET multi-segment changes.
- Blocker tests for the wiring (the wire-up is small enough
that smoke tests suffice).
- Cross-cutting connect() refactors.
- New ISO-TP features (extended addressing, mixed addressing,
TP 2.0).
Tier A — docs only. Self-merge when CI green.
|
🤖 Doc-only change — auto-merge enabled. Will merge once required checks pass. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f413f619e7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - `src-tauri/src/transport/kdcan.rs::connect(...)` currently returns | ||
| a bare `KdcanTransport`. Change the return shape (or wrap it) so | ||
| the trait-object returned is `Box<dyn Transport>` over an | ||
| `IsoTpTransport<KdcanCanBus>` instead of `KdcanTransport`. | ||
| - Same for `enet.rs`. The ENET path already constructs an | ||
| `IsoTpTransport` internally — verify the wiring and surface the |
There was a problem hiding this comment.
In the checked production transport stack, neither module has this connect() or a raw CanBus: transport::open constructs them, K+DCAN already receives complete length-prefixed KWP messages, and ENET receives gateway-reassembled HSFZ payloads. isotp.rs explicitly reserves IsoTpTransport for future raw 8-byte CAN transports, so KdcanCanBus does not exist and wrapping these payload-level transports would either be impossible or reinterpret service bytes as ISO-TP PCI bytes, breaking requests rather than fixing truncation. The central wire-up slice therefore needs to be removed or redesigned around a genuine raw-CAN transport.
Useful? React with 👍 / 👎.
| - `kdcan.rs::request` hardcodes 1000ms. Replace with a configurable | ||
| per-target deadline — default 1000ms, but CIC (0x01), CAS (0x40), | ||
| and other known-slow modules get 3000ms by default. |
There was a problem hiding this comment.
For the proposed slow-module scenario, this remedy directly conflicts with the hardware invariant in CLAUDE.md: K+DCAN slow reads must not be fixed by inflating software timeouts because sequential-read latency is controlled by the FTDI VCP latency timer; the app should detect or alert on that setting. Defining 3000 ms target overrides here would mask misconfigured hardware and make every absent CIC/CAS request wait longer, so the slice should diagnose the latency setting or otherwise establish a distinct protocol-level cause before changing deadlines.
Useful? React with 👍 / 👎.
| - **Tier B for the wire-up only.** The actual change is small | ||
| (two `connect()` paths switch from raw `Transport` to | ||
| `IsoTpTransport<RawBus>`); the file count is low but the | ||
| protected-path exposure is real. Single Tier B PR, gated behind | ||
| human merge per CLAUDE.md §1. |
There was a problem hiding this comment.
The timeout slice explicitly modifies src-tauri/src/transport/kdcan.rs, but CLAUDE.md classifies every file under src-tauri/src/transport/** as Tier B and requires human merge. Saying the wire-up is the only Tier B work, while later listing the timeout change among Tier A PRs, creates a handoff that can cause a safety-relevant transport timing change to be merged autonomously; classify both transport-touching slices consistently as Tier B.
Useful? React with 👍 / 👎.
| The CAN broadcast frames are already on the bus; the transport | ||
| records them in `TrafficEntry`; the renderer just doesn't decode | ||
| them. This slice adds a Live Gauges panel that: | ||
|
|
||
| - Subscribes to the existing traffic log filtered to the broadcast | ||
| IDs (0x0AA, 0x1D0, 0x545). |
There was a problem hiding this comment.
The existing TrafficEntry log records only diagnostic Transport::request pairs (target, request payload, and response payload); it contains neither unsolicited frames nor CAN IDs, and the repository still lists CAN listener mode as unimplemented. Therefore filtering this log for 0x0AA/0x1D0/0x545 can never produce broadcast gauge data on an E-series car. This slice requires a backend/raw-bus listener and a surfaced frame stream before any pure-frontend decoder can work.
Useful? React with 👍 / 👎.
| - A new test in `src/js/test/` (or extend an existing one) that | ||
| exercises the full VIN path: invoke `read_vehicle_info` against | ||
| the simulator with `vin_multi_frame` personality, assert the | ||
| returned VIN matches `WBA…` (the sim's stock VIN). | ||
| - If this fails on the wire-up branch, the wire-up didn't take; this | ||
| test catches that *before* the Tier B lands. | ||
|
|
||
| The test is frontend-only because the simulator is JS-side; the | ||
| Rust side is already covered by `isotp.rs::tests::vin_read_survives_multi_frame_segmentation`. |
There was a problem hiding this comment.
The simulator is Rust-side (SimTransport/SimCanBus), not JS-side, and repo-wide inspection finds no vin_multi_frame personality or tests/transport/isotp.rs; a Node frontend test also cannot invoke the real Tauri read_vehicle_info command without a backend harness. As written, this PR cannot exercise the claimed production path or gate the Tier B change, so it should use a Rust integration test around transport::open/the command state or explicitly add a runnable Tauri integration harness.
Useful? React with 👍 / 👎.
| - `kdcan.rs::request` hardcodes 1000ms. Replace with a configurable | ||
| per-target deadline — default 1000ms, but CIC (0x01), CAS (0x40), | ||
| and other known-slow modules get 3000ms by default. |
There was a problem hiding this comment.
The checked ECU address table identifies target 0x01 as ACSM, the crash-safety/airbag module, not CIC. Implementing this table literally would apply the longer timeout to an unrelated safety module while the intended head unit still uses the 1000 ms default, so CIC reads would continue timing out and absent ACSM probes would become slower. Resolve the real CIC address from the supported addressing model before defining any per-target behavior.
Useful? React with 👍 / 👎.
| | Cycle plan + ROADMAP v0.13.0 header | 🔲 Open | A | This PR lands the plan on `main`. Docs-only. | | ||
| | Wire `IsoTpTransport` into `kdcan.rs::connect()` and `enet.rs::connect()` | 🔲 Open | **B** | Touches `src-tauri/src/transport/{kdcan,enet}.rs`. The `IsoTpTransport` impl is already done + tested — this slice just plugs it into the production path. Flag `transport/` at the top of the PR body, wait for human merge. | | ||
| | VIN-via-ISO-TP regression test | 🔲 Open | A | New frontend test using the simulator's `multi_frame` personality. Catches a missing wire-up *before* the Tier B lands. | | ||
| | KWP2000 slow-module timeout fix | 🔲 Open | A + **B** | Configurable deadline + per-target override table. Touches `src-tauri/src/transport/kdcan.rs::request`. The actual change is small but the protected-path exposure is real. | |
There was a problem hiding this comment.
This newly schedules the KWP2000 timeout fix as a v0.13.0 slice, but the same ROADMAP.md still advertises it under “Ready to Claim” and explicitly says those entries do not conflict with the active cycle. Contributors following that section can therefore start a second implementation concurrently with slice 4, defeating the dispatch plan; move or remove the Ready-to-Claim row when promoting the work into this cycle.
Useful? React with 👍 / 👎.
The first draft of the v0.13.0 plan (PR #150) claimed that ISO-TP multi-frame was implemented but not wired into the production connect() paths, and proposed slice 2 as the wire-up PR. **That premise was wrong.** On re-grounding against the code, the existing isotp.rs module doc-comment is explicit: > K+DCAN cable (kdcan.rs): never reaches the PC. The cable is an > FTDI bridge whose firmware terminates the CAN side; the PC > speaks complete BMW-framed messages... > ENET (enet.rs): never reaches the PC. The ZGW gateway terminates > CAN-side ISO-TP and delivers one reassembled diagnostic payload... So KdcanTransport and EnetTransport already deliver complete payloads — the gateways (FTDI chip, ZGW) handle ISO-TP termination upstream of the app. The 10 existing isotp.rs unit tests verify the implementation works; the integration point is the IsoTpTransport for FUTURE raw-CAN transports (SocketCAN, OBDLink STN, future DoIP socket) that don't exist yet. This commit replaces the v0.13.0 plan with one that drops the ISO-TP wire-up premise and rescopes the cycle to the slices that are real user-visible wins: - Slice 1 (Tier A docs, this PR): the corrected plan + ROADMAP cycle table update. - Slice 2 (Tier A + Tier B): KWP2000 slow-module timeout fix in src-tauri/src/transport/kdcan.rs. The hardcoded 1000ms deadline times out on real E-series CIC/CAS modules. Per-target override table with CIC/CAS at 3000ms. Small but real bug. - Slice 3 (Tier A frontend): E-series CAN broadcast frame decoder (0x0AA RPM, 0x1D0 coolant, 0x545 oil temp). Pure frontend — the bytes are already on the bus from the transport. Renders as a Live Gauges panel. - Slice 4 (Tier A docs): docs/validation/multi-frame.md explaining what isotp.rs is for and why the production stack doesn't need it. 4 slices total (was 6), 1 Tier B (was 2). The 'Ready to Claim' line below the v0.13.0 cycle table still references the new cycle — no further stale-reference cleanup needed beyond what this PR does. Tier A — docs only. Self-merge when CI green.
* docs(v0.13.0): cycle plan — Real Reads, Real Long (6 slices, 2 Tier B)
Slice 1 of v0.13.0: cycle plan + ROADMAP cycle header.
The plan's key insight is that ISO-TP multi-frame is ALREADY
implemented (src-tauri/src/transport/isotp.rs, 430 lines + 10
unit tests) since v0.6.0 — what's been blocking F/G long reads
and E-series CAS multi-frame is the WIRE-UP into the production
connect() paths, not the implementation itself. v0.13.0 is
'plug it in' rather than 'implement it'.
Cycle shape (1 Tier B wire-up + 3 Tier A frontend + 1 Tier A
+ Tier B small fix + 1 docs):
- Slice 1 (Tier A docs): this PR — plan + ROADMAP header.
- Slice 2 (Tier B): wire IsoTpTransport into kdcan.rs::connect()
and enet.rs::connect(). The impl is done; the wire-up is small
(two connect() paths switch from bare Transport to
IsoTpTransport<RawBus>).
- Slice 3 (Tier A): VIN-via-ISO-TP regression test using the
simulator's multi_frame personality. Catches a missing wire-up
before the Tier B lands.
- Slice 4 (Tier A + Tier B): KWP2000 slow-module timeout fix.
Configurable deadline + per-target override table in
kdcan.rs::request. Small Rust change but the protected-path
exposure is real (per CLAUDE.md §1).
- Slice 5 (Tier A): E-series CAN broadcast frame decoder
(0x0AA RPM, 0x1D0 coolant, 0x545 oil temp). Pure frontend —
the bytes are already on the bus from the transport, the
renderer just doesn't decode them. Renders as a Live Gauges
panel using the existing src/js/gauges.js widget.
- Slice 6 (Tier A docs): docs/validation/multi-frame.md harness
doc, same shape as testplans.md / service-functions.md /
dtc-history.md.
Honest ceiling (what we will NOT do this cycle):
- Re-implement ISO-TP (the 10 existing unit tests cover it).
- ENET multi-segment changes.
- Blocker tests for the wiring (the wire-up is small enough
that smoke tests suffice).
- Cross-cutting connect() refactors.
- New ISO-TP features (extended addressing, mixed addressing,
TP 2.0).
Tier A — docs only. Self-merge when CI green.
* docs(v0.13.0): correct plan — drop the ISO-TP wire-up premise
The first draft of the v0.13.0 plan (PR #150) claimed that ISO-TP
multi-frame was implemented but not wired into the production
connect() paths, and proposed slice 2 as the wire-up PR.
**That premise was wrong.** On re-grounding against the code, the
existing isotp.rs module doc-comment is explicit:
> K+DCAN cable (kdcan.rs): never reaches the PC. The cable is an
> FTDI bridge whose firmware terminates the CAN side; the PC
> speaks complete BMW-framed messages...
> ENET (enet.rs): never reaches the PC. The ZGW gateway terminates
> CAN-side ISO-TP and delivers one reassembled diagnostic payload...
So KdcanTransport and EnetTransport already deliver complete
payloads — the gateways (FTDI chip, ZGW) handle ISO-TP termination
upstream of the app. The 10 existing isotp.rs unit tests verify
the implementation works; the integration point is the
IsoTpTransport for FUTURE raw-CAN transports (SocketCAN, OBDLink
STN, future DoIP socket) that don't exist yet.
This commit replaces the v0.13.0 plan with one that drops the
ISO-TP wire-up premise and rescopes the cycle to the slices
that are real user-visible wins:
- Slice 1 (Tier A docs, this PR): the corrected plan + ROADMAP
cycle table update.
- Slice 2 (Tier A + Tier B): KWP2000 slow-module timeout fix in
src-tauri/src/transport/kdcan.rs. The hardcoded 1000ms deadline
times out on real E-series CIC/CAS modules. Per-target override
table with CIC/CAS at 3000ms. Small but real bug.
- Slice 3 (Tier A frontend): E-series CAN broadcast frame decoder
(0x0AA RPM, 0x1D0 coolant, 0x545 oil temp). Pure frontend — the
bytes are already on the bus from the transport. Renders as a
Live Gauges panel.
- Slice 4 (Tier A docs): docs/validation/multi-frame.md explaining
what isotp.rs is for and why the production stack doesn't need
it.
4 slices total (was 6), 1 Tier B (was 2).
The 'Ready to Claim' line below the v0.13.0 cycle table still
references the new cycle — no further stale-reference cleanup
needed beyond what this PR does.
Tier A — docs only. Self-merge when CI green.
---------
Co-authored-by: Jon currie1 <ohgeeceee@users.noreply.github.com>
…sion-sync rule to CLAUDE.md README.md badge line 18 was frozen at v0.6.0 even though v0.7.0 through v0.14.0 have all shipped (or in v0.14.0's case, had their plan merged). Three changes: - README.md: bump release badge to v0.14.0, point at CHANGELOG.md. - CHANGELOG.md: backfill [0.12.0] (Fault Memory cycle, PRs #143-#149 plus the async fix), [0.13.0] (Real Reads Real Long, PRs #150-#153 including the plan correction), [0.14.0] (Live CAN plan only, PR #156). Existing [0.11.0] and older sections untouched. - CLAUDE.md: add golden rule #6 — every release PR must bump both the README badge and the CHANGELOG section in lockstep, so the badge can't drift again. Plan-only cycles use CHANGELOG `### Planned` and leave the README badge alone. pytest backend/tests/ → 151/151 green. No code change, so cargo test and the JS suites are unaffected.
1 participant
What
Slice 1 of v0.13.0 "Real Reads, Real Long": cycle plan + ROADMAP cycle header.
The plan's key insight is that ISO-TP multi-frame is ALREADY implemented (
src-tauri/src/transport/isotp.rs, 430 lines + 10 unit tests includingvin_read_survives_multi_frame_segmentation,dtc_list_reassembles_across_consecutive_frames, etc.) since v0.6.0. What's been blocking F/G long reads and E-series CAS multi-frame responses is the wire-up into the productionconnect()paths inkdcan.rsandenet.rs— those still construct bareKdcanTransport/EnetTransport, notIsoTpTransport.So v0.13.0 is "plug it in" rather than "implement it" — same shape as v0.12.0 (1 Tier B + 4 Tier A + 1 docs), but the Tier B touches
transport/instead ofcommands.rs.Why
Two 🟢 Ready items have been open on the ROADMAP since v0.6.0 (line 56 — "ISO-TP multi-frame") and v0.3.0 (line 52 — "KWP2000 slow-module timeout"). Both are real platform unlocks for every F/G and E-series owner, and the implementation work for the bigger one is already done and tested. v0.13.0 finishes what v0.6.0 started.
The slice also picks up the E-series CAN broadcast frame decoders (🟡 since v0.3.0, line 45) — pure frontend, high user value for E46 owners who currently have no live RPM/coolant display in the app.
What changed
docs/v0.13.0_plan.md— new, 230 lines. The cycle plan, same shape asdocs/v0.12.0_plan.md:src-tauri/src/transport/isotp.rs(read first)ROADMAP.md— +33 lines. New cycle header section between the v0.12.0 closure and "Ready to Claim":## v0.13.0 — "Real Reads, Real Long" (Planned)Cycle scope (6 slices, 2 Tier B)
IsoTpTransportintokdcan.rs::connect()andenet.rs::connect()docs/validation/multi-frame.mdharness docSlice 2 is the gating PR (Tier B). Slices 3 and 5 can land in parallel against slice 2's branch using the simulator's
multi_framepersonality. Slice 4 is a small Tier B change on top of slice 2's surface.What we will NOT do (this cycle, explicit ceiling)
connect()refactors.Verification
git diff --stat origin/main→ROADMAP.md +33, docs/v0.13.0_plan.md +230(two files, no churn)src-tauri/src/transport/isotp.rs(exists, 430 lines, 10 unit tests includingvin_read_survives_multi_frame_segmentation),src-tauri/src/transport/{kdcan,enet,sim}.rs(all exist),src-tauri/src/protocol/mod.rs::read_vin(exists, calls out the multi-frame dependency in its doc-comment).docs/v0.12.0_plan.mdexactly: premise block, honest ceiling block, "what we will NOT do" list, tier split, planned slices table with tier labels, per-PR test plan, execution handoff.Tier
A — docs only, no Rust / commands.rs / transport / protocol change. Self-merge once required CI is green.
Unrelated working-tree noise (not in this PR)
Three files are modified locally but staged only my two intended files:
CLAUDE.md— your own rewrite (Tier C, yours to land)frontend/index.html,frontend/schematics.html— the recurring third-party "network bar" injection (untrusted, not propagated)