Skip to content

Commit

Permalink
fix: return output result from controller
Browse files Browse the repository at this point in the history
  • Loading branch information
acolombier committed Oct 8, 2024
1 parent b67aa58 commit 6d3b329
Show file tree
Hide file tree
Showing 11 changed files with 25 additions and 16 deletions.
9 changes: 6 additions & 3 deletions src/controllers/bulk/bulkcontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -281,13 +282,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;
Expand All @@ -303,8 +304,10 @@ void BulkController::sendBytes(const QByteArray& data) {
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;
}
2 changes: 1 addition & 1 deletion src/controllers/bulk/bulkcontroller.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions src/controllers/controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,8 @@ 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;
// controller. Return true in case of successful completion, false in case of failure.
virtual bool sendBytes(const QByteArray& data) = 0;

private: // but used by ControllerManager

Expand Down
3 changes: 2 additions & 1 deletion src/controllers/hid/hidcontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion src/controllers/hid/hidcontroller.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
4 changes: 3 additions & 1 deletion src/controllers/midi/hss1394controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
2 changes: 1 addition & 1 deletion src/controllers/midi/hss1394controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 5 additions & 3 deletions src/controllers/midi/portmidicontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
2 changes: 1 addition & 1 deletion src/controllers/midi/portmidicontroller.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion src/test/controller_mapping_validation_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion src/test/midicontrollertest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
};

Expand Down

0 comments on commit 6d3b329

Please sign in to comment.