Skip to content

Fix: only log USB event once per debounce window in SerialPortObserver - #106

Merged
heavyrubberslave merged 1 commit into
mainfrom
fix/duplicate-usb-event-log
Aug 7, 2026
Merged

Fix: only log USB event once per debounce window in SerialPortObserver#106
heavyrubberslave merged 1 commit into
mainfrom
fix/duplicate-usb-event-log

Conversation

@heavyrubberslave

@heavyrubberslave heavyrubberslave commented Aug 7, 2026

Copy link
Copy Markdown
Member

Duplicate log lines were seen for a single physical USB plug event because the underlying usb lib can fire multiple native events (e.g. connect twice for a composite USB-serial adapter) close together. The log statement fired on every raw event, before the debounce timer had a chance to collapse them, even though only one actual scan ran.

Now the log only fires when opening a new debounce window (i.e. when no rescan is already pending), and rescanTimer is reset once the scan runs so a later, separate USB event still logs correctly.

Summary by CodeRabbit

  • Bug Fixes

    • Improved USB device event handling to prevent duplicate delayed-rescan log messages during rapid event bursts.
    • Rescan scheduling continues to work correctly after the debounce period completes.
  • Tests

    • Added coverage for suppressing duplicate event logs and logging subsequent events appropriately.

Duplicate log lines were seen for a single physical USB plug event
because the underlying usb lib can fire multiple native events (e.g.
connect twice for a composite USB-serial adapter) close together. The
log statement fired on every raw event, before the debounce timer had
a chance to collapse them, even though only one actual scan ran.

Now the log only fires when opening a new debounce window (i.e. when
no rescan is already pending), and rescanTimer is reset once the scan
runs so a later, separate USB event still logs correctly.
@heavyrubberslave heavyrubberslave added the patch Creates a new patch/bugfix release if merged label Aug 7, 2026
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

USB event handling now suppresses duplicate delayed-rescan logs while a rescan timer is active. The timer resets when the scan starts. Tests cover repeated events during and after the debounce window.

Changes

USB rescan logging

Layer / File(s) Summary
Debounced rescan logging and validation
src/device/transport/serialPortObserver.ts, tests/unit/device/transport/serialPortObserver.spec.ts
The observer logs only the first event during a pending rescan, clears and replaces existing timers, and resets the timer before scanning. Tests verify duplicate-log suppression and logging after the one-second debounce period.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preventing duplicate USB event logs within one debounce window.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/duplicate-usb-event-log

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
tests/unit/device/transport/serialPortObserver.spec.ts (1)

291-305: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the number of delayed scans.

These tests assert only mockLogger.debug. The first test can pass even if both timers execute later. The second test can pass if the later event logs but its replacement timer never runs.

Record the SerialPort.list call count after observer.start(). Advance the fake clock after each event sequence. Assert one additional scan in the first test and two additional scans in the second test.

Proposed test strengthening
             const observer = createObserver();
             await observer.start();
+            const listCallsAfterStart = vi.mocked(SerialPort.list).mock.calls.length;

             const onUsbEvent = getRegisteredUsbEventHandler();

             onUsbEvent();
             onUsbEvent();

             expect(mockLogger.debug).toHaveBeenCalledTimes(1);
+            await vi.advanceTimersByTimeAsync(1000);
+            expect(vi.mocked(SerialPort.list)).toHaveBeenCalledTimes(listCallsAfterStart + 1);

Apply the same baseline in the later-event test. Advance the second timer and assert listCallsAfterStart + 2.

Also applies to: 307-322

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/device/transport/serialPortObserver.spec.ts` around lines 291 -
305, Strengthen the USB debounce tests by asserting delayed scans, not only
debug logging. In each test around observer.start, record the SerialPort.list
call count after startup, advance the fake clock after each event sequence, and
assert the first sequence adds one scan while the later-event replacement-timer
sequence adds two scans. Keep the existing log assertions and event setup
unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/unit/device/transport/serialPortObserver.spec.ts`:
- Around line 291-305: Strengthen the USB debounce tests by asserting delayed
scans, not only debug logging. In each test around observer.start, record the
SerialPort.list call count after startup, advance the fake clock after each
event sequence, and assert the first sequence adds one scan while the
later-event replacement-timer sequence adds two scans. Keep the existing log
assertions and event setup unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 54dd959c-cb78-4d94-b0f0-a52af3d65396

📥 Commits

Reviewing files that changed from the base of the PR and between 4acc487 and 68bad81.

📒 Files selected for processing (2)
  • src/device/transport/serialPortObserver.ts
  • tests/unit/device/transport/serialPortObserver.spec.ts

@heavyrubberslave
heavyrubberslave merged commit 907c6ca into main Aug 7, 2026
7 checks passed
@heavyrubberslave
heavyrubberslave deleted the fix/duplicate-usb-event-log branch August 7, 2026 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

patch Creates a new patch/bugfix release if merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant