Hi Bastien,
On Tue, Jul 1, 2025 at 6:20 AM Bastien Nocera <hadess@xxxxxxxxxx> wrote:
>
> Output clearer debug information so that it's possible to follow the
> decisions made by the bluetoothd daemon when a client such as
> bluetoothctl or the GNOME Bluetooth settings ask it to connect to a
> device.
Well this will only output to syslog though, so the client won't
necessarily see any of this, and this actually requires the daemon to
be running with debug enabled.
> ---
> src/device.c | 41 +++++++++++++++++++++++++++++++++--------
> 1 file changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/src/device.c b/src/device.c
> index 99c0aa67ec0c..d7a859f9df3f 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -2683,6 +2683,7 @@ static DBusMessage *connect_profiles(struct btd_device *dev, uint8_t bdaddr_type
> "Connect") &&
> find_service_with_state(dev->services,
> BTD_SERVICE_STATE_CONNECTED)) {
> + DBG("Already connected to services");
> return dbus_message_new_method_return(msg);
> } else {
> return btd_error_profile_unavailable(msg);
> @@ -2694,8 +2695,10 @@ static DBusMessage *connect_profiles(struct btd_device *dev, uint8_t bdaddr_type
>
> err = connect_next(dev);
> if (err < 0) {
> - if (err == -EALREADY)
> + if (err == -EALREADY) {
> + DBG("Already connected");
> return dbus_message_new_method_return(msg);
> + }
> return btd_error_bredr_conn_from_errno(msg, err);
> }
>
> @@ -2718,14 +2721,24 @@ resolve_services:
> return NULL;
> }
>
> +static const char *bdaddr_type_strs[] = {
> + "BR/EDR",
> + "LE public",
> + "LE random"
> +};
> +
> static DBusMessage *dev_connect(DBusConnection *conn, DBusMessage *msg,
> void *user_data)
> {
> struct btd_device *dev = user_data;
> uint8_t bdaddr_type;
>
> - if (dev->bonding)
> + DBG("Calling \"Connect\" for device %s", dev->path);
We do have D-Bus message logging so this seems excessive to me.
> +
> + if (dev->bonding) {
> + DBG("Bonding in progress");
> return btd_error_in_progress(msg);
> + }
>
> if (dev->bredr_state.connected) {
> /*
> @@ -2734,23 +2747,35 @@ static DBusMessage *dev_connect(DBusConnection *conn, DBusMessage *msg,
> */
> if (dev->bredr_state.svc_resolved &&
> find_service_with_state(dev->services,
> - BTD_SERVICE_STATE_CONNECTED))
> + BTD_SERVICE_STATE_CONNECTED)) {
> bdaddr_type = dev->bdaddr_type;
> - else
> + DBG("Selecting address type %s, as BR/EDR services are resolved "
> + " and connected", bdaddr_type_strs[dev->bdaddr_type]);
> + } else {
> bdaddr_type = BDADDR_BREDR;
> - } else if (dev->le_state.connected && dev->bredr)
> + DBG("Selecting address type BR/EDR, as services not resolved "
> + "or not connected");
> + }
> + } else if (dev->le_state.connected && dev->bredr) {
> bdaddr_type = BDADDR_BREDR;
> - else
> + DBG("Selecting address type BR/EDR, as LE already connected");
> + } else {
> bdaddr_type = select_conn_bearer(dev);
> + DBG("Selecting address type %s", bdaddr_type_strs[dev->bdaddr_type]);
Don't really like this many calls for something so simple, beside we
now have the likes of PreferredBearer for the client to indicate what
bearer it wants to connect, otherwise if we have to explain every
decision we make we basically need to log the entire code, so we
better log what not why.
> + }
>
> if (bdaddr_type != BDADDR_BREDR) {
> int err;
>
> - if (dev->connect)
> + if (dev->connect) {
> + DBG("Device already connecting");
> return btd_error_in_progress(msg);
> + }
>
> - if (dev->le_state.connected)
> + if (dev->le_state.connected) {
> + DBG("Device already connected through LE");
> return dbus_message_new_method_return(msg);
> + }
>
> btd_device_set_temporary(dev, false);
>
> --
> 2.50.0
>
>
--
Luiz Augusto von Dentz
