Fix and improve Logging & Debugging

build/depends.py

@@ -779,6 +779,7 @@ def sources(self, build):
                    "analyzer/analyzerebur128.cpp",
 
                    "controllers/controller.cpp",
+                   "controllers/controllerdebug.cpp",
                    "controllers/controllerengine.cpp",
                    "controllers/controllerenumerator.cpp",
                    "controllers/controllerlearningeventfilter.cpp",

src/controllers/controllerdebug.cpp

@@ -0,0 +1,12 @@
+#include "controllers/controllerdebug.h"
+
+#include "util/cmdlineargs.h"
+
+
+//static
+bool ControllerDebug::s_enabled = false;
+
+//static
+bool ControllerDebug::enabled() {
+    return s_enabled || CmdlineArgs::Instance().getMidiDebug();
+}

src/controllers/controllerdebug.h

@@ -3,33 +3,26 @@
 
 #include <QDebug>
 
-#include "util/cmdlineargs.h"
-#include "util/logging.h"
 
+// Specifies whether or not we should dump incoming data to the console at
+// runtime. This is useful for end-user debugging and script-writing.
 class ControllerDebug {
   public:
-      static ControllerDebug& instance() {
-          static ControllerDebug instance;
-          return instance;
-      }
+    // Any debug statement starting with this prefix bypasses the --logLevel
+    // command line flags.
+    static constexpr const char* kLogMessagePrefix = "CDBG";
 
-      static bool enabled() {
-          return instance().m_enabled;
-      }
+    static bool enabled();
 
-      static void setEnabled(bool enabled) {
-          instance().m_enabled = enabled;
-      }
+    // Override the command-line argument (for testing)
+    static void enable() {
+        s_enabled = true;
+    }
 
   private:
-    ControllerDebug() {
-        // Get --controllerDebug command line option
-        m_enabled = CmdlineArgs::Instance().getMidiDebug();
-    }
+    ControllerDebug() = delete;
 
-    // Specifies whether or not we should dump incoming data to the console at
-    // runtime. This is useful for end-user debugging and script-writing.
-    bool m_enabled;
+    static bool s_enabled;
 };
 
 // Usage: controllerDebug("hello" << "world");
