From 88585478996d9cd43f6798e90947a6870eb1ea81 Mon Sep 17 00:00:00 2001 From: Emir Hasanbegovic Date: Wed, 13 May 2026 20:18:09 -0400 Subject: [PATCH] feat(rumble): forward MSG_RUMBLE to SDL controller (rumble + lightbar) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the dish-side handler for the satellite's reverse-direction rumble message (MSG_RUMBLE = 0x0009). SatelliteClient::processIncoming parses the wire format via a pure static parseRumbleMessage decoder, the per-WifiConnection rumble handler resolves connId → slotId → deviceId via the ConnectionHub bindings, and SDLGamepadBridge::applyRumble drives the matching SDL_GameController. * Wire format documented in satellite/README.md#rumble-return-path: ctrlIdx(u8) strong(u16 BE) weak(u16 BE) durMs(u16 BE) flags(u8) [R, G, B] (u8×3 if flags bit 0 set) * SDL_GameControllerRumble passes the strong/weak magnitudes through verbatim (XInput scale matches). SDL_GameControllerSetLED is invoked when the satellite published a DS4 lightbar colour; both are silent no-ops on pads that don't support the feature. * AppModel::installRumbleHandlers walks the WifiConnection pool on every poolChanged signal and attaches a handler to any new connection that doesn't already have one. Idempotent. * WifiConnection caches the handler so it survives reconnects: markConnected re-installs it on the new SatelliteClient instance. * parseRumbleMessage is exposed as a static helper so it can be unit- tested without driving a live socket. New Catch2 suite covers: byte- layout decoding, stop requests, max magnitudes, lightbar tail, lightbar flag with truncated tail, forward-compat with extra trailing bytes, reserved flag bits, big-endian boundary cases. Stacked on top of feature/ui-overhaul-and-async-bind. PR base set to the parent branch so the diff shows only rumble. Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 37 +++++ src/AppModel.cpp | 33 +++++ src/AppModel.h | 10 ++ src/Input/SDLGamepadBridge.cpp | 27 ++++ src/Input/SDLGamepadBridge.h | 18 +++ src/Network/SatelliteClient.cpp | 39 ++++++ src/Network/SatelliteClient.h | 43 ++++++ src/Network/WifiConnection.cpp | 8 ++ src/Network/WifiConnection.h | 12 ++ tests/CMakeLists.txt | 3 +- tests/test_satellite_client_rumble.cpp | 181 +++++++++++++++++++++++++ 11 files changed, 410 insertions(+), 1 deletion(-) create mode 100644 tests/test_satellite_client_rumble.cpp diff --git a/README.md b/README.md index c1d7b08..8b6d36a 100644 --- a/README.md +++ b/README.md @@ -99,6 +99,43 @@ behaviour stays predictable across platforms: enum), USB VID / PID, and the SDL GUID. Aimed at users reporting *"my pad doesn't work"* — same idea as Android's SatelliteJNI `DEVCAPS` log. +## Rumble (return path) + +Rumble flows the opposite direction to the input hot path: a game on the +satellite host writes to the virtual controller's vibration channel, the +satellite forwards a `MSG_RUMBLE = 0x0009` packet back over the encrypted +UDP socket, and the dish actuates the matching SDL controller. + +``` + ┌──────────────────────┐ ┌──────────────────────┐ ┌──────────────────────┐ + │ SatelliteClient │ ───► │ WifiConnection │ ───► │ SDLGamepadBridge │ + │ • receive thread │ │ • per-conn handler │ │ • applyRumble(...) │ + │ • parseRumbleMsg │ │ (installed by │ │ → SDL_Game- │ + │ • dispatch to │ │ AppModel via │ │ ControllerRumble│ + │ handler │ │ poolChanged) │ │ → ...SetLED │ + └──────────────────────┘ └──────────────────────┘ └──────────┬───────────┘ + │ + ▼ + evdev EVIOCSFF + (or BT-HID rumble) +``` + +The wire format is documented in +[`satellite/README.md`](https://github.com/TinkerNorth/satellite#rumble-return-path). +On the dish-linux side: + +* **Parser** — `SatelliteClient::parseRumbleMessage` is a pure static + decoder so unit tests can exercise byte layouts without a live socket + (see `tests/test_satellite_client_rumble.cpp`). +* **Routing** — `AppModel::installRumbleHandlers` walks the `WifiConnection` + pool on every `poolChanged` and attaches a handler that resolves + `connId → slotId → deviceId` via the `ConnectionHub` bindings, then + calls `SDLGamepadBridge::applyRumble`. +* **Actuation** — `SDL_GameControllerRumble(strong, weak, durMs)` for the + motors; `SDL_GameControllerSetLED(R, G, B)` when the satellite published + a DS4 lightbar colour. Failures are silent — many pads don't support + either operation and there's nothing actionable for the player. + ## Requirements - A reasonably current Linux distro (Ubuntu 22.04+, Fedora 38+, Arch, …) diff --git a/src/AppModel.cpp b/src/AppModel.cpp index f3c2c8d..e2d6785 100644 --- a/src/AppModel.cpp +++ b/src/AppModel.cpp @@ -20,6 +20,11 @@ AppModel::AppModel(std::unique_ptr inhibitor, QObje &AppModel::onBridgeDevicesChanged); QObject::connect(wifi_, &net::WifiConnectionManager::connectionEvent, this, &AppModel::onWifiEvent); + // poolChanged fires every time a WifiConnection is created or transitions + // state — perfect place to make sure new connections have a rumble + // handler. Idempotent on already-wired connections. + QObject::connect(wifi_, &net::WifiConnectionManager::poolChanged, this, + &AppModel::installRumbleHandlers); autoReconnectTimer_->setInterval(15'000); QObject::connect(autoReconnectTimer_, &QTimer::timeout, this, @@ -43,6 +48,34 @@ AppModel::AppModel(std::unique_ptr inhibitor, QObje AppModel::~AppModel() { bridge_->stop(); } +void AppModel::installRumbleHandlers() { + for (auto* conn : wifi_->connections()) { + const QString id = conn->id(); + if (rumbleWiredConnections_.contains(id)) { continue; } + rumbleWiredConnections_.insert(id); + // Capture `this` and the connection id by value. The handler runs on + // the SatelliteClient receive thread; it only reads structures + // protected by their own locks (hub bindings, bridge device map). + conn->setRumbleHandler([this, id](const net::SatelliteClient::RumbleMessage& rm) { + // Find the slot bound to this connection. + QString deviceId; + const auto bindings = hub_->bindings(); + for (auto it = bindings.cbegin(); it != bindings.cend(); ++it) { + if (it.value() == id) { + deviceId = it.key(); + break; + } + } + if (deviceId.isEmpty()) { return; } + // For dish-linux, slot.id == physical device.id (set in rebuild()), + // so the slot id IS the bridge's device id. Hand it straight to + // the SDL bridge. + bridge_->applyRumble(deviceId, rm.strongMagnitude, rm.weakMagnitude, rm.durationMs, + rm.hasLightbar, rm.lightbarR, rm.lightbarG, rm.lightbarB); + }); + } +} + void AppModel::start() { bridge_->start(); wifi_->autoReconnectAll(); diff --git a/src/AppModel.h b/src/AppModel.h index 4f5f570..000cbc0 100644 --- a/src/AppModel.h +++ b/src/AppModel.h @@ -14,6 +14,7 @@ #include #include +#include #include #include @@ -85,6 +86,10 @@ class AppModel : public QObject { void onHubChanged(); void onBridgeDevicesChanged(); void onWifiEvent(const net::ConnectionEvent& evt); + // Walk the WifiConnectionManager pool and install our rumble handler on + // any connection that doesn't already have one. Idempotent — invoked on + // every poolChanged signal so newly-created connections get wired. + void installRumbleHandlers(); std::unique_ptr store_; net::WifiConnectionManager* wifi_; @@ -98,6 +103,11 @@ class AppModel : public QObject { std::unique_ptr inhibitor_; util::ScreenWakeController wake_; + // Set of connection ids we've already attached rumble handlers to, so we + // don't reinstall on every pool churn. WifiConnections live until + // application teardown so this set never gets pruned. + QSet rumbleWiredConnections_; + MainUiState state_; // slotId -> active sender. Read on the SDL gamepad thread; written on the diff --git a/src/Input/SDLGamepadBridge.cpp b/src/Input/SDLGamepadBridge.cpp index 5891dbc..a015b49 100644 --- a/src/Input/SDLGamepadBridge.cpp +++ b/src/Input/SDLGamepadBridge.cpp @@ -153,6 +153,33 @@ void SDLGamepadBridge::runLoop() { SDL_QuitSubSystem(SDL_INIT_GAMECONTROLLER | SDL_INIT_JOYSTICK); } +void SDLGamepadBridge::applyRumble(const QString& deviceId, std::uint16_t strongMagnitude, + std::uint16_t weakMagnitude, std::uint16_t durationMs, + bool hasLightbar, std::uint8_t lightbarR, + std::uint8_t lightbarG, std::uint8_t lightbarB) { + SDL_GameController* gc = nullptr; + { + std::lock_guard lock(mtx_); + for (const auto& [iid, did] : deviceIds_) { + if (did == deviceId) { + if (auto it = openControllers_.find(iid); it != openControllers_.end()) { + gc = it->second; + } + break; + } + } + } + if (gc == nullptr) { return; } + // SDL2's `SDL_GameControllerRumble` returns 0 on success, -1 if the device + // doesn't support rumble — silent: the caller has no recourse beyond the + // satellite-side game already running, which doesn't know either way. + SDL_GameControllerRumble(gc, strongMagnitude, weakMagnitude, durationMs); + if (hasLightbar) { + // SDL_GameControllerSetLED is a no-op on pads without a lightbar. + SDL_GameControllerSetLED(gc, lightbarR, lightbarG, lightbarB); + } +} + void SDLGamepadBridge::rebuildState(int iid) { SDL_GameController* gc = nullptr; std::string deviceId; diff --git a/src/Input/SDLGamepadBridge.h b/src/Input/SDLGamepadBridge.h index 328fecb..10effd1 100644 --- a/src/Input/SDLGamepadBridge.h +++ b/src/Input/SDLGamepadBridge.h @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -42,6 +43,23 @@ class SDLGamepadBridge : public QObject { }; QList devices() const; + // Drive the physical controller's rumble motors. `strongMagnitude` and + // `weakMagnitude` are 16-bit magnitudes matching XInput's scale so they + // can flow through the SDL2 API verbatim. `durationMs == 0` is a "stop" + // signal — SDL itself treats 0 as "do not run", so we forward as-is. + // + // If the controller exposes a lightbar (DualShock 4 / DualSense) and the + // satellite published one, we also call SDL_GameControllerSetLED. Failures + // are silent — many pads don't support either operation and SDL just + // returns -1 in that case. + // + // Thread-safety: callable from any thread; takes the same internal mutex + // that guards the device map. Intended to be invoked from the + // SatelliteClient receive thread. + void applyRumble(const QString& deviceId, std::uint16_t strongMagnitude, + std::uint16_t weakMagnitude, std::uint16_t durationMs, bool hasLightbar, + std::uint8_t lightbarR, std::uint8_t lightbarG, std::uint8_t lightbarB); + signals: void devicesChanged(); diff --git a/src/Network/SatelliteClient.cpp b/src/Network/SatelliteClient.cpp index 2e6486b..d9a5708 100644 --- a/src/Network/SatelliteClient.cpp +++ b/src/Network/SatelliteClient.cpp @@ -235,7 +235,46 @@ void SatelliteClient::processIncoming(const std::uint8_t* buf, std::size_t n) { } else if (msgType == kMsgServerStatus && msgLen >= 2 && plainLen >= 6) { vigemAvailable_.store(plain[4] == 0 ? 0 : 1, std::memory_order_relaxed); activeControllerCount_.store(static_cast(plain[5]), std::memory_order_relaxed); + } else if (msgType == kMsgRumble) { + // The inner header bytes are at plain[0..3]; the payload starts at +4. + // parseRumbleMessage works on the payload region for parity with the + // unit-test seam, so adjust the pointer/length accordingly. + if (plainLen < 4) { return; } + const auto rm = parseRumbleMessage(plain.data() + 4, + static_cast(plainLen) - 4); + if (!rm) { return; } + RumbleHandler handler; + { + std::lock_guard lock(rumbleHandlerMtx_); + handler = rumbleHandler_; + } + if (handler) { handler(*rm); } + } +} + +void SatelliteClient::setRumbleHandler(RumbleHandler handler) { + std::lock_guard lock(rumbleHandlerMtx_); + rumbleHandler_ = std::move(handler); +} + +std::optional +SatelliteClient::parseRumbleMessage(const std::uint8_t* payload, std::size_t len) { + // Mandatory fields: ctrlIdx + strong + weak + dur + flags = 8 bytes. + if (payload == nullptr || len < 8) { return std::nullopt; } + RumbleMessage rm; + rm.controllerIndex = payload[0]; + rm.strongMagnitude = util::readU16Be(payload + 1); + rm.weakMagnitude = util::readU16Be(payload + 3); + rm.durationMs = util::readU16Be(payload + 5); + const std::uint8_t flags = payload[7]; + rm.hasLightbar = (flags & 0x01) != 0; + if (rm.hasLightbar) { + if (len < 11) { return std::nullopt; } // declared lightbar but truncated + rm.lightbarR = payload[8]; + rm.lightbarG = payload[9]; + rm.lightbarB = payload[10]; } + return rm; } } // namespace dish::net diff --git a/src/Network/SatelliteClient.h b/src/Network/SatelliteClient.h index b0aa187..fce3444 100644 --- a/src/Network/SatelliteClient.h +++ b/src/Network/SatelliteClient.h @@ -23,7 +23,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -40,6 +42,7 @@ class SatelliteClient { static constexpr std::uint16_t kMsgControllerAck = 0x0006; static constexpr std::uint16_t kMsgServerStatus = 0x0007; static constexpr std::uint16_t kMsgControllerType = 0x0008; + static constexpr std::uint16_t kMsgRumble = 0x0009; static constexpr std::uint32_t kHeartbeatIntervalMs = 2000; static constexpr int kHeartbeatMissMax = 5; @@ -69,6 +72,40 @@ class SatelliteClient { void sendControllerType(int index, int type); void resetControllerAck() { lastControllerAck_.store(-1, std::memory_order_relaxed); } + // Decoded rumble message from the satellite. `lightbar*` are valid only + // when `hasLightbar` is true (the wire format's optional trailing 3 bytes). + struct RumbleMessage { + int controllerIndex = 0; + std::uint16_t strongMagnitude = 0; + std::uint16_t weakMagnitude = 0; + std::uint16_t durationMs = 0; + bool hasLightbar = false; + std::uint8_t lightbarR = 0; + std::uint8_t lightbarG = 0; + std::uint8_t lightbarB = 0; + }; + + // Install (or replace) the rumble callback. Invoked from the receive + // loop's thread for every parsed MSG_RUMBLE packet. The handler is + // expected to enqueue / forward to the actuator without blocking; we + // hold an internal lock around assignment to avoid a TOCTOU on the read + // side, but the call itself runs unlocked. + using RumbleHandler = std::function; + void setRumbleHandler(RumbleHandler handler); + + // Pure decoder for the MSG_RUMBLE inner payload (the 4-byte header + // {type, length} has already been stripped). Returns std::nullopt on + // truncation; see ClientAdapter::sendRumble for the producer side. Kept + // public + static so it can be exercised by unit tests without a live + // socket. + // + // Wire layout: + // ctrlIdx(1) strong(2 BE) weak(2 BE) durMs(2 BE) flags(1) [R(1) G(1) B(1)] + // + // `flags` bit 0 set ⇒ trailing R/G/B bytes are present (DS4 lightbar). + static std::optional parseRumbleMessage(const std::uint8_t* payload, + std::size_t len); + void startHeartbeat(); void stopHeartbeat(); void startReceiveLoop(); @@ -112,6 +149,12 @@ class SatelliteClient { std::atomic lastControllerAck_{-1}; std::atomic vigemAvailable_{-1}; std::atomic activeControllerCount_{-1}; + + // Read on every parsed MSG_RUMBLE on the receive thread; written from + // the owning thread (Qt main) via setRumbleHandler. A short critical + // section (handler copy under lock) keeps the hot-path call unlocked. + std::mutex rumbleHandlerMtx_; + RumbleHandler rumbleHandler_; }; } // namespace dish::net diff --git a/src/Network/WifiConnection.cpp b/src/Network/WifiConnection.cpp index 98bff7b..a1de5f8 100644 --- a/src/Network/WifiConnection.cpp +++ b/src/Network/WifiConnection.cpp @@ -53,6 +53,7 @@ void WifiConnection::markConnected(std::shared_ptr client, onDead_ = std::move(onDead); client->resetControllerAck(); + if (rumbleHandler_) { client->setRumbleHandler(rumbleHandler_); } client->startReceiveLoop(); client->startHeartbeat(); @@ -180,4 +181,11 @@ void WifiConnection::sendReport(std::uint16_t buttons, std::uint8_t lt, std::uin } } +void WifiConnection::setRumbleHandler(RumbleHandler handler) { + rumbleHandler_ = std::move(handler); + // Apply immediately if a session is already live; otherwise markConnected + // will pick up the new handler the next time it runs. + if (auto c = clientRef_.get()) { c->setRumbleHandler(rumbleHandler_); } +} + } // namespace dish::net diff --git a/src/Network/WifiConnection.h b/src/Network/WifiConnection.h index 72c2350..a00b0e3 100644 --- a/src/Network/WifiConnection.h +++ b/src/Network/WifiConnection.h @@ -70,6 +70,14 @@ class WifiConnection : public QObject { void sendReport(std::uint16_t buttons, std::uint8_t lt, std::uint8_t rt, std::int16_t lx, std::int16_t ly, std::int16_t rx, std::int16_t ry); + // Install the per-connection rumble handler. The handler is invoked from + // the SatelliteClient's receive thread on every MSG_RUMBLE we decode. + // Stored on the WifiConnection (not the per-session SatelliteClient) so + // it survives reconnects: markConnected() re-installs it on the new + // client instance. + using RumbleHandler = std::function; + void setRumbleHandler(RumbleHandler handler); + signals: void changed(); void errorOccurred(const QString& message); @@ -102,6 +110,10 @@ class WifiConnection : public QObject { std::function onDead_; bool controllerAdded_ = false; int pendingControllerType_ = 0; + + // Set once during composition; re-applied to each fresh SatelliteClient + // in markConnected() so we don't lose rumble across reconnects. + RumbleHandler rumbleHandler_; }; } // namespace dish::net diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e62f2ef..929ee9d 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -23,7 +23,8 @@ set(DISH_TEST_SOURCES test_gamepad_input_processor.cpp test_pairing_client_classify.cpp test_screen_wake_controller.cpp - test_freedesktop_screensaver_inhibitor.cpp) + test_freedesktop_screensaver_inhibitor.cpp + test_satellite_client_rumble.cpp) add_executable(DishTests ${DISH_TEST_SOURCES}) target_link_libraries(DishTests diff --git a/tests/test_satellite_client_rumble.cpp b/tests/test_satellite_client_rumble.cpp new file mode 100644 index 0000000..1cf248d --- /dev/null +++ b/tests/test_satellite_client_rumble.cpp @@ -0,0 +1,181 @@ +// SPDX-License-Identifier: LGPL-3.0-or-later +// Copyright (C) 2026 Dish contributors. + +// Coverage for SatelliteClient::parseRumbleMessage — the pure decoder for +// the satellite → dish MSG_RUMBLE payload (wire layout described in the +// declaration). The full I/O path (decrypt + dispatch) is intentionally +// out of scope for this test file; it would require driving the receive +// loop with a fake socket. The decoder is the only part that has its own +// branching logic worth pinning down with unit tests, and it's exposed +// publicly + statically for exactly that reason. + +#include "Network/SatelliteClient.h" + +#include + +#include +#include +#include + +using dish::net::SatelliteClient; + +namespace { + +// Build the mandatory 8-byte rumble payload (no lightbar). Matches the +// producer side in satellite/src/adapters/client_adapter.cpp::sendRumble. +std::array mandatoryPayload(std::uint8_t ctrlIdx, std::uint16_t strong, + std::uint16_t weak, std::uint16_t dur) { + return { + ctrlIdx, + static_cast(strong >> 8), + static_cast(strong & 0xFF), + static_cast(weak >> 8), + static_cast(weak & 0xFF), + static_cast(dur >> 8), + static_cast(dur & 0xFF), + 0x00, // flags = 0 ⇒ no lightbar + }; +} + +// 11-byte payload with the lightbar flag set + RGB tail. +std::array lightbarPayload(std::uint8_t ctrlIdx, std::uint16_t strong, + std::uint16_t weak, std::uint16_t dur, + std::uint8_t r, std::uint8_t g, std::uint8_t b) { + return { + ctrlIdx, + static_cast(strong >> 8), + static_cast(strong & 0xFF), + static_cast(weak >> 8), + static_cast(weak & 0xFF), + static_cast(dur >> 8), + static_cast(dur & 0xFF), + 0x01, // flags bit 0 = lightbar present + r, g, b, + }; +} + +} // namespace + +TEST_CASE("parseRumbleMessage decodes mandatory fields", "[rumble]") { + auto p = mandatoryPayload(/*ctrlIdx=*/3, /*strong=*/0xABCD, /*weak=*/0x1234, /*dur=*/500); + const auto rm = SatelliteClient::parseRumbleMessage(p.data(), p.size()); + REQUIRE(rm.has_value()); + REQUIRE(rm->controllerIndex == 3); + REQUIRE(rm->strongMagnitude == 0xABCD); + REQUIRE(rm->weakMagnitude == 0x1234); + REQUIRE(rm->durationMs == 500); + REQUIRE_FALSE(rm->hasLightbar); + REQUIRE(rm->lightbarR == 0); + REQUIRE(rm->lightbarG == 0); + REQUIRE(rm->lightbarB == 0); +} + +TEST_CASE("parseRumbleMessage decodes a stop request (all zero magnitudes)", "[rumble]") { + auto p = mandatoryPayload(/*ctrlIdx=*/0, /*strong=*/0, /*weak=*/0, /*dur=*/0); + const auto rm = SatelliteClient::parseRumbleMessage(p.data(), p.size()); + REQUIRE(rm.has_value()); + REQUIRE(rm->strongMagnitude == 0); + REQUIRE(rm->weakMagnitude == 0); + REQUIRE(rm->durationMs == 0); +} + +TEST_CASE("parseRumbleMessage decodes max-magnitude payload without overflow", "[rumble]") { + auto p = mandatoryPayload(/*ctrlIdx=*/0xFF, /*strong=*/0xFFFF, /*weak=*/0xFFFF, /*dur=*/0xFFFF); + const auto rm = SatelliteClient::parseRumbleMessage(p.data(), p.size()); + REQUIRE(rm.has_value()); + REQUIRE(rm->controllerIndex == 0xFF); + REQUIRE(rm->strongMagnitude == 0xFFFF); + REQUIRE(rm->weakMagnitude == 0xFFFF); + REQUIRE(rm->durationMs == 0xFFFF); +} + +TEST_CASE("parseRumbleMessage decodes lightbar tail", "[rumble]") { + auto p = lightbarPayload(/*ctrlIdx=*/1, /*strong=*/0x0100, /*weak=*/0x0080, /*dur=*/250, + /*r=*/0xDE, /*g=*/0xAD, /*b=*/0xBE); + const auto rm = SatelliteClient::parseRumbleMessage(p.data(), p.size()); + REQUIRE(rm.has_value()); + REQUIRE(rm->hasLightbar); + REQUIRE(rm->lightbarR == 0xDE); + REQUIRE(rm->lightbarG == 0xAD); + REQUIRE(rm->lightbarB == 0xBE); +} + +TEST_CASE("parseRumbleMessage rejects truncated mandatory section", "[rumble]") { + // Anything shorter than 8 bytes is malformed — the satellite never emits + // such a packet, but a malicious / racing peer could. + std::array shortPayload{}; + REQUIRE_FALSE(SatelliteClient::parseRumbleMessage(shortPayload.data(), shortPayload.size()).has_value()); + + REQUIRE_FALSE(SatelliteClient::parseRumbleMessage(nullptr, 0).has_value()); + REQUIRE_FALSE(SatelliteClient::parseRumbleMessage(shortPayload.data(), 0).has_value()); +} + +TEST_CASE("parseRumbleMessage rejects lightbar flag with truncated tail", "[rumble]") { + // Flags say "lightbar present" but the payload doesn't carry the 3 RGB + // bytes. Treat as malformed rather than guessing zeros — that would mask + // wire-protocol bugs on the satellite side. + auto p = mandatoryPayload(0, 0, 0, 0); + p[7] = 0x01; // flag set, but only 8 bytes total + REQUIRE_FALSE(SatelliteClient::parseRumbleMessage(p.data(), p.size()).has_value()); + + // 10 bytes is also short — RGB needs 3. + std::array p10{}; + p10[7] = 0x01; + REQUIRE_FALSE(SatelliteClient::parseRumbleMessage(p10.data(), p10.size()).has_value()); +} + +TEST_CASE("parseRumbleMessage tolerates extra trailing bytes (forward-compat)", "[rumble]") { + // Future protocol extensions may append fields after the lightbar tail. + // The decoder must return successfully and ignore the unknown bytes. + std::vector p(20, 0xAA); + auto base = lightbarPayload(2, 100, 50, 700, 1, 2, 3); + std::copy(base.begin(), base.end(), p.begin()); + const auto rm = SatelliteClient::parseRumbleMessage(p.data(), p.size()); + REQUIRE(rm.has_value()); + REQUIRE(rm->controllerIndex == 2); + REQUIRE(rm->strongMagnitude == 100); + REQUIRE(rm->lightbarR == 1); + REQUIRE(rm->lightbarG == 2); + REQUIRE(rm->lightbarB == 3); +} + +TEST_CASE("parseRumbleMessage flags bit 1+ are reserved (treated as not-lightbar)", "[rumble]") { + // Only bit 0 currently means anything. Higher bits should NOT toggle + // lightbar parsing — that's reserved for future use. Test that the + // decoder strictly checks bit 0. + auto p = mandatoryPayload(0, 0, 0, 0); + p[7] = 0x02; // bit 1 set, bit 0 clear + const auto rm = SatelliteClient::parseRumbleMessage(p.data(), p.size()); + REQUIRE(rm.has_value()); + REQUIRE_FALSE(rm->hasLightbar); +} + +TEST_CASE("parseRumbleMessage handles big-endian boundary cases", "[rumble]") { + // Cover the byte-swap path: 0x00FF (low byte set), 0xFF00 (high byte set), + // and 0x0100 (a value where naive little-endian read would be wrong). + SECTION("low byte only") { + auto p = mandatoryPayload(0, 0x00FF, 0x00FF, 0x00FF); + const auto rm = SatelliteClient::parseRumbleMessage(p.data(), p.size()); + REQUIRE(rm.has_value()); + REQUIRE(rm->strongMagnitude == 0x00FF); + REQUIRE(rm->weakMagnitude == 0x00FF); + REQUIRE(rm->durationMs == 0x00FF); + } + SECTION("high byte only") { + auto p = mandatoryPayload(0, 0xFF00, 0xFF00, 0xFF00); + const auto rm = SatelliteClient::parseRumbleMessage(p.data(), p.size()); + REQUIRE(rm.has_value()); + REQUIRE(rm->strongMagnitude == 0xFF00); + REQUIRE(rm->weakMagnitude == 0xFF00); + REQUIRE(rm->durationMs == 0xFF00); + } + SECTION("value that flips meaning if endianness is wrong") { + // 0x0100 BE = 256 (correct); LE = 0x0001 (wrong). + auto p = mandatoryPayload(0, 0x0100, 0x0100, 0x0100); + const auto rm = SatelliteClient::parseRumbleMessage(p.data(), p.size()); + REQUIRE(rm.has_value()); + REQUIRE(rm->strongMagnitude == 0x0100); + REQUIRE(rm->weakMagnitude == 0x0100); + REQUIRE(rm->durationMs == 0x0100); + } +}