Re: [PATCH BlueZ v2] battery: improve the display of the charge level

[Roman Smirnov <r.smirnov@omp.ru>]

On Mon, 2025-10-27 at 16:37 +0100, Bastien Nocera wrote:
> On Mon, 2025-10-27 at 18:11 +0300, Roman Smirnov wrote:
> > The battery charge level may fluctuate due to uncalibrated
> > sensors. Commit smooths out such fluctuations.
> > 
> > The algorithm for determining uncalibrated sensors consists of
> > finding the number of changes in charge direction (i.e., "spikes").
> > If the number of spikes is zero, the device is charging or
> > discharging.
> > If there is one spike, it may mean that the device has started
> > charging
> > or has been disconnected from charging. If there are two or more
> > spikes, this is a clear indication of an uncalibrated sensor.
> > 
> > Check that the battery charge is fluctuating. If the battery charge
> > is fluctuating, use the average charge value, otherwise use the
> > current
> > value.
> > 
> > Fixes: https://github.com/bluez/bluez/issues/1612
> 
> When sending a v2, it's customary to add a changelog compared to the
> first version. What changed since v1?
> 
Sorry, I forgot to attach it.

V1 -> V2: Smoothing is only applied to uncalibrated sensors, the
last 8 values are saved instead of 4, and the average value is
used for smoothing instead of the minimum value.

I will attach it to the next version of the patch.
> > ---
> >  src/battery.c | 62
> > +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 62 insertions(+)
> > 
> > diff --git a/src/battery.c b/src/battery.c
> > index 59e4fc570..f97d9b8f3 100644
> > --- a/src/battery.c
> > +++ b/src/battery.c
> > @@ -33,10 +33,16 @@
> >  #define BATTERY_PROVIDER_MANAGER_INTERFACE
> > "org.bluez.BatteryProviderManager1"
> >  
> >  #define BATTERY_MAX_PERCENTAGE 100
> > +#define LAST_CHARGES_SIZE 8
> > +#define MAX_CHARGE_STEP 5
> >  
> >  struct btd_battery {
> >  	char *path; /* D-Bus object path */
> >  	uint8_t percentage; /* valid between 0 to 100 inclusively */
> > +	uint8_t *last_charges; /* last charges received */
> > +	uint8_t lru_charge_id; /* oldest battery charge */
> > +	float avg_charge; /* average battery charge */
> > +	bool is_fluctuating; /* true, if the battery sensor
> > fluctuates */
> >  	char *source; /* Descriptive source of the battery info */
> >  	char *provider_path; /* The provider root path, if any */
> >  };
> > @@ -92,6 +98,11 @@ static struct btd_battery *battery_new(const char
> > *path, const char *source,
> >  	battery = new0(struct btd_battery, 1);
> >  	battery->path = g_strdup(path);
> >  	battery->percentage = UINT8_MAX;
> > +	battery->last_charges = new0(uint8_t, LAST_CHARGES_SIZE);
> > +	battery->lru_charge_id = 0;
> > +	battery->avg_charge = 0;
> > +	battery->is_fluctuating = false;
> > +
> >  	if (source)
> >  		battery->source = g_strdup(source);
> >  	if (provider_path)
> > @@ -105,6 +116,9 @@ static void battery_free(struct btd_battery
> > *battery)
> >  	if (battery->path)
> >  		g_free(battery->path);
> >  
> > +	if (battery->last_charges)
> > +		g_free(battery->last_charges);
> > +
> >  	if (battery->source)
> >  		g_free(battery->source);
> >  
> > @@ -217,6 +231,39 @@ bool btd_battery_unregister(struct btd_battery
> > *battery)
> >  	return true;
> >  }
> >  
> > +static void check_fluctuations(struct btd_battery *battery)
> > +{
> > +	uint8_t spikes = 0;
> > +	int8_t step = 0;
> > +	int8_t direction = 0;
> > +	int8_t prev_direction;
> > +
> > +	for (uint8_t id = 0; id < LAST_CHARGES_SIZE - 1; id++) {
> > +		prev_direction = direction;
> > +		step = battery->last_charges[id] - battery-
> > > last_charges[id + 1];
> > +
> > +		/*
> > +		 * The battery charge fluctuates too much,
> > +		 * which may indicate a battery problem, so
> > +		 * the actual value should be displayed.
> > +		 */
> > +		if (step > MAX_CHARGE_STEP) {
> > +			battery->is_fluctuating = false;
> > +			return;
> > +		}
> > +
> > +		if (step > 0)
> > +			direction = 1;
> > +		else if (step < 0)
> > +			direction = -1;
> > +
> > +		if (direction != prev_direction && !prev_direction)
> > +			spikes++;
> > +	}
> > +
> > +	battery->is_fluctuating = (spikes > 1) ? true : false;
> > +}
> > +
> >  bool btd_battery_update(struct btd_battery *battery, uint8_t
> > percentage)
> >  {
> >  	DBG("path = %s", battery->path);
> > @@ -231,6 +278,21 @@ bool btd_battery_update(struct btd_battery
> > *battery, uint8_t percentage)
> >  		return false;
> >  	}
> >  
> > +	if (!battery->avg_charge)
> > +		battery->avg_charge = percentage;
> > +
> > +	/* exponential smoothing */
> > +	battery->avg_charge = battery->avg_charge * 0.7 + percentage
> > * 0.3;
> > +	battery->last_charges[battery->lru_charge_id] = percentage;
> > +
> > +	if (battery->lru_charge_id == LAST_CHARGES_SIZE - 1)
> > +		check_fluctuations(battery);
> > +
> > +	battery->lru_charge_id = (battery->lru_charge_id + 1) %
> > LAST_CHARGES_SIZE;
> > +
> > +	if (battery->is_fluctuating)
> > +		percentage = battery->avg_charge;
> > +
> >  	if (battery->percentage == percentage)
> >  		return true;
> >