Rework show/hide to reduce the surface called from the SignalUpdate callback

Controllers/MSIMonitorController/MSIMonitorController.cpp

@@ -10,10 +10,31 @@
 |   SPDX-License-Identifier: GPL-2.0-or-later               |
 \*---------------------------------------------------------*/
 
+#include <chrono>
 #include <string.h>
+#include <thread>
 #include "MSIMonitorController.h"
 #include "StringUtils.h"
 
+/*---------------------------------------------------------*\
+| Feature reports are control transfers and this panel      |
+| holds one outstanding for tens of milliseconds.  Back     |
+| to back writes pin the shared control pipe at full        |
+| duty and starve other non periodic traffic: a device      |
+| with no interrupt OUT endpoint writes by SET_REPORT       |
+| and stops answering, while its isochronous endpoints      |
+| keep running.                                             |
+|                                                           |
+| Idle a fixed interval after each write to leave the       |
+| control pipe free.                                        |
+\*---------------------------------------------------------*/
+void MSIMonitorController::SendFeatureReport(uint8_t *data, size_t length)
+{
+    hid_send_feature_report(dev, data, length);
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(MSI_MONITOR_PIPE_IDLE_MS));
+}
+
 MSIMonitorController::MSIMonitorController(hid_device* dev_handle, const hid_device_info& info, std::string dev_name)
 {
     dev                 = dev_handle;
@@ -118,7 +139,7 @@ void MSIMonitorController::Set(uint8_t mode_value, const std::vector<RGBColor> c
     | Send the data (1 packet)                                  |
     \*---------------------------------------------------------*/
 
-    hid_send_feature_report(dev, data, MSI_MONITOR_PACKET_SIZE);
+    SendFeatureReport(data, MSI_MONITOR_PACKET_SIZE);
 }
 
 uint8_t MSIMonitorController::GetLayoutVersion()
@@ -238,5 +259,5 @@ void MSIMonitorController::SetMode72(uint8_t mode_value, uint8_t speed, uint8_t
     \*-----------------------------------------------------*/
     data[MSI_MONITOR_72_STORE_INDEX] = save ? 0x01 : 0x00;
 
-    hid_send_feature_report(dev, data, MSI_MONITOR_72_PACKET_SIZE);
+    SendFeatureReport(data, MSI_MONITOR_72_PACKET_SIZE);
 }

Controllers/MSIMonitorController/MSIMonitorController.h

@@ -19,6 +19,15 @@
 #define MSI_MONITOR_LEDS 9
 #define MSI_MONITOR_PACKET_SIZE 78
 
+/*---------------------------------------------------------*\
+| This panel holds each feature report outstanding for      |
+| tens of ms while its MCU applies the frame, pinning the   |
+| shared control pipe.  Idle briefly after each write so    |
+| other non periodic traffic on the bus is not starved.     |
+| See SendFeatureReport.                                    |
+\*---------------------------------------------------------*/
+#define MSI_MONITOR_PIPE_IDLE_MS   5
+
 /*---------------------------------------------------------*\
 | 0x72 layout (dual control block, e.g. MPG 322URX QD-OLED) |
 \*---------------------------------------------------------*/
@@ -132,6 +141,8 @@ public:
     void SetMode72(uint8_t mode_value, uint8_t speed, uint8_t brightness, RGBColor color1, RGBColor color2, bool user_palette, bool save, const std::vector<RGBColor>& led_colors, bool fill_both_arrays);
 
 private:
+    void SendFeatureReport(uint8_t *data, size_t length);
+
     hid_device *dev;
     std::string description;
     std::string location;

Controllers/SteelSeriesController/SteelSeriesApexBaseController.h

@@ -47,7 +47,7 @@ enum
 enum
 {
     APEX_PACKET_LENGTH                  = 643,
-    APEX_9_PACKET_LENGTH                = 513,      /* Test if required           */
+    APEX_9_PACKET_LENGTH                = 513,      /* Apex 9 direct report size  */
     APEX_PACKET_ID_PROFILE              = 0x89,     /* Profile                    */
     APEX_PACKET_ID_FIRMWARE             = 0x90,     /* Firmware version           */
     APEX_GEN1_PACKET_ID_DIRECT          = 0x3A,     /* Direct mode                */

Controllers/SteelSeriesController/SteelSeriesApexController/SteelSeriesApexController.cpp

@@ -36,6 +36,19 @@ static unsigned int keys[] =
     0x63, 0xFB
 };
 
+/*---------------------------------------------------------*\
+| Models whose direct lighting feature report is shorter    |
+| than APEX_PACKET_LENGTH.  Sending the wrong report size   |
+| makes the keyboard stall the control transfer, so any     |
+| model listed here would silently ignore lighting updates  |
+| if the default length were used.                          |
+\*---------------------------------------------------------*/
+inline static const std::map<unsigned short, unsigned int> direct_packet_length_map =
+{
+    {STEELSERIES_APEX_9_TKL_PID,             APEX_9_PACKET_LENGTH},
+    {STEELSERIES_APEX_9_MINI_PID,            APEX_9_PACKET_LENGTH},
+};
+
 inline static const std::map<unsigned short, protocol_quirk> protocol_map =
 {
     {STEELSERIES_APEX_9_TKL_PID,             APEX_GEN2},
@@ -53,10 +66,15 @@ SteelSeriesApexController::SteelSeriesApexController(hid_device* dev_handle, ste
     proto_type = type;
     kbd_quirk = APEX_GEN1;
     device_pid = pid;
+    direct_packet_length = APEX_PACKET_LENGTH;
     if(protocol_map.count(device_pid) > 0)
     {
         kbd_quirk = protocol_map.at(device_pid);
     }
+    if(direct_packet_length_map.count(device_pid) > 0)
+    {
+        direct_packet_length = direct_packet_length_map.at(device_pid);
+    }
     SendInitialization();
 }
 
@@ -104,6 +122,20 @@ void SteelSeriesApexController::SetLEDsDirect(std::vector<RGBColor> colors)
     int num_keys = sizeof(keys) / sizeof(*keys);
     unsigned char packet_id = APEX_GEN1_PACKET_ID_DIRECT;
 
+    /*-----------------------------------------------------*\
+    | Never write past the end of the report or past        |
+    | the end of the color vector                           |
+    \*-----------------------------------------------------*/
+    if((unsigned int)num_keys > colors.size())
+    {
+        num_keys = (int)colors.size();
+    }
+
+    if((unsigned int)((num_keys * 4) + 3) > direct_packet_length)
+    {
+        num_keys = (int)((direct_packet_length - 3) / 4);
+    }
+
     if(kbd_quirk >= APEX_GEN2)
     {
         /*-------------------------------------------------*\
@@ -134,7 +166,7 @@ void SteelSeriesApexController::SetLEDsDirect(std::vector<RGBColor> colors)
     \*-----------------------------------------------------*/
     buf[0x00]   = 0;
     buf[0x01]   = packet_id;
-    buf[0x02]   = kbd_quirk ? (unsigned char)colors.size() : num_keys;
+    buf[0x02]   = (unsigned char)num_keys;
 
     /*-----------------------------------------------------*\
     | Fill in color data                                    |
@@ -150,7 +182,12 @@ void SteelSeriesApexController::SetLEDsDirect(std::vector<RGBColor> colors)
     /*-----------------------------------------------------*\
     | Send packet                                           |
     \*-----------------------------------------------------*/
-    hid_send_feature_report(dev, buf, APEX_PACKET_LENGTH);
+    int result = hid_send_feature_report(dev, buf, direct_packet_length);
+
+    if(result < 0)
+    {
+        LOG_DEBUG("[%s] Direct lighting update failed, %u byte report", name.c_str(), direct_packet_length);
+    }
 }
 
 /*---------------------------------------------------------*\

Controllers/SteelSeriesController/SteelSeriesApexController/SteelSeriesApexController.h

@@ -34,4 +34,6 @@ private:
     void SelectProfile(unsigned char profile);
     void SendInitialization();
     void SendDeinitialization();
+
+    unsigned int direct_packet_length;
 };

Controllers/ValveSteamMachineController/RGBController_ValveSteamMachine_Linux.cpp

@@ -46,6 +46,14 @@ RGBController_ValveSteamMachine::RGBController_ValveSteamMachine(ValveSteamMachi
         unsigned int effect_flags;
         std::string effect_name = effects[effect_idx];
 
+        /*-------------------------------------------------*\
+        | Set active_mode if this effect is active          |
+        \*-------------------------------------------------*/
+        if(effect_name == controller->GetEffect())
+        {
+            active_mode         = effect_idx;
+        }
+
         /*-------------------------------------------------*\
         | Determine flags and color mode for effect         |
         \*-------------------------------------------------*/
@@ -93,10 +101,10 @@ RGBController_ValveSteamMachine::RGBController_ValveSteamMachine(ValveSteamMachi
         new_mode.color_mode     = effect_color_mode;
         new_mode.speed_min      = 20;
         new_mode.speed_max      = 0;
-        new_mode.speed          = 10;
+        new_mode.speed          = controller->GetDelay();
         new_mode.brightness_min = 0;
         new_mode.brightness_max = 0xFF;
-        new_mode.brightness     = 0x38;
+        new_mode.brightness     = controller->GetBrightness();
         modes.push_back(new_mode);
     }
 

Controllers/ValveSteamMachineController/ValveSteamMachineController_Linux.cpp

@@ -11,7 +11,6 @@
 
 #include "ValveSteamMachineController_Linux.h"
 #include <fstream>
-#include <sstream>
 #include <algorithm>
 
 ValveSteamMachineController::ValveSteamMachineController(std::string dev_name)
@@ -116,6 +115,65 @@ std::vector<std::string> ValveSteamMachineController::GetAvailableEffects()
     return(available_effects);
 }
 
+unsigned int ValveSteamMachineController::GetBrightness()
+{
+    if(led_brightness_scale.is_open())
+    {
+        std::ifstream brightness_file(led_paths[0] + "brightness_scale");
+
+        if(brightness_file.good())
+        {
+            unsigned int brightness = 0;
+            brightness_file >> std::hex >> brightness;
+            brightness_file.close();
+            return(brightness);
+        }
+    }
+
+    return(0);
+}
+
+unsigned int ValveSteamMachineController::GetDelay()
+{
+    if(led_delay.is_open())
+    {
+        std::ifstream delay_file(led_paths[0] + "delay");
+
+        if(delay_file.good())
+        {
+            unsigned int delay = 0;
+            delay_file >> std::hex >> delay;
+            delay_file.close();
+            return(delay);
+        }
+    }
+
+    return(0);
+}
+
+std::string ValveSteamMachineController::GetEffect()
+{
+    /*-----------------------------------------------------*\
+    | Reading the effect on one LED reflects the active     |
+    | effect for all LEDs on the device, so we only need to |
+    | read it from the first LED.                           |
+    \*-----------------------------------------------------*/
+    if(led_effect.size() > 0)
+    {
+        std::ifstream effect_file(led_paths[0] + "effect");
+
+        if(effect_file.good())
+        {
+            std::string effect;
+            std::getline(effect_file, effect);
+            effect_file.close();
+            return(effect);
+        }
+    }
+
+    return("");
+}
+
 void ValveSteamMachineController::ReadAvailableEffects(std::string first_led_path)
 {
     std::ifstream effect_index_file(first_led_path + "effect_index");
@@ -132,20 +190,9 @@ void ValveSteamMachineController::ReadAvailableEffects(std::string first_led_pat
     | read, and is used to set the active effect when       |
     | written.                                              |
     \*-----------------------------------------------------*/
-    std::string content;
-    std::string line;
-
-    while(std::getline(effect_index_file, line))
-    {
-        content += line + " ";
-    }
-
-    effect_index_file.close();
-
-    std::istringstream stream(content);
     std::string token;
 
-    while(stream >> token)
+    while(effect_index_file >> token)
     {
         /*-------------------------------------------------*\
         | Add to list if not already present                |
@@ -176,22 +223,6 @@ void ValveSteamMachineController::SetLEDColor(unsigned int led_idx, unsigned cha
     led_multi_intensity[led_idx].flush();
 }
 
-void ValveSteamMachineController::SetLEDEffect(unsigned int led_idx, std::string effect)
-{
-    if(led_idx >= led_paths.size())
-    {
-        return;
-    }
-
-    if(led_effect.size() <= led_idx)
-    {
-        return;
-    }
-
-    led_effect[led_idx].write(effect.c_str(), effect.length());
-    led_effect[led_idx].flush();
-}
-
 void ValveSteamMachineController::SetEffect(std::string effect)
 {
     /*-----------------------------------------------------*\
@@ -224,4 +255,4 @@ void ValveSteamMachineController::SetDelay(unsigned int delay)
         led_delay.write(delay_str.c_str(), delay_str.length());
         led_delay.flush();
     }
-}
+}

Controllers/ValveSteamMachineController/ValveSteamMachineController_Linux.h

@@ -29,16 +29,20 @@ public:
     std::string                 GetLEDPath(unsigned int led_idx);
 
     std::vector<std::string>    GetAvailableEffects();
+    unsigned int                GetBrightness();
+    unsigned int                GetDelay();
+    std::string                 GetEffect();
+
     bool                        SupportsEffects();
 
     void                        SetLEDColor(unsigned int led_idx, unsigned char red, unsigned char grn, unsigned char blu);
-    void                        SetLEDEffect(unsigned int led_idx, std::string effect);
     void                        SetEffect(std::string effect);
     void                        SetBrightness(unsigned int brightness);
     void                        SetDelay(unsigned int delay);
 
 private:
     std::string                 name;
+
     std::vector<std::string>    led_paths;
     std::vector<std::ofstream>  led_brightness;
     std::vector<std::ofstream>  led_multi_intensity;

DetectionManager.cpp

@@ -31,6 +31,11 @@ using namespace std::chrono_literals;
 \*---------------------------------------------------------*/
 const char* DETECTIONMANAGER = "DetectionManager";
 
+/*---------------------------------------------------------*\
+| Upper bound for WaitForDetection(), in milliseconds       |
+\*---------------------------------------------------------*/
+#define DETECTION_WAIT_TIMEOUT_MS   60000
+
 /*---------------------------------------------------------*\
 | Warning Dialog Strings                                    |
 \*---------------------------------------------------------*/
@@ -502,6 +507,16 @@ void DetectionManager::BeginDetection()
     \*-----------------------------------------------------*/
     if(detection_ready)
     {
+        /*-------------------------------------------------*\
+        | Flag detection as in progress here rather than in |
+        | the background thread.  WaitForDetection() must   |
+        | not be able to return between this call and the   |
+        | thread actually starting, or a caller that only   |
+        | wants the finished device list, such as the CLI,  |
+        | gets an empty one.                                |
+        \*-------------------------------------------------*/
+        detection_in_progress = true;
+
         RunInBackgroundThread(std::bind(&DetectionManager::BackgroundDetectDevices, this));
     }
 }
@@ -518,6 +533,32 @@ std::string DetectionManager::GetDetectionString()
 
 void DetectionManager::WaitForDetection()
 {
+    /*-----------------------------------------------------*\
+    | Taking the mutex is not enough on its own: detection  |
+    | runs in a background thread and there is a window     |
+    | between BeginDetection() and that thread taking the   |
+    | mutex where it would be free.  Wait for the in        |
+    | progress flag to clear first, then take the mutex to  |
+    | make sure the detection thread is done.               |
+    \*-----------------------------------------------------*/
+    /*-----------------------------------------------------*\
+    | Bounded, so that a detection that never starts cannot |
+    | hang the caller forever: waiting too long is a bug,   |
+    | but blocking startup indefinitely would be worse      |
+    \*-----------------------------------------------------*/
+    unsigned int waited_ms = 0;
+
+    while(detection_in_progress.load() && waited_ms < DETECTION_WAIT_TIMEOUT_MS)
+    {
+        std::this_thread::sleep_for(std::chrono::milliseconds(10));
+        waited_ms += 10;
+    }
+
+    if(detection_in_progress.load())
+    {
+        LOG_WARNING("[%s] Gave up waiting for detection after %u ms", DETECTIONMANAGER, waited_ms);
+    }
+
     DetectDevicesMutex.lock();
     DetectDevicesMutex.unlock();
 }

NetworkClient.cpp

@@ -296,14 +296,14 @@ void NetworkClient::StopClient()
     /*-----------------------------------------------------*\
     | Close the ProfileManager listen thread                |
     \*-----------------------------------------------------*/
-    if(profilemanager_thread->thread)
+    if(profilemanager_thread)
     {
         profilemanager_thread->online = false;
         profilemanager_thread->start_cv.notify_all();
         profilemanager_thread->thread->join();
         delete profilemanager_thread->thread;
-        profilemanager_thread->thread = nullptr;
         delete profilemanager_thread;
+        profilemanager_thread = nullptr;
     }
 
     /*-----------------------------------------------------*\

NetworkServer.cpp

@@ -567,14 +567,14 @@ void NetworkServer::StopServer()
     /*-----------------------------------------------------*\
     | Close the ProfileManager listen thread                |
     \*-----------------------------------------------------*/
-    if(profilemanager_thread->thread)
+    if(profilemanager_thread)
     {
         profilemanager_thread->online = false;
         profilemanager_thread->start_cv.notify_all();
         profilemanager_thread->thread->join();
         delete profilemanager_thread->thread;
-        profilemanager_thread->thread = nullptr;
         delete profilemanager_thread;
+        profilemanager_thread = nullptr;
     }
 
     /*-----------------------------------------------------*\

RGBController/RGBController.cpp

@@ -70,11 +70,6 @@
 
 using namespace std::chrono_literals;
 
-/*---------------------------------------------------------*\
-| One counter per thread, shared across all controllers.    |
-\*---------------------------------------------------------*/
-thread_local unsigned int RGBController::SignalCallDepth = 0;
-
 RGBController::RGBController()
 {
     /*-----------------------------------------------------*\
@@ -90,6 +85,9 @@ RGBController::RGBController()
     flags               = 0;
     type                = DEVICE_TYPE_UNKNOWN;
 
+    SignalCalls         = 0;
+    SignalShutdown      = false;
+
     /*-----------------------------------------------------*\
     | Initialize device thread                              |
     \*-----------------------------------------------------*/
@@ -1841,27 +1839,10 @@ void RGBController::RegisterUpdateCallback(RGBControllerCallback new_callback, v
     UpdateMutex.unlock();
 }
 
-/*---------------------------------------------------------*\
-| Wait until no callback call is in flight, so the          |
-| caller may free the callback owner once this returns.     |
-| Skipped when the caller is itself inside a call on        |
-| this thread (see SignalCallDepth).                        |
-\*---------------------------------------------------------*/
-void RGBController::WaitSignalCalls()
-{
-    if(SignalCallDepth != 0)
-    {
-        return;
-    }
-
-    std::unique_lock<std::mutex> wait_lock(SignalMutex);
-    SignalCallsDone.wait(wait_lock, [this]{ return SignalCalls == 0; });
-}
-
 void RGBController::UnregisterUpdateCallback(void * callback_arg)
 {
     UpdateMutex.lock();
-    for(unsigned int callback_idx = 0; callback_idx < UpdateCallbackArgs.size(); callback_idx++ )
+    for(unsigned int callback_idx = 0; callback_idx < UpdateCallbackArgs.size(); callback_idx++)
     {
         if(UpdateCallbackArgs[callback_idx] == callback_arg)
         {
@@ -1872,10 +1853,21 @@ void RGBController::UnregisterUpdateCallback(void * callback_arg)
     UpdateMutex.unlock();
 
     /*-----------------------------------------------------*\
-    | Wait for in-flight calls so the caller may free the   |
-    | callback owner once this returns                      |
+    | If this is executing from within any controller's     |
+    | callback, return immediately rather than waiting for  |
+    | callback completion to avoid deadlock.                |
     \*-----------------------------------------------------*/
-    WaitSignalCalls();
+    if(SignalCalls != 0)
+    {
+        return;
+    }
+
+    /*-----------------------------------------------------*\
+    | Otherwise, wait for all currently active callbacks to |
+    | complete before returning                             |
+    \*-----------------------------------------------------*/
+    std::unique_lock<std::mutex> wait_lock(SignalMutex);
+    SignalCallsDone.wait(wait_lock, [this]{ return SignalCalls == 0; });
 }
 
 void RGBController::ClearCallbacks()
@@ -1886,62 +1878,81 @@ void RGBController::ClearCallbacks()
     UpdateMutex.unlock();
 
     /*-----------------------------------------------------*\
-    | Wait for in-flight calls so the caller may free the   |
-    | callback owners once this returns                     |
+    | If this is executing from within any controller's     |
+    | callback, return immediately rather than waiting for  |
+    | callback completion to avoid deadlock.                |
     \*-----------------------------------------------------*/
-    WaitSignalCalls();
+    if(SignalCalls != 0)
+    {
+        return;
+    }
+
+    /*-----------------------------------------------------*\
+    | Otherwise, wait for all currently active callbacks to |
+    | complete before returning                             |
+    \*-----------------------------------------------------*/
+    std::unique_lock<std::mutex> wait_lock(SignalMutex);
+    SignalCallsDone.wait(wait_lock, [this]{ return SignalCalls == 0; });
 }
 
 void RGBController::SignalUpdate(unsigned int update_reason)
 {
-    /*-----------------------------------------------------*\
-    | Copy the callback list under UpdateMutex and mark a   |
-    | call in flight, then release before invoking so no    |
-    | lock is held across callback code. A frozen           |
-    | controller (shutting down) calls nothing.             |
-    \*-----------------------------------------------------*/
     std::vector<RGBControllerCallback>  callbacks;
     std::vector<void *>                 callback_args;
 
+    /*-----------------------------------------------------*\
+    | Lock the update mutex while obtaining a copy of the   |
+    | callbacks, but release it before actually calling     |
+    | them so that any call to Register/Unregister from     |
+    | within the callback doesn't deadlock.                 |
+    \*-----------------------------------------------------*/
     UpdateMutex.lock();
+
+    /*-----------------------------------------------------*\
+    | Lock the signal mutex and increment the signal call   |
+    | count.  Return immediately if the controller has been |
+    | shut down.                                            |
+    \*-----------------------------------------------------*/
+    SignalMutex.lock();
+    if(SignalShutdown)
     {
-        std::lock_guard<std::mutex> call_lock(SignalMutex);
-
-        if(SignalFrozen)
-        {
-            UpdateMutex.unlock();
-            return;
-        }
-
-        SignalCalls++;
+        UpdateMutex.unlock();
+        return;
     }
+    SignalCalls++;
+    SignalMutex.unlock();
+
+    /*-----------------------------------------------------*\
+    | Copy the list of callbacks and unlock the update      |
+    | mutex                                                 |
+    \*-----------------------------------------------------*/
     callbacks       = UpdateCallbacks;
     callback_args   = UpdateCallbackArgs;
+
     UpdateMutex.unlock();
 
     /*-----------------------------------------------------*\
-    | Invoke the copied callbacks with no lock held         |
+    | Invoke the copied callbacks                           |
     \*-----------------------------------------------------*/
-    SignalCallDepth++;
     for(unsigned int callback_idx = 0; callback_idx < callbacks.size(); callback_idx++)
     {
         callbacks[callback_idx](callback_args[callback_idx], update_reason, this);
     }
-    SignalCallDepth--;
 
     /*-----------------------------------------------------*\
-    | Wake anyone waiting once the last call is out         |
+    | Decrement the signal call count once callbacks have   |
+    | been called.  Notify anyone waiting on signal calls   |
+    | to be done.                                           |
     \*-----------------------------------------------------*/
+    SignalMutex.lock();
+    SignalCalls--;
+
+    if(SignalCalls == 0)
     {
-        std::lock_guard<std::mutex> call_lock(SignalMutex);
-
-        SignalCalls--;
-
-        if(SignalCalls == 0)
-        {
-            SignalCallsDone.notify_all();
-        }
+        SignalCallsDone.notify_all();
     }
+
+    SignalMutex.unlock();
 }
 
 /*---------------------------------------------------------*\
@@ -1962,19 +1973,17 @@ void RGBController::Shutdown()
     AccessMutex.lock();
 
     /*-----------------------------------------------------*\
-    | Freeze the controller so no further SignalUpdate      |
-    | calls its callbacks, then wait for any call already   |
-    | in flight. Nothing is held across teardown, so a late |
-    | or cross-controller SignalUpdate returns at the       |
-    | frozen check instead of blocking forever.             |
+    | Shut down signal handling and unlock mutexes          |
     \*-----------------------------------------------------*/
     UpdateMutex.lock();
-    {
-        std::lock_guard<std::mutex> call_lock(SignalMutex);
-        SignalFrozen = true;
-    }
+    SignalMutex.lock();
+    SignalShutdown = true;
+    SignalMutex.unlock();
     UpdateMutex.unlock();
 
+    /*-----------------------------------------------------*\
+    | Wait for any remaining signal calls to complete       |
+    \*-----------------------------------------------------*/
     std::unique_lock<std::mutex> wait_lock(SignalMutex);
     SignalCallsDone.wait(wait_lock, [this]{ return SignalCalls == 0; });
 }

RGBController/RGBController.h

@@ -320,37 +320,10 @@ private:
     std::vector<RGBControllerCallback>  UpdateCallbacks;
     std::vector<void *>                 UpdateCallbackArgs;
 
-    /*-----------------------------------------------------*\
-    | Callback call state, guarded by SignalMutex. The      |
-    | mutex is never held across callback code: a call      |
-    | copies the callback list, releases, then invokes.     |
-    |                                                       |
-    | SignalCalls  : calls in flight for this controller.   |
-    | SignalFrozen : set at Shutdown; no further calls run, |
-    |                so a late SignalUpdate returns rather  |
-    |                than blocking on teardown.             |
-    |                                                       |
-    | Unregister/ClearCallbacks/Shutdown remove or freeze   |
-    | the list then wait out any call already running, so   |
-    | on return a removed callback is not executing and     |
-    | cannot start again, and its owner may be freed.       |
-    \*-----------------------------------------------------*/
     std::mutex                          SignalMutex;
     std::condition_variable             SignalCallsDone;
-    unsigned int                        SignalCalls     = 0;
-    bool                                SignalFrozen    = false;
-
-    void                                WaitSignalCalls();
-
-    /*-----------------------------------------------------*\
-    | Depth of SignalUpdate callback calls on this thread.  |
-    | Per-thread and shared across all controllers, not     |
-    | per-instance, so WaitSignalCalls can tell when a      |
-    | callback is unregistering from inside its own call    |
-    | and must not wait on itself. Not guarded by           |
-    | SignalMutex; thread_local needs no lock.              |
-    \*-----------------------------------------------------*/
-    static thread_local unsigned int    SignalCallDepth;
+    unsigned int                        SignalCalls;
+    bool                                SignalShutdown;
 
     /*-----------------------------------------------------*\
     | Private path used internally by DetectionManager      |

ResourceManager.cpp

@@ -811,15 +811,27 @@ void ResourceManager::SignalResourceManagerUpdate(unsigned int update_reason)
         server->SignalResourceManagerUpdate(update_reason);
     }
 
+    /*-----------------------------------------------------*\
+    | Snapshot the callback list, then invoke unlocked. A   |
+    | blocking callback must not hold the mutex: the GUI's  |
+    | blocking device list callback waits forever once the  |
+    | event loop has exited at shutdown, and a held mutex   |
+    | then deadlocks the unregister in the dialog           |
+    | destructor. A callback may still fire once after its  |
+    | unregister returns.                                   |
+    \*-----------------------------------------------------*/
     ResourceManagerCallbackMutex.lock();
 
-    for(std::size_t callback_idx = 0; callback_idx < ResourceManagerCallbacks.size(); callback_idx++)
-    {
-        ResourceManagerCallbacks[callback_idx](ResourceManagerCallbackArgs[callback_idx], update_reason);
-    }
+    std::vector<ResourceManagerCallback>    callbacks       = ResourceManagerCallbacks;
+    std::vector<void *>                     callback_args   = ResourceManagerCallbackArgs;
 
     ResourceManagerCallbackMutex.unlock();
 
+    for(std::size_t callback_idx = 0; callback_idx < callbacks.size(); callback_idx++)
+    {
+        callbacks[callback_idx](callback_args[callback_idx], update_reason);
+    }
+
     LOG_TRACE("[%s] ResourceManager update signalled: %d", RESOURCEMANAGER, update_reason);
 }
 

cli.cpp

@@ -487,7 +487,7 @@ void OptionListDevices(std::vector<RGBController *>& rgb_controllers)
         \*---------------------------------------------------------*/
         if(!controller->GetVersion().empty())
         {
-            std::cout << "  Version:        " << controller->GetLocation() << std::endl;
+            std::cout << "  Version:        " << controller->GetVersion() << std::endl;
         }
 
         /*---------------------------------------------------------*\

qt/OpenRGBDevicePage/OpenRGBDevicePage.cpp

@@ -699,6 +699,8 @@ void OpenRGBDevicePage::UpdateLEDList()
                 /*-----------------------------------------*\
                 | Clear LED box                             |
                 \*-----------------------------------------*/
+                int previous_color_index = ui->LEDBox->currentIndex();
+
                 ui->LEDBox->blockSignals(true);
                 ui->LEDBox->clear();
 
@@ -714,9 +716,19 @@ void OpenRGBDevicePage::UpdateLEDList()
                 }
 
                 /*-----------------------------------------*\
-                | Enable LED box                            |
+                | Keep the selected color if it is still in |
+                | range.  Applying a color signals a mode   |
+                | update that rebuilds this box; without    |
+                | this the selection would jump back to 0.  |
                 \*-----------------------------------------*/
-                ui->LEDBox->setCurrentIndex(0);
+                if((previous_color_index > 0) && (previous_color_index < (int)mode_colors))
+                {
+                    ui->LEDBox->setCurrentIndex(previous_color_index);
+                }
+                else
+                {
+                    ui->LEDBox->setCurrentIndex(0);
+                }
                 ui->LEDBox->setEnabled(true);
                 ui->LEDBox->blockSignals(false);
             }
@@ -1019,9 +1031,23 @@ void OpenRGBDevicePage::UpdateLEDUi()
                     color = device->GetModeColor(selected_mode, index);
                 }
 
