Skip to content

fix: error rows written to wrong file and default sensors mismatch - #5

Merged
k-rister merged 1 commit into
mainfrom
fix-mlxreg-bugs
Jul 9, 2026
Merged

fix: error rows written to wrong file and default sensors mismatch#5
k-rister merged 1 commit into
mainfrom
fix-mlxreg-bugs

Conversation

@k-rister

@k-rister k-rister commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fix error rows written to wrong file path: error rows used ${device//://} (colon→slash, creating nested dirs) while normal rows used ${device//:} (remove colons). Data went to different files.
  • Align default sensors with documentation: code defaulted to 1,2,6,127 but README and all usage examples document 1,2,6. Sensor 127 is not a valid MVCR sensor index.

Note: the Jira ticket also flagged the CDM metric class as potentially wrong (count vs power), but investigation shows count is correct per the CDM specification — power watt readings are a gauge/level measurement which falls under count. tool-power using power may itself be a bug.

Closes #4

Test plan

  • CI passes
  • Verify error and normal CSV rows go to the same file
  • Verify default sensor collection matches 1,2,6

🤖 Generated with Claude Code

@k-rister k-rister self-assigned this Jul 6, 2026
@k-rister
k-rister requested a review from HughNhan July 6, 2026 14:00
@project-crucible-tracking project-crucible-tracking Bot moved this to In Progress in Crucible Tracking Jul 6, 2026
Comment thread mlxreg-collect
Comment thread mlxreg-start Outdated
Error rows used ${device//://} which replaces colons with slashes,
creating nested directories instead of writing to the same file as
normal data.  The initial fix (removing colons) could cause
collisions — e.g. 0000:03:00.0 and 0000:030:0.0 both produce
000003000.0.csv.  Using underscores (${device//:/_}) produces
0000_03_00.0.csv which is both unique and reversible.

Update the post-process filename parser to use the simpler
replace('_', ':') instead of the regex-based approach.

Closes: #4

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@k-rister

k-rister commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Updated per review feedback:

  1. Sensor 127: Restored — the README was out of date, not the code. The sensor 127 commit is dropped entirely (no net change to mlxreg-start).

  2. File path separator: Adopted Hugh's underscore suggestion (${device//:/_}). Produces 0000_03_00.0.csv — unique and reversible. Applied to all four locations in mlxreg-collect (header creation, normal rows, both error paths). Updated the post-process filename parser to use the simpler replace('_', ':') instead of the regex approach.

Force-pushed a single clean commit.

@atheurer atheurer left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

File path fix looks good — colon→underscore is the right choice and the post-process reverse mapping is cleaner.

One note: the PR description mentions aligning default sensors from 1,2,6,127 to 1,2,6, but I don't see that change in the diff. Was it dropped or is it planned for a separate PR?

@k-rister

k-rister commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

One note: the PR description mentions aligning default sensors from 1,2,6,127 to 1,2,6, but I don't see that change in the diff. Was it dropped or is it planned for a separate PR?

Apparently 127 is a valid sensor and the documentation was lacking so it remains.

@k-rister
k-rister merged commit 59cca83 into main Jul 9, 2026
1 check passed
@k-rister
k-rister deleted the fix-mlxreg-bugs branch July 9, 2026 13:18
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Crucible Tracking Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix: error-status data written to different files than normal data

3 participants