Skip to content

fix(v0.14.1): clear fault memory actually clears in E90 simulation (#161) - #169

Merged
ohgeeceee merged 1 commit into
mainfrom
fix/issue161-clear-fault-memory
Jul 27, 2026
Merged

fix(v0.14.1): clear fault memory actually clears in E90 simulation (#161)#169
ohgeeceee merged 1 commit into
mainfrom
fix/issue161-clear-fault-memory

Conversation

@ohgeeceee

Copy link
Copy Markdown
Owner

fix(v0.14.1): clear fault memory actually clears in E90 simulation (#161)

Tier B — touches src-tauri/src/transport/sim.rs and src-tauri/Cargo.toml. Flagging the protected path per the project's autonomy rules; waiting for human merge.

What this fixes

Resolves #161 — "Clear fault memory not Working in Simulation".

The user reported two things:

  1. Clicking "Clear fault memory" in E90 simulation mode does not appear to clear the fault memory.
  2. Clicking "Run vehicle test" after clearing should regenerate the simulated faults.

Part 1 — the actual fix (dialog plugin)

Root cause is a Tauri-2-webview window.confirm() flakiness: in some builds the modal auto-dismisses without showing, which short-circuits the click handler at if (!confirm(...)) return and the invoke("clear_faults", ...) never reaches the Rust side. The user sees no change in the fault list and the dialog has already vanished. Same fragility affects the security-access service-routine confirmation gate.

Fix: switch both confirmation gates to the tauri-plugin-dialog ask() API, which always shows a real OS dialog and returns the user's actual choice.

New helper src/js/dialog.js exposes a CommonJS / dual-export ask() that prefers window.__TAURI__.dialog.ask, falls back to window.confirm, and finally returns true if nothing is available (better to run once than silently block a safety gate — the backend still rejects malformed inputs).

Two callsites changed in src/js/main.js:

  • btn-clear-faults click handler (~line 1125)
  • run_service_function confirm gate (~line 1353)

Rust side additions:

  • tauri-plugin-dialog = "2" in Cargo.toml
  • tauri_plugin_dialog::init() plugin registration in src-tauri/src/lib.rs
  • "dialog:default" permission in src-tauri/capabilities/default.json

Part 2 — the sim side (regenerate DTCs on next scan)

The sim's DTC list is now seeded from default_dtcs (and default_freeze) captured at construction. On the next KWP [0x1A, 0x80] identify request (which scan_modules calls per ECU as part of "Run vehicle test"), the identify handler restores the seed list when the current DTC list is empty. This models what a real car would do — re-detect stale faults on a fresh ignition cycle — and gives the user the demo loop they expected.

The refill is silent (is_empty() guard), so it never overwrites a fresh DTC read mid-session. ECUs that started with no faults stay empty.

Verification

Suite Result
cargo test --lib (src-tauri/) 133/133 pass (132 baseline + 3 new sim tests)
cargo test --tests (integration) passes (the async_commands allowlist guard still classifies clear_faults as async fn)
node --test "src/js/**/*.test.js" "src/js/**/*.test.cjs" 206/206 pass (baseline + 6 new dialog tests)
pytest backend/tests/ 166/166 pass (no backend changes)

The 3 new sim tests:

  • clear_dtcs_actually_clears_sim: pre-clear → clear → post-clear; count is 0
  • cleared_dtcs_have_no_freeze_frame: cleared DTCs get 7F 12 31 for readFreezeFrame
  • identify_refills_dtcs_after_clear: clear → identify → seed list (incl. seed freeze) is restored

The 6 new dialog tests cover:

  • Plugin takes precedence over window.confirm
  • Fallback to window.confirm when plugin is absent
  • false propagates correctly when the user clicks Cancel
  • Returns true if neither plugin nor confirm is available (no silent block)
  • Plugin errors are swallowed and fall through to confirm
  • Module loads as CommonJS and exposes ask()

Files changed (staged for this PR)

src-tauri/Cargo.lock                | 160 +++++++++++++++++++++++++++++++++
src-tauri/Cargo.toml                |   8 ++
src-tauri/capabilities/default.json |   6 +-
src-tauri/src/lib.rs                |   5 ++
src-tauri/src/transport/sim.rs      | 165 +++++++++++++++++++++++++++++-
src/index.html                      |   1 +
src/js/dialog.js                    |  58 +++++++++++  (new)
src/js/dialog.test.js               | 121 +++++++++++++++++++++++++  (new)
src/js/main.js                      |   4 +-
9 files changed, 509 insertions(+), 19 deletions(-)

Worktree noise (NOT in this PR)

These changes were already sitting unstaged in the worktree when I created this branch — they look like in-progress work on a separate per-ECU freeze-schema split and should ship in their own PR, not be wedged into this bugfix. They are left untouched and unstaged:

  • src-tauri/src/commands.rs (modified)
  • src-tauri/src/community.rs (modified)
  • community/freeze_schemas.toml (deleted)
  • community/freeze/{12,29,72}.toml and community/freeze/README.md (added)

Please flag if you want them folded in, otherwise they'll come through as a separate slice.

Cross-references

  • Issue: #161
  • Plan: docs/v0.14.0_plan.md (the v0.14.0 cycle this slice bumps toward v0.14.1)

)

Root cause is a Tauri-2-webview `window.confirm()` flakiness: in some
builds the modal auto-dismisses without showing, which short-circuits
the click handler at `if (!confirm(...)) return` and the
`invoke("clear_faults", ...)` never reaches the Rust side. The user
sees no change in the fault list and the dialog has already vanished.
Switch both confirmation gates (the v0.12.0 "Clear fault memory"
button and the security-access service-routine confirm) to the
`tauri-plugin-dialog` `ask()` API, which always shows a real OS dialog
and returns the user's actual choice.

Also add the simulator-side behaviour the issue's "Expected" paragraph
asks for: "If I then click Run vehicle test again, I would expect the
simulated faults to be generated again." The sim's DTC list is now
seeded from `default_dtcs` (and `default_freeze`) captured at
construction; on the next KWP `[0x1A, 0x80]` identify request (which
`scan_modules` calls per ECU), the identify handler restores the seed
list when the current DTC list is empty. This models a real car
re-detecting faults on a fresh ignition cycle and gives the user the
demo loop they expected.

Tier B (touches `src-tauri/src/transport/sim.rs` and
`src-tauri/Cargo.toml`).

Verification:

- `cargo test --lib` -- 133/133 pass. New tests:
  - `clear_dtcs_actually_clears_sim`
  - `cleared_dtcs_have_no_freeze_frame`
  - `identify_refills_dtcs_after_clear`
- `cargo test --tests` -- 1/1 (the `async_commands` allowlist guard
  still classifies `clear_faults` as `async fn`, so no allowlist
  regression).
- `node --test "src/js/**/*.test.js" "src/js/**/*.test.cjs"` -- 206/206
  pass. New tests: 6 covering the dialog helper's three
  fallback paths.
- `pytest backend/tests/` -- 166/166 pass (no backend changes).

Note on the worktree: the changes to `src-tauri/src/commands.rs`,
`src-tauri/src/community.rs`, and `community/freeze/` that were sitting
unstaged when I created this branch are kept untouched and unstaged --
they look like in-progress work on a separate per-ECU freeze-schema
split and should ship in their own PR, not be wedged into this fix.
@ecc-tools

ecc-tools Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6a17891fe5

ℹ️ 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".

Comment thread src/js/dialog.js
Comment on lines +46 to +50
// No dialog surface — caller decides what to do (treat as "yes" so a
// missing dialog never silently blocks a safety gate; the helper's
// callers already pass through to `invoke(...)` and the backend still
// rejects malformed inputs).
return true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Fail closed when confirmation cannot be displayed

When the dialog plugin is unavailable and the host does not expose window.confirm, returning true is interpreted as user consent by the changed service-function handler, so a valid run_service_function request—including an unverified routine that actuates vehicle hardware—runs without any warning. Backend input validation does not confirm user intent; this fallback should return false or throw so destructive actions remain blocked when no confirmation surface exists.

Useful? React with 👍 / 👎.

Comment on lines +449 to +452
if ecu.dtcs.is_empty() && !ecu.default_dtcs.is_empty() {
ecu.dtcs = ecu.default_dtcs.clone();
if !ecu.default_freeze.is_empty() {
ecu.freeze = ecu.default_freeze.clone();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restrict fault reseeding to vehicle scans

Reseeding on every 1A 80 identification request means read-only actions other than “Run vehicle test” undo a clear: export_session identifies each ECU at commands.rs:859 before reading its DTCs, so a snapshot taken immediately after clearing contains the original simulated faults and leaves them restored, while connection_test does the same at line 628 for the DME. Regeneration needs to be tied to the explicit vehicle-scan action rather than the protocol-wide identify handler.

Useful? React with 👍 / 👎.

@ohgeeceee
ohgeeceee merged commit 653c154 into main Jul 27, 2026
13 of 14 checks passed
github-actions Bot pushed a commit that referenced this pull request Jul 27, 2026
…#171)

v0.14.2 ships "live data today, on the bench, with the cable you have."
The user is going real-car with a K+DCAN cable on a 2007 E70 X5 4.8L
(N62/BTU, MSV80-family DME, D-CAN @ 500 kbps). v0.14.0's Tier B
(raw-CAN listener + `get_latest_can_frames`) was explicitly gated
behind an OBDLink SX cable — the K+DCAN cable cannot passively
listen to raw broadcast frames (FTDI firmware terminates ISO-TP
upstream; see `kdcan.rs` doc-comment lines 1-15 + `isotp.rs` lines
1-22). v0.14.2 fills the per-param data, the panel UX, and the
chassis-specific verification doc instead — all Tier A, no
`transport/**` changes, no new crate.

Three planned slices (all Tier A):
1. `community/profiles/n62.toml` enrichment — `0x5C` (oil temp),
   `0x5E` (fuel rate L/h), `0x5F` (runtime), `0x62` (fuel rate g/s);
   removes the unverified `local:10` placeholder once `0x5C` is
   verified on the E70. Adds `[[profile.notes]]` for N62
   instrumentation (valley-pan slow-coolant, sleep-sensitive PIDs).
2. Live Data panel UX polish — polling-rate selector, per-gauge
   peak tracking, range bar, snapshot-CSV button, NRC error surface.
3. `docs/validation/n62-real-car.md` harness doc.

Explicit "what we will NOT do" list:
- No `transport/**` touches.
- No `protocol/**` touches.
- No new crate (`serialport = "4"` already covers everything).
- No modification of the v0.14.0 Live Gauges panel (sim-only).
- No release tag cut (Tier C, user call).

ROADMAP updates:
- New `## v0.14.1` section retroactively closing the PR #169
  issue-#161 fix + PR #170 freeze-schema split.
- New `## v0.14.2` section marked `(In Progress)` with the
  Slices planned table.
- Footer (`*Last updated:` line) bumped to reflect v0.14.1 ship
  + v0.14.2 open + v0.14.0 Tier B gated behind OBDLink SX.

Tier A docs-only. Self-merge when CI green.

Co-authored-by: ohgeeceee <ohgeeceee@users.noreply.github.com>
github-actions Bot pushed a commit that referenced this pull request Jul 30, 2026
Closes the CHANGELOG gap that PR #188 (v0.14.3 slice 4) flagged
in the "Notes on the version surface" section. v0.14.1 and
v0.14.2 shipped without CHANGELOG entries because each cycle's
slice-closeout PR either forgot the version-surface sync step
or deferred it as a separate housekeeping follow-up.

This PR does the backfill from PR commit history:

- ## [0.14.1] — 2026-07-27
  - Tauri 2 `window.confirm()` auto-dismiss fix (PR #169, Tier B)
  - Simulator regenerate-on-identify (PR #169, Tier B)
  - Per-ECU freeze-schema split (PR #170, Tier A)
  Note: PR #169 shipped two slices in one PR (the dialog.js
  helper + the sim regenerate-on-identify). PR #170 is grouped
  under v0.14.1 because the ROADMAP treats the freeze-schema
  split as part of the v0.14.1 housekeeping arc (it was
  originally targeted at v0.14.0 but its tests-only refactor
  landed late and folded into the v0.14.1 PR #171 cycle-table
  retroactive close).

- ## [0.14.2] — 2026-07-29
  - Cycle plan + ROADMAP v0.14.2 header (PR #171, Tier A)
  - `community/profiles/n62.toml` enrichment — `0x5C` oil temp
    (PR #175, Tier A)
  - Live Data panel UX polish — poll-rate, peaks, range bar,
    snapshot-CSV, NRC error surface (PR #177, Tier A)
  - `docs/validation/n62-real-car.md` harness doc (PR #178,
    Tier A)
  - Claude review workflow repair — remove unsupported
    `Bash(gh pr review:*)` tool from `--allowedTools` (PR #176,
    Tier B)
  Note: the original v0.14.3 "Notes on the version surface"
  paragraph omitted PR #176 from the v0.14.2 PR list — fixed in
  this backfill.

Also updates the v0.14.3 "Notes on the version surface" section
to point at this backfill PR instead of flagging it as a
backlog item, and includes PR #176 in the v0.14.2 PR list.

Tier A — docs only. No code changes, no transport/** changes,
no protocol/** changes. PR auto-merge eligible per CLAUDE.md
once CI is green.

Verification:
- [x] CHANGELOG section order preserved: [0.14.0] (line 8) →
      [0.14.1] (line 71) → [0.14.2] (line 105) → [0.14.3]
      (line 168) → [0.13.0] (line 273) — chronological order
      matches merge order (verified via `gh pr list --state
      merged --json number,title,mergedAt`)
- [x] Every PR number cited in a backfill entry exists and
      was actually merged to main (verified via `gh pr view
      N --json mergedAt` for PRs #169, #170, #171, #175, #176,
      #177, #178)
- [x] All slice claims verified against each PR's actual body
      — no fabricated content per the data-over-invention rule
- [x] `node --test src/js/*.test.js` — 163/163 pass (no code
      changes; 5 slice 3b tests absent because this branch is
      from origin/main pre-PR-190)
- [x] `pytest backend/tests/` — 166/166 pass

Cross-references:
- PR #188 — v0.14.3 slice 4 (the cycle-closeout PR that
  flagged this backfill as the appropriate scope)
- CLAUDE.md golden rule #5 — version-surface sync (the rule
  this PR enforces retroactively for v0.14.1 + v0.14.2)
- docs/v0.14.3_plan.md — the cycle plan that calls out the
  forward-roadmap maintenance pattern this PR continues

Co-authored-by: ohgeeceee <ohgeeceee@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clear fault memory not Working in Simulation

1 participant