Skip to content

Commit

Permalink
win_event_device_watcher wait before notify
Browse files Browse the repository at this point in the history 
Events on removal arrive rapidly and so calling the callback for each is
redundant but also wrong:

In testing, disconnection happened in two stages, triggering two events:

OLD group, before disconnection:
\\?\usb#vid_8086&pid_0b3a&mi_00#6&1e67ef5f&0&0000#{e5323777-f976-4f5b-9b55-b94699c46e44}\global
\\?\usb#vid_8086&pid_0b3a&mi_03#6&1e67ef5f&0&0003#{e5323777-f976-4f5b-9b55-b94699c46e44}\global

FIRST event after disconnection, NEW group:
\\?\usb#vid_8086&pid_0b3a&mi_03#6&1e67ef5f&0&0003#{e5323777-f976-4f5b-9b55-b94699c46e44}\global

OLD-NEW = removed
NEW-OLD = added

And operator-() is subtract_sets():
    for each item on LEFT side:
        add to result if none on RIGHT side is a superset of LEFT
In this case:
    NEW is subset of OLD -> OLD is superset of NEW
    OLD-NEW = 1 device removed
    NEW-OLD = 0 device added

SECOND event after disconnection, OLD=previous NEW:
\\?\usb#vid_8086&pid_0b3a&mi_03#6&1e67ef5f&0&0003#{e5323777-f976-4f5b-9b55-b94699c46e44}\global

In this case:
    NEW=empty
    OLD-NEW = 1 device removed (again!?)
    NEW-OLD = 0 device added

Solution is fairly simple, and was already done in udev device-watcher:
wait a short while and only notify of changes after a brief time elapses
post-event. Any new event refreshes the timer.
  • Loading branch information
@maloel
maloel committed Sep 18, 2023
1 parent 25a4d3c commit 0eb5ce5
Showing 1 changed file with 35 additions and 41 deletions.
76 changes: 35 additions & 41 deletions src/mf/mf-backend.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <Windows.h>
#include <dbt.h>
#include <cctype> // std::tolower
#include <rsutils/time/timer.h>

namespace {

Expand Down Expand Up @@ -184,10 +185,9 @@ namespace librealsense
{
public:
win_event_device_watcher(const backend * backend)
: _last( backend->query_uvc_devices(), backend->query_usb_devices(), backend->query_hid_devices() )
, _backend( backend )
{
_data._backend = backend;
_data._stopped = true;
_data._last = backend_device_group(backend->query_uvc_devices(), backend->query_usb_devices(), backend->query_hid_devices());
}
~win_event_device_watcher() { stop(); }

Expand All @@ -199,7 +199,7 @@ namespace librealsense
"Cannot start a running device_watcher" );
LOG_DEBUG( "starting win_event_device_watcher" );
_data._stopped = false;
_data._callback = std::move(callback);
_callback = std::move(callback);
_thread = std::thread([this]() { run(); });
}

Expand All @@ -222,13 +222,15 @@ namespace librealsense
private:
std::thread _thread;
std::mutex _m;
backend_device_group _last;
device_changed_callback _callback;
const backend * const _backend;

struct extra_data {
const backend * _backend;
backend_device_group _last;
device_changed_callback _callback;
rsutils::time::timer _timer{ std::chrono::milliseconds( 100 ) };

bool _stopped;
bool _stopped = true;
bool _changed = false;
HWND hWnd;
HDEVNOTIFY hdevnotifyHW, hdevnotifyUVC, hdevnotify_sensor, hdevnotifyUSB;
} _data;
Expand All @@ -254,11 +256,28 @@ namespace librealsense
{
if (PeekMessage(&msg, _data.hWnd, 0, 0, PM_REMOVE))
{
TranslateMessage(&msg);
DispatchMessage(&msg);
TranslateMessage( &msg );
DispatchMessage( &msg );
}
else
{
if( _data._changed && _data._timer.has_expired() )
{
platform::backend_device_group curr( _backend->query_uvc_devices(),
_backend->query_usb_devices(),
_backend->query_hid_devices() );
if( list_changed( _last.uvc_devices, curr.uvc_devices )
|| list_changed( _last.usb_devices, curr.usb_devices )
|| list_changed( _last.hid_devices, curr.hid_devices ) )
{
_callback( _last, curr );
_last = curr;
}
_data._changed = false;
}
// Yield CPU resources, as this is required for connect/disconnect events only
std::this_thread::sleep_for( std::chrono::milliseconds( 50 ) );
}
else // Yield CPU resources, as this is required for connect/disconnect events only
std::this_thread::sleep_for(std::chrono::milliseconds(50));
}

UnregisterDeviceNotification(_data.hdevnotifyHW);
Expand Down Expand Up @@ -297,11 +316,8 @@ namespace librealsense
break;
auto data = reinterpret_cast< extra_data * >(
GetWindowLongPtr( hWnd, GWLP_USERDATA ) );
backend_device_group next( data->_backend->query_uvc_devices(),
data->_backend->query_usb_devices(),
data->_backend->query_hid_devices() );
/*if (data->_last != next)*/ data->_callback( data->_last, next );
data->_last = next;
data->_changed = true;
data->_timer.start();
break;
}
case DBT_DEVICEREMOVECOMPLETE: {
Expand All @@ -311,30 +327,8 @@ namespace librealsense
if( p_hdr->dbch_devicetype != DBT_DEVTYP_DEVICEINTERFACE )
break;
auto data = reinterpret_cast<extra_data*>(GetWindowLongPtr(hWnd, GWLP_USERDATA));
auto next = data->_last;
std::wstring temp = reinterpret_cast<DEV_BROADCAST_DEVICEINTERFACE*>(lParam)->dbcc_name;
std::string path;
path.reserve(temp.length());
for (wchar_t ch : temp) {
if (ch != L'{') path.push_back(std::tolower(((char*)&ch)[0]));
else break;
}

next.uvc_devices.erase(std::remove_if(next.uvc_devices.begin(), next.uvc_devices.end(), [&path](const uvc_device_info& info)
{ return info.device_path.substr(0, info.device_path.find_first_of("{")) == path; }), next.uvc_devices.end());
// next.usb_devices.erase(std::remove_if(next.usb_devices.begin(), next.usb_devices.end(), [&path](const usb_device_info& info)
// { return info.device_path.substr(0, info.device_path.find_first_of("{")) == path; }), next.usb_devices.end());
next.usb_devices = data->_backend->query_usb_devices();
next.hid_devices.erase(std::remove_if(next.hid_devices.begin(), next.hid_devices.end(), [&path](const hid_device_info& info)
{
auto sub = info.device_path.substr(0, info.device_path.find_first_of("{"));
std::transform(sub.begin(), sub.end(), sub.begin(), ::tolower);
return sub == path;

}), next.hid_devices.end());

/*if (data->_last != next)*/ data->_callback(data->_last, next);
data->_last = next;
data->_changed = true;
data->_timer.start();
}
break;
}
Expand Down

0 comments on commit 0eb5ce5

Please sign in to comment.