feat(hid): device-path addressing to flash a specific bootloader among identical ones

[cptkoolbeenz]

What

Adds path-based HID addressing so a caller can hold and flash one specific PIC32 bootloader when several identical ones are connected.

Multiple bootloaders are indistinguishable by VID/PID (all 04D8:003C) and the bootloader exposes no serial, so ConnectAsync's VID/PID first-match cannot target a particular device. The only discriminator is the USB device path (already carried on HidDeviceInfo.DevicePath).

Changes

• IHidTransport.ConnectByPathAsync / ConnectByPath — open the one device at a given path. HidLibraryTransport refactors the shared enumerate → select → open body into ConnectMatchingDeviceAsync; ConnectAsync's behavior (exception types, ordering, disposed/cancellation checks, already-connected early-return, unused-candidate disposal) is preserved.
• FirmwareUpdateService.UpdateFirmwareAsync gains an optional targetDevicePath (additive, after CancellationToken, mirroring UpdateWifiModuleAsync's skipVersionCheck CA1068 trade-off). When set, WaitForBootloaderDeviceAsync matches that exact path and the connect uses ConnectByPathAsync; when null, single-device first-match behavior is unchanged.

DevicePath is sourced identically (HidSharp device.DevicePath) by the discovery enumerator (HidLibraryDeviceEnumerator) and the transport (HidLibraryPlatform), so a path obtained from discovery matches ConnectByPath.

Why

Enables the daqifi-desktop app-global bootloader watcher to hold all detected bootloaders at once (each held exclusively by path) and flash the one the user selects.

Tests

• Transport: match-by-path opens the exact device among identical ones; no-match → IOException; empty/whitespace path → ArgumentException.
• Flash: two enumerated bootloaders + targetDevicePath → connects by that exact path, not first-match.
• Full suite: 1276 passed / 2 skipped / 0 failed; clean build under TreatWarningsAsErrors.

Backward-compatible: targetDevicePath defaults to null, the interface additions are consumed (not implemented) by daqifi-desktop, and no in-repo IHidTransport caller changes behavior. Reviewed adversarially (3 lenses → per-finding verification); no blocking findings.

🤖 Generated with Claude Code

[qodo-code-review]

PR Summary by Qodo

Add HID device-path targeting for flashing a specific bootloader
✨ Enhancement 🧪 Tests 🕐 40+ Minutes

Description

• Add IHidTransport.ConnectByPath{Async} to open an exact HID device via DevicePath.
• Extend firmware flashing to optionally target a specific bootloader path when multiples exist.
• Add coverage ensuring path targeting selects the correct device and preserves legacy behavior.

Diagram

graph TD
  A["Caller (App)"] --> B["FirmwareUpdateService"] --> C["HidDeviceEnumerator"] --> D["HidDeviceInfo (DevicePath)"]
  B --> E["IHidTransport"] --> F["HidLibraryTransport"] --> G{{"HID Bootloader(s)"}}

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Index-based selection (nth enumerated device)

• ➕ No dependency on platform-specific device path formatting
• ➕ Simple API surface (int index)
• ➖ Unstable ordering across OS/hub changes; brittle in multi-device scenarios
• ➖ Still requires callers to reason about enumeration ordering rather than identity

2. Introduce a higher-level "bootloader handle" from discovery

• ➕ Strongly-typed identity object; avoids callers passing raw strings
• ➕ Can evolve to include other discriminators (location ID, timestamps)
• ➖ Larger API change across discovery + flashing call sites
• ➖ More plumbing and serialization concerns for UI/app layers

3. Maintain internal reservation/locking by VID/PID + path

• ➕ Allows service-level enforcement that only one updater targets a device at once
• ➕ Could centralize hold/release semantics
• ➖ Adds concurrency/state complexity and policy decisions to the library
• ➖ Likely duplicates responsibilities of an app-global watcher

Recommendation: The PR’s approach (connect-by-exact DevicePath plus an optional targetDevicePath in the flashing API) is the most reliable way to disambiguate identical bootloaders when serials are absent. It’s additive, preserves existing first-match behavior when the path is null, and keeps device-holding policy in the caller (e.g., desktop watcher) rather than embedding complex reservation logic in the library.

Files changed (6) +232 / -32

Enhancement (4) +126 / -32

HidLibraryTransport.csImplement ConnectByPath and refactor shared connect logic +56/-18

Implement ConnectByPath and refactor shared connect logic

• Adds 'ConnectByPathAsync'/'ConnectByPath' and refactors the enumerate→filter→open flow into 'ConnectMatchingDeviceAsync' shared with the existing VID/PID(+serial) connect path. Preserves existing behaviors (ordering by DevicePath, early-return when already connected, disposal/cancellation checks, and consistent exception types).

src/Daqifi.Core/Communication/Transport/HidLibraryTransport.cs

IHidTransport.csExtend IHidTransport with path-based connect APIs +16/-0

Extend IHidTransport with path-based connect APIs

• Introduces 'ConnectByPathAsync(string, CancellationToken)' and synchronous 'ConnectByPath(string)' for targeting a specific HID device when VID/PID matching is ambiguous. Documents the intended usage via 'HidDeviceInfo.DevicePath'.

src/Daqifi.Core/Communication/Transport/IHidTransport.cs

FirmwareUpdateService.csAllow firmware updates to target a specific bootloader DevicePath +42/-13

Allow firmware updates to target a specific bootloader DevicePath

• Adds optional 'targetDevicePath' to 'UpdateFirmwareAsync' (kept after CancellationToken with a CA1068 suppression for source compatibility). Updates bootloader waiting logic to match a specific enumerated 'DevicePath' when provided, and connects using 'IHidTransport.ConnectByPathAsync' for that multi-device scenario while preserving legacy VID/PID(+serial) behavior otherwise.

src/Daqifi.Core/Firmware/FirmwareUpdateService.cs

IFirmwareUpdateService.csExpose optional targetDevicePath on firmware update interface +12/-1

Expose optional targetDevicePath on firmware update interface

• Updates the interface contract for 'UpdateFirmwareAsync' to accept an optional HID device path used to disambiguate identical bootloaders. Includes documentation and mirrors the CA1068 ordering trade-off used elsewhere for source compatibility.

src/Daqifi.Core/Firmware/IFirmwareUpdateService.cs

Tests (2) +106 / -0

HidLibraryTransportTests.csAdd ConnectByPathAsync transport coverage +39/-0

Add ConnectByPathAsync transport coverage

• Adds tests verifying path-based connect selects the exact matching device among identical VID/PID candidates. Also covers error behavior for no-match (IOException) and invalid/empty path (ArgumentException).

src/Daqifi.Core.Tests/Communication/Transport/HidLibraryTransportTests.cs

FirmwareUpdateServiceTests.csTest firmware flashing targets a specific bootloader by path +67/-0

Test firmware flashing targets a specific bootloader by path

• Adds a multi-bootloader enumeration test ensuring 'UpdateFirmwareAsync(..., targetDevicePath: ...)' connects via 'ConnectByPathAsync' and does not fall back to VID/PID first-match. Extends the fake HID transport to record connect-by-path attempts.

src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs

[qodo-code-review]

Code Review by Qodo

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

1. Breaking UpdateFirmwareAsync signature ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

Changing IFirmwareUpdateService.UpdateFirmwareAsync to add targetDevicePath is a binary-breaking
change for any consumer compiled against the previous 4-parameter signature (optional parameters
don’t preserve binary compatibility). Updating the Daqifi.Core NuGet can therefore cause runtime
MissingMethodException/interface dispatch failures in downstream apps.

Code

src/Daqifi.Core/Firmware/IFirmwareUpdateService.cs[R37-42]

    Task UpdateFirmwareAsync(
        IStreamingDevice device,
        string hexFilePath,
        IProgress<FirmwareUpdateProgress>? progress = null,
-        CancellationToken cancellationToken = default);
+        CancellationToken cancellationToken = default,
+        string? targetDevicePath = null);

