Fix undefined behavior due to recursive mutex locking, and properly clear sensors when re-enumerating

AdaptiveBrightness/AdaptiveBrightness.ui

@@ -60,6 +60,9 @@
              <layout class="QGridLayout" name="gridLayout_9">
               <item row="1" column="1">
                <widget class="QSlider" name="monitorMaxBrightnessSlider">
+                <property name="value">
+                 <number>50</number>
+                </property>
                 <property name="orientation">
                  <enum>Qt::Horizontal</enum>
                 </property>
@@ -74,6 +77,9 @@
               </item>
               <item row="0" column="1">
                <widget class="QSlider" name="monitorMinBrightnessSlider">
+                <property name="value">
+                 <number>50</number>
+                </property>
                 <property name="orientation">
                  <enum>Qt::Horizontal</enum>
                 </property>
@@ -117,6 +123,9 @@
              <layout class="QGridLayout" name="gridLayout_2">
               <item row="0" column="0">
                <widget class="QSlider" name="monitorBrightnessSlider">
+                <property name="value">
+                 <number>50</number>
+                </property>
                 <property name="orientation">
                  <enum>Qt::Horizontal</enum>
                 </property>
@@ -160,6 +169,9 @@
            </item>
            <item row="0" column="1">
             <widget class="QSlider" name="monitorOverallMinBrightnessSlider">
+             <property name="value">
+              <number>50</number>
+             </property>
              <property name="orientation">
               <enum>Qt::Horizontal</enum>
              </property>
@@ -184,6 +196,9 @@
            </item>
            <item row="1" column="1">
             <widget class="QSlider" name="monitorOverallMaxBrightnessSlider">
+             <property name="value">
+              <number>50</number>
+             </property>
              <property name="orientation">
               <enum>Qt::Horizontal</enum>
              </property>
@@ -243,6 +258,9 @@
           <layout class="QGridLayout" name="gridLayout">
            <item row="0" column="0">
             <widget class="QSlider" name="monitorOverallBrightnessSlider">
+             <property name="value">
+              <number>50</number>
+             </property>
              <property name="orientation">
               <enum>Qt::Horizontal</enum>
              </property>

AdaptiveBrightness/log_to_file.hpp

