From d6231688c57ceb5bdc80c88df23b952d8ca30eb0 Mon Sep 17 00:00:00 2001 From: Abner Gabriel de Souza Date: Thu, 6 Aug 2026 12:45:44 -0300 Subject: [PATCH 1/2] feat(hid,core): drive solid colour over RgbEffects (0x8071) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit G-series wireless mice expose the per-cluster RgbEffects engine (0x8071) instead of ColorLedEffects (0x8070) — a G903 LIGHTSPEED reports 0x8071 and neither 0x8070 nor 0x8080. The 0x8071 wrapper already existed in openlogi-hidpp but had no consumer, so those devices had unreachable LEDs and Capabilities::lighting stayed false, denying them a Lighting tab in the GUI. - hid: set_keyboard_color grows an 0x8071 rung above 0x8070/0x8080. It takes software control of the clusters (required before setRgbClusterEffect), reads the cluster count, and applies the fixed effect to every cluster — a G903's logo and DPI indicator are separate clusters, so writing only the first leaves it half-painted. The fixed effect is located by effectID rather than by a guessed index, since effect indices are per-cluster and firmware-ordered. Volatile like the 0x8070 path, so a colour pick overrides the running onboard effect without spending flash cycles. - core: Capabilities::lighting counts 0x8071, so those devices offer the existing Lighting panel. No IPC or config change — the panel already sends a plain RGB triple. - cli: diag lighting gains --method rgb and picks its device via select_device, so it can reach a mouse behind a receiver instead of only wired keyboards. Verified on a G903 LIGHTSPEED (046d:c539 receiver): cluster_count=2, both clusters driven, red/green/blue all applied via Auto and --method rgb. --- crates/openlogi-cli/src/cmd/diag.rs | 2 +- crates/openlogi-cli/src/cmd/diag/lighting.rs | 65 ++----- crates/openlogi-core/src/device.rs | 21 +- crates/openlogi-hid/src/write/lighting.rs | 191 +++++++++++++++++-- 4 files changed, 211 insertions(+), 68 deletions(-) diff --git a/crates/openlogi-cli/src/cmd/diag.rs b/crates/openlogi-cli/src/cmd/diag.rs index 3a9e0a74a..a46fa02c7 100644 --- a/crates/openlogi-cli/src/cmd/diag.rs +++ b/crates/openlogi-cli/src/cmd/diag.rs @@ -27,7 +27,7 @@ pub enum DiagCmd { Dpi(dpi::DpiArgs), /// Read SmartShift mode → toggle → read back → toggle back → report. Smartshift(smartshift::SmartshiftArgs), - /// Set a wired RGB keyboard to a solid colour (e.g. `ff0000` for red). + /// Set a device's RGB LEDs to a solid colour (e.g. `ff0000` for red). Lighting(lighting::LightingArgs), /// Read or set the HID++ 0x2121 wheel reporting resolution. Wheel(wheel::WheelArgs), diff --git a/crates/openlogi-cli/src/cmd/diag/lighting.rs b/crates/openlogi-cli/src/cmd/diag/lighting.rs index a6cde68a8..b4e219839 100644 --- a/crates/openlogi-cli/src/cmd/diag/lighting.rs +++ b/crates/openlogi-cli/src/cmd/diag/lighting.rs @@ -1,19 +1,24 @@ -//! `openlogi diag lighting ` — set a wired RGB keyboard to a solid -//! colour via HID++ `PerKeyLighting` (0x8080). +//! `openlogi diag lighting ` — set a device's RGB LEDs to a solid +//! colour via HID++ `RgbEffects` (0x8071), `ColorLedEffects` (0x8070) or +//! `PerKeyLighting` (0x8080). //! -//! Targets the first online direct-attached (USB) Logitech device — i.e. a -//! wired G-series keyboard — by VID/PID, so it isn't tied to one model. +//! Picks the first online device exposing one of those, so it reaches a wireless +//! mouse behind a receiver as well as a wired keyboard. -use anyhow::{Result, anyhow}; +use anyhow::Result; use clap::{Args, ValueEnum}; use openlogi_core::color::Rgb; -use openlogi_hid::{DeviceRoute, LightingMethod}; +use openlogi_hid::LightingMethod; + +use crate::cmd::diag::select_device; #[derive(Debug, Clone, Copy, ValueEnum)] pub enum Method { - /// Prefer 0x8070 ColorLedEffects, fall back to 0x8080 per-key (default). + /// Walk 0x8071 → 0x8070 → 0x8080, taking the first exposed (default). Auto, - /// Force 0x8070 ColorLedEffects (the fixed-effect onboard override). + /// Force 0x8071 RgbEffects (the per-cluster fixed-effect override). + Rgb, + /// Force 0x8070 ColorLedEffects (the per-zone fixed-effect override). Effects, /// Force 0x8080 PerKeyLighting (the per-key stream). Perkey, @@ -23,6 +28,7 @@ impl From for LightingMethod { fn from(m: Method) -> Self { match m { Method::Auto => Self::Auto, + Method::Rgb => Self::RgbEffects, Method::Effects => Self::Effects, Method::Perkey => Self::PerKey, } @@ -34,8 +40,8 @@ pub struct LightingArgs { /// Colour as `RRGGBB` hex (e.g. `ff0000` for red). pub color: String, - /// Run against the wired device whose name contains this string - /// (case-insensitive). Useful when several keyboards are connected. + /// Run against the device whose name contains this string + /// (case-insensitive). Useful when several lit devices are connected. #[arg(long, value_name = "NAME")] pub device: Option, @@ -48,42 +54,9 @@ pub async fn run(args: LightingArgs) -> Result<()> { let color: Rgb = args.color.trim_start_matches('#').parse()?; let (r, g, b) = color.components(); - let device_query = args.device; - let needle = device_query.as_deref().map(str::to_lowercase); - - let inventories = openlogi_hid::enumerate().await?; - let (route, name) = inventories - .iter() - .find_map(|inv| { - // Direct (USB-wired) devices carry no receiver UID — that's the - // wired keyboard. Bolt/Unifying receivers (mice) are skipped. - if inv.receiver.unique_id.is_some() { - return None; - } - let paired = inv.paired.iter().find(|p| p.online)?; - let name = paired.codename.clone().unwrap_or_else(|| { - format!( - "{:04x}:{:04x}", - inv.receiver.vendor_id, inv.receiver.product_id - ) - }); - if let Some(ref n) = needle - && !name.to_lowercase().contains(n.as_str()) - { - return None; - } - let route = DeviceRoute::Direct { - vendor_id: inv.receiver.vendor_id, - product_id: inv.receiver.product_id, - }; - Some((route, name)) - }) - .ok_or_else(|| match &device_query { - Some(q) => anyhow!("no wired device matches `--device {q}`"), - None => { - anyhow!("no wired (direct-USB) Logitech device found — is the keyboard plugged in?") - } - })?; + // The three features `set_keyboard_color` can drive — auto-skip devices with + // no LEDs to paint. + let (route, name) = select_device(args.device.as_deref(), &[0x8071, 0x8070, 0x8080]).await?; let method: LightingMethod = args.method.into(); println!("setting {name} ({route}) to #{r:02x}{g:02x}{b:02x} via {method:?}"); diff --git a/crates/openlogi-core/src/device.rs b/crates/openlogi-core/src/device.rs index 2c4e9795b..23c28a77e 100644 --- a/crates/openlogi-core/src/device.rs +++ b/crates/openlogi-core/src/device.rs @@ -91,9 +91,10 @@ pub struct Capabilities { /// Adjustable pointer resolution — HID++ `0x2201` / `0x2202` (AdjustableDpi). pub pointer: bool, /// Solid-colour RGB the lighting panel can actually drive — HID++ - /// `ColorLedEffects` (`0x8070`) or `PerKeyLighting` (`0x8080`), the features - /// `set_keyboard_color` writes. Backlight-only families aren't driven by the - /// panel, so they don't flip this and don't earn an inert Lighting tab. + /// `RgbEffects` (`0x8071`), `ColorLedEffects` (`0x8070`) or `PerKeyLighting` + /// (`0x8080`), the features `set_keyboard_color` writes. Backlight-only + /// families aren't driven by the panel, so they don't flip this and don't + /// earn an inert Lighting tab. pub lighting: bool, /// Native vertical wheel inversion — HID++ `0x2121 HiResWheel` with the /// firmware-reported `has_invert` capability. @@ -111,11 +112,13 @@ impl Capabilities { pub fn from_feature_ids(ids: &[u16]) -> Self { const BUTTONS: [u16; 5] = [0x1b00, 0x1b01, 0x1b02, 0x1b03, 0x1b04]; const POINTER: [u16; 2] = [0x2201, 0x2202]; - // PerKeyLighting (0x8080) and ColorLedEffects (0x8070) — both now driven - // by `set_keyboard_color` (it prefers 0x8070's fixed effect to override a - // running onboard profile, falling back to 0x8080 per-key). Other families + // RgbEffects (0x8071), ColorLedEffects (0x8070) and PerKeyLighting + // (0x8080) — all three driven by `set_keyboard_color`, which walks them + // in that order (0x8071 is the modern per-cluster engine G-series *mice* + // expose instead of 0x8070; both fixed-effect paths override a running + // onboard profile, and 0x8080 per-key is the last resort). Other families // (backlight 0x198x) stay out so they don't earn a tab the panel can't drive. - const LIGHTING: [u16; 2] = [0x8080, 0x8070]; + const LIGHTING: [u16; 3] = [0x8080, 0x8070, 0x8071]; let has = |family: &[u16]| ids.iter().any(|id| family.contains(id)); Self { buttons: has(&BUTTONS), @@ -436,6 +439,10 @@ mod tests { hires_wheel: true, } ); + // A G903 LIGHTSPEED: RgbEffects (0x8071) is the only lighting feature it + // reports — no 0x8070, no 0x8080 — so it must still earn a Lighting tab. + let g903 = Capabilities::from_feature_ids(&[0x1b04, 0x2201, 0x2121, 0x8071]); + assert!(g903.lighting, "0x8071-only device should offer lighting"); // A wired G-series keyboard: PerKeyLighting (0x8080), no DPI/buttons. let keyboard = Capabilities::from_feature_ids(&[0x0001, 0x8080]); assert_eq!( diff --git a/crates/openlogi-hid/src/write/lighting.rs b/crates/openlogi-hid/src/write/lighting.rs index 41e7bad42..adbebb8e0 100644 --- a/crates/openlogi-hid/src/write/lighting.rs +++ b/crates/openlogi-hid/src/write/lighting.rs @@ -6,6 +6,10 @@ use hidpp::{ feature::{ CreatableFeature, color_led_effects::{ColorLedEffectsFeature, Persistence, ZONE_EFFECT_PARAM_COUNT}, + rgb_effects::{ + CLUSTER_EFFECT_PARAM_COUNT, EventsNotificationFlags, PowerModeTarget, + RgbEffectsFeature, RgbPersistence, SwControlFlags, + }, }, }; use tracing::debug; @@ -22,6 +26,11 @@ const PER_KEY_LIGHTING_FEATURE: u16 = 0x8080; /// (`0x8080`) write can't override on G-series keyboards (the firmware keeps /// replaying its stored effect). Preferred for a solid colour for that reason. const COLOR_LED_EFFECTS_FEATURE: u16 = 0x8070; +/// HID++ `RgbEffects` (`0x8071`) — the per-cluster effect engine that succeeds +/// `0x8070`. G-series *mice* expose this one instead: a G903 LIGHTSPEED reports +/// `0x8071` and neither `0x8070` nor `0x8080`, so without this path its LEDs are +/// unreachable and it earns no Lighting tab at all. +const RGB_EFFECTS_FEATURE: u16 = 0x8071; // HID++ 2.0 report ids: 0x12 is the 64-byte "very long" report that streams a // batch of (keyID, R, G, B) entries; 0x11 is the 20-byte "long" report used both @@ -52,25 +61,40 @@ const MAX_COLOR_LED_EFFECT_ZONES: u8 = 4; // Zones are paced apart because the controller can drop closely-spaced reports. const FRAME_GAP: Duration = Duration::from_millis(8); +// 0x8071 `RgbEffects`: `effectID` 0x0001 is the fixed/solid-colour effect. Only +// the *id* is stable — the effect *index* passed to `setRgbClusterEffect` is +// per-cluster and firmware-ordered, so it's located by id at runtime rather than +// assumed (a device's clusters need not agree on ordering). +const RGB_EFFECT_ID_FIXED: u16 = 0x0001; +// Same reasoning as `MAX_COLOR_LED_EFFECT_ZONES`: bound the per-apply work so a +// malformed cluster count can't stall a colour pick. A G903 reports 2 clusters +// (logo and DPI indicator); 8 leaves room for denser devices. +const MAX_RGB_CLUSTERS: u8 = 8; + /// Which HID++ lighting path drives a solid keyboard colour. [`Auto`] is what /// the GUI/agent use; the explicit variants exist for the `diag` A/B test. /// /// [`Auto`]: LightingMethod::Auto #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum LightingMethod { - /// Prefer `ColorLedEffects` (`0x8070`), falling back to `PerKeyLighting` - /// (`0x8080`) when the device exposes no effect engine. + /// Walk `RgbEffects` (`0x8071`) → `ColorLedEffects` (`0x8070`) → + /// `PerKeyLighting` (`0x8080`), taking the first the device exposes. Auto, - /// Force `ColorLedEffects` (`0x8070`) — the fixed-effect override. + /// Force `RgbEffects` (`0x8071`) — the per-cluster fixed-effect override. + RgbEffects, + /// Force `ColorLedEffects` (`0x8070`) — the per-zone fixed-effect override. Effects, /// Force `PerKeyLighting` (`0x8080`) — the per-key stream. PerKey, } -/// Set a keyboard to a solid `(r, g, b)` colour, choosing the HID++ path -/// automatically: the `0x8070` effect engine (which overrides the onboard -/// profile) when present, else the `0x8080` per-key stream. `FeatureUnsupported` -/// when the device exposes neither. +/// Set a device to a solid `(r, g, b)` colour, choosing the HID++ path +/// automatically: the `0x8071` cluster engine (G-series mice), else the `0x8070` +/// zone engine, else the `0x8080` per-key stream. Both effect engines override a +/// running onboard profile. `FeatureUnsupported` when the device exposes none. +/// +/// Named for keyboards because they were the only lit devices when it landed; it +/// drives any device with one of those three features. pub async fn set_keyboard_color( route: &DeviceRoute, r: u8, @@ -80,9 +104,10 @@ pub async fn set_keyboard_color( set_keyboard_color_with(route, LightingMethod::Auto, r, g, b).await } -/// [`set_keyboard_color`] with an explicit [`LightingMethod`]. `Auto` tries -/// `0x8070` first and falls back to `0x8080` only when the effect engine is -/// absent (a missing-`0x8070` `FeatureUnsupported`); any other error propagates. +/// [`set_keyboard_color`] with an explicit [`LightingMethod`]. `Auto` steps down +/// the ladder only on a `FeatureUnsupported` naming the rung's own feature — any +/// other error propagates, so a present-but-failing engine surfaces as a failure +/// instead of silently repainting the device key-by-key. pub async fn set_keyboard_color_with( route: &DeviceRoute, method: LightingMethod, @@ -93,18 +118,38 @@ pub async fn set_keyboard_color_with( match method { LightingMethod::PerKey => set_color_per_key(route, r, g, b).await, LightingMethod::Effects => set_color_effects(route, r, g, b).await, - LightingMethod::Auto => match set_color_effects(route, r, g, b).await { + LightingMethod::RgbEffects => set_color_rgb_effects(route, r, g, b).await, + LightingMethod::Auto => match set_color_rgb_effects(route, r, g, b).await { Err(WriteError::FeatureUnsupported { feature_hex }) - if feature_hex == COLOR_LED_EFFECTS_FEATURE => + if feature_hex == RGB_EFFECTS_FEATURE => { - debug!("no 0x8070 effect engine — falling back to 0x8080 per-key"); - set_color_per_key(route, r, g, b).await + debug!("no 0x8071 cluster engine — trying the 0x8070 zone engine"); + set_color_effects_or_per_key(route, r, g, b).await } other => other, }, } } +/// The `Auto` ladder below `0x8071`: the `0x8070` zone engine, falling back to +/// the `0x8080` per-key stream when the device exposes no effect engine. +async fn set_color_effects_or_per_key( + route: &DeviceRoute, + r: u8, + g: u8, + b: u8, +) -> Result<(), WriteError> { + match set_color_effects(route, r, g, b).await { + Err(WriteError::FeatureUnsupported { feature_hex }) + if feature_hex == COLOR_LED_EFFECTS_FEATURE => + { + debug!("no 0x8070 effect engine — falling back to 0x8080 per-key"); + set_color_per_key(route, r, g, b).await + } + other => other, + } +} + /// Resolve `route`'s runtime feature *index* for HID++ `feature_id`. `Ok(None)` /// when the device doesn't expose it; the index differs per device, so callers /// can't hard-code it. @@ -192,6 +237,124 @@ fn classify_lighting_error(error: hidpp::protocol::v20::Hidpp20Error) -> WriteEr classify_hidpp_error(error, HidppOperation::Lighting, ColorLedEffectsFeature::ID) } +/// Classify a HID++ error from the `RgbEffects` functions. +fn classify_rgb_error(error: hidpp::protocol::v20::Hidpp20Error) -> WriteError { + classify_hidpp_error(error, HidppOperation::Lighting, RgbEffectsFeature::ID) +} + +/// Set a solid colour via `RgbEffects` (`0x8071`): the fixed effect on every +/// cluster, in RAM only. `FeatureUnsupported` when the device exposes no +/// `0x8071`. +/// +/// Every cluster is driven, not just the first: a G903 splits its LEDs into a +/// logo cluster and a DPI-indicator cluster, and lighting only one leaves the +/// device visibly half-painted. +/// +/// Volatile like the `0x8070` path — the effect shows live and overrides the +/// running onboard effect without touching EEPROM, and the agent re-applies the +/// saved colour on device arrival rather than spending flash cycles per pick. +async fn set_color_rgb_effects(route: &DeviceRoute, r: u8, g: u8, b: u8) -> Result<(), WriteError> { + let index = route.device_index(); + with_route(route, move |channel| async move { + let mut device = Device::new(std::sync::Arc::clone(&channel), index) + .await + .map_err(|_| WriteError::DeviceUnreachable { index })?; + let feature = open_feature::(&mut device).await?; + + // `setRgbClusterEffect` is refused until software takes the clusters off + // the device's own effect engine, so claim control before writing. Not + // requesting POWER_MODES: this path never drives power modes, and asking + // for control we don't use would keep the firmware from managing its own + // RGB power saving. + feature + .set_sw_control( + SwControlFlags::ALL_CLUSTERS, + EventsNotificationFlags::empty(), + ) + .await + .map_err(classify_rgb_error)?; + + let cluster_count = feature + .get_device_info() + .await + .map_err(classify_rgb_error)? + .cluster_count; + if cluster_count == 0 { + // Unlike 0x8070 there's no legacy zone count worth guessing here, and + // a blind write would target a cluster the device never claimed. + debug!(index, "0x8071 reported zero clusters — nothing to drive"); + return Err(WriteError::UnsupportedResponse { + operation: HidppOperation::Lighting, + feature_hex: RGB_EFFECTS_FEATURE, + }); + } + let clusters_to_write = cluster_count.min(MAX_RGB_CLUSTERS); + if cluster_count > MAX_RGB_CLUSTERS { + debug!( + index, + cluster_count, + capped_cluster_count = MAX_RGB_CLUSTERS, + "0x8071 cluster count capped to the per-apply write limit" + ); + } + + let mut params = [0u8; CLUSTER_EFFECT_PARAM_COUNT]; + params[0] = r; + params[1] = g; + params[2] = b; + for cluster in 0..clusters_to_write { + let effect = fixed_effect_index(&feature, cluster).await?; + feature + .set_rgb_cluster_effect( + cluster, + effect, + params, + RgbPersistence::VOLATILE, + PowerModeTarget::FullPower, + ) + .await + .map_err(classify_rgb_error)?; + tokio::time::sleep(FRAME_GAP).await; + } + debug!( + index, + cluster_count, clusters_to_write, r, g, b, "set device colour via 0x8071" + ); + Ok(()) + }) + .await +} + +/// Locate `cluster`'s fixed (solid-colour) effect index by scanning its effects +/// for [`RGB_EFFECT_ID_FIXED`]. +/// +/// `UnsupportedResponse` when the cluster offers no fixed effect — it answered, +/// but with nothing this path can drive (an animation-only cluster). +async fn fixed_effect_index(feature: &RgbEffectsFeature, cluster: u8) -> Result { + let effect_count = feature + .get_cluster_info(cluster) + .await + .map_err(classify_rgb_error)? + .effects_number; + for effect in 0..effect_count { + let info = feature + .get_effect_info(cluster, effect) + .await + .map_err(classify_rgb_error)?; + if info.effect_id == RGB_EFFECT_ID_FIXED { + return Ok(effect); + } + } + debug!( + cluster, + effect_count, "0x8071 cluster exposes no fixed effect" + ); + Err(WriteError::UnsupportedResponse { + operation: HidppOperation::Lighting, + feature_hex: RGB_EFFECTS_FEATURE, + }) +} + /// Set a solid colour via `PerKeyLighting` (`0x8080`): stream every key's colour /// in 64-byte `0x12` frames, then commit. `FeatureUnsupported` when the device /// exposes no `0x8080`. From ab919d04749890b11a422bc0d750e977190765a9 Mon Sep 17 00:00:00 2001 From: Abner Gabriel de Souza Date: Thu, 6 Aug 2026 13:42:01 -0300 Subject: [PATCH 2/2] feat(hid,cli): report and undo 0x8071 software control MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A colour write claims RgbEffects software control and must keep it: the claim is what holds the colour, and releasing it hands every cluster back to the device's own effect engine, which resumes its onboard effect and discards the colour (observed on a G903 LIGHTSPEED). The claim is all-or-nothing, so on a device with a cluster this path cannot light it silently extinguishes that cluster. A G903's Primary cluster (the DPI indicator) accepts every write without error and never lights — not with the fixed effect, not with the cycling effect the firmware itself runs there — so claiming control turns it off with no way to take over. Nothing observable distinguishes that cluster beforehand. - hid: dump_rgb_clusters reports the cluster/effect table plus the current software-control state; release_rgb_control hands the clusters back without a power cycle. - cli: diag lighting gains --info (dump and exit) and --release-control (undo and exit); the colour is required unless one of them is given. --- crates/openlogi-cli/src/cmd/diag/lighting.rs | 76 ++++++++- crates/openlogi-cli/src/lib.rs | 23 ++- crates/openlogi-hid/src/lib.rs | 5 +- crates/openlogi-hid/src/write.rs | 5 +- crates/openlogi-hid/src/write/lighting.rs | 154 ++++++++++++++++++- 5 files changed, 250 insertions(+), 13 deletions(-) diff --git a/crates/openlogi-cli/src/cmd/diag/lighting.rs b/crates/openlogi-cli/src/cmd/diag/lighting.rs index b4e219839..01ae2d528 100644 --- a/crates/openlogi-cli/src/cmd/diag/lighting.rs +++ b/crates/openlogi-cli/src/cmd/diag/lighting.rs @@ -38,7 +38,18 @@ impl From for LightingMethod { #[derive(Debug, Args)] pub struct LightingArgs { /// Colour as `RRGGBB` hex (e.g. `ff0000` for red). - pub color: String, + #[arg(required_unless_present_any = ["info", "release_control"])] + pub color: Option, + + /// Dump the device's 0x8071 clusters and their effects, then exit without + /// writing a colour. + #[arg(long)] + pub info: bool, + + /// Hand the 0x8071 clusters back to the device's own effect engine and + /// exit, undoing the software control a colour write takes. + #[arg(long, conflicts_with = "info")] + pub release_control: bool, /// Run against the device whose name contains this string /// (case-insensitive). Useful when several lit devices are connected. @@ -51,20 +62,73 @@ pub struct LightingArgs { } pub async fn run(args: LightingArgs) -> Result<()> { - let color: Rgb = args.color.trim_start_matches('#').parse()?; - let (r, g, b) = color.components(); + // Parse before touching hardware so a typo fails fast rather than after a + // device round-trip. + let color = args + .color + .as_deref() + .map(|c| c.trim_start_matches('#').parse::()) + .transpose()?; // The three features `set_keyboard_color` can drive — auto-skip devices with // no LEDs to paint. let (route, name) = select_device(args.device.as_deref(), &[0x8071, 0x8070, 0x8080]).await?; + println!("device: {name} ({route})"); + + if args.info { + return print_rgb_info(&route).await; + } + + if args.release_control { + openlogi_hid::release_rgb_control(&route).await?; + println!("released software control — the device drives its own RGB again"); + return Ok(()); + } + let Some(color) = color else { + // Unreachable via clap (`required_unless_present`), but the type is an + // Option either way. + anyhow::bail!("a colour is required unless --info is given"); + }; + let (r, g, b) = color.components(); let method: LightingMethod = args.method.into(); - println!("setting {name} ({route}) to #{r:02x}{g:02x}{b:02x} via {method:?}"); + println!("setting {name} to #{r:02x}{g:02x}{b:02x} via {method:?}"); openlogi_hid::set_keyboard_color_with(&route, method, r, g, b).await?; println!("done — {name} should now be solid #{r:02x}{g:02x}{b:02x}"); Ok(()) } +/// Dump the `0x8071` cluster/effect table so a device whose LEDs don't all +/// respond can be told apart from one OpenLogi simply isn't addressing. +async fn print_rgb_info(route: &openlogi_hid::DeviceRoute) -> Result<()> { + let (control, clusters) = openlogi_hid::dump_rgb_clusters(route).await?; + println!( + " software control: all_clusters={} power_modes={}", + control.all_clusters, control.power_modes + ); + if clusters.is_empty() { + println!(" no 0x8071 clusters reported"); + return Ok(()); + } + for cluster in &clusters { + println!( + " cluster {} location={:#06x} effect_persistency={} multiled_pattern={} effects={}", + cluster.index, + cluster.location, + cluster.effect_persistency, + cluster.multiled_pattern, + cluster.effects.len() + ); + for effect in &cluster.effects { + println!( + " effect {:>2} id={:#06x} caps={:#06x} period={}ms", + effect.index, effect.effect_id, effect.effect_capabilities, effect.effect_period + ); + } + } + Ok(()) +} + #[cfg(test)] #[allow( clippy::unwrap_used, @@ -78,7 +142,9 @@ mod color_validation_tests { fn args(color: &str) -> LightingArgs { LightingArgs { - color: color.to_string(), + color: Some(color.to_string()), + info: false, + release_control: false, device: None, method: Method::Auto, } diff --git a/crates/openlogi-cli/src/lib.rs b/crates/openlogi-cli/src/lib.rs index 97af9981f..d6df7a725 100644 --- a/crates/openlogi-cli/src/lib.rs +++ b/crates/openlogi-cli/src/lib.rs @@ -115,13 +115,34 @@ mod tests { match cli.cmd.expect("subcommand present") { Command::Diag(DiagCmd::Lighting(args)) => { - assert_eq!(args.color, "ff0000"); + assert_eq!(args.color.as_deref(), Some("ff0000")); assert!(matches!(args.method, Method::Effects)); } other => panic!("expected Diag(Lighting), got {other:?}"), } } + #[test] + fn lighting_info_makes_the_colour_optional() { + let cli = Cli::try_parse_from(["openlogi", "diag", "lighting", "--info"]) + .expect("--info parses without a colour"); + + match cli.cmd.expect("subcommand present") { + Command::Diag(DiagCmd::Lighting(args)) => { + assert!(args.info); + assert!(args.color.is_none()); + } + other => panic!("expected Diag(Lighting), got {other:?}"), + } + } + + #[test] + fn lighting_without_colour_or_info_is_rejected() { + // The colour stays mandatory for a write — `--info` and + // `--release-control` are the only flags that excuse it. + assert!(Cli::try_parse_from(["openlogi", "diag", "lighting"]).is_err()); + } + #[test] fn lighting_rejects_unknown_method() { let result = Cli::try_parse_from([ diff --git a/crates/openlogi-hid/src/lib.rs b/crates/openlogi-hid/src/lib.rs index b6cf15475..3c886ad6e 100644 --- a/crates/openlogi-hid/src/lib.rs +++ b/crates/openlogi-hid/src/lib.rs @@ -45,8 +45,9 @@ pub use route::{BOLT_PIDS, DIRECT_DEVICE_INDEX, DeviceRoute, UNIFYING_PIDS}; pub use smartshift::{AUTO_DISENGAGE_PERMANENT, SmartShiftMode, SmartShiftStatus}; pub use write::{ DpiCapabilities, DpiInfo, FeatureEntry, HidppFeatureErrorKind, HidppOperation, LightingMethod, - ReprogControlEntry, SharedChannel, WriteError, dump_features, dump_reprog_controls, get_dpi, - get_dpi_info, get_smartshift_status, set_dpi, set_dpi_on, set_keyboard_color, + ReprogControlEntry, RgbClusterEntry, RgbControlState, RgbEffectEntry, SharedChannel, + WriteError, dump_features, dump_reprog_controls, dump_rgb_clusters, get_dpi, get_dpi_info, + get_smartshift_status, release_rgb_control, set_dpi, set_dpi_on, set_keyboard_color, set_keyboard_color_with, set_smartshift, set_smartshift_on, set_smartshift_sensitivity, toggle_smartshift, toggle_smartshift_on, }; diff --git a/crates/openlogi-hid/src/write.rs b/crates/openlogi-hid/src/write.rs index e01c79e2e..2ea9cbad0 100644 --- a/crates/openlogi-hid/src/write.rs +++ b/crates/openlogi-hid/src/write.rs @@ -23,7 +23,10 @@ mod smartshift; pub use diagnostics::{FeatureEntry, ReprogControlEntry, dump_features, dump_reprog_controls}; pub use dpi::{DpiCapabilities, DpiInfo, get_dpi, get_dpi_info, set_dpi}; pub use error::{HidppFeatureErrorKind, HidppOperation, WriteError}; -pub use lighting::{LightingMethod, set_keyboard_color, set_keyboard_color_with}; +pub use lighting::{ + LightingMethod, RgbClusterEntry, RgbControlState, RgbEffectEntry, dump_rgb_clusters, + release_rgb_control, set_keyboard_color, set_keyboard_color_with, +}; pub use shared::{SharedChannel, set_dpi_on, set_smartshift_on, toggle_smartshift_on}; pub use smartshift::{ get_smartshift_status, set_smartshift, set_smartshift_sensitivity, toggle_smartshift, diff --git a/crates/openlogi-hid/src/write/lighting.rs b/crates/openlogi-hid/src/write/lighting.rs index adbebb8e0..c2905472b 100644 --- a/crates/openlogi-hid/src/write/lighting.rs +++ b/crates/openlogi-hid/src/write/lighting.rs @@ -61,10 +61,15 @@ const MAX_COLOR_LED_EFFECT_ZONES: u8 = 4; // Zones are paced apart because the controller can drop closely-spaced reports. const FRAME_GAP: Duration = Duration::from_millis(8); -// 0x8071 `RgbEffects`: `effectID` 0x0001 is the fixed/solid-colour effect. Only -// the *id* is stable — the effect *index* passed to `setRgbClusterEffect` is -// per-cluster and firmware-ordered, so it's located by id at runtime rather than -// assumed (a device's clusters need not agree on ordering). +// 0x8071 `RgbEffects`: `effectID` 0x0001 is the fixed/solid-colour effect. The +// `0x8071` spec is not public — this id follows the `0x8070` effect vocabulary +// and was confirmed on a G903 LIGHTSPEED, where writing it to the logo cluster +// produces the requested solid colour. Treat it as a verified observation, not +// as a documented constant. +// +// Only the *id* is stable — the effect *index* passed to `setRgbClusterEffect` +// is per-cluster and firmware-ordered, so it's located by id at runtime rather +// than assumed (a device's clusters need not agree on ordering). const RGB_EFFECT_ID_FIXED: u16 = 0x0001; // Same reasoning as `MAX_COLOR_LED_EFFECT_ZONES`: bound the per-apply work so a // malformed cluster count can't stall a colour pick. A G903 reports 2 clusters @@ -242,6 +247,126 @@ fn classify_rgb_error(error: hidpp::protocol::v20::Hidpp20Error) -> WriteError { classify_hidpp_error(error, HidppOperation::Lighting, RgbEffectsFeature::ID) } +/// Whether software currently holds `RgbEffects` (`0x8071`) control, and from +/// which subsystems. Read by [`dump_rgb_clusters`]. +/// +/// Control is volatile: it is dropped on a power cycle, which hands the clusters +/// back to the firmware's own effect engine. +#[derive(Debug, Clone, Copy)] +pub struct RgbControlState { + /// Software drives all RGB clusters. + pub all_clusters: bool, + /// Software drives the RGB power modes. + pub power_modes: bool, +} + +/// Snapshot of one `RgbEffects` (`0x8071`) cluster. Returned by +/// [`dump_rgb_clusters`] so a device whose LEDs don't all respond can be +/// identified before OpenLogi decides how to drive them. +#[derive(Debug, Clone)] +pub struct RgbClusterEntry { + /// Cluster index, as reported by the device rather than as requested. + pub index: u8, + /// Raw physical-location code of the cluster. + pub location: u16, + /// Whether the cluster can persist an effect to EEPROM. + pub effect_persistency: bool, + /// Whether the cluster supports multi-LED patterns. + pub multiled_pattern: bool, + /// The effects the cluster offers, in firmware order. + pub effects: Vec, +} + +/// One effect offered by an `RgbEffects` cluster. +#[derive(Debug, Clone, Copy)] +pub struct RgbEffectEntry { + /// Effect index within its cluster — what `setRgbClusterEffect` takes. + pub index: u8, + /// Effect type identifier (`effectID`). + pub effect_id: u16, + /// Effect capability bitmask (meaning depends on `effect_id`). + pub effect_capabilities: u16, + /// Effect period in milliseconds, or `0` when not available. + pub effect_period: u16, +} + +/// Hand the `RgbEffects` (`0x8071`) clusters back to the device's own effect +/// engine, dropping any software control OpenLogi took to paint them. +/// +/// `FeatureUnsupported` when the device exposes no `0x8071`. +pub async fn release_rgb_control(route: &DeviceRoute) -> Result<(), WriteError> { + let index = route.device_index(); + with_route(route, move |channel| async move { + let mut device = Device::new(std::sync::Arc::clone(&channel), index) + .await + .map_err(|_| WriteError::DeviceUnreachable { index })?; + let feature = open_feature::(&mut device).await?; + feature + .set_sw_control(SwControlFlags::empty(), EventsNotificationFlags::empty()) + .await + .map_err(classify_rgb_error)?; + debug!(index, "released 0x8071 software control"); + Ok(()) + }) + .await +} + +/// Walk a device's `RgbEffects` (`0x8071`) clusters and the effects each offers. +/// +/// Read-only: takes no software control and writes no effect. +/// `FeatureUnsupported` when the device exposes no `0x8071`. +pub async fn dump_rgb_clusters( + route: &DeviceRoute, +) -> Result<(RgbControlState, Vec), WriteError> { + let index = route.device_index(); + with_route(route, move |channel| async move { + let mut device = Device::new(std::sync::Arc::clone(&channel), index) + .await + .map_err(|_| WriteError::DeviceUnreachable { index })?; + let feature = open_feature::(&mut device).await?; + let sw = feature.get_sw_control().await.map_err(classify_rgb_error)?; + let control = RgbControlState { + all_clusters: sw.control.contains(SwControlFlags::ALL_CLUSTERS), + power_modes: sw.control.contains(SwControlFlags::POWER_MODES), + }; + let cluster_count = feature + .get_device_info() + .await + .map_err(classify_rgb_error)? + .cluster_count; + + let mut out = Vec::new(); + for cluster in 0..cluster_count.min(MAX_RGB_CLUSTERS) { + let info = feature + .get_cluster_info(cluster) + .await + .map_err(classify_rgb_error)?; + let mut effects = Vec::new(); + for effect in 0..info.effects_number { + let e = feature + .get_effect_info(cluster, effect) + .await + .map_err(classify_rgb_error)?; + effects.push(RgbEffectEntry { + index: e.cluster_effect_index, + effect_id: e.effect_id, + effect_capabilities: e.effect_capabilities, + effect_period: e.effect_period, + }); + } + out.push(RgbClusterEntry { + index: info.cluster_index, + location: info.location, + effect_persistency: info.effect_persistency, + multiled_pattern: info.multiled_pattern, + effects, + }); + } + Ok((control, out)) + }) + .await +} + /// Set a solid colour via `RgbEffects` (`0x8071`): the fixed effect on every /// cluster, in RAM only. `FeatureUnsupported` when the device exposes no /// `0x8071`. @@ -253,6 +378,27 @@ fn classify_rgb_error(error: hidpp::protocol::v20::Hidpp20Error) -> WriteError { /// Volatile like the `0x8070` path — the effect shows live and overrides the /// running onboard effect without touching EEPROM, and the agent re-applies the /// saved colour on device arrival rather than spending flash cycles per pick. +/// +/// # Software control is taken and kept +/// +/// `setRgbClusterEffect` is refused until software claims the clusters, and the +/// claim is what *holds* the colour: releasing it hands every cluster back to +/// the device's own effect engine, which immediately resumes its onboard effect +/// and discards the colour (observed on a G903 LIGHTSPEED). So the claim can't +/// be scoped to the write — it lasts until [`release_rgb_control`], a power +/// cycle, or the next colour. +/// +/// The claim is all-or-nothing (`SwControlFlags::ALL_CLUSTERS`), and that has a +/// cost on hardware with a cluster this path can't light. On a G903 the +/// `Primary` cluster (the DPI indicator) accepts every write without error and +/// never lights — not with the fixed effect, not even with the cycling effect +/// the firmware itself runs there. Claiming control therefore *extinguishes* it: +/// the firmware stops driving it and OpenLogi cannot take over. Its logo cluster +/// is unaffected and takes the colour normally. +/// +/// There is no reliable way to detect this before writing — the device reports +/// the cluster, lists a fixed effect for it, and acknowledges the write. Callers +/// that need the device's own lighting back must call [`release_rgb_control`]. async fn set_color_rgb_effects(route: &DeviceRoute, r: u8, g: u8, b: u8) -> Result<(), WriteError> { let index = route.device_index(); with_route(route, move |channel| async move {