fix: use interface number on Linux and improve logs

(cherry picked from commit a7a40ec)
mixxxdj · Oct 8, 2024 · dd7a5b9 · dd7a5b9
1 parent 4bb5cb7
commit dd7a5b9
Show file tree

Hide file tree

Showing 12 changed files with 30 additions and 28 deletions.
diff --git a/src/controllers/bulk/bulkcontroller.cpp b/src/controllers/bulk/bulkcontroller.cpp
@@ -5,6 +5,7 @@
 #include "controllers/bulk/bulksupported.h"
 #include "controllers/defs_controllers.h"
 #include "moc_bulkcontroller.cpp"
+#include "util/cmdlineargs.h"
 #include "util/time.h"
 #include "util/trace.h"
 
@@ -130,12 +131,10 @@ bool BulkController::matchProductInfo(const ProductInfo& product) {
         return false;
     }
 
-#if defined(__WINDOWS__) || defined(__APPLE__)
     value = product.interface_number.toInt(&ok, 16);
     if (!ok || m_interfaceNumber != static_cast<unsigned int>(value)) {
         return false;
     }
-#endif
 
     // Match found
     return true;
@@ -154,9 +153,7 @@ int BulkController::open() {
                 (bulk_supported[i].product_id == m_productId)) {
             m_inEndpointAddr = bulk_supported[i].in_epaddr;
             m_outEndpointAddr = bulk_supported[i].out_epaddr;
-#if defined(__WINDOWS__) || defined(__APPLE__)
             m_interfaceNumber = bulk_supported[i].interface_number;
-#endif
             break;
         }
     }
@@ -177,7 +174,6 @@ int BulkController::open() {
         return -1;
     }
 
-#if defined(__WINDOWS__) || defined(__APPLE__)
     if (m_interfaceNumber && libusb_kernel_driver_active(m_phandle, m_interfaceNumber) == 1) {
         qCDebug(m_logBase) << "Found a driver active for" << getName();
         if (libusb_detach_kernel_driver(m_phandle, 0) == 0)
@@ -200,7 +196,6 @@ int BulkController::open() {
             qCDebug(m_logBase) << "Claimed interface for" << getName();
         }
     }
-#endif
 
     setOpen(true);
     startEngine();
@@ -281,13 +276,13 @@ void BulkController::send(const QList<int>& data, unsigned int length) {
     sendBytes(temp);
 }
 
-void BulkController::sendBytes(const QByteArray& data) {
+bool BulkController::sendBytes(const QByteArray& data) {
     VERIFY_OR_DEBUG_ASSERT(!m_pMapping ||
             m_pMapping->getDeviceDirection() &
                     LegacyControllerMapping::DeviceDirection::Outgoing) {
         qDebug() << "The mapping for the bulk device" << getName()
                  << "doesn't require sending data. Ignoring sending request.";
-        return;
+        return false;
     }
 
     int ret;
@@ -299,12 +294,14 @@ void BulkController::sendBytes(const QByteArray& data) {
             (unsigned char*)data.constData(),
             data.size(),
             &transferred,
-            0);
+            5000);
     if (ret < 0) {
         qCWarning(m_logOutput) << "Unable to send data to" << getName()
                                << "serial #" << m_sUID << "-" << libusb_error_name(ret);
-    } else {
+        return false;
+    } else if (CmdlineArgs::Instance().getControllerDebug()) {
         qCDebug(m_logOutput) << transferred << "bytes sent to" << getName()
                              << "serial #" << m_sUID;
     }
+    return true;
 }
