All of lore.kernel.org
 help / color / mirror / Atom feed
* [BlueZ RFC] profile: Add probe_on_discover flag
@ 2023-08-02 20:15 Luiz Augusto von Dentz
  2023-08-02 21:41 ` [BlueZ,RFC] " bluez.test.bot
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2023-08-02 20:15 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds probe_on_discover flag which indicates if profile needs to be
probed when the remote_uuid is discovered and changes device logic to
attempt to probe driver when a new UUID is discovered and
probe_on_discover is set.
---
 src/device.c  | 22 +++++++++++++++++-----
 src/profile.h |  5 +++++
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/src/device.c b/src/device.c
index b43ced8b5c91..19ae03f7d98a 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2156,7 +2156,7 @@ done:
 void device_add_eir_uuids(struct btd_device *dev, GSList *uuids)
 {
 	GSList *l;
-	bool added = false;
+	GSList *added = NULL;
 
 	if (dev->bredr_state.svc_resolved || dev->le_state.svc_resolved)
 		return;
@@ -2165,13 +2165,11 @@ void device_add_eir_uuids(struct btd_device *dev, GSList *uuids)
 		const char *str = l->data;
 		if (g_slist_find_custom(dev->eir_uuids, str, bt_uuid_strcmp))
 			continue;
-		added = true;
+		added = g_slist_append(added, (void *)str);
 		dev->eir_uuids = g_slist_append(dev->eir_uuids, g_strdup(str));
 	}
 
-	if (added)
-		g_dbus_emit_property_changed(dbus_conn, dev->path,
-						DEVICE_INTERFACE, "UUIDs");
+	device_probe_profiles(dev, added);
 }
 
 static void add_manufacturer_data(void *data, void *user_data)
@@ -2201,6 +2199,7 @@ static void add_service_data(void *data, void *user_data)
 	struct eir_sd *sd = data;
 	struct btd_device *dev = user_data;
 	bt_uuid_t uuid;
+	GSList *l;
 
 	if (bt_string_to_uuid(&uuid, sd->uuid) < 0)
 		return;
@@ -2208,6 +2207,10 @@ static void add_service_data(void *data, void *user_data)
 	if (!bt_ad_add_service_data(dev->ad, &uuid, sd->data, sd->data_len))
 		return;
 
+	l = g_slist_append(NULL, sd->uuid);
+	device_add_eir_uuids(dev, l);
+	g_slist_free(l);
+
 	g_dbus_emit_property_changed(dbus_conn, dev->path,
 					DEVICE_INTERFACE, "ServiceData");
 }
@@ -3930,6 +3933,12 @@ static bool device_match_profile(struct btd_device *device,
 	if (profile->remote_uuid == NULL)
 		return false;
 
+	/* Don't match if device was just discovered (not connected) and the
+	 * profile don't have probe_on_discover flag set.
+	 */
+	if (!btd_device_is_connected(device) && !profile->probe_on_discover)
+		return false;
+
 	if (g_slist_find_custom(uuids, profile->remote_uuid,
 							bt_uuid_strcmp) == NULL)
 		return false;
@@ -4884,6 +4893,9 @@ void device_probe_profiles(struct btd_device *device, GSList *uuids)
 	struct probe_data d = { device, uuids };
 	char addr[18];
 
+	if (!uuids)
+		return;
+
 	ba2str(&device->bdaddr, addr);
 
 	if (device->blocked) {
diff --git a/src/profile.h b/src/profile.h
index 6871f2f0d7d8..cfc50058812d 100644
--- a/src/profile.h
+++ b/src/profile.h
@@ -33,6 +33,11 @@ struct btd_profile {
 	 */
 	bool experimental;
 
+	/* Indicates the profile needs to be probed when the remote_uuid is
+	 * discovered.
+	 */
+	bool probe_on_discover;
+
 	int (*device_probe) (struct btd_service *service);
 	void (*device_remove) (struct btd_service *service);
 
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* RE: [BlueZ,RFC] profile: Add probe_on_discover flag
  2023-08-02 20:15 [BlueZ RFC] profile: Add probe_on_discover flag Luiz Augusto von Dentz
@ 2023-08-02 21:41 ` bluez.test.bot
  2023-08-09 17:20 ` [BlueZ RFC] " patchwork-bot+bluetooth
  2023-08-09 18:31 ` Pauli Virtanen
  2 siblings, 0 replies; 7+ messages in thread
From: bluez.test.bot @ 2023-08-02 21:41 UTC (permalink / raw)
  To: linux-bluetooth, luiz.dentz

