fix(device): re-apply profile when a transport re-attaches

src/app/AppRoot.cpp

@@ -7,7 +7,9 @@
 #include "desktop/GenericDesktop.h"
 #include "input/UinputInjector.h"
 #include "logging/LogManager.h"
+#include <QDBusConnection>
 #include <QProcessEnvironment>
+#include <QTimer>
 
 namespace logitune {
 
@@ -165,6 +167,19 @@ void AppRoot::wireSignals()
             this, &AppRoot::onPhysicalDeviceAdded);
     connect(&m_deviceManager, &DeviceManager::physicalDeviceRemoved,
             this, &AppRoot::onPhysicalDeviceRemoved);
+    connect(&m_deviceManager, &DeviceManager::physicalDeviceTransportReady,
+            this, &AppRoot::onPhysicalDeviceTransportReady);
+
+    // systemd-logind PrepareForSleep(before): before=true before suspend,
+    // before=false on resume. Re-apply profiles on resume to restore device
+    // state that the firmware reset while powered down.
+    QDBusConnection::systemBus().connect(
+        QStringLiteral("org.freedesktop.login1"),
+        QStringLiteral("/org/freedesktop/login1"),
+        QStringLiteral("org.freedesktop.login1.Manager"),
+        QStringLiteral("PrepareForSleep"),
+        this,
+        SLOT(onPrepareForSleep(bool)));
 
     // Gesture keystroke edits in the UI. saveCurrentProfile re-serializes
     // the displayed profile; the orchestrator's own userChangedSomething
@@ -304,6 +319,32 @@ void AppRoot::onPhysicalDeviceRemoved(PhysicalDevice *device)
         m_deviceModel.setSelectedIndex(0);
 }
 
+// A new transport attached to an existing PhysicalDevice (e.g. Bolt receiver
+// reconnects while a BT session is still alive, or rapid udev remove+add
+// where the PhysicalDevice object was not destroyed). physicalDeviceAdded is
+// NOT re-emitted in this case, so profile re-application needs this path.
+void AppRoot::onPhysicalDeviceTransportReady(PhysicalDevice *device)
+{
+    m_profileOrchestrator.onTransportSetupComplete(device);
+}
+
+// systemd-logind signals PrepareForSleep(true) before suspend and
+// PrepareForSleep(false) on resume. On resume we wait 2 s for HID++ to
+// stabilise, then re-apply the stored profile to every known device.
+// This covers the common case where the Bolt receiver does not send a
+// 0x41 reconnect notification after the laptop wakes from sleep.
+void AppRoot::onPrepareForSleep(bool beforeSleep)
+{
+    if (beforeSleep)
+        return;
+
+    QTimer::singleShot(2000, this, [this]() {
+        qCInfo(lcApp) << "system resumed from sleep — re-applying device profiles";
+        for (PhysicalDevice *pd : m_deviceManager.physicalDevices())
+            m_profileOrchestrator.onTransportSetupComplete(pd);
+    });
+}
+
 // Carousel selection changed. Refresh the UI from the newly-selected
 // device's cached profile. No file I/O, no seeding, no hardware apply —
 // one-time device provisioning happens in onPhysicalDeviceAdded.

src/app/AppRoot.h

@@ -72,6 +72,8 @@ class AppRoot : public QObject {
 private slots:
     void onPhysicalDeviceAdded(PhysicalDevice *device);
     void onPhysicalDeviceRemoved(PhysicalDevice *device);
+    void onPhysicalDeviceTransportReady(PhysicalDevice *device);
+    void onPrepareForSleep(bool beforeSleep);
 
 private:
     void wireSignals();

src/core/DeviceManager.cpp

@@ -520,6 +520,12 @@ void DeviceManager::probeDevice(const QString &devNode)
         emit physicalDeviceAdded(pdPtr);
     } else {
         pdIt->second->attachTransport(sessionPtr);
+        // Signal that a transport just (re-)attached to an existing device so
+        // the profile orchestrator can re-apply settings. The standard
+        // transportSetupComplete path only fires when the signal was already
+        // wired — but enumerateAndSetup() emits setupComplete() before
+        // attachTransport() connects the signal, so we notify explicitly here.
+        emit physicalDeviceTransportReady(pdIt->second.get());
     }
 }
 

