Skip to content

Commit

Permalink
ControllerDebug: Clean up method names and add a way to disable testing
Browse files Browse the repository at this point in the history
  • Loading branch information
Holzhaus committed Dec 18, 2020
1 parent 6c8cb90 commit 4c95980
Show file tree
Hide file tree
Showing 9 changed files with 37 additions and 27 deletions.
2 changes: 1 addition & 1 deletion src/control/controlobjectscript.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#include "moc_controlobjectscript.cpp"

ControlObjectScript::ControlObjectScript(const ConfigKey& key, QObject* pParent)
: ControlProxy(key, pParent, ControllerDebug::shouldAssertForInvalidControlObjects()) {
: ControlProxy(key, pParent, ControllerDebug::controlFlags()) {
}

bool ControlObjectScript::addScriptConnection(const ScriptConnection& conn) {
Expand Down
2 changes: 1 addition & 1 deletion src/controllers/controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ void Controller::receive(const QByteArray& data, mixxx::Duration timestamp) {
triggerActivity();

int length = data.size();
if (ControllerDebug::enabled()) {
if (ControllerDebug::isEnabled()) {
// Formatted packet display
QString message = QString("%1: t:%2, %3 bytes:\n")
.arg(m_sDeviceName,
Expand Down
2 changes: 1 addition & 1 deletion src/controllers/controllerdebug.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ bool ControllerDebug::s_enabled = false;
bool ControllerDebug::s_testing = false;

//static
bool ControllerDebug::enabled() {
bool ControllerDebug::isEnabled() {
return s_enabled || CmdlineArgs::Instance().getMidiDebug();
}
28 changes: 14 additions & 14 deletions src/controllers/controllerdebug.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,25 @@ class ControllerDebug {
static constexpr const char kLogMessagePrefix[] = "CDBG";
static constexpr int kLogMessagePrefixLength = sizeof(kLogMessagePrefix) - 1;

static bool enabled();
static bool isEnabled();

/// Override the command-line argument (for testing)
static void enable() {
s_enabled = true;
static void setEnabled(bool enabled) {
s_enabled = enabled;
}

/// Override the command-line argument (for testing)
static void disable() {
s_enabled = false;
}

static void enableTesting() {
s_enabled = true;
s_testing = true;
static void setTesting(bool isTesting) {
s_testing = isTesting;
}

static ControlFlags shouldAssertForInvalidControlObjects() {
if (s_enabled && !s_testing) {
/// Return the appropriate flag for ControlProxies in mappings.
///
/// Normally, accessing an invalid control from a mapping should *not*
/// throw a debug assertion because controller mappings are considered
/// user data. If we're testing or controller debugging is enabled, we *do*
/// want assertions to prevent overlooking bugs in controller mappings.
static ControlFlags controlFlags() {
if (s_enabled || s_testing) {
return ControlFlag::None;
}

Expand All @@ -55,7 +55,7 @@ class ControllerDebug {
// output for mixxx.log with .noquote(), because in qt5 QDebug() is quoted by default.
#define controllerDebug(stream) \
{ \
if (ControllerDebug::enabled()) { \
if (ControllerDebug::isEnabled()) { \
QDebug(QtDebugMsg).noquote() << ControllerDebug::kLogMessagePrefix << stream; \
} \
}
2 changes: 1 addition & 1 deletion src/controllers/hid/hidcontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ void HidController::sendBytesReport(QByteArray data, unsigned int reportID) {

int result = hid_write(m_pHidDevice, (unsigned char*)data.constData(), data.size());
if (result == -1) {
if (ControllerDebug::enabled()) {
if (ControllerDebug::isEnabled()) {
qWarning() << "Unable to send data to" << getName()
<< "serial #" << m_deviceInfo.serialNumber() << ":"
<< mixxx::convertWCStringToQString(
Expand Down
2 changes: 1 addition & 1 deletion src/controllers/midi/midicontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ void MidiController::createOutputHandlers() {
qWarning() << errorLog;

int deckNum = 0;
if (ControllerDebug::enabled()) {
if (ControllerDebug::isEnabled()) {
failures.append(errorLog);
} else if (PlayerManager::isDeckGroup(group, &deckNum)) {
int numDecks = PlayerManager::numDecks();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ bool ControllerScriptEngineLegacy::initialize() {
} else { // m_pController is nullptr in tests.
args << QJSValue();
}
args << QJSValue(ControllerDebug::enabled());
args << QJSValue(ControllerDebug::isEnabled());
if (!callFunctionOnObjects(m_scriptFunctionPrefixes, "init", args, true)) {
shutdown();
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ void ControllerScriptInterfaceLegacy::setValue(

if (coScript != nullptr) {
ControlObject* pControl = ControlObject::getControl(
coScript->getKey(), ControllerDebug::shouldAssertForInvalidControlObjects());
coScript->getKey(), ControllerDebug::controlFlags());
if (pControl &&
!m_st.ignore(
pControl, coScript->getParameterForValue(newValue))) {
Expand Down Expand Up @@ -152,7 +152,7 @@ void ControllerScriptInterfaceLegacy::setParameter(

if (coScript != nullptr) {
ControlObject* pControl = ControlObject::getControl(
coScript->getKey(), ControllerDebug::shouldAssertForInvalidControlObjects());
coScript->getKey(), ControllerDebug::controlFlags());
if (pControl && !m_st.ignore(pControl, newParameter)) {
coScript->setParameter(newParameter);
}
Expand Down Expand Up @@ -498,7 +498,7 @@ void ControllerScriptInterfaceLegacy::timerEvent(QTimerEvent* event) {
void ControllerScriptInterfaceLegacy::softTakeover(
const QString& group, const QString& name, bool set) {
ControlObject* pControl = ControlObject::getControl(
ConfigKey(group, name), ControllerDebug::shouldAssertForInvalidControlObjects());
ConfigKey(group, name), ControllerDebug::controlFlags());
if (!pControl) {
return;
}
Expand All @@ -512,7 +512,7 @@ void ControllerScriptInterfaceLegacy::softTakeover(
void ControllerScriptInterfaceLegacy::softTakeoverIgnoreNextValue(
const QString& group, const QString& name) {
ControlObject* pControl = ControlObject::getControl(
ConfigKey(group, name), ControllerDebug::shouldAssertForInvalidControlObjects());
ConfigKey(group, name), ControllerDebug::controlFlags());
if (!pControl) {
return;
}
Expand Down
16 changes: 13 additions & 3 deletions src/test/controllerscriptenginelegacy_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class ControllerScriptEngineLegacyTest : public MixxxTest {
QThread::currentThread()->setObjectName("Main");
cEngine = new ControllerScriptEngineLegacy(nullptr);
cEngine->initialize();
ControllerDebug::enableTesting();
ControllerDebug::setTesting(true);
}

void TearDown() override {
Expand Down Expand Up @@ -80,19 +80,29 @@ TEST_F(ControllerScriptEngineLegacyTest, setValue) {
}

TEST_F(ControllerScriptEngineLegacyTest, getValue_InvalidKey) {
ControllerDebug::disable();
ControllerDebug::setEnabled(false);
ControllerDebug::setTesting(false);
EXPECT_TRUE(evaluateAndAssert("engine.getValue('', '');"));
EXPECT_TRUE(evaluateAndAssert("engine.getValue('', 'invalid');"));
EXPECT_TRUE(evaluateAndAssert("engine.getValue('[Invalid]', '');"));
ControllerDebug::enable();
ControllerDebug::setTesting(true);
ControllerDebug::setEnabled(true);
}

TEST_F(ControllerScriptEngineLegacyTest, setValue_InvalidControl) {
ControllerDebug::setEnabled(false);
ControllerDebug::setTesting(false);
EXPECT_TRUE(evaluateAndAssert("engine.setValue('[Nothing]', 'nothing', 1.0);"));
ControllerDebug::setTesting(true);
ControllerDebug::setEnabled(true);
}

TEST_F(ControllerScriptEngineLegacyTest, getValue_InvalidControl) {
ControllerDebug::setEnabled(false);
ControllerDebug::setTesting(false);
EXPECT_TRUE(evaluateAndAssert("engine.getValue('[Nothing]', 'nothing');"));
ControllerDebug::setTesting(true);
ControllerDebug::setEnabled(true);
}

TEST_F(ControllerScriptEngineLegacyTest, setValue_IgnoresNaN) {
Expand Down

0 comments on commit 4c95980

Please sign in to comment.