-                current_color.setRgb(RGBGetRValue(color), RGBGetGValue(color), RGBGetBValue(color));
+                /*-----------------------------------------*\
+                | Repaint the picker only when the stored   |
+                | color differs from what it already shows. |
+                | A self applied color returns as the same  |
+                | RGB, and repainting from that round trip  |
+                | would drop the hue being edited at a low  |
+                | value.  An external mode change brings a  |
+                | different color and still updates.        |
+                \*-----------------------------------------*/
+                RGBColor shown_color = ToRGBColor(current_color.red(), current_color.green(), current_color.blue());
 
-                UpdateColorUi();
+                if(color != shown_color)
+                {
+                    current_color.setRgb(RGBGetRValue(color), RGBGetGValue(color), RGBGetBValue(color));
+
+                    UpdateColorUi();
+                }
             }
             break;
         default:
@@ -2000,7 +2026,7 @@ void OpenRGBDevicePage::UpdateInterface(unsigned int update_reason)
     case RGBCONTROLLER_UPDATE_REASON_HIDDEN:
     case RGBCONTROLLER_UPDATE_REASON_UNHIDDEN:
     case RGBCONTROLLER_UPDATE_REASON_CONFIGUREDEVICE:
-        emit RefreshList();
+        emit ShowHideList();
         break;
 
     case RGBCONTROLLER_UPDATE_REASON_UPDATELEDS:

