diff --git a/.vscode/settings.json b/.vscode/settings.json index 29bb0ce..06ceb3f 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -89,6 +89,5 @@ "memory_resource": "cpp", "stop_token": "cpp", "__availability": "cpp" - }, - "files.autoSave": "afterDelay" + } } \ No newline at end of file diff --git a/src/libhidpp/hid/RawDevice_macos.cpp b/src/libhidpp/hid/RawDevice_macos.cpp index 620870f..5cd7959 100644 --- a/src/libhidpp/hid/RawDevice_macos.cpp +++ b/src/libhidpp/hid/RawDevice_macos.cpp @@ -39,55 +39,58 @@ using namespace HID; extern "C" { // This needs to be declared after `using namespace HID` for some reason. #include #include -#include -#include -#include } -// Linux pre-process defintions -// Copied from https://github.com/1kc/librazermacos/blob/2ca85a2bcdbf5e4b3938b95754d06b96de382bd9/src/include/razercommon.h#L12 -#define HID_REQ_GET_REPORT 0x01 -#define HID_REQ_SET_REPORT 0x09 - -#define USB_TYPE_CLASS (0x01 << 5) -#define USB_RECIP_INTERFACE 0x01 -#define USB_DIR_OUT 0 -#define USB_DIR_IN 0x80 - // Pimpl -struct RawDevice::PrivateImpl { +struct RawDevice::PrivateImpl +{ // Attributes IOHIDDeviceRef iohidDevice; - IOHIDDeviceInterface122 **deviceInterface; - CFIndex maxInputReportSize; CFIndex maxOutputReportSize; + dispatch_queue_t inputQueue; + static void nullifyValues(RawDevice *dev) { dev->_p->iohidDevice = nullptr; dev->_p->maxInputReportSize = 0; dev->_p->maxOutputReportSize = 0; + dev->_p->inputQueue = nullptr; dev->_p->initState(dev); } // State + CFRunLoopRef inputReportRunLoop; + std::vector lastInputReport; // Can't make this atomic, use explicit mutexes to protect this instead + std::atomic ignoreNextRead; - std::atomic waitingForInput; + std::atomic waitingForInput; + std::atomic lastInputReportTime; + std::atomic waitingForInputWasInterrupted; + std::atomic inputRunLoopIsRunning; static void initState(RawDevice *dev) { + dev->_p->inputReportRunLoop = nullptr; + dev->_p->lastInputReport = std::vector(); + dev->_p->ignoreNextRead = false; dev->_p->waitingForInput = false; + dev->_p->lastInputReportTime = -1; + dev->_p->waitingForInputWasInterrupted = false; + dev->_p->inputRunLoopIsRunning = false; } // Concurrency std::mutex mutexLock; + std::condition_variable shouldStopWaitingForInput; + std::condition_variable inputRunLoopStarted; // Dispatch queue config @@ -101,8 +104,178 @@ struct RawDevice::PrivateImpl { sprintf(queueLabel, "%s%s", prefix, debugID.c_str()); return std::string(queueLabel); } + + // Read input reports + // Read thread should be active throughout the lifetime of this RawDevice + + static void stopReadThread(RawDevice *dev) { + CFRunLoopStop(dev->_p->inputReportRunLoop); + } + static void readThread(RawDevice *dev) { + + // Does this: + // - Read input reports + // - Store result into lastInputReport + // - Send notifications that there is new report (via std::condition_variable) + // Discussion: + // - This function is blocking. The thread that calls it becomes the read thread until stopReadThread() is called from another thread. + + // Convenience + RawDevice::PrivateImpl *_p = dev->_p.get(); + + // Setup reportBuffer + CFIndex reportBufferSize = _p->maxInputReportSize; + uint8_t *reportBuffer = (uint8_t *) malloc(reportBufferSize * sizeof(uint8_t)); + memset(reportBuffer, 0, reportBufferSize); // Init with 0s + + // Setup report callback + // IOHIDDeviceGetReportWithCallback has a built-in timeout and might make for more straight forward code + // but Apple docs say it should only be used for feature reports. So we're using + // IOHIDDeviceRegisterInputReportCallback instead. + IOHIDDeviceRegisterInputReportCallback( + _p->iohidDevice, + reportBuffer, // This will get filled when an inputReport occurs + _p->maxInputReportSize, + [] (void *context, IOReturn result, void *sender, IOHIDReportType type, uint32_t reportID, uint8_t *report, CFIndex reportLength) { + + // Get dev from context + // ^ We can't capture `dev` or anything else, because then the enclosing lambda wouldn't decay to a pure c function + RawDevice *devvv = static_cast(context); + + // Lock + std::unique_lock lock(devvv->_p->mutexLock); + + // Debug + Log::debug() << "Received input from device " << Utility_macos::IOHIDDeviceGetDebugIdentifier(devvv->_p->iohidDevice) << std::endl; + + // Store new report + devvv->_p->lastInputReport.assign(report, report + reportLength); + devvv->_p->lastInputReportTime = Utility_macos::timestamp(); + + // Notify waiting thread + devvv->_p->shouldStopWaitingForInput.notify_one(); // We assume that there's only one waiting thread + + }, + dev // Pass `dev` to context + ); + + // Start runLoop + + // Params + + CFRunLoopMode runLoopMode = kCFRunLoopDefaultMode; + + // Store current runLoop + + _p->inputReportRunLoop = CFRunLoopGetCurrent(); + + // Add IOHIDDevice to runLoop. + + // Async callbacks for this IOHIDDevice will be delivered to this runLoop + // We need to call this before CFRunLoopRun, because if the runLoop has nothing to do, it'll immediately exit when we try to run it. + IOHIDDeviceScheduleWithRunLoop(_p->iohidDevice, _p->inputReportRunLoop, runLoopMode); + + // Debug + + Log::debug() << "Starting inputRunLoop on device " << Utility_macos::IOHIDDeviceGetDebugIdentifier(_p->iohidDevice) << std::endl; + + // Observer + + CFRunLoopObserverContext ctx = { + .version = 0, + .info = dev, + .retain = NULL, + .release = NULL, + .copyDescription = NULL, + }; + + CFRunLoopObserverRef observer = CFRunLoopObserverCreate( + kCFAllocatorDefault, + kCFRunLoopEntry | kCFRunLoopExit, + false, + 0, + [](CFRunLoopObserverRef observer, CFRunLoopActivity activity, void *info){ + RawDevice *devvv = (RawDevice *)info; + + if (activity == kCFRunLoopEntry) { + devvv->_p->inputRunLoopIsRunning = true; + devvv->_p->inputRunLoopStarted.notify_one(); + } else { + devvv->_p->inputRunLoopIsRunning = false; // Not sure if useful + } + }, + &ctx + ); + + CFRunLoopAddObserver(_p->inputReportRunLoop, observer, runLoopMode); + + // Start runLoop + // Calling this blocks this thread and until the runLoop exits. + // We only exit the runLoop if one of these happens + // - Device sends input report + // - Timeout happens + // - interruptRead() is called + // + // See HIDAPI https://github.com/libusb/hidapi/blob/master/mac/hid.c for reference + + while (true) { + + // Run runLoop + + CFRunLoopRunResult runLoopResult = CFRunLoopRunInMode(runLoopMode, 1000 /*sec*/, false); + + // Analyze runLoop exit reason + + std::string runLoopResultString; + if (runLoopResult == kCFRunLoopRunFinished) { + runLoopResultString = "Finished"; + } else if (runLoopResult == kCFRunLoopRunHandledSource) { + runLoopResultString = "HandledSource"; + } else if (runLoopResult == kCFRunLoopRunStopped) { + runLoopResultString = "Stopped"; + } else if (runLoopResult == kCFRunLoopRunTimedOut) { + runLoopResultString = "TimedOut"; + } else { + runLoopResultString = "UnknownResult"; + } + Log::debug() << "inputReportRunLoop exited with result: " << runLoopResultString << std::endl; + + // Exit condition + + if (runLoopResult == kCFRunLoopRunFinished // Not sure if this makes sense. I've never seen the result be `finished` + || runLoopResult == kCFRunLoopRunStopped) { + break; + } + + Log::debug() << "Restarting runloop" << std::endl; + } + + // Free reportBuffer + free(reportBuffer); + + // Tear down runLoop + // Edit: disabling for now since it accesses _p which can lead to crash when RunLoopStop() is called from the deconstructor + + return; + + Log::debug() << "Tearing down runloop" << std::endl; + + // Unregister input report callback + // This is probably unnecessary + uint8_t nullReportBuffer[0]; + CFIndex nullReportLength = 0; + IOHIDDeviceRegisterInputReportCallback(_p->iohidDevice, nullReportBuffer, nullReportLength, NULL, NULL); + // ^ Passing NULL for the callback unregisters the previous callback. + // Not sure if redundant when already calling IOHIDDeviceUnscheduleFromRunLoop. + + // Remove device from runLoop + // This is probably unnecessary since we're already stopping the runLoop via CFRunLoopStop() + IOHIDDeviceUnscheduleFromRunLoop(_p->iohidDevice, _p->inputReportRunLoop, kCFRunLoopCommonModes); + } + }; + // Private constructor RawDevice::RawDevice() @@ -134,6 +307,7 @@ RawDevice::RawDevice(const RawDevice &other) : _p(std::make_unique( // ^ Copy iohidDevice. I'm not sure this way of copying works _p->maxInputReportSize = other._p->maxInputReportSize; _p->maxOutputReportSize = other._p->maxOutputReportSize; + _p->inputQueue = dispatch_queue_create(_p->getInputReportQueueLabel(this).c_str(), _p->getInputReportQueueAttrs()); // Reset state _p->initState(this); @@ -157,6 +331,7 @@ RawDevice::RawDevice(RawDevice &&other) : _p(std::make_unique()), _p->iohidDevice = other._p->iohidDevice; _p->maxInputReportSize = other._p->maxInputReportSize; _p->maxOutputReportSize = other._p->maxOutputReportSize; + _p->inputQueue = other._p->inputQueue; // Init state _p->initState(this); @@ -176,6 +351,9 @@ RawDevice::~RawDevice(){ // Debug Log::debug() << "Destroying device " << Utility_macos::IOHIDDeviceGetDebugIdentifier(_p->iohidDevice) << std::endl; + // Stop inputQueue + _p->stopReadThread(this); + // Close IOHIDDevice IOHIDDeviceClose(_p->iohidDevice, kIOHIDOptionsTypeNone); // Not sure if necessary CFRelease(_p->iohidDevice); // Not sure if necessary @@ -245,30 +423,18 @@ RawDevice::RawDevice(const std::string &path) : _p(std::make_unique _p->maxInputReportSize = Utility_macos::IOHIDDeviceGetIntProperty(device, CFSTR(kIOHIDMaxInputReportSizeKey)); _p->maxOutputReportSize = Utility_macos::IOHIDDeviceGetIntProperty(device, CFSTR(kIOHIDMaxOutputReportSizeKey)); - // Get USB Interface + // Create dispatch queue + _p->inputQueue = dispatch_queue_create(_p->getInputReportQueueLabel(this).c_str(), _p->getInputReportQueueAttrs()); - IOCFPlugInInterface **pluginInterface = NULL; - SInt32 score = 0; - kr = IOCreatePlugInInterfaceForService(service, kIOHIDDeviceTypeID, kIOCFPlugInInterfaceID, &pluginInterface, &score); - if (kr != KERN_SUCCESS) { - Log::error() << "Failed to get plugin interface for device " << Utility_macos::IOHIDDeviceGetDebugIdentifier(_p->iohidDevice) << " with error code " << kr << std::endl; - - CFStringRef path = IORegistryEntryCopyPath(service, kIOServicePlane); - Log::debug() << "Service path: " << CFStringGetCStringPtr(path, kCFStringEncodingUTF8) << std::endl; - CFRelease(path); - } else { + // Start listening to input on queue + dispatch_async_f(_p->inputQueue, this, [](void *context) { + RawDevice *thisss = (RawDevice *)context; + thisss->_p->readThread(thisss); + }); - Log::error() << "Successfully obtained plugin interface for device " << Utility_macos::IOHIDDeviceGetDebugIdentifier(_p->iohidDevice) << std::endl; - - IOHIDDeviceInterface122 **deviceInterface = NULL; - HRESULT hResult = (*pluginInterface)->QueryInterface(pluginInterface, CFUUIDGetUUIDBytes(kIOHIDDeviceInterfaceID122), (LPVOID *)&deviceInterface); - if (hResult != 0 || deviceInterface == NULL) { - Log::error() << "Failed to get USB interface for device " << Utility_macos::IOHIDDeviceGetDebugIdentifier(_p->iohidDevice) << std::endl; - } - (*pluginInterface)->Release(pluginInterface); /// Not needed after getting USBInterface - - _p->deviceInterface = deviceInterface; - _p + // Wait until inputReportRunLoop Started + while (!_p->inputRunLoopIsRunning) { + _p->inputRunLoopStarted.wait(lock); } // Debug @@ -289,36 +455,21 @@ int RawDevice::writeReport(const std::vector &report) Log::debug() << "writeReport called on " << Utility_macos::IOHIDDeviceGetDebugIdentifier(_p->iohidDevice) << std::endl; - - if (_p->deviceInterface == NULL) { - return 1; - } - - // Guard report size if (report.size() > _p->maxOutputReportSize) { // TODO: Return meaningful error return 1; } + // Gather args for report sending + IOHIDDeviceRef device = _p->iohidDevice; + IOHIDReportType reportType = kIOHIDReportTypeOutput; + CFIndex reportID = report[0]; + const uint8_t *rawReport = report.data(); + CFIndex reportLength = report.size(); - // (*_p->deviceInterface)->setReport(_p->deviceInterface, kIOHIDReportTypeOutput, report[0], report.) - - // CFIndex reportID = report[0]; /// Should we use this as wIndex? Razer driver just uses 0 as wIndex as far as I can tell. - UInt16 requestLength = report.size(); - const uint8_t *requestData = report.data(); - - IOUSBDevRequest request = { // Copied from https://github.com/1kc/librazermacos/blob/2ca85a2bcdbf5e4b3938b95754d06b96de382bd9/src/lib/razercommon.c#L20 - .bmRequestType = USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_OUT, /// This is from razer driver, but docs say valid values are kUSBStandard, kUSBClass or kUSBVendor. - .bRequest = HID_REQ_SET_REPORT, - .wValue = 0x300, - .wIndex = 0x00, - .wLength = requestLength, // Is this bits or bytes? - .pData = (void *)requestData, - .wLenDone = 0, - }; - - IOReturn r = (*_p->deviceInterface)->DeviceRequest(_p->deviceInterface, &request); + // Send report + IOReturn r = IOHIDDeviceSetReport(device, reportType, reportID, rawReport, reportLength); // Return error code if (r != kIOReturnSuccess) { @@ -340,89 +491,104 @@ int RawDevice::readReport(std::vector &report, int timeout) { // Debug Log::debug() << "readReport called on " << Utility_macos::IOHIDDeviceGetDebugIdentifier(_p->iohidDevice) << std::endl; + // Define constant + double lookbackThreshold = 1 / 100.0; // If a report occured no more than `lookbackThreshold` seconds ago, then we use it. - if (_p->deviceInterface == NULL) { - return 0; + // Preprocess timeout + double timeoutSeconds; + if (timeout < 0) { + // Disable timeout if negative + timeoutSeconds = INFINITY; + } else { + // Convert timeout to seconds instead of milliseconds + timeoutSeconds = timeout / 1000.0; } + // Wait for input + // Block this thread until the next inputReport is issued + // We only stop waiting if one of these happens + // - Device sends input report + // - Timeout happens + // - interruptRead() is called + // + // Use _p->lastInputReport instead of waiting - if it's been less than "lookbackThreshold" since the lastInputReport occured + // The hidpp library expects this function to only receive inputReports that are issued after we start reading. But sometimes starting to read takes a fraction too long making us miss events. + // I think every mouse movement creates input reports - can that lead to problems? + + double lastInputReportTimeBeforeWaiting = _p->lastInputReportTime; + + if ((Utility_macos::timestamp() - lastInputReportTimeBeforeWaiting) <= lookbackThreshold) { + // Last received report is still fresh enough. Return that instead of waiting. + Log::debug() << "Recent event already queued up for device " << Utility_macos::IOHIDDeviceGetDebugIdentifier(_p->iohidDevice) << std::endl; + } else { + // Wait for next input report + + // Init loop state + std::cv_status timeoutStatus = std::cv_status::no_timeout; + auto timeoutTime = std::chrono::system_clock::now() + std::chrono::duration>(timeoutSeconds); // Point in time until which to wait for input + + // Wait for report in a loop + while (true) { + + // Debug + Log::debug() << "Wait for device " << Utility_macos::IOHIDDeviceGetDebugIdentifier(_p->iohidDevice) << std::endl; + + // Ignore read + // Theres a race condition if ignoreNextRead is set to true after this func checks for _p->ignoreNextRead (its set in interruptRead()) but before this func starts wait for input. But this is super unlikely and doesn't have bad consequences. + if (_p->ignoreNextRead) { + _p->ignoreNextRead = false; + return 0; + } + // Wait + _p->waitingForInput = true; // Should only be mutated right here. + timeoutStatus = _p->shouldStopWaitingForInput.wait_until(lock, timeoutTime); // Maybe we should use an extra lock here that just synchronizes readReport() and the readThread()? Might be faster and prevent deadlocks. (In case there are any deadlocks) + _p->waitingForInput = false; + + // Check state + bool newEventReceived = _p->lastInputReportTime > lastInputReportTimeBeforeWaiting; + bool timedOut = timeoutStatus == std::cv_status::timeout ? true : false; // ? Possible race condition if the wait is interrupted due to spurious wakeup but then the timeoutTime is exceeded before we reach here. But this is very unlikely and doesn't have bad consequences. + bool interrupted = _p->waitingForInputWasInterrupted; + + // Update state + _p->waitingForInputWasInterrupted = false; + + // Stop waiting + if (newEventReceived || timedOut || interrupted) { + + // Debug state + if (newEventReceived + timedOut + interrupted > 1) { + Log::warning() << "Waiting for inputReport was stopped with WEIRD state: newReport: " << newEventReceived << " timedOut: " << timedOut << " interrupted: " << interrupted << std::endl; + } else { + Log::debug() << "Waiting for inputReport stopped with state: newReport: " << newEventReceived << " timedOut: " << timedOut << " interrupted: " << interrupted << std::endl; + } + + // Break loop + break; + } + } + } - // Setup reportBuffer - UInt16 bufferSize = (UInt16)_p->maxInputReportSize; - uint8_t buffer[bufferSize]; - - // Setup request return - IOReturn rt; - UInt32 responseLength; - - // Send request and wait for response + // Get return values - if (timeout >= 0) { - - UInt32 t = (UInt32)timeout; - - IOUSBDevRequestTO request = { // Copied from https://github.com/1kc/librazermacos/blob/2ca85a2bcdbf5e4b3938b95754d06b96de382bd9/src/lib/razercommon.c#L20 - .bmRequestType = USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN, // This is from razer driver, but docs say valid values are kUSBStandard, kUSBClass or kUSBVendor. - .bRequest = HID_REQ_GET_REPORT, - .wValue = 0x300, - .wIndex = 0x00, - .wLength = bufferSize, // Is this in bits or bytes? - .pData = &buffer, - .wLenDone = 0, - .noDataTimeout = t, - .completionTimeout = UINT32_MAX, - }; + int returnValue; - if (_p->ignoreNextRead) { // Checking for `ignoreNextRead` as close as possible to the wait, to avoid race conditions - _p->ignoreNextRead = false; - return 0; - } - _p->waitingForInput = true; - rt = (*_p->deviceInterface)->DeviceRequestTO(_p->deviceInterface, &request); - _p->waitingForInput = false; - responseLength = request.wLenDone; - - } else { // No timeout - - IOUSBDevRequest request = { - .bmRequestType = USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN, // This is from razer driver, but docs say valid values are kUSBStandard, kUSBClass or kUSBVendor. - .bRequest = HID_REQ_GET_REPORT, - .wValue = 0x300, - .wIndex = 0x00, - .wLength = bufferSize, - .pData = &buffer, - .wLenDone = 0, - }; + if ((Utility_macos::timestamp() - _p->lastInputReportTime) <= lookbackThreshold) { // Reading was successful. Not sure if this is the best way to check if reading was successful. It might be more robust than using the `newEventReceived` flag. - if (_p->ignoreNextRead) { - _p->ignoreNextRead = false; - return 0; - } + // Write result to the `report` argument and return length + report = _p->lastInputReport; + returnValue = report.size(); - _p->waitingForInput = true; - rt = (*_p->deviceInterface)->DeviceRequest(_p->deviceInterface, &request); - _p->waitingForInput = false; - responseLength = request.wLenDone; + } else { // Reading was interrupted or timedOut + returnValue = 0; } - // Return + // Reset return values + // Not sure if necessary. Theoretically the `lookbackTreshold` should be enough + _p->lastInputReport = std::vector(); + _p->lastInputReportTime = 0; - if (rt == kIOReturnSuccess) { - - report.assign(buffer, buffer + responseLength); - return responseLength; - - } else if (rt == kIOReturnAborted) { - // Reading has been interrupted - return 0; - } else if (rt == kIOReturnNoDevice || rt == kIOReturnNotOpen) { - // This should never happen I think - Log::error() << "readReport failed with code" << rt << std::endl; - return 0; - } else { - // There are no other returns listed in the docs, so this should never happen - Log::error() << "readReport failed with unknown code" << rt << std::endl; - return 0; - } + // Return + return returnValue; } void RawDevice::interruptRead() { @@ -437,13 +603,10 @@ void RawDevice::interruptRead() { if (_p->waitingForInput) { - // Stop waiting - IOReturn rt = (*_p->deviceInterface)->USBDeviceAbortPipeZero(_p->deviceInterface); + // Stop readReport() from waiting, if it's waiting - /// Log errors - if (rt != kIOReturnSuccess) { - Log::error() << "interruptRead failed with code" << rt << std::endl; - } + _p->waitingForInputWasInterrupted = true; + _p->shouldStopWaitingForInput.notify_one(); } else { // If readReport() is not currently waiting, ignore the next call to readReport() // This is the expected behaviour according to https://github.com/cvuchener/hidpp/issues/17#issuecomment-896821785