2022-04-04 22:52:41

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [RFC BlueZ] gap: Don't attempt to read the appearance if already set

From: Luiz Augusto von Dentz <[email protected]>

Devices are unlikely to change appearance over time which is the reason
why we cache then on the storage so this skips reading it on every
reconnection.
---
profiles/gap/gas.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/profiles/gap/gas.c b/profiles/gap/gas.c
index ea3249be9..400818d67 100644
--- a/profiles/gap/gas.c
+++ b/profiles/gap/gas.c
@@ -142,6 +142,11 @@ static void read_appearance_cb(bool success, uint8_t att_ecode,

static void handle_appearance(struct gas *gas, uint16_t value_handle)
{
+ uint16_t value;
+
+ if (!device_get_appearance(gas->device, &value))
+ return;
+
if (!bt_gatt_client_read_value(gas->client, value_handle,
read_appearance_cb, gas, NULL))
DBG("Failed to send request to read appearance");
--
2.35.1


2022-04-05 01:00:49

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [RFC BlueZ] gap: Don't attempt to read the appearance if already set

Hi Adam,

On Mon, Apr 4, 2022 at 1:17 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> From: Luiz Augusto von Dentz <[email protected]>
>
> Devices are unlikely to change appearance over time which is the reason
> why we cache then on the storage so this skips reading it on every
> reconnection.
> ---
> profiles/gap/gas.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/profiles/gap/gas.c b/profiles/gap/gas.c
> index ea3249be9..400818d67 100644
> --- a/profiles/gap/gas.c
> +++ b/profiles/gap/gas.c
> @@ -142,6 +142,11 @@ static void read_appearance_cb(bool success, uint8_t att_ecode,
>
> static void handle_appearance(struct gas *gas, uint16_t value_handle)
> {
> + uint16_t value;
> +
> + if (!device_get_appearance(gas->device, &value))
> + return;
> +
> if (!bt_gatt_client_read_value(gas->client, value_handle,
> read_appearance_cb, gas, NULL))
> DBG("Failed to send request to read appearance");
> --
> 2.35.1

Check if the above works for you.

--
Luiz Augusto von Dentz

2022-04-05 01:39:13

by bluez.test.bot

[permalink] [raw]
Subject: RE: [RFC,BlueZ] gap: Don't attempt to read the appearance if already set

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=628906

---Test result---

Test Summary:
CheckPatch PASS 0.65 seconds
GitLint PASS 0.45 seconds
Prep - Setup ELL PASS 45.95 seconds
Build - Prep PASS 0.44 seconds
Build - Configure PASS 9.39 seconds
Build - Make PASS 1311.83 seconds
Make Check PASS 12.17 seconds
Make Check w/Valgrind PASS 472.21 seconds
Make Distcheck PASS 252.30 seconds
Build w/ext ELL - Configure PASS 8.86 seconds
Build w/ext ELL - Make PASS 1291.83 seconds
Incremental Build with patchesPASS 0.00 seconds



---
Regards,
Linux Bluetooth

2022-04-05 06:02:36

by Paul Menzel

[permalink] [raw]
Subject: Re: [RFC BlueZ] gap: Don't attempt to read the appearance if already set

Dear Luiz,


Am 04.04.22 um 22:17 schrieb Luiz Augusto von Dentz:
> From: Luiz Augusto von Dentz <[email protected]>
>
> Devices are unlikely to change appearance over time which is the reason
> why we cache then on the storage so this skips reading it on every

s/then/them/

> reconnection.


Kind regards,

Paul

> ---
> profiles/gap/gas.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/profiles/gap/gas.c b/profiles/gap/gas.c
> index ea3249be9..400818d67 100644
> --- a/profiles/gap/gas.c
> +++ b/profiles/gap/gas.c
> @@ -142,6 +142,11 @@ static void read_appearance_cb(bool success, uint8_t att_ecode,
>
> static void handle_appearance(struct gas *gas, uint16_t value_handle)
> {
> + uint16_t value;
> +
> + if (!device_get_appearance(gas->device, &value))
> + return;
> +
> if (!bt_gatt_client_read_value(gas->client, value_handle,
> read_appearance_cb, gas, NULL))
> DBG("Failed to send request to read appearance");

2022-04-05 08:04:45

by Adam Pigg

[permalink] [raw]
Subject: Re: [RFC BlueZ] gap: Don't attempt to read the appearance if already set

Hi Luiz

On Mon, 4 Apr 2022 at 23:19, Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Adam,
>
> On Mon, Apr 4, 2022 at 1:17 PM Luiz Augusto von Dentz
> <[email protected]> wrote:
> >
> > From: Luiz Augusto von Dentz <[email protected]>
> >
> > Devices are unlikely to change appearance over time which is the reason
> > why we cache then on the storage so this skips reading it on every
> > reconnection.
> > ---
> > profiles/gap/gas.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/profiles/gap/gas.c b/profiles/gap/gas.c
> > index ea3249be9..400818d67 100644
> > --- a/profiles/gap/gas.c
> > +++ b/profiles/gap/gas.c
> > @@ -142,6 +142,11 @@ static void read_appearance_cb(bool success, uint8_t att_ecode,
> >
> > static void handle_appearance(struct gas *gas, uint16_t value_handle)
> > {
> > + uint16_t value;
> > +
> > + if (!device_get_appearance(gas->device, &value))
> > + return;
> > +
> > if (!bt_gatt_client_read_value(gas->client, value_handle,
> > read_appearance_cb, gas, NULL))
> > DBG("Failed to send request to read appearance");
> > --
> > 2.35.1
>
> Check if the above works for you.
>

Yes, this will work for me.

Reading the appearance still fails, but at least I can manually set it
in the info file, and I can instruct users to do the same,

The other way I was thinking about making it work would be to add some
kind of quirk to skip reading the appearance altogether, which would
probably also require editing a config file anyway, and this way, the
appearance value gets set (im using 0x0192)

Thanks



> --
> Luiz Augusto von Dentz

2022-04-07 01:33:46

by patchwork-bot+bluetooth

[permalink] [raw]
Subject: Re: [RFC BlueZ] gap: Don't attempt to read the appearance if already set

Hello:

This patch was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <[email protected]>:

On Mon, 4 Apr 2022 13:17:50 -0700 you wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> Devices are unlikely to change appearance over time which is the reason
> why we cache then on the storage so this skips reading it on every
> reconnection.
> ---
> profiles/gap/gas.c | 5 +++++
> 1 file changed, 5 insertions(+)

Here is the summary with links:
- [RFC,BlueZ] gap: Don't attempt to read the appearance if already set
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=bbeabca44a3d

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html