[BlueZ,v1,0/3] Detailed error code

Hi BlueZ maintainers,

Chromium OS has been working closely with Linux Bluetooth community to
improve BlueZ stack, and there are increasing needs from applications
building their features around Bluetooth. One of the major feedback
from these application is the lack of the detailed failure reasons as
return for D-Bus method call, and these failure reasons can be used in
metrics, optimizing retry mechanism, hinting the reproduce scenario to
improve BlueZ stack. The current org.bluez.Error.* are serving the
generic errors well. However,g given org.bluez.Error.* errors are used
across different interface context which does not serve the detailed
failure reasons well. (See https://github.com/bluez/bluez/issues/131)


Miao-chen Chou (3):
  BR/EDR and LE connection failure reasons
  Include BtdError code in Connect() return
  Print error code for connect methods

 client/main.c |   3 +-
 src/device.c  |  52 +++++++++------
 src/error.c   | 124 ++++++++++++++++++++++++++++++++++++
 src/error.h   | 171 ++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 331 insertions(+), 19 deletions(-)

Hi Luiz,

On Fri, Jun 18, 2021 at 11:18 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>

> Hi Miao,

>

> On Thu, Jun 17, 2021 at 8:08 PM Miao-chen Chou <mcchou@chromium.org> wrote:

> >

> > The source of Connect() failures can be divided into the following

> > three.

> > - bluetoothd's device interface state transition and profile state

> >   transition

> > - Kernel's L2CAP layer state transition

> > - Potential HCI error codes returned by the remote device

> >

> > Reviewed-by: Alain Michaud <alainm@chromium.org>

> > Reviewed-by: Howard Chung <howardchung@google.com>

> > ---

> >

> >  src/error.c | 124 +++++++++++++++++++++++++++++++++++++

> >  src/error.h | 171 ++++++++++++++++++++++++++++++++++++++++++++++++++++

> >  2 files changed, 295 insertions(+)

> >

> > diff --git a/src/error.c b/src/error.c

> > index 89517075e..b72e3d764 100644

> > --- a/src/error.c

> > +++ b/src/error.c

> > @@ -27,6 +27,7 @@

> >  #include <config.h>

> >  #endif

> >

> > +#include <stdio.h>

> >  #include "gdbus/gdbus.h"

> >

> >  #include "error.h"

> > @@ -43,6 +44,15 @@ DBusMessage *btd_error_invalid_args_str(DBusMessage *msg, const char *str)

> >                                         "%s", str);

> >  }

> >

> > +DBusMessage *btd_error_invalid_args_err(DBusMessage *msg, uint16_t err)

> > +{

> > +       char str[16];

> > +

> > +       sprintf(str, "BtdError:0x%04X", err);

> > +       return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",

> > +                                       "%s", str);

> > +}

> > +

> >  DBusMessage *btd_error_busy(DBusMessage *msg)

> >  {

> >         return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",

> > @@ -79,12 +89,30 @@ DBusMessage *btd_error_in_progress(DBusMessage *msg)

> >                                         "In Progress");

> >  }

> >

> > +DBusMessage *btd_error_in_progress_err(DBusMessage *msg, uint16_t err)

> > +{

> > +       char str[16];

> > +

> > +       sprintf(str, "BtdError:0x%04X", err);

> > +       return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress", "%s",

> > +                                       str);

>

> Was there a problem that you didn't use g_dbus_create_error to convert

> the error? I mean instead of using %s use BtdError:0x%04X, actually it

> might be enough to just have the code portion and skip BtdError as

> that can be assumed implicitly the our error messages would contain

> error codes.

I thought about dropping BtdError from the error message, but I ended
up with BtdError to make the meaning more explicit. I am happy to drop
it if the implicity is fine.
>

> > +}

> > +

> >  DBusMessage *btd_error_not_available(DBusMessage *msg)

> >  {

> >         return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",

> >                                         "Operation currently not available");

> >  }

> >

> > +DBusMessage *btd_error_not_available_err(DBusMessage *msg, uint16_t err)