src/core/DeviceManager.h

@@ -57,6 +57,11 @@ class DeviceManager : public QObject {
     void physicalDeviceAdded(PhysicalDevice *device);
     void physicalDeviceRemoved(PhysicalDevice *device);
 
+    // Emitted when a new transport attaches to an EXISTING PhysicalDevice.
+    // physicalDeviceAdded is NOT re-emitted in this case, so profile
+    // re-application must also connect to this signal.
+    void physicalDeviceTransportReady(PhysicalDevice *device);
+
     void unknownDeviceDetected(uint16_t pid);
 
 private slots:

src/core/DeviceSession.cpp

@@ -138,6 +138,28 @@ QString DeviceSession::deviceId() const
         .arg(m_devicePid, 4, 16, QLatin1Char('0'));
 }
 
+// ---------------------------------------------------------------------------
+// rebuildCommandProcessor()
+// ---------------------------------------------------------------------------
+// Point the command processor at the FeatureDispatcher and Transport that are
+// current *now*. enumerateAndSetup() builds a fresh FeatureDispatcher on every
+// run, so a processor carried over from a previous run would keep a raw
+// pointer to the destroyed dispatcher and use it on the next queued command.
+// Tearing the old one down first also drops any commands queued against it.
+void DeviceSession::rebuildCommandProcessor()
+{
+    if (m_commandProcessor) {
+        m_commandProcessor->clear();
+        m_commandProcessor->stop();
+        m_commandProcessor.reset();
+    }
+    if (!m_features || !m_transport)
+        return;
+    m_commandProcessor = std::make_unique<hidpp::CommandProcessor>(
+        m_features.get(), m_transport.get(), m_deviceIndex);
+    m_commandProcessor->start();
+}
+
 // ---------------------------------------------------------------------------
 // enumerateAndSetup()
 // ---------------------------------------------------------------------------
@@ -451,11 +473,7 @@ void DeviceSession::enumerateAndSetup()
     m_lastResponseTime = QDateTime::currentMSecsSinceEpoch();
     m_enumerating = false;
 
