Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add groundwork for Controller COs #2884

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/controllers/bulk/bulkcontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,12 @@ static QString get_string(libusb_device_handle *handle, u_int8_t id) {
}

BulkController::BulkController(
const QString& group,
libusb_context* context,
libusb_device_handle* handle,
struct libusb_device_descriptor* desc)
: m_context(context),
: Controller(group),
m_context(context),
m_phandle(handle),
in_epaddr(0),
out_epaddr(0) {
Expand Down
1 change: 1 addition & 0 deletions src/controllers/bulk/bulkcontroller.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class BulkController : public Controller {
Q_OBJECT
public:
BulkController(
const QString& group,
libusb_context* context,
libusb_device_handle* handle,
struct libusb_device_descriptor* desc);
Expand Down
8 changes: 7 additions & 1 deletion src/controllers/bulk/bulkenumerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ QList<Controller*> BulkEnumerator::queryDevices() {
ssize_t i = 0;
int err = 0;

int index = 1;
for (i = 0; i < cnt; i++) {
libusb_device *device = list[i];
struct libusb_device_descriptor desc;
Expand All @@ -56,8 +57,13 @@ QList<Controller*> BulkEnumerator::queryDevices() {
continue;
}

// Generate a group for this controller
QString group = QStringLiteral("[BulkController") +
QString::number(index) + QChar(']');
index++;

BulkController* currentDevice =
new BulkController(m_context, handle, &desc);
new BulkController(group, m_context, handle, &desc);
m_devices.push_back(currentDevice);
}
}
Expand Down
20 changes: 17 additions & 3 deletions src/controllers/controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,27 @@
* @brief Base class representing a physical (or software) controller.
*/

#include "controllers/controller.h"

#include <QApplication>
#include <QJSValue>

#include "controllers/controller.h"
#include "control/controlobject.h"
#include "controllers/controllerdebug.h"
#include "controllers/defs_controllers.h"
#include "util/screensaver.h"

Controller::Controller()
: m_pEngine(nullptr),
Controller::Controller(const QString& group)
: m_group(group),
m_pEngine(nullptr),
m_bIsOutputDevice(false),
m_bIsInputDevice(false),
m_bIsOpen(false),
m_bLearning(false) {
m_userActivityInhibitTimer.start();

m_pReloadScripts = make_parented<ControlObject>(ConfigKey(group, "reload_scripts"), this);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QStringLiteral("reload_scripts")

connect(m_pReloadScripts, &ControlObject::valueChanged, this, &Controller::slotReloadScripts);
}

Controller::~Controller() {
Expand Down Expand Up @@ -87,6 +93,14 @@ void Controller::stopLearning() {

}

void Controller::slotReloadScripts(double v) {
if (!v || !m_pEngine) {
return;
}

m_pEngine->reloadScripts();
}

void Controller::send(QList<int> data, unsigned int length) {
// If you change this implementation, also change it in HidController (That
// function is required due to HID devices having report IDs)
Expand Down
16 changes: 15 additions & 1 deletion src/controllers/controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@
#include "controllers/controllervisitor.h"
#include "controllers/engine/controllerengine.h"
#include "util/duration.h"
#include "util/parented_ptr.h"

class ControlObject;
class ControllerJSProxy;

class Controller : public QObject, ConstControllerPresetVisitor {
Q_OBJECT
public:
Controller();
Controller(const QString& group);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

explicit

Minor improvement: Since Controller is supposed to be an abstract base class we could reduce the visibility of the ctor from public to protected.

~Controller() override; // Subclass should call close() at minimum.

/// The object that is exposed to the JS scripts as the "controller" object.
Expand Down Expand Up @@ -64,6 +66,10 @@ class Controller : public QObject, ConstControllerPresetVisitor {
inline const QString& getCategory() const {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could remove all those redundant inline keywords while editing this file.

return m_sDeviceCategory;
}
const QString& getGroup() const {
return m_group;
}

virtual bool isMappable() const = 0;
inline bool isLearning() const {
return m_bLearning;
Expand Down Expand Up @@ -102,6 +108,8 @@ class Controller : public QObject, ConstControllerPresetVisitor {
void startLearning();
void stopLearning();

void slotReloadScripts(double v);

protected:
// The length parameter is here for backwards compatibility for when scripts
// were required to specify it.
Expand Down Expand Up @@ -160,6 +168,10 @@ class Controller : public QObject, ConstControllerPresetVisitor {
// Returns a pointer to the currently loaded controller preset. For internal
// use only.
virtual ControllerPreset* preset() = 0;

/// Group name for control objects
const QString m_group;

ControllerEngine* m_pEngine;

// Verbose and unique device name suitable for display.
Expand All @@ -176,6 +188,8 @@ class Controller : public QObject, ConstControllerPresetVisitor {
bool m_bLearning;
QElapsedTimer m_userActivityInhibitTimer;

parented_ptr<ControlObject> m_pReloadScripts;

friend class ControllerJSProxy;
// accesses lots of our stuff, but in the same thread
friend class ControllerManager;
Expand Down
1 change: 1 addition & 0 deletions src/controllers/engine/controllerengine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ void ControllerEngine::initializeScripts(const QList<ControllerPreset::ScriptFil
QJSValueList args;
args << QJSValue(m_pController->getName());
args << QJSValue(ControllerDebug::enabled());
args << QJSValue(m_pController->getGroup());

// Call the init method for all the prefixes.
bool success = callFunctionOnObjects(m_scriptFunctionPrefixes, "init", args, true);
Expand Down
3 changes: 2 additions & 1 deletion src/controllers/engine/controllerengine.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ class ControllerEngine : public QObject {
m_bTesting = testing;
};

void reloadScripts();

protected:
double getValue(QString group, QString name);
void setValue(QString group, QString name, double newValue);
Expand Down Expand Up @@ -115,7 +117,6 @@ class ControllerEngine : public QObject {
bool evaluateScriptFile(const QFileInfo& scriptFile);
void initializeScriptEngine();
void uninitializeScriptEngine();
void reloadScripts();

void scriptErrorDialog(const QString& detailedError, const QString& key, bool bFatal = false);
void generateScriptFunctions(const QString& code);
Expand Down
7 changes: 4 additions & 3 deletions src/controllers/hid/hidcontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@ ControllerJSProxy* HidController::jsProxy() {
return new HidControllerJSProxy(this);
}

HidController::HidController(const hid_device_info& deviceInfo)
: Controller(),
HidController::HidController(
const QString& group,
const hid_device_info& deviceInfo)
: Controller(group),
m_pHidDevice(NULL) {

// Copy required variables from deviceInfo, which will be freed after
// this class is initialized by caller.
hid_vendor_id = deviceInfo.vendor_id;
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 @@ -20,7 +20,7 @@
class HidController final : public Controller {
Q_OBJECT
public:
HidController(const hid_device_info& deviceInfo);
HidController(const QString& group, const hid_device_info& deviceInfo);
~HidController() override;

ControllerJSProxy* jsProxy() override;
Expand Down
8 changes: 7 additions & 1 deletion src/controllers/hid/hidenumerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ QList<Controller*> HidEnumerator::queryDevices() {
struct hid_device_info *devs, *cur_dev;
devs = hid_enumerate(0x0, 0x0);

int index = 1;
for (cur_dev = devs; cur_dev; cur_dev = cur_dev->next) {
if (isDeviceBlacklisted(cur_dev)) {
// OS/X and windows use usage_page/usage not interface_number
Expand Down Expand Up @@ -99,7 +100,12 @@ QList<Controller*> HidEnumerator::queryDevices() {
continue;
}

HidController* currentDevice = new HidController(*cur_dev);
// Generate a group for this controller
QString group = QStringLiteral("[HidController") +
QString::number(index) + QChar(']');
index++;

HidController* currentDevice = new HidController(group, *cur_dev);
m_devices.push_back(currentDevice);
}
hid_free_enumeration(devs);
Expand Down
3 changes: 2 additions & 1 deletion src/controllers/midi/hss1394controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,11 @@ void DeviceChannelListener::Reconnected() {
}

Hss1394Controller::Hss1394Controller(
const QString& group,
const hss1394::TNodeInfo& deviceInfo,
int deviceIndex,
UserSettingsPointer pConfig)
: MidiController(),
: MidiController(group),
m_deviceInfo(deviceInfo),
m_iDeviceIndex(deviceIndex) {
// Note: We prepend the input stream's index to the device's name to prevent
Expand Down
1 change: 1 addition & 0 deletions src/controllers/midi/hss1394controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class Hss1394Controller : public MidiController {
Q_OBJECT
public:
Hss1394Controller(
const QString& group,
const hss1394::TNodeInfo& deviceInfo,
int deviceIndex,
UserSettingsPointer pConfig);
Expand Down
9 changes: 8 additions & 1 deletion src/controllers/midi/hss1394enumerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ QList<Controller*> Hss1394Enumerator::queryDevices() {
hss1394::uint uNodes = Node::Instance()->GetNodeCount();
qDebug() << " Nodes detected:" << uNodes;

int index = 1;
for(hss1394::uint i=0; i<40; i++) {
TNodeInfo tNodeInfo;
bool bInstalled;
Expand All @@ -44,8 +45,14 @@ QList<Controller*> Hss1394Enumerator::queryDevices() {
QString("%1").arg(tNodeInfo.uGUID.mu32Low, 0, 16),
QString("%1").arg(tNodeInfo.uProtocolVersion, 0, 16));
qDebug() << " " << message;

// Generate a group for this controller
QString group = QStringLiteral("[Hss1374Controller") +
QString::number(index) + QChar(']');
index++;

Hss1394Controller* currentDevice = new Hss1394Controller(
tNodeInfo, i, m_pConfig);
group, tNodeInfo, i, m_pConfig);
m_devices.push_back(currentDevice);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/controllers/midi/midicontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
#include "util/math.h"
#include "util/screensaver.h"

MidiController::MidiController()
: Controller() {
MidiController::MidiController(const QString& group)
: Controller(group) {
setDeviceCategory(tr("MIDI Controller"));
}

Expand Down
2 changes: 1 addition & 1 deletion src/controllers/midi/midicontroller.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
class MidiController : public Controller {
Q_OBJECT
public:
explicit MidiController();
explicit MidiController(const QString& group);
~MidiController() override;

ControllerJSProxy* jsProxy() override;
Expand Down
5 changes: 3 additions & 2 deletions src/controllers/midi/portmidicontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@
#include "controllers/midi/portmidicontroller.h"
#include "controllers/controllerdebug.h"

PortMidiController::PortMidiController(const PmDeviceInfo* inputDeviceInfo,
PortMidiController::PortMidiController(const QString& group,
const PmDeviceInfo* inputDeviceInfo,
const PmDeviceInfo* outputDeviceInfo,
int inputDeviceIndex,
int outputDeviceIndex)
: MidiController(), m_cReceiveMsg_index(0), m_bInSysex(false) {
: MidiController(group), m_cReceiveMsg_index(0), m_bInSysex(false) {
for (unsigned int k = 0; k < MIXXX_PORTMIDI_BUFFER_LEN; ++k) {
// Can be shortened to `m_midiBuffer[k] = {}` with C++11.
m_midiBuffer[k].message = 0;
Expand Down
3 changes: 2 additions & 1 deletion src/controllers/midi/portmidicontroller.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@
class PortMidiController : public MidiController {
Q_OBJECT
public:
PortMidiController(const PmDeviceInfo* inputDeviceInfo,
PortMidiController(const QString& group,
const PmDeviceInfo* inputDeviceInfo,
const PmDeviceInfo* outputDeviceInfo,
int inputDeviceIndex,
int outputDeviceIndex);
Expand Down
9 changes: 8 additions & 1 deletion src/controllers/midi/portmidienumerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ QList<Controller*> PortMidiEnumerator::queryDevices() {
}

// Search for input devices and pair them with output devices if applicable
int index = 1;
for (int i = 0; i < iNumDevices; i++) {
const PmDeviceInfo* deviceInfo = Pm_GetDeviceInfo(i);
if (shouldBlacklistDevice(deviceInfo)) {
Expand Down Expand Up @@ -253,13 +254,19 @@ QList<Controller*> PortMidiEnumerator::queryDevices() {
}
}

// Generate a group for this controller
QString group = QStringLiteral("[PortMidiController") +
QString::number(index) + QChar(']');
index++;

// So at this point, we either have an input-only MIDI device
// (outputDeviceInfo == NULL) or we've found a matching output MIDI
// device (outputDeviceInfo != NULL).

//.... so create our (aggregate) MIDI device!
PortMidiController* currentDevice =
new PortMidiController(inputDeviceInfo,
new PortMidiController(group,
inputDeviceInfo,
outputDeviceInfo,
inputDevIndex,
outputDevIndex);
Expand Down
7 changes: 4 additions & 3 deletions src/test/controller_preset_validation_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ void FakeControllerJSProxy::sendShortMsg(unsigned char status,
Q_UNUSED(byte2);
}

FakeController::FakeController()
: m_bMidiPreset(false),
FakeController::FakeController(const QString& group)
: Controller(group),
m_bMidiPreset(false),
m_bHidPreset(false) {
startEngine();
getEngine()->setTesting(true);
Expand Down Expand Up @@ -77,7 +78,7 @@ bool ControllerPresetValidationTest::testLoadPreset(const PresetInfo& preset) {
return false;
}

FakeController controller;
FakeController controller("[FakeController]");
controller.setDeviceName("Test Controller");
controller.setPreset(*pPreset);
// Do not initialize the scripts.
Expand Down
2 changes: 1 addition & 1 deletion src/test/controller_preset_validation_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class FakeControllerJSProxy : public ControllerJSProxy {
class FakeController : public Controller {
Q_OBJECT
public:
FakeController();
FakeController(const QString& group);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

explicit (although just a test class)

~FakeController() override;

QString presetExtension() override {
Expand Down
6 changes: 4 additions & 2 deletions src/test/midicontrollertest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@

class MockMidiController : public MidiController {
public:
explicit MockMidiController(): MidiController() {}
explicit MockMidiController(const QString& group)
: MidiController(group) {
}
~MockMidiController() override { }

MOCK_METHOD0(open, int());
Expand All @@ -27,7 +29,7 @@ class MockMidiController : public MidiController {
class MidiControllerTest : public MixxxTest {
protected:
void SetUp() override {
m_pController.reset(new MockMidiController());
m_pController.reset(new MockMidiController("[MockMidiControler]"));
}

void addMapping(MidiInputMapping mapping) {
Expand Down
Loading