qt/OpenRGBDevicePage/OpenRGBDevicePage.h

@@ -129,5 +129,7 @@ private slots:
 
 signals:
     void RefreshList();
+    void ShowHideList();
+
     void SetAllDevices(unsigned char red, unsigned char green, unsigned char blue);
 };

qt/OpenRGBDialog/OpenRGBDialog.cpp

@@ -38,6 +38,7 @@
 #include <QCheckBox>
 #include <QScreen>
 #include <QSpinBox>
+#include <QThread>
 
 #include <string>
 #include <functional>
@@ -106,7 +107,15 @@ static void OpenRGBDialogResourceManagerCallback(void * this_ptr, unsigned int u
             break;
 
         case RESOURCEMANAGER_UPDATE_REASON_DEVICE_LIST_UPDATED:
-            QMetaObject::invokeMethod(this_obj, "onDeviceListUpdated", Qt::BlockingQueuedConnection);
+            if(QThread::currentThread() != qApp->thread())
+            {
+                printf("invoking onDeviceListUpdated\n");
+                QMetaObject::invokeMethod(this_obj, "onDeviceListUpdated", Qt::BlockingQueuedConnection);
+            }
+            else
+            {
+                this_obj->onDeviceListUpdated();
+            }
             break;
     }
 }
@@ -577,6 +586,14 @@ void OpenRGBDialog::closeEvent(QCloseEvent *event)
     }
     else
     {
+        /*-------------------------------------------------*\
+        | Stop receiving resource manager callbacks before  |
+        | the event loop dies: plugin teardown signals from |
+        | worker threads would post blocking calls no loop  |
+        | will ever service.                                |
+        \*-------------------------------------------------*/
+        ResourceManager::get()->UnregisterResourceManagerCallback(OpenRGBDialogResourceManagerCallback, this);
+
         plugin_manager->UnloadPlugins();
 
         if(ResourceManager::get()->GetProfileManager()->LoadAutoProfileExit())
@@ -952,6 +969,84 @@ void OpenRGBDialog::ClearDevicesList()
     }
 }
 