Relevance

⭐⭐⭐ High

Strong precedent: PR #198 accepted review to avoid breaking public IFirmwareUpdateService
signature/positional-caller compatibility.

PR-#198

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The interface method signature now includes an additional parameter, and the project is a shipped
package, so compiled downstream consumers can break when updating without recompiling against the
new signature.

src/Daqifi.Core/Firmware/IFirmwareUpdateService.cs[21-43]
src/Daqifi.Core/Daqifi.Core.csproj[9-17]
PR-#198

Agent prompt

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

### Issue description
`IFirmwareUpdateService.UpdateFirmwareAsync`’s signature changed by adding an extra parameter. Even though the parameter is optional, this changes the method’s metadata signature and breaks binary compatibility for already-compiled consumers.

### Issue Context
`Daqifi.Core` is packaged as a public library (NuGet). Public interface signature changes can break downstream apps at runtime when they update the package without recompiling.

### Fix Focus Areas
- src/Daqifi.Core/Firmware/IFirmwareUpdateService.cs[37-43]
- src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[149-185]

Preferred approaches:
- Keep the original `IFirmwareUpdateService.UpdateFirmwareAsync(device, hexFilePath, progress, cancellationToken)` signature unchanged and add a *new* API surface for targeted flashing, e.g.:
 - a new interface `IFirmwareUpdateService2 : IFirmwareUpdateService` with the extended method; `FirmwareUpdateService` implements both.
 - or a new method name like `UpdateFirmwareByPathAsync(...)`.
