From 091d7ceeba00279a40f62e467bed0591597ebade Mon Sep 17 00:00:00 2001 From: Hayodea Hakol Date: Tue, 14 Jan 2025 14:13:56 -0400 Subject: [PATCH] xcbXorg: Parse devSpec params, connect to Xorg displays This patch adds some nicely weighty code for connecting to X displays and managing those connections. Attaching devices will automatically connect to their required X display. Removing all devices dependent on a given X display connection will also disconnect from that Xdisplay. --- hcore/senseApis/senseApiManager.cpp | 4 +- include/user/senseApiDesc.h | 8 +- senseApis/xcbXorg/xcbXorg.cpp | 289 +++++++++++++++++++++++----- 3 files changed, 246 insertions(+), 55 deletions(-) diff --git a/hcore/senseApis/senseApiManager.cpp b/hcore/senseApis/senseApiManager.cpp index cbeaea5..7d95738 100644 --- a/hcore/senseApis/senseApiManager.cpp +++ b/hcore/senseApis/senseApiManager.cpp @@ -228,7 +228,7 @@ void SenseApiManager::attachSenseDevice(const device::SenseDeviceSpec& spec) std::string(__func__) + ": attachDeviceReq() is NULL for library '" + lib.libraryPath + "'"); } - lib.senseApiDesc.sal_mgmt_libOps.attachDeviceReq(&spec); + lib.senseApiDesc.sal_mgmt_libOps.attachDeviceReq(spec); } void SenseApiManager::detachSenseDevice(const device::SenseDeviceSpec& spec) @@ -247,7 +247,7 @@ void SenseApiManager::detachSenseDevice(const device::SenseDeviceSpec& spec) std::string(__func__) + ": detachDeviceReq() is NULL for library '" + lib.libraryPath + "'"); } - lib.senseApiDesc.sal_mgmt_libOps.detachDeviceReq(&spec); + lib.senseApiDesc.sal_mgmt_libOps.detachDeviceReq(spec); } void SenseApiManager::attachAllSenseDevicesFromSpecs(void) diff --git a/include/user/senseApiDesc.h b/include/user/senseApiDesc.h index 1a4a51b..a3987a5 100644 --- a/include/user/senseApiDesc.h +++ b/include/user/senseApiDesc.h @@ -13,8 +13,8 @@ namespace sense_api { */ typedef int (sal_mho_initializeRdyFn)(void); typedef int (sal_mho_finalizeRdyFn)(void); -typedef int (sal_mho_attachDeviceAckFn)(const device::SenseDeviceSpec *const desc); -typedef int (sal_mho_detachDeviceAckFn)(const device::SenseDeviceSpec *const desc); +typedef int (sal_mho_attachDeviceAckFn)(const device::SenseDeviceSpec &desc); +typedef int (sal_mho_detachDeviceAckFn)(const device::SenseDeviceSpec &desc); struct Sal_Mgmt_HkOps { @@ -30,8 +30,8 @@ struct Sal_Mgmt_HkOps typedef int (sal_mlo_initializeIndFn)(void); typedef int (sal_mlo_finalizeIndFn)(void); -typedef int (sal_mlo_attachDeviceReqFn)(const device::SenseDeviceSpec *const desc); -typedef int (sal_mlo_detachDeviceReqFn)(const device::SenseDeviceSpec *const desc); +typedef int (sal_mlo_attachDeviceReqFn)(const device::SenseDeviceSpec &desc); +typedef int (sal_mlo_detachDeviceReqFn)(const device::SenseDeviceSpec &desc); struct Sal_Mgmt_LibOps { diff --git a/senseApis/xcbXorg/xcbXorg.cpp b/senseApis/xcbXorg/xcbXorg.cpp index d183e7e..79841d3 100644 --- a/senseApis/xcbXorg/xcbXorg.cpp +++ b/senseApis/xcbXorg/xcbXorg.cpp @@ -3,34 +3,190 @@ #include #include #include +#include +#include +#include #include #include #include #include "xcbXorg.h" -class AttachedDevice { -public: - AttachedDevice(const hk::device::SenseDeviceSpec &spec) - : deviceSpec(spec) - {} +// Key for identifying unique X server connections +struct XcbConnection +{ + struct XConnectionIdentifier + { + int display; + int screen; - const hk::device::SenseDeviceSpec& getSpec() const { - return deviceSpec; + bool operator<(const XConnectionIdentifier& other) const + { + if (display != other.display) return display < other.display; + return screen < other.screen; + } + + std::string stringify() const + { + std::ostringstream os; + os << "display=" << display << ", screen=" << screen; + return os.str(); + } + }; + + XcbConnection(const XConnectionIdentifier& id) + : connection(nullptr, &xcb_disconnect) + , connectionIdentifier(id) + , refCount(0) + { + // Convert display number to X display string format (e.g., ":0") + std::string displayString = ":" + std::to_string(id.display); + int screenNum; + + connection.reset(xcb_connect(displayString.c_str(), &screenNum)); + if (xcb_connection_has_error(connection.get())) + { + throw std::runtime_error( + std::string(__func__) + ": Failed to connect to X server " + + connectionIdentifier.stringify()); + } + + // Verify we got the screen we asked for + if (screenNum != id.screen) + { + throw std::runtime_error( + std::string(__func__) + ": Connected to wrong screen. " + "Requested " + connectionIdentifier.stringify() + + " but got screen " + std::to_string(screenNum)); + } } -private: - hk::device::SenseDeviceSpec deviceSpec; -}; + // Delete copy/move operations - we'll manage instances through pointers + XcbConnection(const XcbConnection&) = delete; + XcbConnection& operator=(const XcbConnection&) = delete; + XcbConnection(XcbConnection&&) = delete; + XcbConnection& operator=(XcbConnection&&) = delete; -struct XcbConnectionInfo { std::unique_ptr connection; - int screenNumber; + XConnectionIdentifier connectionIdentifier; + std::atomic refCount; - XcbConnectionInfo() - : connection(nullptr, &xcb_disconnect), screenNumber(0) {} +public: + static std::map> + connections; + + static std::shared_ptr getOrCreateConnection( + const XConnectionIdentifier& id) + { + auto it = connections.find(id); + if (it != connections.end()) { + return it->second; + } + + auto conn = std::make_shared(id); + connections.emplace(id, conn); + return conn; + } }; -static XcbConnectionInfo xcbConn; +class AttachedDevice +{ +public: + struct WindowSelector + { + int display; + int screen; + struct + { + uint32_t id; + std::string name; + } window; + bool useWindowId; + + std::string stringify() const + { + std::ostringstream os; + os << "Display: " << display + << ", Screen: " << screen + << ", Window: " << (useWindowId ? + std::to_string(window.id) : + "\"" + window.name + "\""); + return os.str(); + } + }; + + AttachedDevice(const hk::device::SenseDeviceSpec &spec, + std::shared_ptr conn) + : deviceSpec(spec) + , connection(std::move(conn)) + { + windowSelector.display = getRequiredParamAsInt(spec, "display"); + windowSelector.screen = getRequiredParamAsInt(spec, "screen"); + parseWindowSelector(spec.deviceSelector, windowSelector); + } + + hk::device::SenseDeviceSpec deviceSpec; + WindowSelector windowSelector; + std::shared_ptr connection; + +public: + static int getRequiredParamAsInt( + const hk::device::SenseDeviceSpec& spec, + const std::string& paramName) + { + auto it = std::find_if( + spec.providerParams.begin(), + spec.providerParams.end(), + [¶mName](const auto& param) { + return param.first == paramName; + } + ); + + if (it == spec.providerParams.end()) + { + throw std::runtime_error( + "No " + paramName + " specified in provider params"); + } + + try { + return std::stoi(it->second); + } catch (const std::exception& e) { + throw std::runtime_error( + "Failed to parse '" + paramName + "' param value '" + + it->second + "' as integer: " + e.what()); + } + } + + static void parseWindowSelector( + const std::string& selector, + WindowSelector& windowSelector) + { + if (selector.length() >= 2 && + ((selector[0] == '"' && selector.back() == '"') || + (selector[0] == '\'' && selector.back() == '\'') || + (selector[0] == '`' && selector.back() == '`'))) + { + windowSelector.window.name = selector.substr( + 1, selector.length() - 2); + windowSelector.useWindowId = false; + } + else + { + try { + windowSelector.window.id = std::stoul(selector); + windowSelector.useWindowId = true; + } catch (const std::exception&) { + throw std::runtime_error( + "Window selector must be either a quoted string or a " + "numeric ID"); + } + } + } +}; + +// Define the static member +std::map> + XcbConnection::connections; + static std::vector attachedDevices; static hk::sense_api::sal_mlo_initializeIndFn xcbXorg_initializeInd; @@ -60,50 +216,85 @@ const hk::sense_api::SenseApiDesc &HK_GET_SENSE_API_DESC_FN_NAME(void) int xcbXorg_initializeInd(void) { - xcbConn.connection.reset( - xcb_connect(nullptr, &xcbConn.screenNumber)); - - if (xcb_connection_has_error(xcbConn.connection.get())) - { - throw std::runtime_error( - std::string(__func__) + ": Failed to connect to X server"); - } - - std::cout << __func__ << ": Connected to X server, screen number " - << xcbConn.screenNumber << "\n"; return 0; } int xcbXorg_finalizeInd(void) { - if (!xcbConn.connection) { + XcbConnection::connections.clear(); + return 0; +} + +int xcbXorg_attachDeviceReq(const hk::device::SenseDeviceSpec &desc) +{ + // Create temporary device to validate parameters + AttachedDevice::WindowSelector tempSelector; + + tempSelector.display = AttachedDevice::getRequiredParamAsInt( + desc, "display"); + tempSelector.screen = AttachedDevice::getRequiredParamAsInt(desc, "screen"); + + // Get or create connection before constructing device + XcbConnection::XConnectionIdentifier id{ + tempSelector.display, + tempSelector.screen + }; + auto conn = XcbConnection::getOrCreateConnection(id); + + // Create device with validated connection + attachedDevices.emplace_back(desc, conn); + conn->refCount++; + + std::cout << "Attaching X11 window:\n " + << attachedDevices.back().windowSelector.stringify() << "\n" + << " Using " << (conn->refCount > 1 ? "existing" : "new") + << " connection to X server\n"; + + return 0; +} + +int xcbXorg_detachDeviceReq(const hk::device::SenseDeviceSpec &spec) +{ + auto it = std::find_if(attachedDevices.begin(), attachedDevices.end(), + [&spec](const AttachedDevice &device) { + return device.deviceSpec == spec; + } + ); + + if (it == attachedDevices.end()) + { + auto displayIt = std::find_if( + spec.providerParams.begin(), spec.providerParams.end(), + [](const auto& param) { return param.first == "display"; }); + auto screenIt = std::find_if( + spec.providerParams.begin(), spec.providerParams.end(), + [](const auto& param) { return param.first == "screen"; }); + + std::cerr << __func__ << ": Device not attached: " + << "display=" << (displayIt != spec.providerParams.end() + ? displayIt->second : "not specified") + << ", screen=" << (screenIt != spec.providerParams.end() + ? screenIt->second : "not specified") + << ", selector=" << spec.deviceSelector << "\n"; return 0; } - xcbConn.connection.reset(); - std::cout << __func__ << ": Disconnected from X server\n"; - return 0; -} -int xcbXorg_attachDeviceReq(const hk::device::SenseDeviceSpec *const desc) -{ - attachedDevices.emplace_back(*desc); + XcbConnection::XConnectionIdentifier id{ + it->windowSelector.display, + it->windowSelector.screen + }; - std::ostringstream os; - for (const auto& device : attachedDevices) { - os << device.getSpec().stringify(); + // Atomic decrement refcount + int newCount = --it->connection->refCount; + + // If no more references, close the connection + if (newCount == 0) { + XcbConnection::connections.erase(id); + std::cout << "Closed X server connection (display=" + << id.display << ", screen=" << id.screen << ")\n"; } - std::cout << __func__ << ": >>>> Attaching device spec: " << desc->stringify() << "\n" - << " >>>> Current attached devices:\n" << os.str(); - return 0; -} -int xcbXorg_detachDeviceReq(const hk::device::SenseDeviceSpec *const spec) -{ - auto it = std::remove_if(attachedDevices.begin(), attachedDevices.end(), - [spec](const AttachedDevice &device) { - return device.getSpec() == *spec; - }); - attachedDevices.erase(it, attachedDevices.end()); - std::cout << __func__ << ": Detaching device spec\n"; + attachedDevices.erase(it); + std::cout << __func__ << ": Detached device\n"; return 0; }