Skip to content

WPILOGWriter: stop logging gracefully when disk is full to prevent robot loop overruns#260

Open
jasondaming wants to merge 3 commits into
Mechanical-Advantage:mainfrom
jasondaming:fix/wpilogwriter-disk-full-shutdown
Open

WPILOGWriter: stop logging gracefully when disk is full to prevent robot loop overruns#260
jasondaming wants to merge 3 commits into
Mechanical-Advantage:mainfrom
jasondaming:fix/wpilogwriter-disk-full-shutdown

Conversation

@jasondaming
Copy link
Copy Markdown

Summary

Fixes #259

When log storage fills up, synchronous flush() calls in putTable() can block the main robot loop, causing erratic mechanism behavior mid-match (e.g. mechanisms barely responding, commands not executing). This was observed in competition when a USB drive filled up and write errors caused the logger's queue to back up and stall the 20ms robot loop.

  • At startup, reset any stale alerts from a prior run, then check free space:
    • < 50 MB: raise a persistent kError Alert and abort logging entirely
    • < 1 GB: raise a persistent kWarning Alert but continue logging
  • Every ~10 seconds during operation (putTable()), recheck free space with the same thresholds; on < 50 MB, close the log and set isOpen = false so the existing guard short-circuits all future calls without blocking on I/O
  • Clear lowSpaceAlert in the periodic check if space rises back above 1 GB
  • Guard end() against calling log.close() if the log was already closed by the disk-full path
  • Add :::warning admonition to existing-projects.md and vscode-welcome.md documenting the thresholds and advising teams to clear logs regularly

Test plan

  • Build passes (./gradlew :akit:compileJava)
  • Spotless passes (./gradlew spotlessCheck)
  • Tested locally via publishToMavenLocal and consumed in a robot project running in simulation
  • Verified warning Alert fires when free space is below 1 GB at startup
  • Verified error Alert fires and logging stops cleanly when free space is below 50 MB
  • Verified no NPE in end() when logging was stopped early by the disk-full path

🤖 Generated with Claude Code

jasondaming and others added 3 commits March 30, 2026 17:22
When log storage fills up, synchronous flush() calls in putTable() can
block the main robot loop, causing erratic mechanism behavior mid-match.

- At startup: fire kError Alert and abort if < 50 MB free; fire kWarning
  Alert if < 1 GB free but continue logging
- Every ~10 seconds during operation: recheck free space with the same
  thresholds; on < 50 MB, close the log and set isOpen=false so the
  existing guard short-circuits all future calls without blocking on I/O
- Guard end() against calling log.close() if already closed by the
  disk-full path

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add :::warning admonition to existing-projects.md and vscode-welcome.md
  explaining the 1 GB warning and 50 MB shutdown thresholds
- Apply Spotless formatting to WPILOGWriter.java

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… recovers

- Reset both alerts at the top of start() so stale warnings from a
  previous run don't persist after a reboot with a cleared drive
- In the periodic check, explicitly clear lowSpaceAlert in the else
  branch so the alert dismisses if space somehow rises back above 1 GB

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@katzuv
Copy link
Copy Markdown
Contributor

katzuv commented Mar 31, 2026

Shouldn't this be handled at the WPILOGWriter level and not in AdvantageKit?

@jasondaming
Copy link
Copy Markdown
Author

jasondaming commented Mar 31, 2026

WPILOGWriter isn’t a WPILib class — it’s an AdvantageKit class. So if your point is that this should be handled in WPILOGWriter instead of in higher-level logging code, I agree, and that’s what this change does.

If you mean WPILOGWriter itself should be moved into WPILib, I don’t think that makes sense here.

Let me know if I’m misunderstanding your comment.

@katzuv
Copy link
Copy Markdown
Contributor

katzuv commented Mar 31, 2026

Sorry, I misunderstood. I thought AdvantageKit utilizes WPILib's logging capabilities. Is that not the case? I guess maybe not, as AdvantageKit was created before WPILib had logging.

@blaze-developer
Copy link
Copy Markdown
Contributor

blaze-developer commented Mar 31, 2026

AdvantageKit does utilize WPILib's logging. WPILogWriter is the class in AdvantageKit that handles writing periodic tables to WPILOG files

If you are curious, consider reading the source code of that class or some other parts of AdvantageKit for a better understanding of how things are structured. That way you'll be more informed with your contributions and suggestions.

@katzuv
Copy link
Copy Markdown
Contributor

katzuv commented Mar 31, 2026

Thanks, what I meant to say in general is that -- can this feature be added to WPILib's side, perhaps in DataLogWriter? Which seems like AdvantageKit is using.

@jasondaming
Copy link
Copy Markdown
Author

DataLogWriter is not the proper place for this kind of check. It is a basic call and any policy decisions should be handled by the higher level library.

For WPILib we use DataLogManager and take a different approach of deleting old log file until there is enough space. I think that is an equally valid solution to this problem it is just a matter of taste. If AKit wanted to go that direction instead we could.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

USB Drive Full Degrades Code Performance

3 participants