+void OpenRGBDialog::UpdateDevicesListShowHide()
+{
+    /*-----------------------------------------------------*\
+    | Loop through each controller in the tab list and hide |
+    | any controller marked as hidden.                      |
+    \*-----------------------------------------------------*/
+    for(int tab_idx = 0; tab_idx < ui->DevicesTabBar->count(); tab_idx++)
+    {
+        /*-------------------------------------------------*\
+        | Get a pointer to the page at this index in the    |
+        | tab bar                                           |
+        \*-------------------------------------------------*/
+        QWidget* page = ui->DevicesTabBar->widget(tab_idx);
+
+        /*-------------------------------------------------*\
+        | Verify this page is an OpenRGBDevicePage          |
+        \*-------------------------------------------------*/
+        if(dynamic_cast<OpenRGBDevicePage*>(page) != nullptr)
+        {
+            /*---------------------------------------------*\
+            | If the controller for this page is hidden,    |
+            | remove the page from the tab bar and store it |
+            | in the hidden pages vector                    |
+            \*---------------------------------------------*/
+            if(((OpenRGBDevicePage*)page)->GetController()->GetHidden())
+            {
+                hidden_pages.push_back((OpenRGBDevicePage*)page);
+                ui->DevicesTabBar->removeTab(tab_idx);
+
+                /*-----------------------------------------*\
+                | Decrement tab index to account for        |
+                | removing tab                              |
+                \*-----------------------------------------*/
+                tab_idx--;
+            }
+        }
+    }
+
+    /*-----------------------------------------------------*\
+    | Loop through each controller in the hidden pages      |
+    | vector and restore any controller marked as not       |
+    | hidden.                                               |
+    \*-----------------------------------------------------*/
+    for(std::size_t page_idx = 0; page_idx < hidden_pages.size(); page_idx++)
+    {
+        /*-----------------------------------------*\
+        | Get a pointer to the page at this index   |
+        | in the hidden pages vector                |
+        \*-----------------------------------------*/
+        OpenRGBDevicePage*  page        = (OpenRGBDevicePage*)hidden_pages[page_idx];
+        RGBController*      controller  = page->GetController();
+
+        /*-----------------------------------------*\
+        | If the current tab matches the current    |
+        | controller, check if it is hidden         |
+        \*-----------------------------------------*/
+        if(!(controller->GetHidden()))
+        {
+            ui->DevicesTabBar->addTab((QWidget*)page, "");
+
+            /*---------------------------------*\
+            | Create the tab label              |
+            \*---------------------------------*/
+            TabLabel* NewTabLabel = new TabLabel(OpenRGBFont::GetIconIDFromDeviceType(controller->GetDeviceType()), (char *)controller->GetDisplayName().c_str(), (char *)context, false);
+
+            ui->DevicesTabBar->tabBar()->setTabButton(ui->DevicesTabBar->count() - 1, QTabBar::LeftSide, NewTabLabel);
+
+            hidden_pages.erase(hidden_pages.begin() + page_idx);
+
+            /*---------------------------------*\
+            | Decrement page index to account   |
+            | for removing page                 |
+            \*---------------------------------*/
+            page_idx--;
+        }
+    }
+}
+
 void OpenRGBDialog::UpdateDevicesList()
 {
     std::vector<RGBController *> controllers = ResourceManager::get()->GetRGBControllers();
@@ -1134,6 +1229,11 @@ void OpenRGBDialog::UpdateDevicesList()
                     this,
                     SLOT(onDeviceListUpdated()));
 
