chore(deps): upgrade all dependencies, migrate to Rust edition 2024, and fix OBDLink time axis - #81
Conversation
…e_yml Clears all four open Dependabot alerts: - rmcp 0.12 -> 3.1.2 (high) — DNS rebinding in the streamable-HTTP server transport, vulnerable < 1.4.0. The 3.x StreamableHttpService config defaults allowed_hosts to localhost/127.0.0.1/::1, so the existing Default::default() call site is protected as-is. - serde_yml -> serde_norway (medium) and libyml (high) — both unsound and unmaintained with no patched release, so the fix is removal rather than a bump. serde_norway is the maintained serde_yaml fork; unlike serde_yaml_ng its libyaml backend is maintained too. Only two from_str call sites moved. - rand (low) — picked up 0.8.7 via the lockfile refresh, past the 0.8.6 patch. Other majors: egui/eframe/egui_extras 0.34 -> 0.36, egui_plot 0.35 -> 0.37, printpdf 0.9 -> 0.12, rust-i18n 3.1 -> 4.2, plus ~60 in-range transitive bumps (including tar 0.4.46, which carries a PAX header desync fix). Source changes the upgrades forced: - DroppedFile is now a trait with an infallible path() -> &Path rather than an Option<PathBuf> field, so the drop handler maps instead of filter_maps. - Panel::show_inside is renamed to Panel::show. The Window::show(ctx, ..) call sites are a different type and are untouched. - rmcp renamed Content to ContentBlock, and made ServerInfo/Implementation non_exhaustive, so get_info builds them through their constructors. - rmcp 3.x resolves the tool router statically via Self::tool_router() inside #[tool_handler] instead of reading a self.tool_router field, so the field is removed from UltraLogMcpServer. egui 0.36 requires Rust 1.95; verified against 1.97.1. CI already tracks stable. printpdf and rust-i18n needed no source change, so both were verified against throwaway tests before landing: printpdf emits a valid 5960-byte PDF with no warnings from the same op sequence export.rs builds, and all 15 locales still resolve with locale switching and the English fallback intact.
The toolchain bump needed for egui 0.36 brought stricter lints that fire on pre-existing code. None of these change behaviour: - sort_by with a reversed cmp -> sort_by_key with cmp::Reverse - redundant .into_iter() inside zip, redundant & in a format! arg - a guarded division -> checked_div().unwrap_or (the divisor is always >= 4) - vec![..] -> [..] where the binding is only iterated - field-by-field assignment after Default::default() -> struct literal with ..Default::default() - a nested if-let over poll_command() collapsed into one tuple pattern (a let-chain would need edition 2024) Two vacuous assertions were tightened rather than silenced. The lambda test asserted arithmetic on literals, so it now divides by LambdaCalculator's own default stoich and fails if that default moves. The MAX_CHANNELS and MAX_CHART_POINTS range checks are deliberate guardrails on consts, so they keep their assertions behind a scoped allow. tests/common/mod.rs is re-included by each submodule through #[path], so every test binary compiles it separately and reports whichever helpers that binary does not use. Allowed dead_code there and duplicate_mod at the three test crate roots. Converting the submodules to `use crate::common::*` is the real fix but is a test restructure, not a dependency change.
- actions/checkout v4 -> v7 - actions/upload-artifact v4 -> v7 - actions/download-artifact v4 -> v8 - softprops/action-gh-release v1 -> v3 Mostly Node 20 -> 24 runtime moves; all jobs use GitHub-hosted runners, which are past the 2.327.1 minimum. Two behaviour changes worth noting: checkout v7 blocks fork checkouts under pull_request_target and workflow_run, neither of which this repo triggers on; and download-artifact v8 now errors on a digest mismatch instead of warning. Upload and download are bumped together so the artifact formats stay paired. Unlike the dependency bumps these cannot be verified locally — the release workflow needs a CI run to confirm.
Refreshes the CLAUDE.md key-dependency list and the README tech-stack table. Records why serde_yml must not come back, and that rmcp's DNS-rebinding protection depends on leaving the StreamableHttpService config at its default.
The OpenECU Alliance integration was resolving nothing at runtime. Every adapter fetch failed with `missing field 'data_type'` and every protocol fetch with `missing field 'start_bit'`, so startup logged "Successfully refreshed 0 adapters and 0 protocols" and the app always fell through to embedded specs. The two sources disagree on casing. The embedded YAML from ClassicMiniDIY/OECUASpecs is snake_case, but openecualliance.org serves the same data camelCased — fileFormat, dataType, sourceNames, headerRow, extendedId, intervalMs, startBit, byteOrder. One set of structs has to read both, so every multi-word field now carries a camelCase serde alias. snake_case stays the primary name, which keeps the on-disk cache format and the YAML path unchanged. Enum values are snake_case in both sources already. Also tolerates `data_type: boolean`, which is a typo for `bool` in megasquirt-tunerstudio.adapter.yaml — one channel out of 322 across all specs. That single typo was dropping the whole megasquirt adapter. Worth correcting upstream in OECUASpecs; the alias means it stops being fatal either way. None of this surfaced because parse failures are only tracing::warn plus a filter_map. Keeping that leniency at runtime is right — one bad spec should not break startup — so the guard goes in tests instead: every embedded adapter and protocol must deserialize, and both casings must land on the same struct. Failures name the offending spec by its id. Verified end to end: startup now logs 8 adapters and 9 protocols refreshed, with no warnings, against 0 and 18 before.
Sets edition = "2024" and records rust-version = "1.95". The MSRV comes from egui/eframe 0.36, not the edition — 2024 itself only needs 1.85 — so it moves when egui moves. Declaring it turns a too-old toolchain into a clear cargo error instead of a confusing compile failure. cargo fix --edition rewrote one if-let/else in process_ipc_commands into a match; replaced with let-else, which reads better and keeps the 10-commands- per-frame cap exactly as documented in CLAUDE.md. Audited every remaining incompatibility with -W rust-2024-compatibility. Only two sites in this crate change meaning, both relative drop order, both benign: - ipc/server.rs:88 — the accept() temporary versus the JoinHandle from thread::spawn. `stream` is moved into the move closure, so the temporary is partially moved and its OwnedFd drop is already a no-op; dropping a JoinHandle detaches rather than joins, so ordering it later changes nothing. No locks are held across either. - parsers/aim.rs:656 — test-only directory walk; DirStream::drop just closes a dirfd. Verified: fmt, clippy --all-targets -D warnings, 963 tests, release build, and a real app launch with the IPC and MCP servers binding and specs refreshing.
Mechanical follow-up to the edition bump, applied with cargo clippy --fix and
cargo fmt. No behaviour change.
Edition 2024 stabilises let-chains, so clippy's collapsible_if now fires on
every nested `if x { if let Some(y) = .. }`. All 84 sites collapse to
`if x && let Some(y) = ..`. clippy only raises collapsible_if when the inner if
has no else branch, so each rewrite is equivalent.
rustfmt's 2024 style also reorders use statements (uppercase-first sorting) and
adds a trailing semicolon after `return` in a match arm.
Separated from the edition commit so that one stays readable.
Closes #80. An OBDLink (iOS) export spanning 2049 seconds rendered as a 2.05-second log. RomRaider::detect claims any CSV whose first column starts with "time", which makes it the effective catch-all for generic OBD-II exports rather than just Subaru logs — and the parser then divided every timestamp by 1000 regardless of what the header said. OBDLink writes `Time (sec)`; RomRaider's own exports write `Time (msec)`. 2049.573 seconds read as milliseconds is 2.049573. The unit is now taken from the header instead of assumed: Time (msec) / Time(ms) / Time (milliseconds) -> milliseconds Time (sec) / Time (s) / Time (seconds) -> seconds bare Time (no unit) -> milliseconds, unchanged Milliseconds are matched first because "sec" is a substring of "msec". The bare `Time` default is deliberately left as milliseconds: it is RomRaider's own convention, and inferring a unit from the data would be worse than a documented default. Verified against the file attached to the issue, which now parses 7 channels and 8083 records spanning 0.000 to 2049.573 seconds, matching the reported expectation of a 0-2049 axis. Added it under exampleLogs/obdlink/ with an end-to-end test that replicates the BOM and comment-preamble stripping the app does before dispatch, plus unit tests covering each header spelling, the millisecond path, and the European semicolon/decimal-comma locale.
There was a problem hiding this comment.
Pull request overview
This PR performs a broad maintenance + security upgrade across UltraLog, including migrating the crate to Rust edition 2024, refreshing dependency versions (notably egui/eframe, rmcp, printpdf, rust-i18n), and addressing two functional issues found during the upgrade work: RomRaider/OBDLink time-unit handling (issue #80) and OpenECU Alliance spec deserialization from the API’s camelCase JSON.
Changes:
- Migrate the crate to Rust edition 2024 (with
rust-version = "1.95") and apply mechanical let-chain/clippy-driven refactors across UI/parsers/analysis. - Fix RomRaider parsing to honor the time column’s declared unit (
Time (sec)vsTime (msec)), with new end-to-end and unit-level regression tests (closes #80). - Make OpenECU Alliance adapter/protocol types deserialize both snake_case (embedded YAML) and camelCase (API JSON) via
#[serde(alias = ...)], plus add tests to prevent silent spec parsing regressions.
Reviewed changes
Copilot reviewed 57 out of 59 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/parsers/romraider_tests.rs | Adds an end-to-end regression test for OBDLink seconds-based time headers after app preprocessing. |
| tests/parsers/mhd_tests.rs | Import ordering tweak related to comment-stripping helper usage. |
| tests/parsers/emerald_tests.rs | Formatting-only change in test data string initialization. |
| tests/parsers/ecumaster_tests.rs | Formatting-only change in synthetic sample string initialization. |
| tests/parsers.rs | Adds clippy allowance for duplicate common module inclusion across test modules. |
| tests/integration.rs | Adds clippy allowance for duplicate common module inclusion across test modules. |
| tests/integration_tests.rs | Formatting-only change in synthetic RomRaider sample string initialization. |
| tests/core/units_tests.rs | Refactors preference setup to struct-update syntax (more idiomatic + clippy-friendly). |
| tests/core/state_tests.rs | Import ordering + clippy allowances; refactors config setup to struct-update syntax. |
| tests/core/normalize_tests.rs | Switches some small vectors to fixed arrays in tests (style/mechanics). |
| tests/core/mcp_tests.rs | Import ordering + let-chain refactor in responder thread test. |
| tests/core_tests.rs | Adds clippy allowance for duplicate common module inclusion across test modules. |
| tests/common/mod.rs | Adds #![allow(dead_code)] and introduces OBDLink example file constant. |
| src/updater.rs | Minor formatting fixes (missing semicolons) in installer error paths. |
| src/ui/tools_panel.rs | Let-chain refactor to reduce nested conditionals when showing counts. |
| src/ui/tool_properties_panel.rs | Let-chain refactor around DnD drop handling guarded by capacity. |
| src/ui/timeline.rs | Let-chain refactor for cursor initialization and record indicator rendering. |
| src/ui/sidebar.rs | Let-chain refactor around file picker click handling. |
| src/ui/normalization_editor.rs | Let-chain refactor when adding a custom normalization mapping. |
| src/ui/histogram.rs | Extensive let-chain refactors + small string formatting tweak; preserves prior control flow. |
| src/ui/formula_editor.rs | Let-chain refactors and clearer preview/stats rendering logic. |
| src/ui/files_panel.rs | Let-chain refactor around file picker click handling. |
| src/ui/export.rs | Let-chain refactor for optional subtitle rendering (selected file). |
| src/ui/computed_channels_manager.rs | Import ordering + let-chain refactor around template edit action. |
| src/ui/chart.rs | Let-chain refactors for scroll-zoom, click handling, drop handling, cache-hit path. |
| src/ui/analysis_panel.rs | Import ordering + let-chain refactors in results handling and channel availability checks. |
| src/parsers/types.rs | Import ordering change for adapter metadata lookup symbols. |
| src/parsers/speeduino.rs | Let-chain refactors + safer estimated-records computation + test formatting. |
| src/parsers/romraider.rs | Implements header-driven time-unit detection and parsing; adds focused regression tests for issue #80. |
| src/parsers/motorsport_electronics.rs | Formatting-only change to test header constant. |
| src/parsers/locomotive.rs | Let-chain refactor in detection; minor iterator cleanup when filtering rows. |
| src/parsers/haltech.rs | Let-chain refactor in metadata parsing; minor iterator cleanup when filtering rows. |
| src/parsers/aim.rs | Let-chain refactors while extracting metadata tags from binary payload. |
| src/normalize.rs | Let-chain refactors in spec normalization and normalization detection helpers. |
| src/mcp/server.rs | Updates MCP server implementation for rmcp 3.x API changes (ContentBlock, ServerInfo::new, etc.). |
| src/mcp/mod.rs | Reorders re-exports. |
| src/mcp/client.rs | Import ordering tweak. |
| src/ipc/server.rs | Import ordering tweak. |
| src/ipc/handler.rs | Let-chain refactors in computed-channel removal and scatter-plot config. |
| src/expression/mod.rs | Uses sort_by_key(Reverse(..)) and a small formatting simplification in record lookup. |
| src/expression/engine.rs | Replaces a hard-coded PI constant with FRAC_PI_2 in tests. |
| src/bin/test_parser.rs | Import ordering + let-chain refactor in “interesting channels” print logic. |
| src/app.rs | Import ordering + egui API adjustments + let-chain refactors + dropped-files path handling update. |
| src/analytics.rs | Import ordering tweak. |
| src/analysis/filters.rs | Let-chain refactors when parsing numeric analyzer parameters. |
| src/analysis/derived.rs | Let-chain refactors for numeric parameters; improves lambda test to track default stoich. |
| src/analysis/afr.rs | Let-chain refactors for numeric parameters; switches a couple test vectors to arrays. |
| src/adapters/types.rs | Adds serde camelCase aliases to support API JSON; documents load-bearing casing contract. |
| src/adapters/registry.rs | Switches embedded YAML parsing to serde_norway; adds tests guarding against silent spec parse failures and casing regressions. |
| src/adapters/mod.rs | Reorders re-exports. |
| src/adapters/cache.rs | Let-chain refactors in cache directory JSON loading loops. |
| README.md | Updates tech stack section for edition 2024 + newer crate versions. |
| CLAUDE.md | Updates repository guidance for edition 2024/MSRV and documents RomRaider time-unit contract. |
| Cargo.toml | Sets edition 2024 + rust-version 1.95; upgrades dependency versions and replaces serde_yml with serde_norway. |
| build.rs | Let-chain refactors around downloader exit-status checks. |
| .github/workflows/release.yml | Bumps workflow action versions (but includes an invalid actions/checkout@v7 tag). |
| .github/workflows/ci.yml | Bumps workflow action versions (but includes an invalid actions/checkout@v7 tag). |
Suppressed comments (6)
.github/workflows/ci.yml:46
actions/checkout@v7does not appear to be a valid tag in theactions/checkoutrepository, so this workflow will fail at runtime. Use an existing major tag (e.g.v3).
uses: actions/checkout@v7
.github/workflows/ci.yml:93
actions/checkout@v7does not appear to be a valid tag in theactions/checkoutrepository, so this workflow will fail at runtime. Use an existing major tag (e.g.v3).
uses: actions/checkout@v7
.github/workflows/ci.yml:143
actions/checkout@v7does not appear to be a valid tag in theactions/checkoutrepository, so this workflow will fail at runtime. Use an existing major tag (e.g.v3).
uses: actions/checkout@v7
.github/workflows/release.yml:110
actions/checkout@v7does not appear to be a valid tag in theactions/checkoutrepository, so this workflow will fail at runtime. Use an existing major tag (e.g.v3).
uses: actions/checkout@v7
.github/workflows/release.yml:149
actions/checkout@v7does not appear to be a valid tag in theactions/checkoutrepository, so this workflow will fail at runtime. Use an existing major tag (e.g.v3).
uses: actions/checkout@v7
.github/workflows/release.yml:364
actions/checkout@v7does not appear to be a valid tag in theactions/checkoutrepository, so this workflow will fail at runtime. Use an existing major tag (e.g.v3).
uses: actions/checkout@v7
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
| uses: actions/checkout@v7 |
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
| uses: actions/checkout@v7 |
CI caught what the local build could not. `std::env::set_var` became unsafe in edition 2024, and the three call sites in `setup_linux_scaling` sit behind `#[cfg(target_os = "linux")]` — so they never compiled on the macOS host, and neither `cargo fix --edition` nor a `-W rust-2024-compatibility` audit saw them, since both only analyse code compiled for the current target. The calls are sound: `setup_linux_scaling` is the first statement in `main`, before tracing_subscriber::fmt::init, before analytics, and before eframe or the IPC/MCP servers spawn any threads, so nothing else can be reading the environment concurrently. Wrapped in a single unsafe block with a SAFETY comment recording that precondition and why the call must stay first in `main`. Audited the rest of the platform-gated code for the other edition-2024 unsafe changes — set_var/remove_var, static mut refs, extern blocks, no_mangle and friends, unsafe fn bodies, and `gen` as an identifier. These three calls were the only occurrence anywhere in the crate; the Windows and Linux blocks in updater.rs, ui/update_dialog.rs and build.rs use only std and egui APIs that edition 2024 does not touch. That matters because CI only runs tests on Linux, so Windows-gated code would otherwise first compile at release time. Verified by temporarily retargeting the cfg gate so the Linux function builds on the macOS host: it now compiles clean where it previously produced three E0133 errors.
Patch rather than minor, matching the 2.10.1 precedent — that release was also security updates plus a fix, while 2.11.0 and 2.12.0 each carried a `feat`. Nothing on this branch adds a user-facing feature. Contents: the rmcp DNS-rebinding fix and the removal of the unsound serde_yml and libyml crates, the OBDLink time-axis fix (#80), the OpenECU Alliance spec loading fix, the edition 2024 migration, and a full dependency refresh.
Dependency and toolchain upgrade pass, plus two bug fixes found along the way.
Closes #80.
Security
Clears all four open Dependabot alerts, two of them high.
rmcp< 1.4.0)0.12→3.1.2libymlserde_ymlserde_norwayrand0.8.7via lockfile refreshThe rmcp advisory covers the exact transport UltraLog runs. 3.x defaults
allowed_hoststolocalhost/127.0.0.1/::1, and the existing
Default::default()call site inherits that.serde_ymlandlibymlhave no fixed version, so the fix is removal rather than a bump. The0.0.13 release is a deprecation shim that emits
#[deprecated]at every import, which would failclippy -D warnings.serde_norwaywas chosen over the more popularserde_yaml_ngbecause thelatter still depends on the unmaintained
unsafe-libyaml. Twofrom_strcall sites changed.Dependencies
Every direct dependency is now at its latest published version. Majors:
Plus ~60 in-range transitive bumps (including
tar0.4.46, a PAX header desync fix) and six staleversion requirements in
Cargo.tomlrealigned with what actually resolves.Only
matchit(0.8.4 → 0.8.6) is left behind, because axum hard-pinsmatchit =0.8.4.GitHub Actions moved to current majors: checkout v4→v7, upload-artifact v4→v7, download-artifact
v4→v8, gh-release v1→v3.
Rust
Toolchain moved to the latest stable (1.97.1) and the crate migrated from edition 2021 to 2024,
with
rust-version = "1.95"recorded. The MSRV comes from egui 0.36, not the edition — 2024 itselfonly needs 1.85 — so it moves when egui moves.
Every remaining incompatibility was audited with
-W rust-2024-compatibility. Only two sites inthe crate change meaning, both relative drop order, both benign:
ipc/server.rs:88— theaccept()temporary vs. theJoinHandle.streamis moved into themoveclosure, so the temporary is partially moved and itsOwnedFddrop is already a no-op;dropping a
JoinHandledetaches rather than joins. No locks held across either.parsers/aim.rs:656— test-only directory walk;DirStream::dropjust closes a dirfd.Edition 2024 stabilises let-chains, so clippy's
collapsible_iffired on 84 nestedif lets.Those are applied mechanically in their own commit so this one stays readable.
Bug fixes
Issue #80 — OBDLink time axis. A 2049-second OBDLink (iOS) log rendered as 2.05 seconds.
RomRaider::detectclaims any CSV whose first column starts withtime, making it the effectivecatch-all for generic OBD-II exports, and the parser then divided every timestamp by 1000
regardless of the header. OBDLink writes
Time (sec); RomRaider's own exports writeTime (msec).The unit is now read from the header, with milliseconds matched before seconds since
secis asubstring of
msec, and a bareTimekeeping the existing millisecond default. The file from theissue is included under
exampleLogs/obdlink/and now parses 0.000 → 2049.573 s.OpenECU Alliance spec integration was resolving nothing. Found while verifying the upgrades:
every API adapter fetch failed with
missing field 'data_type'and every protocol fetch withmissing field 'start_bit', so the app silently fell back to embedded specs on every launch. Thetwo sources disagree on casing — the embedded YAML is snake_case, the API serves the same data
camelCased — so every multi-word field now carries a camelCase alias. Also tolerates
data_type: boolean, a typo inmegasquirt-tunerstudio.adapter.yaml(1 of 322 channel definitionsacross all specs) that was dropping the whole megasquirt adapter. Worth correcting upstream in
ClassicMiniDIY/OECUASpecsregardless.Startup went from 0 adapters / 0 protocols and 18 warnings to 8 / 9 and zero. This one is
user-visible: field normalization had been running on embedded specs only.
Neither failure was noticed because
registry.rsonly logs atracing::warnandfilter_mapsparse errors away. That leniency is right at runtime, so the guard went into tests instead.
Verification
cargo fmt --all -- --checkcleancargo clippy --all-targets --all-features -- -D warningsexit 0handling)
cargo build --releasesucceedsrefresh cleanly
printpdfandrust-i18ncompiled with zero source changes, which is exactly when a libraryquietly changes behaviour instead of failing loudly, so both were checked against throwaway tests
before landing: printpdf emits a valid 5960-byte PDF from the same op sequence
export.rsbuilds,and all 15 locales still resolve with switching and English fallback intact.
CI
The first run failed to compile on Linux:
std::env::set_varbecame unsafe in edition 2024, andthe three call sites in
setup_linux_scalingare behind#[cfg(target_os = "linux")]— so theynever compiled on the macOS host, and neither
cargo fix --editionnor the-W rust-2024-compatibilityaudit saw them, since both only analyse code compiled for the currenttarget. Fixed in 8af7279, with the safety argument recorded at the call site (it runs as the first
statement in
main, before any thread spawns).Prompted by that, the rest of the platform-gated code was audited for every edition-2024 unsafe
change. Those three calls were the only occurrence in the crate — which matters because CI only
runs tests on Linux, so Windows-gated code would otherwise first compile at release time.
Not verified locally
The GitHub Actions bumps only execute on GitHub.
release.ymlbuilds the cross-platform binaries,so the release workflow needs a CI run to confirm — that is the one part of this branch not
backed by local evidence.
Reviewer notes
style:commits are mechanical (clippy --fix+cargo fmt) and can be skimmed; thesubstantive changes are in
chore(deps):, the twofix(...)commits, andchore: migrate to edition 2024.updates plus a fix); 2.11.0 and 2.12.0 each carried a
feat, and nothing here adds a feature.Say the word if you'd rather signal the egui/edition overhaul with 2.13.0 instead.
rust-version = "1.95"plus edition 2024's MSRV-aware resolver means cargo will now quietlydecline dependency versions needing newer Rust rather than erroring. Nothing is affected today,
but if a future upgrade seems to mysteriously not apply, check there first.