Skip to content

Commit ffa4f23

Browse files
committed
feat(battery): support legacy 0x1000 BatteryStatus and its charging quirk
Older devices such as a Bluetooth-direct MX Master 2S expose only the legacy BatteryStatus feature (0x1000), never the unified 0x1004, so they showed no battery at all. Implement 0x1000 mirroring the SmartShift 0x2110/0x2111 design and have the inventory probe prefer 0x1004, falling back to 0x1000 — the same enhanced-then-legacy order SmartShift uses. 0x1000 reports a percentage but no level bitmask, so the coarse BatteryLevel is derived from fixed display buckets. The 0x1000 firmware can't gauge charge under load: it reports discharge_level=0 status=Recharging while charging, which surfaced as a misleading "Charging · 0%". Two layers handle it: - hold_percentage_while_charging() carries the last-known percentage forward through a charge session (frozen pre-charge value, cache only) so the reading stays trackable once one discharge read exists. - On a cold start (charger plugged before the app opens) there's no prior to hold, so the GUI shows "Charging" instead of the bogus 0% across the card, summary, and tray menu. Adds `openlogi diag battery`, which prints the raw 0x1004/0x1000 report so a claim like "MX2S shows 0% while charging" can be confirmed against the wire. Verified on hardware: MX2S reports 50% (discharging) over BT-direct, and 0% status=Recharging while charging.
1 parent 45070bb commit ffa4f23

11 files changed

Lines changed: 507 additions & 68 deletions

File tree

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
//! `openlogi diag battery` — dump the device's raw battery report.
2+
//!
3+
//! Prints exactly what the firmware returns (unified `0x1004` fields, or legacy
4+
//! `0x1000` `discharge_level`/`next_level`/`status`). Run it once on battery and
5+
//! once with the charger plugged in to see how the device reports while charging
6+
//! — e.g. an MX2S returns `discharge_level=0` mid-charge, which is the device's
7+
//! own limitation, not a bug in the read path.
8+
9+
use anyhow::{Context, Result};
10+
use clap::Args;
11+
12+
use crate::cmd::diag::select_device;
13+
14+
#[derive(Debug, Args)]
15+
pub struct BatteryArgs {
16+
/// Run against the device whose name contains this string
17+
/// (case-insensitive) instead of auto-selecting.
18+
#[arg(long, value_name = "NAME")]
19+
pub device: Option<String>,
20+
}
21+
22+
pub async fn run(args: BatteryArgs) -> Result<()> {
23+
// 0x1004 UnifiedBattery / 0x1000 BatteryStatus — pick a device with either.
24+
let (route, name) = select_device(args.device.as_deref(), &[0x1000, 0x1004]).await?;
25+
println!("device: {name} ({route})");
26+
27+
let line = openlogi_hid::read_battery_raw(&route)
28+
.await
29+
.context("read battery")?;
30+
println!(" {line}");
31+
Ok(())
32+
}

crates/openlogi-cli/src/cmd/diag/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use anyhow::{Result, anyhow};
1010
use clap::Subcommand;
1111
use openlogi_hid::{DeviceRoute, dump_features};
1212

