Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210330170924.16983-1-sonnysasaka@chromium.org> <CABBYNZ+XOdrtNBjGjUtJrV=SdiCEkMc80=4UFX17v+5Z1ojm5w@mail.gmail.com>
In-Reply-To: <CABBYNZ+XOdrtNBjGjUtJrV=SdiCEkMc80=4UFX17v+5Z1ojm5w@mail.gmail.com>
From:   Sonny Sasaka <sonnysasaka@chromium.org>
Date:   Tue, 30 Mar 2021 11:39:02 -0700
Message-ID: <CAO271mkMkjonV2nNxAsH6owKFE9ktEK71ShLKBzCn4p1MPH6ww@mail.gmail.com>
Subject: Re: [PATCH BlueZ] profiles/battery: Always update initial battery value
To:     Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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