Skip to content

Commit

Permalink
Renamed HidIo to HidIoThread
Browse files Browse the repository at this point in the history 
Removed unnecessary parent QObject
Make the pointer to hid_device structure const, but the structure itself is not const. Added a comment to explain this.
Use default constructor for m_lastSentOutputReport
Deleted empty deconstructors
Use atomicStoreRelaxed(m_stop instead of m_stop =
  • Loading branch information
@JoergAtGithub
JoergAtGithub committed Dec 28, 2021
1 parent 42d5608 commit 939a02e
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 44 deletions.
28 changes: 14 additions & 14 deletions src/controllers/hid/hidcontroller.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,42 +103,42 @@ int HidController::open() {
startEngine();

if (m_pHidIo != nullptr) {
qWarning() << "HidIo already present for" << getName();
qWarning() << "HidIoThread already present for" << getName();
} else {
m_pHidIo = new HidIo(m_pHidDevice,
m_pHidIo = new HidIoThread(m_pHidDevice,
std::move(m_deviceInfo),
m_logBase,
m_logInput,
m_logOutput);
m_pHidIo->setObjectName(QString("HidIo %1").arg(getName()));
m_pHidIo->setObjectName(QString("HidIoThread %1").arg(getName()));

connect(m_pHidIo,
&HidIo::receive,
&HidIoThread::receive,
this,
&HidController::receive,
Qt::QueuedConnection);

connect(this,
&HidController::getInputReport,
m_pHidIo,
&HidIo::getInputReport,
&HidIoThread::getInputReport,
Qt::DirectConnection); // Enforces syncronisation of mapping and IO thread

connect(this,
&HidController::sendOutputReport,
m_pHidIo,
&HidIo::sendOutputReport,
&HidIoThread::sendOutputReport,
Qt::QueuedConnection);

connect(this,
&HidController::getFeatureReport,
m_pHidIo,
&HidIo::getFeatureReport,
&HidIoThread::getFeatureReport,
Qt::DirectConnection); // Enforces syncronisation of mapping and IO thread
connect(this,
&HidController::sendFeatureReport,
m_pHidIo,
&HidIo::sendFeatureReport,
&HidIoThread::sendFeatureReport,
Qt::DirectConnection); // Enforces syncronisation of mapping and IO thread

// Controller input needs to be prioritized since it can affect the
Expand All @@ -158,32 +158,32 @@ int HidController::close() {

// Stop the reading thread
if (m_pHidIo == nullptr) {
qWarning() << "HidIo not present for" << getName()
qWarning() << "HidIoThread not present for" << getName()
<< "yet the device is open!";
} else {
disconnect(m_pHidIo,
&HidIo::receive,
&HidIoThread::receive,
this,
&HidController::receive);

disconnect(this,
&HidController::getInputReport,
m_pHidIo,
&HidIo::getInputReport);
&HidIoThread::getInputReport);

disconnect(this,
&HidController::sendOutputReport,
m_pHidIo,
&HidIo::sendOutputReport);
&HidIoThread::sendOutputReport);

disconnect(this,
&HidController::getFeatureReport,
m_pHidIo,
&HidIo::getFeatureReport);
&HidIoThread::getFeatureReport);
disconnect(this,
&HidController::sendFeatureReport,
m_pHidIo,
&HidIo::sendFeatureReport);
&HidIoThread::sendFeatureReport);

m_pHidIo->stop();
hid_set_nonblocking(m_pHidDevice, 1); // Quit blocking
Expand Down
2 changes: 1 addition & 1 deletion src/controllers/hid/hidcontroller.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class HidController final : public Controller {
const mixxx::hid::DeviceInfo m_deviceInfo;

hid_device* m_pHidDevice;
HidIo* m_pHidIo;
HidIoThread* m_pHidIo;
std::shared_ptr<LegacyHidControllerMapping> m_pMapping;

friend class HidControllerJSProxy;
Expand Down
34 changes: 14 additions & 20 deletions src/controllers/hid/hidio.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,23 +22,20 @@ HidIoReport::HidIoReport(const unsigned char& reportId,
m_pHidDevice(device),
m_deviceInfo(std::move(deviceInfo)),
m_logOutput(logOutput),
m_lastSentOutputreport("") {
}

HidIoReport::~HidIoReport() {
m_lastSentOutputReport() {
}

void HidIoReport::sendOutputReport(QByteArray data) {
auto startOfHidWrite = mixxx::Time::elapsed();
if (!m_lastSentOutputreport.compare(data)) {
if (!m_lastSentOutputReport.compare(data)) {
qCDebug(m_logOutput) << "t:" << startOfHidWrite.formatMillisWithUnit()
<< " Skipped identical Output Report for" << m_deviceInfo.formatName()
<< "serial #" << m_deviceInfo.serialNumberRaw()
<< "(Report ID" << m_reportId << ")";
return; // Same data sent last time
}
m_lastSentOutputreport.clear();
m_lastSentOutputreport.append(data);
m_lastSentOutputReport.clear();
m_lastSentOutputReport.append(data);

// Prepend the Report ID to the beginning of data[] per the API..
data.prepend(m_reportId);
Expand All @@ -59,7 +56,7 @@ void HidIoReport::sendOutputReport(QByteArray data) {
}
}

HidIo::HidIo(hid_device* device,
HidIoThread::HidIoThread(hid_device* device,
const mixxx::hid::DeviceInfo&& deviceInfo,
const RuntimeLoggingCategory& logBase,
const RuntimeLoggingCategory& logInput,
Expand All @@ -78,19 +75,16 @@ HidIo::HidIo(hid_device* device,
m_lastPollSize = 0;
}

HidIo::~HidIo() {
}

void HidIo::run() {
m_stop = 0;
void HidIoThread::run() {
atomicStoreRelaxed(m_stop, 0);
while (atomicLoadRelaxed(m_stop) == 0) {
poll();
usleep(1000);
}
}

void HidIo::poll() {
Trace hidRead("HidIo poll");
void HidIoThread::poll() {
Trace hidRead("HidIoThread poll");

// This loop risks becoming a high priority endless loop in case processing
// the mapping JS code takes longer than the controller polling rate.
Expand All @@ -111,7 +105,7 @@ void HidIo::poll() {
}
}

void HidIo::processInputReport(int bytesRead) {
void HidIoThread::processInputReport(int bytesRead) {
Trace process("HidIO processInputReport");
unsigned char* pPreviousBuffer = m_pPollData[(m_pollingBufferIndex + 1) % kNumBuffers];
unsigned char* pCurrentBuffer = m_pPollData[m_pollingBufferIndex];
Expand All @@ -138,7 +132,7 @@ void HidIo::processInputReport(int bytesRead) {
emit receive(incomingData, mixxx::Time::elapsed());
}

QByteArray HidIo::getInputReport(unsigned int reportID) {
QByteArray HidIoThread::getInputReport(unsigned int reportID) {
auto startOfHidGetInputReport = mixxx::Time::elapsed();
int bytesRead;

Expand Down Expand Up @@ -169,7 +163,7 @@ QByteArray HidIo::getInputReport(unsigned int reportID) {
reinterpret_cast<char*>(m_pPollData[m_pollingBufferIndex]), bytesRead);
}

void HidIo::sendOutputReport(const QByteArray& data, unsigned int reportID) {
void HidIoThread::sendOutputReport(const QByteArray& data, unsigned int reportID) {
if (m_outputReports.find(reportID) == m_outputReports.end()) {
std::unique_ptr<HidIoReport> pNewOutputReport;
m_outputReports[reportID] = std::make_unique<HidIoReport>(
Expand All @@ -182,7 +176,7 @@ void HidIo::sendOutputReport(const QByteArray& data, unsigned int reportID) {
poll(); // Polling available Input-Reports is a cheap software only operation, which takes insignificiant time
}

void HidIo::sendFeatureReport(
void HidIoThread::sendFeatureReport(
const QByteArray& reportData, unsigned int reportID) {
auto startOfHidSendFeatureReport = mixxx::Time::elapsed();
QByteArray dataArray;
Expand Down Expand Up @@ -216,7 +210,7 @@ void HidIo::sendFeatureReport(
}
}

QByteArray HidIo::getFeatureReport(
QByteArray HidIoThread::getFeatureReport(
unsigned int reportID) {
auto startOfHidGetFeatureReport = mixxx::Time::elapsed();
unsigned char dataRead[kReportIdSize + kBufferSize];
Expand Down
15 changes: 6 additions & 9 deletions src/controllers/hid/hidio.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,36 +8,33 @@
#include "util/compatibility/qatomic.h"
#include "util/duration.h"

class HidIoReport : public QObject {
Q_OBJECT
class HidIoReport {
public:
HidIoReport(const unsigned char& reportId,
hid_device* device,
const mixxx::hid::DeviceInfo&& deviceInfo,
const RuntimeLoggingCategory& logOutput);
~HidIoReport();
void sendOutputReport(QByteArray data);

private:
const unsigned char m_reportId;
hid_device* m_pHidDevice;
const mixxx::hid::DeviceInfo m_deviceInfo;
const RuntimeLoggingCategory m_logOutput;
QByteArray m_lastSentOutputreport;
QByteArray m_lastSentOutputReport;
};

class HidIo : public QThread {
class HidIoThread : public QThread {
Q_OBJECT
public:
HidIo(hid_device* device,
HidIoThread(hid_device* device,
const mixxx::hid::DeviceInfo&& deviceInfo,
const RuntimeLoggingCategory& logBase,
const RuntimeLoggingCategory& logInput,
const RuntimeLoggingCategory& logOutput);
~HidIo();

void stop() {
m_stop = 1;
atomicStoreRelaxed(m_stop, 1);
}

static constexpr int kNumBuffers = 2;
Expand Down Expand Up @@ -65,7 +62,7 @@ class HidIo : public QThread {
private:
void poll();
void processInputReport(int bytesRead);
hid_device* m_pHidDevice;
hid_device* const m_pHidDevice; // const pointer to the C data structure, which hidapi uses for communication between functions
const mixxx::hid::DeviceInfo m_deviceInfo;
QAtomicInt m_stop;
std::map<unsigned char, std::unique_ptr<HidIoReport>> m_outputReports;
Expand Down

0 comments on commit 939a02e

Please sign in to comment.