diff --git a/src/controllers/bulk/bulkcontroller.h b/src/controllers/bulk/bulkcontroller.h
@@ -63,7 +63,7 @@ class BulkController : public Controller {
   private:
     // For devices which only support a single report, reportID must be set to
     // 0x0.
-    void sendBytes(const QByteArray& data) override;
+    bool sendBytes(const QByteArray& data) override;
 
     bool matchProductInfo(const ProductInfo& product);
 
@@ -76,9 +76,7 @@ class BulkController : public Controller {
     unsigned short m_productId;
     unsigned char m_inEndpointAddr;
     unsigned char m_outEndpointAddr;
-#if defined(__WINDOWS__) || defined(__APPLE__)
     unsigned int m_interfaceNumber;
-#endif
     QString m_manufacturer;
     QString m_product;
 

diff --git a/src/controllers/bulk/bulksupported.h b/src/controllers/bulk/bulksupported.h
@@ -11,8 +11,9 @@ typedef struct bulk_supported {
 } bulk_supported_t;
 
 static bulk_supported_t bulk_supported[] = {
-        {0x06f8, 0xb105, 0x82, 0x03, 0}, // Hercules MP3e2
-        {0x06f8, 0xb107, 0x83, 0x03, 0}, // Hercules Mk4
-        {0x06f8, 0xb100, 0x86, 0x06, 0}, // Hercules Mk2
-        {0x06f8, 0xb120, 0x82, 0x03, 0}, // Hercules MP3 LE / Glow
+        {0x06f8, 0xb105, 0x82, 0x03, 0},    // Hercules MP3e2
+        {0x06f8, 0xb107, 0x83, 0x03, 0},    // Hercules Mk4
+        {0x06f8, 0xb100, 0x86, 0x06, 0},    // Hercules Mk2
+        {0x06f8, 0xb120, 0x82, 0x03, 0},    // Hercules MP3 LE / Glow
+        {0x17cc, 0x1720, 0x00, 0x03, 0x04}, // Traktor NI S4 Mk3
         {0, 0, 0, 0, 0}};
diff --git a/src/controllers/controller.h b/src/controllers/controller.h
@@ -142,7 +142,7 @@ class Controller : public QObject {
   public:
     // This must be reimplemented by sub-classes desiring to send raw bytes to a
     // controller.
-    virtual void sendBytes(const QByteArray& data) = 0;
+    virtual bool sendBytes(const QByteArray& data) = 0;
 
   private: // but used by ControllerManager
 

diff --git a/src/controllers/hid/hidcontroller.cpp b/src/controllers/hid/hidcontroller.cpp
@@ -198,10 +198,11 @@ int HidController::close() {
 /// This function is only for class compatibility with the (midi)controller
 /// and will not do the same as for MIDI devices,
 /// because sending of raw bytes is not a supported HIDAPI feature.
-void HidController::sendBytes(const QByteArray& data) {
+bool HidController::sendBytes(const QByteArray& data) {
     // Some HIDAPI backends will fail if the device uses ReportIDs (as practical all DJ controllers),
     // because 0 is no valid ReportID for these devices.
     m_pHidIoThread->updateCachedOutputReportData(0, data, false);
+    return true;
 }
 
 ControllerJSProxy* HidController::jsProxy() {

diff --git a/src/controllers/hid/hidcontroller.h b/src/controllers/hid/hidcontroller.h
@@ -36,7 +36,7 @@ class HidController final : public Controller {
   private:
     // For devices which only support a single report, reportID must be set to
     // 0x0.
-    void sendBytes(const QByteArray& data) override;
+    bool sendBytes(const QByteArray& data) override;
 
     const mixxx::hid::DeviceInfo m_deviceInfo;
 

diff --git a/src/controllers/midi/hss1394controller.cpp b/src/controllers/midi/hss1394controller.cpp
@@ -185,13 +185,15 @@ void Hss1394Controller::sendShortMsg(unsigned char status, unsigned char byte1,
     }
 }
 
-void Hss1394Controller::sendBytes(const QByteArray& data) {
+bool Hss1394Controller::sendBytes(const QByteArray& data) {
     const int bytesSent = m_pChannel->SendChannelBytes(
             reinterpret_cast<const unsigned char*>(data.constData()), data.size());
 
     qCDebug(m_logOutput) << MidiUtils::formatSysexMessage(getName(), data);
     if (bytesSent != data.size()) {
         qCWarning(m_logOutput) << "Sent" << bytesSent << "of" << data.size() << "bytes (SysEx)";
         //m_pChannel->Flush();
+        return false;
     }
+    return true;
 }
diff --git a/src/controllers/midi/hss1394controller.h b/src/controllers/midi/hss1394controller.h
@@ -53,7 +53,7 @@ class Hss1394Controller : public MidiController {
   private:
     // The sysex data must already contain the start byte 0xf0 and the end byte
     // 0xf7.
-    void sendBytes(const QByteArray& data) override;
+    bool sendBytes(const QByteArray& data) override;
 
     hss1394::TNodeInfo m_deviceInfo;
     int m_iDeviceIndex;

diff --git a/src/controllers/midi/portmidicontroller.cpp b/src/controllers/midi/portmidicontroller.cpp
@@ -222,28 +222,30 @@ void PortMidiController::sendShortMsg(unsigned char status, unsigned char byte1,
     }
 }
 
-void PortMidiController::sendBytes(const QByteArray& data) {
+bool PortMidiController::sendBytes(const QByteArray& data) {
     // PortMidi does not receive a length argument for the buffer we provide to
     // Pm_WriteSysEx. Instead, it scans for a MidiOpCode::EndOfExclusive byte
     // to know when the message is over. If one is not provided, it will
     // overflow the buffer and cause a segfault.
     if (!data.endsWith(MidiUtils::opCodeValue(MidiOpCode::EndOfExclusive))) {
         qCDebug(m_logOutput) << "SysEx message does not end with 0xF7 -- ignoring.";
-        return;
+        return false;
     }
 
     if (m_pOutputDevice.isNull() || !m_pOutputDevice->isOpen()) {
-        return;
+        return false;
     }
 
     PmError err = m_pOutputDevice->writeSysEx((unsigned char*)data.constData());
     if (err == pmNoError) {
         qCDebug(m_logOutput) << QStringLiteral("outgoing: ")
                              << MidiUtils::formatSysexMessage(getName(), data);
+        return true;
     } else {
         // Use two qWarnings() to ensure line break works on all operating systems
         qCWarning(m_logOutput) << "Error sending SysEx message:"
                                << MidiUtils::formatSysexMessage(getName(), data);
         qCWarning(m_logOutput) << "PortMidi error:" << Pm_GetErrorText(err);
     }
+    return false;
 }
diff --git a/src/controllers/midi/portmidicontroller.h b/src/controllers/midi/portmidicontroller.h
@@ -68,7 +68,7 @@ class PortMidiController : public MidiController {
   private:
     // The sysex data must already contain the start byte 0xf0 and the end byte
     // 0xf7.
-    void sendBytes(const QByteArray& data) override;
+    bool sendBytes(const QByteArray& data) override;
 
     bool isPolling() const override {
         return true;

diff --git a/src/test/controller_mapping_validation_test.h b/src/test/controller_mapping_validation_test.h
@@ -119,8 +119,9 @@ class FakeController : public Controller {
         Q_UNUSED(length);
     }
 
-    void sendBytes(const QByteArray& data) override {
+    bool sendBytes(const QByteArray& data) override {
         Q_UNUSED(data);
+        return true;
     }
 
   private slots:

diff --git a/src/test/midicontrollertest.cpp b/src/test/midicontrollertest.cpp
@@ -26,7 +26,7 @@ class MockMidiController : public MidiController {
             void(unsigned char status,
                     unsigned char byte1,
                     unsigned char byte2));
-    MOCK_METHOD1(sendBytes, void(const QByteArray& data));
+    MOCK_METHOD1(sendBytes, bool(const QByteArray& data));
     MOCK_CONST_METHOD0(isPolling, bool());
 };