Skip to content

docs: DEVICE_INTERFACES Status/dedupe + ADR 0001 table tense - #759

Merged
tylerkron merged 4 commits into
mainfrom
cursor/docs-device-interfaces-adr-0001-0845
Sep 27, 2026
Merged

tylerkron merged 4 commits into
mainfrom
cursor/docs-device-interfaces-adr-0001-0845

Conversation

@tylerkron

@tylerkron tylerkron commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

What was wrong

How it was fixed

docs/DEVICE_INTERFACES.md

  • Status lists all six values and links to the reconnect section.
  • Quick Start becomes "Start here": a pointer to the README's See it in 30 seconds plus a
    sentence on what this page is for. Discovery and DIO/PWM recipes likewise link to the README.
    Everything deleted was verified present there first, including the multicast-filtered-network
    escape hatch and the "frequency is device-wide, one timer" note.
  • Kept what only lives here: USB LocationKey, CreateDefault / DiscoverAndConnectAsync,
    continuous discovery, PreserveActiveStream, diagnostics, RunExclusiveAsync, and the mDNS
    notes the README does not carry (Add MDnsDeviceFinder for reliable discovery on home/multi-AP networks (broadcast UDP is fragile) #183, what the finder reads from the PTR/SRV/TXT/A reply,
    SO_REUSEADDR, serial hex→decimal, LocalInterfaceAddress).
  • New Feature support section: check Supports(...), or catch FeatureNotSupportedException
    and show the required-vs-reported version. It is explicit about the two things that trip
    people up:
    • Supports answers for the device, not the link. SdFileTransferOverWifi is only the gate
      on a WiFi/TCP connection, so the SD sample checks
      device.IsUsbConnection || device.Supports(DeviceFeature.SdFileTransferOverWifi) — the same
      predicate EnsureSdFileTransferSupportedOnTransport applies — rather than hiding files a
      USB connection can read on pre-3.7.0 firmware.
    • Not every gated call enforces its gate. The SD calls throw; SetAnalogOutput sends the
      command regardless and a non-NQ3 board rejects it with no exception. Because Supports
      skips board/hardware requirements while DeviceType is still Unknown, the analog-output
      sample also requires an identified board.
    • The catch sample branches on RequiredVersion, which is null for a board/hardware
      shortfall (no upgrade would help).
      Links ADR 0001 rather than restating it.
  • Dropped the trailing marketing Features bullet list.

docs/adr/0001-firmware-feature-gating.md — the ADR is amended, not rewritten. The original
decision text stays; an amendment notes the trigger landed and the table shipped, the
DeviceFeature sketch is marked historical and gains SdFileTransferOverWifi, and the
conclusion and consequences point at the #256/#390 implementation notes instead of describing
them as deferred. #740's follow-up "done" markers and the GetSdLoggingState row are untouched.

Docs only — no behavior change.

🤖 Generated with Claude Code

@tylerkron
tylerkron force-pushed the cursor/docs-device-interfaces-adr-0001-0845 branch from 8d57d99 to fe67999 Compare September 21, 2026 01:17
@tylerkron
tylerkron changed the base branch from main to cursor/docs-device-interfaces-adr-0001-9ce8 September 21, 2026 01:17
@tylerkron
tylerkron changed the base branch from cursor/docs-device-interfaces-adr-0001-9ce8 to main September 21, 2026 01:18
@tylerkron
tylerkron force-pushed the cursor/docs-device-interfaces-adr-0001-0845 branch from fe67999 to 24cdf08 Compare September 21, 2026 01:19
@tylerkron

Copy link
Copy Markdown
Contributor Author

/agentic_review

@qodo-code-review

qodo-code-review Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Unsupported boards get analog commands ✓ Resolved 🐞 Bug ≡ Correctness ⭐ New
Description
The SetAnalogOutput example treats Supports(DeviceFeature.AnalogOutput) as a sufficient guard,
but Supports returns true while the board type is Unknown. If a device reports channels before a
recognizable part number, the documented check passes and SetAnalogOutput sends the command
without a board check.
Code

docs/DEVICE_INTERFACES.md[R1013-1016]

