diff --git a/crates/openlogi-cli/src/cmd/diag/battery.rs b/crates/openlogi-cli/src/cmd/diag/battery.rs new file mode 100644 index 00000000..18e93c2f --- /dev/null +++ b/crates/openlogi-cli/src/cmd/diag/battery.rs @@ -0,0 +1,32 @@ +//! `openlogi diag battery` — dump the device's raw battery report. +//! +//! Prints exactly what the firmware returns (unified `0x1004` fields, or legacy +//! `0x1000` `discharge_level`/`next_level`/`status`). Run it once on battery and +//! once with the charger plugged in to see how the device reports while charging +//! — e.g. an MX2S returns `discharge_level=0` mid-charge, which is the device's +//! own limitation, not a bug in the read path. + +use anyhow::{Context, Result}; +use clap::Args; + +use crate::cmd::diag::select_device; + +#[derive(Debug, Args)] +pub struct BatteryArgs { + /// Run against the device whose name contains this string + /// (case-insensitive) instead of auto-selecting. + #[arg(long, value_name = "NAME")] + pub device: Option, +} + +pub async fn run(args: BatteryArgs) -> Result<()> { + // 0x1004 UnifiedBattery / 0x1000 BatteryStatus — pick a device with either. + let (route, name) = select_device(args.device.as_deref(), &[0x1000, 0x1004]).await?; + println!("device: {name} ({route})"); + + let line = openlogi_hid::read_battery_raw(&route) + .await + .context("read battery")?; + println!(" {line}"); + Ok(()) +} diff --git a/crates/openlogi-cli/src/cmd/diag/mod.rs b/crates/openlogi-cli/src/cmd/diag/mod.rs index d84f17ac..a14a8660 100644 --- a/crates/openlogi-cli/src/cmd/diag/mod.rs +++ b/crates/openlogi-cli/src/cmd/diag/mod.rs @@ -10,6 +10,7 @@ use anyhow::{Result, anyhow}; use clap::Subcommand; use openlogi_hid::{DeviceRoute, dump_features}; +pub mod battery; pub mod dpi; pub mod features; pub mod lighting; @@ -19,6 +20,8 @@ pub mod smartshift; pub enum DiagCmd { /// Dump every HID++ feature the active device reports. Features(features::FeaturesArgs), + /// Read the raw battery report (0x1004 or 0x1000 fields). + Battery(battery::BatteryArgs), /// Read DPI → write a small delta → read back → restore → report. Dpi(dpi::DpiArgs), /// Read SmartShift mode → toggle → read back → toggle back → report. @@ -31,6 +34,7 @@ impl DiagCmd { pub async fn run(self) -> Result<()> { match self { Self::Features(args) => features::run(args).await, + Self::Battery(args) => battery::run(args).await, Self::Dpi(args) => dpi::run(args).await, Self::Smartshift(args) => smartshift::run(args).await, Self::Lighting(args) => lighting::run(args).await, diff --git a/crates/openlogi-gui/src/app.rs b/crates/openlogi-gui/src/app.rs index 5a171e9d..44c05728 100644 --- a/crates/openlogi-gui/src/app.rs +++ b/crates/openlogi-gui/src/app.rs @@ -830,17 +830,32 @@ fn status_dot(online: bool) -> AnyElement { .into_any_element() } +/// True when the device is charging but still reports 0% — the MX2S `0x1000` +/// firmware can't gauge charge under load, and on a cold start there's no +/// pre-charge % cached to carry forward. Show "Charging" without the bogus 0%. +/// ponytail: cold-start only; once any discharge read is cached, the +/// carry-forward in `inventory.rs` holds it and `percentage` is non-zero. +pub(crate) fn battery_charging_no_reading(b: &BatteryInfo) -> bool { + matches!( + b.status, + BatteryStatus::Charging | BatteryStatus::ChargingSlow + ) && b.percentage == 0 +} + /// Battery readout for a gallery card: a charge/level glyph plus the /// percentage, in the muted metadata style. fn battery_view(b: &BatteryInfo, pal: Palette) -> AnyElement { - h_flex() + let row = h_flex() .gap_1() .items_center() .text_xs() .text_color(pal.text_muted) - .child(Icon::new(battery_icon(b)).size_3()) - .child(format!("{}%", b.percentage)) - .into_any_element() + .child(Icon::new(battery_icon(b)).size_3()); + if battery_charging_no_reading(b) { + row.child(tr!("Charging")).into_any_element() + } else { + row.child(format!("{}%", b.percentage)).into_any_element() + } } /// Pick the battery glyph from charge state first (charging / full / error), @@ -1351,22 +1366,32 @@ fn battery_summary(battery: &BatteryInfo, pal: Palette) -> impl IntoElement { .text_xs() .text_color(pal.text_muted) .child(status) - .child(format!("{}%", battery.percentage)), + .child(if battery_charging_no_reading(battery) { + String::new() + } else { + format!("{}%", battery.percentage) + }), ) - .child( - div() + .child({ + let track = div() .h(px(6.)) .w_full() .rounded_full() - .bg(pal.surface_hover) - .child( + .bg(pal.surface_hover); + // Charging with no reliable %: leave the track empty rather than + // drawing the 1%-wide red critical sliver that percentage==0 yields. + if battery_charging_no_reading(battery) { + track + } else { + track.child( div() .h_full() .w(relative_percent(battery.percentage)) .rounded_full() .bg(rgb(battery_color(battery.percentage))), - ), - ) + ) + } + }) } fn sidebar_action( @@ -1726,7 +1751,8 @@ fn accessibility_status(pal: Palette, granted: bool) -> AnyElement { #[cfg(test)] mod tests { use super::{ - Capabilities, DetailTab, DeviceKind, DeviceRecord, DeviceRoute, connection_icon_path, + BatteryInfo, BatteryLevel, BatteryStatus, Capabilities, DetailTab, DeviceKind, + DeviceRecord, DeviceRoute, battery_charging_no_reading, connection_icon_path, }; #[test] @@ -1756,6 +1782,31 @@ mod tests { assert_eq!(connection_icon_path(None), "action-icons/bluetooth.svg"); } + /// "Charging" replaces the bogus percentage only when charging *and* the + /// reading is still 0% (cold start, no cached pre-charge value). A non-zero + /// charge or a real 0% while discharging keeps the number. + #[test] + fn charging_without_reading_suppresses_percentage() { + let b = |percentage, status| BatteryInfo { + percentage, + level: BatteryLevel::Good, + status, + }; + assert!(battery_charging_no_reading(&b(0, BatteryStatus::Charging))); + assert!(battery_charging_no_reading(&b( + 0, + BatteryStatus::ChargingSlow + ))); + assert!(!battery_charging_no_reading(&b( + 40, + BatteryStatus::Charging + ))); + assert!(!battery_charging_no_reading(&b( + 0, + BatteryStatus::Discharging + ))); + } + fn record(kind: DeviceKind, capabilities: Option) -> DeviceRecord { DeviceRecord { config_key: "test".to_string(), diff --git a/crates/openlogi-gui/src/app_menu.rs b/crates/openlogi-gui/src/app_menu.rs index af25f84f..a532a1e3 100644 --- a/crates/openlogi-gui/src/app_menu.rs +++ b/crates/openlogi-gui/src/app_menu.rs @@ -218,6 +218,9 @@ fn device_menu_items(cx: &App) -> Vec { Some(state) if !state.device_list.is_empty() => { for record in &state.device_list { let title = match &record.battery { + Some(battery) if crate::app::battery_charging_no_reading(battery) => { + format!("{} · {}", record.display_name, tr!("Charging")) + } Some(battery) => format!("{} · {}%", record.display_name, battery.percentage), None => record.display_name.clone(), }; diff --git a/crates/openlogi-hid/src/inventory.rs b/crates/openlogi-hid/src/inventory.rs index a745cb66..31b1ccf0 100644 --- a/crates/openlogi-hid/src/inventory.rs +++ b/crates/openlogi-hid/src/inventory.rs @@ -12,7 +12,8 @@ use hidpp::{ device::Device, feature::hires_wheel::HiResWheelFeature, feature::{ - CreatableFeature, device_information::DeviceInformationFeature, + CreatableFeature, battery_status::BatteryStatusFeature, + device_information::DeviceInformationFeature, device_type_and_name::DeviceTypeAndNameFeature, unified_battery::UnifiedBatteryFeature, }, receiver::{ @@ -27,16 +28,17 @@ use hidpp::{ }, }; use openlogi_core::device::{ - BatteryInfo, Capabilities, DeviceInventory, DeviceKind, DeviceModelInfo, DeviceTransports, - PairedDevice, ReceiverInfo, + BatteryInfo, BatteryStatus, Capabilities, DeviceInventory, DeviceKind, DeviceModelInfo, + DeviceTransports, PairedDevice, ReceiverInfo, }; use thiserror::Error; use tokio::time::timeout; use tracing::{debug, warn}; use crate::mappings::{ - map_battery_level, map_battery_status, map_device_type, map_kind, map_unifying_kind, - normalize_serial_number, resolve_device_kind, + legacy_battery_level_from_percentage, map_battery_level, map_battery_status, map_device_type, + map_kind, map_legacy_battery_status, map_unifying_kind, normalize_serial_number, + resolve_device_kind, }; use crate::node_ledger::NodeLedger; use crate::route::DIRECT_DEVICE_INDEX; @@ -120,11 +122,11 @@ const CACHE_MISS_GRACE: u8 = 3; #[derive(Clone)] struct Cached { probe: ProbedFeatures, - /// Runtime index of the `UnifiedBattery` feature in this device's feature - /// table, captured by the full probe. Lets cache hits re-read the volatile - /// battery in one round-trip — no `Device::new` ping, no table walk. - /// `None` when the device exposes no `0x1004`. - battery_index: Option, + /// Which battery feature this device exposes and its runtime index, captured + /// by the full probe. Lets cache hits re-read the volatile battery in one + /// round-trip — no `Device::new` ping, no table walk. `None` when the device + /// exposes neither `0x1004` nor the legacy `0x1000`. + battery: Option, probed_tick: u64, } @@ -145,6 +147,43 @@ fn seen(id: Option) -> CacheOutcome { id.map_or(CacheOutcome::Unkeyed, CacheOutcome::Seen) } +/// The legacy `0x1000` battery feature (MX2S-era mice) reports `discharge_level +/// = 0` while charging — the firmware can't gauge charge under load, so the GUI +/// would show a misleading "Charging · 0%". Carry the last-known percentage +/// forward for the charge so the reading stays trackable. +/// +/// ponytail: a *frozen* pre-charge value, not a live charging %, because no +/// device exposes that on `0x1000`. Only kicks in for the charging-and-zero +/// sentinel; a genuine 0% while discharging (status != Charging) is untouched. +/// Cold edge: app started while already charging has no prior, so it shows 0% +/// until the first discharge read — upgrade to a stored sentinel if that bites. +fn hold_percentage_while_charging( + fresh: BatteryInfo, + prev: Option<&BatteryInfo>, + probe: BatteryProbe, +) -> BatteryInfo { + // Scoped to the legacy 0x1000 quirk: a 0x1004 device that legitimately + // reports 0% while charging must surface that, not a stale prior reading. + if !matches!(probe, BatteryProbe::Legacy(_)) { + return fresh; + } + let charging = matches!( + fresh.status, + BatteryStatus::Charging | BatteryStatus::ChargingSlow + ); + if charging + && fresh.percentage == 0 + && let Some(p) = prev.filter(|p| p.percentage > 0) + { + return BatteryInfo { + percentage: p.percentage, + level: p.level, + status: fresh.status, + }; + } + fresh +} + /// Whether `cached` is stale enough that the device should be re-probed. fn is_stale(cached: &Cached, tick: u64) -> bool { tick.wrapping_sub(cached.probed_tick) >= REFRESH_TICKS @@ -164,7 +203,14 @@ async fn probe_or_reuse( tick: u64, ) -> (ProbedFeatures, CacheOutcome) { if online && cached.is_none_or(|c| is_stale(c, tick)) { - let (fresh, battery_index) = probe_features(channel, index).await; + let (mut fresh, battery) = probe_features(channel, index).await; + if let (Some(reading), Some(probe)) = (fresh.battery.take(), battery) { + fresh.battery = Some(hold_percentage_while_charging( + reading, + cached.and_then(|c| c.probe.battery.as_ref()), + probe, + )); + } // `capabilities` is `Some` exactly when the feature-table walk succeeded; // only then is the probe worth caching. if fresh.capabilities.is_some() { @@ -172,7 +218,7 @@ async fn probe_or_reuse( Some(key) => { let value = Cached { probe: fresh.clone(), - battery_index, + battery, probed_tick: tick, }; (fresh, CacheOutcome::Fresh(key, value)) @@ -195,10 +241,12 @@ async fn probe_or_reuse( // index and fold the reading back into the cache. A failed read // (asleep, mid-host-switch) keeps the last-known value. if online - && let Some(feature_index) = c.battery_index + && let Some(probe) = c.battery && let Some(key) = id.clone() - && let Some(battery) = read_battery(channel, index, feature_index).await + && let Some(battery) = read_battery(channel, index, probe).await { + let battery = + hold_percentage_while_charging(battery, c.probe.battery.as_ref(), probe); let mut entry = c.clone(); entry.probe.battery = Some(battery); return (entry.probe.clone(), CacheOutcome::Update(key, entry)); @@ -933,37 +981,75 @@ struct ProbedFeatures { capabilities: Option, } -/// Read just the battery by addressing the `UnifiedBattery` feature at its -/// known runtime `feature_index` — one round-trip, with no `Device::new` ping -/// and no feature-table walk. This is both the full probe's battery read (the -/// walk just produced the index) and the cheap per-tick refresh for cache hits. -/// `None` when the device doesn't answer (asleep, switched hosts). +/// Which battery feature a device exposes plus its runtime feature index. Newer +/// devices answer the unified `0x1004`; MX2S-era ones only the legacy `0x1000` +/// — the same enhanced-then-legacy split SmartShift has with `0x2111`/`0x2110`. +#[derive(Clone, Copy)] +enum BatteryProbe { + Unified(u8), + Legacy(u8), +} + +/// Read just the battery by addressing its feature at the known runtime index — +/// one round-trip, with no `Device::new` ping and no feature-table walk. This is +/// both the full probe's battery read (the walk just produced the index) and the +/// cheap per-tick refresh for cache hits. `None` when the device doesn't answer +/// (asleep, switched hosts). async fn read_battery( channel: &Arc, slot: u8, - feature_index: u8, + probe: BatteryProbe, ) -> Option { - let feature = UnifiedBatteryFeature::new(Arc::clone(channel), slot, feature_index); - feature - .get_battery_info() - .await - .ok() - .map(|info| BatteryInfo { - percentage: info.charging_percentage, - level: map_battery_level(info.level), - status: map_battery_status(info.status), - }) + match probe { + BatteryProbe::Unified(feature_index) => { + let feature = UnifiedBatteryFeature::new(Arc::clone(channel), slot, feature_index); + feature + .get_battery_info() + .await + .ok() + .map(|info| BatteryInfo { + percentage: info.charging_percentage, + level: map_battery_level(info.level), + status: map_battery_status(info.status), + }) + } + BatteryProbe::Legacy(feature_index) => { + let feature = BatteryStatusFeature::new(Arc::clone(channel), slot, feature_index); + feature + .get_battery_level_status() + .await + .ok() + .map(|info| BatteryInfo { + percentage: info.discharge_level, + level: legacy_battery_level_from_percentage(info.discharge_level), + status: map_legacy_battery_status(info.status), + }) + } + } } -/// Runtime index of the `UnifiedBattery` feature in an enumerated feature-ID -/// table, for [`read_battery`]. The table is 1-based (index 0 is the implicit -/// root feature, which enumeration omits). -fn battery_feature_index(ids: impl IntoIterator) -> Option { - ids.into_iter() - .position(|id| id == UnifiedBatteryFeature::ID) - // A feature table holds at most `u8::MAX` entries (its count is a u8), - // so the 1-based index always fits. - .and_then(|pos| u8::try_from(pos + 1).ok()) +/// Locate a device's battery feature in an enumerated feature-ID table, +/// preferring the unified `0x1004` and falling back to the legacy `0x1000`. The +/// table is 1-based (index 0 is the implicit root feature, which enumeration +/// omits). +fn battery_feature_index(ids: impl IntoIterator) -> Option { + // A feature table holds at most `u8::MAX` entries (its count is a u8), so a + // 1-based index always fits. + let mut legacy = None; + for (pos, id) in ids.into_iter().enumerate() { + // Stop gracefully past u8::MAX instead of `?`-returning None, which would + // discard a `legacy` already found. (The table caps at 255, so unreachable.) + let Ok(index) = u8::try_from(pos + 1) else { + break; + }; + if id == UnifiedBatteryFeature::ID { + return Some(BatteryProbe::Unified(index)); + } + if id == BatteryStatusFeature::ID && legacy.is_none() { + legacy = Some(BatteryProbe::Legacy(index)); + } + } + legacy } /// Open a HID++ session for `slot` and read everything we care about (battery, @@ -973,11 +1059,14 @@ fn battery_feature_index(ids: impl IntoIterator) -> Option { /// `enumerate_features` — the feature table is the Vec that enumeration already /// returns, so capabilities cost no extra round-trip. /// -/// Also returns the `UnifiedBattery` runtime index found by the walk, so later -/// ticks can refresh the battery without repeating it. +/// Also returns the battery feature found by the walk, so later ticks can +/// refresh the battery without repeating it. /// /// Only online, responsive devices reach here. -async fn probe_features(channel: &Arc, slot: u8) -> (ProbedFeatures, Option) { +async fn probe_features( + channel: &Arc, + slot: u8, +) -> (ProbedFeatures, Option) { let mut device = match Device::new(Arc::clone(channel), slot).await { Ok(d) => d, Err(e) => { @@ -987,11 +1076,11 @@ async fn probe_features(channel: &Arc, slot: u8) -> (ProbedFeature }; // The enumeration response IS the device's feature-ID table — capture it // for capability derivation instead of discarding it. - let mut battery_index = None; + let mut battery_probe = None; let mut capabilities = match device.enumerate_features().await { Ok(Some(features)) => { let ids: Vec = features.iter().map(|f| f.id).collect(); - battery_index = battery_feature_index(ids.iter().copied()); + battery_probe = battery_feature_index(ids.iter().copied()); Some(Capabilities::from_feature_ids(&ids)) } Ok(None) => None, @@ -1009,8 +1098,8 @@ async fn probe_features(channel: &Arc, slot: u8) -> (ProbedFeature .is_ok_and(|wheel| wheel.has_invert); } - let battery = match battery_index { - Some(feature_index) => read_battery(channel, slot, feature_index).await, + let battery = match battery_probe { + Some(probe) => read_battery(channel, slot, probe).await, None => None, }; @@ -1072,7 +1161,7 @@ async fn probe_features(channel: &Arc, slot: u8) -> (ProbedFeature kind, capabilities, }, - battery_index, + battery_probe, ) } @@ -1081,15 +1170,71 @@ mod tests { use std::collections::HashSet; use super::{ - CACHE_MISS_GRACE, CacheKey, Cached, Enumerator, ProbedFeatures, REFRESH_TICKS, - UnifiedBatteryFeature, battery_feature_index, is_stale, + BatteryInfo, BatteryProbe, BatteryStatus, BatteryStatusFeature, CACHE_MISS_GRACE, CacheKey, + Cached, Enumerator, ProbedFeatures, REFRESH_TICKS, UnifiedBatteryFeature, + battery_feature_index, hold_percentage_while_charging, is_stale, }; use hidpp::feature::CreatableFeature as _; + use openlogi_core::device::BatteryLevel; + + fn battery(percentage: u8, status: BatteryStatus) -> BatteryInfo { + BatteryInfo { + percentage, + level: BatteryLevel::Good, + status, + } + } + + #[test] + fn charging_zero_holds_last_known_percentage() { + let legacy = BatteryProbe::Legacy(0); + // 0x1000 reports 0% mid-charge: keep the pre-charge reading, new status. + let held = hold_percentage_while_charging( + battery(0, BatteryStatus::Charging), + Some(&battery(85, BatteryStatus::Discharging)), + legacy, + ); + assert_eq!(held.percentage, 85); + assert_eq!(held.status, BatteryStatus::Charging); + + // A real 0% while discharging is left alone (not the charging sentinel). + let discharging = hold_percentage_while_charging( + battery(0, BatteryStatus::Discharging), + Some(&battery(85, BatteryStatus::Discharging)), + legacy, + ); + assert_eq!(discharging.percentage, 0); + + // A non-zero charging reading is trusted as-is, not overwritten. + let live = hold_percentage_while_charging( + battery(40, BatteryStatus::Charging), + Some(&battery(85, BatteryStatus::Discharging)), + legacy, + ); + assert_eq!(live.percentage, 40); + + // No usable prior (cold start mid-charge): nothing to hold, stays 0. + let cold = + hold_percentage_while_charging(battery(0, BatteryStatus::Charging), None, legacy); + assert_eq!(cold.percentage, 0); + } + + #[test] + fn unified_charging_zero_is_not_held() { + // 0x1004 can report a genuine 0% while charging — the legacy hold must + // not mask it with a stale prior. Same inputs that hold for Legacy. + let live = hold_percentage_while_charging( + battery(0, BatteryStatus::Charging), + Some(&battery(85, BatteryStatus::Discharging)), + BatteryProbe::Unified(0), + ); + assert_eq!(live.percentage, 0); + } fn cache_entry(probed_tick: u64) -> Cached { Cached { probe: ProbedFeatures::default(), - battery_index: None, + battery: None, probed_tick, } } @@ -1140,7 +1285,7 @@ mod tests { fn cached_probe_is_reused_until_refresh_ticks() { let cached = Cached { probe: ProbedFeatures::default(), - battery_index: None, + battery: None, probed_tick: 10, }; assert!(!is_stale(&cached, 10), "same tick is fresh"); @@ -1159,17 +1304,43 @@ mod tests { // `enumerate_features` omits the root feature (index 0), so the first // enumerated entry sits at runtime index 1. let table = [0x0001, UnifiedBatteryFeature::ID, 0x2201]; - assert_eq!(battery_feature_index(table), Some(2)); - assert_eq!( - battery_feature_index([UnifiedBatteryFeature::ID]), - Some(1), + assert!(matches!( + battery_feature_index(table), + Some(BatteryProbe::Unified(2)) + )); + assert!( + matches!( + battery_feature_index([UnifiedBatteryFeature::ID]), + Some(BatteryProbe::Unified(1)) + ), "first entry maps to index 1, not 0" ); } + #[test] + fn unified_battery_is_preferred_over_legacy() { + // A device exposing both must use the unified feature, mirroring + // SmartShift's enhanced-then-legacy precedence. + let table = [0x0001, BatteryStatusFeature::ID, UnifiedBatteryFeature::ID]; + assert!(matches!( + battery_feature_index(table), + Some(BatteryProbe::Unified(3)) + )); + } + + #[test] + fn legacy_battery_is_the_fallback() { + // MX2S-era device: only `0x1000`. + let table = [0x0001, BatteryStatusFeature::ID, 0x2201]; + assert!(matches!( + battery_feature_index(table), + Some(BatteryProbe::Legacy(2)) + )); + } + #[test] fn no_battery_feature_means_no_index() { - assert_eq!(battery_feature_index([0x0001, 0x2201, 0x1b04]), None); - assert_eq!(battery_feature_index([]), None); + assert!(battery_feature_index([0x0001, 0x2201, 0x1b04]).is_none()); + assert!(battery_feature_index([]).is_none()); } } diff --git a/crates/openlogi-hid/src/lib.rs b/crates/openlogi-hid/src/lib.rs index 57186224..00749e85 100644 --- a/crates/openlogi-hid/src/lib.rs +++ b/crates/openlogi-hid/src/lib.rs @@ -36,6 +36,7 @@ pub use smartshift::{AUTO_DISENGAGE_PERMANENT, SmartShiftMode, SmartShiftStatus} pub use write::{ DpiCapabilities, DpiInfo, FeatureEntry, HidppFeatureErrorKind, HidppOperation, LightingMethod, SharedChannel, WriteError, dump_features, get_dpi, get_dpi_info, get_smartshift_status, - 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, + read_battery_raw, 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/mappings.rs b/crates/openlogi-hid/src/mappings.rs index 18ab81cb..444c8f41 100644 --- a/crates/openlogi-hid/src/mappings.rs +++ b/crates/openlogi-hid/src/mappings.rs @@ -3,6 +3,7 @@ //! battery level/status, and serial-number normalisation. No I/O — split from //! `inventory` purely to keep that file within size bounds. +use hidpp::feature::battery_status::LegacyBatteryStatus as HidppLegacyBatteryStatus; use hidpp::feature::device_type_and_name::DeviceType as HidppDeviceType; use hidpp::feature::unified_battery::{ BatteryLevel as HidppBatteryLevel, BatteryStatus as HidppBatteryStatus, @@ -112,9 +113,45 @@ pub(crate) fn map_battery_status(status: HidppBatteryStatus) -> BatteryStatus { } } +/// Map a legacy `0x1000` charging status to our [`BatteryStatus`]. +pub(crate) fn map_legacy_battery_status(status: HidppLegacyBatteryStatus) -> BatteryStatus { + match status { + HidppLegacyBatteryStatus::Discharging => BatteryStatus::Discharging, + // The legacy feature splits "charging" into recharging / almost-full; + // both are just "charging" to us. + HidppLegacyBatteryStatus::Recharging | HidppLegacyBatteryStatus::AlmostFull => { + BatteryStatus::Charging + } + HidppLegacyBatteryStatus::SlowRecharge => BatteryStatus::ChargingSlow, + HidppLegacyBatteryStatus::Full => BatteryStatus::Full, + HidppLegacyBatteryStatus::InvalidBattery | HidppLegacyBatteryStatus::ThermalError => { + BatteryStatus::Error + } + _ => BatteryStatus::Unknown, + } +} + +/// Derive a coarse [`BatteryLevel`] from a discharge percentage. The legacy +/// `0x1000` feature reports a percentage but, unlike `0x1004`, no level bitmask, +/// so the bucket is ours to pick. +/// +/// ponytail: fixed display buckets, not device thresholds. Lift to the device's +/// own thresholds only if `0x1000`'s capability query (function 1) is wired up. +pub(crate) fn legacy_battery_level_from_percentage(percentage: u8) -> BatteryLevel { + match percentage { + 90..=u8::MAX => BatteryLevel::Full, + 50..=89 => BatteryLevel::Good, + 20..=49 => BatteryLevel::Low, + _ => BatteryLevel::Critical, + } +} + #[cfg(test)] mod tests { - use super::{DeviceKind, UnifyingDeviceKind, map_unifying_kind, resolve_device_kind}; + use super::{ + BatteryLevel, DeviceKind, UnifyingDeviceKind, legacy_battery_level_from_percentage, + map_unifying_kind, resolve_device_kind, + }; #[test] fn probe_overrides_a_misreporting_register() { @@ -155,6 +192,38 @@ mod tests { ); } + #[test] + fn legacy_percentage_buckets_into_levels() { + assert_eq!( + legacy_battery_level_from_percentage(100), + BatteryLevel::Full + ); + assert_eq!(legacy_battery_level_from_percentage(90), BatteryLevel::Full); + assert_eq!(legacy_battery_level_from_percentage(89), BatteryLevel::Good); + assert_eq!(legacy_battery_level_from_percentage(50), BatteryLevel::Good); + assert_eq!(legacy_battery_level_from_percentage(49), BatteryLevel::Low); + assert_eq!(legacy_battery_level_from_percentage(20), BatteryLevel::Low); + assert_eq!( + legacy_battery_level_from_percentage(19), + BatteryLevel::Critical + ); + assert_eq!( + legacy_battery_level_from_percentage(0), + BatteryLevel::Critical + ); + } + + #[test] + fn legacy_status_value_7_maps_to_unknown_not_vanish() { + use super::{BatteryStatus, HidppLegacyBatteryStatus, map_legacy_battery_status}; + // Value 7 ("other charging error") must parse and surface as Unknown so + // the battery indicator stays visible instead of disappearing. + let mapped = HidppLegacyBatteryStatus::try_from(7u8) + .ok() + .map(map_legacy_battery_status); + assert_eq!(mapped, Some(BatteryStatus::Unknown)); + } + #[test] fn unifying_kind_maps_all_variants() { let cases = [ diff --git a/crates/openlogi-hid/src/write.rs b/crates/openlogi-hid/src/write.rs index 59b23092..e222da6f 100644 --- a/crates/openlogi-hid/src/write.rs +++ b/crates/openlogi-hid/src/write.rs @@ -17,7 +17,9 @@ use hidpp::{ device::Device, feature::CreatableFeature, feature::adjustable_dpi::AdjustableDpiFeature, + feature::battery_status::BatteryStatusFeature, feature::smartshift::{SmartShiftFeature, WheelMode}, + feature::unified_battery::UnifiedBatteryFeature, protocol::v20::{ErrorType, Hidpp20Error}, }; use serde::{Deserialize, Serialize}; @@ -542,6 +544,51 @@ pub async fn get_smartshift_status(route: &DeviceRoute) -> Result Result { + let index = route.device_index(); + with_route(route, move |channel| async move { + let mut device = Device::new(Arc::clone(&channel), index) + .await + .map_err(|_| WriteError::DeviceUnreachable { index })?; + device + .enumerate_features() + .await + .map_err(|e| WriteError::Hidpp(format!("{e:?}")))?; + + if let Some(feature) = device.get_feature::() { + let info = feature + .get_battery_info() + .await + .map_err(|e| WriteError::Hidpp(format!("{e:?}")))?; + return Ok(format!( + "0x1004 UnifiedBattery: percentage={} level={:?} status={:?}", + info.charging_percentage, info.level, info.status + )); + } + if let Some(feature) = device.get_feature::() { + let info = feature + .get_battery_level_status() + .await + .map_err(|e| WriteError::Hidpp(format!("{e:?}")))?; + return Ok(format!( + "0x1000 BatteryStatus: discharge_level={} next_level={} status={:?}", + info.discharge_level, info.next_level, info.status + )); + } + // Reached only when neither 0x1004 nor 0x1000 is present; report the + // preferred feature rather than implying 0x1000 was specifically absent. + Err(WriteError::FeatureUnsupported { + feature_hex: 0x1004, + }) + }) + .await +} + /// Set the SmartShift auto-disengage sensitivity on `route`, preserving the /// current mode. Returns the read-back status after the write so the caller can /// display and verify it. diff --git a/crates/openlogi-hidpp/src/feature/battery_status/mod.rs b/crates/openlogi-hidpp/src/feature/battery_status/mod.rs new file mode 100644 index 00000000..53dea3d8 --- /dev/null +++ b/crates/openlogi-hidpp/src/feature/battery_status/mod.rs @@ -0,0 +1,92 @@ +//! Implements the legacy `BatteryStatus` feature (ID `0x1000`) that reports a +//! device's battery charge as a discharge level plus a charging status. +//! +//! This is the predecessor of `UnifiedBattery` (`0x1004`): older mice such as +//! the MX Master 2S expose `0x1000` and never `0x1004`, so the inventory probe +//! falls back to this feature when the unified one is absent — the same +//! enhanced-then-legacy pattern `SmartShift` uses for `0x2111` / `0x2110`. +//! +//! Only `getBatteryLevelStatus` (function `0`) is implemented; the optional +//! `getBatteryCapability` (function `1`) and the broadcast event aren't needed +//! to display a charge reading. + +use std::{hash::Hash, sync::Arc}; + +use num_enum::{IntoPrimitive, TryFromPrimitive}; + +use crate::{ + channel::HidppChannel, + feature::{CreatableFeature, Feature, FeatureEndpoint}, + protocol::v20::Hidpp20Error, +}; + +/// Implements the legacy `BatteryStatus` / `0x1000` feature. +pub struct BatteryStatusFeature { + /// The endpoint this feature talks to. + endpoint: FeatureEndpoint, +} + +impl CreatableFeature for BatteryStatusFeature { + const ID: u16 = 0x1000; + const STARTING_VERSION: u8 = 0; + + fn new(chan: Arc, device_index: u8, feature_index: u8) -> Self { + Self { + endpoint: FeatureEndpoint::new(chan, device_index, feature_index), + } + } +} + +impl Feature for BatteryStatusFeature {} + +impl BatteryStatusFeature { + /// Reads the current battery level and charging status (function `0`, + /// `getBatteryLevelStatus`). + pub async fn get_battery_level_status(&self) -> Result { + let payload = self.endpoint.call(0, [0; 3]).await?.extend_payload(); + + Ok(LegacyBatteryInfo { + discharge_level: payload[0], + next_level: payload[1], + status: LegacyBatteryStatus::try_from(payload[2]) + .map_err(|_| Hidpp20Error::UnsupportedResponse)?, + }) + } +} + +/// A reading from the legacy `0x1000` `getBatteryLevelStatus` function. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +#[cfg_attr(feature = "serde", derive(serde::Serialize))] +#[non_exhaustive] +pub struct LegacyBatteryInfo { + /// Current battery charge as a percentage (`0`–`100`). Logitech firmware + /// reports this in coarse steps rather than a continuous value. + pub discharge_level: u8, + + /// The next lower discharge step the firmware will report — a hint at the + /// reporting granularity. Unused for display. + pub next_level: u8, + + /// The current charging status. + pub status: LegacyBatteryStatus, +} + +/// Charging status reported by the legacy `0x1000` feature. Values follow the +/// HID++ `batteryStatus` enumeration (see Solaar / `hid-logitech-hidpp`). +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, IntoPrimitive, TryFromPrimitive)] +#[cfg_attr(feature = "serde", derive(serde::Serialize))] +#[non_exhaustive] +#[repr(u8)] +pub enum LegacyBatteryStatus { + Discharging = 0, + Recharging = 1, + AlmostFull = 2, + Full = 3, + SlowRecharge = 4, + InvalidBattery = 5, + ThermalError = 6, + /// "Other charging error" (Solaar lists value 7). Kept explicit so a device + /// reporting it surfaces as Unknown instead of failing the parse and making + /// the battery indicator vanish from the UI. + Other = 7, +} diff --git a/crates/openlogi-hidpp/src/feature/mod.rs b/crates/openlogi-hidpp/src/feature/mod.rs index 213281ca..72f80133 100755 --- a/crates/openlogi-hidpp/src/feature/mod.rs +++ b/crates/openlogi-hidpp/src/feature/mod.rs @@ -9,6 +9,7 @@ use crate::{ }; pub mod adjustable_dpi; +pub mod battery_status; pub mod device_friendly_name; pub mod device_information; pub mod device_type_and_name; diff --git a/crates/openlogi-hidpp/src/feature/registry.rs b/crates/openlogi-hidpp/src/feature/registry.rs index 113a7cb0..0fd88d1c 100755 --- a/crates/openlogi-hidpp/src/feature/registry.rs +++ b/crates/openlogi-hidpp/src/feature/registry.rs @@ -12,7 +12,7 @@ use crate::{ channel::HidppChannel, feature::{ CreatableFeature, adjustable_dpi::AdjustableDpiFeature, - device_friendly_name::DeviceFriendlyNameFeature, + battery_status::BatteryStatusFeature, device_friendly_name::DeviceFriendlyNameFeature, device_information::DeviceInformationFeature, device_type_and_name::DeviceTypeAndNameFeature, feature_set::FeatureSetFeature, hires_wheel::HiResWheelFeature, root::RootFeature, smartshift::SmartShiftFeature, @@ -120,7 +120,7 @@ static KNOWN_FEATURES: LazyLock> = LazyLock::new(|| { 0x00c3 "DfuControlBolt", 0x00d0 "Dfu", 0x00d1 "DfuResumable", - 0x1000 "BatteryStatus", + 0x1000 "BatteryStatus" => BatteryStatusFeature, 0x1001 "BatteryVoltage", 0x1004 "UnifiedBattery" => UnifiedBatteryFeature, 0x1010 "ChargingControl",