feat(v0.14.3): slice 3b — frontend rewire for per-PID NRC + remove UI - #190
Conversation
Cycle plan: docs/v0.14.3_plan.md (slice 3 lines 103-124). Tier B (frontend + CSS; no transport/** changes, no protocol/** changes, no Rust changes — the backend contract was already shipped by PR #187). Per CLAUDE.md Tier B rules: PR is the review; no self-merge even if all checks pass. Awaiting human merge. ## What this slice ships ### 1. live_data_panel.js — new `classifyNrc(err)` helper Categorises each `LiveError` from `read_live_data`'s new `LiveSweepResult { values, errors }` shape into one of three buckets the UI acts on: - "unsupported" — the PID is structurally absent on this ECU (NRC 0x11 / 0x12 / 0x31 / 0x14); user should be offered the "remove from profile" affordance. - "transient" — ECU is unhappy right now (NRC 0x22 conditionsNotCorrect, NRC 0x33 securityAccessRequired, NRC 0x78 responsePending, etc.); no UI action — the next sweep will retry. - "unknown" — no structured sid/nrc to act on (transport timeout, "Not connected", "Unexpected DID response: ..."); same UI treatment as transient. Uses the structured (sid, nrc) fields populated by `protocol::nrc_from_error` (PR #187) as the fast path. Falls back to parsing `err.error` (the verbatim protocol error string) via `parseNrcError` when the structured fields are null, so legacy callers and any race where the backend populated `error` but not the structured fields still get a sensible classification. Exported from the public surface so main.js can call it as `window.beeemuuLiveDataPanel.classifyNrc(err)`. ### 2. live_data_panel.test.js — 5 new tests Pinned tests for classifyNrc covering: - structured (sid, nrc) fast path: unsupported for the four canonical NRCs - structured (sid, nrc) fast path: transient for non-unsupported NRCs - structured nrc=null falls back to parsing err.error - structured nrc=null + unparsable error string returns 'unknown' - defensive: null / non-object / missing fields never throw Total: 20 tests in this file (was 15). ### 3. main.js — three call sites rewired for `LiveSweepResult` The new return shape of `read_live_data` (PR #187) breaks the three call sites that did `const values = await invoke("read_live_data", ...)`. All three are updated: - pollOnce (Live Data tab, line 1365) — main consumer; iterates `result.values` for the gauge loop, then a NEW loop over `result.errors` that classifies each PID and either (a) logs the canonical "unsupported" line + dims the gauge cell + appends a one-click "Remove from profile" button that calls the async `remove_profile_pid` Tauri command, or (b) logs the transient / unknown case and skips UI affordance. Clears any prior dim state on a successful read (PID may recover after a re-key cycle). - buildLogParams (Logging tab channel list, line 2109) — iterates `result.values` only; the Logging tab doesn't render error UI. - logTick (Logging tab chart tick, line 2203) — iterates `result.values` only. New handler `removePidFromProfile(pidId, pidLabel)` — gated behind `window.beeemuuDialog.ask()` (the same `tauri-plugin-dialog` confirmation pattern the v0.14.1 issue-#161 fix uses). On success, removes the gauge cell from the DOM (the PID is gone from the in-memory profile + the on-disk TOML, so the next sweep won't return it). New panel-head count badge `#live-unsupported-count` — hidden when zero, shows "N unsupported on this ECU" otherwise. ### 4. index.html — `#live-unsupported-count` slot One-line addition in the Live Data panel head, next to the profile selector. `hidden` by default; populated by pollOnce. ### 5. app.css — dimmed state + remove button + count badge Three new rule blocks: - `.gauge-cell.dimmed` — opacity 0.45 + " (unsupported)" appended to the label via ::after pseudo-element. Cleared on next sweep. - `.pid-remove` — minimal inline button that sits inside the gauge cell below the peak label. Hover colour is the project's --err. - `.live-unsupported-count` — small muted text in the panel head. ## What this PR does NOT do - No Rust changes. PR #187 already shipped `protocol::nrc_from_error`, `LiveSweepResult { values, errors }`, `LiveError`, and the async `remove_profile_pid` Tauri command. This PR is the frontend consumer. - No transport/** changes (K+DCAN / ENET transports untouched). - No protocol/** changes. - No new crate / dependency. - No change to the v0.14.0 Live Gauges panel (still sim-only, per the v0.14.2 cycle-pick conversation). - No version bumps. The release cut (Cargo.toml + tauri.conf.json + git tag) is a separate Tier C step that follows slice 3b. ## Tier B per CLAUDE.md — `src/js/main.js` + `src/js/live_data_panel.js` + `src/index.html` + `src/css/app.css`. Per CLAUDE.md Tier B rules: "PR is the review; auto-merge disabled even if all checks pass. Awaiting human merge." ## Verification - [x] `node --check src/js/main.js` — rc=0 - [x] `node --check src/js/live_data_panel.js` — rc=0 - [x] `node --test src/js/*.test.js` — 168/168 pass (was 163, +5 new for classifyNrc) - [x] `cargo test --lib --offline protocol::` — 15/15 pass (no regression on the backend surface this slice consumes) - [x] `pytest backend/tests/` — 166/166 pass (one test, test_app_live_endpoint::test_endpoint_returns_ok, was order- dependent flaky on first run; passed on re-run — not introduced by this slice) ## Cross-references - v0.14.3 plan: docs/v0.14.3_plan.md (slice 3 lines 103-124) - Backend contract this PR consumes: PR #187 — feat(v0.14.3): per-PID NRC errors + remove_profile_pid command - Prior slice shipping the NRC-aware helpers + friendlier log line: PR #177 — feat(v0.14.2): Live Data panel UX polish (parseNrcError + isUnsupportedNrc were drafted there as dead-but-not-yet-usable helpers; this slice activates them via the per-PID error loop + classifyNrc) - Re-enabled orphan from v0.14.2 slice 2 WIP: docs/v0.14.3_plan.md §"Open question #2" — the `#live-unsupported-count` badge + the dim/remove UI together fulfil the deferred WIP
|
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: da5dd02680
ℹ️ 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".
| const cell = document.querySelector( | ||
| '.gauge-cell[data-pid-id="' + err.id + '"]' | ||
| ); | ||
| if (!cell) continue; |
There was a problem hiding this comment.
When a PID is unsupported from the first sweep—the primary scenario for this feature—it appears only in result.errors, while ensureGauge is called only for result.values. Consequently this lookup always returns null, and the code skips both dimming and creation of the remove button, leaving users unable to remove a structurally unsupported PID. Create an error-state cell from the error metadata or otherwise provide the removal affordance independently of a prior successful reading.
Useful? React with 👍 / 👎.
| "(NRC 0x" + String(err.nrc).toUpperCase() + ", sid 0x" + | ||
| String(err.sid).toUpperCase() + ")." |
There was a problem hiding this comment.
For the normal structured backend response, sid and nrc are JSON numbers, so String(err.nrc) and String(err.sid) produce decimal text despite the 0x prefix. For example NRC 0x31 and SID 0x22 are logged as NRC 0x49, sid 0x34, which misreports diagnostic results; convert numeric fields with toString(16) as the systemic-error path already does.
Useful? React with 👍 / 👎.
| const cell = document.querySelector( | ||
| '.gauge-cell[data-pid-id="' + pidId + '"]' | ||
| ); | ||
| if (cell) cell.remove(); |
There was a problem hiding this comment.
After remove_profile_pid succeeds, only the DOM cell is removed; the corresponding entry remains in gauges and its value remains in livePeakState. The snapshot exporter iterates gauges, the peak table renders livePeakState, and the animation loop continues ticking every gauge, so a PID the user removed can still appear in snapshots and the peak table while its detached canvas continues being updated. Delete both state entries and refresh the dependent UI on successful removal.
Useful? React with 👍 / 👎.
| const result = await invoke("read_live_data", { profile: $("log-profile").value }); | ||
| const values = Array.isArray(result.values) ? result.values : []; |
There was a problem hiding this comment.
When a channel starts returning a per-PID error during an active recording, the new backend shape resolves successfully with that failure in result.errors, so this loop silently omits the sample and the catch block no longer reports or stops on it. Because the chart connects the surrounding successful points and the Live Data poll need not be running while the Logging tab is active, users can export a recording with an invisible data outage. Inspect result.errors here and either record/indicate the gap or surface the failure and stop the affected recording.
Useful? React with 👍 / 👎.
| const badge = $("live-unsupported-count"); | ||
| if (badge) { | ||
| if (unsupportedCount > 0) { | ||
| badge.textContent = unsupportedCount + " unsupported on this ECU"; | ||
| badge.hidden = false; |
There was a problem hiding this comment.
If a sweep displays a nonzero unsupported count and polling is then stopped, changing #live-profile clears the gauges and peak state but never resets this new badge. It therefore continues claiming that the newly selected profile has unsupported PIDs until another sweep is started, which can leave indefinitely stale ECU/profile status on screen. Reset the badge in the profile-change path along with the other per-profile state.
Useful? React with 👍 / 👎.
| } catch (e) { | ||
| log("Failed to remove " + pidLabel + ": " + e); |
There was a problem hiding this comment.
When the profile TOML cannot be written—for example because the community directory is read-only—the backend removes the PID from its in-memory profile before attempting the write and then returns an error. This catch path treats that as a complete failure and leaves the dimmed cell and remove button in place, but subsequent sweeps no longer contain that PID and retrying the button returns “already removed,” so the UI remains permanently stale for the session. Make the command transactional or return enough partial-success state for this handler to remove/reconcile the cell while accurately reporting that persistence failed.
Useful? React with 👍 / 👎.
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
Closes the v0.14.3 cycle for real. PR #188 (slice 4) closed the cycle table on its own merge but the cycle itself was incomplete — slice 3b was still open, gating the release cut. PR #190 (slice 3b, merged 2026-07-30T18:58:11Z into origin/main @ 10f58c8) closed that gate. This PR updates the stale docs to match reality. ## What this PR ships ### ROADMAP.md - "Last updated" header: bumped from "partial close" to "closed"; tier split bumped from "3 Tier A + 1 Tier B + 1 Tier B still open" to "3 Tier A + 2 Tier B + 0 Tier C"; all five cycle slices now listed as merged (#185, #186, #187, #188, #190). - v0.14.3 cycle section header: "In Progress — slices 1, 2, 4 done; slice 3 split" → "Shipped 2026-07-30". - Cycle table slice 3b row: 🔲 Open → ✅ Done (PR #190), with full notes describing the `classifyNrc` helper, the per-PID dim + one-click-remove UI, the `#live-unsupported-count` panel-head badge, and the new `.gauge-cell.dimmed` / `.pid-remove` CSS. - Cycle table other rows updated to attribute by PR number (#188) instead of the original "this PR" placeholder. - Release-cut paragraph: "Cannot be cut until slice 3b merges" → "is a separate Tier C step — all five cycle slices are merged but the version-surface bump requires an explicit release-cut PR". ### CHANGELOG.md - v0.14.3 "Cycle status" blockquote: bumped from "slices 1, 2, 3a, 4 merged; slice 3b still open" to "all five slices merged — #185 (decoders), #186 (profile entries), #187 (slice 3a backend), #188 (slice 4 harness extension + cycle closeout), #190 (slice 3b frontend rewire)". - v0.14.3 "Added — Tier B surface" bullet: merged the slice 3a (PR #187) and slice 3b (PR #190) descriptions into a single "Per-PID NRC backend + frontend + remove-from-profile UI" entry with PR sub-bullets. Removed the now-stale "slice 3b is still open" note. - v0.14.3 "Notes on the version surface" — README badge paragraph: rewrote to acknowledge slice 3b is now merged but the release-cut PR hasn't run, so the badge correctly stays at v0.14.0 until the Tier C release-cut PR. - v0.14.3 closing paragraph: "follows slice 3b's merge" → "follows the release-cut PR's merge. Until that lands, this entry stays `## [0.14.3] — Unreleased`". ## What this PR does NOT do - ❌ No README badge bump. CLAUDE.md golden rule #5 still applies — the release-cut PR (Tier C) hasn't run, so v0.14.3 isn't a released version. Bumping now would replace one lie with another. - ❌ No `Cargo.toml` / `tauri.conf.json` version bumps. These are part of the Tier C release cut (separate PR). - ❌ No git tag, release notes, or installer build. Tier C. - ❌ No code changes. Docs only. ## Tier **A** — `CHANGELOG.md` + `ROADMAP.md`. PR auto-merge eligible per CLAUDE.md Tier A rules. ## Verification - [x] `node --test src/js/*.test.js` — 168/168 pass (includes the slice 3b `classifyNrc` tests added in PR #190) - [x] `cargo test --lib --offline protocol::` — 15/15 pass (regression check; no code changes) - [x] `npm run build` — `rc=0`, both MSI + NSIS bundles built (1m 52s) - [x] ROADMAP cycle section header now reads "Shipped 2026-07-30" - [x] CHANGELOG `[0.14.3]` header preserved as "Unreleased" (correctly — release cut is separate) ## Cross-references - PR #188 — v0.14.3 slice 4 (the original cycle-closeout PR that flagged slice 3b as the gating slice) - PR #190 — v0.14.3 slice 3b (now merged; the close this PR's docs are catching up to) - PR #192 — v0.14.1 + v0.14.2 CHANGELOG backfill (the previous round of version-surface housekeeping) - CLAUDE.md golden rule #5 — version-surface sync (the rule this PR enforces retroactively for v0.14.3's slice 3b) Co-authored-by: ohgeeceee <ohgeeceee@users.noreply.github.com>
Tier A, docs only. v0.14.5 slice 2 of 3. Closes the cycle. Mirrors the v0.14.2 N62 harness doc (docs/validation/n62-real-car.md, PR #178 + PR #188) and adapts it for the E9x N52 / N54 family that v0.14.5 slice 1 (PR #223) just enriched. The doc is the report-back loop an E9x owner runs on a real car to lift the `[needs verification, N5x/E9x bench]` markers PR #223 placed on the four new OBD-II PIDs (`0x5C` oil temp, `0x5E` fuel rate L/h, `0x5F` engine runtime, `0x62` fuel rate g/s). Five-section shape (mirrors the N62 harness doc): 1. What this is — the per-PID list, N52 BSD note, N54 charge-air / boost / HPFP context. 2. Step 1 — wire-up — K+DCAN cable on E9x (pin 6+14 D-CAN, pin 7 K-line fallback). 3. Step 2 — cold readings — 13 N52 PIDs + 15 N54 PIDs. Critical rows: oil temp (N52 BSD failure mode flagged), fuel rate at idle/WOT. 4. Step 3 — running readings — 13 N52 PIDs + 15 N54 PIDs. N54 ranges bumped for twin-turbo (WOT ~80-150 L/h vs N52's ~50-90 L/h). 5. Step 4 — report template — markdown block with chassis + firmware + cable + profile + per-state readings. 6. Step 5 — what we do with the report — passing report removes the verification markers; failing report reverts per-PID. **The N52-specific BSD-not-supported failure mode is documented explicitly:** if the N52 DME returns an NRC for `0x5C` on a given firmware, the protocol reverts the oil entry to `local:10` placeholder via the v0.14.3 PR #187 per-PID NRC surface + `remove_profile_pid` async Tauri command. The N52 BSD oil-condition sensor note is the load-bearing difference from the N62 harness doc. The N54-specific sections (charge-air / boost / HPFP rail / WOT fuel-rate ranges) are the N54-specific additions. No transport/**, protocol/**, commands.rs, or frontend changes. No new crates. No new BMW hex descriptions. No git tag v0.14.5 (Tier C release cut is the next step after this PR lands). Cross-references: v0.14.5 plan (PR #222), v0.14.5 slice 1 (PR #223), N62 cycle predecessors (PRs #175, #185, #186, #187, #188, #190, #208), docs/DECODE_FUNCTIONS.md § 3/10/11/12. Author note: commit authored with ohgeeceee@users.noreply.github.com to bypass GH007 (private-email push block). Content unchanged. Co-authored-by: ohgeeceee <ohgeeceee@users.noreply.github.com>
1 participant
Slice 3b of v0.14.3 — "Finish the Bench"
Cycle plan:
docs/v0.14.3_plan.md(slice 3 lines 103–124). Tier B — touchessrc/js/main.js+src/js/live_data_panel.js+src/index.html+src/css/app.css. No Rust changes (the backend contract was already shipped by PR #187). Per CLAUDE.md Tier B rules: PR is the review; auto-merge disabled even if all checks pass. Awaiting human merge.What this slice ships
1.
live_data_panel.js— newclassifyNrc(err)helperCategorises each
LiveErrorfromread_live_data's newLiveSweepResult { values, errors }shape into one of three buckets the UI acts on:"unsupported"— the PID is structurally absent on this ECU (NRC 0x11 / 0x12 / 0x31 / 0x14); the user gets the "remove from profile" affordance."transient"— ECU is unhappy right now (NRC 0x22 conditionsNotCorrect, 0x33 securityAccessRequired, 0x78 responsePending); no UI action — the next sweep retries."unknown"— no structured sid/nrc to act on (transport timeout, "Not connected", "Unexpected DID response: ..."); same treatment as transient.Uses the structured
(sid, nrc)fields populated byprotocol::nrc_from_error(PR #187) as the fast path. Falls back to parsingerr.error(the verbatim protocol error string) viaparseNrcErrorwhen the structured fields are null, so legacy callers and any race where the backend populatederrorbut not the structured fields still get a sensible classification.Exported as
window.beeemuuLiveDataPanel.classifyNrc(err).2.
live_data_panel.test.js— 5 new testsPinned tests for
classifyNrccovering structured fast path (unsupported + transient), theerr.errorfallback path, unparseable-error →"unknown", and defensive null/non-object inputs. Total: 20 tests in this file (was 15).3.
main.js— three call sites rewired forLiveSweepResultThe new return shape of
read_live_data(PR #187) breaks the three call sites that didconst values = await invoke("read_live_data", ...). All three are updated:pollOnce(Live Data tab, line 1365) — main consumer. Iteratesresult.valuesfor the gauge loop, then a new loop overresult.errorsthat classifies each PID and either:remove_profile_pidTauri command (PR feat(v0.14.3): per-PID NRC errors + remove_profile_pid command #187), ORClears any prior dim state on a successful read (PID may recover after a re-key cycle).
buildLogParams(Logging tab channel list, line 2109) — iteratesresult.valuesonly; the Logging tab doesn't render error UI.logTick(Logging tab chart tick, line 2203) — iteratesresult.valuesonly.New handler
removePidFromProfile(pidId, pidLabel)— gated behindwindow.beeemuuDialog.ask()(the sametauri-plugin-dialogconfirmation pattern the v0.14.1 issue-#161 fix uses). On success, removes the gauge cell from the DOM (the PID is gone from the in-memory profile + the on-disk TOML, so the next sweep won't return it).New panel-head count badge
#live-unsupported-count— hidden when zero, shows "N unsupported on this ECU" otherwise.4.
index.html—#live-unsupported-countslotOne-line addition in the Live Data panel head, next to the profile selector.
hiddenby default; populated by pollOnce.5.
app.css— dimmed state + remove button + count badgeThree new rule blocks:
.gauge-cell.dimmed— opacity 0.45 + " (unsupported)" appended to the label via::afterpseudo-element. Cleared on next sweep..pid-remove— minimal inline button that sits inside the gauge cell below the peak label. Hover colour is the project's--err..live-unsupported-count— small muted text in the panel head.What this PR does NOT do
protocol::nrc_from_error,LiveSweepResult { values, errors },LiveError, and the asyncremove_profile_pidTauri command. This PR is the frontend consumer.transport/**changes (K+DCAN / ENET transports untouched).protocol/**changes.Cargo.toml+tauri.conf.json+ git tag) is a separate Tier C step that follows this PR.Tier
B per CLAUDE.md —
src/js/main.js+src/js/live_data_panel.js+src/index.html+src/css/app.css. Per CLAUDE.md Tier B rules: PR is the review; auto-merge disabled even if all checks pass. Awaiting human merge.Verification
node --check src/js/main.js— rc=0node --check src/js/live_data_panel.js— rc=0node --test src/js/*.test.js— 168/168 pass (was 163, +5 new forclassifyNrc)cargo test --lib --offline protocol::— 15/15 pass (no regression on the backend surface this slice consumes)pytest backend/tests/— 166/166 pass (one test,test_app_live_endpoint::test_endpoint_returns_ok, was order-dependent flaky on first run; passed on re-run — not introduced by this slice)Cross-references
docs/v0.14.3_plan.md(slice 3 lines 103–124)parseNrcError+isUnsupportedNrcwere drafted there as dead-but-not-yet-usable helpers; this slice activates them via the per-PID error loop +classifyNrc)docs/v0.14.3_plan.md§"Open question ci(deps): bump actions/checkout from 4 to 7 #2" — the#live-unsupported-countbadge + the dim/remove UI together fulfil the deferred WIP