[tylerkron]

What was wrong

Three small Core docs/duplication issues, checked against current main. None of the open PRs in #770, #771, #781, #790, or #805–#814 own them. #805 deletes vacuous finder timeout facts; this does not touch tests or timeout values.

SD listing and log-extension docs

ISdCardOperations.StartSdCardLoggingAsync still said an omitted file name is log_YYYYMMDD_HHMMSS with .dat for TestData. There is no TestData format.

SdCardListIncompleteException and the GetSdCardFilesAsync remarks still said the firmware emits no end-of-listing marker, and that the exception only means the SYSTem:ERRor? terminator never came back.

Serial probe-timeout comments

SerialDeviceFinder's hard-timeout rationale still summed DeviceWakeUpDelayMs 200 + EchoDisableSettleMs 250 + ResponseTimeoutMs 1000 (~1.5s). EchoDisableSettleMs is gone. The PortProbeHardTimeoutMs test seam still said tests shrink the timeout "instead of waiting 8s per case". The default hard timeout is 3s. Timeout behavior is unchanged.

Discovery connect cloned options twice

ConnectWiFiDeviceAsync and ConnectSerialDeviceAsync each hand-copied the same settable DeviceConnectionOptions properties before applying the discovered name. A new property would be dropped on discovery connect and kept on the direct TCP path.

What the sibling or implementation already does

