Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <4e26a59c2ba25be76f96aa1806ab2132f5f7e069.camel@iki.fi>
Subject: Re: [BlueZ RFC] profile: Add probe_on_discover flag
From:   Pauli Virtanen <pav@iki.fi>
To:     Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc:     linux-bluetooth@vger.kernel.org
Date:   Wed, 09 Aug 2023 21:31:37 +0300
In-Reply-To: <20230802201538.584029-1-luiz.dentz@gmail.com>
References: <20230802201538.584029-1-luiz.dentz@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

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>
>=20
> 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.

>=20
> 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 =3D false;
> +	GSList *added =3D NULL;
> =20
>  	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 =3D l->data;
>  		if (g_slist_find_custom(dev->eir_uuids, str, bt_uuid_strcmp))
>  			continue;
> -		added =3D true;
> +		added =3D g_slist_append(added, (void *)str);
>  		dev->eir_uuids =3D g_slist_append(dev->eir_uuids, g_strdup(str));
>  	}
> =20
> -	if (added)
> -		g_dbus_emit_property_changed(dbus_conn, dev->path,
> -						DEVICE_INTERFACE, "UUIDs");
> +	device_probe_profiles(dev, added);
>  }
> =20
>  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 =3D data;
>  	struct btd_device *dev =3D user_data;
>  	bt_uuid_t uuid;
> +	GSList *l;
> =20
>  	if (bt_string_to_uuid(&uuid, sd->uuid) < 0)
>  		return;
> @@ -2208,6 +2207,10 @@ static void add_service_data(void *data, void *use=
r_data)
>  	if (!bt_ad_add_service_data(dev->ad, &uuid, sd->data, sd->data_len))
>  		return;
> =20
> +	l =3D 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 =3D=3D NULL)
>  		return false;
> =20
> +	/* 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) =3D=3D NULL)
>  		return false;
> @@ -4884,6 +4893,9 @@ void device_probe_profiles(struct btd_device *devic=
e, GSList *uuids)
>  	struct probe_data d =3D { device, uuids };
>  	char addr[18];
> =20
> +	if (!uuids)
> +		return;
> +
>  	ba2str(&device->bdaddr, addr);
> =20
>  	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;
> =20
> +	/* 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);
> =20