Skip to content

Report device removal promptly and hold back a device that is still enumerating - #15631

Merged
remibettan merged 19 commits into
realsenseai:developmentfrom
Nir-Az:partial-device-enum
Sep 14, 2026
Merged

remibettan merged 19 commits into
realsenseai:developmentfrom
Nir-Az:partial-device-enum

Conversation

@Nir-Az

@Nir-Az Nir-Az commented Sep 2, 2026 •

Copy link
Copy Markdown
Collaborator

Overview: A camera that re-enumerates in place — after a hardware reset or a firmware update — is now reported as removed within ~100 ms of the OS notification on Windows, and on both Windows and Linux a device is held back until the interfaces it declares have surfaced, so it is not published with only part of its sensors.

Tracked on [RSDEV-14305]

Why

Both watchers decided what changed by diffing two full enumeration snapshots. Three failures came out of that.

1. On Windows the removal was reported late, or not at all. The snapshot is dominated by query_hid_devices(), which walks every sensor the Windows Sensor API knows about. Measured on a developer laptop:

INSTR enumeration: hid 4260ms (20) usb 1ms (10) uvc 84ms (7)   <- camera absent
INSTR enumeration: hid  155ms (23) usb 1ms (10) uvc 33ms (7)   <- camera present

A reset only takes the camera away for 3–6 s, so it was often back — on identical device paths — before the snapshot completed, and list_changed() saw nothing. The application then kept a device whose handles were dead: the viewer left it in its panel, and FW error polling queried it once a second forever.

2. A device was published before all its interfaces had surfaced. On Linux, a D585 came back from a hardware reset with ['Motion Module', 'Stereo Module'] instead of its five sensors in 1 of 6 attempts — missing Depth Mapping Camera, Perception and RGB Camera. On Windows the same class of failure showed up during a firmware update as a depth-only device seconds ahead of the real one.

PR #15058 added a Windows gate for this, but it deferred behind any HID-class composite, including unrelated webcams, and its budget was a single global 15 s rather than per device.

3. Snapshot diffing cannot see a device that leaves and returns on the same path — no ordering or timing fix can change that.

Why it shows on some machines and not others

The Windows trigger is how much the Sensor API has to enumerate, not the camera:

host HID Sensor Collections ghost sensor entries reproduces
developer laptop (Intel Sensor Hub + integrated webcam + accumulated RealSense ghosts) 5 6 yes
bench with three RealSense cameras 3 2 no
CI bench few few no — nightly is green

That is why this was found by hand rather than by CI, and why the CI baseline run below is green with the new assertion but without the fix.

What changed

Windows — src/mf/mf-backend.cpp

  • Removals are reported from the OS notification and _last, with no enumeration at all — the notification names the interface and _last says which device owned it. Removal no longer waits on a multi-second Sensor API walk, and a camera that returns on the same paths is still correctly reported as having left.
  • Removals act at composite granularity: a camera always takes its camera interfaces with it, so those are the reliable signal. Acting on a lone HID removal would drop just the IMU and republish the very partial device this watcher exists to prevent.
  • The USB list is re-read rather than filtered on removal: a composite contributes USB entries under two unrelated ids, so matching by id leaves strays behind that fire a spurious second callback. That query costs about a millisecond even while a camera is missing.
  • The arrival gate is rewritten as incomplete_composites(), replacing hid_binding_in_progress(). It waits only on HID instances carrying a Sensor-class collection — found by searching the whole subtree under the HID interface, so a driver stack that inserts an intermediate node cannot hide it, and gives each composite its own budget. A plain HID interface — a vendor-defined control on an unrelated webcam — never appears in the Sensor API, and waiting on one deferred every device on the machine.
  • Queries are taken in explicit statements rather than as constructor arguments, since argument evaluation order is unspecified and MSVC picks right-to-left, which made the UVC list the stalest of the three.

Linux — src/linux/udev-device-watcher.{cpp,h}