> > +{

> > +       char str[16];

> > +

> > +       sprintf(str, "BtdError:0x%04X", err);

> > +       return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable", "%s",

> > +                                       str);

> > +}

> > +

> >  DBusMessage *btd_error_does_not_exist(DBusMessage *msg)

> >  {

> >         return g_dbus_create_error(msg, ERROR_INTERFACE ".DoesNotExist",

> > @@ -121,8 +149,104 @@ DBusMessage *btd_error_not_ready(DBusMessage *msg)

> >                                         "Resource Not Ready");

> >  }

> >

> > +DBusMessage *btd_error_not_ready_err(DBusMessage *msg, uint16_t err)

> > +{

> > +       char str[16];

> > +

> > +       sprintf(str, "BtdError:0x%04X", err);

> > +       return g_dbus_create_error(msg, ERROR_INTERFACE ".NotReady", "%s", str);

> > +}

> > +

> >  DBusMessage *btd_error_failed(DBusMessage *msg, const char *str)

> >  {

> >         return g_dbus_create_error(msg, ERROR_INTERFACE

> >                                         ".Failed", "%s", str);

> >  }

> > +

> > +DBusMessage *btd_error_failed_err(DBusMessage *msg, uint16_t err)

> > +{

> > +       char str[16];

> > +

> > +       sprintf(str, "BtdError:0x%04X", err);

> > +       return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed", "%s", str);

> > +}

> > +

> > +uint16_t btd_error_bredr_conn_from_errno(int errno_code)

> > +{

> > +       switch (-errno_code) {

> > +       case EALREADY:

> > +       case EISCONN: // Fall through

> > +               return BTD_ERR_BREDR_CONN_ALREADY_CONNECTED;

> > +       case EHOSTDOWN:

> > +               return BTD_ERR_BREDR_CONN_PAGE_TIMEOUT;

> > +       case ENOPROTOOPT:

> > +               return BTD_ERR_BREDR_CONN_PROFILE_UNAVAILABLE;

> > +       case EIO:

> > +               return BTD_ERR_BREDR_CONN_CREATE_SOCKET;

> > +       case EINVAL:

> > +               return BTD_ERR_BREDR_CONN_INVALID_ARGUMENTS;

> > +       case EHOSTUNREACH:

> > +               return BTD_ERR_BREDR_CONN_ADAPTER_NOT_POWERED;

> > +       case EOPNOTSUPP:

> > +       case EPROTONOSUPPORT: // Fall through

> > +               return BTD_ERR_BREDR_CONN_NOT_SUPPORTED;

> > +       case EBADFD:

> > +               return BTD_ERR_BREDR_CONN_BAD_SOCKET;

> > +       case ENOMEM:

> > +               return BTD_ERR_BREDR_CONN_MEMORY_ALLOC;

> > +       case EBUSY:

> > +               return BTD_ERR_BREDR_CONN_BUSY;

> > +       case EMLINK:

> > +               return BTD_ERR_BREDR_CONN_SYNC_CONNECT_LIMIT;

> > +       case ETIMEDOUT:

> > +               return BTD_ERR_BREDR_CONN_TIMEOUT;

> > +       case ECONNREFUSED:

> > +               return BTD_ERR_BREDR_CONN_REFUSED;

> > +       case ECONNRESET:

> > +               return BTD_ERR_BREDR_CONN_TERM_BY_REMOTE;

> > +       case ECONNABORTED:

> > +               return BTD_ERR_BREDR_CONN_TERM_BY_LOCAL;

> > +       case EPROTO:

> > +               return BTD_ERR_BREDR_CONN_PROTO_ERROR;

> > +       default:

> > +               return BTD_ERR_BREDR_CONN_UNKNOWN;

> > +       }

> > +}

> > +

> > +uint16_t btd_error_le_conn_from_errno(int errno_code)

