feat(v0.14.3): per-PID NRC errors + remove_profile_pid command - #187
Merged
Conversation
Slice 3a of v0.14.3 'Finish the Bench'. Tier B (touches
src-tauri/src/protocol/** + commands.rs, adds one new Tauri
command). Backend half — slice 3b re-wires the frontend to
consume the new shape and surface the per-PID dim/remove UX.
## What this slice ships
**1. `protocol::nrc_from_error(msg: &str) -> Option<(u8, u8)>`**
(pub, in src-tauri/src/protocol/mod.rs). Pure parser that
extracts (sid, nrc) from the error string produced by
`service()`:
"ECU rejected service 22: Conditions not correct (NRC 22)"
^------^ ^-^
nrc parser sid parser
Co-located with `nrc_text` (the producer) so the format
contract has one source of truth. Case-insensitive on both
hex digits; tolerant of extra whitespace at the boundary.
4 unit tests pin the format and the failure modes (transport
timeouts, non-NRC errors, malformed hex).
**2. `read_live_data` return-type split.** Pre-v0.14.3, a single
per-PID failure short-circuited the whole sweep with
`Err(String)`; the frontend had no way to tell which PID
tripped the error and so couldn't offer a 'remove this
unsupported PID from the profile' affordance. Now returns
`LiveSweepResult { values, errors }`:
- `values`: `Vec<LiveValue>` — only the PIDs whose read AND
decode succeeded (matches pre-v0.14.3 semantics for the
happy path; no growth in the values array).
- `errors`: `Vec<LiveError>` — one entry per failed PID with
`{ id, label, sid, nrc, error }`. The structured `sid` +
`nrc` fields are populated via `nrc_from_error`; the
verbatim `error` string is preserved for the frontend's
fallback parser and for non-NRC errors (transport timeouts,
'Not connected', 'Unexpected ... response').
The whole sweep still returns `Err(_)` for systemic
problems (no transport, unknown profile, poisoned state lock)
— never for a single per-PID NRC.
**3. `live::remove_param_from_profile(profile_id, param_id)`**
(in src-tauri/src/data/live.rs). Mutates the in-memory profile
registry: removes the matching `LiveParam`, returns `true`
when something was removed / `false` for unknown ids.
Idempotent — calling twice on the same id is a no-op the
second time. 3 unit tests pin the happy path + idempotency +
unknown-profile failure mode.
**4. `remove_profile_pid` Tauri command.** New `async fn`
in src-tauri/src/commands.rs:
- Step 1: in-memory removal via `live::remove_param_from_profile`.
Returns an error to the frontend if the param id is unknown
(so the UI can distinguish 'removed' from 'already gone').
- Step 2: serialise the updated profile with `live::profile_to_toml`
(round-trips `[profile.theme]` blocks).
- Step 3: write to `<community>/profiles/<id>.toml` via
`tokio::fs::write`. `create_dir_all(parent)` first —
mirrors `save_freeze_schema`'s pattern.
- Returns the full file path the backend wrote, so the
frontend can show the user 'Removed from <path>'.
- Async because of the file I/O. NOT in the SYNC_ALLOWLIST
(per tests/async_commands.rs the allowlist gates the
*inverse*: sync commands are forbidden).
**5. Cargo.toml.** Adds `fs` to tokio's features so the
file write in `remove_profile_pid` can use `tokio::fs`.
No new crate enters the graph; tokio is already in the tree
via Tauri.
**6. lib.rs.** Registers `commands::remove_profile_pid` in
the `invoke_handler` so the frontend can call it.
## Why the frontend will need its own PR
The pre-v0.14.3 `main.js::pollOnce` does `for (const v of
values)` against the result, where `values` was a
`Vec<LiveValue>`. Slice 3a returns `{ values, errors }`
instead. The frontend has to switch to
`result.values.forEach(...) + result.errors.forEach(...) `,
which is a non-trivial rewire of the same area that landed
the v0.14.2 slice 2 PR (#177). That's slice 3b; this PR
lands the backend it consumes.
## Verification
- `cargo test --offline --lib data::live` — 35/35 pass
(was 32 + 3 new `remove_param_from_profile` tests).
- `cargo test --offline --lib protocol::` — 15/15 pass
(was 11 + 4 new `nrc_from_error` tests).
- `cargo test --offline --lib` — 149/149 pass (full
src-tauri suite; no regressions; +7 tests total).
- `cargo check --offline` — clean.
- `cargo clippy --offline --lib` — no new warnings on
src/protocol/mod.rs, src/data/live.rs, src/commands.rs.
- Pre-existing `cargo fmt` diff in src/analysis.rs is
unrelated to this PR.
- `node --test src/js/**/*.test.{js,cjs}` — 221/221 pass
(no JS changes; baseline unaffected).
- `node scripts/lint-toml.js` — 37/0 clean (no TOML changes).
## Tier
B. Touches `src-tauri/src/protocol/**` + `src-tauri/src/commands.rs`
+ adds a new Tauri command. `src-tauri/src/transport/**`
untouched. Will need a human merge per CLAUDE.md Tier B
rules; auto-merge disabled even if all checks pass.
Wait — actually this is the *cycle plan's* Tier B call.
Looking at the file paths: `protocol/**` + `commands.rs`
are both on the protected-path list. Slice 3b (frontend-only)
will land as Tier A after this. I haven't changed the
release tag or pushed to a production server, so no Tier C.
Contributor
|
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: 0e9f667200
ℹ️ 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 on lines
300
to
+303
| pub async fn read_live_data( | ||
| state: tauri::State<'_, AppState>, | ||
| profile: String, | ||
| ) -> Result<Vec<live::LiveValue>, String> { | ||
| ) -> Result<LiveSweepResult, String> { |
There was a problem hiding this comment.
Every successful invocation now serializes an object shaped as { values, errors }, but all three existing callers still treat the result as the previous array: pollOnce and logTick iterate it at src/js/main.js:1367-1368 and 2121-2123, while buildLogParams calls .forEach at 2031-2039. Consequently, the first successful live poll throws values is not iterable and stops polling, and logging setup similarly fails; update these consumers to unwrap .values and process .errors, or preserve the existing return contract.
Useful? React with 👍 / 👎.
Comment on lines
+756
to
+760
| // Step 2: serialise the updated profile to TOML. `profile_to_toml` | ||
| // already exists; it round-trips the `[profile.theme]` block too | ||
| // so a per-profile gauge theme survives the edit. | ||
| let toml_src = live::profile_to_toml(&profile_id) | ||
| .ok_or_else(|| format!("Profile '{profile_id}' vanished mid-edit"))?; |
There was a problem hiding this comment.
When this command is used on profiles such as n55, n57, or b58, profile_to_toml serializes the remaining parameters without their LiveParam.enum_map. The subsequent write replaces the original profile file, so after restart every remaining u8_enum parameter loses labels such as gear and engine state and renders raw 0xNN ? values instead; serialize each parameter's enum map before using this output as persistent storage.
Useful? React with 👍 / 👎.
Comment on lines
+764
to
+766
| let dir = crate::community::find_dir() | ||
| .unwrap_or_else(|| PathBuf::from("community")); | ||
| let path = dir.join("profiles").join(format!("{profile_id}.toml")); |
There was a problem hiding this comment.
This assumes every loaded profile came from <profile_id>.toml, but the loader also accepts profiles.toml and files whose basename differs from the ID. For the repository's e70_n62_example profile, this creates e70_n62_example.toml while the original remains in example_e70_n62.toml; sorted startup loading reads the generated file first and then replaces it with the original profile, so the removed PID returns on the next launch. Track and update the source file or load persisted edit overlays after all source profiles.
Useful? React with 👍 / 👎.
Comment on lines
+764
to
+766
| let dir = crate::community::find_dir() | ||
| .unwrap_or_else(|| PathBuf::from("community")); | ||
| let path = dir.join("profiles").join(format!("{profile_id}.toml")); |
There was a problem hiding this comment.
Profile IDs loaded through community TOML or import_profiles are not restricted to safe filename components, yet the ID is inserted directly into this path. A profile with an ID such as ../../src-tauri/Cargo can therefore make the remove command overwrite src-tauri/Cargo.toml (or another writable .toml outside community/profiles) when the user removes one of its parameters; validate the ID as a filename or derive a sanitized, confined path before writing.
Useful? React with 👍 / 👎.
| // Step 2: serialise the updated profile to TOML. `profile_to_toml` | ||
| // already exists; it round-trips the `[profile.theme]` block too | ||
| // so a per-profile gauge theme survives the edit. | ||
| let toml_src = live::profile_to_toml(&profile_id) |
There was a problem hiding this comment.
When a sweep exposes multiple unsupported PIDs and the user removes two in quick succession, both async commands can snapshot the profile before either awaited write completes. For example, removal A can serialize a profile that still contains PID B, removal B can serialize the fully updated profile, and then A's write can finish last, restoring PID B on disk even though the in-memory store says it was removed. Serialize the mutation/snapshot/write sequence per profile or regenerate the snapshot under a write queue immediately before persistence.
Useful? React with 👍 / 👎.
4 tasks
github-actions Bot
pushed a commit
that referenced
this pull request
Jul 30, 2026
docs/validation/n62-real-car.md: add the three v0.14.3 PIDs (0x5E fuel rate L/h, 0x5F engine runtime s, 0x62 fuel rate g/s) to Step 2 cold-reading + Step 3 running-reading tables. Add critical-row paragraph for fuel-rate failure modes. Extend Step 4 report template + Step 5 consequences. Update cross-references to point at PRs #185/#186/#187 and docs/DECODE_FUNCTIONS.md sections 10-12. ROADMAP.md: close v0.14.3 cycle table (slices 1, 2, 3a, 4 done; slice 3b frontend rewire still open). Bump "Last updated" header. Correct the stale v0.14.2 cycle-table notes that said the deferred PIDs were v0.14.3+ work (they ship in v0.14.3). CHANGELOG.md: add the first entry since v0.14.0 — marked "Unreleased" because slice 3b is still open and the v0.14.3 release cut cannot happen until slice 3b merges. Notes on the version surface explain why the README badge stays at v0.14.0 (CLAUDE.md golden rule #5: don't let the badge lie about an incomplete cycle). Also flags v0.14.1 and v0.14.2 as missing CHANGELOG entries; the maintainer's housekeeping follow-up is the appropriate scope for that backfill. Tier A (docs only). No code changes, no transport/** changes, no protocol/** changes. PR auto-merge eligible per CLAUDE.md once CI is green. Findings flagged (not fixed in this PR — Tier B surface, CLAUDE.md golden rule #3: don't widen PR scope): - src-tauri/src/data/live.rs::tests::remove_param_from_profile_is_idempotent fails on origin/main @ 093b063 (PR #187). Test asserts N-1 params after second remove; returns N. Likely HashSet ordering issue in the test fixture (not the production code path). Worth a separate Tier B fix PR. Co-authored-by: ohgeeceee <ohgeeceee@users.noreply.github.com>
3 tasks
ohgeeceee
added a commit
that referenced
this pull request
Jul 30, 2026
…nfig-fix) (#189) The review workflow's `claude-code-action@v1` (floating tag) has been failing on every PR since at least 2026-07-29 with: Internal error: directory mismatch for directory "/home/runner/work/_actions/anthropics/claude-code-action/<ref>/tsconfig.json", fd 4. ... Claude result reported subtype success with is_error:true Confirmed in the repo: 10 consecutive claude-review run failures spanning the v0.14.2-slice3, v0.14.3-slice1/2/3, v0.14.3-doc-rot and PRs #188 / #187 / #186 / #185 branches. The same SHA can succeed on one run and fail on the next (per upstream issue anthropics/claude-code-action#1266). Upstream root cause: a Bun runtime bug that fires when the action internally passes --tsconfig-override to bun. Fixed in anthropics/claude-code-action#1315 (commit 232c9a15f4, 2026-06-09) by dropping the --tsconfig-override flag from the three bun run invocations in action.yml. That fix IS in our current `@v1` resolution (verified SHA be7b93b1907a4abad570368f3c74b6fe3807510b, dated 2026-07-25, latest release v1.0.183). So the fix landed but the abort case still fires on our ubuntu-24.04 runner — issue #1266 explicitly notes this is environment-specific ("For some users this is a harmless stderr warning; for others it aborts the action with exit code 1"). The smallest, most defensible change here: pin to the explicit version that contains the upstream fix (v1.0.183, the latest at this time) instead of the floating `@v1`. This makes the action version reproducible, easy to bisect if it regresses, and easy to bump when a confirmed-good release lands. Past fix attempts in this repo: - PR #173 (empty — abandoned) - PR #174 (empty — abandoned) - PR #176 (removed `Bash(gh pr review:*)` from --allowedTools — didn't fix the underlying issue, the bug has continued firing since) Files changed (3): - .github/workflows/claude-review.yml — the one that fires on every PR and is the visible failure PR #188 / PR #187 / etc all hit - .github/workflows/claude.yml — Claude Code Action itself - .github/workflows/claude-implement-issue.yml — Claude Code Implement Issue workflow Tier A (CI workflows per CLAUDE.md). PR auto-merge eligible. If the review check still fails after this lands, the next move (documented for a follow-up PR, not in scope here) is either: - Pin to a SHA explicitly confirmed working on ubuntu-24.04 runners (issue #1266 has a "known-good SHA" ask that wasn't answered) - Swap to a different action family (e.g. claude-code-base-action has fewer internal bun invocations and doesn't trip the same bug) - Disable the workflow trigger and rely on local agent dispatch, the workaround issue #1266's reporter ended up using Verification: - [x] Workflow YAML lints clean (CI lint step on the patch output) - [x] v1.0.183 is the latest release at this time per https://github.com/anthropics/claude-code-action/releases (verified 2026-07-30) - [x] Same SHA (`be7b93b1`) is what `@v1` resolves to currently, so this pin is functionally equivalent to the floating tag for today — but is now explicit and reproducible Co-authored-by: ohgeeceee <ohgeeceee@users.noreply.github.com>
This was referenced Jul 30, 2026
ohgeeceee
added a commit
that referenced
this pull request
Jul 31, 2026
…#190) 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 Co-authored-by: ohgeeceee <ohgeeceee@users.noreply.github.com>
6 tasks
ohgeeceee
added a commit
that referenced
this pull request
Jul 31, 2026
…issue #191) (#193) Closes #191. The three `remove_param_from_profile_*` tests shipped with PR #187 (v0.14.3 slice 3a) failed intermittently under `cargo test --lib --offline` parallel execution: thread 'data::live::tests::remove_param_from_profile_is_idempotent' panicked at src/data/live.rs:743:9: assertion `left == right` failed left: 10 right: 9 ## Root cause The three tests share a process-global profile store (`static STORE: OnceLock<RwLock<Vec<Profile>>>` in `data::live.rs`). `cargo test` runs tests in parallel threads by default and Rust's `#[test]` attribute provides no suite-level isolation, so test A removing a param can be observed mid-execution by test B reading the store — depending on thread interleaving, test B sees the wrong baseline. `profile_params()` (a reader) does `store().read()`, and `remove_param_from_profile` (a writer) does `store().write()`. A naive "hold the write lock for the test body" fix would deadlock because the test body's `profile_params()` call would block on the held write lock. ## Fix A dedicated `static TEST_LOCK: Mutex<()>` inside the `tests` module serialises the three tests against each other without touching the store's RwLock. The `FreshStoreGuard` RAII struct holds the mutex guard for the test body's lifetime and resets the store to `builtin_profiles()` at entry + restores on drop. Other tests (e.g. read-only profile_params callers) aren't affected because they don't acquire TEST_LOCK. Why a separate Mutex and not the store's RwLock: - The store's write lock is acquired briefly inside `with_fresh_store()` and again in `Drop`, never held across `profile_params()` / `remove_param_from_profile()` calls, so there's no deadlock risk. - TEST_LOCK is a test-scope ordering lock. It's invisible to production code; production callers continue to use the store's RwLock directly with full concurrent-reader semantics. ## Verification - [x] `cargo test --lib --offline remove_param_from_profile` — 3/3 pass (was 1/3 or 2/3 before fix, depending on interleaving). Stable across 5 consecutive runs. - [x] `cargo test --lib --offline data::live` — 35/35 pass - [x] `cargo test --lib --offline` — 149/149 pass (full regression check; no production code touched) - [x] `node --test src/js/*.test.js` — 163/163 pass (no JS changes) - [x] `pytest backend/tests/` — 166/166 pass - [x] `npm run build` — rc=0, both MSI + NSIS bundles built ## Tier **B** per CLAUDE.md — `src-tauri/src/data/live.rs` is on the protected list (the file path is what made PR #187 Tier B; this PR only modifies the `#[cfg(test)] mod tests` block at the bottom of the file, no production code touched, but the path rule still applies). Per CLAUDE.md Tier B rules: PR is the review; auto-merge disabled even if all checks pass. Awaiting human merge. ## Diff +61 / -0. Test-only changes inside `#[cfg(test)] mod tests`. No production-code changes. ## Cross-references - Issue #191 — "[bug] data::live::tests::remove_param_from_profile_is_idempotent fails on origin/main (PR #187)" - PR #187 — feat(v0.14.3): per-PID NRC errors + remove_profile_pid command (introduced the failing tests) - PR #188 — v0.14.3 slice 4 (originally flagged the failure as out-of-scope; this PR is the focused fix) - PR #189 — fix(ci): pin anthropics/claude-code-action (a different, unrelated workflow issue; not touched here) Co-authored-by: ohgeeceee <ohgeeceee@users.noreply.github.com>
5 tasks
ohgeeceee
added a commit
that referenced
this pull request
Jul 31, 2026
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>
This was referenced Jul 31, 2026
github-actions Bot
pushed a commit
that referenced
this pull request
Aug 2, 2026
…rate / runtime PIDs (#223) Tier A, data only. v0.14.5 slice 1 of 3. Mirrors the v0.14.2 N62 slice 1 (PR #175) and the v0.14.3 N62 slice 2 (PR #186) pattern: swap the unverified `local:10` oil-temp placeholder for the standard SAE J1979 PID `0x5C`, then add the three v0.14.3 PIDs (0x5E fuel rate L/h, 0x5F engine runtime, 0x62 fuel rate g/s). Each new entry carries the `[needs verification, N5x/E9x bench]` marker per the v0.14.3 N62 discipline. Changes per file: n52.toml: - oil placeholder: `local:10` (labelled `[UNVERIFIED placeholder]`) -> `obd:5C` (engine oil temperature, `byte - 40 °C`, decoder `temp_u8`) - 3 new entries: fuel_rate_lh (0x5E), engine_runtime (0x5F), fuel_rate_gs (0x62) - profile label: `[community, oil temp unverified]` -> `[community]` (the conservative-sourcing marker stays until a real-car report lifts it) - header block: BSD oil-condition sensor note (N52's known oil-temp quirk — DME reads oil condition via BSD, not KWP2000; the OBD-II `0x5C` swap is the surface the desktop app reads, but the harness-doc path reverts to `local:10` if the DME returns an NRC for `0x5C`) - 10 -> 13 [[profile.param]] entries n54.toml: - same oil placeholder swap + 3 new entries - profile label: `[community, oil temp unverified]` -> `[community]` - header block: BSD note + charge-air / boost / HPFP / idle-voltage context (N54-specific; twin-turbo pulls more fuel at WOT than the NA N52 / N62 — fuel-rate ranges bumped to ~80-150 L/h and ~60-100 g/s) - 12 -> 15 [[profile.param]] entries No Rust change. All four OBD-II PIDs reuse decoders that shipped in v0.14.3 PR #185 (temp_u8, u16_fiftieths, u32_be, u16_half). The v0.14.3 PR #187 per-PID NRC surface flags the N52 / N54 `0x5C` BSD-not-supported failure mode in the UI; the `remove_profile_pid` async command writes the updated TOML behind a tauri-plugin-dialog confirmation per the issue-#161 fix pattern. Bench verification on the E9x is the gating step — see `docs/validation/n5x-real-car.md` (v0.14.5 slice 2). 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>
5 tasks
github-actions Bot
pushed a commit
that referenced
this pull request
Aug 2, 2026
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>
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.
Slice 3a of v0.14.3 — "Finish the Bench"
Cycle plan:
docs/v0.14.3_plan.md. Tier B — touchessrc-tauri/src/protocol/**+src-tauri/src/commands.rs+ adds a new Tauri command. No Tier A self-merge per CLAUDE.md; waiting on a human merge even if CI is green.What this slice ships
1.
protocol::nrc_from_error— structured (sid, nrc) from the protocol error stringpub fn nrc_from_error(msg: &str) -> Option<(u8, u8)>insrc-tauri/src/protocol/mod.rs. Parses the canonicalservice()error format:Co-located with
nrc_text(the producer) so the format contract has one source of truth. Case-insensitive on both hex digits; tolerant of extra whitespace. 4 unit tests pin the happy path + failure modes (transport timeouts, non-NRC errors, malformed hex).2.
read_live_datareturn-type splitPre-v0.14.3, a single per-PID failure short-circuited the whole sweep with
Err(String); the frontend had no way to tell which PID tripped the error. Now returnsLiveSweepResult { values, errors }:values:Vec<LiveValue>— only the PIDs whose read AND decode succeeded.errors:Vec<LiveError>— one entry per failed PID with{ id, label, sid, nrc, error }. Structuredsid+nrcare populated vianrc_from_error; the verbatimerrorstring is preserved for the frontend's fallback parser and for non-NRC errors (timeouts, 'Not connected', 'Unexpected ... response').The whole sweep still returns
Err(_)for systemic problems (no transport, unknown profile, poisoned state lock) — never for a single per-PID NRC.3.
live::remove_param_from_profile+remove_profile_pidTauri commandIn-memory helper (
live.rs) + new async Tauri command (commands.rs) that:LiveParamfrom the in-memory profile registry (returnstruewhen something was removed,falsefor unknown ids).live::profile_to_toml(round-trips[profile.theme]blocks).<community>/profiles/<id>.tomlviatokio::fs::write(create_dir_all(parent)first, mirroringsave_freeze_schema's pattern).'Removed from <path>'.Async because of the file I/O. NOT in the
SYNC_ALLOWLIST(pertests/async_commands.rsthe allowlist gates the inverse: sync commands are forbidden).4. Cargo.toml —
fsfeature on existing tokio deptokio = { version = "1", features = ["time", "fs"] }. No new crate enters the graph; tokio is already in the tree via Tauri. Thefsfeature enablestokio::fs::writeforremove_profile_pid.5. lib.rs
Registers
commands::remove_profile_pidin theinvoke_handlerso the frontend can call it.Why the frontend will need its own PR
The pre-v0.14.3
main.js::pollOncedoesfor (const v of values)against the result, wherevalueswas aVec<LiveValue>. Slice 3a returns{ values, errors }instead. The frontend has to switch toresult.values.forEach(...) + result.errors.forEach(...), which is a non-trivial rewire of the same area that landed the v0.14.2 slice 2 PR (#177). That's slice 3b; this PR lands the backend it consumes.Verification
cargo test --offline --lib data::live— 35/35 pass (was 32 + 3 newremove_param_from_profiletests).cargo test --offline --lib protocol::— 15/15 pass (was 11 + 4 newnrc_from_errortests).cargo test --offline --lib— 149/149 pass (full src-tauri suite; no regressions; +7 tests total).cargo check --offline— clean.cargo clippy --offline --lib— no new warnings on src/protocol/mod.rs, src/data/live.rs, src/commands.rs.node --test src/js/**/*.test.{js,cjs}— 221/221 pass (no JS changes; baseline unaffected).node scripts/lint-toml.js— 37/0 clean (no TOML changes).Tier
B —
src-tauri/src/protocol/**+src-tauri/src/commands.rs+ new Tauri command.src-tauri/src/transport/**untouched. Per CLAUDE.md Tier B rules: PR is the review; auto-merge disabled even if all checks pass. Awaiting human merge.