+            connect(NewPage,
+                    SIGNAL(ShowHideList()),
+                    this,
+                    SLOT(onDeviceListShowHide()));
+
             if(controllers[controller_idx]->GetHidden())
             {
                 hidden_pages.push_back(NewPage);
@@ -1467,6 +1567,11 @@ void OpenRGBDialog::onDeviceListUpdated()
     UpdateDevicesList();
 }
 
+void OpenRGBDialog::onDeviceListShowHide()
+{
+    UpdateDevicesListShowHide();
+}
+
 void OpenRGBDialog::onDetectionProgressUpdated()
 {
     ui->DetectionProgressBar->setValue(ResourceManager::get()->GetDetectionPercent());

qt/OpenRGBDialog/OpenRGBDialog.h

@@ -148,6 +148,7 @@ private:
 
     void ClearDevicesList();
     void UpdateDevicesList();
+    void UpdateDevicesListShowHide();
     void closeEvent(QCloseEvent *event) override;
     bool SelectConfigProfile(const std::string name);
 
@@ -166,6 +167,9 @@ private:
     void UpdateTabs();
     bool isCompactTabMode();
 
+public slots:
+    void onDeviceListUpdated();
+
 private slots:
     void on_Exit();
     void on_LightsOff();
@@ -176,7 +180,7 @@ private slots:
     void on_QuickBlue();
     void on_QuickMagenta();
     void on_QuickWhite();
-    void onDeviceListUpdated();
+    void onDeviceListShowHide();
     void onDetectionProgressUpdated();
     void onDetectionStarted();
     void onDetectionEnded();

scripts/build-appimage.sh

@@ -94,7 +94,7 @@ make install INSTALL_ROOT=AppDir
 
 #-----------------------------------------------------------------------#
 # Make sure Qt plugin finds QML sources so it can deploy the imported   #
-# files                                                                 #
+# files.  Also deploy Wayland support and extra modules used by plugins #
 #-----------------------------------------------------------------------#
 export QML_SOURCES_PATHS="$REPO_ROOT"/src
 
@@ -107,7 +107,33 @@ if [ ! -f $LIBHIDAPI_LIBUSB_PATH ]; then
     LIBHIDAPI_LIBUSB_PATH=$(find /usr/lib -name "libhidapi-libusb.so")
 fi
 
-linuxdeploy-"$ARCH_LINUXDEPLOY".AppImage --appdir AppDir -e "$TARGET" -i "$REPO_ROOT"/qt/org.openrgb.OpenRGB.png -d "$REPO_ROOT"/qt/org.openrgb.OpenRGB.desktop --library "$LIBHIDAPI_LIBUSB_PATH"
+#-----------------------------------------------------------------------#
+# Create custom AppRun script to set environment variables for Qt       #
+# plugins to find bundled libraries (handles both Qt5 and Qt6)          #
+#-----------------------------------------------------------------------#
+cat > AppRun << 'APPRUN_EOF'
+#!/bin/bash
+
+# Get the absolute path to the AppDir
+HERE="$(cd "$(dirname "$(readlink -f "${0}")")" && pwd)"
+
+# Set LD_LIBRARY_PATH so plugins can find bundled Qt libraries
+export LD_LIBRARY_PATH="${HERE}/usr/lib"
+
+# Execute the main binary
+exec "${HERE}/usr/bin/OpenRGB" "$@"
+APPRUN_EOF
+
+chmod +x AppRun
+
+if [ "$QT_SELECT" = "qt6" ]; then
+    QT6_OPENGL_PATH=$(find /usr/lib -name "libQt6OpenGL.so.6")
+    QT6_CORE5COMPAT_PATH=$(find /usr/lib -name "libQt6Core5Compat.so.6")
+    linuxdeploy-"$ARCH_LINUXDEPLOY".AppImage --appdir AppDir --executable "$TARGET" --custom-apprun AppRun --icon-file "$REPO_ROOT"/qt/org.openrgb.OpenRGB.png --desktop-file "$REPO_ROOT"/qt/org.openrgb.OpenRGB.desktop --library "$LIBHIDAPI_LIBUSB_PATH" --library "$QT6_OPENGL_PATH" --library "$QT6_CORE5COMPAT_PATH"
+else
+    linuxdeploy-"$ARCH_LINUXDEPLOY".AppImage --appdir AppDir --executable "$TARGET" --custom-apprun AppRun --icon-file "$REPO_ROOT"/qt/org.openrgb.OpenRGB.png --desktop-file "$REPO_ROOT"/qt/org.openrgb.OpenRGB.desktop --library "$LIBHIDAPI_LIBUSB_PATH"
+fi
+
 linuxdeploy-plugin-qt-"$ARCH_LINUXDEPLOY".AppImage --appdir AppDir
 linuxdeploy-"$ARCH_LINUXDEPLOY".AppImage --appdir AppDir --output appimage