From 0eb5ce5ef9ea5b0aa66f28ad69f2d29710bd519a Mon Sep 17 00:00:00 2001 From: Eran Date: Mon, 18 Sep 2023 17:57:14 +0300 Subject: [PATCH] win_event_device_watcher wait before notify 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. --- src/mf/mf-backend.cpp | 76 ++++++++++++++++++++----------------------- 1 file changed, 35 insertions(+), 41 deletions(-) diff --git a/src/mf/mf-backend.cpp b/src/mf/mf-backend.cpp index d7462f68cf..f78f58fc04 100644 --- a/src/mf/mf-backend.cpp +++ b/src/mf/mf-backend.cpp @@ -18,6 +18,7 @@ #include #include #include // std::tolower +#include namespace { @@ -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(); } @@ -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(); }); } @@ -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; @@ -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); @@ -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: { @@ -311,30 +327,8 @@ namespace librealsense if( p_hdr->dbch_devicetype != DBT_DEVTYP_DEVICEINTERFACE ) break; auto data = reinterpret_cast(GetWindowLongPtr(hWnd, GWLP_USERDATA)); - auto next = data->_last; - std::wstring temp = reinterpret_cast(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; }