@@ -40,7 +33,7 @@ class ControllerDebug {
 #define controllerDebug(stream)       \
 {                                     \
     if (ControllerDebug::enabled()) { \
-        QDebug(QtDebugMsg) << mixxx::Logging::kControllerDebugPrefix << stream; \
+        QDebug(QtDebugMsg) << ControllerDebug::kLogMessagePrefix << stream; \
     }                                 \
 }                                     \
 

src/test/controllerengine_test.cpp

@@ -18,7 +18,7 @@ class ControllerEngineTest : public MixxxTest {
         mixxx::Time::setTestElapsedTime(mixxx::Duration::fromMillis(10));
         QThread::currentThread()->setObjectName("Main");
         cEngine = new ControllerEngine(nullptr);
-        ControllerDebug::setEnabled(true);
+        ControllerDebug::enable();
         cEngine->setPopups(false);
     }
 

src/util/cmdlineargs.cpp

@@ -13,7 +13,7 @@ CmdlineArgs::CmdlineArgs()
       m_safeMode(false),
       m_debugAssertBreak(false),
       m_settingsPathSet(false),
-      m_logLevel(mixxx::Logging::kLogLevelDefault),
+      m_logLevel(mixxx::LogLevel::Default),
 // We are not ready to switch to XDG folders under Linux, so keeping $HOME/.mixxx as preferences folder. see lp:1463273
 #ifdef __LINUX__
     m_settingsPath(QDir::homePath().append("/").append(SETTINGS_PATH)) {
@@ -59,13 +59,13 @@ bool CmdlineArgs::Parse(int &argc, char **argv) {
             logLevelSet = true;
             auto level = QLatin1String(argv[i+1]);
             if (level == "debug") {
-                m_logLevel = mixxx::Logging::LogLevel::Debug;
+                m_logLevel = mixxx::LogLevel::Debug;
             } else if (level == "info") {
-                m_logLevel = mixxx::Logging::LogLevel::Info;
+                m_logLevel = mixxx::LogLevel::Info;
             } else if (level == "warning") {
-                m_logLevel = mixxx::Logging::LogLevel::Warning;
+                m_logLevel = mixxx::LogLevel::Warning;
             } else if (level == "critical") {
-                m_logLevel = mixxx::Logging::LogLevel::Critical;
+                m_logLevel = mixxx::LogLevel::Critical;
             } else {
                 fputs("\nlogLevel argument wasn't 'debug', 'info', 'warning', or 'critical'! Mixxx will only output\n\
 warnings and errors to the console unless this is set properly.\n", stdout);
@@ -88,7 +88,7 @@ warnings and errors to the console unless this is set properly.\n", stdout);
     // If --logLevel was unspecified and --developer is enabled then set
     // logLevel to debug.
     if (m_developer && !logLevelSet) {
-        m_logLevel = mixxx::Logging::LogLevel::Debug;
+        m_logLevel = mixxx::LogLevel::Debug;
     }
 
     return true;

src/util/cmdlineargs.h

@@ -26,7 +26,7 @@ class CmdlineArgs final {
     bool getSafeMode() const { return m_safeMode; }
     bool getDebugAssertBreak() const { return m_debugAssertBreak; }
     bool getSettingsPathSet() const { return m_settingsPathSet; }
-    mixxx::Logging::LogLevel getLogLevel() const { return m_logLevel; }
+    mixxx::LogLevel getLogLevel() const { return m_logLevel; }
     bool getTimelineEnabled() const { return !m_timelinePath.isEmpty(); }
     const QString& getLocale() const { return m_locale; }
     const QString& getSettingsPath() const { return m_settingsPath; }
@@ -47,7 +47,7 @@ class CmdlineArgs final {
     bool m_safeMode;
     bool m_debugAssertBreak;
     bool m_settingsPathSet; // has --settingsPath been set on command line ?
-    mixxx::Logging::LogLevel m_logLevel; // Level of logging message verbosity
+    mixxx::LogLevel m_logLevel; // Level of logging message verbosity
     QString m_locale;
     QString m_settingsPath;
     QString m_resourcePath;

src/util/logging.cpp

@@ -15,17 +15,21 @@
 #include <QtDebug>
 #include <QtGlobal>
 
+#include "controllers/controllerdebug.h"
 #include "util/assert.h"
 
 namespace mixxx {
+
+// Initialize the log level with the default value
+LogLevel Logging::s_logLevel = LogLevel::Default;
+
 namespace {
 
 // Mutex guarding g_logfile.
 QMutex g_mutexLogfile;
 // The file handle for Mixxx's log file.
 QFile g_logfile;
 // The log level.
-Logging::LogLevel g_logLevel = Logging::kLogLevelDefault;
 // Whether to break on debug assertions.
 bool g_debugAssertBreak = false;
 
@@ -67,26 +71,26 @@ void MessageHandler(QtMsgType type,
             tag = "Debug [";
             baSize += strlen(tag);
 #if QT_VERSION < QT_VERSION_CHECK(5, 0, 0)
-            isControllerDebug = strncmp(input, Logging::kControllerDebugPrefix,
-                                        strlen(Logging::kControllerDebugPrefix)) == 0;
+            isControllerDebug = strncmp(input, ControllerDebug::kLogMessagePrefix,
+                                        strlen(ControllerDebug::kLogMessagePrefix)) == 0;
 #else
             isControllerDebug = input.startsWith(QLatin1String(
-                Logging::kControllerDebugPrefix));
+                ControllerDebug::kLogMessagePrefix));
 #endif
-            shouldPrint = g_logLevel >= Logging::LogLevel::Debug ||
+            shouldPrint = Logging::enabled(LogLevel::Debug) ||
                     isControllerDebug;
             break;
 #if QT_VERSION >= QT_VERSION_CHECK(5, 5, 0)
         case QtInfoMsg:
             tag = "Info [";
             baSize += strlen(tag);
-            shouldPrint = g_logLevel >= Logging::LogLevel::Info;
+            shouldPrint = Logging::enabled(LogLevel::Info);
             break;
 #endif
         case QtWarningMsg:
             tag = "Warning [";
             baSize += strlen(tag);
-            shouldPrint = g_logLevel >= Logging::LogLevel::Warning;
+            shouldPrint = Logging::enabled(LogLevel::Warning);
             break;
         case QtCriticalMsg:
             tag = "Critical [";
@@ -118,13 +122,13 @@ void MessageHandler(QtMsgType type,
 #if QT_VERSION < QT_VERSION_CHECK(5, 0, 0)
     const char* inputOffset = input;
     if (isControllerDebug) {
-        inputOffset += strlen(Logging::kControllerDebugPrefix) + 1;
+        inputOffset += strlen(ControllerDebug::kLogMessagePrefix) + 1;
     }
     baSize += strlen(inputOffset);
 #else
     QByteArray input8Bit;
     if (isControllerDebug) {
-        input8Bit = input.mid(strlen(Logging::kControllerDebugPrefix) + 1).toLocal8Bit();
+        input8Bit = input.mid(strlen(ControllerDebug::kLogMessagePrefix) + 1).toLocal8Bit();
     } else {
         input8Bit = input.toLocal8Bit();
     }
@@ -161,19 +165,16 @@ void MessageHandler(QtMsgType type,
 #else
         // The "%s" is intentional. See -Werror=format-security.
         qFatal("%s", input8Bit.constData());
-#endif
+#endif // QT_VERSION
         return;
-#endif
+#endif // MIXXX_DEBUG_ASSERTIONS_FATAL
     }
 
     writeToLog(ba, shouldPrint, shouldFlush);
 }
 
 }  // namespace
 
-// static
-constexpr Logging::LogLevel Logging::kLogLevelDefault;
-
 // static
 void Logging::initialize(const QString& settingsPath, LogLevel logLevel,
                          bool debugAssertBreak) {
@@ -182,6 +183,8 @@ void Logging::initialize(const QString& settingsPath, LogLevel logLevel,
         return;
     }
 
+    s_logLevel = logLevel;
+
     QString logFileName;
     QDir settingsDir(settingsPath);
 
@@ -211,7 +214,6 @@ void Logging::initialize(const QString& settingsPath, LogLevel logLevel,
     // without the lock.
     g_logfile.setFileName(logFileName);
     g_logfile.open(QIODevice::WriteOnly | QIODevice::Text);
-    g_logLevel = logLevel;
     g_debugAssertBreak = debugAssertBreak;
 
     // Install the Qt message handler.

src/util/logging.h

@@ -5,27 +5,37 @@
 
 namespace mixxx {
 
+enum class LogLevel {
+    Critical = 0,
+    Warning = 1,
+    Info = 2,
+    Debug = 3,
+    Default = Warning,
+};
+
 class Logging {
   public:
-    enum class LogLevel {
-        Critical = 0,
-        Warning = 1,
-        Info = 2,
-        Debug = 3
-    };
-    static constexpr LogLevel kLogLevelDefault = LogLevel::Warning;
-    // Any debug statement starting with this prefix bypasses the --logLevel
-    // command line flags.
-    static constexpr const char* kControllerDebugPrefix = "CDBG";
-
     // These are not thread safe. Only call them on Mixxx startup and shutdown.
     static void initialize(const QString& settingsPath,
                            LogLevel logLevel,
                            bool debugAssertBreak);
     static void shutdown();
 
+    // Query the current log level
+    static LogLevel logLevel() {
+        return s_logLevel;
+    }
+    static bool enabled(LogLevel logLevel) {
+        return s_logLevel >= logLevel;
+    }
+    static bool debugEnabled() {
+        return enabled(LogLevel::Debug);
+    }
+
   private:
     Logging() = delete;
+
+    static LogLevel s_logLevel;
 };
 
 }  // namespace mixxx