feat(device): modernize device interfaces — async + CancellationToken on IStreamingDevice, disposability on IDevice - #469
Conversation
…cDisposable on IDevice Follow-on to #333, closes #460. - IDevice extends IAsyncDisposable. ConnectAsync/DisconnectAsync are now genuine abstract interface members instead of default-interface-method shims that quietly wrapped Connect()/Disconnect() — every implementer must honor the CancellationToken itself. - IStreamingDevice extends IConfirmingDeviceAdministration, so the confirming ADC-calibration/voltage-precision calls are reachable directly off an IStreamingDevice reference with no cast. - IStreamingDevice gains cancellable ...Async twins for every streaming/channel/DIO/PWM/analog-output/reboot method (StartStreamingAsync, EnableChannelAsync, SetDioValueAsync, SetPwmEnabledAsync, SetAnalogOutputAsync, RebootAsync, etc.), with default-interface-method bodies that thinly wrap the synchronous call — most of this surface has no genuine async machinery underneath today, so that default is an honest implementation, not a stopgap. DaqifiStreamingDevice implements them explicitly as regular class members too, so they're callable directly on the concrete type the same way the existing sync methods are. - Sync methods are unchanged and remain the primary API for existing callers. - Updated the five IStreamingDevice test fakes in FirmwareUpdateServiceTests.cs / LanChipInfoProviderExtensionsTests.cs for the new interface shape (ConnectAsync/DisconnectAsync, DisposeAsync, the 9 confirming calibration members). - README.md and docs/DEVICE_INTERFACES.md updated. Breaking change: IDevice/IStreamingDevice implementers must add ConnectAsync, DisconnectAsync, DisposeAsync, and the IConfirmingDeviceAdministration calibration members. Batch into the same release note as #333 per the issue.
PR Summary by QodoModernize device interfaces: cancellable async streaming ops + IAsyncDisposable
AI Description
Diagram
High-Level Assessment
Files changed (9)
|
Code Review by Qodo
1.
|
…cking Qodo review (PR #469): RebootAsync delegated to the synchronous Reboot(), which tears down through the blocking DaqifiDevice.Disconnect() — up to a 10s stall on the caller thread, contradicting the documented non-blocking contract. RebootAsync now sends the reboot command directly and awaits DaqifiDevice.DisconnectAsync instead, mirroring how ConnectAsync/ DisconnectAsync are genuinely async on the base class. The synchronous Reboot() is unchanged for callers that want the blocking behavior.
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 70cfbd4 |
…tion Qodo review (PR #469, round 2): RebootAsync checked IsConnected before the cancellation token, so a pre-cancelled call against a disconnected device surfaced DeviceNotConnectedException instead of OperationCanceledException — inconsistent with every other ...Async member on the class, which all check cancellation first. Swapped the order and added a regression test.
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 66a019b |
… test Qodo review (PR #469, round 3): the disconnected+pre-cancelled RebootAsync test only checked the exception type, not that cancellation actually short-circuited before any side effect. Adds the same empty-SentMessages/still-disconnected assertions the sibling cancellation tests already make.
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit b0d1fa2 |
Bench testRan a real-hardware check of `RebootAsync` on the bench Nq1 (fw 3.7.2, USB CDC, `/dev/cu.usbmodem1101`) against this branch's `Daqifi.Core` — this is the one behavior in the PR a fake-transport unit test can't exercise (real link-drop timing racing the async teardown). Result: pass.
No code changes resulted from this run. |
Summary
Closes #460. Follow-on to #333 (not yet merged): that ticket promotes missing members and fixes factory return types; this one is the shape of the contracts — the three inconsistencies the issue calls out.
IStreamingDeviceasync surface. Every streaming/channel/DIO/PWM/analog-output/reboot method now has a cancellable...Asynctwin declared directly on the interface (StartStreamingAsync,EnableChannelAsync,SetDioValueAsync,SetPwmEnabledAsync,SetAnalogOutputAsync,RebootAsync, …).IStreamingDevicealso now extendsIConfirmingDeviceAdministration, so the confirming ADC-calibration/voltage-precision calls are reachable directly off anIStreamingDevicereference — no separate cast.IDevice.ConnectAsync/DisconnectAsyncare genuine abstract members now, not default-interface-method shims that quietly wrappedConnect()/Disconnect(). Every implementer must honor theCancellationTokenitself.IDeviceextendsIAsyncDisposable. A consumer holding only the interface can nowawait using/await device.DisposeAsync()without a cast toDaqifiDevice.Sync methods are unchanged and remain the primary API for existing callers — nothing was removed.
Design notes
...Asyncsurface (streaming/channel/DIO/PWM/output/reboot) has no genuine async machinery underneath today —DaqifiStreamingDevice's implementations are fire-and-forget synchronous writes. So the interface's default-interface-method body for each is a thin, cancellable wrapper over the sync call — that's an honest implementation of what's actually happening, not a stopgap (unlike the oldConnectAsync/DisconnectAsyncshims, which hid genuinely async machinery that already existed inDaqifiDevice).DaqifiStreamingDevicealso implements each of these explicitly as a regular class member (not relying purely on the DIM default), so they're callable directly on the concrete type the same way the existing sync methods are — a DIM-only default is only reachable through the interface type.DaqifiStreamingDevicealready runs real async machinery for those (drain-error-queue + confirm), onIConfirmingDeviceAdministration. MakingIStreamingDeviceextend that interface (rather than duplicating differently-behaved members with the same names) reuses it directly with zero code changes to the concrete class.Breaking changes
Any
IDevice/IStreamingDeviceimplementer outside this repo needs to add:ConnectAsync,DisconnectAsync(previously optional via DIM)DisposeAsync(IAsyncDisposable)IConfirmingDeviceAdministrationcalibration members (previously only reachable viais IConfirmingDeviceAdministrationcast)Per the issue, batch this into the same release note as #333 when both ship.
Testing
IStreamingDevicetest fakes inFirmwareUpdateServiceTests.cs/LanChipInfoProviderExtensionsTests.csfor the new interface shape.DaqifiStreamingDeviceAsyncSurfaceTests.cs: delegation + cancellation behavior for the new...Asyncmethods,IDevice.ConnectAsync/DisconnectAsyncreachable through the interface,IDevice : IAsyncDisposable, and a minimalIStreamingDeviceimplementer proving the default-interface-method bodies work when reached only through the interface type.dotnet build— solution builds clean (0 warnings, 0 errors).dotnet test— full suite passes: 2846 passed, 2 skipped (real-hardware tests), 0 failed, both net9.0 and net10.0. PlusDaqifi.Mcp.Tests(23 passed).dotnet format --verify-no-changes— no new formatting issues introduced (pre-existing drift in files this PR doesn't touch).No bench hardware test was needed: this PR only reshapes interface contracts and adds thin wrappers over already-tested code paths (
DaqifiStreamingDevice's sync methods,DaqifiDevice's existingConnectAsync/DisconnectAsync/DisposeAsync), all covered by the unit suite above.Docs
IDevice/IStreamingDevicecore-interface sections, the "Connecting and disconnecting without blocking" section, and Channel Management examples.🤖 Generated with Claude Code