fix(api): typed DeviceNotConnectedException from Core's connectivity guards (closes #395)

[tylerkron]

Problem

Core's up-front connectivity guards threw a plain InvalidOperationException("Device is not connected."). A client that wants to log "the device went away" (ordinary, expected — the user pressed Disconnect while a refresh was in flight, WiFi dropped mid-transfer) at warning with reconnect guidance, and genuine defects at error with a Sentry issue, had no type to match on. The only lever was the message string, which breaks silently on any wording change — and the failure mode is quiet: a regression to false alerts, visible only in alert volume.

That sat badly beside Core's own direction elsewhere: TransportNotConnectedException, SdCardNotPresentException, FeatureNotSupportedException and ScpiInitializationErrorException are all typed precisely so callers don't read messages.

The issue also flags a real asymmetry: deeper in ExecuteTextCommandAsync the transport check does throw the typed TransportNotConnectedException, but Disconnect() sets _isDisconnecting and updates Status before that point — so in an actual disconnect race the untyped guard fired first and the typed one was unreachable.

The change

New DeviceNotConnectedException, thrown from every guard that previously threw the untyped exception.

Sites converted	43 in DaqifiStreamingDevice.cs, 3 in DaqifiDevice.cs (Send<T>, ExecuteRawCaptureAsync, ExecuteTextCommandCoreAsync)
Also converted	InitializeAsync's !IsConnected guard (same condition, different wording)
Shutdown guards	ExecuteTextCommandCoreAsync's _disposed || _isDisconnecting check, and the ObjectDisposedException race on _textExchangeLock — both throw DeviceNotConnectedException with IsShuttingDown = true

Type choice: a new type, not TransportNotConnectedException

The issue offered either. I added a new type because the two conditions are genuinely different, and reusing the transport type would be a lie in the disconnect race: a device can fail this guard while holding a perfectly healthy transport (mid-Disconnect()), and a transport can drop while the device still reports IsConnected. They are documented as deliberate siblings, not a hierarchy — neither derives from the other, and a test pins that in both directions.

Base class: derives from InvalidOperationException

Deliberate, and the same trade TransportNotConnectedException already made one layer down. Existing catch (InvalidOperationException) sites keep working unchanged, so consumers migrate on their own schedule. Verified against a real device (see below): a legacy catch (InvalidOperationException) still catches the guard.

The messages are also byte-for-byte unchanged, so downstream code still matching on the string keeps working during migration.

Disposing/disconnecting: a property, not a second type

IsShuttingDown distinguishes "a Disconnect()/Dispose() is in flight or done" from "was never connected". A second exception type would have been more machinery than the distinction earns: the downstream workaround this issue was filed from treats both cases identically (log at warning, offer reconnect). The flag is there for callers that want to go further — e.g. suppress a retry prompt when the user themselves initiated the disconnect.

What stayed an InvalidOperationException

The line is exactly "device went away" vs "the app has a bug". These are bugs and were deliberately left alone — a test asserts the re-entrancy guard is not assignable to DeviceNotConnectedException:

• ExecuteTextCommandAsync re-entrancy from a setupAction
• "requires a transport-based connection" / "has no transport or stream to send on" (producer-less constructor)
• "Cannot delete/format/download … while logging to SD card"
• The analog-channel bitmask range check

Breaking-change surface & caller audit

Source-compatible for catch; NOT compatible with exact-type assertions. That is the honest headline: xUnit's Assert.Throws<T> (and MSTest's Assert.ThrowsException<T>) match the exact type and reject derived types. catch (InvalidOperationException) keeps working; Assert.Throws<InvalidOperationException>(...) does not.

Repo-wide sweep (src/, src/Daqifi.Mcp, tests, docs/):

