Skip to content

fix: prevent spurious Lost status during intentional disconnect#162

Merged
tylerkron merged 1 commit intomainfrom
fix/serial-disconnect-spurious-lost-status
Apr 4, 2026
Merged

fix: prevent spurious Lost status during intentional disconnect#162
tylerkron merged 1 commit intomainfrom
fix/serial-disconnect-spurious-lost-status

Conversation

@tylerkron
Copy link
Copy Markdown
Contributor

Summary

  • Adds _isDisconnecting flag to DaqifiDevice to distinguish intentional disconnects from unexpected transport drops
  • Prevents the spurious Connected → Lost → Disconnected transition when calling Disconnect() — now goes straight to Disconnected
  • Unexpected transport losses still correctly report Lost status

Closes #161

Test plan

  • New test DaqifiDevice_Disconnect_ShouldNotReportLostStatus verifies no Lost status during intentional disconnect
  • Existing test DaqifiDevice_TransportConnectionLost_ShouldUpdateStatus still passes for unexpected drops
  • All 9 transport tests pass on both net8.0 and net9.0

🤖 Generated with Claude Code

When Disconnect() is called, the transport fires StatusChanged(false)
before Disconnect() sets Status = Disconnected, causing an unwanted
Connected → Lost → Disconnected transition. Add an _isDisconnecting
flag so OnTransportStatusChanged() skips the Lost state during
intentional disconnects while still reporting it for unexpected drops.

Closes #161

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@tylerkron tylerkron requested a review from a team as a code owner April 4, 2026 04:38
@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Prevent spurious Lost status during intentional disconnect

🐞 Bug fix 🧪 Tests

Grey Divider

Walkthroughs

Description
• Adds _isDisconnecting flag to distinguish intentional disconnects from unexpected transport
  drops
• Prevents spurious Connected → Lost → Disconnected transition during Disconnect() calls
• New test verifies no Lost status reported during intentional disconnect
• Existing tests for unexpected transport losses remain unaffected
Diagram
flowchart LR
  A["Disconnect called"] --> B["Set _isDisconnecting flag"]
  B --> C["Transport fires StatusChanged"]
  C --> D{"_isDisconnecting?"}
  D -->|Yes| E["Skip Lost state"]
  D -->|No| F["Report Lost status"]
  E --> G["Set Disconnected status"]
  F --> G
  G --> H["Clear _isDisconnecting flag"]
Loading

Grey Divider

File Changes

1. src/Daqifi.Core/Device/DaqifiDevice.cs 🐞 Bug fix +6/-2

Add disconnect flag to prevent spurious Lost status

• Added _isDisconnecting private boolean field to track intentional disconnect state
• Set flag to true at start of Disconnect() method
• Reset flag to false in finally block after disconnect completes
• Modified OnTransportStatusChanged() to skip Lost status when _isDisconnecting is true

src/Daqifi.Core/Device/DaqifiDevice.cs


2. src/Daqifi.Core.Tests/Device/DaqifiDeviceWithTransportTests.cs 🧪 Tests +23/-1

Add test for intentional disconnect without Lost status

• Added new test DaqifiDevice_Disconnect_ShouldNotReportLostStatus() to verify intentional
 disconnect behavior
• Test connects device, subscribes to status changes, calls disconnect, and asserts Lost status is
 never reported
• Test verifies device transitions directly to Disconnected status
• Existing DaqifiDevice_TransportConnectionLost_ShouldUpdateStatus() test remains unchanged

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


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review bot commented Apr 4, 2026

Code Review by Qodo

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

Grey Divider

Great, no issues found!

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

Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@tylerkron tylerkron merged commit 0829573 into main Apr 4, 2026
1 check passed
@tylerkron tylerkron deleted the fix/serial-disconnect-spurious-lost-status branch April 4, 2026 04:42
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.

bug: serial streaming session ends with 'Lost' status on disconnect

1 participant