Skip to content

Conversation

@superg
Copy link
Owner

@superg superg commented Dec 29, 2025

Summary by CodeRabbit

Release Notes

  • New Features
    • Added --scsi-timeout command-line option to configure SCSI operation timeout duration
    • Default timeout set to 50000 milliseconds (50 seconds)
    • Timeout is now consistently applied across all drive readiness checks and SCSI communications

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 29, 2025

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

The PR introduces a configurable SCSI timeout by adding a --scsi-timeout command-line option to Options (default 50000), updating the SPTD constructor to accept and store the timeout as an instance parameter, removing the timeout parameter from the sendCommand method to use the stored value, and propagating this timeout through SPTD instantiation sites across redumper and debug modules.

Changes

Cohort / File(s) Summary
Configuration Addition
options.ixx
Added new public scsi_timeout field (int) with default value 50000; added --scsi-timeout command-line option parsing and help documentation
Core SPTD Constructor & Timeout Handling
scsi/sptd.ixx
Constructor signature changed to accept uint32_t timeout parameter; added private _timeout member; removed timeout parameter from sendCommand method; updated all platform-specific timeout usages to reference _timeout instead of per-call timeout
Integration Updates
redumper.ixx, debug.ixx
Updated first_ready_drive() signature to accept const Options& options; updated all SPTD instantiation call sites to pass options.scsi_timeout to constructor; updated call sites of first_ready_drive() to pass options parameter

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • SCSI timeout option #327 — Both PRs modify the SPTD constructor and propagate scsi_timeout through Options, redumper, and debug modules
  • plextor flasher #283 — Adds Plextor flasher code using SPTD, which depends on the updated SPTD constructor and sendCommand API signatures in this PR
  • Macos fixes #291 — Modifies SPTD class and scsi/sptd.ixx, directly affected by or related to constructor and sendCommand behavior changes

Poem

🐰 A timeout now travels with SPTD,
No more passed with each command so free,
From options it flows, a default so grand,
Fifty thousand milliseconds at hand!
Configuration refined with a whisker and care. ✨

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch scsi_timeout

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2719903 and 377c2e8.

📒 Files selected for processing (4)
  • debug.ixx
  • options.ixx
  • redumper.ixx
  • scsi/sptd.ixx

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@superg superg closed this Dec 29, 2025
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.

2 participants