• 41 existing tests asserted the exact type at a disconnected guard and are updated to DeviceNotConnectedException. Tests asserting InvalidOperationException for other conditions (_WhenLogging_, _OverNonUsbConnection_, _BeyondBitmaskRange_, _ProducerlessConstructor_, re-entrancy) are deliberately untouched — they now double as regression coverage for the line drawn above.
• Test names that said ThrowsInvalidOperationException for a disconnect scenario were renamed to name the real type.
• No catch (InvalidOperationException) in Core sits on a device-guard path. The two that exist (ProcessExternalProcessRunner, SerialStreamTransport) are unrelated.
• src/Daqifi.Mcp left unchanged. Its Require(deviceId) guards are application-level (registry lookup / stale-entry eviction, with their own messages), not the Core device API this issue is about. Its tests assert message substrings and all 23 still pass.
• XML docs updated across DaqifiStreamingDevice, DaqifiDevice, ISdCardOperations, IDeviceDiagnostics. Two stale compound docs were split and corrected while I was in them: "not connected or currently logging" now names both exceptions separately, and "not connected or not using a USB/serial transport" was simply wrong — that path throws FeatureNotSupportedException now (SD-over-WiFi gating), not an InvalidOperationException.
• docs/DEVICE_INTERFACES.md gains a "Telling 'the device went away' apart from a bug" section under Error Handling, with the catch-ordering rule and the sibling relationship spelled out.

What downstream can delete

