Re: [BlueZ] device: Better "Connect" debug

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2024-10-29 at 11:36 -0400, Luiz Augusto von Dentz wrote:
> Hi Bastien,
> 
> On Tue, Oct 29, 2024 at 11:17 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.
> > ---
> >  src/device.c | 39 ++++++++++++++++++++++++++++++++-------
> >  1 file changed, 32 insertions(+), 7 deletions(-)
> > 
> > diff --git a/src/device.c b/src/device.c
> > index 7585184de50c..8756aef41aab 100644
> > --- a/src/device.c
> > +++ b/src/device.c
> > @@ -2462,10 +2462,13 @@ static DBusMessage *connect_profiles(struct
> > btd_device *dev, uint8_t bdaddr_type
> >         DBG("%s %s, client %s", dev->path, uuid ? uuid : "(all)",
> >                                                
> > dbus_message_get_sender(msg));
> > 
> > -       if (dev->pending || dev->connect || dev->browse)
> > +       if (dev->pending || dev->connect || dev->browse) {
> > +               DBG("Cannot connect, connection busy");
> >                 return btd_error_in_progress_str(msg,
> > ERR_BREDR_CONN_BUSY);
> 
> I think for this type of debug we are better off introducing directly
> into our gdbus lib so we get the information of both method calls and
> replies when debug is enabled, thoughts?

I don't really mind leaving the debug messages that are already
associated with a return value out of the patch.

What did you have in mind for modifying gdbus? Stashing debug messages
in the reply?

> 
> > +       }
> > 
> >         if (!btd_adapter_get_powered(dev->adapter)) {
> > +               DBG("Cannot connect, adapter is not powered");
> >                 return btd_error_not_ready_str(msg,
> >                                        
> > ERR_BREDR_CONN_ADAPTER_NOT_POWERED);
> >         }
> > @@ -2482,8 +2485,10 @@ 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 {
> > +                               DBG("Exhausted the list of BR/EDR
> > profiles to connect to");
> >                                 return
> > btd_error_not_available_str(msg,
> >                                        
> > ERR_BREDR_CONN_PROFILE_UNAVAILABLE);
> >                         }
> > @@ -2494,8 +2499,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_failed(msg,
> >                                        
> > btd_error_bredr_conn_from_errno(err));
> >         }
> > @@ -2568,12 +2575,20 @@ static uint8_t select_conn_bearer(struct
> > btd_device *dev)
> >         return dev->bdaddr_type;
> >  }
> > 
> > +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;
> > 
> > +       DBG("Calling \"Connect\" for device %s", dev->path);
> > +
> >         if (dev->bredr_state.connected) {
> >                 /*
> >                  * Check if services have been resolved and there
> > is at least
> > @@ -2581,20 +2596,30 @@ 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]);
> > +       }
> > 
> >         if (bdaddr_type != BDADDR_BREDR) {
> >                 int err;
> > 
> > -               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.47.0
> > 
> > 
> 
> 





[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux