feat(v0.12.0): wire readFaults -> recordDtcRead + opt-in toggle - #146
Conversation
Slice 4 of v0.12.0 Fault Memory (docs/v0.12.0_plan.md):
recording wired into the live DTC-read pipeline + an opt-in toggle
in the Settings-style area of the Fault memory panel. Tier A
frontend only — no Rust / commands.rs / transport / protocol change.
Frontend:
- <input type="checkbox" id="record-dtc-history"> added next to the
Read button. Default off so existing users don't get a surprise
file in ~/beeemuu-exports/. Title attribute explains where the
file lives and what the toggle does.
- saveSettings() persists ws.recordDtcHistory; loadSettings()
restores it. Workspace files written before v0.12.0 won't have
the key, so the restore defaults to false (sanitiser preserves
the contract that unknown keys are dropped).
- readFaults() calls window.beeemuuDtcHistory.recordDtcRead(vin,
selectedAddress, dtcs) after a successful non-empty read when the
toggle is on. Best-effort: a recording failure is logged to the
status bar but does NOT break the DTC read UI. Empty reads are
not recorded (timeline is "I saw these faults", not "I read
this module").
- VIN is pulled from the existing #info-vin text (same source the
walkthrough bundle uses). Null if the user hasn't read vehicle
info yet.
- Toggle change handler calls saveSettings() and surfaces a
one-line explainer in the status log the first time it's turned
on, so users see exactly what got created in their home dir.
Workspace:
- sanitizeWorkspace() accepts recordDtcHistory: boolean. Wrong-
typed values (string "true", number 1, null) are dropped so a
hand-edited or buggy workspace.json file can't silently enable
recording.
- 2 new tests in workspace.test.cjs:
- recordDtcHistory accepts only boolean
- recordDtcHistory coexists with other keys without
interfering
The recording call is a no-op until PR #144 (slice 2 Tier B Rust
commands) lands — the wrapper's backendError path kicks in
gracefully and the readFaults UI continues to work as before.
Verification:
- node --check on workspace.js + main.js: pass
- node --test "src/js/**/*.test.js" "src/js/**/*.test.cjs": 122 pass,
0 fail (was 120 before this PR — the 2 new workspace tests).
- Caught + fixed 1 test-bug in my own implementation: the first
assertion expected parseWorkspace to return {} for a file with
only wrong-typed keys, but the contract is null (the existing
'no usable keys -> don't restore anything' rule). Fixed the
assertion and added a comment explaining the contract.
Tier A — self-merge when CI is green.
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
![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: a38cdaf40a
ℹ️ 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".
| $("record-dtc-history").checked | ||
| ) { | ||
| try { | ||
| const vin = ($("info-vin") && $("info-vin").textContent) ? $("info-vin").textContent.trim() : null; |
There was a problem hiding this comment.
Whenever history recording is enabled, this lookup still produces null because the page has no #info-vin element: renderVehicleInfo() creates only generic .info-val nodes, while the VIN is available in lastVehicleInfo.vin and the connection response. Consequently every recorded entry loses its vehicle identity, so later per-car history queries can mix or omit records from different vehicles.
Useful? React with 👍 / 👎.
| ) { | ||
| try { | ||
| const vin = ($("info-vin") && $("info-vin").textContent) ? $("info-vin").textContent.trim() : null; | ||
| const summary = await window.beeemuuDtcHistory.recordDtcRead(vin, selectedAddress, dtcs); |
There was a problem hiding this comment.
If the user selects another ECU while the preceding read_faults request is pending, selectedAddress changes before this line executes, so the returned DTCs from the original ECU are permanently recorded against the newly selected ECU. Capture the requested address before the first await and use that same value for both read_faults and recordDtcRead.
Useful? React with 👍 / 👎.
| $("record-dtc-history").addEventListener("change", (e) => { | ||
| saveSettings(); | ||
| if (e.target.checked) { | ||
| log("DTC history recording ON — reads are logged to ~/beeemuu-exports/dtc-history.jsonl. Clear via Settings."); |
There was a problem hiding this comment.
When a user enables recording, this message directs them to clear the resulting VIN/DTC history through Settings, but the target tree contains no Settings view or history-clear control, and clearDtcHistory() is never called outside its module tests. Users therefore cannot follow the advertised deletion path; either wire a clear action or point them to an option that actually exists.
Useful? React with 👍 / 👎.
| // DTC read UI. Empty-dtc reads are not recorded by design (the | ||
| // timeline is "I saw these faults", not "I read this module"). | ||
| if ( | ||
| dtcs.length > 0 && |
There was a problem hiding this comment.
When an ECU successfully returns an empty DTC array, this guard skips the history call entirely, despite the checkbox promising to record every DTC read. The resulting timeline cannot distinguish a clean scan from a module that was never checked, which makes the persisted history incomplete; pass successful empty reads through so the history layer can represent the scan result.
Useful? React with 👍 / 👎.
1 participant
What
Wires the v0.12.0 Fault Memory recording into the live DTC-read pipeline plus an opt-in Settings toggle. Tier A frontend only — no Rust / commands.rs / transport / protocol change.
Why
The plan (
docs/v0.12.0_plan.mdslice 4): "Hookdtc_history.recordDtcRead(...)intoreadFaults()after a successful read; add a Settings toggle for the opt-in recording (off by default)."Off by default so existing users don't get a surprise file in
~/beeemuu-exports/without explanation. The toggle's tooltip explains exactly what gets created and where.What changed
src/js/main.js:readFaults()callswindow.beeemuuDtcHistory.recordDtcRead(vin, selectedAddress, dtcs)after a successful non-empty read when the toggle is on. Best-effort: a recording failure is logged to the status bar but does not break the DTC-read UI. Empty reads are not recorded (the timeline is "I saw these faults", not "I read this module").#info-vintext (same source the v0.11.0 walkthrough bundle uses). Null if the user hasn't read vehicle info yet.saveSettings()persistsws.recordDtcHistory.loadSettings()restores it — older workspace files (pre-v0.12.0) won't have the key, so the restore defaults tofalse.saveSettings()and surfaces a one-line explainer in the status log the first time it's turned on, so users see exactly what got created in their home dir.src/js/workspace.js:sanitizeWorkspace()acceptsrecordDtcHistory: boolean. Wrong-typed values ("true",1,null) are dropped so a hand-edited or buggy workspace.json can't silently enable recording.src/index.html: new<input type="checkbox" id="record-dtc-history"> Record historylabel next to the Read button. Title attribute explains the toggle's effect.src/js/test/workspace.test.cjs: 2 new tests for the boolean shape + coexistence with other keys.Verification
node --checkonworkspace.js+main.js→ pass.node --test "src/js/**/*.test.js" "src/js/**/*.test.cjs"→ 122 pass, 0 fail (was 120 before this PR — the 2 new workspace tests).parseWorkspaceto return{}for a file with only wrong-typed keys, but the contract isnull(the existing "no usable keys → don't restore anything" rule). Fixed the assertion and added a comment explaining the contract.Dependency on PR #144 (Tier B, awaiting human merge)
The recording call invokes
record_dtc_readfrom the Tauri command surface added in PR #144. Until #144 lands onmain, the recording call'sresolveInvoke()finds the IPC backend (it does — Tauri 2.x'swindow.__TAURI_INTERNALS__.invokeis wired by the Tauri webview at runtime) but the named command doesn't exist server-side yet. Thedtc_history.jswrapper surfaces abackendErrorin that case, whichreadFaults()catches and logs as"DTC history record skipped: <message>"— the DTC read itself completes normally.Once #144 lands, the same code starts recording to disk without any further changes to slice 4.
Tier
A — frontend only (no
transport/,protocol/,commands.rs, no Rust change). Self-merge once required CI is green.v0.12.0 progress
Slices 5 and 6 are independent — they only read from the history file, which slice 4 produces.
Unrelated working-tree noise (not in this PR)
Three files are modified locally but staged only my four 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)