FIFO mode option for HID OutputReports

CMakeLists.txt

@@ -2935,6 +2935,7 @@ if(HID)
   target_sources(mixxx-lib PRIVATE
     src/controllers/hid/hidcontroller.cpp
     src/controllers/hid/hidiothread.cpp
+    src/controllers/hid/hidioglobaloutputreportfifo.cpp
     src/controllers/hid/hidiooutputreport.cpp
     src/controllers/hid/hiddevice.cpp
     src/controllers/hid/hidenumerator.cpp

src/controllers/hid/hidcontroller.h

@@ -71,32 +71,64 @@ class HidControllerJSProxy : public ControllerJSProxy {
     /// @param dataList Data to send as list of bytes
     /// @param length Unused but mandatory argument
     /// @param reportID 1...255 for HID devices that uses ReportIDs - or 0 for devices, which don't use ReportIDs
-    /// @param resendUnchangedReport If set, the report will also be send, if the data are unchanged since last sending
+    /// @param useNonSkippingFIFO (optional)
+    ///  - False (default):
+    ///    - Reports with identical data will be sent only once.
+    ///    - If reports were superseded by newer data before they could be sent,
+    ///      the oudated data will be skipped.
+    ///    - This mode works for all USB HID class compatible reports,
+    ///      in these each field represents the state of a control (e.g. an LED).
+    ///    - This mode works best in overload situations, where more reports
+    ///      are to be sent, than can be processed.
+    ///  - True:
+    ///    - The report will not be skipped under any circumstances,
+    ///      except FIFO memory overflow.
+    ///    - All reports with useNonSkippingFIFO set True will be send before
+    ///      any cached report with useNonSkippingFIFO set False.
+    ///    - All reports with useNonSkippingFIFO set True will be send in
+    ///      strict First In / First Out (FIFO) order.
+    ///    - Limit the use of this mode to the places, where it is really necessary.
     Q_INVOKABLE void send(const QList<int>& dataList,
             unsigned int length,
             quint8 reportID,
-            bool resendUnchangedReport = false) {
+            bool useNonSkippingFIFO = false) {
         Q_UNUSED(length);
         QByteArray dataArray;
         dataArray.reserve(dataList.size());
         for (int datum : dataList) {
             dataArray.append(datum);
         }
-        sendOutputReport(reportID, dataArray, resendUnchangedReport);
+        sendOutputReport(reportID, dataArray, useNonSkippingFIFO);
     }
 
     /// @brief Sends an OutputReport to HID device
     /// @param reportID 1...255 for HID devices that uses ReportIDs - or 0 for devices, which don't use ReportIDs
     /// @param dataArray Data to send as byte array (Javascript type Uint8Array)
-    /// @param resendUnchangedReport If set, the report will also be send, if the data are unchanged since last sending
+    /// @param useNonSkippingFIFO (optional)
+    ///  - False (default):
+    ///    - Reports with identical data will be sent only once.
+    ///    - If reports were superseded by newer data before they could be sent,
+    ///      the oudated data will be skipped.
+    ///    - This mode works for all USB HID class compatible reports,
+    ///      in these each field represents the state of a control (e.g. an LED).
+    ///    - This mode works best in overload situations, where more reports
+    ///      are to be sent, than can be processed.
+    ///  - True:
+    ///    - The report will not be skipped under any circumstances,
+    ///      except FIFO memory overflow.
+    ///    - All reports with useNonSkippingFIFO set True will be send before
+    ///      any cached report with useNonSkippingFIFO set False.
+    ///    - All reports with useNonSkippingFIFO set True will be send in
+    ///      strict First In / First Out (FIFO) order.
+    ///    - Limit the use of this mode to the places, where it is really necessary.
     Q_INVOKABLE void sendOutputReport(quint8 reportID,
             const QByteArray& dataArray,
-            bool resendUnchangedReport = false) {
+            bool useNonSkippingFIFO = false) {
         VERIFY_OR_DEBUG_ASSERT(m_pHidController->m_pHidIoThread) {
             return;
         }
         m_pHidController->m_pHidIoThread->updateCachedOutputReportData(
-                reportID, dataArray, resendUnchangedReport);
+                reportID, dataArray, useNonSkippingFIFO);
     }
 
     /// @brief getInputReport receives an InputReport from the HID device on request.

src/controllers/hid/hidioglobaloutputreportfifo.cpp

@@ -0,0 +1,97 @@
+#include "controllers/hid/hidioglobaloutputreportfifo.h"
+
+#include <hidapi.h>
+
+#include "controllers/defs_controllers.h"
+#include "controllers/hid/legacyhidcontrollermappingfilehandler.h"
+#include "util/compatibility/qbytearray.h"
+#include "util/compatibility/qmutex.h"
+#include "util/string.h"
+#include "util/time.h"
+#include "util/trace.h"
+
+namespace {
+constexpr int kMaxHidErrorMessageSize = 512;
+constexpr int kSizeOfFifoInReports = 32;
+} // namespace
+
+HidIoGlobalOutputReportFifo::HidIoGlobalOutputReportFifo()
+        : m_fifoQueue(kSizeOfFifoInReports) {
+}
+
+void HidIoGlobalOutputReportFifo::addReportDatasetToFifo(const quint8 reportId,
+        const QByteArray& reportData,
+        const mixxx::hid::DeviceInfo& deviceInfo,
+        const RuntimeLoggingCategory& logOutput) {
+    // First byte must always be the ReportID-Byte
+    QByteArray report(std::move(reportData));
+    report.prepend(reportId);
+
+    // Swap report to lockless FIFO queue
+    bool success = m_fifoQueue.try_emplace(std::move(report));
+
+    // Handle the case, that the FIFO queue is full - which is an error case
+    if (!success) {
+        // If the FIFO is full, we skip the report dataset even
+        // in non-skipping mode, to keep the controller mapping thread
+        // responsive for InputReports from the controller.
+        // Alternative would be to block processing of the controller
+        // mapping thread, until the FIFO has space again.
+        qCWarning(logOutput)
+                << "FIFO overflow: Unable to add OutputReport " << reportId
+                << "to the global cache for non-skipping sending of OututReports for"
+                << deviceInfo.formatName();
+    }
+}
+
+bool HidIoGlobalOutputReportFifo::sendNextReportDataset(QMutex* pHidDeviceAndPollMutex,
+        hid_device* pHidDevice,
+        const mixxx::hid::DeviceInfo& deviceInfo,
+        const RuntimeLoggingCategory& logOutput) {
+    auto startOfHidWrite = mixxx::Time::elapsed();
+
+    auto pFront = m_fifoQueue.front();
+
+    if (pFront == nullptr) {
+        // No data in FIFO to be send
+        // Return with false, to signal the caller, that no time consuming IO
+        // operation was ncessary
+        return false;
+    }
+
+    auto fifoUsedEntries = m_fifoQueue.size();
+
+    QByteArray dataToSend(std::move(*pFront));
+    m_fifoQueue.pop();
+
+    auto hidDeviceLock = lockMutex(pHidDeviceAndPollMutex);
+
+    // hid_write can take several milliseconds, because hidapi synchronizes
+    // the asyncron HID communication from the OS
+    int result = hid_write(pHidDevice,
+            reinterpret_cast<const unsigned char*>(dataToSend.constData()),
+            dataToSend.size());
+    if (result == -1) {
+        qCWarning(logOutput) << "Unable to send data to" << deviceInfo.formatName() << ":"
+                             << mixxx::convertWCStringToQString(
+                                        hid_error(pHidDevice),
+                                        kMaxHidErrorMessageSize);
+    }
+
+    hidDeviceLock.unlock();
+
+    if (result != -1) {
+        qCDebug(logOutput) << "t:" << startOfHidWrite.formatMillisWithUnit()
+                           << " " << result << "bytes (including ReportID of"
+                           << static_cast<quint8>(dataToSend[0])
+                           << ") sent from non-skipping FIFO ("
+                           << fifoUsedEntries << "/" << kSizeOfFifoInReports
+                           << "used) - Needed: "
+                           << (mixxx::Time::elapsed() - startOfHidWrite)
+                                      .formatMicrosWithUnit();
+    }
+
+    // Return with true, to signal the caller, that the time consuming hid_write
+    // operation was executed
+    return true;
+}

src/controllers/hid/hidioglobaloutputreportfifo.h

@@ -0,0 +1,31 @@
+#pragma once
+
+#include "controllers/controller.h"
+#include "controllers/hid/hiddevice.h"
+#include "rigtorp/SPSCQueue.h"
+#include "util/duration.h"
+
+/// Stores and sends OutputReports (independent of the ReportID) in First In /
+/// First Out (FIFO) order
+class HidIoGlobalOutputReportFifo {
+  public:
+    HidIoGlobalOutputReportFifo();
+
+    /// Caches new OutputReport to the FIFO, which will later be send by the IO thread
+    void addReportDatasetToFifo(const quint8 reportId,
+            const QByteArray& reportData,
+            const mixxx::hid::DeviceInfo& deviceInfo,
+            const RuntimeLoggingCategory& logOutput);
+
+    /// Sends the next OutputReport from FIFO to the HID device,
+    /// when if any report is cached in FIFO.
+    /// Returns true if a time consuming hid_write operation was executed.
+    bool sendNextReportDataset(QMutex* pHidDeviceAndPollMutex,
+            hid_device* pHidDevice,
+            const mixxx::hid::DeviceInfo& deviceInfo,
+            const RuntimeLoggingCategory& logOutput);
+
+  private:
+    // Lockless FIFO queue
+    rigtorp::SPSCQueue<QByteArray> m_fifoQueue;
+};

src/controllers/hid/hidiooutputreport.cpp

@@ -29,35 +29,41 @@ HidIoOutputReport::HidIoOutputReport(
 void HidIoOutputReport::updateCachedData(const QByteArray& data,
         const mixxx::hid::DeviceInfo& deviceInfo,
         const RuntimeLoggingCategory& logOutput,
-        bool resendUnchangedReport) {
+        HidIoGlobalOutputReportFifo* pGlobalOutputReportFifo,
+        bool useNonSkippingFIFO) {
     auto cacheLock = lockMutex(&m_cachedDataMutex);
 
     if (!m_lastCachedDataSize) {
         // First call updateCachedData for this report
         m_lastCachedDataSize = data.size();
 
     } else {
-        if (m_possiblyUnsentDataCached) {
+        if (m_possiblyUnsentDataCached && !useNonSkippingFIFO) {
             qCDebug(logOutput) << "t:" << mixxx::Time::elapsed().formatMillisWithUnit()
-                               << "Skipped superseded OutputReport"
-                               << deviceInfo.formatName() << "serial #"
-                               << deviceInfo.serialNumber() << "(Report ID"
-                               << m_reportId << ")";
+                               << "skipped superseded OutputReport data for ReportID"
+                               << m_reportId;
         }
 
         // The size of an HID report is defined in a HID device and can't vary at runtime
         if (m_lastCachedDataSize != data.size()) {
-            qCWarning(logOutput) << "Size of report (with Report ID"
-                                 << m_reportId << ") changed from"
-                                 << m_lastCachedDataSize
-                                 << "to" << data.size() << "for"
-                                 << deviceInfo.formatName() << "serial #"
-                                 << deviceInfo.serialNumber()
-                                 << "- This indicates a bug in the mapping code!";
+            qCWarning(logOutput)
+                    << "Size of OutputReport ( with ReportID" << m_reportId
+                    << ") changed from" << m_lastCachedDataSize << "to"
+                    << data.size()
+                    << "- This indicates a bug in the mapping code!";
             m_lastCachedDataSize = data.size();
         }
     }
 
+    // m_possiblyUnsentDataCached must be set while m_cachedDataMutex is locked
+    // This step covers the case that data for the report are cached in skipping mode,
+    // succeed by a non-skipping send of the same report
+    if (useNonSkippingFIFO) {
+        m_possiblyUnsentDataCached = false;
+        m_lastSentData.clear();
+        return;
+    }
+
     // Deep copy with reusing the already allocated heap memory
     // The first byte with the ReportID is not overwritten
     qByteArrayReplaceWithPositionAndSize(&m_cachedData,
@@ -66,7 +72,6 @@ void HidIoOutputReport::updateCachedData(const QByteArray& data,
             data.constData(),
             data.size());
     m_possiblyUnsentDataCached = true;
-    m_resendUnchangedReport = resendUnchangedReport;
 }
 
 bool HidIoOutputReport::sendCachedData(QMutex* pHidDeviceAndPollMutex,
@@ -82,7 +87,7 @@ bool HidIoOutputReport::sendCachedData(QMutex* pHidDeviceAndPollMutex,
         return false;
     }
 
-    if (!(m_resendUnchangedReport || m_lastSentData.compare(m_cachedData))) {
+    if (!m_lastSentData.compare(m_cachedData)) {
         // An HID OutputReport can contain only HID OutputItems.
         // HID OutputItems are defined to represent the state of one or more similar controls or LEDs.
         // Only HID Feature items may be attributes of other items.
@@ -96,10 +101,8 @@ bool HidIoOutputReport::sendCachedData(QMutex* pHidDeviceAndPollMutex,
         cacheLock.unlock();
 
         qCDebug(logOutput) << "t:" << startOfHidWrite.formatMillisWithUnit()
-                           << " Skipped identical Output Report for"
-                           << deviceInfo.formatName() << "serial #"
-                           << deviceInfo.serialNumber() << "(Report ID"
-                           << m_reportId << ")";
+                           << "Skipped sending identical OutputReport data from cache for ReportID"
+                           << m_reportId;
 
         // Return with false, to signal the caller, that no time consuming IO operation was necessary
         return false;
@@ -123,7 +126,7 @@ bool HidIoOutputReport::sendCachedData(QMutex* pHidDeviceAndPollMutex,
             reinterpret_cast<const unsigned char*>(m_lastSentData.constData()),
             m_lastSentData.size());
     if (result == -1) {
-        qCWarning(logOutput) << "Unable to send data to" << deviceInfo.formatName() << ":"
+        qCWarning(logOutput) << "Unable to send data to device :"
                              << mixxx::convertWCStringToQString(
                                         hid_error(pHidDevice),
                                         kMaxHidErrorMessageSize);
@@ -147,9 +150,9 @@ bool HidIoOutputReport::sendCachedData(QMutex* pHidDeviceAndPollMutex,
     }
 
     qCDebug(logOutput) << "t:" << startOfHidWrite.formatMillisWithUnit() << " "
-                       << result << "bytes sent to" << deviceInfo.formatName()
-                       << "serial #" << deviceInfo.serialNumber()
-                       << "(including report ID of" << m_reportId << ") - Needed: "
+                       << result << "bytes ( including ReportID of"
+                       << static_cast<quint8>(m_reportId)
+                       << ") sent from skipping cache - Needed:"
                        << (mixxx::Time::elapsed() - startOfHidWrite).formatMicrosWithUnit();
 
     // Return with true, to signal the caller, that the time consuming hid_write operation was executed

src/controllers/hid/hidiooutputreport.h

@@ -2,6 +2,7 @@
 
 #include "controllers/controller.h"
 #include "controllers/hid/hiddevice.h"
+#include "controllers/hid/hidioglobaloutputreportfifo.h"
 #include "util/compatibility/qmutex.h"
 #include "util/duration.h"
 
@@ -11,10 +12,10 @@ class HidIoOutputReport {
 
     /// Caches new report data, which will later send by the IO thread
     void updateCachedData(const QByteArray& data,
-
             const mixxx::hid::DeviceInfo& deviceInfo,
             const RuntimeLoggingCategory& logOutput,
-            bool resendUnchangedReport);
+            HidIoGlobalOutputReportFifo* pGlobalOutputReportFifo,
+            bool useNonSkippingFIFO);
 
     /// Sends the OutputReport to the HID device, when changed data are cached.
     /// Returns true if a time consuming hid_write operation was executed.
@@ -28,12 +29,11 @@ class HidIoOutputReport {
     QByteArray m_lastSentData;
 
     /// Mutex must be locked when reading/writing m_cachedData
-    /// or m_possiblyUnsentDataCached, m_resendUnchangedReport
+    /// or m_possiblyUnsentDataCached
     QMutex m_cachedDataMutex;
 
     QByteArray m_cachedData;
     bool m_possiblyUnsentDataCached;
-    bool m_resendUnchangedReport;
 
     /// Due to swapping of the QbyteArrays, we need to store
     /// this information independent of the QBytearray size

src/controllers/hid/hidiothread.cpp

@@ -43,6 +43,7 @@ HidIoThread::HidIoThread(
           m_pHidDevice(pHidDevice),
           m_lastPollSize(0),
           m_pollingBufferIndex(0),
+          m_globalOutputReportFifo(),
           m_runLoopSemaphore(1) {
     // Initializing isn't strictly necessary but is good practice.
     for (int i = 0; i < kNumBuffers; i++) {
@@ -188,7 +189,7 @@ QByteArray HidIoThread::getInputReport(quint8 reportID) {
 
 void HidIoThread::updateCachedOutputReportData(quint8 reportID,
         const QByteArray& data,
-        bool resendUnchangedReport) {
+        bool useNonSkippingFIFO) {
     auto mapLock = lockMutex(&m_outputReportMapMutex);
     if (m_outputReports.find(reportID) == m_outputReports.end()) {
         std::unique_ptr<HidIoOutputReport> pNewOutputReport;
@@ -204,11 +205,30 @@ void HidIoThread::updateCachedOutputReportData(quint8 reportID,
 
     mapLock.unlock();
 
+    // If useNonSkippingFIFO is false, the report data are cached here
+    // If useNonSkippingFIFO is true, this cache is cleared
     actualOutputReportIterator->second->updateCachedData(
-            data, m_deviceInfo, m_logOutput, resendUnchangedReport);
+            data, m_deviceInfo, m_logOutput, &m_globalOutputReportFifo, useNonSkippingFIFO);
+
+    // If useNonSkippingFIFO is true, put the new report dataset on the FIFO
+    if (useNonSkippingFIFO) {
+        m_globalOutputReportFifo.addReportDatasetToFifo(reportID, data, m_deviceInfo, m_logOutput);
+    }
 }
 
 bool HidIoThread::sendNextCachedOutputReport() {
+    // 1.) Send non-skipping reports from FIFO
+    if (m_globalOutputReportFifo.sendNextReportDataset(&m_hidDeviceAndPollMutex,
+                m_pHidDevice,
+                m_deviceInfo,
+                m_logOutput)) {
+        // Return after each time consuming sendCachedData
+        return true;
+    }
+
+    // 2.) If non non-skipping reports were in the FIFO, send the skipable reports
+    // from the m_outputReports cache
+
     // m_outputReports.size() doesn't need mutex protection, because the value of i is not used.
     // i is just a counter to prevent infinite loop execution.
     // If the map size increases, this loop will execute one iteration more,