> > +{

> > +       switch (-errno_code) {

> > +       case EINVAL:

> > +               return BTD_ERR_LE_CONN_INVALID_ARGUMENTS;

> > +       case EHOSTUNREACH:

> > +               return BTD_ERR_LE_CONN_ADAPTER_NOT_POWERED;

> > +       case EOPNOTSUPP:

> > +       case EPROTONOSUPPORT: // Fall through

> > +               return BTD_ERR_LE_CONN_NOT_SUPPORTED;

> > +       case EALREADY:

> > +       case EISCONN: // Fall through

> > +               return BTD_ERR_LE_CONN_ALREADY_CONNECTED;

> > +       case EBADFD:

> > +               return BTD_ERR_LE_CONN_BAD_SOCKET;

> > +       case ENOMEM:

> > +               return BTD_ERR_LE_CONN_MEMORY_ALLOC;

> > +       case EBUSY:

> > +               return BTD_ERR_LE_CONN_BUSY;

> > +       case ECONNREFUSED:

> > +               return BTD_ERR_LE_CONN_REFUSED;

> > +       case EIO:

> > +               return BTD_ERR_LE_CONN_CREATE_SOCKET;

> > +       case ETIMEDOUT:

> > +               return BTD_ERR_LE_CONN_TIMEOUT;

> > +       case EMLINK:

> > +               return BTD_ERR_LE_CONN_SYNC_CONNECT_LIMIT;

> > +       case ECONNRESET:

> > +               return BTD_ERR_LE_CONN_TERM_BY_REMOTE;

> > +       case ECONNABORTED:

> > +               return BTD_ERR_LE_CONN_TERM_BY_LOCAL;

> > +       case EPROTO:

> > +               return BTD_ERR_LE_CONN_PROTO_ERROR;

> > +       default:

> > +               return BTD_ERR_LE_CONN_UNKNOWN;

> > +       }

> > +}

> > diff --git a/src/error.h b/src/error.h

> > index 7c8cad066..f258cc4e7 100644

> > --- a/src/error.h

> > +++ b/src/error.h

> > @@ -24,22 +24,193 @@

> >   */

> >

> >  #include <dbus/dbus.h>

> > +#include <stdint.h>

> >

> >  #define ERROR_INTERFACE "org.bluez.Error"

> >

> > +/* BR/EDR connection failure reasons

> > + * BT_ERR_* should be used as one of the parameters to btd_error_*_err().

> > + */

> > +

> > +/* Either the profile is already connected or ACL connection is in

> > + * place.

> > + * errno: EALREADY, EISCONN

> > + */

> > +#define BTD_ERR_BREDR_CONN_ALREADY_CONNECTED   0x0001

> > +/* Failed due to page timeout.

> > + * errno: EHOSTDOWN

> > + */

> > +#define BTD_ERR_BREDR_CONN_PAGE_TIMEOUT                0x0002

> > +/* Failed to find connectable services or the target service.

> > + * errno: ENOPROTOOPT

> > + */

> > +#define BTD_ERR_BREDR_CONN_PROFILE_UNAVAILABLE 0x0003

> > +/* Failed to complete the SDP search.

> > + * errno: none

> > + */

> > +#define BTD_ERR_BREDR_CONN_SDP_SEARCH          0x0004

> > +/* Failed to create or connect to BT IO socket. This can also indicate

> > + * hardware failure in the controller.

> > + * errno: EIO

> > + */

> > +#define BTD_ERR_BREDR_CONN_CREATE_SOCKET       0x0005

> > +/* Failed due to invalid arguments.

> > + * errno: EINVAL

> > + */

> > +#define BTD_ERR_BREDR_CONN_INVALID_ARGUMENTS   0x0006

> > +/* Failed due to adapter not powered.

> > + * errno: EHOSTUNREACH

> > + */

> > +#define BTD_ERR_BREDR_CONN_ADAPTER_NOT_POWERED 0x0007

> > +/* Failed due to unsupported state transition of L2CAP channel or other

> > + * features either by the local host or the remote.

> > + * errno: EOPNOTSUPP, EPROTONOSUPPORT

> > + */

> > +#define BTD_ERR_BREDR_CONN_NOT_SUPPORTED       0x0008

