From d99a1fc6ce41525a36c9884d52b7b5b73e01b6f0 Mon Sep 17 00:00:00 2001
From: Archie Pusaka <apusaka@google.com>
Date: Mon, 21 Aug 2023 16:42:22 +0800
Subject: [PATCH] Floss: Avoid non-empty battery result
Some peripherals don't have BAS profile but have other means to
convey the battery level. We have preferences to display the battery
level from BAS profile if that exists though.
However, sometimes (e.g. after device disconnection) the BAS profile
might reports empty battery result and it stays that way even after
reconnection. This causes battery conveyed via other means to be
meaningless because we use the empty result from BAS instead.
Update the behavior to prefer non-empty battery reports. Also, avoid
entering empty battery report by providing a remove battery API to
remove the entry instead of setting it empty.
Bug: 295577710
Tag: #Floss
Test: Disconnect/reconnect WH-1000XM3 multiple times. Observe the
battery level is always displayed.
Change-Id: I10dd8ae8624f001cf539f5e0ac9b29655a76440b
---
.../src/iface_battery_provider_manager.rs | 5 +++++
.../rust/linux/stack/src/battery_manager.rs | 14 +++++++++++++-
.../stack/src/battery_provider_manager.rs | 19 +++++++++++++++++++
.../rust/linux/stack/src/battery_service.rs | 10 +++-------
4 files changed, 40 insertions(+), 8 deletions(-)
diff --git a/system/gd/rust/linux/service/src/iface_battery_provider_manager.rs b/system/gd/rust/linux/service/src/iface_battery_provider_manager.rs
index f2ce28bd2bc..dac6457c8a3 100644
--- a/system/gd/rust/linux/service/src/iface_battery_provider_manager.rs
+++ b/system/gd/rust/linux/service/src/iface_battery_provider_manager.rs
@@ -41,4 +41,9 @@ impl IBatteryProviderManager for IBatteryProviderManagerDBus {
fn set_battery_info(&mut self, battery_provider_id: u32, battery_set: BatterySet) {
dbus_generated!()
}
+
+ #[dbus_method("RemoveBatteryInfo")]
+ fn remove_battery_info(&mut self, battery_provider_id: u32, address: String, uuid: String) {
+ dbus_generated!()
+ }
}
diff --git a/system/gd/rust/linux/stack/src/battery_manager.rs b/system/gd/rust/linux/stack/src/battery_manager.rs
index 5723b686bbd..393761c4486 100644
--- a/system/gd/rust/linux/stack/src/battery_manager.rs
+++ b/system/gd/rust/linux/stack/src/battery_manager.rs
@@ -3,6 +3,7 @@ use crate::callbacks::Callbacks;
use crate::uuid;
use crate::Message;
use crate::RPCProxy;
+use itertools::Itertools;
use std::sync::{Arc, Mutex};
use tokio::sync::mpsc::Sender;
@@ -137,11 +138,22 @@ impl Batteries {
}
}
+ pub fn remove_battery_set(&mut self, uuid: &String) {
+ self.0.retain(|battery_set| &battery_set.source_uuid != uuid);
+ }
+
+ pub fn is_empty(&self) -> bool {
+ self.0.is_empty()
+ }
+
/// Returns the best BatterySet from among reported battery data.
pub fn pick_best(&self) -> Option<BatterySet> {
self.0
.iter()
- .find(|battery_set| battery_set.source_uuid == uuid::BAS)
+ .filter(|battery_set| !battery_set.batteries.is_empty())
+ // Now we prefer BAS, but we might need to prioritize other sources first
+ // TODO (b/295577710): Make a preference list
+ .find_or_first(|battery_set| battery_set.source_uuid == uuid::BAS)
.or_else(|| self.0.first())
.cloned()
}
diff --git a/system/gd/rust/linux/stack/src/battery_provider_manager.rs b/system/gd/rust/linux/stack/src/battery_provider_manager.rs
index b8e0c2ea909..a0138e2787c 100644
--- a/system/gd/rust/linux/stack/src/battery_provider_manager.rs
+++ b/system/gd/rust/linux/stack/src/battery_provider_manager.rs
@@ -25,6 +25,9 @@ pub trait IBatteryProviderManager {
/// Updates the battery information for the battery associated with battery_id.
fn set_battery_info(&mut self, battery_provider_id: u32, battery_set: BatterySet);
+
+ /// Removes the battery information for the battery associated with battery_id.
+ fn remove_battery_info(&mut self, battery_provider_id: u32, address: String, uuid: String);
}
/// Represents the BatteryProviderManager, a central point for collecting battery information from
@@ -75,6 +78,16 @@ impl IBatteryProviderManager for BatteryProviderManager {
self.remove_battery_provider_callback(battery_provider_id);
}
+ fn remove_battery_info(&mut self, _battery_provider_id: u32, address: String, uuid: String) {
+ if let Some(batteries) = self.battery_info.get_mut(&address) {
+ batteries.remove_battery_set(&uuid);
+
+ if batteries.is_empty() {
+ self.battery_info.remove(&address);
+ }
+ }
+ }
+
fn set_battery_info(&mut self, _battery_provider_id: u32, battery_set: BatterySet) {
debug!(
"BatteryProviderManager received BatterySet for [{}] from \"{}\": {:?}",
@@ -82,11 +95,17 @@ impl IBatteryProviderManager for BatteryProviderManager {
battery_set.source_info.clone(),
battery_set.clone()
);
+
+ if battery_set.batteries.is_empty() {
+ return;
+ }
+
let batteries = self
.battery_info
.entry(battery_set.address.clone())
.or_insert_with(|| Batteries::new());
batteries.add_or_update_battery_set(battery_set);
+
if let Some(best_battery_set) = batteries.pick_best() {
let tx = self.tx.clone();
tokio::spawn(async move {
diff --git a/system/gd/rust/linux/stack/src/battery_service.rs b/system/gd/rust/linux/stack/src/battery_service.rs
index f822fa30206..a8f8ae44944 100644
--- a/system/gd/rust/linux/stack/src/battery_service.rs
+++ b/system/gd/rust/linux/stack/src/battery_service.rs
@@ -284,14 +284,10 @@ impl BatteryService {
None => return,
}
// Let BatteryProviderManager know that BAS no longer has a battery for this device.
- self.battery_provider_manager.lock().unwrap().set_battery_info(
+ self.battery_provider_manager.lock().unwrap().remove_battery_info(
self.battery_provider_id,
- BatterySet::new(
- remote_address.clone(),
- uuid::BAS.to_string(),
- "BAS".to_string(),
- vec![],
- ),
+ remote_address.clone(),
+ uuid::BAS.to_string(),
);
self.battery_sets.remove(&remote_address);
}
--
GitLab