From f2fd9678657b83947f4b1fe6fd50ae0a2d17e473 Mon Sep 17 00:00:00 2001 From: pixl Date: Sat, 29 Apr 2023 18:57:09 -0400 Subject: [PATCH] Fix receivers not seeing device connection events --- src/logid/Receiver.cpp | 16 ++- src/logid/Receiver.h | 4 +- src/logid/backend/hidpp/Device.cpp | 48 ++++---- src/logid/backend/hidpp10/Device.cpp | 3 +- src/logid/backend/hidpp10/Receiver.cpp | 3 +- src/logid/backend/hidpp10/ReceiverMonitor.cpp | 112 +++++++++--------- src/logid/backend/hidpp10/ReceiverMonitor.h | 2 +- 7 files changed, 90 insertions(+), 98 deletions(-) diff --git a/src/logid/Receiver.cpp b/src/logid/Receiver.cpp index 909f972..ba54a1f 100644 --- a/src/logid/Receiver.cpp +++ b/src/logid/Receiver.cpp @@ -64,10 +64,10 @@ Receiver::Receiver(const std::string& path, const std::shared_ptr& manager) : hidpp10::ReceiverMonitor(path, manager, manager->config()->io_timeout.value_or( - defaults::io_timeout)), + defaults::io_timeout)), _path(path), _manager(manager), _nickname(manager), _ipc_node(manager->receiversNode()->make_child(_nickname)), - _ipc_interface(_ipc_node->make_interface(this)) { + _ipc_interface(_ipc_node->make_interface(this)) { ready(); } @@ -114,9 +114,8 @@ void Receiver::addDevice(hidpp::DeviceConnectionEvent event) { if (!event.linkEstablished) return; - hidpp::Device hidpp_device( - receiver(), event, - manager->config()->io_timeout.value_or(defaults::io_timeout)); + hidpp::Device hidpp_device(receiver(), event, + manager->config()->io_timeout.value_or(defaults::io_timeout)); auto version = hidpp_device.version(); @@ -132,9 +131,8 @@ void Receiver::addDevice(hidpp::DeviceConnectionEvent event) { manager->addExternalDevice(device); } catch (hidpp10::Error& e) { - logPrintf(ERROR, - "Caught HID++ 1.0 error while trying to initialize " - "%s:%d: %s", _path.c_str(), event.index, e.what()); + logPrintf(ERROR, "Caught HID++ 1.0 error while trying to initialize %s:%d: %s", + _path.c_str(), event.index, e.what()); } catch (hidpp20::Error& e) { logPrintf(ERROR, "Caught HID++ 2.0 error while trying to initialize " "%s:%d: %s", _path.c_str(), event.index, e.what()); @@ -167,6 +165,6 @@ std::shared_ptr Receiver::rawReceiver() { return receiver(); } -Receiver::ReceiverIPC::ReceiverIPC(Receiver* receiver) : +Receiver::IPC::IPC(Receiver* receiver) : ipcgull::interface(SERVICE_ROOT_NAME ".Receiver", {}, {}, {}) { } diff --git a/src/logid/Receiver.h b/src/logid/Receiver.h index 14d8a96..019b9da 100644 --- a/src/logid/Receiver.h +++ b/src/logid/Receiver.h @@ -78,9 +78,9 @@ namespace logid { const ReceiverNickname _nickname; std::shared_ptr _ipc_node; - class ReceiverIPC : public ipcgull::interface { + class IPC : public ipcgull::interface { public: - explicit ReceiverIPC(Receiver* receiver); + explicit IPC(Receiver* receiver); }; std::shared_ptr _ipc_interface; diff --git a/src/logid/backend/hidpp/Device.cpp b/src/logid/backend/hidpp/Device.cpp index d90e3ae..38cd0e1 100644 --- a/src/logid/backend/hidpp/Device.cpp +++ b/src/logid/backend/hidpp/Device.cpp @@ -117,21 +117,19 @@ void Device::_init() { */ if (_index == hidpp::DefaultDevice) { _raw_handler = _raw_device->addEventHandler( - { - []( - const std::vector& report) -> bool { - return (report[Offset::Type] == Report::Type::Short || - report[Offset::Type] == Report::Type::Long); - }, - [this](const std::vector& report) -> void { - Report _report(report); - this->handleEvent(_report); - }}); + {[](const std::vector& report) -> bool { + return (report[Offset::Type] == Report::Type::Short || + report[Offset::Type] == Report::Type::Long); + }, + [this](const std::vector& report) -> void { + Report _report(report); + this->handleEvent(_report); + }}); try { auto rsp = sendReport({ReportType::Short, _index, - hidpp20::FeatureID::ROOT, hidpp20::Root::Ping, - hidpp::softwareID}); + hidpp20::FeatureID::ROOT, hidpp20::Root::Ping, + hidpp::softwareID}); if (rsp.deviceIndex() != _index) { throw InvalidDevice(InvalidDevice::VirtualNode); } @@ -146,18 +144,16 @@ void Device::_init() { } _raw_handler = _raw_device->addEventHandler( - { - - [index = this->_index]( - const std::vector& report) -> bool { - return (report[Offset::Type] == Report::Type::Short || - report[Offset::Type] == Report::Type::Long) && - (report[Offset::DeviceIndex] == index); - }, - [this](const std::vector& report) -> void { - Report _report(report); - this->handleEvent(_report); - }}); + {[index = this->_index]( + const std::vector& report) -> bool { + return (report[Offset::Type] == Report::Type::Short || + report[Offset::Type] == Report::Type::Long) && + (report[Offset::DeviceIndex] == index); + }, + [this](const std::vector& report) -> void { + Report _report(report); + this->handleEvent(_report); + }}); try { try { @@ -250,9 +246,9 @@ Report Device::sendReport(const Report& report) { if (std::holds_alternative(response)) { return std::get(response); - } else if(std::holds_alternative(response)) { + } else if (std::holds_alternative(response)) { throw hidpp10::Error(std::get(response).error_code); - } else if(std::holds_alternative(response)) { + } else if (std::holds_alternative(response)) { throw hidpp20::Error(std::get(response).error_code); } diff --git a/src/logid/backend/hidpp10/Device.cpp b/src/logid/backend/hidpp10/Device.cpp index 9aca401..38cf769 100644 --- a/src/logid/backend/hidpp10/Device.cpp +++ b/src/logid/backend/hidpp10/Device.cpp @@ -57,8 +57,7 @@ hidpp::Report Device::sendReport(const hidpp::Report& report) { response_slot.sub_id = report.subId(); _sendReport(report); - bool valid = _response_cv.wait_for( - lock, io_timeout, + bool valid = _response_cv.wait_for(lock, io_timeout, [&response_slot]() { return response_slot.response.has_value(); }); diff --git a/src/logid/backend/hidpp10/Receiver.cpp b/src/logid/backend/hidpp10/Receiver.cpp index 094af5b..6e2da12 100644 --- a/src/logid/backend/hidpp10/Receiver.cpp +++ b/src/logid/backend/hidpp10/Receiver.cpp @@ -175,8 +175,7 @@ std::string Receiver::getDeviceName(hidpp::DeviceIndex index) { return name; } -hidpp::DeviceIndex Receiver::deviceDisconnectionEvent(const hidpp::Report& -report) { +hidpp::DeviceIndex Receiver::deviceDisconnectionEvent(const hidpp::Report& report) { assert(report.subId() == DeviceDisconnection); return report.deviceIndex(); } diff --git a/src/logid/backend/hidpp10/ReceiverMonitor.cpp b/src/logid/backend/hidpp10/ReceiverMonitor.cpp index 6401ccc..d75adcc 100644 --- a/src/logid/backend/hidpp10/ReceiverMonitor.cpp +++ b/src/logid/backend/hidpp10/ReceiverMonitor.cpp @@ -34,46 +34,48 @@ ReceiverMonitor::ReceiverMonitor(const std::string& path, ReceiverMonitor::~ReceiverMonitor() { if (ev_handler.has_value()) - _receiver->removeEventHandler(ev_handler.value()); + _receiver->rawDevice()->removeEventHandler(ev_handler.value()); } void ReceiverMonitor::ready() { if (!ev_handler.has_value()) { hidpp::EventHandler event_handler; - event_handler.condition = [](hidpp::Report& report) -> bool { - return (report.subId() == Receiver::DeviceConnection || - report.subId() == Receiver::DeviceDisconnection); - }; + ev_handler = _receiver->rawDevice()->addEventHandler( + {[](const std::vector& report) -> bool { + if (report[Offset::Type] == Report::Type::Short || + report[Offset::Type] == Report::Type::Long) { + uint8_t sub_id = report[Offset::SubID]; + return (sub_id == Receiver::DeviceConnection || + sub_id == Receiver::DeviceDisconnection); + } + return false; + }, [this](const std::vector& raw) -> void { + /* Running in a new thread prevents deadlocks since the + * receiver may be enumerating. + */ + hidpp::Report report(raw); - event_handler.callback = [this](hidpp::Report& report) -> void { - /* Running in a new thread prevents deadlocks since the - * receiver may be enumerating. - */ - spawn_task([this, report, - path = this->_receiver->rawDevice()->rawPath()]() { - if (report.subId() == Receiver::DeviceConnection) { - try { - this->addDevice(this->_receiver->deviceConnectionEvent - (report)); - } catch (std::exception& e) { - logPrintf(ERROR, "Failed to add device %d to receiver " - "on %s: %s", report.deviceIndex(), - path.c_str(), e.what()); - } - } else if (report.subId() == Receiver::DeviceDisconnection) { - try { - this->removeDevice(this->_receiver-> - deviceDisconnectionEvent(report)); - } catch (std::exception& e) { - logPrintf(ERROR, "Failed to remove device %d from " - "receiver on %s: %s", report.deviceIndex(), - path.c_str(), e.what()); - } + spawn_task([this, report, path = this->_receiver->rawDevice()->rawPath()]() { + if (report.subId() == Receiver::DeviceConnection) { + try { + this->addDevice(this->_receiver->deviceConnectionEvent(report)); + } catch (std::exception& e) { + logPrintf(ERROR, "Failed to add device %d to receiver on %s: %s", + report.deviceIndex(), path.c_str(), e.what()); + } + } else if (report.subId() == Receiver::DeviceDisconnection) { + try { + this->removeDevice( + this->_receiver->deviceDisconnectionEvent(report)); + } catch (std::exception& e) { + logPrintf(ERROR, "Failed to remove device %d from " + "receiver on %s: %s", report.deviceIndex(), + path.c_str(), e.what()); + } + } + }); } - }); - }; - - ev_handler = _receiver->addEventHandler(event_handler); + }); } enumerate(); @@ -87,30 +89,28 @@ void ReceiverMonitor::waitForDevice(hidpp::DeviceIndex index) { auto handler_id = std::make_shared(); *handler_id = _receiver->rawDevice()->addEventHandler( - { - [index](const std::vector& report) -> bool { - return report[Offset::DeviceIndex] == index; - }, - [this, index, handler_id]( - [[maybe_unused]] const std::vector& report) { - hidpp::DeviceConnectionEvent event{}; - event.withPayload = false; - event.linkEstablished = true; - event.index = index; - event.fromTimeoutCheck = true; + {[index](const std::vector& report) -> bool { + return report[Offset::DeviceIndex] == index; + }, + [this, index, handler_id]([[maybe_unused]] const std::vector& report) { + hidpp::DeviceConnectionEvent event{}; + event.withPayload = false; + event.linkEstablished = true; + event.index = index; + event.fromTimeoutCheck = true; - spawn_task([this, event, handler_id]() { - assert(handler_id); - try { - _receiver->rawDevice()->removeEventHandler(*handler_id); - addDevice(event); - } catch (std::exception& e) { - logPrintf(ERROR, "Failed to add device %d to receiver on %s: %s", - event.index, _receiver->rawDevice()->rawPath().c_str(), - e.what()); - } - }); - } + spawn_task([this, event, handler_id]() { + assert(handler_id); + try { + _receiver->rawDevice()->removeEventHandler(*handler_id); + addDevice(event); + } catch (std::exception& e) { + logPrintf(ERROR, "Failed to add device %d to receiver on %s: %s", + event.index, _receiver->rawDevice()->rawPath().c_str(), + e.what()); + } + }); + } }); } diff --git a/src/logid/backend/hidpp10/ReceiverMonitor.h b/src/logid/backend/hidpp10/ReceiverMonitor.h index c147e80..7fdb80b 100644 --- a/src/logid/backend/hidpp10/ReceiverMonitor.h +++ b/src/logid/backend/hidpp10/ReceiverMonitor.h @@ -57,7 +57,7 @@ namespace logid::backend::hidpp10 { private: std::shared_ptr _receiver; - std::optional ev_handler; + std::optional ev_handler; }; }