> > +/* Failed due to the socket is in bad state.

> > + * errno: EBADFD

> > + */

> > +#define BTD_ERR_BREDR_CONN_BAD_SOCKET          0x0009

> > +/* Failed to allocate memory in either host stack or controller.

> > + * errno: ENOMEM

> > + */

> > +#define BTD_ERR_BREDR_CONN_MEMORY_ALLOC                0x000A

> > +/* Failed due to other ongoing operations, such as pairing, busy L2CAP

> > + * channel or the operation disallowed by the controller.

> > + * errno: EBUSY

> > + */

> > +#define BTD_ERR_BREDR_CONN_BUSY                        0x000B

> > +/* Failed due to reaching the synchronous connection limit to a device.

> > + * errno: EMLINK

> > + */

> > +#define BTD_ERR_BREDR_CONN_SYNC_CONNECT_LIMIT  0x000C

> > +/* Failed due to connection timeout

> > + * errno: ETIMEDOUT

> > + */

> > +#define BTD_ERR_BREDR_CONN_TIMEOUT             0x000D

> > +/* Refused by the remote device due to limited resource, security reason

> > + * or unacceptable address type.

> > + * errno: ECONNREFUSED

> > + */

> > +#define BTD_ERR_BREDR_CONN_REFUSED             0x000E

> > +/* Terminated by the remote device due to limited resource or power

> > + * off.

> > + * errno: ECONNRESET

> > + */

> > +#define BTD_ERR_BREDR_CONN_TERM_BY_REMOTE      0x000F

> > +/* Terminated by the local host.

> > + * errno: ECONNABORTED

> > + */

> > +#define BTD_ERR_BREDR_CONN_TERM_BY_LOCAL       0x0010

> > +/* Failed due to LMP protocol error.

> > + * errno: EPROTO

> > + */

> > +#define BTD_ERR_BREDR_CONN_PROTO_ERROR         0x0011

> > +/* Failed due to cancellation caused by adapter drop, unexpected device drop,

> > + * or incoming disconnection request before connection request is completed.

> > + * errno: none

> > + */

> > +#define BTD_ERR_BREDR_CONN_CANCELED            0x0012

> > +/* Failed due to unknown reason.

> > + * errno: ENOSYS

> > + */

> > +#define BTD_ERR_BREDR_CONN_UNKNOWN             0x0013

> > +

> > +/* LE connection failure reasons

> > + * BT_ERR_* should be used as one of the parameters to btd_error_*_err().

> > + */

> > +

> > +/* Failed due to invalid arguments.

> > + * errno: EINVAL

> > + */

> > +#define BTD_ERR_LE_CONN_INVALID_ARGUMENTS      0x0101

> > +/* Failed due to adapter not powered.

> > + * errno: EHOSTUNREACH

> > + */

> > +#define BTD_ERR_LE_CONN_ADAPTER_NOT_POWERED    0x0102

> > +/* Failed due to unsupported state transition of L2CAP channel or other

> > + * features (e.g. LE features) either by the local host or the remote.

> > + * errno: EOPNOTSUPP, EPROTONOSUPPORT

> > + */

> > +#define BTD_ERR_LE_CONN_NOT_SUPPORTED          0x0103

> > +/* Either the BT IO is already connected or LE link connection in place.

> > + * errno: EALREADY, EISCONN

> > + */

> > +#define BTD_ERR_LE_CONN_ALREADY_CONNECTED      0x0104

> > +/* Failed due to the socket is in bad state.

> > + * errno: EBADFD

> > + */

> > +#define BTD_ERR_LE_CONN_BAD_SOCKET             0x0105

> > +/* Failed to allocate memory in either host stack or controller.

> > + * errno: ENOMEM

> > + */

> > +#define BTD_ERR_LE_CONN_MEMORY_ALLOC           0x0106

> > +/* Failed due to other ongoing operations, such as pairing, connecting, busy

> > + * L2CAP channel or the operation disallowed by the controller.

> > + * errno: EBUSY

> > + */

> > +#define BTD_ERR_LE_CONN_BUSY                   0x0107