@@ -17,7 +17,7 @@ static inline void rotateLog(const std::filesystem::path& logFileName, int numRo
     const auto generateRotateNames = [&]() {
         std::vector<QString> rotateFileNames = {QString().fromStdString(logFileName.string())};
 
-        for(size_t i = 1; i < numRotatingLogs; ++i) {
+        for(int i = 1; i < numRotatingLogs; ++i) {
             auto rotateName = logFileName.stem();
             rotateName += std::string(".") + std::to_string(i);
             rotateName += logFileName.extension();

AdaptiveBrightness/main_window.cpp

@@ -5,6 +5,7 @@
 
 #include <cmath>
 
+#include <QAbstractButton>
 #include <QCloseEvent>
 #include <QGroupBox>
 #include <QSlider>
@@ -18,16 +19,17 @@
 #include "sensor_driver.hpp"
 #include "utils.hpp"
 
-MainWindow::MainWindow(QWidget* parent) : QMainWindow(parent)
+MainWindow::MainWindow(QWidget* parent) : QMainWindow(parent), m_mtx(QMutex::RecursionMode::Recursive)
 {
     qDebug(ltr("Creating main window"));
 
     m_ui.setupUi(this);
 
-    m_sensors = enumerateSensors();
-    m_monitors = enumerateMonitors();
+    enumSensors();
+    enumMonitors();
 
     setupMonitorsTab();
+    setupSensorsTab();
 
     setupTrayIcon();
 
@@ -50,6 +52,35 @@ void MainWindow::closeEvent(QCloseEvent* event)
     event->ignore();
 }
 
+void MainWindow::enumSensors()
+{
+    m_sensors.clear();
+
+    auto sensors = enumerateSensors();
+
+    for(auto& sensor: sensors) {
+        m_sensors.push_back({std::move(sensor)});
+    }
+}
+
+void MainWindow::enumMonitors()
+{
+    m_mtx.lock();
+
+    m_ui.monitorDropdown->clear();
+    m_monitors.clear();
+
+    auto monitors = enumerateMonitors();
+
+    for(auto& monitor: monitors) {
+        const auto currentBrightness = monitor.getBrightness();
+        m_monitors.push_back({std::move(monitor), 0.f, 1.f, currentBrightness, false});
+        m_ui.monitorDropdown->addItem(m_monitors.back().driver.getName());
+    }
+
+    m_mtx.unlock();
+}
+
 void MainWindow::loadIcons()
 {
     qDebug(ltr("Loading icons"));
@@ -86,24 +117,113 @@ void MainWindow::loadIcons()
 }
 
 void MainWindow::setupMonitorsTab()
+{
+    connect(m_ui.monitorDropdown, QOverload<int>::of(&QComboBox::currentIndexChanged), this, &MainWindow::updateCurrentMonitorGUI);
+    connect(m_ui.monitorEnumerateButton, &QAbstractButton::clicked, [this](bool) { enumMonitors(); });
+    connect(m_ui.monitorResetButton, &QAbstractButton::clicked, [this](bool) { resetAllMonitorSettings(); });
+
+    setupLimitsSlider();
+    setupOverallLimitsSlider();
+
+    setupBrightness();
+    setupOverallBrightness();
+
+    resetAllMonitorSettings();
+}
+
+void MainWindow::setupLimitsSlider()
+{
+    connect(m_ui.monitorMinBrightnessSlider, &QSlider::valueChanged, [this](int value) {
+        m_ui.monitorMinBrightnessValueLabel->setText(tr("%1%").arg(value, 3));
+        if(value > m_ui.monitorMaxBrightnessSlider->value()) {
+            m_ui.monitorMaxBrightnessSlider->setValue(value);
+        }
+        const auto curIdx = m_ui.monitorDropdown->currentIndex();
+        m_monitors[curIdx].minBrightness = utils::map(value, 0, 100, 0.f, 1.f);
+    });
+    connect(m_ui.monitorMaxBrightnessSlider, &QSlider::valueChanged, [this](int value) {
+        m_ui.monitorMaxBrightnessValueLabel->setText(tr("%1%").arg(value, 3));
+        if(value < m_ui.monitorMinBrightnessSlider->value()) {
+            m_ui.monitorMinBrightnessSlider->setValue(value);
+        }
+        const auto curIdx = m_ui.monitorDropdown->currentIndex();
+        m_monitors[curIdx].maxBrightness = utils::map(value, 0, 100, 0.f, 1.f);
+    });
+}
+
+void MainWindow::setupOverallLimitsSlider()
+{
+    connect(m_ui.monitorOverallMinBrightnessSlider, &QSlider::valueChanged, [this](int value) {
+        m_ui.monitorOverallMinBrightnessValueLabel->setText(tr("%1%").arg(value, 3));
+        if(value > m_ui.monitorOverallMaxBrightnessSlider->value()) {
+            m_ui.monitorOverallMaxBrightnessSlider->setValue(value);
+        }
+        for(auto& monitor: m_monitors) {
+            monitor.minBrightness = utils::map(value, 0, 100, 0.f, 1.f);
+        }
+        updateCurrentMonitorGUI(m_ui.monitorDropdown->currentIndex());
+    });
+    connect(m_ui.monitorOverallMaxBrightnessSlider, &QSlider::valueChanged, [this](int value) {
+        m_ui.monitorOverallMaxBrightnessValueLabel->setText(tr("%1%").arg(value, 3));
+        if(value < m_ui.monitorOverallMinBrightnessSlider->value()) {
+            m_ui.monitorOverallMinBrightnessSlider->setValue(value);
+        }
+        for(auto& monitor: m_monitors) {
+            monitor.maxBrightness = utils::map(value, 0, 100, 0.f, 1.f);
+        }
+        updateCurrentMonitorGUI(m_ui.monitorDropdown->currentIndex());
+    });
+}
+
+void MainWindow::setupBrightness()
 {
     m_ui.monitorBrightnessSlider->setDisabled(true);
-    m_ui.monitorOverallBrightnessSlider->setDisabled(true);
-
-    m_ui.monitorOverrideCheck->setChecked(false);
-    connect(m_ui.monitorOverrideCheck, &QCheckBox::stateChanged,
-            [this](int state) { m_ui.monitorBrightnessSlider->setEnabled(state == Qt::CheckState::Checked); });
-
-    m_ui.monitorOverallOverrideCheck->setChecked(false);
-    connect(m_ui.monitorOverallOverrideCheck, &QCheckBox::stateChanged,
-            [this](int state) { m_ui.monitorOverallBrightnessSlider->setEnabled(state == Qt::CheckState::Checked); });
 
     connect(m_ui.monitorBrightnessSlider, &QSlider::valueChanged, [this](int value) {
-        const auto mappedBrightness = utils::map(value, 0, 100, 0.f, 1.f);
-        const auto newBrightness = std::clamp(mappedBrightness, 0.f, 1.f);
+        m_ui.monitorBrightnessValueLabel->setText(tr("%1%").arg(value, 3));
+        const auto newBrightness = utils::map(value, 0, 100, 0.f, 1.f);
         qDebug(ltr("Overriding brightness with %1").arg(newBrightness));
-        m_brightness = newBrightness;
+        const auto curIdx = m_ui.monitorDropdown->currentIndex();
+        m_monitors[curIdx].brightness = newBrightness;
     });
+
+    m_ui.monitorOverrideCheck->setChecked(false);
+    connect(m_ui.monitorOverrideCheck, &QCheckBox::stateChanged, [this](int state) {
+        m_ui.monitorBrightnessSlider->setEnabled(state == Qt::CheckState::Checked);
+        const auto curIdx = m_ui.monitorDropdown->currentIndex();
+        m_monitors[curIdx].manualOverride = state == Qt::CheckState::Checked;
+    });
+}
+
+void MainWindow::setupOverallBrightness()
+{
+    m_ui.monitorOverallBrightnessSlider->setDisabled(true);
+
+    connect(m_ui.monitorOverallBrightnessSlider, &QSlider::valueChanged, [this](int value) {
+        m_ui.monitorOverallBrightnessValueLabel->setText(tr("%1%").arg(value, 3));
+        const auto newBrightness = utils::map(value, 0, 100, 0.f, 1.f);
+        qDebug(ltr("Overriding overall brightness with %1").arg(newBrightness));
+        for(auto& monitor: m_monitors) {
+            if(monitor.manualOverride) {
+                monitor.brightness = newBrightness;
+            }
+        }
+        updateCurrentMonitorGUI(m_ui.monitorDropdown->currentIndex());
+    });
+
+    m_ui.monitorOverallOverrideCheck->setChecked(false);
+    connect(m_ui.monitorOverallOverrideCheck, &QCheckBox::stateChanged, [this](int state) {
+        m_ui.monitorOverallBrightnessSlider->setEnabled(state == Qt::CheckState::Checked);
+        for(auto& monitor: m_monitors) {
+            monitor.manualOverride = state == Qt::CheckState::Checked;
+        }
+        updateCurrentMonitorGUI(m_ui.monitorDropdown->currentIndex());
+    });
+}
+
+void MainWindow::setupSensorsTab()
+{
+    m_ui.tabs->removeTab(1);
 }
 
 void MainWindow::setupTrayIcon()
@@ -157,13 +277,33 @@ void MainWindow::setupCallbackTimer()
     qDebug(ltr("Callback timer started"));
 }
 
+void MainWindow::resetAllMonitorSettings()
+{
+    m_ui.monitorOverallMinBrightnessSlider->setValue(0);
+    m_ui.monitorOverallMaxBrightnessSlider->setValue(100);
+    m_ui.monitorOverallOverrideCheck->setCheckState(Qt::CheckState::Unchecked);
+}
+
+void MainWindow::updateCurrentMonitorGUI(int index)
+{
+    if(index == -1) {
+        return;
+    }
+
+    m_ui.monitorMinBrightnessSlider->setValue(utils::map(m_monitors[index].minBrightness, 0.f, 1.f, 0, 100));
+    m_ui.monitorMaxBrightnessSlider->setValue(utils::map(m_monitors[index].maxBrightness, 0.f, 1.f, 0, 100));
+    m_ui.monitorBrightnessSlider->setValue(utils::map(m_monitors[index].brightness, 0.f, 1.f, 0, 100));
+    m_ui.monitorOverrideCheck->setChecked(m_monitors[index].manualOverride);
+}
+
 void MainWindow::updateState()
 {
+    m_mtx.lock();
+
     qDebug(ltr("Updating state"));
 
     bool sensorError = false;
     bool monitorError = false;
-    size_t iconBrightness = 0;
 
     const auto getAverageSensorValue = [this](float& brightness) {
         if(m_sensors.empty()) {
@@ -173,8 +313,8 @@ void MainWindow::updateState()
         std::vector<float> values;
 
         for(auto& sensor: m_sensors) {
-            const auto sensorValues = sensor.readValues();
-            if(!sensor) {
+            const auto sensorValues = sensor.driver.readValues();
+            if(!sensor.driver) {
                 return false;
             }
             values.insert(values.end(), sensorValues.begin(), sensorValues.end());
@@ -184,35 +324,22 @@ void MainWindow::updateState()
         return true;
     };
 
-    const auto setAllMonitorsBrightness = [this](float& brightness) {
-        if(m_monitors.empty()) {
-            return false;
-        }
+    auto brightness = NAN;
 
-        bool errorOccurred = false;
-        for(auto& monitor: m_monitors) {
-            if(!monitor.setBrightness(brightness)) {
-                errorOccurred = true;
-            }
-        }
-        return !errorOccurred;
-    };
-
-    if(!getAverageSensorValue(m_brightness)) {
-        m_sensors = enumerateSensors();
+    if(!getAverageSensorValue(brightness)) {
+        enumSensors();
         qInfo(ltr("At least one sensor became invalid, re-enumerating sensors found %1 valid sensor(s)").arg(m_sensors.size()));
         sensorError = true;
     }
 
-    const auto roundedBrightness = std::round(utils::map(m_brightness, 0.f, 1.f, 0, 8));
-    iconBrightness = std::clamp<size_t>(roundedBrightness, 0, 8);
-
-    if(!setAllMonitorsBrightness(m_brightness)) {
-        m_monitors = enumerateMonitors();
+    if(!setMonitorBrightness(brightness)) {
+        enumMonitors();
         qInfo(ltr("At least one monitor became invalid, re-enumerating monitors found %1 valid monitor(s)").arg(m_monitors.size()));
         monitorError = true;
     }
 
+    const auto iconBrightness = utils::map(brightness, 0.f, 1.f, size_t{0}, size_t{8});
+
     if(!sensorError && !monitorError && m_trayIcon) {
         m_trayIcon->setIcon(*m_sensorStateIcons[iconBrightness]);
     }
@@ -224,10 +351,52 @@ void MainWindow::updateState()
     }
 
     if(m_trayIcon) {
-        m_trayIcon->setToolTip(tr("AdaptiveBrightness - %1%").arg(m_brightness * 100, 0, 'f', 0));
+        m_trayIcon->setToolTip(tr("AdaptiveBrightness - %1%").arg(brightness * 100, 0, 'f', 0));
     }
 
-    m_ui.monitorBrightnessSlider->setValue(utils::map(m_brightness, 0.f, 1.f, 0, 100));
+    m_mtx.unlock();
+}
+
+bool MainWindow::setMonitorBrightness(float& brightness)
+{
+    if(m_monitors.empty()) {
+        return false;
+    }
+
+    bool errorOccurred = false;
+    bool allMonitorsSameBrightness = true;
+
+    for(size_t i = 0; i < m_monitors.size(); ++i) {
+        if(!std::isnan(brightness) && !m_monitors[i].manualOverride) {
+            m_monitors[i].brightness = brightness;
+        }
+
+        m_monitors[i].brightness = std::clamp(m_monitors[i].brightness, m_monitors[i].minBrightness, m_monitors[i].maxBrightness);
+
+        if(i > 0 && m_monitors[i - 1].brightness != m_monitors[i].brightness) {
+            allMonitorsSameBrightness = false;
+        }
+
+        if(!m_monitors[i].driver.setBrightness(m_monitors[i].brightness)) {
+            errorOccurred = true;
+        }
+        else if(m_ui.monitorDropdown->currentIndex() == static_cast<int>(i)) {
+            updateCurrentMonitorGUI(static_cast<int>(i));
+        }
+    }
+
+    if(!errorOccurred) {
+        auto avgBrightness = 0.f;
+        std::for_each(m_monitors.begin(), m_monitors.end(), [&avgBrightness](const auto& mon) { avgBrightness += mon.brightness; });
+        avgBrightness /= m_monitors.size();
+        brightness = avgBrightness;
+    }
+
+    if(allMonitorsSameBrightness) {
+        m_ui.monitorOverallBrightnessSlider->setValue(utils::map(brightness, 0.f, 1.f, 0, 100));
+    }
+
+    return !errorOccurred;
 }
 
 void MainWindow::trayIconClicked(QSystemTrayIcon::ActivationReason reason)

AdaptiveBrightness/main_window.hpp

@@ -8,6 +8,7 @@
 #include <QIcon>
 #include <QMainWindow>
 #include <QMenu>
+#include <QMutex>
 #include <QSlider>
 #include <QSystemTrayIcon>
 #include <QThread>
@@ -28,14 +29,30 @@ class MainWindow : public QMainWindow {
   private:
     void closeEvent(QCloseEvent* event) override;
 
+    void enumSensors();
+    void enumMonitors();
+
     void loadIcons();
 
     void setupMonitorsTab();
+    void setupLimitsSlider();
+    void setupOverallLimitsSlider();
+    void setupBrightness();
+    void setupOverallBrightness();
+
+    void setupSensorsTab();
+
     void setupTrayIcon();
     void setupCallbackTimer();
 
+    void resetAllMonitorSettings();
+
+    void updateCurrentMonitorGUI(int index);
+
     void updateState();
 
+    bool setMonitorBrightness(float& brightness);
+
     void trayIconClicked(QSystemTrayIcon::ActivationReason reason);
 
     Ui::MainWindow m_ui;
@@ -51,9 +68,20 @@ class MainWindow : public QMainWindow {
 
     QThread* m_timerThread = nullptr;
     QTimer* m_timer = nullptr;
+    QMutex m_mtx;
 
-    float m_brightness = 0.5f;
+    struct SensorData {
+        Sensor driver;
+    };
 
-    std::vector<Sensor> m_sensors;
-    std::vector<Monitor> m_monitors;
+    struct MonitorData {
+        Monitor driver;
+        float minBrightness;
+        float maxBrightness;
+        float brightness;
+        bool manualOverride;
+    };
+
+    std::vector<SensorData> m_sensors;
+    std::vector<MonitorData> m_monitors;
 };

AdaptiveBrightness/monitor.hpp

@@ -57,6 +57,8 @@ class Monitor {
 
 #if defined(__clang__) || defined(__GNUC__)
 
+    #include <QString>
+
 class Monitor {
   public:
     Monitor();
@@ -64,6 +66,8 @@ class Monitor {
     bool setBrightness(float);
     float getBrightness();
 
+    QString getName() const;
+
     operator bool() const;
 
   private:

AdaptiveBrightness/monitor_linux.cpp

@@ -26,6 +26,12 @@ float Monitor::getBrightness()
     return NAN;
 }
 
+QString Monitor::getName() const
+{
+    qWarning(ltr("Getting monitor name not implemented"));
+    return "Unknown";
+}
+
 Monitor::operator bool() const
 {
     qWarning(ltr("Checking monitor validity not implemented"));

AdaptiveBrightness/monitor_win.cpp

@@ -64,8 +64,7 @@ bool Monitor::setBrightness(float percentage)
             return false;
     }
 
-    const auto mappedBrightness = utils::map(percentage, 0.f, 1.f, m_brightnessRange->first, m_brightnessRange->second);
-    const auto newBrightness = std::clamp<DWORD>(std::round(mappedBrightness), m_brightnessRange->first, m_brightnessRange->second);
+    const auto newBrightness = utils::map(percentage, 0.f, 1.f, m_brightnessRange->first, m_brightnessRange->second);
 
     if(!SetMonitorBrightness(m_physicalMonitor.handle, newBrightness)) {
         qCritical(ltr("Failed to set brightness of monitor '%1' with handle '0x%2' and physical handle '0x%3', error: %4")
@@ -110,7 +109,7 @@ float Monitor::getBrightness()
     m_brightnessRange = std::pair{minBrightness, maxBrightness};
     m_errorOccurred = false;
 
-    return std::clamp(utils::map(brightness, minBrightness, maxBrightness, 0.f, 1.f), 0.f, 1.f);
+    return utils::map(brightness, minBrightness, maxBrightness, 0.f, 1.f);
 }
 
 QString Monitor::getName() const

AdaptiveBrightness/sensor.cpp

@@ -47,8 +47,7 @@ std::vector<float> Sensor::readValues()
     std::vector<float> normalizedValues;
 
     for(const auto& value: values) {
-        const auto mappedValue = utils::map(value, m_range->first, m_range->second, 0.f, 1.f);
-        normalizedValues.push_back(std::clamp(mappedValue, 0.f, 1.f));
+        normalizedValues.push_back(utils::map(value, m_range->first, m_range->second, 0.f, 1.f));
     }
 
     return normalizedValues;

AdaptiveBrightness/utils.hpp

@@ -1,17 +1,30 @@
 #pragma once
 
+#include <algorithm>
 #include <array>
+#include <type_traits>
 
+#include <cmath>
 #include <cstddef>
 
 namespace utils {
 
 template<typename InType, typename OutType>
-auto map(InType value, InType inMin, InType inMax, OutType outMin, OutType outMax)
+auto interpolate(InType value, InType inMin, InType inMax, OutType outMin, OutType outMax)
 {
     return (value - inMin) * (outMax - outMin) / (inMax - inMin) + outMin;
 }
 
+template<typename InType, typename OutType>
+auto map(InType value, InType inMin, InType inMax, OutType outMin, OutType outMax)
+{
+    if constexpr(std::is_integral_v<OutType>) {
+        return std::clamp(static_cast<OutType>(std::round(interpolate(value, inMin, inMax, outMin, outMax))), outMin, outMax);
+    }
+
+    return std::clamp(static_cast<OutType>(interpolate(value, inMin, inMax, outMin, outMax)), outMin, outMax);
+}
+
 template<typename>
 struct array_size;
 

CMakeLists.txt

@@ -71,10 +71,16 @@ target_link_libraries(AdaptiveBrightness
                       Qt5::SerialPort
 )
 
+if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang|AppleClang")
+    target_link_libraries(AdaptiveBrightness
+                          stdc++fs
+    )
+endif()
+
 if(MSVC)
-target_link_libraries(AdaptiveBrightness
-                      Dxva2.lib
-)
+    target_link_libraries(AdaptiveBrightness
+                          Dxva2.lib
+    )
 endif()
 
 if(MSVC)

firmware/README.md

@@ -56,7 +56,7 @@ Found DFU: [0483:df11] ver=2200, devnum=57, cfg=1, intf=0, path="2-13.2.2", alt=
 Found DFU: [0483:df11] ver=2200, devnum=57, cfg=1, intf=0, path="2-13.2.2", alt=0, name="@Internal Flash  /0x08000000/032*0001Kg", serial="FFFFFFFEFFFF"
 ```
 
-The firmware can then be flashing using:
+The firmware can then be flashed using:
 
 ```console
 $ dfu-util -a 0 -s 0x08000000 -D AdaptiveBrightnessFirmware.bin