Merge pull request #2970 from Be-ing/hid_polling

HidController: loop until no more messages are available on poll

4 files changed, 51 insertions, 15 deletions

diff --git a/src/controllers/controllermanager.cpp b/src/controllers/controllermanager.cpp
index a232fbfa29..b6ab1f485e 100644
--- a/src/controllers/controllermanager.cpp
+++ b/src/controllers/controllermanager.cpp
@@ -27,18 +27,18 @@
 #include "controllers/bulk/bulkenumerator.h"
 #endif
 
-namespace {
 // http://developer.qt.nokia.com/wiki/Threads_Events_QObjects
 
 // Poll every 1ms (where possible) for good controller response
 #ifdef __LINUX__
 // Many Linux distros ship with the system tick set to 250Hz so 1ms timer
 // reportedly causes CPU hosage. See Bug #990992 rryan 6/2012
-const int kPollIntervalMillis = 5;
+const mixxx::Duration ControllerManager::kPollInterval = mixxx::Duration::fromMillis(5);
 #else
-const int kPollIntervalMillis = 1;
+const mixxx::Duration ControllerManager::kPollInterval = mixxx::Duration::fromMillis(1);
 #endif
 
+namespace {
 /// Strip slashes and spaces from device name, so that it can be used as config
 /// key or a filename.
 QString sanitizeDeviceName(QString name) {
@@ -97,7 +97,7 @@ ControllerManager::ControllerManager(UserSettingsPointer pConfig)
         QDir().mkpath(userPresets);
     }
 
-    m_pollTimer.setInterval(kPollIntervalMillis);
+    m_pollTimer.setInterval(kPollInterval.toIntegerMillis());
     connect(&m_pollTimer, SIGNAL(timeout()),
             this, SLOT(pollDevices()));
 
@@ -359,7 +359,7 @@ void ControllerManager::pollDevices() {
     }
 
     mixxx::Duration duration = mixxx::Time::elapsed() - start;
-    if (duration > mixxx::Duration::fromMillis(kPollIntervalMillis)) {
+    if (duration > kPollInterval) {
         m_skipPoll = true;
     }
     //qDebug() << "ControllerManager::pollDevices()" << duration << start;
diff --git a/src/controllers/controllermanager.h b/src/controllers/controllermanager.h
index 8837d90358..fba7f70462 100644
--- a/src/controllers/controllermanager.h
+++ b/src/controllers/controllermanager.h
@@ -30,6 +30,8 @@ class ControllerManager : public QObject {
     ControllerManager(UserSettingsPointer pConfig);
     virtual ~ControllerManager();
 
+    static const mixxx::Duration kPollInterval;
+
     QList<Controller*> getControllers() const;
     QList<Controller*> getControllerList(bool outputDevices=true, bool inputDevices=true);
     ControllerLearningEventFilter* getControllerLearningEventFilter() const;
diff --git a/src/controllers/hid/hidcontroller.cpp b/src/controllers/hid/hidcontroller.cpp
index 08bff7ef12..1b1ca2bec5 100644
--- a/src/controllers/hid/hidcontroller.cpp
+++ b/src/controllers/hid/hidcontroller.cpp
@@ -18,7 +18,8 @@
 
 HidController::HidController(const hid_device_info& deviceInfo, UserSettingsPointer pConfig)
         : Controller(pConfig),
-          m_pHidDevice(NULL) {
+          m_pHidDevice(nullptr),
+          m_iPollingBufferIndex(0) {
     // Copy required variables from deviceInfo, which will be freed after
     // this class is initialized by caller.
     hid_vendor_id = deviceInfo.vendor_id;
@@ -215,6 +216,10 @@ int HidController::open() {
         return -1;
     }
 
+    for (int i = 0; i < kNumBuffers; i++) {
+        memset(m_pPollData[i], 0, kBufferSize);
+    }
+
     setOpen(true);
     startEngine();
 
@@ -243,16 +248,42 @@ int HidController::close() {
 bool HidController::poll() {
     Trace hidRead("HidController poll");
 
-    int result = hid_read(m_pHidDevice, m_pPollData, sizeof(m_pPollData) / sizeof(m_pPollData[0]));
-    if (result == -1) {
-        return false;
-    } else if (result > 0) {
+    // This loop risks becoming a high priority endless loop in case processing
+    // the mapping JS code takes longer than the controller polling rate.
+    // This could stall other low priority tasks.
+    // There is no safety net for this because it has not been demonstrated to be
+    // a problem in practice.
+    while (true) {
+        // Cycle between buffers so the memcmp below does not require deep copying to another buffer.
+        unsigned char* pPreviousBuffer = m_pPollData[m_iPollingBufferIndex];
+        m_iPollingBufferIndex = (m_iPollingBufferIndex + 1) % kNumBuffers;
+        unsigned char* pCurrentBuffer = m_pPollData[m_iPollingBufferIndex];
+
+        int bytesRead = hid_read(m_pHidDevice, pCurrentBuffer, kBufferSize);
+        if (bytesRead < 0) {
+            // -1 is the only error value according to hidapi documentation.
+            DEBUG_ASSERT(bytesRead == -1);
+            return false;
+        } else if (bytesRead == 0) {
+            return true;
+        }
+
         Trace process("HidController process packet");
-        QByteArray outData(reinterpret_cast<char*>(m_pPollData), result);
-        receive(outData, mixxx::Time::elapsed());
+        // Some controllers such as the Gemini GMX continuously send input packets even if it
+        // is identical to the previous packet. If this loop processed all those redundant
+        // packets, it would be a big performance problem to run JS code for every packet and
+        // would be unnecessary.
+        // This assumes that the redundant packets all use the same report ID. In practice we
+        // have not encountered any controllers that send redundant packets with different report
+        // IDs. If any such devices exist, this may be changed to use a separate buffer to store
+        // the last packet for each report ID.
+        if (memcmp(pCurrentBuffer, pPreviousBuffer, kBufferSize) == 0) {
+            continue;
+        }
+        auto incomingData = QByteArray::fromRawData(
+                reinterpret_cast<char*>(pCurrentBuffer), bytesRead);
+        receive(incomingData, mixxx::Time::elapsed());
     }
-
-    return true;
 }
 
 bool HidController::isPolling() const {
diff --git a/src/controllers/hid/hidcontroller.h b/src/controllers/hid/hidcontroller.h
index 543684ee76..65e06fd4f2 100644
--- a/src/controllers/hid/hidcontroller.h
+++ b/src/controllers/hid/hidcontroller.h
@@ -89,7 +89,10 @@ class HidController final : public Controller {
     hid_device* m_pHidDevice;
     HidControllerPreset m_preset;
 
-    unsigned char m_pPollData[255];
+    static constexpr int kNumBuffers = 2;
+    static constexpr int kBufferSize = 255;
+    unsigned char m_pPollData[kNumBuffers][kBufferSize];
+    int m_iPollingBufferIndex;
 };
 
 #endif