13+
pub mod battery;
1314
pub mod dpi;
1415
pub mod features;
1516
pub mod lighting;
@@ -19,6 +20,8 @@ pub mod smartshift;
1920
pub enum DiagCmd {
2021
/// Dump every HID++ feature the active device reports.
2122
Features(features::FeaturesArgs),
23+
/// Read the raw battery report (0x1004 or 0x1000 fields).
24+
Battery(battery::BatteryArgs),
2225
/// Read DPI → write a small delta → read back → restore → report.
2326
Dpi(dpi::DpiArgs),
2427
/// Read SmartShift mode → toggle → read back → toggle back → report.
@@ -31,6 +34,7 @@ impl DiagCmd {
3134
pub async fn run(self) -> Result<()> {
3235
match self {
3336
Self::Features(args) => features::run(args).await,
37+
Self::Battery(args) => battery::run(args).await,
3438
Self::Dpi(args) => dpi::run(args).await,
3539
Self::Smartshift(args) => smartshift::run(args).await,
3640
Self::Lighting(args) => lighting::run(args).await,

crates/openlogi-gui/src/app.rs

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -817,17 +817,32 @@ fn status_dot(online: bool) -> AnyElement {
817817
.into_any_element()
818818
}
819819

820+
/// True when the device is charging but still reports 0% — the MX2S `0x1000`
821+
/// firmware can't gauge charge under load, and on a cold start there's no
822+
/// pre-charge % cached to carry forward. Show "Charging" without the bogus 0%.
823+
/// ponytail: cold-start only; once any discharge read is cached, the
824+
/// carry-forward in `inventory.rs` holds it and `percentage` is non-zero.
825+
pub(crate) fn battery_charging_no_reading(b: &BatteryInfo) -> bool {
826+
matches!(
827+
b.status,
828+
BatteryStatus::Charging | BatteryStatus::ChargingSlow
829+
) && b.percentage == 0
830+
}
831+
820832
/// Battery readout for a gallery card: a charge/level glyph plus the
821833
/// percentage, in the muted metadata style.
822834
fn battery_view(b: &BatteryInfo, pal: Palette) -> AnyElement {
823-
h_flex()
835+
let row = h_flex()
824836
.gap_1()
825837
.items_center()
826838
.text_xs()
827839
.text_color(pal.text_muted)
828-
.child(Icon::new(battery_icon(b)).size_3())
829-
.child(format!("{}%", b.percentage))
830-
.into_any_element()
840+
.child(Icon::new(battery_icon(b)).size_3());
841+
if battery_charging_no_reading(b) {
842+
row.child(tr!("Charging")).into_any_element()
843+
} else {
844+
row.child(format!("{}%", b.percentage)).into_any_element()
845+
}
831846
}
832847

833848
/// Pick the battery glyph from charge state first (charging / full / error),
@@ -1338,7 +1353,11 @@ fn battery_summary(battery: &BatteryInfo, pal: Palette) -> impl IntoElement {
13381353
.text_xs()
13391354
.text_color(pal.text_muted)
13401355
.child(status)
1341-
.child(format!("{}%", battery.percentage)),
1356+
.child(if battery_charging_no_reading(battery) {
1357+
String::new()
1358+
} else {
1359+
format!("{}%", battery.percentage)
1360+
}),
13421361
)
13431362
.child(
13441363
div()
@@ -1697,7 +1716,35 @@ fn accessibility_status(pal: Palette, granted: bool) -> AnyElement {
16971716

16981717
#[cfg(test)]
16991718
mod tests {
1700-
use super::{Capabilities, DetailTab, DeviceKind, DeviceRecord};
1719+
use super::{
1720+
BatteryInfo, BatteryLevel, BatteryStatus, Capabilities, DetailTab, DeviceKind,
1721+
DeviceRecord, battery_charging_no_reading,
1722+
};
1723+
1724+
/// "Charging" replaces the bogus percentage only when charging *and* the
1725+
/// reading is still 0% (cold start, no cached pre-charge value). A non-zero
1726+
/// charge or a real 0% while discharging keeps the number.
1727+
#[test]
1728+
fn charging_without_reading_suppresses_percentage() {
1729+
let b = |percentage, status| BatteryInfo {
1730+
percentage,
1731+
level: BatteryLevel::Good,
1732+
status,
1733+
};
1734+
assert!(battery_charging_no_reading(&b(0, BatteryStatus::Charging)));
1735+
assert!(battery_charging_no_reading(&b(
1736+
0,
1737+
BatteryStatus::ChargingSlow
1738+
)));
1739+
assert!(!battery_charging_no_reading(&b(
1740+
40,
1741+
BatteryStatus::Charging
1742+
)));
1743+
assert!(!battery_charging_no_reading(&b(
1744+
0,
1745+
BatteryStatus::Discharging
1746+
)));
1747+
}
17011748

17021749
fn record(kind: DeviceKind, capabilities: Option<Capabilities>) -> DeviceRecord {
17031750
DeviceRecord {

crates/openlogi-gui/src/app_menu.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,9 @@ fn device_menu_items(cx: &App) -> Vec<MenuItem> {
218218
Some(state) if !state.device_list.is_empty() => {
219219
for record in &state.device_list {
220220
let title = match &record.battery {
221+
Some(battery) if crate::app::battery_charging_no_reading(battery) => {
222+
format!("{} · {}", record.display_name, tr!("Charging"))
223+
}
221224
Some(battery) => format!("{} · {}%", record.display_name, battery.percentage),
222225
None => record.display_name.clone(),
223226
};

0 commit comments

Comments
 (0)