No arrival gate existed here; this adds one, using the same principle with a sysfs probe.

  • The expected interfaces come from the device's own USB configuration descriptor as sysfs exposes it. A bInterfaceClass 0e interface of subclass 01 (VideoControl) must have surfaced a /dev/videoN the backend lists; uvcvideo registers the node against the VideoControl interface and its VideoStreaming siblings never get one, so requiring a node for those would hold every camera back until its budget expired.
  • A class-03 interface is only waited on when its HID child is bound to hid-sensor-hub — the IMU path, which surfaces as an IIO sensor. A button or vendor HID binds hid-generic and never reaches the HID backend at all.
  • Held-back devices are filtered out of the current snapshot rather than deferring the whole callback, so removals of other devices are never delayed.
  • Windows filters the same way, and only for arrivals: a composite already published is never retracted, since the Sensor API drops entries transiently and filtering a live device out would surface as a spurious removal followed by a re-add.

Both

  • The budget is 10 s per device; once it expires the device is published as-is, which is what lets a genuinely partial device through. Expiry logs a warning once per device, naming the device and how long it waited — without it, a too-small budget degrades silently.

Behaviour change

hardware_reset() and firmware updates now raise a removed + added pair where the released SDK sometimes raised nothing. That is the point — the old device's handles are stale after a reboot — but an application that cached a device across a reset will now see it replaced.

Tests added

  • unit-tests/live/hw-reset/pytest-sanity.py — records the sensor set before the reset and asserts the device comes back with all of it, sampled inside the devices-changed callback, because a partial device is superseded by a repaired one a second or two later so polling afterwards would only ever see the good one. Also bounds Windows USB removal latency at 2.5 s.
  • tools/realsense-viewer/tests/hw-reset/test-hw-reset.cpp — the GUI test already checked that the device disappears and returns; it now also latches the sensor set of the first device the viewer publishes and asserts it is complete.
  • unit-tests/live/tools/pytest-realsense-viewer.py — the viewer GUI tests now run on D5xx as well as D4xx, via device_each.

Test plan

Windows:

  • D455, developer laptop (the host that reproduced worst), hardware_reset x10 — 10/10 complete, removal 0.78–1.04 s. Before: 7.5–15 s to report removal.
  • Latency from the OS notification itself, measured in the SDK log: removal callback 0.124–0.183 s (median 0.145 s), arrival publish 0.44–1.29 s (median 0.65 s). The floor is the 100 ms debounce plus the 50 ms tick sleep.
  • D435I, second host, hardware_reset x10 — 10/10 complete, 0.72–0.84 s; 5 instrumented cold plugs — 5/5 complete, zero gate deferrals.
  • D585 Prototype — hardware_reset x10 and 3 DFU firmware flashes, all complete; the recovery device is not gated and appears 0.35 s after the removal.
  • Multi-camera (D455 + D436 on an Acroname, plus two laptop webcams and a D555) — 5 resets of the D455: only the D455 is ever reported removed, the others stay held, 5/5 complete.
  • Overlapping arrivals, the case raised in review — with one composite forced to look incomplete, the unrelated camera used to be blocked until that budget expired (published at 23.0 s); it now publishes at 14.6 s while the other stays held and follows at 19.9 s on expiry. An earlier "3.66 s apart" staggered measurement did not exercise this: the second camera was not in the snapshot yet when the first completed.

Linux (Ubuntu 22.04):

  • D585 Prototype, the case that reproduced 1-in-6 — clean after the fix.
  • Four SKUs on one bus (D436, D455, D585 Prototype, D585S) x4 iterations — 16/16 pass; removal latency 1.19–1.92 s.
  • Per-SKU cold plugs — zero holds, published 1.8–5.4 s. Iteration time dropped 114 s to 49 s once the VideoControl-subclass check landed; before it, every USB camera was held for the full budget and published on expiry, which masked the race for the wrong reason.

CI:

  • LibCI full suite — SUCCESS, 726 tests ran, 0 failed.
  • LibCI targeted -r hw-reset — SUCCESS, 12 ran, 0 failed.
  • Baseline: development plus the new assertion only, -r hw-reset — also green. CI hardware does not reproduce the partial-device race, so the red-before/green-after evidence is the bench measurements above, not CI.

🤖 Generated by AI