- If you must keep a single method name, consider extension methods (not interface methods) to provide the “targetDevicePath” convenience without altering the interface contract.

Update docs/tests accordingly.

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

---

2. Breaking IHidTransport interface ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

Adding ConnectByPathAsync/ConnectByPath to the public IHidTransport interface is a breaking change
for any external implementer compiled against the old interface, which can fail to load under the
updated package. This is a high-risk change for a public NuGet surface area.

Code

src/Daqifi.Core/Communication/Transport/IHidTransport.cs[R66-80]

+    /// <summary>
+    /// Connects to the specific HID device at <paramref name="devicePath"/>. Use this to target one
+    /// device among several identical ones (same VID/PID, and no serial to tell them apart), where
+    /// <see cref="ConnectAsync(int,int,string?,CancellationToken)"/>'s first-match cannot distinguish
+    /// them. The path is the value reported by discovery (<c>HidDeviceInfo.DevicePath</c>).
+    /// </summary>
+    /// <param name="devicePath">Platform-specific HID device path to connect to.</param>
+    /// <param name="cancellationToken">Cancellation token.</param>
+    Task ConnectByPathAsync(string devicePath, CancellationToken cancellationToken = default);
+
+    /// <summary>
+    /// Connects to the specific HID device at <paramref name="devicePath"/> synchronously.
+    /// </summary>
+    /// <param name="devicePath">Platform-specific HID device path to connect to.</param>
+    void ConnectByPath(string devicePath);

Relevance

⭐⭐ Medium

Only indirect evidence: team accepted API-compatibility reviews for IFirmwareUpdateService changes
in PR #198.

PR-#198

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The diff adds new members to a public interface in a packaged library; this is a known
breaking-change pattern for downstream implementers.

src/Daqifi.Core/Communication/Transport/IHidTransport.cs[6-81]
src/Daqifi.Core/Daqifi.Core.csproj[9-17]

Agent prompt

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

### Issue description
New abstract members were added to the public `IHidTransport` interface. This breaks external implementers (and can cause runtime type load failures) when they consume the updated package.

### Issue Context
`Daqifi.Core` is distributed as a NuGet package and `IHidTransport` is public.

### Fix Focus Areas
- src/Daqifi.Core/Communication/Transport/IHidTransport.cs[66-81]

Compatible alternatives:
- Prefer a new derived interface, e.g. `IHidTransportByPath : IHidTransport`, containing the new methods; have `HidLibraryTransport` implement it.
- Or use default interface implementations for the new members (e.g., default methods throwing `NotSupportedException`) so existing implementers still load.

Adjust call sites to depend on the new interface only when path-based addressing is required.

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

---

3. Timeout hides target path ✓ Resolved 🐞 Bug ◔ Observability

Description

When targetDevicePath is provided, the WaitingForBootloader timeout message still only reports
VID/PID, not the requested path, making failures ambiguous (no bootloader present vs wrong path
requested). This reduces diagnosability for the new path-targeting behavior.

Code

src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[R1096-1103]

+            // Multiple identical bootloaders can be enumerated at once; when a specific one was
+            // requested, match it by path (the rest stay held by the caller). Otherwise take the
+            // first match, preserving the single-device behavior.
+            var match = targetDevicePath == null
+                ? devices.FirstOrDefault()
+                : devices.FirstOrDefault(d =>
+                    string.Equals(d.DevicePath, targetDevicePath, StringComparison.Ordinal));
+            if (match != null)

Relevance

⭐⭐⭐ High

They accept improving timeout/diagnostic messages in firmware flow (multiple accepted
timeout-message tweaks in PR #200).

PR-#200

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The bootloader wait now selects by exact path when targetDevicePath is set, but the timeout builder
still emits only VID/PID details, losing the discriminator that determines the match condition.

src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[1068-1109]
src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[1631-1655]

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 firmware update flow can now target a specific bootloader via `targetDevicePath`, but the `WaitingForBootloader` timeout message does not include the requested path. This makes timeouts confusing and harder to debug.

### Issue Context
`WaitForBootloaderDeviceAsync` changes behavior based on `targetDevicePath`, but `BuildStateTimeoutMessage` always describes the condition as “No matching HID bootloader device was enumerated for VID/PID …”.

### Fix Focus Areas
- src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[1068-1109]
- src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[1591-1655]
- src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[321-325]

Suggested fixes (pick one):
1) Include the path in the `operation` string passed to `ExecuteWithStateTimeoutAsync` when `targetDevicePath != null`.
2) Store the requested `targetDevicePath` in a private field at the start of `RunPic32UpdateAsync` and have `BuildStateTimeoutMessage` append `TargetPath=...` when present.

Ensure the message still reads well in the non-targeted case (null path).

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

---

4. Unvalidated target path ✓ Resolved 🐞 Bug ☼ Reliability

Description

FirmwareUpdateService.UpdateFirmwareAsync accepts targetDevicePath but never validates it, so
whitespace/invalid values will cause WaitForBootloaderDeviceAsync to poll until the
WaitingForBootloader state timeout rather than failing fast. This turns a caller bug into a
long-running timeout that is hard to diagnose.

Code

src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[R149-155]

    public async Task UpdateFirmwareAsync(
        IStreamingDevice device,
        string hexFilePath,
        IProgress<FirmwareUpdateProgress>? progress = null,
-        CancellationToken cancellationToken = default)
+        CancellationToken cancellationToken = default,
+        string? targetDevicePath = null)
+#pragma warning restore CA1068

Relevance

⭐⭐⭐ High

Team often accepts fail-fast input validation; similar defensive-guard feedback accepted repeatedly
(e.g., PR #116, #201).

PR-#116
PR-#201

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new parameter is accepted and forwarded without validation, and the bootloader wait now performs
an exact path match, so invalid/whitespace input cannot match and will only terminate via timeout.

src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[149-185]
src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[1068-1109]

Agent prompt

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

### Issue description
`FirmwareUpdateService.UpdateFirmwareAsync(..., string? targetDevicePath = null)` propagates `targetDevicePath` into the bootloader wait/connect flow without validating it. If a caller passes whitespace (or any clearly invalid value), the code will never find a match and will only fail via the state timeout.

### Issue Context
- `IHidTransport.ConnectByPathAsync` already rejects null/whitespace paths.
- `HidDeviceInfo` also rejects null/whitespace `DevicePath`.

### Fix Focus Areas
- src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[149-185]

Implement a fast-fail validation in `UpdateFirmwareAsync` (or at the start of `RunPic32UpdateAsync`) such as:
- If `targetDevicePath != null && string.IsNullOrWhiteSpace(targetDevicePath)`, throw `ArgumentException`.
- Optionally normalize `targetDevicePath = null` when it is whitespace to preserve “no targeting” behavior.

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

---

5. Ignore-case path matching ✓ Resolved 🐞 Bug ≡ Correctness

Description

ConnectByPathAsync and the targeted-bootloader selection compare HID DevicePath using
OrdinalIgnoreCase unconditionally. Because the HID layer is explicitly cross-platform (Windows/Linux
via HidSharp and macOS via IOKit), this bakes in a Windows-specific case-insensitivity assumption
and can lead to ambiguous matching if a non-Windows backend ever treats paths as case-sensitive or
if two paths differ only by case.

Code

src/Daqifi.Core/Communication/Transport/HidLibraryTransport.cs[R126-130]

+        // Windows HID device paths are case-insensitive and their casing can vary across enumerations,
+        // so match case-insensitively (real device paths never differ only by case, so this can't
+        // mis-target). Mirrors the OrdinalIgnoreCase used for the serial filter in ConnectAsync.
+        await ConnectMatchingDeviceAsync(
+            candidate => string.Equals(candidate.DevicePath, devicePath, StringComparison.OrdinalIgnoreCase),

Relevance

⭐⭐ Medium

No prior reviews on HID DevicePath casing; team is cross-platform focused (macOS HID backend PR
#263).

PR-#263
PR-#201

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The implementation and test enforce OrdinalIgnoreCase matching based on a Windows-only rationale,
but the HID stack is explicitly multi-platform, meaning the comparison semantics are being applied
beyond the platform justified in comments/tests.

src/Daqifi.Core/Communication/Transport/HidLibraryTransport.cs[22-28]
src/Daqifi.Core/Communication/Transport/HidLibraryTransport.cs[117-133]
src/Daqifi.Core/Communication/Transport/HidPlatformFactory.cs[14-28]
src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[1110-1117]
src/Daqifi.Core.Tests/Communication/Transport/HidLibraryTransportTests.cs[127-139]

Agent prompt

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

### Issue description
`DevicePath` comparisons for targeted HID connection are currently performed with `StringComparison.OrdinalIgnoreCase` unconditionally. The code comments justify this based on Windows behavior, but the transport/enumerator are explicitly cross-platform (Windows/Linux via HidSharp; macOS via IOKit), so the comparison semantics should be explicit per-platform (or the docs/comments should be updated to state that `DevicePath` is always treated as a case-insensitive identifier).

### Issue Context
- `HidLibraryTransport.ConnectByPathAsync` uses `OrdinalIgnoreCase` for matching.
- `FirmwareUpdateService.WaitForBootloaderDeviceAsync` uses the same `OrdinalIgnoreCase` match when `targetDevicePath` is provided.
- The new unit test asserts case-insensitive matching unconditionally, which will need to be updated if the comparison becomes platform-specific.

### Fix Focus Areas
- src/Daqifi.Core/Communication/Transport/HidLibraryTransport.cs[117-133]
- src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[1110-1117]
- src/Daqifi.Core.Tests/Communication/Transport/HidLibraryTransportTests.cs[127-139]

### Suggested fix
- Use a platform-gated comparison for `DevicePath` (e.g., `OrdinalIgnoreCase` on Windows, `Ordinal` elsewhere) **or** clearly document that device paths are treated case-insensitively on all supported platforms and update the comments accordingly.
- If you gate by OS, update/guard the `ConnectByPathAsync_MatchesPathCaseInsensitively` test so it only asserts ignore-case behavior on Windows (or adjusts expectations on other OSes).

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

---

6. Misleading HidSharp comment ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The new comments state that the HID device path “always originates from the same in-process HidSharp
enumeration”, but the codebase explicitly uses native IOKit on macOS via IHidPlatform, so this
statement is not universally true. This can mislead maintainers into making incorrect cross-platform
assumptions about path provenance/format/casing guarantees.

Code

src/Daqifi.Core/Communication/Transport/HidLibraryTransport.cs[R126-133]

+        // Match the device path case-sensitively (Ordinal). A device path is an OS-level identifier —
+        // case-sensitive on Linux/macOS — so a case-insensitive compare would impose Windows semantics
+        // on the cross-platform HID layer. It is also unnecessary here: the path always originates from
+        // the same in-process HidSharp enumeration the caller used to obtain it, so its casing is
+        // consistent. (The serial filter uses OrdinalIgnoreCase because a serial is a user-facing
+        // string, not an OS path identifier.)
+        await ConnectMatchingDeviceAsync(
+            candidate => string.Equals(candidate.DevicePath, devicePath, StringComparison.Ordinal),

Relevance

⭐⭐⭐ High

Team often fixes misleading docs/comments; comment/implementation mismatches were accepted in PRs
#201 and #98.

PR-#201
PR-#98
PR-#160

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The comments in HidLibraryTransport and FirmwareUpdateService explicitly attribute path provenance
to HidSharp, but HidLibraryDeviceEnumerator documents that macOS uses native IOKit (not HidSharp)
via IHidPlatform, making the new comment technically incorrect on that platform.

src/Daqifi.Core/Communication/Transport/HidLibraryTransport.cs[126-133]
src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[1110-1118]
src/Daqifi.Core/Communication/Transport/HidLibraryDeviceEnumerator.cs[10-16]

Agent prompt

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

## Issue description
Comments added in the new path-matching logic claim the device path always comes from the same in-process HidSharp enumeration. This is inaccurate on macOS, where enumeration is backed by native IOKit via the IHidPlatform abstraction.

## Issue Context
The code behavior (Ordinal path matching) is fine; the problem is the comment’s over-specific assertion about HidSharp, which is not true on all platforms in this repo.

## Fix Focus Areas
- src/Daqifi.Core/Communication/Transport/HidLibraryTransport.cs[126-131]
- src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[1113-1114]

## Suggested change
Update the comments to refer to “the same in-process HID enumeration (via IHidPlatform / the same enumerator used by discovery)” rather than “HidSharp enumeration”, and avoid wording like “always” that is not platform-true.

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

---

[cptkoolbeenz]

Addressed all 4 review findings (commit 3683475)

1 & 2 — Breaking public API (UpdateFirmwareAsync signature + new IHidTransport members): reworked to be non-breaking rather than disagree:

• IFirmwareUpdateService.UpdateFirmwareAsync(device, hex, progress, ct) is restored unchanged; targeting is a new overload added as a default interface method (throws NotSupportedException by default), so existing callers and implementers are unaffected.
• IHidTransport.ConnectByPathAsync/ConnectByPath are now default interface methods (throwing default), overridden by HidLibraryTransport — no break for external implementers.
• Side benefit: the desktop's existing 4-arg UpdateFirmwareAsync Moq setups stay valid (no CS0854).

3 — Unvalidated target path: UpdateFirmwareAsync now fast-fails with ArgumentException on a whitespace targetDevicePath (null still = untargeted) instead of polling to the WaitingForBootloader timeout.

4 — Timeout hides target path: the WaitingForBootloader timeout message now appends Target device path requested: <path> when targeting.

Tests: +1 (whitespace validation); full suite 1277 passed / 2 skipped / 0 failed, clean build under TreatWarningsAsErrors.

[cptkoolbeenz]

/improve

[cptkoolbeenz]

/agentic_review

[qodo-code-review]

PR Code Suggestions ✨

Warning

/improve is deprecated. Use /agentic_review instead (removal date not yet scheduled).

Category	Suggestion	Impact
Possible issue	Make path comparison resilient Change the device path string comparison to be case-insensitive (StringComparison.OrdinalIgnoreCase) to handle varying casing across enumerations on platforms like Windows. src/Daqifi.Core/Communication/Transport/HidLibraryTransport.cs [35-38] await ConnectMatchingDeviceAsync( - candidate => string.Equals(candidate.DevicePath, devicePath, StringComparison.Ordinal), + candidate => string.Equals(candidate.DevicePath, devicePath, StringComparison.OrdinalIgnoreCase), $"Path={devicePath}", cancellationToken).ConfigureAwait(false); [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: Using OrdinalIgnoreCase for device paths improves resilience on case-insensitive operating systems like Windows, reducing the risk of false negatives during device matching.	Low
	Validate and use requested path Add validation to ensure the enumerated bootloaderDevice.DevicePath matches the targetDevicePath before connecting. src/Daqifi.Core/Firmware/FirmwareUpdateService.cs [116-132] // When a specific device was requested (several identical bootloaders present), connect to // that exact device by path — bootloaderDevice was matched on the same path. Otherwise fall // back to VID/PID(+serial) first-match for the single-device case. if (targetDevicePath != null) { + if (string.IsNullOrEmpty(bootloaderDevice.DevicePath) || + !string.Equals(bootloaderDevice.DevicePath, targetDevicePath, StringComparison.Ordinal)) + { + throw new IOException( + $"Enumerated bootloader device path did not match requested path '{targetDevicePath}'."); + } + await _hidTransport - .ConnectByPathAsync(bootloaderDevice.DevicePath, ct) + .ConnectByPathAsync(targetDevicePath, ct) .ConfigureAwait(false); } else { await _hidTransport.ConnectAsync( _options.BootloaderVendorId, _options.BootloaderProductId, bootloaderDevice.SerialNumber, ct).ConfigureAwait(false); } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 2 __ Why: The bootloaderDevice is already explicitly matched and filtered by targetDevicePath in WaitForBootloaderDeviceAsync, making this additional validation redundant.	Low
Update

[qodo-code-review]

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

[cptkoolbeenz]

/improve pass 2 (commit baf1a3c)

• Make path comparison resilient (importance 6): ✅ Applied — ConnectByPathAsync and the bootloader wait now match device paths with StringComparison.OrdinalIgnoreCase (Windows HID paths are case-insensitive and casing can drift across enumerations; real paths never differ only by case, so no mis-target risk). Consistent with the serial-filter comparison. +1 test.
• Validate and use requested path (importance 2): ❌ Declined — redundant, as the suggestion's own note says: WaitForBootloaderDeviceAsync already returns the device matched by targetDevicePath, so bootloaderDevice.DevicePath is guaranteed to be that device. Re-checking before connect adds nothing, and using the enumerated device's actual DevicePath (vs the requested string) is the correct value to open.

[cptkoolbeenz]

/improve

[cptkoolbeenz]

/agentic_review

[qodo-code-review]

PR Code Suggestions ✨

Warning

/improve is deprecated. Use /agentic_review instead (removal date not yet scheduled).

No code suggestions found for the PR.

[qodo-code-review]

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

[cptkoolbeenz]

pass 3 ($(SHA))

• Ignore-case path matching (new): ✅ Addressed — reverted to StringComparison.Ordinal in ConnectByPathAsync and the bootloader wait. You're right that OrdinalIgnoreCase (added last pass per a /improve suggestion) imposed Windows-specific semantics on the cross-platform HID layer. A device path is an OS identifier (case-sensitive on Linux/macOS), and in this flow the path always comes from the same in-process HidSharp enumeration the caller used, so casing is consistent and Ordinal is correct. Removed the case-insensitive unit test.

This supersedes the pass-2 /improve "make path comparison resilient" suggestion — Ordinal is the right call for cross-platform OS path identifiers.

[cptkoolbeenz]

/agentic_review

[qodo-code-review]

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

[cptkoolbeenz]

pass 4 (eaabb15): ✅ Fixed the misleading HidSharp comment — reworded to 'the same in-process HID enumeration (via IHidPlatform)' (macOS uses IOKit, not HidSharp) and dropped 'always'. Comment-only; Ordinal matching unchanged.

[cptkoolbeenz]

/agentic_review

[qodo-code-review]

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