Hi Simon,
On Tue, Jul 25, 2023 at 1:56 AM Simon Mikuda
<simon.mikuda@xxxxxxxxxxxxxxxxxxx> wrote:
>
> We'll prefer to pair and discover services on connected bearer first.
>
> There was a problem with pairing, that select_conn_bearer returned BR/EDR
> even when we have connection to LE bearer only. In these situation we should
> pair over connected bearer, since connection to another bearer can fail.
>
> Similar problem with discovery that after connection on LE bearer discovery
> was requested on BR/EDR bearer which can cause connection error (e.g. Page
> Timeout).
> ---
> src/device.c | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/src/device.c b/src/device.c
> index 6f28e261e..446e978ee 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -2504,6 +2504,16 @@ static uint8_t select_conn_bearer(struct btd_device *dev)
> return dev->bdaddr_type;
> }
>
> +static uint8_t select_active_bearer(struct btd_device *dev)
> +{
> + if (dev->bredr_state.connected)
> + return BDADDR_BREDR;
> + else if (dev->le_state.connected)
> + return dev->bdaddr_type == BDADDR_BREDR
> + ? BDADDR_LE_PUBLIC : dev->bdaddr_type;
The code above assumes BR/EDR has the priority in case both are
connected, perhaps we should have a clear policy in case both are
connected.
> + return select_conn_bearer(dev);
> +}
> +
> static DBusMessage *dev_connect(DBusConnection *conn, DBusMessage *msg,
> void *user_data)
> {
> @@ -3018,7 +3028,7 @@ static DBusMessage *pair_device(DBusConnection *conn, DBusMessage *msg,
> else if (device->le_state.bonded)
> bdaddr_type = BDADDR_BREDR;
> else
> - bdaddr_type = select_conn_bearer(device);
> + bdaddr_type = select_active_bearer(device);
>
> state = get_state(device, bdaddr_type);
>
> @@ -3055,7 +3065,7 @@ static DBusMessage *pair_device(DBusConnection *conn, DBusMessage *msg,
> err = device_connect_le(device);
> else
> err = adapter_create_bonding(adapter, &device->bdaddr,
> - device->bdaddr_type,
> + bdaddr_type,
> io_cap);
> } else {
> err = adapter_create_bonding(adapter, &device->bdaddr,
> @@ -6207,12 +6217,9 @@ static bool start_discovery_cb(gpointer user_data)
> {
> struct btd_device *device = user_data;
>
> - if (device->bredr)
> - device_browse_sdp(device, NULL);
> - else
> - device_browse_gatt(device, NULL);
> -
> device->discov_timer = 0;
> + device_discover_services(device, select_active_bearer(device),
> + NULL);
Perhaps in case of discovery we could do both in parallel, although if
the remote side supports GATT services as part of SDP we may end up
with redundant discovery but bt_gatt_client/gatt_db shall be able to
handle that.
> return FALSE;
> }
> --
> 2.34.1
>
--
Luiz Augusto von Dentz