@sysrsbuild-gh-agentic sysrsbuild-gh-agentic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Auto-generated review by rs-agentic bot. Focused on potential bugs and guideline violations.

Comment thread src/mf/mf-backend.cpp Resolved
Comment thread src/mf/mf-backend.cpp Resolved
@Nir-Az Nir-Az changed the title Wait for all camera and HID interfaces before publishing a re-enumerated device Fix Windows device-watcher enumeration: partial devices and lost reboot events Sep 2, 2026
@Nir-Az Nir-Az changed the title Fix Windows device-watcher enumeration: partial devices and lost reboot events Windows: report device removal promptly and never publish a device missing sensors Sep 3, 2026

@sysrsbuild-gh-agentic sysrsbuild-gh-agentic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⚠️ [rs-agentic] Auto-review on sync commit — focused on the new/rewritten code in mf-backend.cpp.

instance_id_key(std::string const&) — unsafe narrow-to-wide conversion

static std::string instance_id_key( std::string const & device_path )
{
    std::wstring wide( device_path.begin(), device_path.end() );  // byte-cast, not UTF-8 decode
    return instance_id_key( wide.c_str() );
}

std::wstring(str.begin(), str.end()) widens each char by value, treating each byte as a code point. For a UTF-8 encoded uvc.device_path with any multi-byte sequence (e.g. a vendor string with a non-ASCII character), the resulting wide string is garbled — and the instance_id_from_device_path call inside the LPCWSTR overload will then extract a corrupted instance-ID fragment, causing the lookup in removed_instance_ids to silently miss the match (removal not recognized, no callback fired).

In practice Windows USB device interface paths are ASCII-only (\\?\USB#VID_xxxx&PID_yyyy&MI_zz#...) so this doesn't trigger today, but using MultiByteToWideChar(CP_UTF8, ...) or the project's own wchar_from_utf8 helper would make it correct by construction.


Note: the open thread on info.unique_id = uvc.unique_id (line 285) remains unaddressed.

@sysrsbuild-gh-agentic sysrsbuild-gh-agentic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Auto-review of new commit (rs-agentic bot).

Comment thread src/mf/mf-backend.cpp Resolved

@sysrsbuild-gh-agentic sysrsbuild-gh-agentic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed new commits — one potential bug flagged below in the Linux incomplete-device check.

Comment thread src/linux/udev-device-watcher.cpp Resolved
@Nir-Az
Nir-Az requested a review from remibettan September 7, 2026 08:08
@Nir-Az Nir-Az changed the title Windows: report device removal promptly and never publish a device missing sensors Report device removal promptly and hold back a device that is still enumerating Sep 7, 2026
Comment thread src/mf/mf-backend.cpp Resolved
Comment thread src/mf/mf-backend.cpp Resolved
Comment thread src/mf/mf-backend.cpp Resolved
@Nir-Az
Nir-Az requested a review from remibettan September 9, 2026 11:30
remibettan
remibettan previously approved these changes Sep 10, 2026
Comment thread src/linux/udev-device-watcher.cpp Resolved
Comment thread src/linux/udev-device-watcher.h Resolved
Comment thread src/linux/udev-device-watcher.cpp Resolved
Comment thread src/mf/mf-backend.cpp Resolved
remibettan and others added 3 commits September 10, 2026 10:15
Cosmetic: `_warned_incomplete` was indented 8 spaces instead of 4, so it
visually sat inside the preceding `_incomplete_since` map declaration.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Mirror the Windows guard: a composite already in `_devices_data` stays
published even if `incomplete_devices()` flags it for a tick. Without
this, a transient sysfs miss (e.g. `/dev/videoN` symlink not visible
during an unrelated bind on the same composite) would surface as a
spurious removal followed by a re-add.

Also compute the wait as a `waited` local and only warn on subsequent
sightings, so the "still incomplete after Xs" log cannot ever print 0s.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Compute the wait as a `waited` local and skip the warn branch on the
first sighting of a composite. Behavior is unchanged at MAX_DEFERRAL=10s
today; a future edit that lowered the budget could otherwise make the
log line silently print 0s.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@remibettan
remibettan merged commit 37decd4 into realsenseai:development Sep 14, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants