From f7c4f13c905e6a5f2029b54ec72b4e42f7f75dac Mon Sep 17 00:00:00 2001 From: Dylan Deverill Date: Sat, 20 Jun 2026 23:04:38 -0700 Subject: [PATCH] Address review feedback on rumble forwarding PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Drop Tick()/m_wantsGameMode auto-enable machinery (out of scope for this PR — belongs in a separate submission) - Remove unused CMD_TRIGGER_RUMBLE constant - Fix WriteOutputReport comment to accurately describe padding behaviour - Fix CancelIo OVERLAPPED use-after-free: drain with GetOverlappedResult before the stack OVERLAPPED goes out of scope - Strip redundant /wd4819 /wd4828 MSVC flags; /utf-8 already covers them - Add comment to HeartbeatLoop explaining the 40 ms tick rationale Co-Authored-By: Dylan Deverill --- CMakeLists.txt | 10 +++++----- src/app/ControllerManager.cpp | 18 ------------------ src/app/ControllerManager.h | 2 -- src/app/TrayApp.cpp | 8 -------- src/app/TrayApp.h | 1 - src/hid/HidDevice.cpp | 9 +++++++-- src/hid/HidDevice.h | 4 ++-- src/steam/SteamController.cpp | 5 +++++ src/steam/SteamController.h | 1 - 9 files changed, 19 insertions(+), 39 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3bf6a1e..e37657a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -12,7 +12,7 @@ add_library(SteamController STATIC target_include_directories(SteamController PUBLIC src) target_link_libraries(SteamController PUBLIC hid setupapi) if (MSVC) - target_compile_options(SteamController PRIVATE /W4 /WX /utf-8 /wd4819 /wd4828) + target_compile_options(SteamController PRIVATE /W4 /WX /utf-8) endif() # Diagnostic probe tool (console) @@ -21,7 +21,7 @@ add_executable(SteamProbe ) target_link_libraries(SteamProbe PRIVATE SteamController) if (MSVC) - target_compile_options(SteamProbe PRIVATE /W4 /WX /utf-8 /wd4819 /wd4828) + target_compile_options(SteamProbe PRIVATE /W4 /WX /utf-8) set_target_properties(SteamProbe PROPERTIES LINK_FLAGS_DEBUG "/SUBSYSTEM:CONSOLE") endif() @@ -34,7 +34,7 @@ target_include_directories(ViGEmClient PUBLIC ) target_link_libraries(ViGEmClient PUBLIC setupapi) if (MSVC) - target_compile_options(ViGEmClient PRIVATE /W0 /utf-8 /wd4819 /wd4828) + target_compile_options(ViGEmClient PRIVATE /W0 /utf-8) endif() # RawGameController diagnostic probe (WinRT) @@ -43,7 +43,7 @@ add_executable(RawControllerProbe ) target_link_libraries(RawControllerProbe PRIVATE windowsapp) if (MSVC) - target_compile_options(RawControllerProbe PRIVATE /W4 /utf-8 /wd4819 /wd4828) + target_compile_options(RawControllerProbe PRIVATE /W4 /utf-8) endif() # System tray app (no console window) @@ -58,5 +58,5 @@ add_executable(SteamlessController WIN32 target_include_directories(SteamlessController PRIVATE resources) target_link_libraries(SteamlessController PRIVATE SteamController ViGEmClient) if (MSVC) - target_compile_options(SteamlessController PRIVATE /W4 /WX /utf-8 /wd4819 /wd4828) + target_compile_options(SteamlessController PRIVATE /W4 /WX /utf-8) endif() diff --git a/src/app/ControllerManager.cpp b/src/app/ControllerManager.cpp index beb54ee..86d6604 100644 --- a/src/app/ControllerManager.cpp +++ b/src/app/ControllerManager.cpp @@ -20,24 +20,9 @@ void ControllerManager::OnDeviceChange() { TryOpen(); else if (g_ctrl && !g_ctrl->IsOpen()) Close(/*restoreLizard=*/false); - - if (m_connected && m_wantsGameMode && !m_gameModeActive) - EnableGameMode(); -} - -void ControllerManager::Tick() { - if (!m_connected) { - TryOpen(); - } else if (g_ctrl && !g_ctrl->IsOpen()) { - Close(/*restoreLizard=*/false); - } - - if (m_connected && m_wantsGameMode && !m_gameModeActive) - EnableGameMode(); } void ControllerManager::EnableGameMode() { - m_wantsGameMode = true; if (!m_connected || m_gameModeActive) return; if (!g_ctrl->DisableLizardMode()) return; @@ -67,7 +52,6 @@ void ControllerManager::EnableGameMode() { } void ControllerManager::DisableGameMode() { - m_wantsGameMode = false; if (!m_gameModeActive) return; StopReadLoop(); if (g_ctrl && g_ctrl->IsOpen()) @@ -99,8 +83,6 @@ void ControllerManager::TryOpen() { if (g_ctrl->Open()) { m_connected = true; m_onStateChanged(m_connected, m_gameModeActive, false); - if (m_wantsGameMode && !m_gameModeActive) - EnableGameMode(); } } diff --git a/src/app/ControllerManager.h b/src/app/ControllerManager.h index 39c399e..1d6a3eb 100644 --- a/src/app/ControllerManager.h +++ b/src/app/ControllerManager.h @@ -21,7 +21,6 @@ public: // Called when Windows reports a device arrival or removal (WM_DEVICECHANGE). void OnDeviceChange(); - void Tick(); // Toggle game mode on/off. No-op if controller is not connected. void EnableGameMode(); @@ -47,7 +46,6 @@ private: StateChangedFn m_onStateChanged; bool m_connected = false; bool m_gameModeActive = false; - bool m_wantsGameMode = true; bool m_trackpadMouseEnabled = false; bool m_backButtonsEnabled = false; bool m_useLeftTrackpad = false; diff --git a/src/app/TrayApp.cpp b/src/app/TrayApp.cpp index 1ead2e9..fae9d58 100644 --- a/src/app/TrayApp.cpp +++ b/src/app/TrayApp.cpp @@ -14,8 +14,6 @@ TrayApp::TrayApp() { } TrayApp::~TrayApp() { - if (m_hwnd) - KillTimer(m_hwnd, TIMER_RECONNECT); RemoveTrayIcon(); g_app = nullptr; } @@ -55,7 +53,6 @@ bool TrayApp::Init(HINSTANCE hInstance) { LoadSettings(); AddTrayIcon(); UpdateTrayIcon(m_controller->IsConnected(), m_controller->IsGameModeActive(), false); - SetTimer(m_hwnd, TIMER_RECONNECT, 2000, nullptr); return true; } @@ -125,11 +122,6 @@ LRESULT TrayApp::HandleMessage(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) { m_controller->OnDeviceChange(); return TRUE; - case WM_TIMER: - if (wp == TIMER_RECONNECT) - m_controller->Tick(); - return 0; - case WM_DESTROY: PostQuitMessage(0); return 0; diff --git a/src/app/TrayApp.h b/src/app/TrayApp.h index 5825ce6..8914583 100644 --- a/src/app/TrayApp.h +++ b/src/app/TrayApp.h @@ -39,7 +39,6 @@ private: static constexpr UINT IDM_BACKBUTTONS = 1004; static constexpr UINT IDM_LEFT_TRACKPAD = 1005; static constexpr UINT IDM_STARTUP = 1006; - static constexpr UINT_PTR TIMER_RECONNECT = 1; static constexpr UINT WM_TRAY = WM_APP + 1; static constexpr UINT TRAY_UID = 1; }; diff --git a/src/hid/HidDevice.cpp b/src/hid/HidDevice.cpp index 8033dd6..e5a8dad 100644 --- a/src/hid/HidDevice.cpp +++ b/src/hid/HidDevice.cpp @@ -191,10 +191,15 @@ bool HidDevice::WriteOutputReport(const uint8_t* data, size_t size) { BOOL ok = WriteFile(m_handle, buffer, static_cast(len), &bytesWritten, &ov); if (!ok && GetLastError() == ERROR_IO_PENDING) { DWORD wait = WaitForSingleObject(event, 1000); - if (wait == WAIT_OBJECT_0) + if (wait == WAIT_OBJECT_0) { ok = GetOverlappedResult(m_handle, &ov, &bytesWritten, FALSE); - else + } else { + // CancelIo is asynchronous — the kernel can still complete the IRP + // and write into `ov` after it returns. Block until the cancelled + // (or already-completed) I/O drains before `ov` leaves scope. CancelIo(m_handle); + ok = GetOverlappedResult(m_handle, &ov, &bytesWritten, TRUE); + } } DWORD err = ok ? ERROR_SUCCESS : GetLastError(); diff --git a/src/hid/HidDevice.h b/src/hid/HidDevice.h index c587afd..9b257d4 100644 --- a/src/hid/HidDevice.h +++ b/src/hid/HidDevice.h @@ -27,8 +27,8 @@ public: // data[0] must be the report ID. Padded to OutputReportByteLength automatically. bool SendOutputReport(const uint8_t* data, size_t size); - // Write a HID output report through the interrupt output endpoint. - // Some devices use short output reports that should not be padded. + // Write a HID output report through the interrupt output endpoint via WriteFile. + // data[0] must be the report ID. Padded to OutputReportByteLength automatically. bool WriteOutputReport(const uint8_t* data, size_t size); // Send a HID feature report (SET_REPORT Feature type via EP0 control pipe). diff --git a/src/steam/SteamController.cpp b/src/steam/SteamController.cpp index f3598e1..a2a866f 100644 --- a/src/steam/SteamController.cpp +++ b/src/steam/SteamController.cpp @@ -166,6 +166,11 @@ void SteamController::HeartbeatLoop() { BuildCmd(buf, CMD_CLEAR_DIGITAL_MAPPINGS); int lizardTicks = 0; + // Fixed 40 ms tick: active rumble must be re-sent roughly every 50 ms or + // the controller's safety timeout stops the motors. The lizard-mode + // keep-alive only needs ~800 ms, so it fires every 20th tick. Don't sleep + // longer when rumble is idle — rumble starting mid-sleep would stutter + // until the next tick. while (m_running.load()) { if (lizardTicks <= 0) { std::lock_guard lock(m_commandMutex); diff --git a/src/steam/SteamController.h b/src/steam/SteamController.h index 57298c0..85b0aeb 100644 --- a/src/steam/SteamController.h +++ b/src/steam/SteamController.h @@ -34,7 +34,6 @@ public: static constexpr uint8_t CMD_SET_DEFAULT_MAPPINGS = 0x85; // ← lizard on static constexpr uint8_t CMD_SET_SETTINGS = 0x87; static constexpr uint8_t CMD_GET_SETTINGS = 0x89; - static constexpr uint8_t CMD_TRIGGER_RUMBLE = 0xEB; static constexpr uint8_t OUT_REPORT_HAPTIC_RUMBLE = 0x80; // Setting key IDs (go in the payload of CMD_SET_SETTINGS)