The Avalonia port (daqifi-avalonia#2) can delete its string-matching workaround outright:

private static bool IsCoreDisconnectGuard(InvalidOperationException ex) =>
    ex.Message.Contains("Device is not connected", StringComparison.OrdinalIgnoreCase)
    || ex.Message.Contains("disposing or disconnecting", StringComparison.OrdinalIgnoreCase);

…and replace every call site with catch (DeviceNotConnectedException). Both branches of that predicate map onto the one type, so nothing is lost; IsShuttingDown recovers the distinction the second branch was reaching for. That helper also silently missed the "…because the device is disposed." race path — which now carries IsShuttingDown = true like its siblings, so the typed catch is strictly more complete than the string match ever was.

daqifi-desktop has no Core-message matching (its NOT_CONNECTED_MESSAGE const guards its own throws in AbstractStreamingDevice), so it needs no change to keep building, and Daqifi.Desktop.Test asserts message text rather than the Core type, so those keep passing too.

Tests

New DeviceNotConnectedExceptionTests:

• Type contract: derives from InvalidOperationException; not related to TransportNotConnectedException in either direction; default message; ctor behavior; IsShuttingDown defaults.
• 19 synchronous guard sites and 22 asynchronous ones (SD, network, diagnostics, friendly name) as theories, each asserting the type, the message, and IsShuttingDown == false.
• The disposing/disconnecting distinction: not-connected (false) vs _isDisconnecting (true) vs _disposed (true) vs the racing-Dispose()-disposed-semaphore path (true).
• Source compatibility: a legacy catch (InvalidOperationException) still catches the guard, and a disconnect is separable from an unrelated InvalidOperationException without reading either message.

Full suite green on net9.0 and net10.0 — 1987 passed, 2 skipped, plus 23 in Daqifi.Mcp.Tests. Release build clean, 0 warnings (the project already runs TreatWarningsAsErrors + GenerateDocumentationFile, so every new cref resolves).

Bench evidence

Real DAQiFi Nyquist 1, firmware 3.7.2, USB CDC on /dev/cu.usbmodem1101. Non-destructive throughout — no format, no delete, no reboot, no firmware flash, no WiFi reconfiguration, nothing unplugged. The disconnect below is a clean software-only Disconnect(); the physical-unplug case belongs to #382.

No regression while connected (example CLI built against this branch):

SD LIST → 31 file(s)                                              exit 0
stream  → 10 Hz, 3 s, channels 0+1, samples flowing, clean stop   exit 0

Post-Disconnect(), every guard surfaces the typed exception:

[bench] connected: IsConnected=True status=Connected
[bench] firmware=3.7.2 serial=9090539562006014104
[bench] SD LIST returned 31 file(s) while connected
[bench] disconnected: IsConnected=False status=Disconnected
[ OK ] GetSdCardFilesAsync:     DeviceNotConnectedException (IsShuttingDown=False) "Device is not connected."
[ OK ] GetSdCardStorageAsync:   DeviceNotConnectedException (IsShuttingDown=False) "Device is not connected."
[ OK ] StopSdCardLoggingAsync:  DeviceNotConnectedException (IsShuttingDown=False) "Device is not connected."
[ OK ] DeleteSdCardFileAsync:   DeviceNotConnectedException (IsShuttingDown=False) "Device is not connected."
[ OK ] DownloadSdCardFileAsync: DeviceNotConnectedException (IsShuttingDown=False) "Device is not connected."
[ OK ] GetSystemLogAsync:       DeviceNotConnectedException (IsShuttingDown=False) "Device is not connected."
[ OK ] StartStreaming:          DeviceNotConnectedException (IsShuttingDown=False) "Device is not connected."
[ OK ] Send:                    DeviceNotConnectedException (IsShuttingDown=False) "Device is not connected."
[ OK ] legacy catch (InvalidOperationException) still catches it: DeviceNotConnectedException
[bench] ALL CHECKS PASSED

That last line is the back-compat claim verified on hardware, not just in a unit test.

Sibling PRs

Developed in parallel with other issues touching adjacent code; textual conflicts are possible, semantic overlap is not:

• api: a timed-out SD LIST is indistinguishable from an empty SD card #396 owns the timeout/return semantics inside ExecuteTextCommandCoreAsync's body. This PR only re-types the guards at the top of these methods and changes no timeout behavior.
• api: retire four downstream workarounds — SD download failure typing, TimestampProcessor reset semantics, FirmwareUpdateException Verifying overload #398 owns typing exceptions inside SdCardFileReceiver — a distinct site this PR does not touch.
• reliability: TcpStreamTransport never reports a silent link death — IsConnected stays true indefinitely #394 is the companion issue from the same downstream audit.

Follow-up (not in scope here)

HidLibraryTransport still throws a plain InvalidOperationException("HID transport is not connected.") where TransportNotConnectedException would fit. That's a transport-layer site, not one of the device guards this issue covers, so it's left for a separate change.

closes #395

Not merging — opened for review.

🤖 Generated with Claude Code

[qodo-code-review]

PR Summary by Qodo

fix(api): Add typed DeviceNotConnectedException for connectivity guards

🐞 Bug fix ✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Replace untyped "Device is not connected" guards with DeviceNotConnectedException across Core
 device APIs.
• Flag disconnect/dispose races via IsShuttingDown on the text-command execution path.
• Update docs and tests to classify expected disconnects without message-string matching.

Diagram

sequenceDiagram
actor Client
participant Streaming as DaqifiStreamingDevice
participant Core as DaqifiDevice
participant Transport as Transport/Stream
participant DNC as "DeviceNotConnectedException"
participant TNC as "TransportNotConnectedException"

Client->>Streaming: Call device operation
Streaming->>Core: Execute / delegate
Core->>Core: Connectivity guards
alt Not connected
Core-->>Client: throw DNC
else Disposing/Disconnecting
Core-->>Client: throw DNC (IsShuttingDown=true)
else Connected
Core->>Transport: Send / ExecuteTextCommand
alt Transport dropped mid-operation
Transport-->>Client: throw TNC
else Success
Transport-->>Client: Responses
end
end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Reuse TransportNotConnectedException for all connectivity failures

• ➕ No new public exception type to document or test
• ➕ Single catch type for most “device unavailable” cases
• ➖ Conflates device-state disconnect/shutdown with transport-level drop
• ➖ Misleads callers during Disconnect()/Dispose() races where transport may still be healthy
• ➖ Harder to provide nuanced UX (e.g., suppress retry prompts on user-initiated disconnect)

2. Introduce a common ConnectivityException base (DeviceNotConnectedException : ConnectivityException, TransportNotConnectedException : ConnectivityException)

• ➕ Allows a single catch for “any connectivity” while preserving specificity
• ➕ Encodes the conceptual relationship explicitly in the type system
• ➖ Bigger API surface change (new base type + inheritance changes)
• ➖ Potentially breaking for consumers relying on existing inheritance/exception filters

3. Return typed Result/Error codes instead of throwing

• ➕ Avoids exceptions for expected disconnects; can simplify control flow
• ➕ Can carry richer machine-readable metadata
• ➖ Large, invasive API redesign across sync/async surfaces
• ➖ Not aligned with existing exception-based contract in Core

Recommendation: The PR’s approach (new DeviceNotConnectedException sibling to TransportNotConnectedException, deriving from InvalidOperationException) is the best fit: it fixes the message-matching problem, cleanly distinguishes device-state vs transport-drop cases, and preserves backwards compatibility for existing catch (InvalidOperationException) consumers. A shared ConnectivityException base could be valuable long-term, but it’s a larger breaking-shape change than needed to resolve #395.

Files changed (20) +653 / -155

Enhancement (1) +104 / -0

DeviceNotConnectedException.csIntroduce DeviceNotConnectedException with IsShuttingDown flag +104/-0

Introduce DeviceNotConnectedException with IsShuttingDown flag

• Adds a new public exception type deriving from InvalidOperationException to represent device-state connectivity failures. Preserves the historical default message and exposes IsShuttingDown to distinguish teardown races (Disconnect/Dispose in flight) from never-connected states.

src/Daqifi.Core/Device/DeviceNotConnectedException.cs

Bug fix (2) +97 / -76

DaqifiDevice.csThrow DeviceNotConnectedException from core device connectivity guards +32/-16

Throw DeviceNotConnectedException from core device connectivity guards

• Replaces several pre-flight !IsConnected guards (Send, ExecuteRawCaptureAsync, text-command core path, InitializeAsync) to throw DeviceNotConnectedException instead of a plain InvalidOperationException. Converts dispose/disconnect text-command guards and the lock disposal race to DeviceNotConnectedException with IsShuttingDown=true, and updates XML exception documentation accordingly.

src/Daqifi.Core/Device/DaqifiDevice.cs

DaqifiStreamingDevice.csThrow DeviceNotConnectedException across streaming device guard sites +65/-60

Throw DeviceNotConnectedException across streaming device guard sites

• Updates dozens of connectivity guards throughout the streaming device API (stream control, channel management, SD, network, diagnostics) to throw DeviceNotConnectedException while keeping existing messages intact. Updates XML docs to reflect the typed exception.

src/Daqifi.Core/Device/DaqifiStreamingDevice.cs

Tests (14) +380 / -60

DaqifiDeviceInitializeTests.csUpdate InitializeAsync disconnected test to expect DeviceNotConnectedException +1/-1

Update InitializeAsync disconnected test to expect DeviceNotConnectedException

• Switches the InitializeAsync disconnected guard assertion from InvalidOperationException to DeviceNotConnectedException while keeping message validation.

src/Daqifi.Core.Tests/Device/DaqifiDeviceInitializeTests.cs

DaqifiDeviceTests.csUpdate DaqifiDevice.Send disconnected test to typed exception +2/-2

Update DaqifiDevice.Send disconnected test to typed exception

• Renames and updates the Send() disconnected test to assert DeviceNotConnectedException instead of InvalidOperationException.

src/Daqifi.Core.Tests/Device/DaqifiDeviceTests.cs

DaqifiDeviceTextCommandLockTests.csUpdate text-command guard tests to DeviceNotConnectedException +7/-7

Update text-command guard tests to DeviceNotConnectedException

• Adjusts dispose/disconnect and validation-failure tests on the ExecuteTextCommandAsync path to assert DeviceNotConnectedException, ensuring lock release behavior remains validated.

src/Daqifi.Core.Tests/Device/DaqifiDeviceTextCommandLockTests.cs

DaqifiDeviceWithMessageProducerTests.csExpect DeviceNotConnectedException when sending without connection (message producer) +1/-1

Expect DeviceNotConnectedException when sending without connection (message producer)

• Updates the disconnected Send() test for producer-based devices to assert DeviceNotConnectedException.

src/Daqifi.Core.Tests/Device/DaqifiDeviceWithMessageProducerTests.cs

DaqifiDeviceWithTransportTests.csExpect DeviceNotConnectedException when sending without connection (transport) +1/-1

Expect DeviceNotConnectedException when sending without connection (transport)

• Updates the disconnected Send() test for transport-constructed devices to assert DeviceNotConnectedException.

src/Daqifi.Core.Tests/Device/DaqifiDeviceWithTransportTests.cs

DaqifiStreamingDeviceChannelManagementTests.csSwitch channel-management disconnected assertions to DeviceNotConnectedException +11/-11

Switch channel-management disconnected assertions to DeviceNotConnectedException

• Updates channel-management and PWM-related tests to assert the new typed guard exception across multiple disconnected operations.

src/Daqifi.Core.Tests/Device/DaqifiStreamingDeviceChannelManagementTests.cs

DaqifiStreamingDeviceFriendlyNameTests.csUpdate SetFriendlyNameAsync disconnected test to typed exception +1/-1

Update SetFriendlyNameAsync disconnected test to typed exception

• Changes the not-connected async friendly-name test to expect DeviceNotConnectedException.

src/Daqifi.Core.Tests/Device/DaqifiStreamingDeviceFriendlyNameTests.cs

DaqifiStreamingDevicePwmFrequencyTests.csUpdate PWM frequency disconnected test to typed exception +1/-1

Update PWM frequency disconnected test to typed exception

• Ensures SetPwmFrequency throws DeviceNotConnectedException when disconnected and still performs no sends.

src/Daqifi.Core.Tests/Device/DaqifiStreamingDevicePwmFrequencyTests.cs

DaqifiStreamingDeviceTests.csUpdate streaming and calibration disconnected tests to typed exception +12/-12

Update streaming and calibration disconnected tests to typed exception

• Renames and updates multiple disconnected-operation tests (streaming start/stop, NVM persistence, calibration APIs) to assert DeviceNotConnectedException.

src/Daqifi.Core.Tests/Device/DaqifiStreamingDeviceTests.cs

DeviceNotConnectedExceptionTests.csAdd contract + coverage suite for DeviceNotConnectedException and guard sites +320/-0

Add contract + coverage suite for DeviceNotConnectedException and guard sites

• Introduces comprehensive tests covering type hierarchy, default message compatibility, sibling relationship to TransportNotConnectedException, guard behavior across many sync/async APIs, and shutdown-race classification via IsShuttingDown. Also verifies legacy catch (InvalidOperationException) still catches the guard.

src/Daqifi.Core.Tests/Device/DeviceNotConnectedExceptionTests.cs

DeviceDiagnosticsTests.csUpdate diagnostics disconnected tests to typed exception +3/-3

Update diagnostics disconnected tests to typed exception

• Switches disconnected diagnostics API tests (system log, clear log, memory diagnostics) to assert DeviceNotConnectedException.

src/Daqifi.Core.Tests/Device/Diagnostics/DeviceDiagnosticsTests.cs

GetLanChipInfoAsyncTests.csUpdate LAN chip info disconnected test to typed exception +1/-1

Update LAN chip info disconnected test to typed exception

• Changes GetLanChipInfoAsync disconnected assertion to DeviceNotConnectedException.

src/Daqifi.Core.Tests/Device/GetLanChipInfoAsyncTests.cs

NetworkConfigurableTests.csUpdate network configuration disconnected tests to typed exception +10/-10

Update network configuration disconnected tests to typed exception

• Updates PrepareSdInterface/PrepareLanInterface and async network configuration guard tests to assert DeviceNotConnectedException and preserve message expectations.

src/Daqifi.Core.Tests/Device/Network/NetworkConfigurableTests.cs

SdCardOperationsTests.csUpdate SD card disconnected tests to typed exception +9/-9

Update SD card disconnected tests to typed exception

• Switches SD card operation tests (files, storage, logging, delete/format, downloads, min free space) to assert DeviceNotConnectedException.

src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs

Documentation (3) +72 / -19

DEVICE_INTERFACES.mdDocument typed disconnect classification with DeviceNotConnectedException +47/-0

Document typed disconnect classification with DeviceNotConnectedException

• Adds guidance and sample code showing how to catch DeviceNotConnectedException to treat disconnects as expected warnings. Explains IsShuttingDown semantics and the sibling relationship with TransportNotConnectedException.

docs/DEVICE_INTERFACES.md

IDeviceDiagnostics.csUpdate diagnostics interface docs to reference DeviceNotConnectedException +8/-8

Update diagnostics interface docs to reference DeviceNotConnectedException

• Adjusts XML documentation on IDeviceDiagnostics methods to specify DeviceNotConnectedException for not-connected guards.

src/Daqifi.Core/Device/Diagnostics/IDeviceDiagnostics.cs

ISdCardOperations.csUpdate SD card interface docs to reference DeviceNotConnectedException +17/-11

Update SD card interface docs to reference DeviceNotConnectedException

• Updates XML documentation across ISdCardOperations to list DeviceNotConnectedException for not-connected guards and clarify remaining InvalidOperationException cases (e.g., logging-in-progress).

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

[tylerkron]

/agentic_review

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}