+// Ask before you offer it: analog output is Nyquist 3 hardware only.
+if (device.Supports(DeviceFeature.AnalogOutput))
+{
+    device.SetAnalogOutput(0, 2.5);
Relevance

●●● Strong

Concrete documentation safety bug; team accepts hardware-gating corrections and capability-guard
examples.

PR-#281
PR-#389

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The support tests explicitly assert that an unknown board supports analog output; support evaluation
skips board requirements in that state. The output operation then sends the command without checking
the board.

src/Daqifi.Core.Tests/Device/DeviceFeatureSupportTests.cs[211-226]
src/Daqifi.Core/Device/DaqifiDevice.cs[194-205]
src/Daqifi.Core/Device/Internal/ChannelControlOperations.cs[343-376]
src/Daqifi.Core/Device/DaqifiDevice.cs[3293-3301]

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 analog-output example presents `Supports` as a sufficient guard even though it returns true for an unidentified board.
## Fix Focus Areas
- docs/DEVICE_INTERFACES.md[1006-1017]
## Recommended Fix
Require the board to be identified as Nyquist 3 before offering analog output, and explain why `Supports` alone does not establish board support while the board type is unknown.

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


2. Users see a blank firmware requirement ✓ Resolved 🐞 Bug ≡ Correctness ⭐ New
Description
The exception example always prints needs {ex.RequiredVersion}, although EnsureSupported sets
that value to null when hardware, rather than firmware, fails the check. If an identified device
lacks SD or WiFi hardware, the example reports needs , device reports ... instead of explaining
the unsupported feature.
Code

docs/DEVICE_INTERFACES.md[R1038-1039]

+    // e.g. "SdFileTransferOverWifi: needs 3.7.0, device reports 3.6.1 (Nyquist1)"
+    Console.WriteLine($"{ex.Feature}: needs {ex.RequiredVersion}, device reports {ex.ActualVersion} ({ex.Board})");
Relevance

●●● Strong

Concrete documentation correctness bug; nullable hardware failures make the shown exception output
misleading.

PR-#347
PR-#389

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The SD-over-WiFi requirement includes both SD and WiFi hardware. EnsureSupported omits the
required version for a hardware failure, while the new example interpolates that nullable value
without checking it.

src/Daqifi.Core/Device/DeviceFeatureRequirements.cs[99-105]
src/Daqifi.Core/Device/DaqifiDevice.cs[230-248]
docs/DEVICE_INTERFACES.md[1033-1044]

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 documented exception handler renders an empty firmware requirement when a hardware shortfall leaves `RequiredVersion` null.
## Fix Focus Areas
- docs/DEVICE_INTERFACES.md[1031-1044]
## Recommended Fix
Branch on `ex.RequiredVersion`: display the required and reported versions when present, and display an unsupported-board-or-hardware message when absent.

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


3. USB users skip available SD files ✓ Resolved 🐞 Bug ≡ Correctness
Description
The feature-support example requires DeviceFeature.SdFileTransferOverWifi before every
GetSdCardFilesAsync() call, including calls made over USB. On USB or serial transports the
implementation intentionally bypasses this WiFi-only gate, so the documented check prevents
applications from offering valid SD operations on older SD-capable firmware.
Code

docs/DEVICE_INTERFACES.md[R1008-1010]

+// SD list/get/delete over this WiFi connection needs firmware >= 3.7.0 plus SD and WiFi
+// hardware. Over USB the same operations run on any SD-capable firmware and are not gated.
+if (device.Supports(DeviceFeature.SdFileTransferOverWifi))
Relevance

●●● Strong

Accepted documentation correctness findings are common; this transport-specific gating example
clearly suppresses valid USB operations.

PR-#291
PR-#476

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The documentation says USB operations are not gated, but its immediately preceding sample
unconditionally checks the WiFi-specific feature. The implementation returns before applying that
feature requirement for USB, while the requirement table requires firmware v3.7.0 plus SD and WiFi
hardware, proving that the sample can incorrectly suppress supported USB/serial operations.

docs/DEVICE_INTERFACES.md[1008-1013]
src/Daqifi.Core/Device/SdCard/SdCardOperations.cs[160-183]
src/Daqifi.Core/Device/DeviceFeatureRequirements.cs[98-103]

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 feature-support example gates `GetSdCardFilesAsync()` on `DeviceFeature.SdFileTransferOverWifi` without checking the active transport, so consumers may incorrectly disable valid USB/serial SD operations on firmware below v3.7.0.

## Fix Focus Areas
- docs/DEVICE_INTERFACES.md[1008-1010]

## Recommended Fix
Update the example to state that `SdFileTransferOverWifi` should be checked only for WiFi/TCP connections, while USB/serial SD-capable devices can call the operation without that feature check. If the sample cannot express the transport distinction clearly, replace it with separate WiFi and USB examples and preserve the existing exception-handling guidance.

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


Grey Divider

Context sources
✅ Compliance rules (platform): 13 rules
Review mode: ⏭️ Skipped: The latest push changes only documentation prose and examples, with no runtime, configuration, schema, or build behavior affected.

Grey Divider

Tip of the day
💡 Did you know, you can describe a rule in plain language on the Rules page and Qodo drafts it for you

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit 5b5a31b

Results up to commit 24cdf08 🚀 Fast


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


Remediation recommended
1. USB users skip available SD files ✓ Resolved 🐞 Bug ≡ Correctness
Description
The feature-support example requires DeviceFeature.SdFileTransferOverWifi before every
GetSdCardFilesAsync() call, including calls made over USB. On USB or serial transports the
implementation intentionally bypasses this WiFi-only gate, so the documented check prevents
applications from offering valid SD operations on older SD-capable firmware.
Code

docs/DEVICE_INTERFACES.md[R1008-1010]

+// SD list/get/delete over this WiFi connection needs firmware >= 3.7.0 plus SD and WiFi
+// hardware. Over USB the same operations run on any SD-capable firmware and are not gated.
+if (device.Supports(DeviceFeature.SdFileTransferOverWifi))
Relevance

●●● Strong

Accepted documentation correctness findings are common; this transport-specific gating example
clearly suppresses valid USB operations.

PR-#291
PR-#476

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The documentation says USB operations are not gated, but its immediately preceding sample
unconditionally checks the WiFi-specific feature. The implementation returns before applying that
feature requirement for USB, while the requirement table requires firmware v3.7.0 plus SD and WiFi
hardware, proving that the sample can incorrectly suppress supported USB/serial operations.

docs/DEVICE_INTERFACES.md[1008-1013]
src/Daqifi.Core/Device/SdCard/SdCardOperations.cs[160-183]
src/Daqifi.Core/Device/DeviceFeatureRequirements.cs[98-103]

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 feature-support example gates `GetSdCardFilesAsync()` on `DeviceFeature.SdFileTransferOverWifi` without checking the active transport, so consumers may incorrectly disable valid USB/serial SD operations on firmware below v3.7.0.

## Fix Focus Areas
- docs/DEVICE_INTERFACES.md[1008-1010]

## Recommended Fix
Update the example to state that `SdFileTransferOverWifi` should be checked only for WiFi/TCP connections, while USB/serial SD-capable devices can call the operation without that feature check. If the sample cannot express the transport distinction clearly, replace it with separate WiFi and USB examples and preserve the existing exception-handling guidance.

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


Grey Divider

Qodo Logo

@tylerkron
tylerkron marked this pull request as ready for review September 21, 2026 01:34
@tylerkron
tylerkron requested a review from a team as a code owner September 21, 2026 01:34
@tylerkron

Copy link
Copy Markdown
Contributor Author

Reviewed (Claude): approved, and restacked on #740 — it is not redundant with it (the ADR hunks are disjoint and the six-value ConnectionStatus fix plus the new Feature support section are new), but both rewrote the same Quick Start block. This branch now carries #740's two commits as its base and resolves that hunk by linking to the README instead of keeping a third copy of the same sample; #740's CI-OS fix, its follow-up "done" markers and the GetSdLoggingState row are untouched. Merge #740 first. Everything deleted here was verified present in the README first. Qodo-clean on 24cdf08, CI green — ready for review.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Deduplicate device guidance and update the feature-gating ADR

📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Lists every connection status and documents supported feature checks.
• Replaces duplicated recipes with links to canonical README examples.
• Amends ADR 0001 to reflect completed feature-gating work.
Diagram

graph TD
  Reader["Library user"] --> Guide["Interface guide"] --> Readme["README recipes"]
  Guide --> API["Device APIs"] --> Support["Feature support"]
  Guide --> ADR["Gating ADR"] --> Support
Loading
High-Level Assessment

The chosen approach is appropriate: keep copy-paste recipes canonical in the README while preserving specialized interface details in the deeper guide. Retaining duplicate samples would invite further drift, while generated shared snippets would add disproportionate tooling complexity for two documentation surfaces.

Files changed (2) +129 / -159

Documentation (2) +129 / -159
DEVICE_INTERFACES.mdConsolidate recipes and document complete status and feature gating +109/-146

Consolidate recipes and document complete status and feature gating

• Lists all six connection statuses and adds practical Supports/FeatureNotSupportedException guidance. Replaces duplicated quick-start, discovery, DIO, and PWM recipes with canonical README links while retaining advanced discovery, transport, diagnostics, and interface-specific notes. It also corrects the CI platform statement and removes the redundant marketing feature list.

docs/DEVICE_INTERFACES.md

0001-firmware-feature-gating.mdAmend feature-gating ADR for shipped implementations +20/-13

Amend feature-gating ADR for shipped implementations

• Marks completed follow-ups and dead-code removal with their implementation references. Updates historical language to distinguish the original decision from the subsequently shipped requirement table, capability reader, and SdFileTransferOverWifi gate.

docs/adr/0001-firmware-feature-gating.md

Comment thread docs/DEVICE_INTERFACES.md Outdated
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 24cdf08

cursoragent and others added 3 commits September 27, 2026 10:10
Leave #740's Quick Start typed API, CI-OS sentence, and ADR follow-up
checkboxes alone. This is the leftover stale copy: six-value Status,
README clones replaced with links, Features bullets dropped, Supports
recipe added, and ADR 0001 amended to present tense for the shipped table.

Co-authored-by: Tyler Kron <tylerkron@gmail.com>
Review follow-ups on this PR:

- Rebased onto #740, which is reviewed and edits the same two files. The
  only textual conflict was the Quick Start block both PRs rewrote; #740's
  corrected snippet is now identical to the README's, so this takes the
  link and renames the section "Start here" rather than shipping a third
  copy to drift. #740's CI-OS fix and its ADR follow-up/table edits are
  untouched.
- The feature-support sample called GetSdCardFilesAsync twice, guarded and
  then unguarded, which read as a copy-paste. Split into the two real
  choices: check first, or catch and report. Braces on the single-line if.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Supports(SdFileTransferOverWifi) answers for the device, not the link,
so the sample's unconditional check would hide SD files a USB
connection can read on pre-3.7.0 firmware (Qodo). The check is now
IsUsbConnection || Supports(...), the same predicate
EnsureSdFileTransferSupportedOnTransport applies.

The section also claimed every gated call throws
FeatureNotSupportedException. Only the SD calls enforce their gate;
SetAnalogOutput sends regardless and a non-NQ3 board rejects it with no
exception, so Supports is the only guard there. Say so, and note that
RequiredVersion is null for a board/hardware shortfall.

Restore the mDNS reply-parsing detail (PTR/SRV/TXT/A, the TXT keys,
ConnectionType.WiFi) that the dedupe dropped without a README copy, and
list :SPACe? under SdFileTransferOverWifi in the ADR sketch to match
the enum.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tylerkron
tylerkron force-pushed the cursor/docs-device-interfaces-adr-0001-0845 branch from 24cdf08 to a35a30a Compare September 27, 2026 16:14
@tylerkron

Copy link
Copy Markdown
Contributor Author

/agentic_review

Comment thread docs/DEVICE_INTERFACES.md Outdated
Comment thread docs/DEVICE_INTERFACES.md Outdated
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit a35a30a

…ersion

Supports skips board and hardware requirements while DeviceType is
Unknown, so Supports(AnalogOutput) alone answers true for a board that
has not reported its part number, and SetAnalogOutput has no gate of its
own behind it. The sample now also requires an identified board, and the
text explains why (Qodo).

The catch sample printed "needs {RequiredVersion}" unconditionally,
which renders "needs ," for a board/hardware shortfall where
RequiredVersion is null. It now branches on it (Qodo).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tylerkron

Copy link
Copy Markdown
Contributor Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 5b5a31b

@tylerkron

Copy link
Copy Markdown
Contributor Author

Qodo-clean, CI green — ready for review

@tylerkron
tylerkron added this pull request to the merge queue Sep 27, 2026
Merged via the queue into main with commit 0a275c0 Sep 27, 2026
4 checks passed
@tylerkron
tylerkron deleted the cursor/docs-device-interfaces-adr-0001-0845 branch September 27, 2026 17:26
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