> > +/* Failed due to that LE is not enabled or the attempt is refused by the remote

> > + * device due to limited resource, security reason or unacceptable address type.

> > + * errno: ECONNREFUSED

> > + */

> > +#define BTD_ERR_LE_CONN_REFUSED                        0x0108

> > +/* Failed to create or connect to BT IO socket. This can also indicate

> > + * hardware failure in the controller.

> > + * errno: EIO

> > + */

> > +#define BTD_ERR_LE_CONN_CREATE_SOCKET          0x0109

> > +/* Failed due to connection timeout

> > + * errno: ETIMEDOUT

> > + */

> > +#define BTD_ERR_LE_CONN_TIMEOUT                        0x010A

> > +/* Failed due to reaching the synchronous connection limit to a device.

> > + * errno: EMLINK

> > + */

> > +#define BTD_ERR_LE_CONN_SYNC_CONNECT_LIMIT     0x010B

> > +/* Terminated by the remote device due to limited resource or power

> > + * off.

> > + * errno: ECONNRESET

> > + */

> > +#define BTD_ERR_LE_CONN_TERM_BY_REMOTE         0x010C

> > +/* Terminated by the local host.

> > + * errno: ECONNABORTED

> > + */

> > +#define BTD_ERR_LE_CONN_TERM_BY_LOCAL          0x010D

> > +/* Failed due to LL protocol error.

> > + * errno: EPROTO

> > + */

> > +#define BTD_ERR_LE_CONN_PROTO_ERROR            0x010E

> > +/* Failed to complete the GATT browsing.

> > + * errno: none

> > + */

> > +#define BTD_ERR_LE_CONN_GATT_BROWSE            0x010F

> > +/* Failed due to unknown reason.

> > + * errno: ENOSYS

> > + */

> > +#define BTD_ERR_LE_CONN_UNKNOWN                        0x0110

> > +

> >  DBusMessage *btd_error_invalid_args(DBusMessage *msg);

> >  DBusMessage *btd_error_invalid_args_str(DBusMessage *msg, const char *str);

> > +DBusMessage *btd_error_invalid_args_err(DBusMessage *msg, uint16_t err);

> >  DBusMessage *btd_error_busy(DBusMessage *msg);

> >  DBusMessage *btd_error_already_exists(DBusMessage *msg);

> >  DBusMessage *btd_error_not_supported(DBusMessage *msg);

> >  DBusMessage *btd_error_not_connected(DBusMessage *msg);

> >  DBusMessage *btd_error_already_connected(DBusMessage *msg);

> >  DBusMessage *btd_error_not_available(DBusMessage *msg);

> > +DBusMessage *btd_error_not_available_err(DBusMessage *msg, uint16_t err);

> >  DBusMessage *btd_error_in_progress(DBusMessage *msg);

> > +DBusMessage *btd_error_in_progress_err(DBusMessage *msg, uint16_t err);

> >  DBusMessage *btd_error_does_not_exist(DBusMessage *msg);

> >  DBusMessage *btd_error_not_authorized(DBusMessage *msg);

> >  DBusMessage *btd_error_not_permitted(DBusMessage *msg, const char *str);

> >  DBusMessage *btd_error_no_such_adapter(DBusMessage *msg);

> >  DBusMessage *btd_error_agent_not_available(DBusMessage *msg);

> >  DBusMessage *btd_error_not_ready(DBusMessage *msg);

> > +DBusMessage *btd_error_not_ready_err(DBusMessage *msg, uint16_t err);

> >  DBusMessage *btd_error_failed(DBusMessage *msg, const char *str);

> > +DBusMessage *btd_error_failed_err(DBusMessage *msg, uint16_t err);

> > +

> > +uint16_t btd_error_bredr_conn_from_errno(int errno_code);

> > +uint16_t btd_error_le_conn_from_errno(int errno_code);

> > --

> > 2.32.0.288.g62a8d224e6-goog

> >

>

>

> --

> Luiz Augusto von Dentz

Thanks,
Miao