feat(sdcard): add SD card storage capacity query (closes #206)#214
feat(sdcard): add SD card storage capacity query (closes #206)#214tylerkron wants to merge 5 commits into
Conversation
Adds GetSdCardStorageAsync on ISdCardOperations / DaqifiStreamingDevice wrapping the firmware's SYSTem:STORage:SD:SPACe? query so callers can read FreeBytes/TotalBytes (with computed UsedBytes) as typed values instead of raw response strings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Review Summary by QodoAdd SD card storage capacity query capability
WalkthroughsDescription• Adds GetSdCardStorageAsync method to query SD card free/total bytes • Implements SdCardStorageInfo record with computed UsedBytes property • Creates SdCardSpaceParser for parsing firmware response format • Follows existing SD card operation patterns with retry and error handling Diagramflowchart LR
A["Device Query"] -->|"SYSTem:STORage:SD:SPACe?"| B["Firmware Response"]
B -->|"free,total"| C["SdCardSpaceParser"]
C -->|"Parse"| D["SdCardStorageInfo"]
D -->|"FreeBytes, TotalBytes, UsedBytes"| E["Caller"]
B -->|"Error Detection"| F["Exception Translation"]
F -->|"No Card"| G["SdCardNotPresentException"]
F -->|"SCPI Error"| H["SdCardOperationException"]
File Changes1. src/Daqifi.Core/Device/SdCard/SdCardStorageInfo.cs
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
…rd retry; reject free>total Addresses Qodo review on #214: - Guard GetSdCardStorageAsync with _isLoggingToSdCard check to match the Delete/Download/Format pattern. Avoids silently stopping an active SD recording. - Skip the retry loop when the firmware response contains the "No SD Card Detected" marker (non-transient). Prevents misclassifying a no-card condition as a generic SCPI error if the marker isn't repeated on retry. - Parser rejects free > total as malformed, so callers always see a typed exception with raw response rather than a negative UsedBytes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks for the review. Addressed in 62b8def: 1. Storage query stops logging — fixed. Added 2. 3. Retries no-card responses — fixed. Added a 4. No free/total validation — fixed. All 1020 tests pass on net9.0 and net10.0. |
|
Hardware smoke test — NQ1, FW reports Tested the new `SYSTem:STORage:SD:SPACe?` SCPI command directly on a live device (no SD card inserted — covers the second bullet of the test plan). Sequence run (mirrors the C# `PrepareSdInterface` → query → `PrepareLanInterface` pattern): ``` Verifications:
So: when this PR's `GetSdCardStorageAsync` is invoked against a card-less device, the firmware delivers exactly the response shape the new parser keys off, and the existing `ContainsNoSdCardMarker` heuristic detects it. `SdCardNotPresentException` will fire correctly. Also confirms the no-retry path (`!ContainsScpiError(lines) || ContainsNoSdCardMarker(lines)` short-circuit) is triggered as designed — no wasted retries on a non-transient condition. Two notes (not blocking — neither is PR #214 behavior):
|
|
With-SD-card path: ✅ VERIFIED on hardware (32 GB Silicon Power SDHC card, new firmware build): ``` Response shape `','` matches the parser test fixtures (`SdCardSpaceParser.TryParse("1048576000,2097152000", ...)` style) verbatim. `SdCardSpaceParser.TryParse` on this input → `FreeBytes=31_899_189_248`, `TotalBytes=31_902_400_512`, computed `UsedBytes ≈ 3.21 MB` — consistent with the single `smoke5m.bin` (3,046,387 B) reported by `LIST?`. Also cross-checks cleanly:
Both test-plan checklist items now satisfied — no-card path verified earlier (`SdCardNotPresentException` substring match), with-card path verified here (clean parse + correct typed values). (Earlier failures were on an older firmware build whose SD subsystem couldn't enumerate any card — unrelated to PR #214 and now fixed by the firmware rebuild.) |
|
Sign-off: both paths now verified end-to-end on the current firmware build.
Bonus observation on the new firmware: the `finally`-block `PrepareLanInterface` cleanup that I'd previously flagged as a concern (because `SD:ENAble 0` would return `-200` after a failed query on the older firmware) is now clean — both `SD:ENAble 0` and `LAN:ENAbled 1` return OK after a no-card `SPACe?` failure, and the error queue ends at `0,"No error"`. So the C# `finally` block won't leave a stale `-200` for the next unrelated operation to trip over. Both unchecked items in the test plan are satisfied:
LGTM from the hardware side — leaving formal PR approval to the assigned reviewer. |
cptkoolbeenz
left a comment
cptkoolbeenz
left a comment
There was a problem hiding this comment.
Hardware-verified end-to-end on NQ1 (32 GB SDHC card with files / card removed). Both checklist items in the test plan satisfied; SCPI response shapes match the parser fixtures verbatim. Details in prior comments.
2 participants
Summary
GetSdCardStorageAsynconISdCardOperations/DaqifiStreamingDevice, wrapping the firmware'sSYSTem:STORage:SD:SPACe?query so callers get typedFreeBytes/TotalBytes/ computedUsedBytesinstead of raw strings.SdCardStorageInforecord (public sealed recordto match sibling SD card types) andSdCardSpaceParserfor the"free,total"response.ScpiMessageProducer.GetSdSpace.Implementation follows the existing
GetSdCardFilesAsyncpattern: defensiveStopStreaming,PrepareSdInterfacewith SPI-settle delay, single retry on transient**ERROR,PrepareLanInterfaceinfinally, and typedSdCardNotPresentException/SdCardOperationExceptiontranslation on failure.Scope notes:
SYSTem:STORage:SD:INFO?(CID register) wrapper from feat: Add SD card storage capacity query #206 is intentionally skipped — the issue marks it optional and the core acceptance criteria are met by the storage query.SdCardFilesystemExceptionpath:SPACe?reads FATFS metadata rather than iterating a directory, so the "Failed to open directory" marker doesn't apply.Closes #206.
Test plan
dotnet testpasses (net9.0 + net10.0): 1018 passed, 0 failed.GetSdCardStorageAsynccovering: disconnected throws, correct SCPI commands sent, response parsing, LAN interface restoration, defensive stop streaming, retry-on-transient-error, persistent SCPI error →SdCardOperationException, no-card →SdCardNotPresentException, LAN restoration on error path.ScpiMessageProducer.GetSdSpacecommand test.SdCardNotPresentException).🤖 Generated with Claude Code