-    if (!m_commandProcessor && m_features && m_transport) {
-        m_commandProcessor = std::make_unique<hidpp::CommandProcessor>(
-            m_features.get(), m_transport.get(), m_deviceIndex);
-        m_commandProcessor->start();
-    }
+    rebuildCommandProcessor();
 
     // Start periodic battery polling (60s)
     if (!m_batteryPollTimer) {

src/core/DeviceSession.h

@@ -14,7 +14,7 @@
 #include <optional>
 #include <vector>
 
-namespace logitune::test { class AppRootFixture; }
+namespace logitune::test { class AppRootFixture; class DeviceSessionFixture; }
 
 namespace logitune {
 
@@ -24,6 +24,7 @@ class IDevice;
 class DeviceSession : public QObject {
     Q_OBJECT
     friend class test::AppRootFixture;
+    friend class test::DeviceSessionFixture;
 
 public:
     DeviceSession(std::unique_ptr<hidpp::HidrawDevice> device,
@@ -128,6 +129,7 @@ class DeviceSession : public QObject {
 private:
     void checkSleepWake();
     bool isDirectDevice(uint16_t pid) const;
+    void rebuildCommandProcessor();
 
     DeviceRegistry *m_registry = nullptr;
     const IDevice *m_activeDevice = nullptr;

src/core/hidpp/CommandProcessor.h

@@ -43,6 +43,10 @@ class CommandProcessor : public QObject {
     /// Number of pending commands.
     int pending() const;
 
+    /// The dispatcher this processor sends through. Held by raw pointer, so
+    /// whoever replaces the dispatcher must replace the processor with it.
+    const FeatureDispatcher *features() const { return m_features; }
+
 signals:
     void queueDrained();
 

tests/helpers/AppRootFixture.h

@@ -179,6 +179,14 @@ class AppRootFixture : public ::testing::Test {
         m_ctrl->m_profileModel.selectTab(hwIndex);
     }
 
+    /// Fire the DeviceManager signal that marks a transport re-attaching to a
+    /// PhysicalDevice that already exists. DeviceManager::probeDevice emits it
+    /// from a udev callback and needs a real /dev node, so tests raise it here
+    /// and exercise everything downstream of it.
+    void emitTransportReady(PhysicalDevice *device) {
+        emit m_ctrl->m_deviceManager.physicalDeviceTransportReady(device);
+    }
+
     void pressButton(uint16_t controlId) {
         m_ctrl->m_buttonDispatcher.onDivertedButtonPressed(controlId, true);
     }

tests/helpers/DeviceSessionFixture.h

@@ -0,0 +1,48 @@
+#pragma once
+#include <gtest/gtest.h>
+#include <memory>
+
+#include "DeviceRegistry.h"
+#include "DeviceSession.h"
+#include "helpers/TestFixtures.h"
+#include "hidpp/FeatureDispatcher.h"
+#include "hidpp/HidrawDevice.h"
+
+namespace logitune::test {
+
+/// Reaches into the transport-owned internals of a DeviceSession.
+///
+/// enumerateAndSetup() cannot run without hardware — FeatureDispatcher::
+/// enumerate() fails against /dev/null and the function bails before it
+/// reaches anything worth asserting on. Tests therefore drive the one step
+/// that a re-enumeration performs on already-built state: swap the
+/// dispatcher, then rebuild what points at it.
+class DeviceSessionFixture : public ::testing::Test {
+protected:
+    void SetUp() override {
+        ensureApp();
+        auto hidraw = std::make_unique<hidpp::HidrawDevice>("/dev/null");
+        m_session = std::make_unique<DeviceSession>(
+            std::move(hidraw), 0xFF, QStringLiteral("Bluetooth"), &m_registry);
+    }
+
+    /// The half of enumerateAndSetup() that survives without a real device:
+    /// a fresh FeatureDispatcher replaces the previous one, and everything
+    /// holding a pointer into it has to be rebuilt.
+    void reenumerate() {
+        m_session->m_features = std::make_unique<hidpp::FeatureDispatcher>();
+        m_session->rebuildCommandProcessor();
+    }
+
+    hidpp::FeatureDispatcher *dispatcher() const {
+        return m_session->m_features.get();
+    }
+    hidpp::CommandProcessor *processor() const {
+        return m_session->m_commandProcessor.get();
+    }
+
+    DeviceRegistry                 m_registry;
+    std::unique_ptr<DeviceSession> m_session;
+};
+
+} // namespace logitune::test

tests/test_device_reconnect.cpp

@@ -61,3 +61,48 @@ TEST_F(AppRootFixture, ButtonDiversionsMatchHwProfile) {
     pressButton(0x0056);
     EXPECT_EQ(m_injector->lastArg("injectKeystroke"), "Alt+Right");
 }
+
+// ---------------------------------------------------------------------------
+// Transport re-attach
+// ---------------------------------------------------------------------------
+// When a transport re-attaches to a PhysicalDevice that already exists —
+// a Bolt receiver coming back while the Bluetooth session is still alive, or
+// a udev remove+add fast enough that the PhysicalDevice was never destroyed —
+// DeviceManager does not emit physicalDeviceAdded a second time. Nothing then
+// re-applies the profile, and the device keeps whatever its firmware reset to
+// (issue #132).
+//
+// DeviceManager::probeDevice needs a real /dev node, so these tests start one
+// step downstream: at the signal it emits and the AppRoot slot behind it.
+
+TEST_F(AppRootFixture, TransportReadyReappliesStoredProfile) {
+    const QString kSerial = QStringLiteral("mock-serial");
+    profileEngine().cachedProfile(kSerial, QStringLiteral("default")).dpi = 2400;
+
+    // The link dropped and the device came back at its firmware default.
+    m_session->setDPI(400);
+    ASSERT_EQ(m_session->currentDPI(), 400);
+
+    emitTransportReady(m_physicalDevice);
+
+    EXPECT_EQ(m_session->currentDPI(), 2400);
+}
+
+// The re-applied profile is the one the device is actually on, not "default".
+// A per-app profile that was active before the drop has to survive it.
+TEST_F(AppRootFixture, TransportReadyReappliesActiveAppProfile) {
+    const QString kSerial = QStringLiteral("mock-serial");
+    createAppProfile("google-chrome", "Google Chrome");
+    // createAppProfile writes the profile to disk; the cache is what
+    // onTransportSetupComplete reads, so set the DPI there too.
+    profileEngine().cachedProfile(kSerial, QStringLiteral("Google Chrome")).dpi = 3200;
+    focusApp("google-chrome");
+    ASSERT_EQ(profileEngine().hardwareProfile(kSerial),
+              QStringLiteral("Google Chrome"));
+
+    m_session->setDPI(400);
+
+    emitTransportReady(m_physicalDevice);
+
+    EXPECT_EQ(m_session->currentDPI(), 3200);
+}

tests/test_device_session.cpp

@@ -4,6 +4,7 @@
 #include "DeviceRegistry.h"
 #include "hidpp/HidrawDevice.h"
 #include "hidpp/HidppTypes.h"
+#include "helpers/DeviceSessionFixture.h"
 
 using namespace logitune;
 using namespace logitune::hidpp;
@@ -208,3 +209,47 @@ TEST_F(DeviceSessionTest, AccessorsReturnNullables) {
     EXPECT_NE(session->transport(), nullptr);
     EXPECT_NE(session->device(), nullptr);
 }
+
+// ---------------------------------------------------------------------------
+// Command processor lifetime across re-enumeration
+// ---------------------------------------------------------------------------
+// enumerateAndSetup() builds a new FeatureDispatcher every time it runs. The
+// CommandProcessor holds that dispatcher by raw pointer, so a processor kept
+// from an earlier run points at freed memory the moment the dispatcher is
+// replaced. Issue #132 hit this on every reconnect.
+
+using logitune::test::DeviceSessionFixture;
+
+TEST_F(DeviceSessionFixture, FirstEnumerationBuildsProcessorForCurrentDispatcher) {
+    ASSERT_EQ(processor(), nullptr);
+
+    reenumerate();
+
+    ASSERT_NE(processor(), nullptr);
+    EXPECT_EQ(processor()->features(), dispatcher());
+}
+
+TEST_F(DeviceSessionFixture, ReEnumerationRepointsProcessorAtNewDispatcher) {
+    reenumerate();
+    auto *firstDispatcher = dispatcher();
+    ASSERT_NE(firstDispatcher, nullptr);
+    ASSERT_EQ(processor()->features(), firstDispatcher);
+
+    reenumerate();
+
+    // The dispatcher the first processor was built against is gone. Reading
+    // through it is the use-after-free, so the processor must have followed
+    // the swap rather than survived it.
+    ASSERT_NE(dispatcher(), firstDispatcher);
+    EXPECT_EQ(processor()->features(), dispatcher());
+}
+
+TEST_F(DeviceSessionFixture, RebuildDropsCommandsQueuedAgainstTheOldDispatcher) {
+    reenumerate();
+    processor()->enqueue(logitune::hidpp::FeatureId::AdjustableDPI, 0x03, {});
+    ASSERT_GT(processor()->pending(), 0);
+
+    reenumerate();
+
+    EXPECT_EQ(processor()->pending(), 0);
+}