[-- Attachment #1: Type: text/plain, Size: 948 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=772324

---Test result---

Test Summary:
CheckPatch                    PASS      0.55 seconds
GitLint                       PASS      0.35 seconds
BuildEll                      PASS      27.75 seconds
BluezMake                     PASS      1007.36 seconds
MakeCheck                     PASS      12.02 seconds
MakeDistcheck                 PASS      158.45 seconds
CheckValgrind                 PASS      259.81 seconds
CheckSmatch                   PASS      345.99 seconds
bluezmakeextell               PASS      106.11 seconds
IncrementalBuild              PASS      850.98 seconds
ScanBuild                     PASS      1094.63 seconds



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [BlueZ RFC] profile: Add probe_on_discover flag
  2023-08-02 20:15 [BlueZ RFC] profile: Add probe_on_discover flag Luiz Augusto von Dentz
  2023-08-02 21:41 ` [BlueZ,RFC] " bluez.test.bot
@ 2023-08-09 17:20 ` patchwork-bot+bluetooth
  2023-08-09 18:31 ` Pauli Virtanen
  2 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+bluetooth @ 2023-08-09 17:20 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hello:

This patch was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Wed,  2 Aug 2023 13:15:38 -0700 you wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> This adds probe_on_discover flag which indicates if profile needs to be
> probed when the remote_uuid is discovered and changes device logic to
> attempt to probe driver when a new UUID is discovered and
> probe_on_discover is set.
> 
> [...]

Here is the summary with links:
  - [BlueZ,RFC] profile: Add probe_on_discover flag
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=67a26abe53bf

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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [BlueZ RFC] profile: Add probe_on_discover flag
  2023-08-02 20:15 [BlueZ RFC] profile: Add probe_on_discover flag Luiz Augusto von Dentz
  2023-08-02 21:41 ` [BlueZ,RFC] " bluez.test.bot
  2023-08-09 17:20 ` [BlueZ RFC] " patchwork-bot+bluetooth
@ 2023-08-09 18:31 ` Pauli Virtanen
  2023-08-09 19:17   ` Luiz Augusto von Dentz
  2 siblings, 1 reply; 7+ messages in thread
From: Pauli Virtanen @ 2023-08-09 18:31 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

ke, 2023-08-02 kello 13:15 -0700, Luiz Augusto von Dentz kirjoitti:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> This adds probe_on_discover flag which indicates if profile needs to be
> probed when the remote_uuid is discovered and changes device logic to
> attempt to probe driver when a new UUID is discovered and
> probe_on_discover is set.
> ---
>  src/device.c  | 22 +++++++++++++++++-----
>  src/profile.h |  5 +++++
>  2 files changed, 22 insertions(+), 5 deletions(-)

Note this seems to break (unicast) BAP for me, bap_probe does not seem
to be called any more on the devices.

Previously it is called immediately on bluetoothd start, not clear if
that is how it should be.

> 
> diff --git a/src/device.c b/src/device.c
> index b43ced8b5c91..19ae03f7d98a 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -2156,7 +2156,7 @@ done:
>  void device_add_eir_uuids(struct btd_device *dev, GSList *uuids)
>  {
>  	GSList *l;
> -	bool added = false;
> +	GSList *added = NULL;
>  
>  	if (dev->bredr_state.svc_resolved || dev->le_state.svc_resolved)
>  		return;
> @@ -2165,13 +2165,11 @@ void device_add_eir_uuids(struct btd_device *dev, GSList *uuids)
>  		const char *str = l->data;
>  		if (g_slist_find_custom(dev->eir_uuids, str, bt_uuid_strcmp))
>  			continue;
> -		added = true;
> +		added = g_slist_append(added, (void *)str);
>  		dev->eir_uuids = g_slist_append(dev->eir_uuids, g_strdup(str));
>  	}
>  
> -	if (added)
> -		g_dbus_emit_property_changed(dbus_conn, dev->path,
> -						DEVICE_INTERFACE, "UUIDs");
> +	device_probe_profiles(dev, added);
>  }
>  
>  static void add_manufacturer_data(void *data, void *user_data)
> @@ -2201,6 +2199,7 @@ static void add_service_data(void *data, void *user_data)
>  	struct eir_sd *sd = data;
>  	struct btd_device *dev = user_data;
>  	bt_uuid_t uuid;
> +	GSList *l;
>  
>  	if (bt_string_to_uuid(&uuid, sd->uuid) < 0)
>  		return;
> @@ -2208,6 +2207,10 @@ static void add_service_data(void *data, void *user_data)
>  	if (!bt_ad_add_service_data(dev->ad, &uuid, sd->data, sd->data_len))
>  		return;
>  
> +	l = g_slist_append(NULL, sd->uuid);
> +	device_add_eir_uuids(dev, l);
> +	g_slist_free(l);
> +
>  	g_dbus_emit_property_changed(dbus_conn, dev->path,
>  					DEVICE_INTERFACE, "ServiceData");
>  }
> @@ -3930,6 +3933,12 @@ static bool device_match_profile(struct btd_device *device,
>  	if (profile->remote_uuid == NULL)
>  		return false;
>  
> +	/* Don't match if device was just discovered (not connected) and the
> +	 * profile don't have probe_on_discover flag set.
> +	 */
> +	if (!btd_device_is_connected(device) && !profile->probe_on_discover)
> +		return false;
> +
>  	if (g_slist_find_custom(uuids, profile->remote_uuid,
>  							bt_uuid_strcmp) == NULL)
>  		return false;
> @@ -4884,6 +4893,9 @@ void device_probe_profiles(struct btd_device *device, GSList *uuids)
>  	struct probe_data d = { device, uuids };
>  	char addr[18];
>  
> +	if (!uuids)
> +		return;
> +
>  	ba2str(&device->bdaddr, addr);
>  
>  	if (device->blocked) {
> diff --git a/src/profile.h b/src/profile.h
> index 6871f2f0d7d8..cfc50058812d 100644
> --- a/src/profile.h
> +++ b/src/profile.h
> @@ -33,6 +33,11 @@ struct btd_profile {
>  	 */
>  	bool experimental;
>  
> +	/* Indicates the profile needs to be probed when the remote_uuid is
> +	 * discovered.
> +	 */
> +	bool probe_on_discover;
> +
>  	int (*device_probe) (struct btd_service *service);
>  	void (*device_remove) (struct btd_service *service);
>  


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [BlueZ RFC] profile: Add probe_on_discover flag
  2023-08-09 18:31 ` Pauli Virtanen
@ 2023-08-09 19:17   ` Luiz Augusto von Dentz
  2023-08-09 19:39     ` Pauli Virtanen
  0 siblings, 1 reply; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2023-08-09 19:17 UTC (permalink / raw)
  To: Pauli Virtanen; +Cc: linux-bluetooth

Hi Pauli,

On Wed, Aug 9, 2023 at 11:31 AM Pauli Virtanen <pav@iki.fi> wrote:
>
> Hi Luiz,
>
> ke, 2023-08-02 kello 13:15 -0700, Luiz Augusto von Dentz kirjoitti:
> > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >
> > This adds probe_on_discover flag which indicates if profile needs to be
> > probed when the remote_uuid is discovered and changes device logic to
> > attempt to probe driver when a new UUID is discovered and
> > probe_on_discover is set.
> > ---
> >  src/device.c  | 22 +++++++++++++++++-----
> >  src/profile.h |  5 +++++
> >  2 files changed, 22 insertions(+), 5 deletions(-)
>
> Note this seems to break (unicast) BAP for me, bap_probe does not seem
> to be called any more on the devices.

On master?

> Previously it is called immediately on bluetoothd start, not clear if
> that is how it should be.

It should be probed as per usual, so there might be a bug with it then.

> >
> > diff --git a/src/device.c b/src/device.c
> > index b43ced8b5c91..19ae03f7d98a 100644
> > --- a/src/device.c
> > +++ b/src/device.c
> > @@ -2156,7 +2156,7 @@ done:
> >  void device_add_eir_uuids(struct btd_device *dev, GSList *uuids)
> >  {
> >       GSList *l;
> > -     bool added = false;
> > +     GSList *added = NULL;
> >
> >       if (dev->bredr_state.svc_resolved || dev->le_state.svc_resolved)
> >               return;
> > @@ -2165,13 +2165,11 @@ void device_add_eir_uuids(struct btd_device *dev, GSList *uuids)
> >               const char *str = l->data;
> >               if (g_slist_find_custom(dev->eir_uuids, str, bt_uuid_strcmp))
> >                       continue;
> > -             added = true;
> > +             added = g_slist_append(added, (void *)str);
> >               dev->eir_uuids = g_slist_append(dev->eir_uuids, g_strdup(str));
> >       }
> >
> > -     if (added)
> > -             g_dbus_emit_property_changed(dbus_conn, dev->path,
> > -                                             DEVICE_INTERFACE, "UUIDs");
> > +     device_probe_profiles(dev, added);
> >  }
> >
> >  static void add_manufacturer_data(void *data, void *user_data)
> > @@ -2201,6 +2199,7 @@ static void add_service_data(void *data, void *user_data)
> >       struct eir_sd *sd = data;
> >       struct btd_device *dev = user_data;
> >       bt_uuid_t uuid;
> > +     GSList *l;
> >
> >       if (bt_string_to_uuid(&uuid, sd->uuid) < 0)
> >               return;
> > @@ -2208,6 +2207,10 @@ static void add_service_data(void *data, void *user_data)
> >       if (!bt_ad_add_service_data(dev->ad, &uuid, sd->data, sd->data_len))
> >               return;
> >
> > +     l = g_slist_append(NULL, sd->uuid);
> > +     device_add_eir_uuids(dev, l);
> > +     g_slist_free(l);
> > +
> >       g_dbus_emit_property_changed(dbus_conn, dev->path,
> >                                       DEVICE_INTERFACE, "ServiceData");
> >  }
> > @@ -3930,6 +3933,12 @@ static bool device_match_profile(struct btd_device *device,
> >       if (profile->remote_uuid == NULL)
> >               return false;
> >
> > +     /* Don't match if device was just discovered (not connected) and the
> > +      * profile don't have probe_on_discover flag set.
> > +      */
> > +     if (!btd_device_is_connected(device) && !profile->probe_on_discover)
> > +             return false;
> > +
> >       if (g_slist_find_custom(uuids, profile->remote_uuid,
> >                                                       bt_uuid_strcmp) == NULL)
> >               return false;
> > @@ -4884,6 +4893,9 @@ void device_probe_profiles(struct btd_device *device, GSList *uuids)
> >       struct probe_data d = { device, uuids };
> >       char addr[18];
> >
> > +     if (!uuids)
> > +             return;
> > +
> >       ba2str(&device->bdaddr, addr);
> >
> >       if (device->blocked) {
> > diff --git a/src/profile.h b/src/profile.h
> > index 6871f2f0d7d8..cfc50058812d 100644
> > --- a/src/profile.h
> > +++ b/src/profile.h
> > @@ -33,6 +33,11 @@ struct btd_profile {
> >        */
> >       bool experimental;
> >
> > +     /* Indicates the profile needs to be probed when the remote_uuid is
> > +      * discovered.
> > +      */
> > +     bool probe_on_discover;
> > +
> >       int (*device_probe) (struct btd_service *service);
> >       void (*device_remove) (struct btd_service *service);
> >
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [BlueZ RFC] profile: Add probe_on_discover flag
  2023-08-09 19:17   ` Luiz Augusto von Dentz
@ 2023-08-09 19:39     ` Pauli Virtanen
  2023-08-09 19:53       ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 7+ messages in thread
From: Pauli Virtanen @ 2023-08-09 19:39 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

ke, 2023-08-09 kello 12:17 -0700, Luiz Augusto von Dentz kirjoitti:
> Hi Pauli,
> 
> On Wed, Aug 9, 2023 at 11:31 AM Pauli Virtanen <pav@iki.fi> wrote:
> > 
> > Hi Luiz,
> > 
> > ke, 2023-08-02 kello 13:15 -0700, Luiz Augusto von Dentz kirjoitti:
> > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > 
> > > This adds probe_on_discover flag which indicates if profile needs to be
> > > probed when the remote_uuid is discovered and changes device logic to
> > > attempt to probe driver when a new UUID is discovered and
> > > probe_on_discover is set.
> > > ---
> > >  src/device.c  | 22 +++++++++++++++++-----
> > >  src/profile.h |  5 +++++
> > >  2 files changed, 22 insertions(+), 5 deletions(-)
> > 
> > Note this seems to break (unicast) BAP for me, bap_probe does not seem
> > to be called any more on the devices.
> 
> On master?

Yes, at 3370c462552b0 I have on bluetoothd startup

elo 09 22:35:02 fedora bluetoothd[2611]: src/adapter.c:load_irks() hci0 irks 2
elo 09 22:35:02 fedora bluetoothd[2611]: src/adapter.c:load_conn_params() hci0 conn params 2
elo 09 22:35:02 fedora bluetoothd[2611]: src/device.c:device_probe_profiles() Probing profiles for device 28:3D:C2:4A:7D:2A
elo 09 22:35:02 fedora bluetoothd[2611]: profiles/audio/csip.c:csip_probe() 28:3D:C2:4A:7D:2A
elo 09 22:35:02 fedora bluetoothd[2611]: profiles/audio/csip.c:csip_data_add() data 0x603000018250
elo 09 22:35:02 fedora bluetoothd[2611]: src/service.c:change_state() 0x60400000b1d0: device 28:3D:C2:4A:7D:2A profile csip state changed: unavailable -> disconnected (0)
elo 09 22:35:02 fedora bluetoothd[2611]: profiles/audio/bass.c:bass_probe() 28:3D:C2:4A:7D:2A
elo 09 22:35:02 fedora bluetoothd[2611]: profiles/audio/bass.c:bass_data_add() data 0x603000018310
elo 09 22:35:02 fedora bluetoothd[2611]: src/service.c:change_state() 0x60400000b210: device 28:3D:C2:4A:7D:2A profile bass state changed: unavailable -> disconnected (0)
elo 09 22:35:02 fedora bluetoothd[2611]: profiles/audio/bap.c:bap_probe() 28:3D:C2:4A:7D:2A
elo 09 22:35:02 fedora bluetoothd[2611]: profiles/audio/bap.c:bap_data_add() data 0x60b000019060
elo 09 22:35:02 fedora bluetoothd[2611]: src/service.c:change_state() 0x60400000b250: device 28:3D:C2:4A:7D:2A profile bap state changed: unavailable -> disconnected (0)
elo 09 22:35:02 fedora bluetoothd[2611]: profiles/battery/battery.c:batt_probe() BATT profile probe (28:3D:C2:4A:7D:2A)
elo 09 22:35:02 fedora bluetoothd[2611]: src/service.c:change_state() 0x60400000b310: device 28:3D:C2:4A:7D:2A profile batt-profile state changed: unavailable -> disconnected (0)
elo 09 22:35:02 fedora bluetoothd[2611]: src/service.c:change_state() 0x60400000b350: device 28:3D:C2:4A:7D:2A profile deviceinfo state changed: unavailable -> disconnected (0)
elo 09 22:35:02 fedora bluetoothd[2611]: profiles/gap/gas.c:gap_probe() GAP profile probe (28:3D:C2:4A:7D:2A)
elo 09 22:35:02 fedora bluetoothd[2611]: src/service.c:change_state() 0x60400000b390: device 28:3D:C2:4A:7D:2A profile gap-profile state changed: unavailable -> disconnected (0)
elo 09 22:35:02 fedora bluetoothd[2611]: src/device.c:device_probe_profiles() Probing profiles for device 28:3D:C2:4A:7E:DA
...

but at 67a26abe53bf it becomes

elo 09 22:37:00 fedora bluetoothd[2861]: src/adapter.c:load_irks() hci0 irks 2
elo 09 22:37:00 fedora bluetoothd[2861]: src/adapter.c:load_conn_params() hci0 conn params 2
elo 09 22:37:00 fedora bluetoothd[2861]: src/device.c:device_probe_profiles() Probing profiles for device 28:3D:C2:4A:7D:2A
elo 09 22:37:00 fedora bluetoothd[2861]: src/device.c:device_probe_profiles() Probing profiles for device 28:3D:C2:4A:7E:DA

The device is not connected at this point, but it also doesn't probe
the profiles when connecting.

> 
> > Previously it is called immediately on bluetoothd start, not clear if
> > that is how it should be.
> 
> It should be probed as per usual, so there might be a bug with it then.
> 
> > > 
> > > diff --git a/src/device.c b/src/device.c
> > > index b43ced8b5c91..19ae03f7d98a 100644
> > > --- a/src/device.c
> > > +++ b/src/device.c
> > > @@ -2156,7 +2156,7 @@ done:
> > >  void device_add_eir_uuids(struct btd_device *dev, GSList *uuids)
> > >  {
> > >       GSList *l;
> > > -     bool added = false;
> > > +     GSList *added = NULL;
> > > 
> > >       if (dev->bredr_state.svc_resolved || dev->le_state.svc_resolved)
> > >               return;
> > > @@ -2165,13 +2165,11 @@ void device_add_eir_uuids(struct btd_device *dev, GSList *uuids)
> > >               const char *str = l->data;
> > >               if (g_slist_find_custom(dev->eir_uuids, str, bt_uuid_strcmp))
> > >                       continue;
> > > -             added = true;
> > > +             added = g_slist_append(added, (void *)str);
> > >               dev->eir_uuids = g_slist_append(dev->eir_uuids, g_strdup(str));
> > >       }
> > > 
> > > -     if (added)
> > > -             g_dbus_emit_property_changed(dbus_conn, dev->path,
> > > -                                             DEVICE_INTERFACE, "UUIDs");
> > > +     device_probe_profiles(dev, added);
> > >  }
> > > 
> > >  static void add_manufacturer_data(void *data, void *user_data)
> > > @@ -2201,6 +2199,7 @@ static void add_service_data(void *data, void *user_data)
> > >       struct eir_sd *sd = data;
> > >       struct btd_device *dev = user_data;
> > >       bt_uuid_t uuid;
> > > +     GSList *l;
> > > 
> > >       if (bt_string_to_uuid(&uuid, sd->uuid) < 0)
> > >               return;
> > > @@ -2208,6 +2207,10 @@ static void add_service_data(void *data, void *user_data)
> > >       if (!bt_ad_add_service_data(dev->ad, &uuid, sd->data, sd->data_len))
> > >               return;
> > > 
> > > +     l = g_slist_append(NULL, sd->uuid);
> > > +     device_add_eir_uuids(dev, l);
> > > +     g_slist_free(l);
> > > +
> > >       g_dbus_emit_property_changed(dbus_conn, dev->path,
> > >                                       DEVICE_INTERFACE, "ServiceData");
> > >  }
> > > @@ -3930,6 +3933,12 @@ static bool device_match_profile(struct btd_device *device,
> > >       if (profile->remote_uuid == NULL)
> > >               return false;
> > > 
> > > +     /* Don't match if device was just discovered (not connected) and the
> > > +      * profile don't have probe_on_discover flag set.
> > > +      */
> > > +     if (!btd_device_is_connected(device) && !profile->probe_on_discover)
> > > +             return false;
> > > +
> > >       if (g_slist_find_custom(uuids, profile->remote_uuid,
> > >                                                       bt_uuid_strcmp) == NULL)
> > >               return false;
> > > @@ -4884,6 +4893,9 @@ void device_probe_profiles(struct btd_device *device, GSList *uuids)
> > >       struct probe_data d = { device, uuids };
> > >       char addr[18];
> > > 
> > > +     if (!uuids)
> > > +             return;
> > > +
> > >       ba2str(&device->bdaddr, addr);
> > > 
> > >       if (device->blocked) {
> > > diff --git a/src/profile.h b/src/profile.h
> > > index 6871f2f0d7d8..cfc50058812d 100644
> > > --- a/src/profile.h
> > > +++ b/src/profile.h
> > > @@ -33,6 +33,11 @@ struct btd_profile {
> > >        */
> > >       bool experimental;
> > > 
> > > +     /* Indicates the profile needs to be probed when the remote_uuid is
> > > +      * discovered.
> > > +      */
> > > +     bool probe_on_discover;
> > > +
> > >       int (*device_probe) (struct btd_service *service);
> > >       void (*device_remove) (struct btd_service *service);
> > > 
> > 
> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [BlueZ RFC] profile: Add probe_on_discover flag
  2023-08-09 19:39     ` Pauli Virtanen
@ 2023-08-09 19:53       ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2023-08-09 19:53 UTC (permalink / raw)
  To: Pauli Virtanen; +Cc: linux-bluetooth

Hi Pauli,

On Wed, Aug 9, 2023 at 12:39 PM Pauli Virtanen <pav@iki.fi> wrote:
>
> Hi Luiz,
>
> ke, 2023-08-09 kello 12:17 -0700, Luiz Augusto von Dentz kirjoitti:
> > Hi Pauli,
> >
> > On Wed, Aug 9, 2023 at 11:31 AM Pauli Virtanen <pav@iki.fi> wrote:
> > >
> > > Hi Luiz,
> > >
> > > ke, 2023-08-02 kello 13:15 -0700, Luiz Augusto von Dentz kirjoitti:
> > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > >
> > > > This adds probe_on_discover flag which indicates if profile needs to be
> > > > probed when the remote_uuid is discovered and changes device logic to
> > > > attempt to probe driver when a new UUID is discovered and
> > > > probe_on_discover is set.
> > > > ---
> > > >  src/device.c  | 22 +++++++++++++++++-----
> > > >  src/profile.h |  5 +++++
> > > >  2 files changed, 22 insertions(+), 5 deletions(-)
> > >
> > > Note this seems to break (unicast) BAP for me, bap_probe does not seem
> > > to be called any more on the devices.
> >
> > On master?
>
> Yes, at 3370c462552b0 I have on bluetoothd startup
>
> elo 09 22:35:02 fedora bluetoothd[2611]: src/adapter.c:load_irks() hci0 irks 2
> elo 09 22:35:02 fedora bluetoothd[2611]: src/adapter.c:load_conn_params() hci0 conn params 2
> elo 09 22:35:02 fedora bluetoothd[2611]: src/device.c:device_probe_profiles() Probing profiles for device 28:3D:C2:4A:7D:2A
> elo 09 22:35:02 fedora bluetoothd[2611]: profiles/audio/csip.c:csip_probe() 28:3D:C2:4A:7D:2A
> elo 09 22:35:02 fedora bluetoothd[2611]: profiles/audio/csip.c:csip_data_add() data 0x603000018250
> elo 09 22:35:02 fedora bluetoothd[2611]: src/service.c:change_state() 0x60400000b1d0: device 28:3D:C2:4A:7D:2A profile csip state changed: unavailable -> disconnected (0)
> elo 09 22:35:02 fedora bluetoothd[2611]: profiles/audio/bass.c:bass_probe() 28:3D:C2:4A:7D:2A
> elo 09 22:35:02 fedora bluetoothd[2611]: profiles/audio/bass.c:bass_data_add() data 0x603000018310
> elo 09 22:35:02 fedora bluetoothd[2611]: src/service.c:change_state() 0x60400000b210: device 28:3D:C2:4A:7D:2A profile bass state changed: unavailable -> disconnected (0)
> elo 09 22:35:02 fedora bluetoothd[2611]: profiles/audio/bap.c:bap_probe() 28:3D:C2:4A:7D:2A
> elo 09 22:35:02 fedora bluetoothd[2611]: profiles/audio/bap.c:bap_data_add() data 0x60b000019060
> elo 09 22:35:02 fedora bluetoothd[2611]: src/service.c:change_state() 0x60400000b250: device 28:3D:C2:4A:7D:2A profile bap state changed: unavailable -> disconnected (0)
> elo 09 22:35:02 fedora bluetoothd[2611]: profiles/battery/battery.c:batt_probe() BATT profile probe (28:3D:C2:4A:7D:2A)
> elo 09 22:35:02 fedora bluetoothd[2611]: src/service.c:change_state() 0x60400000b310: device 28:3D:C2:4A:7D:2A profile batt-profile state changed: unavailable -> disconnected (0)
> elo 09 22:35:02 fedora bluetoothd[2611]: src/service.c:change_state() 0x60400000b350: device 28:3D:C2:4A:7D:2A profile deviceinfo state changed: unavailable -> disconnected (0)
> elo 09 22:35:02 fedora bluetoothd[2611]: profiles/gap/gas.c:gap_probe() GAP profile probe (28:3D:C2:4A:7D:2A)
> elo 09 22:35:02 fedora bluetoothd[2611]: src/service.c:change_state() 0x60400000b390: device 28:3D:C2:4A:7D:2A profile gap-profile state changed: unavailable -> disconnected (0)
> elo 09 22:35:02 fedora bluetoothd[2611]: src/device.c:device_probe_profiles() Probing profiles for device 28:3D:C2:4A:7E:DA
> ...
>
> but at 67a26abe53bf it becomes
>
> elo 09 22:37:00 fedora bluetoothd[2861]: src/adapter.c:load_irks() hci0 irks 2
> elo 09 22:37:00 fedora bluetoothd[2861]: src/adapter.c:load_conn_params() hci0 conn params 2
> elo 09 22:37:00 fedora bluetoothd[2861]: src/device.c:device_probe_profiles() Probing profiles for device 28:3D:C2:4A:7D:2A
> elo 09 22:37:00 fedora bluetoothd[2861]: src/device.c:device_probe_profiles() Probing profiles for device 28:3D:C2:4A:7E:DA
>
> The device is not connected at this point, but it also doesn't probe
> the profiles when connecting.

Yep, seems like we should be checking if the device is temporary
rather than connected:

https://patchwork.kernel.org/project/bluetooth/patch/20230809194620.1595792-1-luiz.dentz@gmail.com/

> >
> > > Previously it is called immediately on bluetoothd start, not clear if
> > > that is how it should be.
> >
> > It should be probed as per usual, so there might be a bug with it then.
> >
> > > >
> > > > diff --git a/src/device.c b/src/device.c
> > > > index b43ced8b5c91..19ae03f7d98a 100644
> > > > --- a/src/device.c
> > > > +++ b/src/device.c
> > > > @@ -2156,7 +2156,7 @@ done:
> > > >  void device_add_eir_uuids(struct btd_device *dev, GSList *uuids)
> > > >  {
> > > >       GSList *l;
> > > > -     bool added = false;
> > > > +     GSList *added = NULL;
> > > >
> > > >       if (dev->bredr_state.svc_resolved || dev->le_state.svc_resolved)
> > > >               return;
> > > > @@ -2165,13 +2165,11 @@ void device_add_eir_uuids(struct btd_device *dev, GSList *uuids)
> > > >               const char *str = l->data;
> > > >               if (g_slist_find_custom(dev->eir_uuids, str, bt_uuid_strcmp))
> > > >                       continue;
> > > > -             added = true;
> > > > +             added = g_slist_append(added, (void *)str);
> > > >               dev->eir_uuids = g_slist_append(dev->eir_uuids, g_strdup(str));
> > > >       }
> > > >
> > > > -     if (added)
> > > > -             g_dbus_emit_property_changed(dbus_conn, dev->path,
> > > > -                                             DEVICE_INTERFACE, "UUIDs");
> > > > +     device_probe_profiles(dev, added);
> > > >  }
> > > >
> > > >  static void add_manufacturer_data(void *data, void *user_data)
> > > > @@ -2201,6 +2199,7 @@ static void add_service_data(void *data, void *user_data)
> > > >       struct eir_sd *sd = data;
> > > >       struct btd_device *dev = user_data;
> > > >       bt_uuid_t uuid;
> > > > +     GSList *l;
> > > >
> > > >       if (bt_string_to_uuid(&uuid, sd->uuid) < 0)
> > > >               return;
> > > > @@ -2208,6 +2207,10 @@ static void add_service_data(void *data, void *user_data)
> > > >       if (!bt_ad_add_service_data(dev->ad, &uuid, sd->data, sd->data_len))
> > > >               return;
> > > >
> > > > +     l = g_slist_append(NULL, sd->uuid);
> > > > +     device_add_eir_uuids(dev, l);
> > > > +     g_slist_free(l);
> > > > +
> > > >       g_dbus_emit_property_changed(dbus_conn, dev->path,
> > > >                                       DEVICE_INTERFACE, "ServiceData");
> > > >  }
> > > > @@ -3930,6 +3933,12 @@ static bool device_match_profile(struct btd_device *device,
> > > >       if (profile->remote_uuid == NULL)
> > > >               return false;
> > > >
> > > > +     /* Don't match if device was just discovered (not connected) and the
> > > > +      * profile don't have probe_on_discover flag set.
> > > > +      */
> > > > +     if (!btd_device_is_connected(device) && !profile->probe_on_discover)
> > > > +             return false;
> > > > +
> > > >       if (g_slist_find_custom(uuids, profile->remote_uuid,
> > > >                                                       bt_uuid_strcmp) == NULL)
> > > >               return false;
> > > > @@ -4884,6 +4893,9 @@ void device_probe_profiles(struct btd_device *device, GSList *uuids)
> > > >       struct probe_data d = { device, uuids };
> > > >       char addr[18];
> > > >
> > > > +     if (!uuids)
> > > > +             return;
> > > > +
> > > >       ba2str(&device->bdaddr, addr);
> > > >
> > > >       if (device->blocked) {
> > > > diff --git a/src/profile.h b/src/profile.h
> > > > index 6871f2f0d7d8..cfc50058812d 100644
> > > > --- a/src/profile.h
> > > > +++ b/src/profile.h
> > > > @@ -33,6 +33,11 @@ struct btd_profile {
> > > >        */
> > > >       bool experimental;
> > > >
> > > > +     /* Indicates the profile needs to be probed when the remote_uuid is
> > > > +      * discovered.
> > > > +      */
> > > > +     bool probe_on_discover;
> > > > +
> > > >       int (*device_probe) (struct btd_service *service);
> > > >       void (*device_remove) (struct btd_service *service);
> > > >
> > >
> >
> >
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-08-09 19:54 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-02 20:15 [BlueZ RFC] profile: Add probe_on_discover flag Luiz Augusto von Dentz
2023-08-02 21:41 ` [BlueZ,RFC] " bluez.test.bot
2023-08-09 17:20 ` [BlueZ RFC] " patchwork-bot+bluetooth
2023-08-09 18:31 ` Pauli Virtanen
2023-08-09 19:17   ` Luiz Augusto von Dentz
2023-08-09 19:39     ` Pauli Virtanen
2023-08-09 19:53       ` Luiz Augusto von Dentz

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.