Repository navigation
fix(mhd): support MHD Tuning CSV logs and '#'-prefixed metadata blocks (#78) - #79
Merged
Merged
Conversation
…ocks
MHD Tuning exports (and OBD-II apps such as OBDLink) write a block of
'#'-prefixed metadata lines ahead of the CSV header row, plus a UTF-8
BOM. No parser looked past that preamble, so detection fell through to
Haltech and the log opened with zero channels.
- Add src/parsers/mhd.rs — detects the '#Ecu CALID'/'#Ecu PRGID' markers
or the trailing MHD tune column, reads units from the header
('(*C)' -> °C, '(*CRK)' -> °CRK, '(-)' -> unitless), treats Time as
seconds relative to the first sample, and carries the last known value
over unparseable fields
- Normalize text content before format detection in parse_text_content:
strip a leading UTF-8 BOM, then drop a leading '#' comment block via
the new parsers::strip_leading_comment_lines, so every other CSV
parser sees the real header row
- Wire MHD through Channel/Meta/EcuType, the app dispatch, and
test_parser
- Add a redacted MHD example log and parser integration tests
Fixes #78
There was a problem hiding this comment.
Pull request overview
This pull request fixes issue #78 by ensuring text-based log format detection sees the real CSV header (not a leading UTF-8 BOM or #-prefixed metadata preamble) and by adding a dedicated MHD Tuning CSV parser so MHD logs aren’t misdetected as other Time-prefixed formats.
Changes:
- Add central text preprocessing in
UltraLogApp::parse_text_contentto strip UTF-8 BOM and drop leading#metadata blocks before running the existing text parser detection chain (with an MHD-specific raw-text exception). - Add a new
src/parsers/mhd.rsparser (with unit normalization and metadata capture) and integration/unit tests for detection and parsing behavior. - Update docs/examples and bump the crate version to
2.12.0.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/parsers/mod.rs | Registers the new MHD parser integration test module. |
| tests/parsers/mhd_tests.rs | Adds integration tests for MHD detection/parsing and comment-block stripping behavior. |
| tests/common/mod.rs | Adds the MHD example log path constant used by tests. |
| src/parsers/types.rs | Wires Mhd into Channel, Meta, and EcuType. |
| src/parsers/mod.rs | Exposes the new MHD parser and adds strip_leading_comment_lines() helper + tests. |
| src/parsers/mhd.rs | Implements the MHD Tuning CSV parser and its unit/detection logic. |
| src/bin/test_parser.rs | Mirrors the app’s preprocessing and adds explicit MHD detection/parsing in the CLI utility. |
| src/app.rs | Adds preprocessing + MHD special-case dispatch in parse_text_content. |
| README.md | Documents MHD support, updates the tree listing, and adds an MHD trademark note. |
| exampleLogs/mhd/mhd-n55-pull.csv | Adds a VIN-redacted MHD sample log used for tests/verification. |
| CLAUDE.md | Updates repo documentation to include MHD and documents the text preprocessing invariant. |
| Cargo.toml | Bumps version to 2.12.0. |
| Cargo.lock | Updates locked ultralog version to 2.12.0. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+218
to
+221
| let channels: Vec<Channel> = header_fields[1..] | ||
| .iter() | ||
| .map(|h| Channel::Mhd(MhdChannel::from_header(h))) | ||
| .collect(); |
- Exclude the MHD tune-file column from channels: its header is a file name and its data column is a constant map marker, not a logged signal. The name is already captured in MhdMeta::tune. - Keep the parsed unit when a header is only a unit, e.g. "(Bar)" - the raw text still becomes the name so the channel stays addressable
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
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.
Fixes #78
Problem
MHD Tuning CSV exports opened with zero channels — the app appeared to do nothing.
The file from the issue looks like this:
Two things broke it:
#Encoding: UTF-8, not the header row, so no parser matched and detection fell through to the Haltech default — which only acceptsHH:MM:SSdata rows and returned 0 channels, 0 points.@andris-silis reported the same failure with OBDLink exports, which use the same
#preamble convention.Fix
Central preprocessing in
UltraLogApp::parse_text_content(src/app.rs), so no parser has to repeat it:#-prefixed comment lines via the newparsers::strip_leading_comment_lines, so the real CSV header row is the first line detection seesThat alone fixes the OBDLink class of failure. Only the leading contiguous block is removed — a
#after the header row stays as data.New MHD parser (
src/parsers/mhd.rs). Without it, MHD logs would now be picked up by RomRaider, which treatsTimeas milliseconds and would compress a 29-second pull into 29 ms. The parser:#Ecu CALID/#Ecu PRGIDmarkers, or on the trailing tune column (MHD 5.26 IJE0S ST2V7.bin)(*C)→ °C,(*)→ °,(*CRK)→ °CRK,(-)→ unitless.rfindon the parens keeps names likeMAF Req (wgdc) (g/s)intactTimeas seconds, normalized relative to the first sampletimesstays sorted (computed-channel time-shift lookups binary-search it)#Encoding/#Ecu CALID/#Ecu PRGID/#VINand the tune name intoMhdMetaMHD detection deliberately runs on the raw text, ahead of the chain, because its fingerprint lives in the comment block the strip removes. That invariant is documented in
CLAUDE.md.Version
2.11.0→2.12.0. 2.11.0 was never released (latest tag isv2.10.1), but this adds a user-facing ECU format, so it gets its own minor.Verification
cargo run --bin test_parseragainst the exact file attached to the issue:228 data rows out of 233 file lines (4 comment + 1 header) — nothing dropped. Spot-checked value alignment against the raw row.
cargo test— all pass, including 6 new MHD integration tests and 13 new unit testscargo fmt --all -- --check— cleancargo clippy --all-targets --all-features— no findings in any changed filecargo build --release— cleanNotes
exampleLogs/mhd/mhd-n55-pull.csvis the reporter's file with the VIN redacted — it is real customer data and the VIN identifies their car.cargo clippy --all-targets --all-features -- -D warningscurrently fails on pre-existing findings insrc/analysis/derived.rs,src/expression/engine.rs,tests/parsers/romraider_tests.rs, and dead code intests/common/mod.rs. These are onmainalready (newer clippy than CI's toolchain) and are out of scope here.