fix(CSVLogManager): write rotation-stop marker once and header on open - #112
Merged
DonLakeFlyer merged 1 commit intoSep 5, 2026
Merged
Conversation
rotationIsEnding() called csvLogRotationStop() and then csvStopRotationPulseLog(), which itself called csvLogRotationStop() again, so every rotation end wrote two COMMAND_ID_STOP_ROTATION rows to the full pulse log. Drop the nested call so the marker is written once, paired with the unconditional START written by startRotation(). Also write the CSV header immediately after opening each log file instead of lazily on the first pulse. The files are opened WriteOnly (truncating), so the header is still written exactly once; runs that receive no pulses now produce a header-only file instead of an empty one.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused changes correctly implement both documented fixes without introducing unresolved issues.
Pull request overview
Ensures CSV logs receive headers on creation and prevents duplicate rotation-stop records.
Changes:
- Writes pulse headers immediately after successful file opening.
- Removes the redundant STOP marker emitted while closing rotation logs.
File summaries
| File | Description |
|---|---|
custom/src/CSVLogManager.h |
Declares the header-writing helper. |
custom/src/CSVLogManager.cc |
Writes headers on open and eliminates duplicate STOP logging. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Summary
Two related fixes in
CSVLogManager:Duplicate rotation-stop marker.
rotationIsEnding()calledcsvLogRotationStop()and thencsvStopRotationPulseLog(), which itself calledcsvLogRotationStop()again. Every rotation end wrote twoCOMMAND_ID_STOP_ROTATIONrows to the full pulse log. The nested call is removed so the marker is written exactly once, paired with the unconditional START written bystartRotation().CSV header written on open. The header is now written immediately after each log file is opened (
_csvWritePulseHeader) instead of lazily on the first pulse guarded bysize() == 0. Files are openedWriteOnly(truncating), so the header is still written exactly once.Behavior changes
Notes
custom/tree yet; addingCSVLogManagercoverage (start/stop/reopen, header-once, empty-run) is a follow-up.