• StartSdCardLoggingSessionAsync already maps the extension: Json → .json, Csv → .csv, otherwise .bin. The public session overload already says "a format-appropriate extension" and does not mention TestData. The public StartSdCardLoggingAsync param text now matches the session implementation: .bin for Protobuf, .json for JSON, .csv for CSV.
• GetSdCardFilesAsync already throws SdCardListIncompleteException on both paths. The missing-answer path is unchanged: the SYSTem:ERRor? terminator appended to SYSTem:STORage:SD:LISt? never comes back (closes api: a timed-out SD LIST is indistinguishable from an empty SD card #396). After a reply that terminator closed, SdCardFileListParser.GetListingStatus reads the device marker (__END_OF_LIST__, firmware test(device): fold EndToEndTests into DaqifiDeviceWithTransportTests #794). FAILED and INCOMPLETE throw, and any status word other than OK is treated as incomplete and throws too. A missing marker on a terminated reply is the pre-test(device): fold EndToEndTests into DaqifiDeviceWithTransportTests #794 firmware and is not an error — SdCardListingStatus.Unterminated already says so. The public exception text on ISdCardOperations.GetSdCardFilesAsync, the exception type remarks, and the implementation remarks now say both paths. The exception's runtime message is unchanged.
• DiscoverAsync already budgets a healthy port at ~1.2s from DeviceWakeUpDelayMs + ResponseTimeoutMs. The hard-timeout comment now cites that pair, and the test seam cites DefaultPortProbeHardTimeoutMs (3s). The historical note that an 8s ceiling felt sluggish next to the 2–3s sweep stays, as the reason the ceiling is 3s.
• ConnectTcpAsync and ConnectSerialAsync already pass the caller's DeviceConnectionOptions instance through. Discovery connect is the only path that must clone, because it may replace a default DeviceName with IDeviceInfo.Name. Both discovery methods now call CopyOptionsWithNameOverride, which copies the same properties and applies the same name rule as the two hand-copies did.

No behavior change. Tests were not edited.

Verification

• dotnet build src/Daqifi.Core/Daqifi.Core.csproj: 0 warnings, 0 errors (net9.0 and net10.0).
• SdCardOperationsTests on net10.0: 180 passed, 0 failed. Those tests cover the missing-terminator path, __END_OF_LIST__ OK as an empty card, and __END_OF_LIST__ INCOMPLETE / FAILED throwing SdCardListIncompleteException.

  

[tylerkron]

/agentic_review

[qodo-code-review]

PR Summary by Qodo

Share discovery option copying and correct Core documentation

✨ Enhancement 📝 Documentation 🕐 10-20 Minutes

AI Description

• Centralize option copying for WiFi and serial discovery connections to prevent the two paths from
 diverging.
• Correct SD listing and log-extension documentation to match existing firmware handling.
• Align serial probe-timeout comments with the current 3-second limit; timeout behavior is
 unchanged.

Diagram

graph TD
  I["Discovered device"] --> R["Connection routing"] --> W["WiFi path"] --> C["Options copy"] --> T["Transport connect"]
  R --> S["Serial path"] --> C

Loading

High-Level Assessment

Keep the shared helper in the factory: cloning and discovered-name substitution are specific to discovery connections. Moving cloning into DeviceConnectionOptions would broaden its responsibility without eliminating the need to maintain a property copy when options change.

Files changed (5) +93 / -69

Refactor (1) +33 / -34

DaqifiDeviceFactory.csShare discovery connection option copying +33/-34

Share discovery connection option copying

• Extracts the identical WiFi and serial option-copying logic into one helper. Both paths retain the discovered-name override when the caller has not supplied a name.

src/Daqifi.Core/Device/DaqifiDeviceFactory.cs

Documentation (4) +60 / -35

SerialDeviceFinder.csCorrect serial probe-timeout comments +9/-7

Correct serial probe-timeout comments

• Removes a reference to a retired settle delay and describes the current 1.2-second timed probe portion and 3-second hard timeout. Timeout values and behavior do not change.

src/Daqifi.Core/Device/Discovery/SerialDeviceFinder.cs

ISdCardOperations.csClarify public SD listing and log-format documentation +8/-4

Clarify public SD listing and log-format documentation

• Documents incomplete listings reported by the device marker as well as missing terminator replies. Replaces the obsolete TestData extension with the CSV extension in the logging parameter documentation.

src/Daqifi.Core/Device/SdCard/ISdCardOperations.cs

SdCardListIncompleteException.csDocument both incomplete-listing failure paths +22/-13

Document both incomplete-listing failure paths

• Clarifies that the exception covers missing terminator replies and non-OK device listing markers. Notes that a missing marker on older firmware is not itself an error; the exception's runtime message is unchanged.

src/Daqifi.Core/Device/SdCard/SdCardListIncompleteException.cs

SdCardOperations.csAlign SD listing remarks with marker handling +21/-11

Align SD listing remarks with marker handling

• Updates method documentation to explain terminator-based reply detection and the existing handling of failed, incomplete, and unknown marker statuses. No listing logic changes.

src/Daqifi.Core/Device/SdCard/SdCardOperations.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

1. Empty-card docs overlook the marker ✗ Dismissed

Description

SdCardListIncompleteException says that no listing bytes are identical on the wire to a healthy
empty directory, although current firmware appends an end-of-listing marker even when there are no
file lines. For current-firmware replies, that marker distinguishes an empty listing from missing
listing content; the revised GetSdCardFilesAsync remarks repeat the misleading byte-for-byte
explanation.

Code

src/Daqifi.Core/Device/SdCard/SdCardListIncompleteException.cs[R12-13]

+/// An empty directory produces no file lines, so "no listing bytes" is byte-for-byte identical
+/// to a healthy empty card on the wire. Core therefore appends a <c>SYSTem:ERRor?</c> query to

Relevance

●●● Strong

Accepted marker-related documentation corrections closely match precedent; empty listings include
the firmware marker, so remarks should distinguish file lines from listing bytes.

PR-#550
PR-#513

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The parser documents that the device appends __END_OF_LIST__ to a directory listing and handles
that line separately from file entries. The revised exception and operation remarks nonetheless
describe a healthy empty listing as indistinguishable on the wire from one with no listing bytes.

src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[16-20]
src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[52-61]
src/Daqifi.Core/Device/SdCard/SdCardOperations.cs[203-209]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The revised SD-card remarks conflate an empty directory's lack of file lines with a reply containing no listing bytes. Current firmware appends an end-of-listing marker even for an empty directory.

## Fix Focus Areas
- src/Daqifi.Core/Device/SdCard/SdCardListIncompleteException.cs[12-17]
- src/Daqifi.Core/Device/SdCard/SdCardOperations.cs[203-209]

## Recommended Fix
Explain that empty directories contain no file-entry lines, but current firmware still sends a marker. Qualify the byte-identical ambiguity as applying to firmware without that marker, and retain the explanation of why the error-query terminator is used.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Tickets: 🎫 api: a timed-out SD LIST is indistinguishable from an empty SD card

✅ Compliance rules (platform): 13 rules

Review mode: Auto: ⏭️ Skipped: Documentation-only comment corrections with no behavioral or semantic code changes.

Tip of the day

💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 7e8a0a1

Results up to commit fb43302 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Empty-card docs overlook the marker ✗ Dismissed

Description

SdCardListIncompleteException says that no listing bytes are identical on the wire to a healthy
empty directory, although current firmware appends an end-of-listing marker even when there are no
file lines. For current-firmware replies, that marker distinguishes an empty listing from missing
listing content; the revised GetSdCardFilesAsync remarks repeat the misleading byte-for-byte
explanation.

Code

src/Daqifi.Core/Device/SdCard/SdCardListIncompleteException.cs[R12-13]

+/// An empty directory produces no file lines, so "no listing bytes" is byte-for-byte identical
+/// to a healthy empty card on the wire. Core therefore appends a <c>SYSTem:ERRor?</c> query to

Relevance

●●● Strong

Accepted marker-related documentation corrections closely match precedent; empty listings include
the firmware marker, so remarks should distinguish file lines from listing bytes.

PR-#550
PR-#513

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The parser documents that the device appends __END_OF_LIST__ to a directory listing and handles
that line separately from file entries. The revised exception and operation remarks nonetheless
describe a healthy empty listing as indistinguishable on the wire from one with no listing bytes.

src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[16-20]
src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[52-61]
src/Daqifi.Core/Device/SdCard/SdCardOperations.cs[203-209]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The revised SD-card remarks conflate an empty directory's lack of file lines with a reply containing no listing bytes. Current firmware appends an end-of-listing marker even for an empty directory.

## Fix Focus Areas
- src/Daqifi.Core/Device/SdCard/SdCardListIncompleteException.cs[12-17]
- src/Daqifi.Core/Device/SdCard/SdCardOperations.cs[203-209]

## Recommended Fix
Explain that empty directories contain no file-entry lines, but current firmware still sends a marker. Qualify the byte-identical ambiguity as applying to firmware without that marker, and retain the explanation of why the error-query terminator is used.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 7e8a0a1