* [PATCH BlueZ] profiles/battery: Always update initial battery value
@ 2021-03-30 17:11 Sonny Sasaka
2021-03-30 18:05 ` [BlueZ] " bluez.test.bot
0 siblings, 1 reply; 7+ messages in thread
From: Sonny Sasaka @ 2021-03-30 17:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Sonny Sasaka, Alain Michaud
Due to cache in gatt db, bluetoothd fails to update publish the battery
value after reconnection when the battery value does not change compared
to before reconnection. For initial battery value, we should update the
value to D-Bus regardless of the cache value.
Reviewed-by: Alain Michaud <alainm@chromium.org>
---
profiles/battery/battery.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
index 81f849d57..0f8d6ef18 100644
--- a/profiles/battery/battery.c
+++ b/profiles/battery/battery.c
@@ -88,12 +88,13 @@ static void batt_reset(struct batt *batt)
}
static void parse_battery_level(struct batt *batt,
- const uint8_t *value)
+ const uint8_t *value,
+ bool force_update)
{
uint8_t percentage;
percentage = value[0];
- if (batt->percentage != percentage) {
+ if (force_update || batt->percentage != percentage) {
batt->percentage = percentage;
DBG("Battery Level updated: %d%%", percentage);
if (!batt->battery) {
@@ -110,7 +111,7 @@ static void batt_io_value_cb(uint16_t value_handle, const uint8_t *value,
struct batt *batt = user_data;
if (value_handle == batt->batt_level_io_handle) {
- parse_battery_level(batt, value);
+ parse_battery_level(batt, value, false /* force_update */);
} else {
g_assert_not_reached();
}
@@ -134,7 +135,7 @@ static void batt_io_ccc_written_cb(uint16_t att_ecode, void *user_data)
return;
}
- parse_battery_level(batt, batt->initial_value);
+ parse_battery_level(batt, batt->initial_value, true /* force_update */);
g_free (batt->initial_value);
batt->initial_value = NULL;
--
2.29.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [BlueZ] profiles/battery: Always update initial battery value
2021-03-30 17:11 [PATCH BlueZ] profiles/battery: Always update initial battery value Sonny Sasaka
@ 2021-03-30 18:05 ` bluez.test.bot
0 siblings, 0 replies; 7+ messages in thread
From: bluez.test.bot @ 2021-03-30 18:05 UTC (permalink / raw)
To: linux-bluetooth, sonnysasaka
[-- Attachment #1: Type: text/plain, Size: 758 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=458225
---Test result---
##############################
Test: CheckPatch - PASS
##############################
Test: CheckGitLint - PASS
##############################
Test: CheckBuild: Setup ELL - PASS
##############################
Test: CheckBuild: Setup - PASS
##############################
Test: CheckBuild - PASS
##############################
Test: MakeCheck - PASS
##############################
Test: CheckBuild w/external ell - PASS
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH BlueZ] profiles/battery: Always update initial battery value
2021-03-30 18:25 ` Luiz Augusto von Dentz
@ 2021-03-30 18:39 ` Sonny Sasaka
0 siblings, 0 replies; 7+ messages in thread
From: Sonny Sasaka @ 2021-03-30 18:39 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hi Luiz,
That's a good idea. Please take a look at the latest patch titled
"Reset battery value cache on disconnect".
On Tue, Mar 30, 2021 at 11:25 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Sonny,
>
> On Tue, Mar 30, 2021 at 10:12 AM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> >
> > Due to cache in gatt db, bluetoothd fails to update publish the battery
> > value after reconnection when the battery value does not change compared
> > to before reconnection. For initial battery value, we should update the
> > value to D-Bus regardless of the cache value.
> >
> > ---
> > profiles/battery/battery.c | 9 +++++----
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
> > index 81f849d57..0f8d6ef18 100644
> > --- a/profiles/battery/battery.c
> > +++ b/profiles/battery/battery.c
> > @@ -88,12 +88,13 @@ static void batt_reset(struct batt *batt)
> > }
> >
> > static void parse_battery_level(struct batt *batt,
> > - const uint8_t *value)
> > + const uint8_t *value,
> > + bool force_update)
> > {
> > uint8_t percentage;
> >
> > percentage = value[0];
> > - if (batt->percentage != percentage) {
> > + if (force_update || batt->percentage != percentage) {
> > batt->percentage = percentage;
> > DBG("Battery Level updated: %d%%", percentage);
> > if (!batt->battery) {
> > @@ -110,7 +111,7 @@ static void batt_io_value_cb(uint16_t value_handle, const uint8_t *value,
> > struct batt *batt = user_data;
> >
> > if (value_handle == batt->batt_level_io_handle) {
> > - parse_battery_level(batt, value);
> > + parse_battery_level(batt, value, false /* force_update */);
> > } else {
> > g_assert_not_reached();
> > }
> > @@ -134,7 +135,7 @@ static void batt_io_ccc_written_cb(uint16_t att_ecode, void *user_data)
> > return;
> > }
> >
> > - parse_battery_level(batt, batt->initial_value);
> > + parse_battery_level(batt, batt->initial_value, true /* force_update */);
>
> I guess it would have been better to reset the initial_value on disconnect.
>
> > g_free (batt->initial_value);
> > batt->initial_value = NULL;
> >
> > --
> > 2.29.2
> >
>
>
> --
> Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH BlueZ] profiles/battery: Always update initial battery value
2021-03-30 17:09 [PATCH BlueZ] " Sonny Sasaka
2021-03-30 17:16 ` Bastien Nocera
@ 2021-03-30 18:25 ` Luiz Augusto von Dentz
2021-03-30 18:39 ` Sonny Sasaka
1 sibling, 1 reply; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2021-03-30 18:25 UTC (permalink / raw)
To: Sonny Sasaka; +Cc: linux-bluetooth
Hi Sonny,
On Tue, Mar 30, 2021 at 10:12 AM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
>
> Due to cache in gatt db, bluetoothd fails to update publish the battery
> value after reconnection when the battery value does not change compared
> to before reconnection. For initial battery value, we should update the
> value to D-Bus regardless of the cache value.
>
> ---
> profiles/battery/battery.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
> index 81f849d57..0f8d6ef18 100644
> --- a/profiles/battery/battery.c
> +++ b/profiles/battery/battery.c
> @@ -88,12 +88,13 @@ static void batt_reset(struct batt *batt)
> }
>
> static void parse_battery_level(struct batt *batt,
> - const uint8_t *value)
> + const uint8_t *value,
> + bool force_update)
> {
> uint8_t percentage;
>
> percentage = value[0];
> - if (batt->percentage != percentage) {
> + if (force_update || batt->percentage != percentage) {
> batt->percentage = percentage;
> DBG("Battery Level updated: %d%%", percentage);
> if (!batt->battery) {
> @@ -110,7 +111,7 @@ static void batt_io_value_cb(uint16_t value_handle, const uint8_t *value,
> struct batt *batt = user_data;
>
> if (value_handle == batt->batt_level_io_handle) {
> - parse_battery_level(batt, value);
> + parse_battery_level(batt, value, false /* force_update */);
> } else {
> g_assert_not_reached();
> }
> @@ -134,7 +135,7 @@ static void batt_io_ccc_written_cb(uint16_t att_ecode, void *user_data)
> return;
> }
>
> - parse_battery_level(batt, batt->initial_value);
> + parse_battery_level(batt, batt->initial_value, true /* force_update */);
I guess it would have been better to reset the initial_value on disconnect.
> g_free (batt->initial_value);
> batt->initial_value = NULL;
>
> --
> 2.29.2
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH BlueZ] profiles/battery: Always update initial battery value
2021-03-30 17:16 ` Bastien Nocera
@ 2021-03-30 18:23 ` Sonny Sasaka
0 siblings, 0 replies; 7+ messages in thread
From: Sonny Sasaka @ 2021-03-30 18:23 UTC (permalink / raw)
To: Bastien Nocera; +Cc: BlueZ
Hi Bastien,
That's a good idea. I updated it in v2.
On Tue, Mar 30, 2021 at 10:16 AM Bastien Nocera <hadess@hadess.net> wrote:
>
> On Tue, 2021-03-30 at 10:09 -0700, Sonny Sasaka wrote:
> > Due to cache in gatt db, bluetoothd fails to update publish the battery
> > value after reconnection when the battery value does not change
> > compared
> > to before reconnection. For initial battery value, we should update the
> > value to D-Bus regardless of the cache value.
> >
> > ---
> > profiles/battery/battery.c | 9 +++++----
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
> > index 81f849d57..0f8d6ef18 100644
> > --- a/profiles/battery/battery.c
> > +++ b/profiles/battery/battery.c
> > @@ -88,12 +88,13 @@ static void batt_reset(struct batt *batt)
> > }
> >
> > static void parse_battery_level(struct batt *batt,
> > - const uint8_t *value)
> > + const uint8_t *value,
> > + bool force_update)
> > {
> > uint8_t percentage;
> >
> > percentage = value[0];
> > - if (batt->percentage != percentage) {
> > + if (force_update || batt->percentage != percentage) {
> > batt->percentage = percentage;
> > DBG("Battery Level updated: %d%%", percentage);
> > if (!batt->battery) {
> > @@ -110,7 +111,7 @@ static void batt_io_value_cb(uint16_t value_handle,
> > const uint8_t *value,
> > struct batt *batt = user_data;
> >
> > if (value_handle == batt->batt_level_io_handle) {
> > - parse_battery_level(batt, value);
> > + parse_battery_level(batt, value, false /* force_update
> > */);
> > } else {
> > g_assert_not_reached();
> > }
> > @@ -134,7 +135,7 @@ static void batt_io_ccc_written_cb(uint16_t
> > att_ecode, void *user_data)
> > return;
> > }
> >
> > - parse_battery_level(batt, batt->initial_value);
> > + parse_battery_level(batt, batt->initial_value, true /*
> > force_update */);
>
> If you need to do this, that means that you should probably declare an
> enum instead.
>
> This is old, but still relevant:
> https://blog.ometer.com/2011/01/20/boolean-parameters-are-wrong/
>
> > g_free (batt->initial_value);
> > batt->initial_value = NULL;
> >
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH BlueZ] profiles/battery: Always update initial battery value
2021-03-30 17:09 [PATCH BlueZ] " Sonny Sasaka
@ 2021-03-30 17:16 ` Bastien Nocera
2021-03-30 18:23 ` Sonny Sasaka
2021-03-30 18:25 ` Luiz Augusto von Dentz
1 sibling, 1 reply; 7+ messages in thread
From: Bastien Nocera @ 2021-03-30 17:16 UTC (permalink / raw)
To: Sonny Sasaka, linux-bluetooth
On Tue, 2021-03-30 at 10:09 -0700, Sonny Sasaka wrote:
> Due to cache in gatt db, bluetoothd fails to update publish the battery
> value after reconnection when the battery value does not change
> compared
> to before reconnection. For initial battery value, we should update the
> value to D-Bus regardless of the cache value.
>
> ---
> profiles/battery/battery.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
> index 81f849d57..0f8d6ef18 100644
> --- a/profiles/battery/battery.c
> +++ b/profiles/battery/battery.c
> @@ -88,12 +88,13 @@ static void batt_reset(struct batt *batt)
> }
>
> static void parse_battery_level(struct batt *batt,
> - const uint8_t *value)
> + const uint8_t *value,
> + bool force_update)
> {
> uint8_t percentage;
>
> percentage = value[0];
> - if (batt->percentage != percentage) {
> + if (force_update || batt->percentage != percentage) {
> batt->percentage = percentage;
> DBG("Battery Level updated: %d%%", percentage);
> if (!batt->battery) {
> @@ -110,7 +111,7 @@ static void batt_io_value_cb(uint16_t value_handle,
> const uint8_t *value,
> struct batt *batt = user_data;
>
> if (value_handle == batt->batt_level_io_handle) {
> - parse_battery_level(batt, value);
> + parse_battery_level(batt, value, false /* force_update
> */);
> } else {
> g_assert_not_reached();
> }
> @@ -134,7 +135,7 @@ static void batt_io_ccc_written_cb(uint16_t
> att_ecode, void *user_data)
> return;
> }
>
> - parse_battery_level(batt, batt->initial_value);
> + parse_battery_level(batt, batt->initial_value, true /*
> force_update */);
If you need to do this, that means that you should probably declare an
enum instead.
This is old, but still relevant:
https://blog.ometer.com/2011/01/20/boolean-parameters-are-wrong/
> g_free (batt->initial_value);
> batt->initial_value = NULL;
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH BlueZ] profiles/battery: Always update initial battery value
@ 2021-03-30 17:09 Sonny Sasaka
2021-03-30 17:16 ` Bastien Nocera
2021-03-30 18:25 ` Luiz Augusto von Dentz
0 siblings, 2 replies; 7+ messages in thread
From: Sonny Sasaka @ 2021-03-30 17:09 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Sonny Sasaka
Due to cache in gatt db, bluetoothd fails to update publish the battery
value after reconnection when the battery value does not change compared
to before reconnection. For initial battery value, we should update the
value to D-Bus regardless of the cache value.
---
profiles/battery/battery.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
index 81f849d57..0f8d6ef18 100644
--- a/profiles/battery/battery.c
+++ b/profiles/battery/battery.c
@@ -88,12 +88,13 @@ static void batt_reset(struct batt *batt)
}
static void parse_battery_level(struct batt *batt,
- const uint8_t *value)
+ const uint8_t *value,
+ bool force_update)
{
uint8_t percentage;
percentage = value[0];
- if (batt->percentage != percentage) {
+ if (force_update || batt->percentage != percentage) {
batt->percentage = percentage;
DBG("Battery Level updated: %d%%", percentage);
if (!batt->battery) {
@@ -110,7 +111,7 @@ static void batt_io_value_cb(uint16_t value_handle, const uint8_t *value,
struct batt *batt = user_data;
if (value_handle == batt->batt_level_io_handle) {
- parse_battery_level(batt, value);
+ parse_battery_level(batt, value, false /* force_update */);
} else {
g_assert_not_reached();
}
@@ -134,7 +135,7 @@ static void batt_io_ccc_written_cb(uint16_t att_ecode, void *user_data)
return;
}
- parse_battery_level(batt, batt->initial_value);
+ parse_battery_level(batt, batt->initial_value, true /* force_update */);
g_free (batt->initial_value);
batt->initial_value = NULL;
--
2.29.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-03-30 18:40 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-30 17:11 [PATCH BlueZ] profiles/battery: Always update initial battery value Sonny Sasaka
2021-03-30 18:05 ` [BlueZ] " bluez.test.bot
-- strict thread matches above, loose matches on Subject: below --
2021-03-30 17:09 [PATCH BlueZ] " Sonny Sasaka
2021-03-30 17:16 ` Bastien Nocera
2021-03-30 18:23 ` Sonny Sasaka
2021-03-30 18:25 ` Luiz Augusto von Dentz
2021-03-30 18:39 ` Sonny Sasaka
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.