Pull 2

[chrfranke]

Summary by CodeRabbit

• Bug Fixes

  • Improved ATA pass-through handling so returned data is only applied when appropriate, reducing the chance of incorrect response handling.

• New Features

  • Device Statistics output now shows clearer human-readable details for select fields, including formatted capacity and time values.
  • JSON output for these entries now includes the same extra descriptive text.

[coderabbitai]

Warning

Review limit reached

@chrfranke, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 35 minutes and 38 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c070e360-09ef-4843-828d-09ca64eddc3e

📥 Commits

Reviewing files that changed from the base of the PR and between fdd24ed and a31a8d1.

📒 Files selected for processing (2)

• lib/dev_jmb39x_raid.cpp
• src/ataprint.cpp

📝 Walkthrough

Walkthrough

Two independent fixes: jmb39x_device::ata_pass_through now restricts response buffer copy and checksum suppression to data_in commands only (was incorrectly applied to data_out too). ataprint.cpp gains a get_device_statistics_extra_info() helper that formats capacity and timestamp strings for specific Device Statistics page 1 fields, appended to printed rows and JSON output.

Changes

JMB39x ATA pass-through data_in guard

Layer / File(s)	Summary
data_in guard fix lib/dev_jmb39x_raid.cpp	Buffer zeroing, payload copy, and checksum-warning suppression are now inside an explicit data_in branch instead of any non-no_data path.

Device Statistics extra info

Layer / File(s)	Summary
Extra info helper and output integration src/ataprint.cpp	Adds get_device_statistics_extra_info() formatting capacity (offsets 0x018, 0x028) and timestamp (0x038) fields; print_device_statistics_page() appends the result in brackets and sets jrefi["string"] for JSON.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 A data_in check, now properly placed,
No data_out buffers shall ever be erased.
And statistics pages now sparkle with flair—
Capacities, timestamps, displayed with great care!
Hop hop, little fix, the diff looks just right. ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title is too generic and does not describe the changes in ATA pass-through handling or device statistics output.	Rename it to something specific, such as "Fix ATA pass-through data_in handling and add device statistics extra info".

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch pull-2

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/dev_jmb39x_raid.cpp`:
- Around line 758-764: The SMART READ LOG self-test fixup is still not fully
direction-safe because the `!is_no_data` check can allow
`jmb_fix_selftest_log_index()` to mutate the caller buffer for mismatched
requests. Update the condition in the `ata_cmd_in::data_in` handling path so the
self-test log index fixup is gated on `data_in` as well, keeping all
response-side writes restricted to inbound data buffers and matching the
existing direction guard in this block.

In `@src/ataprint.cpp`:
- Around line 1809-1810: The per-entry JSON "string" values for the capacity
counters are being formatted with a locale-dependent decimal separator, which
makes them inconsistent with the existing JSON strings. Update the capacity
handling in ataprint.cpp where format_capacity() is used for the 0x018/0x028
cases, and the related path around the other affected lines, so it explicitly
uses the same locale-neutral "." separator already used by
set_json_globals_from_device_statistics() before assigning infostr into
jrefi["string"]. Keep the output format consistent with the host_reads and
host_writes JSON strings.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b83f8774-6165-46a9-8ccb-f664139ee253

📥 Commits

Reviewing files that changed from the base of the PR and between 8828fa2 and fdd24ed.

📒 Files selected for processing (2)

• lib/dev_jmb39x_raid.cpp
• src/ataprint.cpp