[BlueZ PATCH v2 0/3] Detailed error code

[BlueZ PATCH v2 0/3] Detailed error code

[Miao-chen Chou]

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)

Changes in v2:
- Add documentation for error codes

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 +-
 doc/error-code.txt | 266 +++++++++++++++++++++++++++++++++++++++++++++
 src/device.c       |  52 ++++++---
 src/error.c        | 111 +++++++++++++++++++
 src/error.h        |  52 +++++++++
 5 files changed, 465 insertions(+), 19 deletions(-)
 create mode 100644 doc/error-code.txt

-- 
2.32.0.93.g670b81a890-goog

[BlueZ PATCH v2 1/3] error: BR/EDR and LE connection failure reasons

[Miao-chen Chou]

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

This also added error-code.txt to describe these error codes.

Reviewed-by: Alain Michaud <alainm@chromium.org>
Reviewed-by: Howard Chung <howardchung@google.com>
---

Changes in v2:
- Add error-code.txt
- Remove BtdError from return string

 doc/error-code.txt | 266 +++++++++++++++++++++++++++++++++++++++++++++
 src/error.c        | 111 +++++++++++++++++++
 src/error.h        |  52 +++++++++
 3 files changed, 429 insertions(+)
 create mode 100644 doc/error-code.txt

diff --git a/doc/error-code.txt b/doc/error-code.txt
new file mode 100644
index 000000000..e91324855
--- /dev/null
+++ b/doc/error-code.txt
@@ -0,0 +1,266 @@
+D-Bus Method Return Error Codes
+===============================
+
+The motivation of having detailed error codes is to provide context-based
+failure reasons along with D-Bus method return so that D-Bus clients can
+build metrics and optimize their application based on these failure reasons.
+For instance, a client can build retry mechanism for a connection failure or
+improve the bottleneck of use scenario based on actionable metrics.
+
+These error codes are context-based but not necessarily tied to interface or
+method calls. For instance, if a pairing request failed due to connection
+failure, connection error would be attached to the method return of Pair().
+
+BR/EDR connection already connected
+===================================
+	code:	0x0001
+	errno:	EALREADY, EISCONN
+
+Either the profile is already connected or ACL connection is in place.
+
+BR/EDR connection page timeout
+==============================
+	code:	0x0002
+	errno:	EHOSTDOWN
+
+Failed due to page timeout.
+
+BR/EDR connection profile unavailable 
+=====================================
+	code:	0x0003
+	errno:	ENOPROTOOPT
+
+Failed to find connectable services or the target service.
+
+BR/EDR connection SDP search
+============================
+	code:	0x0004
+	errno:	none
+
+Failed to complete the SDP search.
+
+BR/EDR connection create socket
+===============================
+	code:	0x0005
+	errno:	EIO
+
+Failed to create or connect to BT IO socket. This can also indicate hardware
+failure in the controller.
+
+BR/EDR connection invalid arguments
+===================================
+	code:	0x0006
+	errno:	EHOSTUNREACH
+
+Failed due to invalid arguments.
+
+BR/EDR connection not powered
+=============================
+	code:	0x0007
+	errno:	EHOSTUNREACH
+
+Failed due to adapter not powered.
+
+BR/EDR connection not supported
+===============================
+	code:	0x0008
+	errno:	EOPNOTSUPP, EPROTONOSUPPORT
+
+Failed due to unsupported state transition of L2CAP channel or other features
+either by the local host or the remote.
+
+BR/EDR connection bad socket
+============================
+	code:	0x0009
+	errno:	EBADFD
+
+Failed due to the socket is in bad state.
+
+BR/EDR connection memory allocation
+===================================
+	code:	0x000A
+	errno:	EBADFD
+
+Failed to allocate memory in either host stack or controller.
+
+BR/EDR connection busy
+======================
+	code:	0x000B
+	errno:	EBUSY
+
+Failed due to other ongoing operations, such as pairing, busy L2CAP channel or
+the operation disallowed by the controller.
+
+BR/EDR connection concurrent connection limit
+=============================================
+	code:	0x000C
+	errno:	EMLINK
+
+Failed due to reaching the concurrent connection limit to a device.
+
+BR/EDR connection timeout
+=========================
+	code:	0x000D
+	errno:	ETIMEDOUT
+
+Failed due to connection timeout
+
+BR/EDR connection refused
+=========================
+	code:	0x000E
+	errno:	ECONNREFUSED
+
+Refused by the remote device due to limited resource, security reason or
+unacceptable address type.
+
+BR/EDR connection aborted by remote
+===================================
+	code:	0x000F
+	errno:	ECONNRESET
+
+Terminated by the remote device due to limited resource or power off.
+
+BR/EDR connection aborted by local
+==================================
+	code:	0x0010
+	errno:	ECONNABORTED
+
+Aborted by the local host.
+
+BR/EDR connection protocol error
+================================
+	code:	0x0011
+	errno:	EPROTO
+
+Failed due to LMP protocol error.
+
+BR/EDR connection canceled
+==========================
+	code:	0x0012
+	errno:	none
+
+Failed due to cancellation caused by adapter drop, unexpected device drop, or
+incoming disconnection request before connection request is completed.
+
+BR/EDR connection unknown error
+===============================
+	code:	0x0013
+	errno:	ENOSYS
+
+Failed due to unknown reason.
+
+LE connection invalid arguments
+===============================
+	code:	0x0101
+	errno:	EINVAL
+
+Failed due to invalid arguments.
+
+LE connection not powered
+=========================
+	code:	0x0102
+	errno:	EHOSTUNREACH
+
+Failed due to adapter not powered.
+
+LE connection not supported
+===========================
+	code:	0x0103
+	errno:	EOPNOTSUPP, EPROTONOSUPPORT
+
+Failed due to unsupported state transition of L2CAP channel or other features
+(e.g. LE features) either by the local host or the remote.
+
+LE connection already connected
+===============================
+	code:	0x0104
+	errno: EALREADY, EISCONN
+
+Either the BT IO is already connected or LE link connection in place.
+
+LE connection bad socket
+========================
+	code:	0x0105
+	errno: EBADFD
+
+Failed due to the socket is in bad state.
+
+LE connection memory allocation
+===============================
+	code:	0x0106
+	errno: ENOMEM
+
+Failed to allocate memory in either host stack or controller.
+
+LE connection busy
+==================
+	code:	0x0107
+	errno:	EBUSY
+
+Failed due to other ongoing operations, such as pairing, connecting, busy
+L2CAP channel or the operation disallowed by the controller.
+
+LE connection refused
+=====================
+	code:	0x0108
+	errno:	ECONNREFUSED
+
+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.
+
+LE connection create socket
+===========================
+	code:	0x0109
+	errno:	EIO
+
+Failed to create or connect to BT IO socket. This can also indicate hardware
+failure in the controller.
+
+LE connection timeout
+=====================
+	code:	0x010A
+	errno:	ETIMEDOUT
+
+Failed due to connection timeout
+
+LE connection concurrent connection limit
+=========================================
+	code:	0x010B
+	errno:	EMLINK
+
+Failed due to reaching the synchronous connection limit to a device.
+
+LE connection abort by remote
+=============================
+	code:	0x010C
+	errno:	ECONNRESET
+
+Aborted by the remote device due to limited resource or power off.
+
+LE connection abort by local
+============================
+	code:	0x010D
+	errno:	ECONNABORTED
+
+Aborted by the local host.
+
+LE connection link layer protocol error 
+=======================================
+	code:	0x010E
+	errno:	EPROTO
+
+Failed due to link layer protocol error.
+
+LE connection GATT browsing
+===========================
+	code:	0x010F
+	errno:	none
+
+Failed to complete the GATT browsing.
+
+LE connection unknown error
+===========================
+	code:	0x0110
+	errno:	ENOSYS
+
+ Failed due to unknown reason.
diff --git a/src/error.c b/src/error.c
index 89517075e..73602c4bf 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,12 @@ DBusMessage *btd_error_invalid_args_str(DBusMessage *msg, const char *str)
 					"%s", str);
 }
 
+DBusMessage *btd_error_invalid_args_err(DBusMessage *msg, uint16_t err)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
+					"0x%04X", err);
+}
+
 DBusMessage *btd_error_busy(DBusMessage *msg)
 {
 	return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
@@ -79,12 +86,24 @@ DBusMessage *btd_error_in_progress(DBusMessage *msg)
 					"In Progress");
 }
 
+DBusMessage *btd_error_in_progress_err(DBusMessage *msg, uint16_t err)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress", "0x%04X",
+					err);
+}
+
 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)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
+					"0x%04X", err);
+}
+
 DBusMessage *btd_error_does_not_exist(DBusMessage *msg)
 {
 	return g_dbus_create_error(msg, ERROR_INTERFACE ".DoesNotExist",
@@ -121,8 +140,100 @@ DBusMessage *btd_error_not_ready(DBusMessage *msg)
 					"Resource Not Ready");
 }
 
+DBusMessage *btd_error_not_ready_err(DBusMessage *msg, uint16_t err)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE ".NotReady", "0x%04X",
+					err);
+}
+
 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)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed", "0x%04X",
+					err);
+}
+
+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_CNCR_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_ABORT_BY_REMOTE;
+	case ECONNABORTED:
+		return BTD_ERR_BREDR_CONN_ABORT_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_ABORT_BY_REMOTE;
+	case ECONNABORTED:
+		return BTD_ERR_LE_CONN_ABORT_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..74d433aca 100644
--- a/src/error.h
+++ b/src/error.h
@@ -24,22 +24,74 @@
  */
 
 #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().
+ */
+#define BTD_ERR_BREDR_CONN_ALREADY_CONNECTED	0x0001
+#define BTD_ERR_BREDR_CONN_PAGE_TIMEOUT		0x0002
+#define BTD_ERR_BREDR_CONN_PROFILE_UNAVAILABLE	0x0003
+#define BTD_ERR_BREDR_CONN_SDP_SEARCH		0x0004
+#define BTD_ERR_BREDR_CONN_CREATE_SOCKET	0x0005
+#define BTD_ERR_BREDR_CONN_INVALID_ARGUMENTS	0x0006
+#define BTD_ERR_BREDR_CONN_ADAPTER_NOT_POWERED	0x0007
+#define BTD_ERR_BREDR_CONN_NOT_SUPPORTED	0x0008
+#define BTD_ERR_BREDR_CONN_BAD_SOCKET		0x0009
+#define BTD_ERR_BREDR_CONN_MEMORY_ALLOC		0x000A
+#define BTD_ERR_BREDR_CONN_BUSY			0x000B
+#define BTD_ERR_BREDR_CONN_CNCR_CONNECT_LIMIT	0x000C
+#define BTD_ERR_BREDR_CONN_TIMEOUT		0x000D
+#define BTD_ERR_BREDR_CONN_REFUSED		0x000E
+#define BTD_ERR_BREDR_CONN_ABORT_BY_REMOTE	0x000F
+#define BTD_ERR_BREDR_CONN_ABORT_BY_LOCAL	0x0010
+#define BTD_ERR_BREDR_CONN_PROTO_ERROR		0x0011
+#define BTD_ERR_BREDR_CONN_CANCELED		0x0012
+#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().
+ */
+#define BTD_ERR_LE_CONN_INVALID_ARGUMENTS	0x0101
+#define BTD_ERR_LE_CONN_ADAPTER_NOT_POWERED	0x0102
+#define BTD_ERR_LE_CONN_NOT_SUPPORTED		0x0103
+#define BTD_ERR_LE_CONN_ALREADY_CONNECTED	0x0104
+#define BTD_ERR_LE_CONN_BAD_SOCKET		0x0105
+#define BTD_ERR_LE_CONN_MEMORY_ALLOC		0x0106
+#define BTD_ERR_LE_CONN_BUSY			0x0107
+#define BTD_ERR_LE_CONN_REFUSED			0x0108
+#define BTD_ERR_LE_CONN_CREATE_SOCKET		0x0109
+#define BTD_ERR_LE_CONN_TIMEOUT			0x010A
+#define BTD_ERR_LE_CONN_SYNC_CONNECT_LIMIT	0x010B
+#define BTD_ERR_LE_CONN_ABORT_BY_REMOTE		0x010C
+#define BTD_ERR_LE_CONN_ABORT_BY_LOCAL		0x010D
+#define BTD_ERR_LE_CONN_PROTO_ERROR		0x010E
+#define BTD_ERR_LE_CONN_GATT_BROWSE		0x010F
+#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.93.g670b81a890-goog

Re: [BlueZ PATCH v2 1/3] error: BR/EDR and LE connection failure reasons

[Marcel Holtmann]

Hi Miao-chen,

> 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
> 
> This also added error-code.txt to describe these error codes.
> 
> Reviewed-by: Alain Michaud <alainm@chromium.org>
> Reviewed-by: Howard Chung <howardchung@google.com>
> ---
> 
> Changes in v2:
> - Add error-code.txt
> - Remove BtdError from return string
> 
> doc/error-code.txt | 266 +++++++++++++++++++++++++++++++++++++++++++++
> src/error.c        | 111 +++++++++++++++++++
> src/error.h        |  52 +++++++++
> 3 files changed, 429 insertions(+)
> create mode 100644 doc/error-code.txt

please split documentation and code changes into separate patches.


> 
> diff --git a/doc/error-code.txt b/doc/error-code.txt
> new file mode 100644
> index 000000000..e91324855
> --- /dev/null
> +++ b/doc/error-code.txt
> @@ -0,0 +1,266 @@
> +D-Bus Method Return Error Codes
> +===============================
> +
> +The motivation of having detailed error codes is to provide context-based
> +failure reasons along with D-Bus method return so that D-Bus clients can
> +build metrics and optimize their application based on these failure reasons.
> +For instance, a client can build retry mechanism for a connection failure or
> +improve the bottleneck of use scenario based on actionable metrics.
> +
> +These error codes are context-based but not necessarily tied to interface or
> +method calls. For instance, if a pairing request failed due to connection
> +failure, connection error would be attached to the method return of Pair().
> +
> +BR/EDR connection already connected
> +===================================
> +	code:	0x0001
> +	errno:	EALREADY, EISCONN

I would rather see connnection-already-connected instead of 0x0001 in the D-Bus error message.

> +
> +Either the profile is already connected or ACL connection is in place.
> +
> +BR/EDR connection page timeout
> +==============================
> +	code:	0x0002
> +	errno:	EHOSTDOWN
> +
> +Failed due to page timeout.
> +
> +BR/EDR connection profile unavailable 
> +=====================================
> +	code:	0x0003
> +	errno:	ENOPROTOOPT
> +
> +Failed to find connectable services or the target service.
> +
> +BR/EDR connection SDP search
> +============================
> +	code:	0x0004
> +	errno:	none
> +
> +Failed to complete the SDP search.
> +
> +BR/EDR connection create socket
> +===============================
> +	code:	0x0005
> +	errno:	EIO
> +
> +Failed to create or connect to BT IO socket. This can also indicate hardware
> +failure in the controller.
> +
> +BR/EDR connection invalid arguments
> +===================================
> +	code:	0x0006
> +	errno:	EHOSTUNREACH
> +
> +Failed due to invalid arguments.
> +
> +BR/EDR connection not powered
> +=============================
> +	code:	0x0007
> +	errno:	EHOSTUNREACH
> +
> +Failed due to adapter not powered.
> +
> +BR/EDR connection not supported
> +===============================
> +	code:	0x0008
> +	errno:	EOPNOTSUPP, EPROTONOSUPPORT
> +
> +Failed due to unsupported state transition of L2CAP channel or other features
> +either by the local host or the remote.
> +
> +BR/EDR connection bad socket
> +============================
> +	code:	0x0009
> +	errno:	EBADFD
> +
> +Failed due to the socket is in bad state.
> +
> +BR/EDR connection memory allocation
> +===================================
> +	code:	0x000A
> +	errno:	EBADFD
> +
> +Failed to allocate memory in either host stack or controller.

If this happens, then the code is wrong. Should be an ENOMEM.

> +
> +BR/EDR connection busy
> +======================
> +	code:	0x000B
> +	errno:	EBUSY
> +
> +Failed due to other ongoing operations, such as pairing, busy L2CAP channel or
> +the operation disallowed by the controller.
> +
> +BR/EDR connection concurrent connection limit
> +=============================================
> +	code:	0x000C
> +	errno:	EMLINK
> +
> +Failed due to reaching the concurrent connection limit to a device.
> +
> +BR/EDR connection timeout
> +=========================
> +	code:	0x000D
> +	errno:	ETIMEDOUT
> +
> +Failed due to connection timeout
> +
> +BR/EDR connection refused
> +=========================
> +	code:	0x000E
> +	errno:	ECONNREFUSED
> +
> +Refused by the remote device due to limited resource, security reason or
> +unacceptable address type.
> +
> +BR/EDR connection aborted by remote
> +===================================
> +	code:	0x000F
> +	errno:	ECONNRESET
> +
> +Terminated by the remote device due to limited resource or power off.
> +
> +BR/EDR connection aborted by local
> +==================================
> +	code:	0x0010
> +	errno:	ECONNABORTED
> +
> +Aborted by the local host.
> +
> +BR/EDR connection protocol error
> +================================
> +	code:	0x0011
> +	errno:	EPROTO
> +
> +Failed due to LMP protocol error.
> +
> +BR/EDR connection canceled
> +==========================
> +	code:	0x0012
> +	errno:	none
> +
> +Failed due to cancellation caused by adapter drop, unexpected device drop, or
> +incoming disconnection request before connection request is completed.
> +
> +BR/EDR connection unknown error
> +===============================
> +	code:	0x0013
> +	errno:	ENOSYS
> +
> +Failed due to unknown reason.
> +
> +LE connection invalid arguments
> +===============================
> +	code:	0x0101
> +	errno:	EINVAL
> +
> +Failed due to invalid arguments.
> +
> +LE connection not powered
> +=========================
> +	code:	0x0102
> +	errno:	EHOSTUNREACH
> +
> +Failed due to adapter not powered.
> +
> +LE connection not supported
> +===========================
> +	code:	0x0103
> +	errno:	EOPNOTSUPP, EPROTONOSUPPORT
> +
> +Failed due to unsupported state transition of L2CAP channel or other features
> +(e.g. LE features) either by the local host or the remote.
> +
> +LE connection already connected
> +===============================
> +	code:	0x0104
> +	errno: EALREADY, EISCONN
> +
> +Either the BT IO is already connected or LE link connection in place.
> +
> +LE connection bad socket
> +========================
> +	code:	0x0105
> +	errno: EBADFD
> +
> +Failed due to the socket is in bad state.
> +
> +LE connection memory allocation
> +===============================
> +	code:	0x0106
> +	errno: ENOMEM
> +
> +Failed to allocate memory in either host stack or controller.
> +
> +LE connection busy
> +==================
> +	code:	0x0107
> +	errno:	EBUSY
> +
> +Failed due to other ongoing operations, such as pairing, connecting, busy
> +L2CAP channel or the operation disallowed by the controller.
> +
> +LE connection refused
> +=====================
> +	code:	0x0108
> +	errno:	ECONNREFUSED
> +
> +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.
> +
> +LE connection create socket
> +===========================
> +	code:	0x0109
> +	errno:	EIO
> +
> +Failed to create or connect to BT IO socket. This can also indicate hardware
> +failure in the controller.
> +
> +LE connection timeout
> +=====================
> +	code:	0x010A
> +	errno:	ETIMEDOUT
> +
> +Failed due to connection timeout
> +
> +LE connection concurrent connection limit
> +=========================================
> +	code:	0x010B
> +	errno:	EMLINK
> +
> +Failed due to reaching the synchronous connection limit to a device.
> +
> +LE connection abort by remote
> +=============================
> +	code:	0x010C
> +	errno:	ECONNRESET
> +
> +Aborted by the remote device due to limited resource or power off.
> +
> +LE connection abort by local
> +============================
> +	code:	0x010D
> +	errno:	ECONNABORTED
> +
> +Aborted by the local host.
> +
> +LE connection link layer protocol error 
> +=======================================
> +	code:	0x010E
> +	errno:	EPROTO
> +
> +Failed due to link layer protocol error.
> +
> +LE connection GATT browsing
> +===========================
> +	code:	0x010F
> +	errno:	none
> +
> +Failed to complete the GATT browsing.
> +
> +LE connection unknown error
> +===========================
> +	code:	0x0110
> +	errno:	ENOSYS
> +
> + Failed due to unknown reason.
> diff --git a/src/error.c b/src/error.c
> index 89517075e..73602c4bf 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,12 @@ DBusMessage *btd_error_invalid_args_str(DBusMessage *msg, const char *str)
> 					"%s", str);
> }
> 
> +DBusMessage *btd_error_invalid_args_err(DBusMessage *msg, uint16_t err)
> +{
> +	return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
> +					"0x%04X", err);
> +}
> +
> DBusMessage *btd_error_busy(DBusMessage *msg)
> {
> 	return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
> @@ -79,12 +86,24 @@ DBusMessage *btd_error_in_progress(DBusMessage *msg)
> 					"In Progress");
> }
> 
> +DBusMessage *btd_error_in_progress_err(DBusMessage *msg, uint16_t err)
> +{
> +	return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress", "0x%04X",
> +					err);
> +}
> +
> 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)
> +{
> +	return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
> +					"0x%04X", err);
> +}
> +
> DBusMessage *btd_error_does_not_exist(DBusMessage *msg)
> {
> 	return g_dbus_create_error(msg, ERROR_INTERFACE ".DoesNotExist",
> @@ -121,8 +140,100 @@ DBusMessage *btd_error_not_ready(DBusMessage *msg)
> 					"Resource Not Ready");
> }
> 
> +DBusMessage *btd_error_not_ready_err(DBusMessage *msg, uint16_t err)
> +{
> +	return g_dbus_create_error(msg, ERROR_INTERFACE ".NotReady", "0x%04X",
> +					err);
> +}
> +
> 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)
> +{
> +	return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed", "0x%04X",
> +					err);
> +}
> +
> +uint16_t btd_error_bredr_conn_from_errno(int errno_code)
> +{
> +	switch (-errno_code) {
> +	case EALREADY:
> +	case EISCONN: // Fall through

Don’t do this Fall through. It is actually not a fall through per se. This is just a statement with two case labels. That is perfectly normal and no compiler should complain. And frankly no C-programmer should be confused if this was intentional or not.

> +		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_CNCR_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_ABORT_BY_REMOTE;
> +	case ECONNABORTED:
> +		return BTD_ERR_BREDR_CONN_ABORT_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_ABORT_BY_REMOTE;
> +	case ECONNABORTED:
> +		return BTD_ERR_LE_CONN_ABORT_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..74d433aca 100644
> --- a/src/error.h
> +++ b/src/error.h
> @@ -24,22 +24,74 @@
>  */
> 
> #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().
> + */
> +#define BTD_ERR_BREDR_CONN_ALREADY_CONNECTED	0x0001
> +#define BTD_ERR_BREDR_CONN_PAGE_TIMEOUT		0x0002
> +#define BTD_ERR_BREDR_CONN_PROFILE_UNAVAILABLE	0x0003
> +#define BTD_ERR_BREDR_CONN_SDP_SEARCH		0x0004
> +#define BTD_ERR_BREDR_CONN_CREATE_SOCKET	0x0005
> +#define BTD_ERR_BREDR_CONN_INVALID_ARGUMENTS	0x0006
> +#define BTD_ERR_BREDR_CONN_ADAPTER_NOT_POWERED	0x0007
> +#define BTD_ERR_BREDR_CONN_NOT_SUPPORTED	0x0008
> +#define BTD_ERR_BREDR_CONN_BAD_SOCKET		0x0009
> +#define BTD_ERR_BREDR_CONN_MEMORY_ALLOC		0x000A
> +#define BTD_ERR_BREDR_CONN_BUSY			0x000B
> +#define BTD_ERR_BREDR_CONN_CNCR_CONNECT_LIMIT	0x000C
> +#define BTD_ERR_BREDR_CONN_TIMEOUT		0x000D
> +#define BTD_ERR_BREDR_CONN_REFUSED		0x000E
> +#define BTD_ERR_BREDR_CONN_ABORT_BY_REMOTE	0x000F
> +#define BTD_ERR_BREDR_CONN_ABORT_BY_LOCAL	0x0010
> +#define BTD_ERR_BREDR_CONN_PROTO_ERROR		0x0011
> +#define BTD_ERR_BREDR_CONN_CANCELED		0x0012
> +#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().
> + */
> +#define BTD_ERR_LE_CONN_INVALID_ARGUMENTS	0x0101
> +#define BTD_ERR_LE_CONN_ADAPTER_NOT_POWERED	0x0102
> +#define BTD_ERR_LE_CONN_NOT_SUPPORTED		0x0103
> +#define BTD_ERR_LE_CONN_ALREADY_CONNECTED	0x0104
> +#define BTD_ERR_LE_CONN_BAD_SOCKET		0x0105
> +#define BTD_ERR_LE_CONN_MEMORY_ALLOC		0x0106
> +#define BTD_ERR_LE_CONN_BUSY			0x0107
> +#define BTD_ERR_LE_CONN_REFUSED			0x0108
> +#define BTD_ERR_LE_CONN_CREATE_SOCKET		0x0109
> +#define BTD_ERR_LE_CONN_TIMEOUT			0x010A
> +#define BTD_ERR_LE_CONN_SYNC_CONNECT_LIMIT	0x010B
> +#define BTD_ERR_LE_CONN_ABORT_BY_REMOTE		0x010C
> +#define BTD_ERR_LE_CONN_ABORT_BY_LOCAL		0x010D
> +#define BTD_ERR_LE_CONN_PROTO_ERROR		0x010E
> +#define BTD_ERR_LE_CONN_GATT_BROWSE		0x010F
> +#define BTD_ERR_LE_CONN_UNKNOWN			0x0110
> +

What is the intention to split BR/EDR and LE here. You do know up-front what connection type you are. Trying to figure out from the error code what connection you have been trying to establish is plain wrong.

The description is that you want to know exactly where the connection failed. And I think that can be established independent from the transport.

In addition, I don’t like the 0x00?? vs 0x01?? reservation of any number. That always goes bad at some point in the future.

Regards

Marcel

Re: [BlueZ PATCH v2 1/3] error: BR/EDR and LE connection failure reasons

[Miao-chen Chou]

On Fri, Jun 25, 2021 at 10:39 PM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Miao-chen,
>
> > 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
> >
> > This also added error-code.txt to describe these error codes.
> >
> > Reviewed-by: Alain Michaud <alainm@chromium.org>
> > Reviewed-by: Howard Chung <howardchung@google.com>
> > ---
> >
> > Changes in v2:
> > - Add error-code.txt
> > - Remove BtdError from return string
> >
> > doc/error-code.txt | 266 +++++++++++++++++++++++++++++++++++++++++++++
> > src/error.c        | 111 +++++++++++++++++++
> > src/error.h        |  52 +++++++++
> > 3 files changed, 429 insertions(+)
> > create mode 100644 doc/error-code.txt
>
> please split documentation and code changes into separate patches.
>
>
> >
> > diff --git a/doc/error-code.txt b/doc/error-code.txt
> > new file mode 100644
> > index 000000000..e91324855
> > --- /dev/null
> > +++ b/doc/error-code.txt
> > @@ -0,0 +1,266 @@
> > +D-Bus Method Return Error Codes
> > +===============================
> > +
> > +The motivation of having detailed error codes is to provide context-based
> > +failure reasons along with D-Bus method return so that D-Bus clients can
> > +build metrics and optimize their application based on these failure reasons.
> > +For instance, a client can build retry mechanism for a connection failure or
> > +improve the bottleneck of use scenario based on actionable metrics.
> > +
> > +These error codes are context-based but not necessarily tied to interface or
> > +method calls. For instance, if a pairing request failed due to connection
> > +failure, connection error would be attached to the method return of Pair().
> > +
> > +BR/EDR connection already connected
> > +===================================
> > +     code:   0x0001
> > +     errno:  EALREADY, EISCONN
>
> I would rather see connnection-already-connected instead of 0x0001 in the D-Bus error message.
Having a code attached instead of a string description makes it easier
for a D-Bus client to interpret and map to corresponding handlers IMO.
For instead, a client can simplily use the code in a switch case to
perform retry or error reporting.
>
> > +
> > +Either the profile is already connected or ACL connection is in place.
> > +
> > +BR/EDR connection page timeout
> > +==============================
> > +     code:   0x0002
> > +     errno:  EHOSTDOWN
> > +
> > +Failed due to page timeout.
> > +
> > +BR/EDR connection profile unavailable
> > +=====================================
> > +     code:   0x0003
> > +     errno:  ENOPROTOOPT
> > +
> > +Failed to find connectable services or the target service.
> > +
> > +BR/EDR connection SDP search
> > +============================
> > +     code:   0x0004
> > +     errno:  none
> > +
> > +Failed to complete the SDP search.
> > +
> > +BR/EDR connection create socket
> > +===============================
> > +     code:   0x0005
> > +     errno:  EIO
> > +
> > +Failed to create or connect to BT IO socket. This can also indicate hardware
> > +failure in the controller.
> > +
> > +BR/EDR connection invalid arguments
> > +===================================
> > +     code:   0x0006
> > +     errno:  EHOSTUNREACH
> > +
> > +Failed due to invalid arguments.
> > +
> > +BR/EDR connection not powered
> > +=============================
> > +     code:   0x0007
> > +     errno:  EHOSTUNREACH
> > +
> > +Failed due to adapter not powered.
> > +
> > +BR/EDR connection not supported
> > +===============================
> > +     code:   0x0008
> > +     errno:  EOPNOTSUPP, EPROTONOSUPPORT
> > +
> > +Failed due to unsupported state transition of L2CAP channel or other features
> > +either by the local host or the remote.
> > +
> > +BR/EDR connection bad socket
> > +============================
> > +     code:   0x0009
> > +     errno:  EBADFD
> > +
> > +Failed due to the socket is in bad state.
> > +
> > +BR/EDR connection memory allocation
> > +===================================
> > +     code:   0x000A
> > +     errno:  EBADFD
> > +
> > +Failed to allocate memory in either host stack or controller.
>
> If this happens, then the code is wrong. Should be an ENOMEM.
My mistake, this should be ENOMEM. Corrected in v3.
>
> > +
> > +BR/EDR connection busy
> > +======================
> > +     code:   0x000B
> > +     errno:  EBUSY
> > +
> > +Failed due to other ongoing operations, such as pairing, busy L2CAP channel or
> > +the operation disallowed by the controller.
> > +
> > +BR/EDR connection concurrent connection limit
> > +=============================================
> > +     code:   0x000C
> > +     errno:  EMLINK
> > +
> > +Failed due to reaching the concurrent connection limit to a device.
> > +
> > +BR/EDR connection timeout
> > +=========================
> > +     code:   0x000D
> > +     errno:  ETIMEDOUT
> > +
> > +Failed due to connection timeout
> > +
> > +BR/EDR connection refused
> > +=========================
> > +     code:   0x000E
> > +     errno:  ECONNREFUSED
> > +
> > +Refused by the remote device due to limited resource, security reason or
> > +unacceptable address type.
> > +
> > +BR/EDR connection aborted by remote
> > +===================================
> > +     code:   0x000F
> > +     errno:  ECONNRESET
> > +
> > +Terminated by the remote device due to limited resource or power off.
> > +
> > +BR/EDR connection aborted by local
> > +==================================
> > +     code:   0x0010
> > +     errno:  ECONNABORTED
> > +
> > +Aborted by the local host.
> > +
> > +BR/EDR connection protocol error
> > +================================
> > +     code:   0x0011
> > +     errno:  EPROTO
> > +
> > +Failed due to LMP protocol error.
> > +
> > +BR/EDR connection canceled
> > +==========================
> > +     code:   0x0012
> > +     errno:  none
> > +
> > +Failed due to cancellation caused by adapter drop, unexpected device drop, or
> > +incoming disconnection request before connection request is completed.
> > +
> > +BR/EDR connection unknown error
> > +===============================
> > +     code:   0x0013
> > +     errno:  ENOSYS
> > +
> > +Failed due to unknown reason.
> > +
> > +LE connection invalid arguments
> > +===============================
> > +     code:   0x0101
> > +     errno:  EINVAL
> > +
> > +Failed due to invalid arguments.
> > +
> > +LE connection not powered
> > +=========================
> > +     code:   0x0102
> > +     errno:  EHOSTUNREACH
> > +
> > +Failed due to adapter not powered.
> > +
> > +LE connection not supported
> > +===========================
> > +     code:   0x0103
> > +     errno:  EOPNOTSUPP, EPROTONOSUPPORT
> > +
> > +Failed due to unsupported state transition of L2CAP channel or other features
> > +(e.g. LE features) either by the local host or the remote.
> > +
> > +LE connection already connected
> > +===============================
> > +     code:   0x0104
> > +     errno: EALREADY, EISCONN
> > +
> > +Either the BT IO is already connected or LE link connection in place.
> > +
> > +LE connection bad socket
> > +========================
> > +     code:   0x0105
> > +     errno: EBADFD
> > +
> > +Failed due to the socket is in bad state.
> > +
> > +LE connection memory allocation
> > +===============================
> > +     code:   0x0106
> > +     errno: ENOMEM
> > +
> > +Failed to allocate memory in either host stack or controller.
> > +
> > +LE connection busy
> > +==================
> > +     code:   0x0107
> > +     errno:  EBUSY
> > +
> > +Failed due to other ongoing operations, such as pairing, connecting, busy
> > +L2CAP channel or the operation disallowed by the controller.
> > +
> > +LE connection refused
> > +=====================
> > +     code:   0x0108
> > +     errno:  ECONNREFUSED
> > +
> > +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.
> > +
> > +LE connection create socket
> > +===========================
> > +     code:   0x0109
> > +     errno:  EIO
> > +
> > +Failed to create or connect to BT IO socket. This can also indicate hardware
> > +failure in the controller.
> > +
> > +LE connection timeout
> > +=====================
> > +     code:   0x010A
> > +     errno:  ETIMEDOUT
> > +
> > +Failed due to connection timeout
> > +
> > +LE connection concurrent connection limit
> > +=========================================
> > +     code:   0x010B
> > +     errno:  EMLINK
> > +
> > +Failed due to reaching the synchronous connection limit to a device.
> > +
> > +LE connection abort by remote
> > +=============================
> > +     code:   0x010C
> > +     errno:  ECONNRESET
> > +
> > +Aborted by the remote device due to limited resource or power off.
> > +
> > +LE connection abort by local
> > +============================
> > +     code:   0x010D
> > +     errno:  ECONNABORTED
> > +
> > +Aborted by the local host.
> > +
> > +LE connection link layer protocol error
> > +=======================================
> > +     code:   0x010E
> > +     errno:  EPROTO
> > +
> > +Failed due to link layer protocol error.
> > +
> > +LE connection GATT browsing
> > +===========================
> > +     code:   0x010F
> > +     errno:  none
> > +
> > +Failed to complete the GATT browsing.
> > +
> > +LE connection unknown error
> > +===========================
> > +     code:   0x0110
> > +     errno:  ENOSYS
> > +
> > + Failed due to unknown reason.
> > diff --git a/src/error.c b/src/error.c
> > index 89517075e..73602c4bf 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,12 @@ DBusMessage *btd_error_invalid_args_str(DBusMessage *msg, const char *str)
> >                                       "%s", str);
> > }
> >
> > +DBusMessage *btd_error_invalid_args_err(DBusMessage *msg, uint16_t err)
> > +{
> > +     return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
> > +                                     "0x%04X", err);
> > +}
> > +
> > DBusMessage *btd_error_busy(DBusMessage *msg)
> > {
> >       return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
> > @@ -79,12 +86,24 @@ DBusMessage *btd_error_in_progress(DBusMessage *msg)
> >                                       "In Progress");
> > }
> >
> > +DBusMessage *btd_error_in_progress_err(DBusMessage *msg, uint16_t err)
> > +{
> > +     return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress", "0x%04X",
> > +                                     err);
> > +}
> > +
> > 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)
> > +{
> > +     return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
> > +                                     "0x%04X", err);
> > +}
> > +
> > DBusMessage *btd_error_does_not_exist(DBusMessage *msg)
> > {
> >       return g_dbus_create_error(msg, ERROR_INTERFACE ".DoesNotExist",
> > @@ -121,8 +140,100 @@ DBusMessage *btd_error_not_ready(DBusMessage *msg)
> >                                       "Resource Not Ready");
> > }
> >
> > +DBusMessage *btd_error_not_ready_err(DBusMessage *msg, uint16_t err)
> > +{
> > +     return g_dbus_create_error(msg, ERROR_INTERFACE ".NotReady", "0x%04X",
> > +                                     err);
> > +}
> > +
> > 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)
> > +{
> > +     return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed", "0x%04X",
> > +                                     err);
> > +}
> > +
> > +uint16_t btd_error_bredr_conn_from_errno(int errno_code)
> > +{
> > +     switch (-errno_code) {
> > +     case EALREADY:
> > +     case EISCONN: // Fall through
>
> Don’t do this Fall through. It is actually not a fall through per se. This is just a statement with two case labels. That is perfectly normal and no compiler should complain. And frankly no C-programmer should be confused if this was intentional or not.
Corrected in v3.
>
> > +             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_CNCR_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_ABORT_BY_REMOTE;
> > +     case ECONNABORTED:
> > +             return BTD_ERR_BREDR_CONN_ABORT_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_ABORT_BY_REMOTE;
> > +     case ECONNABORTED:
> > +             return BTD_ERR_LE_CONN_ABORT_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..74d433aca 100644
> > --- a/src/error.h
> > +++ b/src/error.h
> > @@ -24,22 +24,74 @@
> >  */
> >
> > #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().
> > + */
> > +#define BTD_ERR_BREDR_CONN_ALREADY_CONNECTED 0x0001
> > +#define BTD_ERR_BREDR_CONN_PAGE_TIMEOUT              0x0002
> > +#define BTD_ERR_BREDR_CONN_PROFILE_UNAVAILABLE       0x0003
> > +#define BTD_ERR_BREDR_CONN_SDP_SEARCH                0x0004
> > +#define BTD_ERR_BREDR_CONN_CREATE_SOCKET     0x0005
> > +#define BTD_ERR_BREDR_CONN_INVALID_ARGUMENTS 0x0006
> > +#define BTD_ERR_BREDR_CONN_ADAPTER_NOT_POWERED       0x0007
> > +#define BTD_ERR_BREDR_CONN_NOT_SUPPORTED     0x0008
> > +#define BTD_ERR_BREDR_CONN_BAD_SOCKET                0x0009
> > +#define BTD_ERR_BREDR_CONN_MEMORY_ALLOC              0x000A
> > +#define BTD_ERR_BREDR_CONN_BUSY                      0x000B
> > +#define BTD_ERR_BREDR_CONN_CNCR_CONNECT_LIMIT        0x000C
> > +#define BTD_ERR_BREDR_CONN_TIMEOUT           0x000D
> > +#define BTD_ERR_BREDR_CONN_REFUSED           0x000E
> > +#define BTD_ERR_BREDR_CONN_ABORT_BY_REMOTE   0x000F
> > +#define BTD_ERR_BREDR_CONN_ABORT_BY_LOCAL    0x0010
> > +#define BTD_ERR_BREDR_CONN_PROTO_ERROR               0x0011
> > +#define BTD_ERR_BREDR_CONN_CANCELED          0x0012
> > +#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().
> > + */
> > +#define BTD_ERR_LE_CONN_INVALID_ARGUMENTS    0x0101
> > +#define BTD_ERR_LE_CONN_ADAPTER_NOT_POWERED  0x0102
> > +#define BTD_ERR_LE_CONN_NOT_SUPPORTED                0x0103
> > +#define BTD_ERR_LE_CONN_ALREADY_CONNECTED    0x0104
> > +#define BTD_ERR_LE_CONN_BAD_SOCKET           0x0105
> > +#define BTD_ERR_LE_CONN_MEMORY_ALLOC         0x0106
> > +#define BTD_ERR_LE_CONN_BUSY                 0x0107
> > +#define BTD_ERR_LE_CONN_REFUSED                      0x0108
> > +#define BTD_ERR_LE_CONN_CREATE_SOCKET                0x0109
> > +#define BTD_ERR_LE_CONN_TIMEOUT                      0x010A
> > +#define BTD_ERR_LE_CONN_SYNC_CONNECT_LIMIT   0x010B
> > +#define BTD_ERR_LE_CONN_ABORT_BY_REMOTE              0x010C
> > +#define BTD_ERR_LE_CONN_ABORT_BY_LOCAL               0x010D
> > +#define BTD_ERR_LE_CONN_PROTO_ERROR          0x010E
> > +#define BTD_ERR_LE_CONN_GATT_BROWSE          0x010F
> > +#define BTD_ERR_LE_CONN_UNKNOWN                      0x0110
> > +
>
> What is the intention to split BR/EDR and LE here. You do know up-front what connection type you are. Trying to figure out from the error code what connection you have been trying to establish is plain wrong.
In fact the up-front connection type is not necessarily known. In the
case of dual-mode devices, such as Bose QC35, a D-Bus client can issue
Connect(), and it depends on the timing of connection request (adv
usually arrive first compared to inquiry result), it can be either
BR/EDR or LE link being established. Another aspect of this is the
metrics collection, where knowing transport can be handy. For
instance, we can associate the certain error to particular use cases
at application layer, and that can help targeting the bottleneck to
tackle.
>
> The description is that you want to know exactly where the connection failed. And I think that can be established independent from the transport.
Indeed the intention is to know where it failed exactly. However, as
mentioned above, transport information is also an important piece of
information to know.
>
> In addition, I don’t like the 0x00?? vs 0x01?? reservation of any number. That always goes bad at some point in the future.
As replied above, having a code attached instead of a string
description makes it easier for a D-Bus client to interpret and map to
corresponding handlers, but I am happy to explore other options as
well.
>
> Regards
>
> Marcel
>
Thanks,
Miao

Re: [BlueZ PATCH v2 1/3] error: BR/EDR and LE connection failure reasons

[Luiz Augusto von Dentz]

Hi Miao,

On Wed, Jul 14, 2021 at 2:13 PM Miao-chen Chou <mcchou@chromium.org> wrote:
>
> On Fri, Jun 25, 2021 at 10:39 PM Marcel Holtmann <marcel@holtmann.org> wrote:
> >
> > Hi Miao-chen,
> >
> > > 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
> > >
> > > This also added error-code.txt to describe these error codes.
> > >
> > > Reviewed-by: Alain Michaud <alainm@chromium.org>
> > > Reviewed-by: Howard Chung <howardchung@google.com>
> > > ---
> > >
> > > Changes in v2:
> > > - Add error-code.txt
> > > - Remove BtdError from return string
> > >
> > > doc/error-code.txt | 266 +++++++++++++++++++++++++++++++++++++++++++++
> > > src/error.c        | 111 +++++++++++++++++++
> > > src/error.h        |  52 +++++++++
> > > 3 files changed, 429 insertions(+)
> > > create mode 100644 doc/error-code.txt
> >
> > please split documentation and code changes into separate patches.
> >
> >
> > >
> > > diff --git a/doc/error-code.txt b/doc/error-code.txt
> > > new file mode 100644
> > > index 000000000..e91324855
> > > --- /dev/null
> > > +++ b/doc/error-code.txt
> > > @@ -0,0 +1,266 @@
> > > +D-Bus Method Return Error Codes
> > > +===============================
> > > +
> > > +The motivation of having detailed error codes is to provide context-based
> > > +failure reasons along with D-Bus method return so that D-Bus clients can
> > > +build metrics and optimize their application based on these failure reasons.
> > > +For instance, a client can build retry mechanism for a connection failure or
> > > +improve the bottleneck of use scenario based on actionable metrics.
> > > +
> > > +These error codes are context-based but not necessarily tied to interface or
> > > +method calls. For instance, if a pairing request failed due to connection
> > > +failure, connection error would be attached to the method return of Pair().
> > > +
> > > +BR/EDR connection already connected
> > > +===================================
> > > +     code:   0x0001
> > > +     errno:  EALREADY, EISCONN
> >
> > I would rather see connnection-already-connected instead of 0x0001 in the D-Bus error message.
> Having a code attached instead of a string description makes it easier
> for a D-Bus client to interpret and map to corresponding handlers IMO.
> For instead, a client can simplily use the code in a switch case to
> perform retry or error reporting.

Well you first need to decode it since the error message is still a
string, but I would have to agree that doing things like strcmp for
the error message might be more error prone than extracting the error
code since the former is more likely to change, the weird thing is
that D-Bus seems to allow passing more than just a error message
(https://dbus.freedesktop.org/doc/dbus-specification.html):

 'Errors

 Messages of type ERROR are most commonly replies to a METHOD_CALL,
but may be returned in reply to any kind of message. The message bus
for example will return an ERROR in reply to a signal emission if the
bus does not have enough memory to send the signal.

 An ERROR may have any arguments, but if the first argument is a
STRING, it must be an error message. The error message may be logged
or shown to the user in some way.'

But it looks like libdbus does expect only a string:

https://dbus.freedesktop.org/doc/api/html/group__DBusMessage.html#ga2ab896965aec97fb21293affeed36232

Well perhaps it needs to constructed manually with the use of
dbus_message_new_method_return if you want to pass parameters other
than error_name and error_message so we could include a third one e.g.
error_code.

> >
> > > +
> > > +Either the profile is already connected or ACL connection is in place.
> > > +
> > > +BR/EDR connection page timeout
> > > +==============================
> > > +     code:   0x0002
> > > +     errno:  EHOSTDOWN
> > > +
> > > +Failed due to page timeout.
> > > +
> > > +BR/EDR connection profile unavailable
> > > +=====================================
> > > +     code:   0x0003
> > > +     errno:  ENOPROTOOPT
> > > +
> > > +Failed to find connectable services or the target service.
> > > +
> > > +BR/EDR connection SDP search
> > > +============================
> > > +     code:   0x0004
> > > +     errno:  none
> > > +
> > > +Failed to complete the SDP search.
> > > +
> > > +BR/EDR connection create socket
> > > +===============================
> > > +     code:   0x0005
> > > +     errno:  EIO
> > > +
> > > +Failed to create or connect to BT IO socket. This can also indicate hardware
> > > +failure in the controller.
> > > +
> > > +BR/EDR connection invalid arguments
> > > +===================================
> > > +     code:   0x0006
> > > +     errno:  EHOSTUNREACH
> > > +
> > > +Failed due to invalid arguments.
> > > +
> > > +BR/EDR connection not powered
> > > +=============================
> > > +     code:   0x0007
> > > +     errno:  EHOSTUNREACH
> > > +
> > > +Failed due to adapter not powered.
> > > +
> > > +BR/EDR connection not supported
> > > +===============================
> > > +     code:   0x0008
> > > +     errno:  EOPNOTSUPP, EPROTONOSUPPORT
> > > +
> > > +Failed due to unsupported state transition of L2CAP channel or other features
> > > +either by the local host or the remote.
> > > +
> > > +BR/EDR connection bad socket
> > > +============================
> > > +     code:   0x0009
> > > +     errno:  EBADFD
> > > +
> > > +Failed due to the socket is in bad state.
> > > +
> > > +BR/EDR connection memory allocation
> > > +===================================
> > > +     code:   0x000A
> > > +     errno:  EBADFD
> > > +
> > > +Failed to allocate memory in either host stack or controller.
> >
> > If this happens, then the code is wrong. Should be an ENOMEM.
> My mistake, this should be ENOMEM. Corrected in v3.
> >
> > > +
> > > +BR/EDR connection busy
> > > +======================
> > > +     code:   0x000B
> > > +     errno:  EBUSY
> > > +
> > > +Failed due to other ongoing operations, such as pairing, busy L2CAP channel or
> > > +the operation disallowed by the controller.
> > > +
> > > +BR/EDR connection concurrent connection limit
> > > +=============================================
> > > +     code:   0x000C
> > > +     errno:  EMLINK
> > > +
> > > +Failed due to reaching the concurrent connection limit to a device.
> > > +
> > > +BR/EDR connection timeout
> > > +=========================
> > > +     code:   0x000D
> > > +     errno:  ETIMEDOUT
> > > +
> > > +Failed due to connection timeout
> > > +
> > > +BR/EDR connection refused
> > > +=========================
> > > +     code:   0x000E
> > > +     errno:  ECONNREFUSED
> > > +
> > > +Refused by the remote device due to limited resource, security reason or
> > > +unacceptable address type.
> > > +
> > > +BR/EDR connection aborted by remote
> > > +===================================
> > > +     code:   0x000F
> > > +     errno:  ECONNRESET
> > > +
> > > +Terminated by the remote device due to limited resource or power off.
> > > +
> > > +BR/EDR connection aborted by local
> > > +==================================
> > > +     code:   0x0010
> > > +     errno:  ECONNABORTED
> > > +
> > > +Aborted by the local host.
> > > +
> > > +BR/EDR connection protocol error
> > > +================================
> > > +     code:   0x0011
> > > +     errno:  EPROTO
> > > +
> > > +Failed due to LMP protocol error.
> > > +
> > > +BR/EDR connection canceled
> > > +==========================
> > > +     code:   0x0012
> > > +     errno:  none
> > > +
> > > +Failed due to cancellation caused by adapter drop, unexpected device drop, or
> > > +incoming disconnection request before connection request is completed.
> > > +
> > > +BR/EDR connection unknown error
> > > +===============================
> > > +     code:   0x0013
> > > +     errno:  ENOSYS
> > > +
> > > +Failed due to unknown reason.
> > > +
> > > +LE connection invalid arguments
> > > +===============================
> > > +     code:   0x0101
> > > +     errno:  EINVAL
> > > +
> > > +Failed due to invalid arguments.
> > > +
> > > +LE connection not powered
> > > +=========================
> > > +     code:   0x0102
> > > +     errno:  EHOSTUNREACH
> > > +
> > > +Failed due to adapter not powered.
> > > +
> > > +LE connection not supported
> > > +===========================
> > > +     code:   0x0103
> > > +     errno:  EOPNOTSUPP, EPROTONOSUPPORT
> > > +
> > > +Failed due to unsupported state transition of L2CAP channel or other features
> > > +(e.g. LE features) either by the local host or the remote.
> > > +
> > > +LE connection already connected
> > > +===============================
> > > +     code:   0x0104
> > > +     errno: EALREADY, EISCONN
> > > +
> > > +Either the BT IO is already connected or LE link connection in place.
> > > +
> > > +LE connection bad socket
> > > +========================
> > > +     code:   0x0105
> > > +     errno: EBADFD
> > > +
> > > +Failed due to the socket is in bad state.
> > > +
> > > +LE connection memory allocation
> > > +===============================
> > > +     code:   0x0106
> > > +     errno: ENOMEM
> > > +
> > > +Failed to allocate memory in either host stack or controller.
> > > +
> > > +LE connection busy
> > > +==================
> > > +     code:   0x0107
> > > +     errno:  EBUSY
> > > +
> > > +Failed due to other ongoing operations, such as pairing, connecting, busy
> > > +L2CAP channel or the operation disallowed by the controller.
> > > +
> > > +LE connection refused
> > > +=====================
> > > +     code:   0x0108
> > > +     errno:  ECONNREFUSED
> > > +
> > > +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.
> > > +
> > > +LE connection create socket
> > > +===========================
> > > +     code:   0x0109
> > > +     errno:  EIO
> > > +
> > > +Failed to create or connect to BT IO socket. This can also indicate hardware
> > > +failure in the controller.
> > > +
> > > +LE connection timeout
> > > +=====================
> > > +     code:   0x010A
> > > +     errno:  ETIMEDOUT
> > > +
> > > +Failed due to connection timeout
> > > +
> > > +LE connection concurrent connection limit
> > > +=========================================
> > > +     code:   0x010B
> > > +     errno:  EMLINK
> > > +
> > > +Failed due to reaching the synchronous connection limit to a device.
> > > +
> > > +LE connection abort by remote
> > > +=============================
> > > +     code:   0x010C
> > > +     errno:  ECONNRESET
> > > +
> > > +Aborted by the remote device due to limited resource or power off.
> > > +
> > > +LE connection abort by local
> > > +============================
> > > +     code:   0x010D
> > > +     errno:  ECONNABORTED
> > > +
> > > +Aborted by the local host.
> > > +
> > > +LE connection link layer protocol error
> > > +=======================================
> > > +     code:   0x010E
> > > +     errno:  EPROTO
> > > +
> > > +Failed due to link layer protocol error.
> > > +
> > > +LE connection GATT browsing
> > > +===========================
> > > +     code:   0x010F
> > > +     errno:  none
> > > +
> > > +Failed to complete the GATT browsing.
> > > +
> > > +LE connection unknown error
> > > +===========================
> > > +     code:   0x0110
> > > +     errno:  ENOSYS
> > > +
> > > + Failed due to unknown reason.
> > > diff --git a/src/error.c b/src/error.c
> > > index 89517075e..73602c4bf 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,12 @@ DBusMessage *btd_error_invalid_args_str(DBusMessage *msg, const char *str)
> > >                                       "%s", str);
> > > }
> > >
> > > +DBusMessage *btd_error_invalid_args_err(DBusMessage *msg, uint16_t err)
> > > +{
> > > +     return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
> > > +                                     "0x%04X", err);
> > > +}
> > > +
> > > DBusMessage *btd_error_busy(DBusMessage *msg)
> > > {
> > >       return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
> > > @@ -79,12 +86,24 @@ DBusMessage *btd_error_in_progress(DBusMessage *msg)
> > >                                       "In Progress");
> > > }
> > >
> > > +DBusMessage *btd_error_in_progress_err(DBusMessage *msg, uint16_t err)
> > > +{
> > > +     return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress", "0x%04X",
> > > +                                     err);
> > > +}
> > > +
> > > 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)
> > > +{
> > > +     return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
> > > +                                     "0x%04X", err);
> > > +}
> > > +
> > > DBusMessage *btd_error_does_not_exist(DBusMessage *msg)
> > > {
> > >       return g_dbus_create_error(msg, ERROR_INTERFACE ".DoesNotExist",
> > > @@ -121,8 +140,100 @@ DBusMessage *btd_error_not_ready(DBusMessage *msg)
> > >                                       "Resource Not Ready");
> > > }
> > >
> > > +DBusMessage *btd_error_not_ready_err(DBusMessage *msg, uint16_t err)
> > > +{
> > > +     return g_dbus_create_error(msg, ERROR_INTERFACE ".NotReady", "0x%04X",
> > > +                                     err);
> > > +}
> > > +
> > > 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)
> > > +{
> > > +     return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed", "0x%04X",
> > > +                                     err);
> > > +}
> > > +
> > > +uint16_t btd_error_bredr_conn_from_errno(int errno_code)
> > > +{
> > > +     switch (-errno_code) {
> > > +     case EALREADY:
> > > +     case EISCONN: // Fall through
> >
> > Don’t do this Fall through. It is actually not a fall through per se. This is just a statement with two case labels. That is perfectly normal and no compiler should complain. And frankly no C-programmer should be confused if this was intentional or not.
> Corrected in v3.
> >
> > > +             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_CNCR_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_ABORT_BY_REMOTE;
> > > +     case ECONNABORTED:
> > > +             return BTD_ERR_BREDR_CONN_ABORT_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_ABORT_BY_REMOTE;
> > > +     case ECONNABORTED:
> > > +             return BTD_ERR_LE_CONN_ABORT_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..74d433aca 100644
> > > --- a/src/error.h
> > > +++ b/src/error.h
> > > @@ -24,22 +24,74 @@
> > >  */
> > >
> > > #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().
> > > + */
> > > +#define BTD_ERR_BREDR_CONN_ALREADY_CONNECTED 0x0001
> > > +#define BTD_ERR_BREDR_CONN_PAGE_TIMEOUT              0x0002
> > > +#define BTD_ERR_BREDR_CONN_PROFILE_UNAVAILABLE       0x0003
> > > +#define BTD_ERR_BREDR_CONN_SDP_SEARCH                0x0004
> > > +#define BTD_ERR_BREDR_CONN_CREATE_SOCKET     0x0005
> > > +#define BTD_ERR_BREDR_CONN_INVALID_ARGUMENTS 0x0006
> > > +#define BTD_ERR_BREDR_CONN_ADAPTER_NOT_POWERED       0x0007
> > > +#define BTD_ERR_BREDR_CONN_NOT_SUPPORTED     0x0008
> > > +#define BTD_ERR_BREDR_CONN_BAD_SOCKET                0x0009
> > > +#define BTD_ERR_BREDR_CONN_MEMORY_ALLOC              0x000A
> > > +#define BTD_ERR_BREDR_CONN_BUSY                      0x000B
> > > +#define BTD_ERR_BREDR_CONN_CNCR_CONNECT_LIMIT        0x000C
> > > +#define BTD_ERR_BREDR_CONN_TIMEOUT           0x000D
> > > +#define BTD_ERR_BREDR_CONN_REFUSED           0x000E
> > > +#define BTD_ERR_BREDR_CONN_ABORT_BY_REMOTE   0x000F
> > > +#define BTD_ERR_BREDR_CONN_ABORT_BY_LOCAL    0x0010
> > > +#define BTD_ERR_BREDR_CONN_PROTO_ERROR               0x0011
> > > +#define BTD_ERR_BREDR_CONN_CANCELED          0x0012
> > > +#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().
> > > + */
> > > +#define BTD_ERR_LE_CONN_INVALID_ARGUMENTS    0x0101
> > > +#define BTD_ERR_LE_CONN_ADAPTER_NOT_POWERED  0x0102
> > > +#define BTD_ERR_LE_CONN_NOT_SUPPORTED                0x0103
> > > +#define BTD_ERR_LE_CONN_ALREADY_CONNECTED    0x0104
> > > +#define BTD_ERR_LE_CONN_BAD_SOCKET           0x0105
> > > +#define BTD_ERR_LE_CONN_MEMORY_ALLOC         0x0106
> > > +#define BTD_ERR_LE_CONN_BUSY                 0x0107
> > > +#define BTD_ERR_LE_CONN_REFUSED                      0x0108
> > > +#define BTD_ERR_LE_CONN_CREATE_SOCKET                0x0109
> > > +#define BTD_ERR_LE_CONN_TIMEOUT                      0x010A
> > > +#define BTD_ERR_LE_CONN_SYNC_CONNECT_LIMIT   0x010B
> > > +#define BTD_ERR_LE_CONN_ABORT_BY_REMOTE              0x010C
> > > +#define BTD_ERR_LE_CONN_ABORT_BY_LOCAL               0x010D
> > > +#define BTD_ERR_LE_CONN_PROTO_ERROR          0x010E
> > > +#define BTD_ERR_LE_CONN_GATT_BROWSE          0x010F
> > > +#define BTD_ERR_LE_CONN_UNKNOWN                      0x0110
> > > +
> >
> > What is the intention to split BR/EDR and LE here. You do know up-front what connection type you are. Trying to figure out from the error code what connection you have been trying to establish is plain wrong.
> In fact the up-front connection type is not necessarily known. In the
> case of dual-mode devices, such as Bose QC35, a D-Bus client can issue
> Connect(), and it depends on the timing of connection request (adv
> usually arrive first compared to inquiry result), it can be either
> BR/EDR or LE link being established. Another aspect of this is the
> metrics collection, where knowing transport can be handy. For
> instance, we can associate the certain error to particular use cases
> at application layer, and that can help targeting the bottleneck to
> tackle.
> >
> > The description is that you want to know exactly where the connection failed. And I think that can be established independent from the transport.
> Indeed the intention is to know where it failed exactly. However, as
> mentioned above, transport information is also an important piece of
> information to know.
> >
> > In addition, I don’t like the 0x00?? vs 0x01?? reservation of any number. That always goes bad at some point in the future.
> As replied above, having a code attached instead of a string
> description makes it easier for a D-Bus client to interpret and map to
> corresponding handlers, but I am happy to explore other options as
> well.
> >
> > Regards
> >
> > Marcel
> >
> Thanks,
> Miao



-- 
Luiz Augusto von Dentz

Re: [BlueZ PATCH v2 1/3] error: BR/EDR and LE connection failure reasons

[Miao-chen Chou]

Hi Luiz,

On Wed, Jul 14, 2021 at 3:06 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Miao,
>
> On Wed, Jul 14, 2021 at 2:13 PM Miao-chen Chou <mcchou@chromium.org> wrote:
> >
> > On Fri, Jun 25, 2021 at 10:39 PM Marcel Holtmann <marcel@holtmann.org> wrote:
> > >
> > > Hi Miao-chen,
> > >
> > > > 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
> > > >
> > > > This also added error-code.txt to describe these error codes.
> > > >
> > > > Reviewed-by: Alain Michaud <alainm@chromium.org>
> > > > Reviewed-by: Howard Chung <howardchung@google.com>
> > > > ---
> > > >
> > > > Changes in v2:
> > > > - Add error-code.txt
> > > > - Remove BtdError from return string
> > > >
> > > > doc/error-code.txt | 266 +++++++++++++++++++++++++++++++++++++++++++++
> > > > src/error.c        | 111 +++++++++++++++++++
> > > > src/error.h        |  52 +++++++++
> > > > 3 files changed, 429 insertions(+)
> > > > create mode 100644 doc/error-code.txt
> > >
> > > please split documentation and code changes into separate patches.
> > >
> > >
> > > >
> > > > diff --git a/doc/error-code.txt b/doc/error-code.txt
> > > > new file mode 100644
> > > > index 000000000..e91324855
> > > > --- /dev/null
> > > > +++ b/doc/error-code.txt
> > > > @@ -0,0 +1,266 @@
> > > > +D-Bus Method Return Error Codes
> > > > +===============================
> > > > +
> > > > +The motivation of having detailed error codes is to provide context-based
> > > > +failure reasons along with D-Bus method return so that D-Bus clients can
> > > > +build metrics and optimize their application based on these failure reasons.
> > > > +For instance, a client can build retry mechanism for a connection failure or
> > > > +improve the bottleneck of use scenario based on actionable metrics.
> > > > +
> > > > +These error codes are context-based but not necessarily tied to interface or
> > > > +method calls. For instance, if a pairing request failed due to connection
> > > > +failure, connection error would be attached to the method return of Pair().
> > > > +
> > > > +BR/EDR connection already connected
> > > > +===================================
> > > > +     code:   0x0001
> > > > +     errno:  EALREADY, EISCONN
> > >
> > > I would rather see connnection-already-connected instead of 0x0001 in the D-Bus error message.
> > Having a code attached instead of a string description makes it easier
> > for a D-Bus client to interpret and map to corresponding handlers IMO.
> > For instead, a client can simplily use the code in a switch case to
> > perform retry or error reporting.
>
> Well you first need to decode it since the error message is still a
> string, but I would have to agree that doing things like strcmp for
> the error message might be more error prone than extracting the error
> code since the former is more likely to change, the weird thing is
> that D-Bus seems to allow passing more than just a error message
> (https://dbus.freedesktop.org/doc/dbus-specification.html):
>
>  'Errors
>
>  Messages of type ERROR are most commonly replies to a METHOD_CALL,
> but may be returned in reply to any kind of message. The message bus
> for example will return an ERROR in reply to a signal emission if the
> bus does not have enough memory to send the signal.
>
>  An ERROR may have any arguments, but if the first argument is a
> STRING, it must be an error message. The error message may be logged
> or shown to the user in some way.'
>
> But it looks like libdbus does expect only a string:
>
> https://dbus.freedesktop.org/doc/api/html/group__DBusMessage.html#ga2ab896965aec97fb21293affeed36232
>
> Well perhaps it needs to constructed manually with the use of
> dbus_message_new_method_return if you want to pass parameters other
> than error_name and error_message so we could include a third one e.g.
> error_code.
It's good to learn that D-Bus allows other types to be attached in
error return. My only concern around introducing new types to the
error return is that it changes how the D-Bus client interprets the
error return. What do you think?
>
> > >
> > > > +
> > > > +Either the profile is already connected or ACL connection is in place.
> > > > +
> > > > +BR/EDR connection page timeout
> > > > +==============================
> > > > +     code:   0x0002
> > > > +     errno:  EHOSTDOWN
> > > > +
> > > > +Failed due to page timeout.
> > > > +
> > > > +BR/EDR connection profile unavailable
> > > > +=====================================
> > > > +     code:   0x0003
> > > > +     errno:  ENOPROTOOPT
> > > > +
> > > > +Failed to find connectable services or the target service.
> > > > +
> > > > +BR/EDR connection SDP search
> > > > +============================
> > > > +     code:   0x0004
> > > > +     errno:  none
> > > > +
> > > > +Failed to complete the SDP search.
> > > > +
> > > > +BR/EDR connection create socket
> > > > +===============================
> > > > +     code:   0x0005
> > > > +     errno:  EIO
> > > > +
> > > > +Failed to create or connect to BT IO socket. This can also indicate hardware
> > > > +failure in the controller.
> > > > +
> > > > +BR/EDR connection invalid arguments
> > > > +===================================
> > > > +     code:   0x0006
> > > > +     errno:  EHOSTUNREACH
> > > > +
> > > > +Failed due to invalid arguments.
> > > > +
> > > > +BR/EDR connection not powered
> > > > +=============================
> > > > +     code:   0x0007
> > > > +     errno:  EHOSTUNREACH
> > > > +
> > > > +Failed due to adapter not powered.
> > > > +
> > > > +BR/EDR connection not supported
> > > > +===============================
> > > > +     code:   0x0008
> > > > +     errno:  EOPNOTSUPP, EPROTONOSUPPORT
> > > > +
> > > > +Failed due to unsupported state transition of L2CAP channel or other features
> > > > +either by the local host or the remote.
> > > > +
> > > > +BR/EDR connection bad socket
> > > > +============================
> > > > +     code:   0x0009
> > > > +     errno:  EBADFD
> > > > +
> > > > +Failed due to the socket is in bad state.
> > > > +
> > > > +BR/EDR connection memory allocation
> > > > +===================================
> > > > +     code:   0x000A
> > > > +     errno:  EBADFD
> > > > +
> > > > +Failed to allocate memory in either host stack or controller.
> > >
> > > If this happens, then the code is wrong. Should be an ENOMEM.
> > My mistake, this should be ENOMEM. Corrected in v3.
> > >
> > > > +
> > > > +BR/EDR connection busy
> > > > +======================
> > > > +     code:   0x000B
> > > > +     errno:  EBUSY
> > > > +
> > > > +Failed due to other ongoing operations, such as pairing, busy L2CAP channel or
> > > > +the operation disallowed by the controller.
> > > > +
> > > > +BR/EDR connection concurrent connection limit
> > > > +=============================================
> > > > +     code:   0x000C
> > > > +     errno:  EMLINK
> > > > +
> > > > +Failed due to reaching the concurrent connection limit to a device.
> > > > +
> > > > +BR/EDR connection timeout
> > > > +=========================
> > > > +     code:   0x000D
> > > > +     errno:  ETIMEDOUT
> > > > +
> > > > +Failed due to connection timeout
> > > > +
> > > > +BR/EDR connection refused
> > > > +=========================
> > > > +     code:   0x000E
> > > > +     errno:  ECONNREFUSED
> > > > +
> > > > +Refused by the remote device due to limited resource, security reason or
> > > > +unacceptable address type.
> > > > +
> > > > +BR/EDR connection aborted by remote
> > > > +===================================
> > > > +     code:   0x000F
> > > > +     errno:  ECONNRESET
> > > > +
> > > > +Terminated by the remote device due to limited resource or power off.
> > > > +
> > > > +BR/EDR connection aborted by local
> > > > +==================================
> > > > +     code:   0x0010
> > > > +     errno:  ECONNABORTED
> > > > +
> > > > +Aborted by the local host.
> > > > +
> > > > +BR/EDR connection protocol error
> > > > +================================
> > > > +     code:   0x0011
> > > > +     errno:  EPROTO
> > > > +
> > > > +Failed due to LMP protocol error.
> > > > +
> > > > +BR/EDR connection canceled
> > > > +==========================
> > > > +     code:   0x0012
> > > > +     errno:  none
> > > > +
> > > > +Failed due to cancellation caused by adapter drop, unexpected device drop, or
> > > > +incoming disconnection request before connection request is completed.
> > > > +
> > > > +BR/EDR connection unknown error
> > > > +===============================
> > > > +     code:   0x0013
> > > > +     errno:  ENOSYS
> > > > +
> > > > +Failed due to unknown reason.
> > > > +
> > > > +LE connection invalid arguments
> > > > +===============================
> > > > +     code:   0x0101
> > > > +     errno:  EINVAL
> > > > +
> > > > +Failed due to invalid arguments.
> > > > +
> > > > +LE connection not powered
> > > > +=========================
> > > > +     code:   0x0102
> > > > +     errno:  EHOSTUNREACH
> > > > +
> > > > +Failed due to adapter not powered.
> > > > +
> > > > +LE connection not supported
> > > > +===========================
> > > > +     code:   0x0103
> > > > +     errno:  EOPNOTSUPP, EPROTONOSUPPORT
> > > > +
> > > > +Failed due to unsupported state transition of L2CAP channel or other features
> > > > +(e.g. LE features) either by the local host or the remote.
> > > > +
> > > > +LE connection already connected
> > > > +===============================
> > > > +     code:   0x0104
> > > > +     errno: EALREADY, EISCONN
> > > > +
> > > > +Either the BT IO is already connected or LE link connection in place.
> > > > +
> > > > +LE connection bad socket
> > > > +========================
> > > > +     code:   0x0105
> > > > +     errno: EBADFD
> > > > +
> > > > +Failed due to the socket is in bad state.
> > > > +
> > > > +LE connection memory allocation
> > > > +===============================
> > > > +     code:   0x0106
> > > > +     errno: ENOMEM
> > > > +
> > > > +Failed to allocate memory in either host stack or controller.
> > > > +
> > > > +LE connection busy
> > > > +==================
> > > > +     code:   0x0107
> > > > +     errno:  EBUSY
> > > > +
> > > > +Failed due to other ongoing operations, such as pairing, connecting, busy
> > > > +L2CAP channel or the operation disallowed by the controller.
> > > > +
> > > > +LE connection refused
> > > > +=====================
> > > > +     code:   0x0108
> > > > +     errno:  ECONNREFUSED
> > > > +
> > > > +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.
> > > > +
> > > > +LE connection create socket
> > > > +===========================
> > > > +     code:   0x0109
> > > > +     errno:  EIO
> > > > +
> > > > +Failed to create or connect to BT IO socket. This can also indicate hardware
> > > > +failure in the controller.
> > > > +
> > > > +LE connection timeout
> > > > +=====================
> > > > +     code:   0x010A
> > > > +     errno:  ETIMEDOUT
> > > > +
> > > > +Failed due to connection timeout
> > > > +
> > > > +LE connection concurrent connection limit
> > > > +=========================================
> > > > +     code:   0x010B
> > > > +     errno:  EMLINK
> > > > +
> > > > +Failed due to reaching the synchronous connection limit to a device.
> > > > +
> > > > +LE connection abort by remote
> > > > +=============================
> > > > +     code:   0x010C
> > > > +     errno:  ECONNRESET
> > > > +
> > > > +Aborted by the remote device due to limited resource or power off.
> > > > +
> > > > +LE connection abort by local
> > > > +============================
> > > > +     code:   0x010D
> > > > +     errno:  ECONNABORTED
> > > > +
> > > > +Aborted by the local host.
> > > > +
> > > > +LE connection link layer protocol error
> > > > +=======================================
> > > > +     code:   0x010E
> > > > +     errno:  EPROTO
> > > > +
> > > > +Failed due to link layer protocol error.
> > > > +
> > > > +LE connection GATT browsing
> > > > +===========================
> > > > +     code:   0x010F
> > > > +     errno:  none
> > > > +
> > > > +Failed to complete the GATT browsing.
> > > > +
> > > > +LE connection unknown error
> > > > +===========================
> > > > +     code:   0x0110
> > > > +     errno:  ENOSYS
> > > > +
> > > > + Failed due to unknown reason.
> > > > diff --git a/src/error.c b/src/error.c
> > > > index 89517075e..73602c4bf 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,12 @@ DBusMessage *btd_error_invalid_args_str(DBusMessage *msg, const char *str)
> > > >                                       "%s", str);
> > > > }
> > > >
> > > > +DBusMessage *btd_error_invalid_args_err(DBusMessage *msg, uint16_t err)
> > > > +{
> > > > +     return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
> > > > +                                     "0x%04X", err);
> > > > +}
> > > > +
> > > > DBusMessage *btd_error_busy(DBusMessage *msg)
> > > > {
> > > >       return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
> > > > @@ -79,12 +86,24 @@ DBusMessage *btd_error_in_progress(DBusMessage *msg)
> > > >                                       "In Progress");
> > > > }
> > > >
> > > > +DBusMessage *btd_error_in_progress_err(DBusMessage *msg, uint16_t err)
> > > > +{
> > > > +     return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress", "0x%04X",
> > > > +                                     err);
> > > > +}
> > > > +
> > > > 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)
> > > > +{
> > > > +     return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
> > > > +                                     "0x%04X", err);
> > > > +}
> > > > +
> > > > DBusMessage *btd_error_does_not_exist(DBusMessage *msg)
> > > > {
> > > >       return g_dbus_create_error(msg, ERROR_INTERFACE ".DoesNotExist",
> > > > @@ -121,8 +140,100 @@ DBusMessage *btd_error_not_ready(DBusMessage *msg)
> > > >                                       "Resource Not Ready");
> > > > }
> > > >
> > > > +DBusMessage *btd_error_not_ready_err(DBusMessage *msg, uint16_t err)
> > > > +{
> > > > +     return g_dbus_create_error(msg, ERROR_INTERFACE ".NotReady", "0x%04X",
> > > > +                                     err);
> > > > +}
> > > > +
> > > > 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)
> > > > +{
> > > > +     return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed", "0x%04X",
> > > > +                                     err);
> > > > +}
> > > > +
> > > > +uint16_t btd_error_bredr_conn_from_errno(int errno_code)
> > > > +{
> > > > +     switch (-errno_code) {
> > > > +     case EALREADY:
> > > > +     case EISCONN: // Fall through
> > >
> > > Don’t do this Fall through. It is actually not a fall through per se. This is just a statement with two case labels. That is perfectly normal and no compiler should complain. And frankly no C-programmer should be confused if this was intentional or not.
> > Corrected in v3.
> > >
> > > > +             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_CNCR_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_ABORT_BY_REMOTE;
> > > > +     case ECONNABORTED:
> > > > +             return BTD_ERR_BREDR_CONN_ABORT_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_ABORT_BY_REMOTE;
> > > > +     case ECONNABORTED:
> > > > +             return BTD_ERR_LE_CONN_ABORT_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..74d433aca 100644
> > > > --- a/src/error.h
> > > > +++ b/src/error.h
> > > > @@ -24,22 +24,74 @@
> > > >  */
> > > >
> > > > #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().
> > > > + */
> > > > +#define BTD_ERR_BREDR_CONN_ALREADY_CONNECTED 0x0001
> > > > +#define BTD_ERR_BREDR_CONN_PAGE_TIMEOUT              0x0002
> > > > +#define BTD_ERR_BREDR_CONN_PROFILE_UNAVAILABLE       0x0003
> > > > +#define BTD_ERR_BREDR_CONN_SDP_SEARCH                0x0004
> > > > +#define BTD_ERR_BREDR_CONN_CREATE_SOCKET     0x0005
> > > > +#define BTD_ERR_BREDR_CONN_INVALID_ARGUMENTS 0x0006
> > > > +#define BTD_ERR_BREDR_CONN_ADAPTER_NOT_POWERED       0x0007
> > > > +#define BTD_ERR_BREDR_CONN_NOT_SUPPORTED     0x0008
> > > > +#define BTD_ERR_BREDR_CONN_BAD_SOCKET                0x0009
> > > > +#define BTD_ERR_BREDR_CONN_MEMORY_ALLOC              0x000A
> > > > +#define BTD_ERR_BREDR_CONN_BUSY                      0x000B
> > > > +#define BTD_ERR_BREDR_CONN_CNCR_CONNECT_LIMIT        0x000C
> > > > +#define BTD_ERR_BREDR_CONN_TIMEOUT           0x000D
> > > > +#define BTD_ERR_BREDR_CONN_REFUSED           0x000E
> > > > +#define BTD_ERR_BREDR_CONN_ABORT_BY_REMOTE   0x000F
> > > > +#define BTD_ERR_BREDR_CONN_ABORT_BY_LOCAL    0x0010
> > > > +#define BTD_ERR_BREDR_CONN_PROTO_ERROR               0x0011
> > > > +#define BTD_ERR_BREDR_CONN_CANCELED          0x0012
> > > > +#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().
> > > > + */
> > > > +#define BTD_ERR_LE_CONN_INVALID_ARGUMENTS    0x0101
> > > > +#define BTD_ERR_LE_CONN_ADAPTER_NOT_POWERED  0x0102
> > > > +#define BTD_ERR_LE_CONN_NOT_SUPPORTED                0x0103
> > > > +#define BTD_ERR_LE_CONN_ALREADY_CONNECTED    0x0104
> > > > +#define BTD_ERR_LE_CONN_BAD_SOCKET           0x0105
> > > > +#define BTD_ERR_LE_CONN_MEMORY_ALLOC         0x0106
> > > > +#define BTD_ERR_LE_CONN_BUSY                 0x0107
> > > > +#define BTD_ERR_LE_CONN_REFUSED                      0x0108
> > > > +#define BTD_ERR_LE_CONN_CREATE_SOCKET                0x0109
> > > > +#define BTD_ERR_LE_CONN_TIMEOUT                      0x010A
> > > > +#define BTD_ERR_LE_CONN_SYNC_CONNECT_LIMIT   0x010B
> > > > +#define BTD_ERR_LE_CONN_ABORT_BY_REMOTE              0x010C
> > > > +#define BTD_ERR_LE_CONN_ABORT_BY_LOCAL               0x010D
> > > > +#define BTD_ERR_LE_CONN_PROTO_ERROR          0x010E
> > > > +#define BTD_ERR_LE_CONN_GATT_BROWSE          0x010F
> > > > +#define BTD_ERR_LE_CONN_UNKNOWN                      0x0110
> > > > +
> > >
> > > What is the intention to split BR/EDR and LE here. You do know up-front what connection type you are. Trying to figure out from the error code what connection you have been trying to establish is plain wrong.
> > In fact the up-front connection type is not necessarily known. In the
> > case of dual-mode devices, such as Bose QC35, a D-Bus client can issue
> > Connect(), and it depends on the timing of connection request (adv
> > usually arrive first compared to inquiry result), it can be either
> > BR/EDR or LE link being established. Another aspect of this is the
> > metrics collection, where knowing transport can be handy. For
> > instance, we can associate the certain error to particular use cases
> > at application layer, and that can help targeting the bottleneck to
> > tackle.
> > >
> > > The description is that you want to know exactly where the connection failed. And I think that can be established independent from the transport.
> > Indeed the intention is to know where it failed exactly. However, as
> > mentioned above, transport information is also an important piece of
> > information to know.
> > >
> > > In addition, I don’t like the 0x00?? vs 0x01?? reservation of any number. That always goes bad at some point in the future.
> > As replied above, having a code attached instead of a string
> > description makes it easier for a D-Bus client to interpret and map to
> > corresponding handlers, but I am happy to explore other options as
> > well.
> > >
> > > Regards
> > >
> > > Marcel
> > >
> > Thanks,
> > Miao
>
>
>
> --
> Luiz Augusto von Dentz
Regards,
Miao

Re: [BlueZ PATCH v2 1/3] error: BR/EDR and LE connection failure reasons

[Luiz Augusto von Dentz]

Hi Miao,

On Thu, Jul 15, 2021 at 4:21 PM Miao-chen Chou <mcchou@chromium.org> wrote:
>
> Hi Luiz,
>
> On Wed, Jul 14, 2021 at 3:06 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Miao,
> >
> > On Wed, Jul 14, 2021 at 2:13 PM Miao-chen Chou <mcchou@chromium.org> wrote:
> > >
> > > On Fri, Jun 25, 2021 at 10:39 PM Marcel Holtmann <marcel@holtmann.org> wrote:
> > > >
> > > > Hi Miao-chen,
> > > >
> > > > > 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
> > > > >
> > > > > This also added error-code.txt to describe these error codes.
> > > > >
> > > > > Reviewed-by: Alain Michaud <alainm@chromium.org>
> > > > > Reviewed-by: Howard Chung <howardchung@google.com>
> > > > > ---
> > > > >
> > > > > Changes in v2:
> > > > > - Add error-code.txt
> > > > > - Remove BtdError from return string
> > > > >
> > > > > doc/error-code.txt | 266 +++++++++++++++++++++++++++++++++++++++++++++
> > > > > src/error.c        | 111 +++++++++++++++++++
> > > > > src/error.h        |  52 +++++++++
> > > > > 3 files changed, 429 insertions(+)
> > > > > create mode 100644 doc/error-code.txt
> > > >
> > > > please split documentation and code changes into separate patches.
> > > >
> > > >
> > > > >
> > > > > diff --git a/doc/error-code.txt b/doc/error-code.txt
> > > > > new file mode 100644
> > > > > index 000000000..e91324855
> > > > > --- /dev/null
> > > > > +++ b/doc/error-code.txt
> > > > > @@ -0,0 +1,266 @@
> > > > > +D-Bus Method Return Error Codes
> > > > > +===============================
> > > > > +
> > > > > +The motivation of having detailed error codes is to provide context-based
> > > > > +failure reasons along with D-Bus method return so that D-Bus clients can
> > > > > +build metrics and optimize their application based on these failure reasons.
> > > > > +For instance, a client can build retry mechanism for a connection failure or
> > > > > +improve the bottleneck of use scenario based on actionable metrics.
> > > > > +
> > > > > +These error codes are context-based but not necessarily tied to interface or
> > > > > +method calls. For instance, if a pairing request failed due to connection
> > > > > +failure, connection error would be attached to the method return of Pair().
> > > > > +
> > > > > +BR/EDR connection already connected
> > > > > +===================================
> > > > > +     code:   0x0001
> > > > > +     errno:  EALREADY, EISCONN
> > > >
> > > > I would rather see connnection-already-connected instead of 0x0001 in the D-Bus error message.
> > > Having a code attached instead of a string description makes it easier
> > > for a D-Bus client to interpret and map to corresponding handlers IMO.
> > > For instead, a client can simplily use the code in a switch case to
> > > perform retry or error reporting.
> >
> > Well you first need to decode it since the error message is still a
> > string, but I would have to agree that doing things like strcmp for
> > the error message might be more error prone than extracting the error
> > code since the former is more likely to change, the weird thing is
> > that D-Bus seems to allow passing more than just a error message
> > (https://dbus.freedesktop.org/doc/dbus-specification.html):
> >
> >  'Errors
> >
> >  Messages of type ERROR are most commonly replies to a METHOD_CALL,
> > but may be returned in reply to any kind of message. The message bus
> > for example will return an ERROR in reply to a signal emission if the
> > bus does not have enough memory to send the signal.
> >
> >  An ERROR may have any arguments, but if the first argument is a
> > STRING, it must be an error message. The error message may be logged
> > or shown to the user in some way.'
> >
> > But it looks like libdbus does expect only a string:
> >
> > https://dbus.freedesktop.org/doc/api/html/group__DBusMessage.html#ga2ab896965aec97fb21293affeed36232
> >
> > Well perhaps it needs to constructed manually with the use of
> > dbus_message_new_method_return if you want to pass parameters other
> > than error_name and error_message so we could include a third one e.g.
> > error_code.
> It's good to learn that D-Bus allows other types to be attached in
> error return. My only concern around introducing new types to the
> error return is that it changes how the D-Bus client interprets the
> error return. What do you think?

I guess it is worth checking if that would break clients, if it does
then it is probably a no go, either way it would probably need a
specialized error handling to go beyond the err_message so perhaps it
is safer to stick to err_name and err_message only and encode the
error code on the err_message.

> >
> > > >
> > > > > +
> > > > > +Either the profile is already connected or ACL connection is in place.
> > > > > +
> > > > > +BR/EDR connection page timeout
> > > > > +==============================
> > > > > +     code:   0x0002
> > > > > +     errno:  EHOSTDOWN
> > > > > +
> > > > > +Failed due to page timeout.
> > > > > +
> > > > > +BR/EDR connection profile unavailable
> > > > > +=====================================
> > > > > +     code:   0x0003
> > > > > +     errno:  ENOPROTOOPT
> > > > > +
> > > > > +Failed to find connectable services or the target service.
> > > > > +
> > > > > +BR/EDR connection SDP search
> > > > > +============================
> > > > > +     code:   0x0004
> > > > > +     errno:  none
> > > > > +
> > > > > +Failed to complete the SDP search.
> > > > > +
> > > > > +BR/EDR connection create socket
> > > > > +===============================
> > > > > +     code:   0x0005
> > > > > +     errno:  EIO
> > > > > +
> > > > > +Failed to create or connect to BT IO socket. This can also indicate hardware
> > > > > +failure in the controller.
> > > > > +
> > > > > +BR/EDR connection invalid arguments
> > > > > +===================================
> > > > > +     code:   0x0006
> > > > > +     errno:  EHOSTUNREACH
> > > > > +
> > > > > +Failed due to invalid arguments.
> > > > > +
> > > > > +BR/EDR connection not powered
> > > > > +=============================
> > > > > +     code:   0x0007
> > > > > +     errno:  EHOSTUNREACH
> > > > > +
> > > > > +Failed due to adapter not powered.
> > > > > +
> > > > > +BR/EDR connection not supported
> > > > > +===============================
> > > > > +     code:   0x0008
> > > > > +     errno:  EOPNOTSUPP, EPROTONOSUPPORT
> > > > > +
> > > > > +Failed due to unsupported state transition of L2CAP channel or other features
> > > > > +either by the local host or the remote.
> > > > > +
> > > > > +BR/EDR connection bad socket
> > > > > +============================
> > > > > +     code:   0x0009
> > > > > +     errno:  EBADFD
> > > > > +
> > > > > +Failed due to the socket is in bad state.
> > > > > +
> > > > > +BR/EDR connection memory allocation
> > > > > +===================================
> > > > > +     code:   0x000A
> > > > > +     errno:  EBADFD
> > > > > +
> > > > > +Failed to allocate memory in either host stack or controller.
> > > >
> > > > If this happens, then the code is wrong. Should be an ENOMEM.
> > > My mistake, this should be ENOMEM. Corrected in v3.
> > > >
> > > > > +
> > > > > +BR/EDR connection busy
> > > > > +======================
> > > > > +     code:   0x000B
> > > > > +     errno:  EBUSY
> > > > > +
> > > > > +Failed due to other ongoing operations, such as pairing, busy L2CAP channel or
> > > > > +the operation disallowed by the controller.
> > > > > +
> > > > > +BR/EDR connection concurrent connection limit
> > > > > +=============================================
> > > > > +     code:   0x000C
> > > > > +     errno:  EMLINK
> > > > > +
> > > > > +Failed due to reaching the concurrent connection limit to a device.
> > > > > +
> > > > > +BR/EDR connection timeout
> > > > > +=========================
> > > > > +     code:   0x000D
> > > > > +     errno:  ETIMEDOUT
> > > > > +
> > > > > +Failed due to connection timeout
> > > > > +
> > > > > +BR/EDR connection refused
> > > > > +=========================
> > > > > +     code:   0x000E
> > > > > +     errno:  ECONNREFUSED
> > > > > +
> > > > > +Refused by the remote device due to limited resource, security reason or
> > > > > +unacceptable address type.
> > > > > +
> > > > > +BR/EDR connection aborted by remote
> > > > > +===================================
> > > > > +     code:   0x000F
> > > > > +     errno:  ECONNRESET
> > > > > +
> > > > > +Terminated by the remote device due to limited resource or power off.
> > > > > +
> > > > > +BR/EDR connection aborted by local
> > > > > +==================================
> > > > > +     code:   0x0010
> > > > > +     errno:  ECONNABORTED
> > > > > +
> > > > > +Aborted by the local host.
> > > > > +
> > > > > +BR/EDR connection protocol error
> > > > > +================================
> > > > > +     code:   0x0011
> > > > > +     errno:  EPROTO
> > > > > +
> > > > > +Failed due to LMP protocol error.
> > > > > +
> > > > > +BR/EDR connection canceled
> > > > > +==========================
> > > > > +     code:   0x0012
> > > > > +     errno:  none
> > > > > +
> > > > > +Failed due to cancellation caused by adapter drop, unexpected device drop, or
> > > > > +incoming disconnection request before connection request is completed.
> > > > > +
> > > > > +BR/EDR connection unknown error
> > > > > +===============================
> > > > > +     code:   0x0013
> > > > > +     errno:  ENOSYS
> > > > > +
> > > > > +Failed due to unknown reason.
> > > > > +
> > > > > +LE connection invalid arguments
> > > > > +===============================
> > > > > +     code:   0x0101
> > > > > +     errno:  EINVAL
> > > > > +
> > > > > +Failed due to invalid arguments.
> > > > > +
> > > > > +LE connection not powered
> > > > > +=========================
> > > > > +     code:   0x0102
> > > > > +     errno:  EHOSTUNREACH
> > > > > +
> > > > > +Failed due to adapter not powered.
> > > > > +
> > > > > +LE connection not supported
> > > > > +===========================
> > > > > +     code:   0x0103
> > > > > +     errno:  EOPNOTSUPP, EPROTONOSUPPORT
> > > > > +
> > > > > +Failed due to unsupported state transition of L2CAP channel or other features
> > > > > +(e.g. LE features) either by the local host or the remote.
> > > > > +
> > > > > +LE connection already connected
> > > > > +===============================
> > > > > +     code:   0x0104
> > > > > +     errno: EALREADY, EISCONN
> > > > > +
> > > > > +Either the BT IO is already connected or LE link connection in place.
> > > > > +
> > > > > +LE connection bad socket
> > > > > +========================
> > > > > +     code:   0x0105
> > > > > +     errno: EBADFD
> > > > > +
> > > > > +Failed due to the socket is in bad state.
> > > > > +
> > > > > +LE connection memory allocation
> > > > > +===============================
> > > > > +     code:   0x0106
> > > > > +     errno: ENOMEM
> > > > > +
> > > > > +Failed to allocate memory in either host stack or controller.
> > > > > +
> > > > > +LE connection busy
> > > > > +==================
> > > > > +     code:   0x0107
> > > > > +     errno:  EBUSY
> > > > > +
> > > > > +Failed due to other ongoing operations, such as pairing, connecting, busy
> > > > > +L2CAP channel or the operation disallowed by the controller.
> > > > > +
> > > > > +LE connection refused
> > > > > +=====================
> > > > > +     code:   0x0108
> > > > > +     errno:  ECONNREFUSED
> > > > > +
> > > > > +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.
> > > > > +
> > > > > +LE connection create socket
> > > > > +===========================
> > > > > +     code:   0x0109
> > > > > +     errno:  EIO
> > > > > +
> > > > > +Failed to create or connect to BT IO socket. This can also indicate hardware
> > > > > +failure in the controller.
> > > > > +
> > > > > +LE connection timeout
> > > > > +=====================
> > > > > +     code:   0x010A
> > > > > +     errno:  ETIMEDOUT
> > > > > +
> > > > > +Failed due to connection timeout
> > > > > +
> > > > > +LE connection concurrent connection limit
> > > > > +=========================================
> > > > > +     code:   0x010B
> > > > > +     errno:  EMLINK
> > > > > +
> > > > > +Failed due to reaching the synchronous connection limit to a device.
> > > > > +
> > > > > +LE connection abort by remote
> > > > > +=============================
> > > > > +     code:   0x010C
> > > > > +     errno:  ECONNRESET
> > > > > +
> > > > > +Aborted by the remote device due to limited resource or power off.
> > > > > +
> > > > > +LE connection abort by local
> > > > > +============================
> > > > > +     code:   0x010D
> > > > > +     errno:  ECONNABORTED
> > > > > +
> > > > > +Aborted by the local host.
> > > > > +
> > > > > +LE connection link layer protocol error
> > > > > +=======================================
> > > > > +     code:   0x010E
> > > > > +     errno:  EPROTO
> > > > > +
> > > > > +Failed due to link layer protocol error.
> > > > > +
> > > > > +LE connection GATT browsing
> > > > > +===========================
> > > > > +     code:   0x010F
> > > > > +     errno:  none
> > > > > +
> > > > > +Failed to complete the GATT browsing.
> > > > > +
> > > > > +LE connection unknown error
> > > > > +===========================
> > > > > +     code:   0x0110
> > > > > +     errno:  ENOSYS
> > > > > +
> > > > > + Failed due to unknown reason.
> > > > > diff --git a/src/error.c b/src/error.c
> > > > > index 89517075e..73602c4bf 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,12 @@ DBusMessage *btd_error_invalid_args_str(DBusMessage *msg, const char *str)
> > > > >                                       "%s", str);
> > > > > }
> > > > >
> > > > > +DBusMessage *btd_error_invalid_args_err(DBusMessage *msg, uint16_t err)
> > > > > +{
> > > > > +     return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
> > > > > +                                     "0x%04X", err);
> > > > > +}
> > > > > +
> > > > > DBusMessage *btd_error_busy(DBusMessage *msg)
> > > > > {
> > > > >       return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
> > > > > @@ -79,12 +86,24 @@ DBusMessage *btd_error_in_progress(DBusMessage *msg)
> > > > >                                       "In Progress");
> > > > > }
> > > > >
> > > > > +DBusMessage *btd_error_in_progress_err(DBusMessage *msg, uint16_t err)
> > > > > +{
> > > > > +     return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress", "0x%04X",
> > > > > +                                     err);
> > > > > +}
> > > > > +
> > > > > 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)
> > > > > +{
> > > > > +     return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
> > > > > +                                     "0x%04X", err);
> > > > > +}
> > > > > +
> > > > > DBusMessage *btd_error_does_not_exist(DBusMessage *msg)
> > > > > {
> > > > >       return g_dbus_create_error(msg, ERROR_INTERFACE ".DoesNotExist",
> > > > > @@ -121,8 +140,100 @@ DBusMessage *btd_error_not_ready(DBusMessage *msg)
> > > > >                                       "Resource Not Ready");
> > > > > }
> > > > >
> > > > > +DBusMessage *btd_error_not_ready_err(DBusMessage *msg, uint16_t err)
> > > > > +{
> > > > > +     return g_dbus_create_error(msg, ERROR_INTERFACE ".NotReady", "0x%04X",
> > > > > +                                     err);
> > > > > +}
> > > > > +
> > > > > 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)
> > > > > +{
> > > > > +     return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed", "0x%04X",
> > > > > +                                     err);
> > > > > +}
> > > > > +
> > > > > +uint16_t btd_error_bredr_conn_from_errno(int errno_code)
> > > > > +{
> > > > > +     switch (-errno_code) {
> > > > > +     case EALREADY:
> > > > > +     case EISCONN: // Fall through
> > > >
> > > > Don’t do this Fall through. It is actually not a fall through per se. This is just a statement with two case labels. That is perfectly normal and no compiler should complain. And frankly no C-programmer should be confused if this was intentional or not.
> > > Corrected in v3.
> > > >
> > > > > +             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_CNCR_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_ABORT_BY_REMOTE;
> > > > > +     case ECONNABORTED:
> > > > > +             return BTD_ERR_BREDR_CONN_ABORT_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_ABORT_BY_REMOTE;
> > > > > +     case ECONNABORTED:
> > > > > +             return BTD_ERR_LE_CONN_ABORT_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..74d433aca 100644
> > > > > --- a/src/error.h
> > > > > +++ b/src/error.h
> > > > > @@ -24,22 +24,74 @@
> > > > >  */
> > > > >
> > > > > #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().
> > > > > + */
> > > > > +#define BTD_ERR_BREDR_CONN_ALREADY_CONNECTED 0x0001
> > > > > +#define BTD_ERR_BREDR_CONN_PAGE_TIMEOUT              0x0002
> > > > > +#define BTD_ERR_BREDR_CONN_PROFILE_UNAVAILABLE       0x0003
> > > > > +#define BTD_ERR_BREDR_CONN_SDP_SEARCH                0x0004
> > > > > +#define BTD_ERR_BREDR_CONN_CREATE_SOCKET     0x0005
> > > > > +#define BTD_ERR_BREDR_CONN_INVALID_ARGUMENTS 0x0006
> > > > > +#define BTD_ERR_BREDR_CONN_ADAPTER_NOT_POWERED       0x0007
> > > > > +#define BTD_ERR_BREDR_CONN_NOT_SUPPORTED     0x0008
> > > > > +#define BTD_ERR_BREDR_CONN_BAD_SOCKET                0x0009
> > > > > +#define BTD_ERR_BREDR_CONN_MEMORY_ALLOC              0x000A
> > > > > +#define BTD_ERR_BREDR_CONN_BUSY                      0x000B
> > > > > +#define BTD_ERR_BREDR_CONN_CNCR_CONNECT_LIMIT        0x000C
> > > > > +#define BTD_ERR_BREDR_CONN_TIMEOUT           0x000D
> > > > > +#define BTD_ERR_BREDR_CONN_REFUSED           0x000E
> > > > > +#define BTD_ERR_BREDR_CONN_ABORT_BY_REMOTE   0x000F
> > > > > +#define BTD_ERR_BREDR_CONN_ABORT_BY_LOCAL    0x0010
> > > > > +#define BTD_ERR_BREDR_CONN_PROTO_ERROR               0x0011
> > > > > +#define BTD_ERR_BREDR_CONN_CANCELED          0x0012
> > > > > +#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().
> > > > > + */
> > > > > +#define BTD_ERR_LE_CONN_INVALID_ARGUMENTS    0x0101
> > > > > +#define BTD_ERR_LE_CONN_ADAPTER_NOT_POWERED  0x0102
> > > > > +#define BTD_ERR_LE_CONN_NOT_SUPPORTED                0x0103
> > > > > +#define BTD_ERR_LE_CONN_ALREADY_CONNECTED    0x0104
> > > > > +#define BTD_ERR_LE_CONN_BAD_SOCKET           0x0105
> > > > > +#define BTD_ERR_LE_CONN_MEMORY_ALLOC         0x0106
> > > > > +#define BTD_ERR_LE_CONN_BUSY                 0x0107
> > > > > +#define BTD_ERR_LE_CONN_REFUSED                      0x0108
> > > > > +#define BTD_ERR_LE_CONN_CREATE_SOCKET                0x0109
> > > > > +#define BTD_ERR_LE_CONN_TIMEOUT                      0x010A
> > > > > +#define BTD_ERR_LE_CONN_SYNC_CONNECT_LIMIT   0x010B
> > > > > +#define BTD_ERR_LE_CONN_ABORT_BY_REMOTE              0x010C
> > > > > +#define BTD_ERR_LE_CONN_ABORT_BY_LOCAL               0x010D
> > > > > +#define BTD_ERR_LE_CONN_PROTO_ERROR          0x010E
> > > > > +#define BTD_ERR_LE_CONN_GATT_BROWSE          0x010F
> > > > > +#define BTD_ERR_LE_CONN_UNKNOWN                      0x0110
> > > > > +
> > > >
> > > > What is the intention to split BR/EDR and LE here. You do know up-front what connection type you are. Trying to figure out from the error code what connection you have been trying to establish is plain wrong.
> > > In fact the up-front connection type is not necessarily known. In the
> > > case of dual-mode devices, such as Bose QC35, a D-Bus client can issue
> > > Connect(), and it depends on the timing of connection request (adv
> > > usually arrive first compared to inquiry result), it can be either
> > > BR/EDR or LE link being established. Another aspect of this is the
> > > metrics collection, where knowing transport can be handy. For
> > > instance, we can associate the certain error to particular use cases
> > > at application layer, and that can help targeting the bottleneck to
> > > tackle.
> > > >
> > > > The description is that you want to know exactly where the connection failed. And I think that can be established independent from the transport.
> > > Indeed the intention is to know where it failed exactly. However, as
> > > mentioned above, transport information is also an important piece of
> > > information to know.
> > > >
> > > > In addition, I don’t like the 0x00?? vs 0x01?? reservation of any number. That always goes bad at some point in the future.
> > > As replied above, having a code attached instead of a string
> > > description makes it easier for a D-Bus client to interpret and map to
> > > corresponding handlers, but I am happy to explore other options as
> > > well.
> > > >
> > > > Regards
> > > >
> > > > Marcel
> > > >
> > > Thanks,
> > > Miao
> >
> >
> >
> > --
> > Luiz Augusto von Dentz
> Regards,
> Miao



-- 
Luiz Augusto von Dentz

Re: [BlueZ PATCH v2 1/3] error: BR/EDR and LE connection failure reasons

[Marcel Holtmann]

Hi Miao-chen,

>>> 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
>>> 
>>> This also added error-code.txt to describe these error codes.
>>> 
>>> Reviewed-by: Alain Michaud <alainm@chromium.org>
>>> Reviewed-by: Howard Chung <howardchung@google.com>
>>> ---
>>> 
>>> Changes in v2:
>>> - Add error-code.txt
>>> - Remove BtdError from return string
>>> 
>>> doc/error-code.txt | 266 +++++++++++++++++++++++++++++++++++++++++++++
>>> src/error.c        | 111 +++++++++++++++++++
>>> src/error.h        |  52 +++++++++
>>> 3 files changed, 429 insertions(+)
>>> create mode 100644 doc/error-code.txt
>> 
>> please split documentation and code changes into separate patches.
>> 
>> 
>>> 
>>> diff --git a/doc/error-code.txt b/doc/error-code.txt
>>> new file mode 100644
>>> index 000000000..e91324855
>>> --- /dev/null
>>> +++ b/doc/error-code.txt
>>> @@ -0,0 +1,266 @@
>>> +D-Bus Method Return Error Codes
>>> +===============================
>>> +
>>> +The motivation of having detailed error codes is to provide context-based
>>> +failure reasons along with D-Bus method return so that D-Bus clients can
>>> +build metrics and optimize their application based on these failure reasons.
>>> +For instance, a client can build retry mechanism for a connection failure or
>>> +improve the bottleneck of use scenario based on actionable metrics.
>>> +
>>> +These error codes are context-based but not necessarily tied to interface or
>>> +method calls. For instance, if a pairing request failed due to connection
>>> +failure, connection error would be attached to the method return of Pair().
>>> +
>>> +BR/EDR connection already connected
>>> +===================================
>>> +     code:   0x0001
>>> +     errno:  EALREADY, EISCONN
>> 
>> I would rather see connnection-already-connected instead of 0x0001 in the D-Bus error message.
> Having a code attached instead of a string description makes it easier
> for a D-Bus client to interpret and map to corresponding handlers IMO.
> For instead, a client can simplily use the code in a switch case to
> perform retry or error reporting.

this argument has been made since forever. However D-Bus is heavily string based and we also have our API heavily string and descriptive. So I rather keep it that way.

>> 
>>> +
>>> +Either the profile is already connected or ACL connection is in place.
>>> +
>>> +BR/EDR connection page timeout
>>> +==============================
>>> +     code:   0x0002
>>> +     errno:  EHOSTDOWN
>>> +
>>> +Failed due to page timeout.
>>> +
>>> +BR/EDR connection profile unavailable
>>> +=====================================
>>> +     code:   0x0003
>>> +     errno:  ENOPROTOOPT
>>> +
>>> +Failed to find connectable services or the target service.
>>> +
>>> +BR/EDR connection SDP search
>>> +============================
>>> +     code:   0x0004
>>> +     errno:  none
>>> +
>>> +Failed to complete the SDP search.
>>> +
>>> +BR/EDR connection create socket
>>> +===============================
>>> +     code:   0x0005
>>> +     errno:  EIO
>>> +
>>> +Failed to create or connect to BT IO socket. This can also indicate hardware
>>> +failure in the controller.
>>> +
>>> +BR/EDR connection invalid arguments
>>> +===================================
>>> +     code:   0x0006
>>> +     errno:  EHOSTUNREACH
>>> +
>>> +Failed due to invalid arguments.
>>> +
>>> +BR/EDR connection not powered
>>> +=============================
>>> +     code:   0x0007
>>> +     errno:  EHOSTUNREACH
>>> +
>>> +Failed due to adapter not powered.
>>> +
>>> +BR/EDR connection not supported
>>> +===============================
>>> +     code:   0x0008
>>> +     errno:  EOPNOTSUPP, EPROTONOSUPPORT
>>> +
>>> +Failed due to unsupported state transition of L2CAP channel or other features
>>> +either by the local host or the remote.
>>> +
>>> +BR/EDR connection bad socket
>>> +============================
>>> +     code:   0x0009
>>> +     errno:  EBADFD
>>> +
>>> +Failed due to the socket is in bad state.
>>> +
>>> +BR/EDR connection memory allocation
>>> +===================================
>>> +     code:   0x000A
>>> +     errno:  EBADFD
>>> +
>>> +Failed to allocate memory in either host stack or controller.
>> 
>> If this happens, then the code is wrong. Should be an ENOMEM.
> My mistake, this should be ENOMEM. Corrected in v3.
>> 
>>> +
>>> +BR/EDR connection busy
>>> +======================
>>> +     code:   0x000B
>>> +     errno:  EBUSY
>>> +
>>> +Failed due to other ongoing operations, such as pairing, busy L2CAP channel or
>>> +the operation disallowed by the controller.
>>> +
>>> +BR/EDR connection concurrent connection limit
>>> +=============================================
>>> +     code:   0x000C
>>> +     errno:  EMLINK
>>> +
>>> +Failed due to reaching the concurrent connection limit to a device.
>>> +
>>> +BR/EDR connection timeout
>>> +=========================
>>> +     code:   0x000D
>>> +     errno:  ETIMEDOUT
>>> +
>>> +Failed due to connection timeout
>>> +
>>> +BR/EDR connection refused
>>> +=========================
>>> +     code:   0x000E
>>> +     errno:  ECONNREFUSED
>>> +
>>> +Refused by the remote device due to limited resource, security reason or
>>> +unacceptable address type.
>>> +
>>> +BR/EDR connection aborted by remote
>>> +===================================
>>> +     code:   0x000F
>>> +     errno:  ECONNRESET
>>> +
>>> +Terminated by the remote device due to limited resource or power off.
>>> +
>>> +BR/EDR connection aborted by local
>>> +==================================
>>> +     code:   0x0010
>>> +     errno:  ECONNABORTED
>>> +
>>> +Aborted by the local host.
>>> +
>>> +BR/EDR connection protocol error
>>> +================================
>>> +     code:   0x0011
>>> +     errno:  EPROTO
>>> +
>>> +Failed due to LMP protocol error.
>>> +
>>> +BR/EDR connection canceled
>>> +==========================
>>> +     code:   0x0012
>>> +     errno:  none
>>> +
>>> +Failed due to cancellation caused by adapter drop, unexpected device drop, or
>>> +incoming disconnection request before connection request is completed.
>>> +
>>> +BR/EDR connection unknown error
>>> +===============================
>>> +     code:   0x0013
>>> +     errno:  ENOSYS
>>> +
>>> +Failed due to unknown reason.
>>> +
>>> +LE connection invalid arguments
>>> +===============================
>>> +     code:   0x0101
>>> +     errno:  EINVAL
>>> +
>>> +Failed due to invalid arguments.
>>> +
>>> +LE connection not powered
>>> +=========================
>>> +     code:   0x0102
>>> +     errno:  EHOSTUNREACH
>>> +
>>> +Failed due to adapter not powered.
>>> +
>>> +LE connection not supported
>>> +===========================
>>> +     code:   0x0103
>>> +     errno:  EOPNOTSUPP, EPROTONOSUPPORT
>>> +
>>> +Failed due to unsupported state transition of L2CAP channel or other features
>>> +(e.g. LE features) either by the local host or the remote.
>>> +
>>> +LE connection already connected
>>> +===============================
>>> +     code:   0x0104
>>> +     errno: EALREADY, EISCONN
>>> +
>>> +Either the BT IO is already connected or LE link connection in place.
>>> +
>>> +LE connection bad socket
>>> +========================
>>> +     code:   0x0105
>>> +     errno: EBADFD
>>> +
>>> +Failed due to the socket is in bad state.
>>> +
>>> +LE connection memory allocation
>>> +===============================
>>> +     code:   0x0106
>>> +     errno: ENOMEM
>>> +
>>> +Failed to allocate memory in either host stack or controller.
>>> +
>>> +LE connection busy
>>> +==================
>>> +     code:   0x0107
>>> +     errno:  EBUSY
>>> +
>>> +Failed due to other ongoing operations, such as pairing, connecting, busy
>>> +L2CAP channel or the operation disallowed by the controller.
>>> +
>>> +LE connection refused
>>> +=====================
>>> +     code:   0x0108
>>> +     errno:  ECONNREFUSED
>>> +
>>> +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.
>>> +
>>> +LE connection create socket
>>> +===========================
>>> +     code:   0x0109
>>> +     errno:  EIO
>>> +
>>> +Failed to create or connect to BT IO socket. This can also indicate hardware
>>> +failure in the controller.
>>> +
>>> +LE connection timeout
>>> +=====================
>>> +     code:   0x010A
>>> +     errno:  ETIMEDOUT
>>> +
>>> +Failed due to connection timeout
>>> +
>>> +LE connection concurrent connection limit
>>> +=========================================
>>> +     code:   0x010B
>>> +     errno:  EMLINK
>>> +
>>> +Failed due to reaching the synchronous connection limit to a device.
>>> +
>>> +LE connection abort by remote
>>> +=============================
>>> +     code:   0x010C
>>> +     errno:  ECONNRESET
>>> +
>>> +Aborted by the remote device due to limited resource or power off.
>>> +
>>> +LE connection abort by local
>>> +============================
>>> +     code:   0x010D
>>> +     errno:  ECONNABORTED
>>> +
>>> +Aborted by the local host.
>>> +
>>> +LE connection link layer protocol error
>>> +=======================================
>>> +     code:   0x010E
>>> +     errno:  EPROTO
>>> +
>>> +Failed due to link layer protocol error.
>>> +
>>> +LE connection GATT browsing
>>> +===========================
>>> +     code:   0x010F
>>> +     errno:  none
>>> +
>>> +Failed to complete the GATT browsing.
>>> +
>>> +LE connection unknown error
>>> +===========================
>>> +     code:   0x0110
>>> +     errno:  ENOSYS
>>> +
>>> + Failed due to unknown reason.
>>> diff --git a/src/error.c b/src/error.c
>>> index 89517075e..73602c4bf 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,12 @@ DBusMessage *btd_error_invalid_args_str(DBusMessage *msg, const char *str)
>>>                                      "%s", str);
>>> }
>>> 
>>> +DBusMessage *btd_error_invalid_args_err(DBusMessage *msg, uint16_t err)
>>> +{
>>> +     return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
>>> +                                     "0x%04X", err);
>>> +}
>>> +
>>> DBusMessage *btd_error_busy(DBusMessage *msg)
>>> {
>>>      return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
>>> @@ -79,12 +86,24 @@ DBusMessage *btd_error_in_progress(DBusMessage *msg)
>>>                                      "In Progress");
>>> }
>>> 
>>> +DBusMessage *btd_error_in_progress_err(DBusMessage *msg, uint16_t err)
>>> +{
>>> +     return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress", "0x%04X",
>>> +                                     err);
>>> +}
>>> +
>>> 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)
>>> +{
>>> +     return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
>>> +                                     "0x%04X", err);
>>> +}
>>> +
>>> DBusMessage *btd_error_does_not_exist(DBusMessage *msg)
>>> {
>>>      return g_dbus_create_error(msg, ERROR_INTERFACE ".DoesNotExist",
>>> @@ -121,8 +140,100 @@ DBusMessage *btd_error_not_ready(DBusMessage *msg)
>>>                                      "Resource Not Ready");
>>> }
>>> 
>>> +DBusMessage *btd_error_not_ready_err(DBusMessage *msg, uint16_t err)
>>> +{
>>> +     return g_dbus_create_error(msg, ERROR_INTERFACE ".NotReady", "0x%04X",
>>> +                                     err);
>>> +}
>>> +
>>> 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)
>>> +{
>>> +     return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed", "0x%04X",
>>> +                                     err);
>>> +}
>>> +
>>> +uint16_t btd_error_bredr_conn_from_errno(int errno_code)
>>> +{
>>> +     switch (-errno_code) {
>>> +     case EALREADY:
>>> +     case EISCONN: // Fall through
>> 
>> Don’t do this Fall through. It is actually not a fall through per se. This is just a statement with two case labels. That is perfectly normal and no compiler should complain. And frankly no C-programmer should be confused if this was intentional or not.
> Corrected in v3.
>> 
>>> +             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_CNCR_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_ABORT_BY_REMOTE;
>>> +     case ECONNABORTED:
>>> +             return BTD_ERR_BREDR_CONN_ABORT_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_ABORT_BY_REMOTE;
>>> +     case ECONNABORTED:
>>> +             return BTD_ERR_LE_CONN_ABORT_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..74d433aca 100644
>>> --- a/src/error.h
>>> +++ b/src/error.h
>>> @@ -24,22 +24,74 @@
>>> */
>>> 
>>> #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().
>>> + */
>>> +#define BTD_ERR_BREDR_CONN_ALREADY_CONNECTED 0x0001
>>> +#define BTD_ERR_BREDR_CONN_PAGE_TIMEOUT              0x0002
>>> +#define BTD_ERR_BREDR_CONN_PROFILE_UNAVAILABLE       0x0003
>>> +#define BTD_ERR_BREDR_CONN_SDP_SEARCH                0x0004
>>> +#define BTD_ERR_BREDR_CONN_CREATE_SOCKET     0x0005
>>> +#define BTD_ERR_BREDR_CONN_INVALID_ARGUMENTS 0x0006
>>> +#define BTD_ERR_BREDR_CONN_ADAPTER_NOT_POWERED       0x0007
>>> +#define BTD_ERR_BREDR_CONN_NOT_SUPPORTED     0x0008
>>> +#define BTD_ERR_BREDR_CONN_BAD_SOCKET                0x0009
>>> +#define BTD_ERR_BREDR_CONN_MEMORY_ALLOC              0x000A
>>> +#define BTD_ERR_BREDR_CONN_BUSY                      0x000B
>>> +#define BTD_ERR_BREDR_CONN_CNCR_CONNECT_LIMIT        0x000C
>>> +#define BTD_ERR_BREDR_CONN_TIMEOUT           0x000D
>>> +#define BTD_ERR_BREDR_CONN_REFUSED           0x000E
>>> +#define BTD_ERR_BREDR_CONN_ABORT_BY_REMOTE   0x000F
>>> +#define BTD_ERR_BREDR_CONN_ABORT_BY_LOCAL    0x0010
>>> +#define BTD_ERR_BREDR_CONN_PROTO_ERROR               0x0011
>>> +#define BTD_ERR_BREDR_CONN_CANCELED          0x0012
>>> +#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().
>>> + */
>>> +#define BTD_ERR_LE_CONN_INVALID_ARGUMENTS    0x0101
>>> +#define BTD_ERR_LE_CONN_ADAPTER_NOT_POWERED  0x0102
>>> +#define BTD_ERR_LE_CONN_NOT_SUPPORTED                0x0103
>>> +#define BTD_ERR_LE_CONN_ALREADY_CONNECTED    0x0104
>>> +#define BTD_ERR_LE_CONN_BAD_SOCKET           0x0105
>>> +#define BTD_ERR_LE_CONN_MEMORY_ALLOC         0x0106
>>> +#define BTD_ERR_LE_CONN_BUSY                 0x0107
>>> +#define BTD_ERR_LE_CONN_REFUSED                      0x0108
>>> +#define BTD_ERR_LE_CONN_CREATE_SOCKET                0x0109
>>> +#define BTD_ERR_LE_CONN_TIMEOUT                      0x010A
>>> +#define BTD_ERR_LE_CONN_SYNC_CONNECT_LIMIT   0x010B
>>> +#define BTD_ERR_LE_CONN_ABORT_BY_REMOTE              0x010C
>>> +#define BTD_ERR_LE_CONN_ABORT_BY_LOCAL               0x010D
>>> +#define BTD_ERR_LE_CONN_PROTO_ERROR          0x010E
>>> +#define BTD_ERR_LE_CONN_GATT_BROWSE          0x010F
>>> +#define BTD_ERR_LE_CONN_UNKNOWN                      0x0110
>>> +
>> 
>> What is the intention to split BR/EDR and LE here. You do know up-front what connection type you are. Trying to figure out from the error code what connection you have been trying to establish is plain wrong.
> In fact the up-front connection type is not necessarily known. In the
> case of dual-mode devices, such as Bose QC35, a D-Bus client can issue
> Connect(), and it depends on the timing of connection request (adv
> usually arrive first compared to inquiry result), it can be either
> BR/EDR or LE link being established. Another aspect of this is the
> metrics collection, where knowing transport can be handy. For
> instance, we can associate the certain error to particular use cases
> at application layer, and that can help targeting the bottleneck to
> tackle.

Then we need to find a different way to convey the transport chosen. Doing this by error message is a bad idea.

>> 
>> The description is that you want to know exactly where the connection failed. And I think that can be established independent from the transport.
> Indeed the intention is to know where it failed exactly. However, as
> mentioned above, transport information is also an important piece of
> information to know.

We need to find a different way to inform about which transport failed (or better which was chosen in the first place).

>> 
>> In addition, I don’t like the 0x00?? vs 0x01?? reservation of any number. That always goes bad at some point in the future.
> As replied above, having a code attached instead of a string
> description makes it easier for a D-Bus client to interpret and map to
> corresponding handlers, but I am happy to explore other options as
> well.

You are creating namespaces within a 16-bit integer. That is never a good idea. It always goes wrong at some point in the future and then we are stuck with an API.

Regards

Marcel

Re: [BlueZ PATCH v2 1/3] error: BR/EDR and LE connection failure reasons

[Miao-chen Chou]

+Alain Michaud

On Mon, Jul 19, 2021 at 12:57 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Miao-chen,
>
> >>> 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
> >>>
> >>> This also added error-code.txt to describe these error codes.
> >>>
> >>> Reviewed-by: Alain Michaud <alainm@chromium.org>
> >>> Reviewed-by: Howard Chung <howardchung@google.com>
> >>> ---
> >>>
> >>> Changes in v2:
> >>> - Add error-code.txt
> >>> - Remove BtdError from return string
> >>>
> >>> doc/error-code.txt | 266 +++++++++++++++++++++++++++++++++++++++++++++
> >>> src/error.c        | 111 +++++++++++++++++++
> >>> src/error.h        |  52 +++++++++
> >>> 3 files changed, 429 insertions(+)
> >>> create mode 100644 doc/error-code.txt
> >>
> >> please split documentation and code changes into separate patches.
> >>
> >>
> >>>
> >>> diff --git a/doc/error-code.txt b/doc/error-code.txt
> >>> new file mode 100644
> >>> index 000000000..e91324855
> >>> --- /dev/null
> >>> +++ b/doc/error-code.txt
> >>> @@ -0,0 +1,266 @@
> >>> +D-Bus Method Return Error Codes
> >>> +===============================
> >>> +
> >>> +The motivation of having detailed error codes is to provide context-based
> >>> +failure reasons along with D-Bus method return so that D-Bus clients can
> >>> +build metrics and optimize their application based on these failure reasons.
> >>> +For instance, a client can build retry mechanism for a connection failure or
> >>> +improve the bottleneck of use scenario based on actionable metrics.
> >>> +
> >>> +These error codes are context-based but not necessarily tied to interface or
> >>> +method calls. For instance, if a pairing request failed due to connection
> >>> +failure, connection error would be attached to the method return of Pair().
> >>> +
> >>> +BR/EDR connection already connected
> >>> +===================================
> >>> +     code:   0x0001
> >>> +     errno:  EALREADY, EISCONN
> >>
> >> I would rather see connnection-already-connected instead of 0x0001 in the D-Bus error message.
> > Having a code attached instead of a string description makes it easier
> > for a D-Bus client to interpret and map to corresponding handlers IMO.
> > For instead, a client can simplily use the code in a switch case to
> > perform retry or error reporting.
>
> this argument has been made since forever. However D-Bus is heavily string based and we also have our API heavily string and descriptive. So I rather keep it that way.
>
> >>
> >>> +
> >>> +Either the profile is already connected or ACL connection is in place.
> >>> +
> >>> +BR/EDR connection page timeout
> >>> +==============================
> >>> +     code:   0x0002
> >>> +     errno:  EHOSTDOWN
> >>> +
> >>> +Failed due to page timeout.
> >>> +
> >>> +BR/EDR connection profile unavailable
> >>> +=====================================
> >>> +     code:   0x0003
> >>> +     errno:  ENOPROTOOPT
> >>> +
> >>> +Failed to find connectable services or the target service.
> >>> +
> >>> +BR/EDR connection SDP search
> >>> +============================
> >>> +     code:   0x0004
> >>> +     errno:  none
> >>> +
> >>> +Failed to complete the SDP search.
> >>> +
> >>> +BR/EDR connection create socket
> >>> +===============================
> >>> +     code:   0x0005
> >>> +     errno:  EIO
> >>> +
> >>> +Failed to create or connect to BT IO socket. This can also indicate hardware
> >>> +failure in the controller.
> >>> +
> >>> +BR/EDR connection invalid arguments
> >>> +===================================
> >>> +     code:   0x0006
> >>> +     errno:  EHOSTUNREACH
> >>> +
> >>> +Failed due to invalid arguments.
> >>> +
> >>> +BR/EDR connection not powered
> >>> +=============================
> >>> +     code:   0x0007
> >>> +     errno:  EHOSTUNREACH
> >>> +
> >>> +Failed due to adapter not powered.
> >>> +
> >>> +BR/EDR connection not supported
> >>> +===============================
> >>> +     code:   0x0008
> >>> +     errno:  EOPNOTSUPP, EPROTONOSUPPORT
> >>> +
> >>> +Failed due to unsupported state transition of L2CAP channel or other features
> >>> +either by the local host or the remote.
> >>> +
> >>> +BR/EDR connection bad socket
> >>> +============================
> >>> +     code:   0x0009
> >>> +     errno:  EBADFD
> >>> +
> >>> +Failed due to the socket is in bad state.
> >>> +
> >>> +BR/EDR connection memory allocation
> >>> +===================================
> >>> +     code:   0x000A
> >>> +     errno:  EBADFD
> >>> +
> >>> +Failed to allocate memory in either host stack or controller.
> >>
> >> If this happens, then the code is wrong. Should be an ENOMEM.
> > My mistake, this should be ENOMEM. Corrected in v3.
> >>
> >>> +
> >>> +BR/EDR connection busy
> >>> +======================
> >>> +     code:   0x000B
> >>> +     errno:  EBUSY
> >>> +
> >>> +Failed due to other ongoing operations, such as pairing, busy L2CAP channel or
> >>> +the operation disallowed by the controller.
> >>> +
> >>> +BR/EDR connection concurrent connection limit
> >>> +=============================================
> >>> +     code:   0x000C
> >>> +     errno:  EMLINK
> >>> +
> >>> +Failed due to reaching the concurrent connection limit to a device.
> >>> +
> >>> +BR/EDR connection timeout
> >>> +=========================
> >>> +     code:   0x000D
> >>> +     errno:  ETIMEDOUT
> >>> +
> >>> +Failed due to connection timeout
> >>> +
> >>> +BR/EDR connection refused
> >>> +=========================
> >>> +     code:   0x000E
> >>> +     errno:  ECONNREFUSED
> >>> +
> >>> +Refused by the remote device due to limited resource, security reason or
> >>> +unacceptable address type.
> >>> +
> >>> +BR/EDR connection aborted by remote
> >>> +===================================
> >>> +     code:   0x000F
> >>> +     errno:  ECONNRESET
> >>> +
> >>> +Terminated by the remote device due to limited resource or power off.
> >>> +
> >>> +BR/EDR connection aborted by local
> >>> +==================================
> >>> +     code:   0x0010
> >>> +     errno:  ECONNABORTED
> >>> +
> >>> +Aborted by the local host.
> >>> +
> >>> +BR/EDR connection protocol error
> >>> +================================
> >>> +     code:   0x0011
> >>> +     errno:  EPROTO
> >>> +
> >>> +Failed due to LMP protocol error.
> >>> +
> >>> +BR/EDR connection canceled
> >>> +==========================
> >>> +     code:   0x0012
> >>> +     errno:  none
> >>> +
> >>> +Failed due to cancellation caused by adapter drop, unexpected device drop, or
> >>> +incoming disconnection request before connection request is completed.
> >>> +
> >>> +BR/EDR connection unknown error
> >>> +===============================
> >>> +     code:   0x0013
> >>> +     errno:  ENOSYS
> >>> +
> >>> +Failed due to unknown reason.
> >>> +
> >>> +LE connection invalid arguments
> >>> +===============================
> >>> +     code:   0x0101
> >>> +     errno:  EINVAL
> >>> +
> >>> +Failed due to invalid arguments.
> >>> +
> >>> +LE connection not powered
> >>> +=========================
> >>> +     code:   0x0102
> >>> +     errno:  EHOSTUNREACH
> >>> +
> >>> +Failed due to adapter not powered.
> >>> +
> >>> +LE connection not supported
> >>> +===========================
> >>> +     code:   0x0103
> >>> +     errno:  EOPNOTSUPP, EPROTONOSUPPORT
> >>> +
> >>> +Failed due to unsupported state transition of L2CAP channel or other features
> >>> +(e.g. LE features) either by the local host or the remote.
> >>> +
> >>> +LE connection already connected
> >>> +===============================
> >>> +     code:   0x0104
> >>> +     errno: EALREADY, EISCONN
> >>> +
> >>> +Either the BT IO is already connected or LE link connection in place.
> >>> +
> >>> +LE connection bad socket
> >>> +========================
> >>> +     code:   0x0105
> >>> +     errno: EBADFD
> >>> +
> >>> +Failed due to the socket is in bad state.
> >>> +
> >>> +LE connection memory allocation
> >>> +===============================
> >>> +     code:   0x0106
> >>> +     errno: ENOMEM
> >>> +
> >>> +Failed to allocate memory in either host stack or controller.
> >>> +
> >>> +LE connection busy
> >>> +==================
> >>> +     code:   0x0107
> >>> +     errno:  EBUSY
> >>> +
> >>> +Failed due to other ongoing operations, such as pairing, connecting, busy
> >>> +L2CAP channel or the operation disallowed by the controller.
> >>> +
> >>> +LE connection refused
> >>> +=====================
> >>> +     code:   0x0108
> >>> +     errno:  ECONNREFUSED
> >>> +
> >>> +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.
> >>> +
> >>> +LE connection create socket
> >>> +===========================
> >>> +     code:   0x0109
> >>> +     errno:  EIO
> >>> +
> >>> +Failed to create or connect to BT IO socket. This can also indicate hardware
> >>> +failure in the controller.
> >>> +
> >>> +LE connection timeout
> >>> +=====================
> >>> +     code:   0x010A
> >>> +     errno:  ETIMEDOUT
> >>> +
> >>> +Failed due to connection timeout
> >>> +
> >>> +LE connection concurrent connection limit
> >>> +=========================================
> >>> +     code:   0x010B
> >>> +     errno:  EMLINK
> >>> +
> >>> +Failed due to reaching the synchronous connection limit to a device.
> >>> +
> >>> +LE connection abort by remote
> >>> +=============================
> >>> +     code:   0x010C
> >>> +     errno:  ECONNRESET
> >>> +
> >>> +Aborted by the remote device due to limited resource or power off.
> >>> +
> >>> +LE connection abort by local
> >>> +============================
> >>> +     code:   0x010D
> >>> +     errno:  ECONNABORTED
> >>> +
> >>> +Aborted by the local host.
> >>> +
> >>> +LE connection link layer protocol error
> >>> +=======================================
> >>> +     code:   0x010E
> >>> +     errno:  EPROTO
> >>> +
> >>> +Failed due to link layer protocol error.
> >>> +
> >>> +LE connection GATT browsing
> >>> +===========================
> >>> +     code:   0x010F
> >>> +     errno:  none
> >>> +
> >>> +Failed to complete the GATT browsing.
> >>> +
> >>> +LE connection unknown error
> >>> +===========================
> >>> +     code:   0x0110
> >>> +     errno:  ENOSYS
> >>> +
> >>> + Failed due to unknown reason.
> >>> diff --git a/src/error.c b/src/error.c
> >>> index 89517075e..73602c4bf 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,12 @@ DBusMessage *btd_error_invalid_args_str(DBusMessage *msg, const char *str)
> >>>                                      "%s", str);
> >>> }
> >>>
> >>> +DBusMessage *btd_error_invalid_args_err(DBusMessage *msg, uint16_t err)
> >>> +{
> >>> +     return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
> >>> +                                     "0x%04X", err);
> >>> +}
> >>> +
> >>> DBusMessage *btd_error_busy(DBusMessage *msg)
> >>> {
> >>>      return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
> >>> @@ -79,12 +86,24 @@ DBusMessage *btd_error_in_progress(DBusMessage *msg)
> >>>                                      "In Progress");
> >>> }
> >>>
> >>> +DBusMessage *btd_error_in_progress_err(DBusMessage *msg, uint16_t err)
> >>> +{
> >>> +     return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress", "0x%04X",
> >>> +                                     err);
> >>> +}
> >>> +
> >>> 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)
> >>> +{
> >>> +     return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
> >>> +                                     "0x%04X", err);
> >>> +}
> >>> +
> >>> DBusMessage *btd_error_does_not_exist(DBusMessage *msg)
> >>> {
> >>>      return g_dbus_create_error(msg, ERROR_INTERFACE ".DoesNotExist",
> >>> @@ -121,8 +140,100 @@ DBusMessage *btd_error_not_ready(DBusMessage *msg)
> >>>                                      "Resource Not Ready");
> >>> }
> >>>
> >>> +DBusMessage *btd_error_not_ready_err(DBusMessage *msg, uint16_t err)
> >>> +{
> >>> +     return g_dbus_create_error(msg, ERROR_INTERFACE ".NotReady", "0x%04X",
> >>> +                                     err);
> >>> +}
> >>> +
> >>> 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)
> >>> +{
> >>> +     return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed", "0x%04X",
> >>> +                                     err);
> >>> +}
> >>> +
> >>> +uint16_t btd_error_bredr_conn_from_errno(int errno_code)
> >>> +{
> >>> +     switch (-errno_code) {
> >>> +     case EALREADY:
> >>> +     case EISCONN: // Fall through
> >>
> >> Don’t do this Fall through. It is actually not a fall through per se. This is just a statement with two case labels. That is perfectly normal and no compiler should complain. And frankly no C-programmer should be confused if this was intentional or not.
> > Corrected in v3.
> >>
> >>> +             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_CNCR_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_ABORT_BY_REMOTE;
> >>> +     case ECONNABORTED:
> >>> +             return BTD_ERR_BREDR_CONN_ABORT_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_ABORT_BY_REMOTE;
> >>> +     case ECONNABORTED:
> >>> +             return BTD_ERR_LE_CONN_ABORT_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..74d433aca 100644
> >>> --- a/src/error.h
> >>> +++ b/src/error.h
> >>> @@ -24,22 +24,74 @@
> >>> */
> >>>
> >>> #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().
> >>> + */
> >>> +#define BTD_ERR_BREDR_CONN_ALREADY_CONNECTED 0x0001
> >>> +#define BTD_ERR_BREDR_CONN_PAGE_TIMEOUT              0x0002
> >>> +#define BTD_ERR_BREDR_CONN_PROFILE_UNAVAILABLE       0x0003
> >>> +#define BTD_ERR_BREDR_CONN_SDP_SEARCH                0x0004
> >>> +#define BTD_ERR_BREDR_CONN_CREATE_SOCKET     0x0005
> >>> +#define BTD_ERR_BREDR_CONN_INVALID_ARGUMENTS 0x0006
> >>> +#define BTD_ERR_BREDR_CONN_ADAPTER_NOT_POWERED       0x0007
> >>> +#define BTD_ERR_BREDR_CONN_NOT_SUPPORTED     0x0008
> >>> +#define BTD_ERR_BREDR_CONN_BAD_SOCKET                0x0009
> >>> +#define BTD_ERR_BREDR_CONN_MEMORY_ALLOC              0x000A
> >>> +#define BTD_ERR_BREDR_CONN_BUSY                      0x000B
> >>> +#define BTD_ERR_BREDR_CONN_CNCR_CONNECT_LIMIT        0x000C
> >>> +#define BTD_ERR_BREDR_CONN_TIMEOUT           0x000D
> >>> +#define BTD_ERR_BREDR_CONN_REFUSED           0x000E
> >>> +#define BTD_ERR_BREDR_CONN_ABORT_BY_REMOTE   0x000F
> >>> +#define BTD_ERR_BREDR_CONN_ABORT_BY_LOCAL    0x0010
> >>> +#define BTD_ERR_BREDR_CONN_PROTO_ERROR               0x0011
> >>> +#define BTD_ERR_BREDR_CONN_CANCELED          0x0012
> >>> +#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().
> >>> + */
> >>> +#define BTD_ERR_LE_CONN_INVALID_ARGUMENTS    0x0101
> >>> +#define BTD_ERR_LE_CONN_ADAPTER_NOT_POWERED  0x0102
> >>> +#define BTD_ERR_LE_CONN_NOT_SUPPORTED                0x0103
> >>> +#define BTD_ERR_LE_CONN_ALREADY_CONNECTED    0x0104
> >>> +#define BTD_ERR_LE_CONN_BAD_SOCKET           0x0105
> >>> +#define BTD_ERR_LE_CONN_MEMORY_ALLOC         0x0106
> >>> +#define BTD_ERR_LE_CONN_BUSY                 0x0107
> >>> +#define BTD_ERR_LE_CONN_REFUSED                      0x0108
> >>> +#define BTD_ERR_LE_CONN_CREATE_SOCKET                0x0109
> >>> +#define BTD_ERR_LE_CONN_TIMEOUT                      0x010A
> >>> +#define BTD_ERR_LE_CONN_SYNC_CONNECT_LIMIT   0x010B
> >>> +#define BTD_ERR_LE_CONN_ABORT_BY_REMOTE              0x010C
> >>> +#define BTD_ERR_LE_CONN_ABORT_BY_LOCAL               0x010D
> >>> +#define BTD_ERR_LE_CONN_PROTO_ERROR          0x010E
> >>> +#define BTD_ERR_LE_CONN_GATT_BROWSE          0x010F
> >>> +#define BTD_ERR_LE_CONN_UNKNOWN                      0x0110
> >>> +
> >>
> >> What is the intention to split BR/EDR and LE here. You do know up-front what connection type you are. Trying to figure out from the error code what connection you have been trying to establish is plain wrong.
> > In fact the up-front connection type is not necessarily known. In the
> > case of dual-mode devices, such as Bose QC35, a D-Bus client can issue
> > Connect(), and it depends on the timing of connection request (adv
> > usually arrive first compared to inquiry result), it can be either
> > BR/EDR or LE link being established. Another aspect of this is the
> > metrics collection, where knowing transport can be handy. For
> > instance, we can associate the certain error to particular use cases
> > at application layer, and that can help targeting the bottleneck to
> > tackle.
>
> Then we need to find a different way to convey the transport chosen. Doing this by error message is a bad idea.
>
> >>
> >> The description is that you want to know exactly where the connection failed. And I think that can be established independent from the transport.
> > Indeed the intention is to know where it failed exactly. However, as
> > mentioned above, transport information is also an important piece of
> > information to know.
>
> We need to find a different way to inform about which transport failed (or better which was chosen in the first place).
>
> >>
> >> In addition, I don’t like the 0x00?? vs 0x01?? reservation of any number. That always goes bad at some point in the future.
> > As replied above, having a code attached instead of a string
> > description makes it easier for a D-Bus client to interpret and map to
> > corresponding handlers, but I am happy to explore other options as
> > well.
>
> You are creating namespaces within a 16-bit integer. That is never a good idea. It always goes wrong at some point in the future and then we are stuck with an API.
>
> Regards
>
> Marcel
>

Re: [BlueZ PATCH v2 1/3] error: BR/EDR and LE connection failure reasons

[Alain Michaud]

Hi Marcel,


On Tue, Jul 20, 2021 at 4:57 PM Miao-chen Chou <mcchou@chromium.org> wrote:
>
> +Alain Michaud
>
> On Mon, Jul 19, 2021 at 12:57 AM Marcel Holtmann <marcel@holtmann.org> wrote:
> >
> > Hi Miao-chen,
> >
> > >>> 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
> > >>>
> > >>> This also added error-code.txt to describe these error codes.
> > >>>
> > >>> Reviewed-by: Alain Michaud <alainm@chromium.org>
> > >>> Reviewed-by: Howard Chung <howardchung@google.com>
> > >>> ---
> > >>>
> > >>> Changes in v2:
> > >>> - Add error-code.txt
> > >>> - Remove BtdError from return string
> > >>>
> > >>> doc/error-code.txt | 266 +++++++++++++++++++++++++++++++++++++++++++++
> > >>> src/error.c        | 111 +++++++++++++++++++
> > >>> src/error.h        |  52 +++++++++
> > >>> 3 files changed, 429 insertions(+)
> > >>> create mode 100644 doc/error-code.txt
> > >>
> > >> please split documentation and code changes into separate patches.
> > >>
> > >>
> > >>>
> > >>> diff --git a/doc/error-code.txt b/doc/error-code.txt
> > >>> new file mode 100644
> > >>> index 000000000..e91324855
> > >>> --- /dev/null
> > >>> +++ b/doc/error-code.txt
> > >>> @@ -0,0 +1,266 @@
> > >>> +D-Bus Method Return Error Codes
> > >>> +===============================
> > >>> +
> > >>> +The motivation of having detailed error codes is to provide context-based
> > >>> +failure reasons along with D-Bus method return so that D-Bus clients can
> > >>> +build metrics and optimize their application based on these failure reasons.
> > >>> +For instance, a client can build retry mechanism for a connection failure or
> > >>> +improve the bottleneck of use scenario based on actionable metrics.
> > >>> +
> > >>> +These error codes are context-based but not necessarily tied to interface or
> > >>> +method calls. For instance, if a pairing request failed due to connection
> > >>> +failure, connection error would be attached to the method return of Pair().
> > >>> +
> > >>> +BR/EDR connection already connected
> > >>> +===================================
> > >>> +     code:   0x0001
> > >>> +     errno:  EALREADY, EISCONN
> > >>
> > >> I would rather see connnection-already-connected instead of 0x0001 in the D-Bus error message.
> > > Having a code attached instead of a string description makes it easier
> > > for a D-Bus client to interpret and map to corresponding handlers IMO.
> > > For instead, a client can simplily use the code in a switch case to
> > > perform retry or error reporting.
> >
> > this argument has been made since forever. However D-Bus is heavily string based and we also have our API heavily string and descriptive. So I rather keep it that way.
> >
> > >>
> > >>> +
> > >>> +Either the profile is already connected or ACL connection is in place.
> > >>> +
> > >>> +BR/EDR connection page timeout
> > >>> +==============================
> > >>> +     code:   0x0002
> > >>> +     errno:  EHOSTDOWN
> > >>> +
> > >>> +Failed due to page timeout.
> > >>> +
> > >>> +BR/EDR connection profile unavailable
> > >>> +=====================================
> > >>> +     code:   0x0003
> > >>> +     errno:  ENOPROTOOPT
> > >>> +
> > >>> +Failed to find connectable services or the target service.
> > >>> +
> > >>> +BR/EDR connection SDP search
> > >>> +============================
> > >>> +     code:   0x0004
> > >>> +     errno:  none
> > >>> +
> > >>> +Failed to complete the SDP search.
> > >>> +
> > >>> +BR/EDR connection create socket
> > >>> +===============================
> > >>> +     code:   0x0005
> > >>> +     errno:  EIO
> > >>> +
> > >>> +Failed to create or connect to BT IO socket. This can also indicate hardware
> > >>> +failure in the controller.
> > >>> +
> > >>> +BR/EDR connection invalid arguments
> > >>> +===================================
> > >>> +     code:   0x0006
> > >>> +     errno:  EHOSTUNREACH
> > >>> +
> > >>> +Failed due to invalid arguments.
> > >>> +
> > >>> +BR/EDR connection not powered
> > >>> +=============================
> > >>> +     code:   0x0007
> > >>> +     errno:  EHOSTUNREACH
> > >>> +
> > >>> +Failed due to adapter not powered.
> > >>> +
> > >>> +BR/EDR connection not supported
> > >>> +===============================
> > >>> +     code:   0x0008
> > >>> +     errno:  EOPNOTSUPP, EPROTONOSUPPORT
> > >>> +
> > >>> +Failed due to unsupported state transition of L2CAP channel or other features
> > >>> +either by the local host or the remote.
> > >>> +
> > >>> +BR/EDR connection bad socket
> > >>> +============================
> > >>> +     code:   0x0009
> > >>> +     errno:  EBADFD
> > >>> +
> > >>> +Failed due to the socket is in bad state.
> > >>> +
> > >>> +BR/EDR connection memory allocation
> > >>> +===================================
> > >>> +     code:   0x000A
> > >>> +     errno:  EBADFD
> > >>> +
> > >>> +Failed to allocate memory in either host stack or controller.
> > >>
> > >> If this happens, then the code is wrong. Should be an ENOMEM.
> > > My mistake, this should be ENOMEM. Corrected in v3.
> > >>
> > >>> +
> > >>> +BR/EDR connection busy
> > >>> +======================
> > >>> +     code:   0x000B
> > >>> +     errno:  EBUSY
> > >>> +
> > >>> +Failed due to other ongoing operations, such as pairing, busy L2CAP channel or
> > >>> +the operation disallowed by the controller.
> > >>> +
> > >>> +BR/EDR connection concurrent connection limit
> > >>> +=============================================
> > >>> +     code:   0x000C
> > >>> +     errno:  EMLINK
> > >>> +
> > >>> +Failed due to reaching the concurrent connection limit to a device.
> > >>> +
> > >>> +BR/EDR connection timeout
> > >>> +=========================
> > >>> +     code:   0x000D
> > >>> +     errno:  ETIMEDOUT
> > >>> +
> > >>> +Failed due to connection timeout
> > >>> +
> > >>> +BR/EDR connection refused
> > >>> +=========================
> > >>> +     code:   0x000E
> > >>> +     errno:  ECONNREFUSED
> > >>> +
> > >>> +Refused by the remote device due to limited resource, security reason or
> > >>> +unacceptable address type.
> > >>> +
> > >>> +BR/EDR connection aborted by remote
> > >>> +===================================
> > >>> +     code:   0x000F
> > >>> +     errno:  ECONNRESET
> > >>> +
> > >>> +Terminated by the remote device due to limited resource or power off.
> > >>> +
> > >>> +BR/EDR connection aborted by local
> > >>> +==================================
> > >>> +     code:   0x0010
> > >>> +     errno:  ECONNABORTED
> > >>> +
> > >>> +Aborted by the local host.
> > >>> +
> > >>> +BR/EDR connection protocol error
> > >>> +================================
> > >>> +     code:   0x0011
> > >>> +     errno:  EPROTO
> > >>> +
> > >>> +Failed due to LMP protocol error.
> > >>> +
> > >>> +BR/EDR connection canceled
> > >>> +==========================
> > >>> +     code:   0x0012
> > >>> +     errno:  none
> > >>> +
> > >>> +Failed due to cancellation caused by adapter drop, unexpected device drop, or
> > >>> +incoming disconnection request before connection request is completed.
> > >>> +
> > >>> +BR/EDR connection unknown error
> > >>> +===============================
> > >>> +     code:   0x0013
> > >>> +     errno:  ENOSYS
> > >>> +
> > >>> +Failed due to unknown reason.
> > >>> +
> > >>> +LE connection invalid arguments
> > >>> +===============================
> > >>> +     code:   0x0101
> > >>> +     errno:  EINVAL
> > >>> +
> > >>> +Failed due to invalid arguments.
> > >>> +
> > >>> +LE connection not powered
> > >>> +=========================
> > >>> +     code:   0x0102
> > >>> +     errno:  EHOSTUNREACH
> > >>> +
> > >>> +Failed due to adapter not powered.
> > >>> +
> > >>> +LE connection not supported
> > >>> +===========================
> > >>> +     code:   0x0103
> > >>> +     errno:  EOPNOTSUPP, EPROTONOSUPPORT
> > >>> +
> > >>> +Failed due to unsupported state transition of L2CAP channel or other features
> > >>> +(e.g. LE features) either by the local host or the remote.
> > >>> +
> > >>> +LE connection already connected
> > >>> +===============================
> > >>> +     code:   0x0104
> > >>> +     errno: EALREADY, EISCONN
> > >>> +
> > >>> +Either the BT IO is already connected or LE link connection in place.
> > >>> +
> > >>> +LE connection bad socket
> > >>> +========================
> > >>> +     code:   0x0105
> > >>> +     errno: EBADFD
> > >>> +
> > >>> +Failed due to the socket is in bad state.
> > >>> +
> > >>> +LE connection memory allocation
> > >>> +===============================
> > >>> +     code:   0x0106
> > >>> +     errno: ENOMEM
> > >>> +
> > >>> +Failed to allocate memory in either host stack or controller.
> > >>> +
> > >>> +LE connection busy
> > >>> +==================
> > >>> +     code:   0x0107
> > >>> +     errno:  EBUSY
> > >>> +
> > >>> +Failed due to other ongoing operations, such as pairing, connecting, busy
> > >>> +L2CAP channel or the operation disallowed by the controller.
> > >>> +
> > >>> +LE connection refused
> > >>> +=====================
> > >>> +     code:   0x0108
> > >>> +     errno:  ECONNREFUSED
> > >>> +
> > >>> +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.
> > >>> +
> > >>> +LE connection create socket
> > >>> +===========================
> > >>> +     code:   0x0109
> > >>> +     errno:  EIO
> > >>> +
> > >>> +Failed to create or connect to BT IO socket. This can also indicate hardware
> > >>> +failure in the controller.
> > >>> +
> > >>> +LE connection timeout
> > >>> +=====================
> > >>> +     code:   0x010A
> > >>> +     errno:  ETIMEDOUT
> > >>> +
> > >>> +Failed due to connection timeout
> > >>> +
> > >>> +LE connection concurrent connection limit
> > >>> +=========================================
> > >>> +     code:   0x010B
> > >>> +     errno:  EMLINK
> > >>> +
> > >>> +Failed due to reaching the synchronous connection limit to a device.
> > >>> +
> > >>> +LE connection abort by remote
> > >>> +=============================
> > >>> +     code:   0x010C
> > >>> +     errno:  ECONNRESET
> > >>> +
> > >>> +Aborted by the remote device due to limited resource or power off.
> > >>> +
> > >>> +LE connection abort by local
> > >>> +============================
> > >>> +     code:   0x010D
> > >>> +     errno:  ECONNABORTED
> > >>> +
> > >>> +Aborted by the local host.
> > >>> +
> > >>> +LE connection link layer protocol error
> > >>> +=======================================
> > >>> +     code:   0x010E
> > >>> +     errno:  EPROTO
> > >>> +
> > >>> +Failed due to link layer protocol error.
> > >>> +
> > >>> +LE connection GATT browsing
> > >>> +===========================
> > >>> +     code:   0x010F
> > >>> +     errno:  none
> > >>> +
> > >>> +Failed to complete the GATT browsing.
> > >>> +
> > >>> +LE connection unknown error
> > >>> +===========================
> > >>> +     code:   0x0110
> > >>> +     errno:  ENOSYS
> > >>> +
> > >>> + Failed due to unknown reason.
> > >>> diff --git a/src/error.c b/src/error.c
> > >>> index 89517075e..73602c4bf 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,12 @@ DBusMessage *btd_error_invalid_args_str(DBusMessage *msg, const char *str)
> > >>>                                      "%s", str);
> > >>> }
> > >>>
> > >>> +DBusMessage *btd_error_invalid_args_err(DBusMessage *msg, uint16_t err)
> > >>> +{
> > >>> +     return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
> > >>> +                                     "0x%04X", err);
> > >>> +}
> > >>> +
> > >>> DBusMessage *btd_error_busy(DBusMessage *msg)
> > >>> {
> > >>>      return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
> > >>> @@ -79,12 +86,24 @@ DBusMessage *btd_error_in_progress(DBusMessage *msg)
> > >>>                                      "In Progress");
> > >>> }
> > >>>
> > >>> +DBusMessage *btd_error_in_progress_err(DBusMessage *msg, uint16_t err)
> > >>> +{
> > >>> +     return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress", "0x%04X",
> > >>> +                                     err);
> > >>> +}
> > >>> +
> > >>> 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)
> > >>> +{
> > >>> +     return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
> > >>> +                                     "0x%04X", err);
> > >>> +}
> > >>> +
> > >>> DBusMessage *btd_error_does_not_exist(DBusMessage *msg)
> > >>> {
> > >>>      return g_dbus_create_error(msg, ERROR_INTERFACE ".DoesNotExist",
> > >>> @@ -121,8 +140,100 @@ DBusMessage *btd_error_not_ready(DBusMessage *msg)
> > >>>                                      "Resource Not Ready");
> > >>> }
> > >>>
> > >>> +DBusMessage *btd_error_not_ready_err(DBusMessage *msg, uint16_t err)
> > >>> +{
> > >>> +     return g_dbus_create_error(msg, ERROR_INTERFACE ".NotReady", "0x%04X",
> > >>> +                                     err);
> > >>> +}
> > >>> +
> > >>> 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)
> > >>> +{
> > >>> +     return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed", "0x%04X",
> > >>> +                                     err);
> > >>> +}
> > >>> +
> > >>> +uint16_t btd_error_bredr_conn_from_errno(int errno_code)
> > >>> +{
> > >>> +     switch (-errno_code) {
> > >>> +     case EALREADY:
> > >>> +     case EISCONN: // Fall through
> > >>
> > >> Don’t do this Fall through. It is actually not a fall through per se. This is just a statement with two case labels. That is perfectly normal and no compiler should complain. And frankly no C-programmer should be confused if this was intentional or not.
> > > Corrected in v3.
> > >>
> > >>> +             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_CNCR_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_ABORT_BY_REMOTE;
> > >>> +     case ECONNABORTED:
> > >>> +             return BTD_ERR_BREDR_CONN_ABORT_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_ABORT_BY_REMOTE;
> > >>> +     case ECONNABORTED:
> > >>> +             return BTD_ERR_LE_CONN_ABORT_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..74d433aca 100644
> > >>> --- a/src/error.h
> > >>> +++ b/src/error.h
> > >>> @@ -24,22 +24,74 @@
> > >>> */
> > >>>
> > >>> #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().
> > >>> + */
> > >>> +#define BTD_ERR_BREDR_CONN_ALREADY_CONNECTED 0x0001
> > >>> +#define BTD_ERR_BREDR_CONN_PAGE_TIMEOUT              0x0002
> > >>> +#define BTD_ERR_BREDR_CONN_PROFILE_UNAVAILABLE       0x0003
> > >>> +#define BTD_ERR_BREDR_CONN_SDP_SEARCH                0x0004
> > >>> +#define BTD_ERR_BREDR_CONN_CREATE_SOCKET     0x0005
> > >>> +#define BTD_ERR_BREDR_CONN_INVALID_ARGUMENTS 0x0006
> > >>> +#define BTD_ERR_BREDR_CONN_ADAPTER_NOT_POWERED       0x0007
> > >>> +#define BTD_ERR_BREDR_CONN_NOT_SUPPORTED     0x0008
> > >>> +#define BTD_ERR_BREDR_CONN_BAD_SOCKET                0x0009
> > >>> +#define BTD_ERR_BREDR_CONN_MEMORY_ALLOC              0x000A
> > >>> +#define BTD_ERR_BREDR_CONN_BUSY                      0x000B
> > >>> +#define BTD_ERR_BREDR_CONN_CNCR_CONNECT_LIMIT        0x000C
> > >>> +#define BTD_ERR_BREDR_CONN_TIMEOUT           0x000D
> > >>> +#define BTD_ERR_BREDR_CONN_REFUSED           0x000E
> > >>> +#define BTD_ERR_BREDR_CONN_ABORT_BY_REMOTE   0x000F
> > >>> +#define BTD_ERR_BREDR_CONN_ABORT_BY_LOCAL    0x0010
> > >>> +#define BTD_ERR_BREDR_CONN_PROTO_ERROR               0x0011
> > >>> +#define BTD_ERR_BREDR_CONN_CANCELED          0x0012
> > >>> +#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().
> > >>> + */
> > >>> +#define BTD_ERR_LE_CONN_INVALID_ARGUMENTS    0x0101
> > >>> +#define BTD_ERR_LE_CONN_ADAPTER_NOT_POWERED  0x0102
> > >>> +#define BTD_ERR_LE_CONN_NOT_SUPPORTED                0x0103
> > >>> +#define BTD_ERR_LE_CONN_ALREADY_CONNECTED    0x0104
> > >>> +#define BTD_ERR_LE_CONN_BAD_SOCKET           0x0105
> > >>> +#define BTD_ERR_LE_CONN_MEMORY_ALLOC         0x0106
> > >>> +#define BTD_ERR_LE_CONN_BUSY                 0x0107
> > >>> +#define BTD_ERR_LE_CONN_REFUSED                      0x0108
> > >>> +#define BTD_ERR_LE_CONN_CREATE_SOCKET                0x0109
> > >>> +#define BTD_ERR_LE_CONN_TIMEOUT                      0x010A
> > >>> +#define BTD_ERR_LE_CONN_SYNC_CONNECT_LIMIT   0x010B
> > >>> +#define BTD_ERR_LE_CONN_ABORT_BY_REMOTE              0x010C
> > >>> +#define BTD_ERR_LE_CONN_ABORT_BY_LOCAL               0x010D
> > >>> +#define BTD_ERR_LE_CONN_PROTO_ERROR          0x010E
> > >>> +#define BTD_ERR_LE_CONN_GATT_BROWSE          0x010F
> > >>> +#define BTD_ERR_LE_CONN_UNKNOWN                      0x0110
> > >>> +
> > >>
> > >> What is the intention to split BR/EDR and LE here. You do know up-front what connection type you are. Trying to figure out from the error code what connection you have been trying to establish is plain wrong.
> > > In fact the up-front connection type is not necessarily known. In the
> > > case of dual-mode devices, such as Bose QC35, a D-Bus client can issue
> > > Connect(), and it depends on the timing of connection request (adv
> > > usually arrive first compared to inquiry result), it can be either
> > > BR/EDR or LE link being established. Another aspect of this is the
> > > metrics collection, where knowing transport can be handy. For
> > > instance, we can associate the certain error to particular use cases
> > > at application layer, and that can help targeting the bottleneck to
> > > tackle.
> >
> > Then we need to find a different way to convey the transport chosen. Doing this by error message is a bad idea.
> >
> > >>
> > >> The description is that you want to know exactly where the connection failed. And I think that can be established independent from the transport.
> > > Indeed the intention is to know where it failed exactly. However, as
> > > mentioned above, transport information is also an important piece of
> > > information to know.
> >
> > We need to find a different way to inform about which transport failed (or better which was chosen in the first place).
I would love to hear your thoughts on an alternative.  Many of the
Apis are transport agnostic (Connect/Pair may end up connecting to
either available transports for dual mode devices), but yet the error
that results from them are not.  Errors from one transport doesn't
make sense for one and vice versa.  A platform wanting to leverage
telemetry and metrics to drive ecosystem improvements would ultimately
need to know the difference even if the applications may not need to
care.


> >
> > >>
> > >> In addition, I don’t like the 0x00?? vs 0x01?? reservation of any number. That always goes bad at some point in the future.
> > > As replied above, having a code attached instead of a string
> > > description makes it easier for a D-Bus client to interpret and map to
> > > corresponding handlers, but I am happy to explore other options as
> > > well.
> >
> > You are creating namespaces within a 16-bit integer. That is never a good idea. It always goes wrong at some point in the future and then we are stuck with an API.
> >
> > Regards
> >
> > Marcel
> >

Re: [BlueZ PATCH v2 1/3] error: BR/EDR and LE connection failure reasons

[Marcel Holtmann]

Hi Alain,

>>>>>> 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
>>>>>> 
>>>>>> This also added error-code.txt to describe these error codes.
>>>>>> 
>>>>>> Reviewed-by: Alain Michaud <alainm@chromium.org>
>>>>>> Reviewed-by: Howard Chung <howardchung@google.com>
>>>>>> ---
>>>>>> 
>>>>>> Changes in v2:
>>>>>> - Add error-code.txt
>>>>>> - Remove BtdError from return string
>>>>>> 
>>>>>> doc/error-code.txt | 266 +++++++++++++++++++++++++++++++++++++++++++++
>>>>>> src/error.c        | 111 +++++++++++++++++++
>>>>>> src/error.h        |  52 +++++++++
>>>>>> 3 files changed, 429 insertions(+)
>>>>>> create mode 100644 doc/error-code.txt
>>>>> 
>>>>> please split documentation and code changes into separate patches.
>>>>> 
>>>>> 
>>>>>> 
>>>>>> diff --git a/doc/error-code.txt b/doc/error-code.txt
>>>>>> new file mode 100644
>>>>>> index 000000000..e91324855
>>>>>> --- /dev/null
>>>>>> +++ b/doc/error-code.txt
>>>>>> @@ -0,0 +1,266 @@
>>>>>> +D-Bus Method Return Error Codes
>>>>>> +===============================
>>>>>> +
>>>>>> +The motivation of having detailed error codes is to provide context-based
>>>>>> +failure reasons along with D-Bus method return so that D-Bus clients can
>>>>>> +build metrics and optimize their application based on these failure reasons.
>>>>>> +For instance, a client can build retry mechanism for a connection failure or
>>>>>> +improve the bottleneck of use scenario based on actionable metrics.
>>>>>> +
>>>>>> +These error codes are context-based but not necessarily tied to interface or
>>>>>> +method calls. For instance, if a pairing request failed due to connection
>>>>>> +failure, connection error would be attached to the method return of Pair().
>>>>>> +
>>>>>> +BR/EDR connection already connected
>>>>>> +===================================
>>>>>> +     code:   0x0001
>>>>>> +     errno:  EALREADY, EISCONN
>>>>> 
>>>>> I would rather see connnection-already-connected instead of 0x0001 in the D-Bus error message.
>>>> Having a code attached instead of a string description makes it easier
>>>> for a D-Bus client to interpret and map to corresponding handlers IMO.
>>>> For instead, a client can simplily use the code in a switch case to
>>>> perform retry or error reporting.
>>> 
>>> this argument has been made since forever. However D-Bus is heavily string based and we also have our API heavily string and descriptive. So I rather keep it that way.
>>> 
>>>>> 
>>>>>> +
>>>>>> +Either the profile is already connected or ACL connection is in place.
>>>>>> +
>>>>>> +BR/EDR connection page timeout
>>>>>> +==============================
>>>>>> +     code:   0x0002
>>>>>> +     errno:  EHOSTDOWN
>>>>>> +
>>>>>> +Failed due to page timeout.
>>>>>> +
>>>>>> +BR/EDR connection profile unavailable
>>>>>> +=====================================
>>>>>> +     code:   0x0003
>>>>>> +     errno:  ENOPROTOOPT
>>>>>> +
>>>>>> +Failed to find connectable services or the target service.
>>>>>> +
>>>>>> +BR/EDR connection SDP search
>>>>>> +============================
>>>>>> +     code:   0x0004
>>>>>> +     errno:  none
>>>>>> +
>>>>>> +Failed to complete the SDP search.
>>>>>> +
>>>>>> +BR/EDR connection create socket
>>>>>> +===============================
>>>>>> +     code:   0x0005
>>>>>> +     errno:  EIO
>>>>>> +
>>>>>> +Failed to create or connect to BT IO socket. This can also indicate hardware
>>>>>> +failure in the controller.
>>>>>> +
>>>>>> +BR/EDR connection invalid arguments
>>>>>> +===================================
>>>>>> +     code:   0x0006
>>>>>> +     errno:  EHOSTUNREACH
>>>>>> +
>>>>>> +Failed due to invalid arguments.
>>>>>> +
>>>>>> +BR/EDR connection not powered
>>>>>> +=============================
>>>>>> +     code:   0x0007
>>>>>> +     errno:  EHOSTUNREACH
>>>>>> +
>>>>>> +Failed due to adapter not powered.
>>>>>> +
>>>>>> +BR/EDR connection not supported
>>>>>> +===============================
>>>>>> +     code:   0x0008
>>>>>> +     errno:  EOPNOTSUPP, EPROTONOSUPPORT
>>>>>> +
>>>>>> +Failed due to unsupported state transition of L2CAP channel or other features
>>>>>> +either by the local host or the remote.
>>>>>> +
>>>>>> +BR/EDR connection bad socket
>>>>>> +============================
>>>>>> +     code:   0x0009
>>>>>> +     errno:  EBADFD
>>>>>> +
>>>>>> +Failed due to the socket is in bad state.
>>>>>> +
>>>>>> +BR/EDR connection memory allocation
>>>>>> +===================================
>>>>>> +     code:   0x000A
>>>>>> +     errno:  EBADFD
>>>>>> +
>>>>>> +Failed to allocate memory in either host stack or controller.
>>>>> 
>>>>> If this happens, then the code is wrong. Should be an ENOMEM.
>>>> My mistake, this should be ENOMEM. Corrected in v3.
>>>>> 
>>>>>> +
>>>>>> +BR/EDR connection busy
>>>>>> +======================
>>>>>> +     code:   0x000B
>>>>>> +     errno:  EBUSY
>>>>>> +
>>>>>> +Failed due to other ongoing operations, such as pairing, busy L2CAP channel or
>>>>>> +the operation disallowed by the controller.
>>>>>> +
>>>>>> +BR/EDR connection concurrent connection limit
>>>>>> +=============================================
>>>>>> +     code:   0x000C
>>>>>> +     errno:  EMLINK
>>>>>> +
>>>>>> +Failed due to reaching the concurrent connection limit to a device.
>>>>>> +
>>>>>> +BR/EDR connection timeout
>>>>>> +=========================
>>>>>> +     code:   0x000D
>>>>>> +     errno:  ETIMEDOUT
>>>>>> +
>>>>>> +Failed due to connection timeout
>>>>>> +
>>>>>> +BR/EDR connection refused
>>>>>> +=========================
>>>>>> +     code:   0x000E
>>>>>> +     errno:  ECONNREFUSED
>>>>>> +
>>>>>> +Refused by the remote device due to limited resource, security reason or
>>>>>> +unacceptable address type.
>>>>>> +
>>>>>> +BR/EDR connection aborted by remote
>>>>>> +===================================
>>>>>> +     code:   0x000F
>>>>>> +     errno:  ECONNRESET
>>>>>> +
>>>>>> +Terminated by the remote device due to limited resource or power off.
>>>>>> +
>>>>>> +BR/EDR connection aborted by local
>>>>>> +==================================
>>>>>> +     code:   0x0010
>>>>>> +     errno:  ECONNABORTED
>>>>>> +
>>>>>> +Aborted by the local host.
>>>>>> +
>>>>>> +BR/EDR connection protocol error
>>>>>> +================================
>>>>>> +     code:   0x0011
>>>>>> +     errno:  EPROTO
>>>>>> +
>>>>>> +Failed due to LMP protocol error.
>>>>>> +
>>>>>> +BR/EDR connection canceled
>>>>>> +==========================
>>>>>> +     code:   0x0012
>>>>>> +     errno:  none
>>>>>> +
>>>>>> +Failed due to cancellation caused by adapter drop, unexpected device drop, or
>>>>>> +incoming disconnection request before connection request is completed.
>>>>>> +
>>>>>> +BR/EDR connection unknown error
>>>>>> +===============================
>>>>>> +     code:   0x0013
>>>>>> +     errno:  ENOSYS
>>>>>> +
>>>>>> +Failed due to unknown reason.
>>>>>> +
>>>>>> +LE connection invalid arguments
>>>>>> +===============================
>>>>>> +     code:   0x0101
>>>>>> +     errno:  EINVAL
>>>>>> +
>>>>>> +Failed due to invalid arguments.
>>>>>> +
>>>>>> +LE connection not powered
>>>>>> +=========================
>>>>>> +     code:   0x0102
>>>>>> +     errno:  EHOSTUNREACH
>>>>>> +
>>>>>> +Failed due to adapter not powered.
>>>>>> +
>>>>>> +LE connection not supported
>>>>>> +===========================
>>>>>> +     code:   0x0103
>>>>>> +     errno:  EOPNOTSUPP, EPROTONOSUPPORT
>>>>>> +
>>>>>> +Failed due to unsupported state transition of L2CAP channel or other features
>>>>>> +(e.g. LE features) either by the local host or the remote.
>>>>>> +
>>>>>> +LE connection already connected
>>>>>> +===============================
>>>>>> +     code:   0x0104
>>>>>> +     errno: EALREADY, EISCONN
>>>>>> +
>>>>>> +Either the BT IO is already connected or LE link connection in place.
>>>>>> +
>>>>>> +LE connection bad socket
>>>>>> +========================
>>>>>> +     code:   0x0105
>>>>>> +     errno: EBADFD
>>>>>> +
>>>>>> +Failed due to the socket is in bad state.
>>>>>> +
>>>>>> +LE connection memory allocation
>>>>>> +===============================
>>>>>> +     code:   0x0106
>>>>>> +     errno: ENOMEM
>>>>>> +
>>>>>> +Failed to allocate memory in either host stack or controller.
>>>>>> +
>>>>>> +LE connection busy
>>>>>> +==================
>>>>>> +     code:   0x0107
>>>>>> +     errno:  EBUSY
>>>>>> +
>>>>>> +Failed due to other ongoing operations, such as pairing, connecting, busy
>>>>>> +L2CAP channel or the operation disallowed by the controller.
>>>>>> +
>>>>>> +LE connection refused
>>>>>> +=====================
>>>>>> +     code:   0x0108
>>>>>> +     errno:  ECONNREFUSED
>>>>>> +
>>>>>> +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.
>>>>>> +
>>>>>> +LE connection create socket
>>>>>> +===========================
>>>>>> +     code:   0x0109
>>>>>> +     errno:  EIO
>>>>>> +
>>>>>> +Failed to create or connect to BT IO socket. This can also indicate hardware
>>>>>> +failure in the controller.
>>>>>> +
>>>>>> +LE connection timeout
>>>>>> +=====================
>>>>>> +     code:   0x010A
>>>>>> +     errno:  ETIMEDOUT
>>>>>> +
>>>>>> +Failed due to connection timeout
>>>>>> +
>>>>>> +LE connection concurrent connection limit
>>>>>> +=========================================
>>>>>> +     code:   0x010B
>>>>>> +     errno:  EMLINK
>>>>>> +
>>>>>> +Failed due to reaching the synchronous connection limit to a device.
>>>>>> +
>>>>>> +LE connection abort by remote
>>>>>> +=============================
>>>>>> +     code:   0x010C
>>>>>> +     errno:  ECONNRESET
>>>>>> +
>>>>>> +Aborted by the remote device due to limited resource or power off.
>>>>>> +
>>>>>> +LE connection abort by local
>>>>>> +============================
>>>>>> +     code:   0x010D
>>>>>> +     errno:  ECONNABORTED
>>>>>> +
>>>>>> +Aborted by the local host.
>>>>>> +
>>>>>> +LE connection link layer protocol error
>>>>>> +=======================================
>>>>>> +     code:   0x010E
>>>>>> +     errno:  EPROTO
>>>>>> +
>>>>>> +Failed due to link layer protocol error.
>>>>>> +
>>>>>> +LE connection GATT browsing
>>>>>> +===========================
>>>>>> +     code:   0x010F
>>>>>> +     errno:  none
>>>>>> +
>>>>>> +Failed to complete the GATT browsing.
>>>>>> +
>>>>>> +LE connection unknown error
>>>>>> +===========================
>>>>>> +     code:   0x0110
>>>>>> +     errno:  ENOSYS
>>>>>> +
>>>>>> + Failed due to unknown reason.
>>>>>> diff --git a/src/error.c b/src/error.c
>>>>>> index 89517075e..73602c4bf 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,12 @@ DBusMessage *btd_error_invalid_args_str(DBusMessage *msg, const char *str)
>>>>>>                                     "%s", str);
>>>>>> }
>>>>>> 
>>>>>> +DBusMessage *btd_error_invalid_args_err(DBusMessage *msg, uint16_t err)
>>>>>> +{
>>>>>> +     return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
>>>>>> +                                     "0x%04X", err);
>>>>>> +}
>>>>>> +
>>>>>> DBusMessage *btd_error_busy(DBusMessage *msg)
>>>>>> {
>>>>>>     return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
>>>>>> @@ -79,12 +86,24 @@ DBusMessage *btd_error_in_progress(DBusMessage *msg)
>>>>>>                                     "In Progress");
>>>>>> }
>>>>>> 
>>>>>> +DBusMessage *btd_error_in_progress_err(DBusMessage *msg, uint16_t err)
>>>>>> +{
>>>>>> +     return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress", "0x%04X",
>>>>>> +                                     err);
>>>>>> +}
>>>>>> +
>>>>>> 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)
>>>>>> +{
>>>>>> +     return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
>>>>>> +                                     "0x%04X", err);
>>>>>> +}
>>>>>> +
>>>>>> DBusMessage *btd_error_does_not_exist(DBusMessage *msg)
>>>>>> {
>>>>>>     return g_dbus_create_error(msg, ERROR_INTERFACE ".DoesNotExist",
>>>>>> @@ -121,8 +140,100 @@ DBusMessage *btd_error_not_ready(DBusMessage *msg)
>>>>>>                                     "Resource Not Ready");
>>>>>> }
>>>>>> 
>>>>>> +DBusMessage *btd_error_not_ready_err(DBusMessage *msg, uint16_t err)
>>>>>> +{
>>>>>> +     return g_dbus_create_error(msg, ERROR_INTERFACE ".NotReady", "0x%04X",
>>>>>> +                                     err);
>>>>>> +}
>>>>>> +
>>>>>> 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)
>>>>>> +{
>>>>>> +     return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed", "0x%04X",
>>>>>> +                                     err);
>>>>>> +}
>>>>>> +
>>>>>> +uint16_t btd_error_bredr_conn_from_errno(int errno_code)
>>>>>> +{
>>>>>> +     switch (-errno_code) {
>>>>>> +     case EALREADY:
>>>>>> +     case EISCONN: // Fall through
>>>>> 
>>>>> Don’t do this Fall through. It is actually not a fall through per se. This is just a statement with two case labels. That is perfectly normal and no compiler should complain. And frankly no C-programmer should be confused if this was intentional or not.
>>>> Corrected in v3.
>>>>> 
>>>>>> +             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_CNCR_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_ABORT_BY_REMOTE;
>>>>>> +     case ECONNABORTED:
>>>>>> +             return BTD_ERR_BREDR_CONN_ABORT_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_ABORT_BY_REMOTE;
>>>>>> +     case ECONNABORTED:
>>>>>> +             return BTD_ERR_LE_CONN_ABORT_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..74d433aca 100644
>>>>>> --- a/src/error.h
>>>>>> +++ b/src/error.h
>>>>>> @@ -24,22 +24,74 @@
>>>>>> */
>>>>>> 
>>>>>> #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().
>>>>>> + */
>>>>>> +#define BTD_ERR_BREDR_CONN_ALREADY_CONNECTED 0x0001
>>>>>> +#define BTD_ERR_BREDR_CONN_PAGE_TIMEOUT              0x0002
>>>>>> +#define BTD_ERR_BREDR_CONN_PROFILE_UNAVAILABLE       0x0003
>>>>>> +#define BTD_ERR_BREDR_CONN_SDP_SEARCH                0x0004
>>>>>> +#define BTD_ERR_BREDR_CONN_CREATE_SOCKET     0x0005
>>>>>> +#define BTD_ERR_BREDR_CONN_INVALID_ARGUMENTS 0x0006
>>>>>> +#define BTD_ERR_BREDR_CONN_ADAPTER_NOT_POWERED       0x0007
>>>>>> +#define BTD_ERR_BREDR_CONN_NOT_SUPPORTED     0x0008
>>>>>> +#define BTD_ERR_BREDR_CONN_BAD_SOCKET                0x0009
>>>>>> +#define BTD_ERR_BREDR_CONN_MEMORY_ALLOC              0x000A
>>>>>> +#define BTD_ERR_BREDR_CONN_BUSY                      0x000B
>>>>>> +#define BTD_ERR_BREDR_CONN_CNCR_CONNECT_LIMIT        0x000C
>>>>>> +#define BTD_ERR_BREDR_CONN_TIMEOUT           0x000D
>>>>>> +#define BTD_ERR_BREDR_CONN_REFUSED           0x000E
>>>>>> +#define BTD_ERR_BREDR_CONN_ABORT_BY_REMOTE   0x000F
>>>>>> +#define BTD_ERR_BREDR_CONN_ABORT_BY_LOCAL    0x0010
>>>>>> +#define BTD_ERR_BREDR_CONN_PROTO_ERROR               0x0011
>>>>>> +#define BTD_ERR_BREDR_CONN_CANCELED          0x0012
>>>>>> +#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().
>>>>>> + */
>>>>>> +#define BTD_ERR_LE_CONN_INVALID_ARGUMENTS    0x0101
>>>>>> +#define BTD_ERR_LE_CONN_ADAPTER_NOT_POWERED  0x0102
>>>>>> +#define BTD_ERR_LE_CONN_NOT_SUPPORTED                0x0103
>>>>>> +#define BTD_ERR_LE_CONN_ALREADY_CONNECTED    0x0104
>>>>>> +#define BTD_ERR_LE_CONN_BAD_SOCKET           0x0105
>>>>>> +#define BTD_ERR_LE_CONN_MEMORY_ALLOC         0x0106
>>>>>> +#define BTD_ERR_LE_CONN_BUSY                 0x0107
>>>>>> +#define BTD_ERR_LE_CONN_REFUSED                      0x0108
>>>>>> +#define BTD_ERR_LE_CONN_CREATE_SOCKET                0x0109
>>>>>> +#define BTD_ERR_LE_CONN_TIMEOUT                      0x010A
>>>>>> +#define BTD_ERR_LE_CONN_SYNC_CONNECT_LIMIT   0x010B
>>>>>> +#define BTD_ERR_LE_CONN_ABORT_BY_REMOTE              0x010C
>>>>>> +#define BTD_ERR_LE_CONN_ABORT_BY_LOCAL               0x010D
>>>>>> +#define BTD_ERR_LE_CONN_PROTO_ERROR          0x010E
>>>>>> +#define BTD_ERR_LE_CONN_GATT_BROWSE          0x010F
>>>>>> +#define BTD_ERR_LE_CONN_UNKNOWN                      0x0110
>>>>>> +
>>>>> 
>>>>> What is the intention to split BR/EDR and LE here. You do know up-front what connection type you are. Trying to figure out from the error code what connection you have been trying to establish is plain wrong.
>>>> In fact the up-front connection type is not necessarily known. In the
>>>> case of dual-mode devices, such as Bose QC35, a D-Bus client can issue
>>>> Connect(), and it depends on the timing of connection request (adv
>>>> usually arrive first compared to inquiry result), it can be either
>>>> BR/EDR or LE link being established. Another aspect of this is the
>>>> metrics collection, where knowing transport can be handy. For
>>>> instance, we can associate the certain error to particular use cases
>>>> at application layer, and that can help targeting the bottleneck to
>>>> tackle.
>>> 
>>> Then we need to find a different way to convey the transport chosen. Doing this by error message is a bad idea.
>>> 
>>>>> 
>>>>> The description is that you want to know exactly where the connection failed. And I think that can be established independent from the transport.
>>>> Indeed the intention is to know where it failed exactly. However, as
>>>> mentioned above, transport information is also an important piece of
>>>> information to know.
>>> 
>>> We need to find a different way to inform about which transport failed (or better which was chosen in the first place).
> I would love to hear your thoughts on an alternative.  Many of the
> Apis are transport agnostic (Connect/Pair may end up connecting to
> either available transports for dual mode devices), but yet the error
> that results from them are not.  Errors from one transport doesn't
> make sense for one and vice versa.  A platform wanting to leverage
> telemetry and metrics to drive ecosystem improvements would ultimately
> need to know the difference even if the applications may not need to
> care.

and we might have made a mistake in the API design and should have given the caller more control. We need to review the API design and see if things have to change. Just glueing things on at the end makes me suspicious.

Regards

Marcel

Re: [BlueZ PATCH v2 1/3] error: BR/EDR and LE connection failure reasons

[Szymon Janc]

Hi,

On Friday, 23 July 2021 09:38:40 CEST Marcel Holtmann wrote:

> >>>>> What is the intention to split BR/EDR and LE here. You do know
> >>>>> up-front what connection type you are. Trying to figure out from the
> >>>>> error code what connection you have been trying to establish is plain
> >>>>> wrong.>>>> 
> >>>> In fact the up-front connection type is not necessarily known. In the
> >>>> case of dual-mode devices, such as Bose QC35, a D-Bus client can issue
> >>>> Connect(), and it depends on the timing of connection request (adv
> >>>> usually arrive first compared to inquiry result), it can be either
> >>>> BR/EDR or LE link being established. Another aspect of this is the
> >>>> metrics collection, where knowing transport can be handy. For
> >>>> instance, we can associate the certain error to particular use cases
> >>>> at application layer, and that can help targeting the bottleneck to
> >>>> tackle.
> >>> 
> >>> Then we need to find a different way to convey the transport chosen.
> >>> Doing this by error message is a bad idea.>>> 
> >>>>> The description is that you want to know exactly where the connection
> >>>>> failed. And I think that can be established independent from the
> >>>>> transport.>>>> 
> >>>> Indeed the intention is to know where it failed exactly. However, as
> >>>> mentioned above, transport information is also an important piece of
> >>>> information to know.
> >>> 
> >>> We need to find a different way to inform about which transport failed
> >>> (or better which was chosen in the first place).> 
> > I would love to hear your thoughts on an alternative.  Many of the
> > Apis are transport agnostic (Connect/Pair may end up connecting to
> > either available transports for dual mode devices), but yet the error
> > that results from them are not.  Errors from one transport doesn't
> > make sense for one and vice versa.  A platform wanting to leverage
> > telemetry and metrics to drive ecosystem improvements would ultimately
> > need to know the difference even if the applications may not need to
> > care.
> 
> and we might have made a mistake in the API design and should have given the
> caller more control. We need to review the API design and see if things
> have to change. Just glueing things on at the end makes me suspicious.

Some (5, wow!) years back I've loosely proposed split for org.bluez.Device API 
that was meant to handle some of the dual mode devices issues we've been 
seeing [1] [2]. 

We never got time to fully implement it (mostly due to hacking around device.c 
instead of properly splitting internal implementation into device_le.c and 
device_bredr.c) but got some very initial PoC running.

With new interfaces old Device1 could be simply super-set of two for legacy 
applications purposes.

Maybe such approach is worth re-considering? 


[1] https://marc.info/?l=linux-bluetooth&m=145710680912268&w=2
[2] https://marc.info/?l=linux-bluetooth&m=145973293118003&w=2


-- 
pozdrawiam
Szymon Janc

Re: [BlueZ PATCH v2 1/3] error: BR/EDR and LE connection failure reasons

[Alain Michaud]

On Fri, Jul 23, 2021 at 4:20 AM Szymon Janc <szymon.janc@codecoup.pl> wrote:
>
> Hi,
>
> On Friday, 23 July 2021 09:38:40 CEST Marcel Holtmann wrote:
>
> > >>>>> What is the intention to split BR/EDR and LE here. You do know
> > >>>>> up-front what connection type you are. Trying to figure out from the
> > >>>>> error code what connection you have been trying to establish is plain
> > >>>>> wrong.>>>>
> > >>>> In fact the up-front connection type is not necessarily known. In the
> > >>>> case of dual-mode devices, such as Bose QC35, a D-Bus client can issue
> > >>>> Connect(), and it depends on the timing of connection request (adv
> > >>>> usually arrive first compared to inquiry result), it can be either
> > >>>> BR/EDR or LE link being established. Another aspect of this is the
> > >>>> metrics collection, where knowing transport can be handy. For
> > >>>> instance, we can associate the certain error to particular use cases
> > >>>> at application layer, and that can help targeting the bottleneck to
> > >>>> tackle.
> > >>>
> > >>> Then we need to find a different way to convey the transport chosen.
> > >>> Doing this by error message is a bad idea.>>>
> > >>>>> The description is that you want to know exactly where the connection
> > >>>>> failed. And I think that can be established independent from the
> > >>>>> transport.>>>>
> > >>>> Indeed the intention is to know where it failed exactly. However, as
> > >>>> mentioned above, transport information is also an important piece of
> > >>>> information to know.
> > >>>
> > >>> We need to find a different way to inform about which transport failed
> > >>> (or better which was chosen in the first place).>
> > > I would love to hear your thoughts on an alternative.  Many of the
> > > Apis are transport agnostic (Connect/Pair may end up connecting to
> > > either available transports for dual mode devices), but yet the error
> > > that results from them are not.  Errors from one transport doesn't
> > > make sense for one and vice versa.  A platform wanting to leverage
> > > telemetry and metrics to drive ecosystem improvements would ultimately
> > > need to know the difference even if the applications may not need to
> > > care.
> >
> > and we might have made a mistake in the API design and should have given the
> > caller more control. We need to review the API design and see if things
> > have to change. Just glueing things on at the end makes me suspicious.
>
> Some (5, wow!) years back I've loosely proposed split for org.bluez.Device API
> that was meant to handle some of the dual mode devices issues we've been
> seeing [1] [2].
>
> We never got time to fully implement it (mostly due to hacking around device.c
> instead of properly splitting internal implementation into device_le.c and
> device_bredr.c) but got some very initial PoC running.
>
> With new interfaces old Device1 could be simply super-set of two for legacy
> applications purposes.
>
> Maybe such approach is worth re-considering?
>
>
> [1] https://marc.info/?l=linux-bluetooth&m=145710680912268&w=2
> [2] https://marc.info/?l=linux-bluetooth&m=145973293118003&w=2
Definitely worth reconsidering.  We've recently introduced a ConnectLE
Api as a stop gap measure for something very similar.  I'd love to see
the equivalent of a ConnectClassic / ConnectLE distinction.

However, I still believe the "Generic" Connect serves its purpose as
you alluded to above.  Even if it could be built using layers, I still
believe there is value from a telemetry POV to understand the errors
from the field better and there, the distinction between the specific
transport matter.  Just like today, I suspect many applications will
continue to use the generic "Connect" as to not replicate the logic it
ultimately implements for dealing with dual mode devices, yet
results/metrics would be important.

The team strives to improve interoperability at a large scale in the
ecosystem, I believe these data point distinctions are a critical
enabler to get better insights into the specific errors customers are
seeing.  A counter proposal that would be acceptable to you, achieves
the objectives and that the team can implement would be a nice next
step.

Thanks!
Alain
>
>
> --
> pozdrawiam
> Szymon Janc
>
>

Re: [BlueZ PATCH v2 1/3] error: BR/EDR and LE connection failure reasons

[Miao-chen Chou]

Hi All,

Thanks Alain and Szymon for chiming in.

Doing a through-out API revamp is definitely worth considering.
However, I'd like to point out the fact that these error messages will
likely be applicable even after API revamp, since the majority of the
error codes are based on errno returned by the kernel.
Another comment was on the format of error return. I'd like to hear
your feedback on how we present errors in D-Bus return. The two
options that we have on the table is (1) string form of error
description (2) string form of uint16 error code.  (1) is more
extendable while (2) is easier to be processed by the client.

Regards,
Miao

On Fri, Jul 23, 2021 at 6:20 AM Alain Michaud <alainmichaud@google.com> wrote:
>
> On Fri, Jul 23, 2021 at 4:20 AM Szymon Janc <szymon.janc@codecoup.pl> wrote:
> >
> > Hi,
> >
> > On Friday, 23 July 2021 09:38:40 CEST Marcel Holtmann wrote:
> >
> > > >>>>> What is the intention to split BR/EDR and LE here. You do know
> > > >>>>> up-front what connection type you are. Trying to figure out from the
> > > >>>>> error code what connection you have been trying to establish is plain
> > > >>>>> wrong.>>>>
> > > >>>> In fact the up-front connection type is not necessarily known. In the
> > > >>>> case of dual-mode devices, such as Bose QC35, a D-Bus client can issue
> > > >>>> Connect(), and it depends on the timing of connection request (adv
> > > >>>> usually arrive first compared to inquiry result), it can be either
> > > >>>> BR/EDR or LE link being established. Another aspect of this is the
> > > >>>> metrics collection, where knowing transport can be handy. For
> > > >>>> instance, we can associate the certain error to particular use cases
> > > >>>> at application layer, and that can help targeting the bottleneck to
> > > >>>> tackle.
> > > >>>
> > > >>> Then we need to find a different way to convey the transport chosen.
> > > >>> Doing this by error message is a bad idea.>>>
> > > >>>>> The description is that you want to know exactly where the connection
> > > >>>>> failed. And I think that can be established independent from the
> > > >>>>> transport.>>>>
> > > >>>> Indeed the intention is to know where it failed exactly. However, as
> > > >>>> mentioned above, transport information is also an important piece of
> > > >>>> information to know.
> > > >>>
> > > >>> We need to find a different way to inform about which transport failed
> > > >>> (or better which was chosen in the first place).>
> > > > I would love to hear your thoughts on an alternative.  Many of the
> > > > Apis are transport agnostic (Connect/Pair may end up connecting to
> > > > either available transports for dual mode devices), but yet the error
> > > > that results from them are not.  Errors from one transport doesn't
> > > > make sense for one and vice versa.  A platform wanting to leverage
> > > > telemetry and metrics to drive ecosystem improvements would ultimately
> > > > need to know the difference even if the applications may not need to
> > > > care.
> > >
> > > and we might have made a mistake in the API design and should have given the
> > > caller more control. We need to review the API design and see if things
> > > have to change. Just glueing things on at the end makes me suspicious.
> >
> > Some (5, wow!) years back I've loosely proposed split for org.bluez.Device API
> > that was meant to handle some of the dual mode devices issues we've been
> > seeing [1] [2].
> >
> > We never got time to fully implement it (mostly due to hacking around device.c
> > instead of properly splitting internal implementation into device_le.c and
> > device_bredr.c) but got some very initial PoC running.
> >
> > With new interfaces old Device1 could be simply super-set of two for legacy
> > applications purposes.
> >
> > Maybe such approach is worth re-considering?
> >
> >
> > [1] https://marc.info/?l=linux-bluetooth&m=145710680912268&w=2
> > [2] https://marc.info/?l=linux-bluetooth&m=145973293118003&w=2
> Definitely worth reconsidering.  We've recently introduced a ConnectLE
> Api as a stop gap measure for something very similar.  I'd love to see
> the equivalent of a ConnectClassic / ConnectLE distinction.
>
> However, I still believe the "Generic" Connect serves its purpose as
> you alluded to above.  Even if it could be built using layers, I still
> believe there is value from a telemetry POV to understand the errors
> from the field better and there, the distinction between the specific
> transport matter.  Just like today, I suspect many applications will
> continue to use the generic "Connect" as to not replicate the logic it
> ultimately implements for dealing with dual mode devices, yet
> results/metrics would be important.
>
> The team strives to improve interoperability at a large scale in the
> ecosystem, I believe these data point distinctions are a critical
> enabler to get better insights into the specific errors customers are
> seeing.  A counter proposal that would be acceptable to you, achieves
> the objectives and that the team can implement would be a nice next
> step.
>
> Thanks!
> Alain
> >
> >
> > --
> > pozdrawiam
> > Szymon Janc
> >
> >

RE: Detailed error code

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 22971 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=507451

---Test result---

Test Summary:
CheckPatch                    FAIL      1.35 seconds
GitLint                       PASS      0.39 seconds
Prep - Setup ELL              PASS      45.57 seconds
Build - Prep                  PASS      0.13 seconds
Build - Configure             PASS      7.62 seconds
Build - Make                  FAIL      161.23 seconds
Make Check                    FAIL      0.53 seconds
Make Distcheck                FAIL      149.84 seconds
Build w/ext ELL - Configure   PASS      7.74 seconds
Build w/ext ELL - Make        FAIL      153.16 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
Output:
error: BR/EDR and LE connection failure reasons
ERROR:TRAILING_WHITESPACE: trailing whitespace
#51: FILE: doc/error-code.txt:28:
+BR/EDR connection profile unavailable $

ERROR:TRAILING_WHITESPACE: trailing whitespace
#270: FILE: doc/error-code.txt:247:
+LE connection link layer protocol error $

WARNING:PREFER_FALLTHROUGH: Prefer 'fallthrough;' over fallthrough comment
#366: FILE: src/error.c:152:
+	case EISCONN: // Fall through

WARNING:PREFER_FALLTHROUGH: Prefer 'fallthrough;' over fallthrough comment
#379: FILE: src/error.c:165:
+	case EPROTONOSUPPORT: // Fall through

WARNING:PREFER_FALLTHROUGH: Prefer 'fallthrough;' over fallthrough comment
#412: FILE: src/error.c:198:
+	case EPROTONOSUPPORT: // Fall through

WARNING:PREFER_FALLTHROUGH: Prefer 'fallthrough;' over fallthrough comment
#415: FILE: src/error.c:201:
+	case EISCONN: // Fall through

- total: 2 errors, 4 warnings, 483 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

NOTE: Whitespace errors detected.
      You may wish to use scripts/cleanpatch or scripts/cleanfile

"[PATCH] error: BR/EDR and LE connection failure reasons" has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - PASS
Desc: Run gitlint with rule in .gitlint

##############################
Test: Prep - Setup ELL - PASS
Desc: Clone, build, and install ELL

##############################
Test: Build - Prep - PASS
Desc: Prepare environment for build

##############################
Test: Build - Configure - PASS
Desc: Configure the BlueZ source tree

##############################
Test: Build - Make - FAIL
Desc: Build the BlueZ source tree
Output:
src/error.c: In function ‘btd_error_bredr_conn_from_errno’:
src/error.c:151:7: error: ‘EALREADY’ undeclared (first use in this function)
  151 |  case EALREADY:
      |       ^~~~~~~~
src/error.c:151:7: note: each undeclared identifier is reported only once for each function it appears in
src/error.c:152:7: error: ‘EISCONN’ undeclared (first use in this function)
  152 |  case EISCONN: // Fall through
      |       ^~~~~~~
src/error.c:154:7: error: ‘EHOSTDOWN’ undeclared (first use in this function)
  154 |  case EHOSTDOWN:
      |       ^~~~~~~~~
src/error.c:156:7: error: ‘ENOPROTOOPT’ undeclared (first use in this function)
  156 |  case ENOPROTOOPT:
      |       ^~~~~~~~~~~
src/error.c:158:7: error: ‘EIO’ undeclared (first use in this function)
  158 |  case EIO:
      |       ^~~
src/error.c:160:7: error: ‘EINVAL’ undeclared (first use in this function)
  160 |  case EINVAL:
      |       ^~~~~~
src/error.c:162:7: error: ‘EHOSTUNREACH’ undeclared (first use in this function)
  162 |  case EHOSTUNREACH:
      |       ^~~~~~~~~~~~
src/error.c:164:7: error: ‘EOPNOTSUPP’ undeclared (first use in this function)
  164 |  case EOPNOTSUPP:
      |       ^~~~~~~~~~
src/error.c:165:7: error: ‘EPROTONOSUPPORT’ undeclared (first use in this function)
  165 |  case EPROTONOSUPPORT: // Fall through
      |       ^~~~~~~~~~~~~~~
src/error.c:167:7: error: ‘EBADFD’ undeclared (first use in this function)
  167 |  case EBADFD:
      |       ^~~~~~
src/error.c:169:7: error: ‘ENOMEM’ undeclared (first use in this function)
  169 |  case ENOMEM:
      |       ^~~~~~
src/error.c:171:7: error: ‘EBUSY’ undeclared (first use in this function)
  171 |  case EBUSY:
      |       ^~~~~
src/error.c:173:7: error: ‘EMLINK’ undeclared (first use in this function)
  173 |  case EMLINK:
      |       ^~~~~~
src/error.c:175:7: error: ‘ETIMEDOUT’ undeclared (first use in this function)
  175 |  case ETIMEDOUT:
      |       ^~~~~~~~~
src/error.c:177:7: error: ‘ECONNREFUSED’ undeclared (first use in this function)
  177 |  case ECONNREFUSED:
      |       ^~~~~~~~~~~~
src/error.c:179:7: error: ‘ECONNRESET’ undeclared (first use in this function)
  179 |  case ECONNRESET:
      |       ^~~~~~~~~~
src/error.c:181:7: error: ‘ECONNABORTED’ undeclared (first use in this function)
  181 |  case ECONNABORTED:
      |       ^~~~~~~~~~~~
src/error.c:183:7: error: ‘EPROTO’ undeclared (first use in this function)
  183 |  case EPROTO:
      |       ^~~~~~
src/error.c: In function ‘btd_error_le_conn_from_errno’:
src/error.c:193:7: error: ‘EINVAL’ undeclared (first use in this function)
  193 |  case EINVAL:
      |       ^~~~~~
src/error.c:195:7: error: ‘EHOSTUNREACH’ undeclared (first use in this function)
  195 |  case EHOSTUNREACH:
      |       ^~~~~~~~~~~~
src/error.c:197:7: error: ‘EOPNOTSUPP’ undeclared (first use in this function)
  197 |  case EOPNOTSUPP:
      |       ^~~~~~~~~~
src/error.c:198:7: error: ‘EPROTONOSUPPORT’ undeclared (first use in this function)
  198 |  case EPROTONOSUPPORT: // Fall through
      |       ^~~~~~~~~~~~~~~
src/error.c:200:7: error: ‘EALREADY’ undeclared (first use in this function)
  200 |  case EALREADY:
      |       ^~~~~~~~
src/error.c:201:7: error: ‘EISCONN’ undeclared (first use in this function)
  201 |  case EISCONN: // Fall through
      |       ^~~~~~~
src/error.c:203:7: error: ‘EBADFD’ undeclared (first use in this function)
  203 |  case EBADFD:
      |       ^~~~~~
src/error.c:205:7: error: ‘ENOMEM’ undeclared (first use in this function)
  205 |  case ENOMEM:
      |       ^~~~~~
src/error.c:207:7: error: ‘EBUSY’ undeclared (first use in this function)
  207 |  case EBUSY:
      |       ^~~~~
src/error.c:209:7: error: ‘ECONNREFUSED’ undeclared (first use in this function)
  209 |  case ECONNREFUSED:
      |       ^~~~~~~~~~~~
src/error.c:211:7: error: ‘EIO’ undeclared (first use in this function)
  211 |  case EIO:
      |       ^~~
src/error.c:213:7: error: ‘ETIMEDOUT’ undeclared (first use in this function)
  213 |  case ETIMEDOUT:
      |       ^~~~~~~~~
src/error.c:215:7: error: ‘EMLINK’ undeclared (first use in this function)
  215 |  case EMLINK:
      |       ^~~~~~
src/error.c:217:7: error: ‘ECONNRESET’ undeclared (first use in this function)
  217 |  case ECONNRESET:
      |       ^~~~~~~~~~
src/error.c:219:7: error: ‘ECONNABORTED’ undeclared (first use in this function)
  219 |  case ECONNABORTED:
      |       ^~~~~~~~~~~~
src/error.c:221:7: error: ‘EPROTO’ undeclared (first use in this function)
  221 |  case EPROTO:
      |       ^~~~~~
make[1]: *** [Makefile:9301: src/bluetoothd-error.o] Error 1
make: *** [Makefile:4134: all] Error 2


##############################
Test: Make Check - FAIL
Desc: Run 'make check'
Output:
src/error.c: In function ‘btd_error_bredr_conn_from_errno’:
src/error.c:151:7: error: ‘EALREADY’ undeclared (first use in this function)
  151 |  case EALREADY:
      |       ^~~~~~~~
src/error.c:151:7: note: each undeclared identifier is reported only once for each function it appears in
src/error.c:152:7: error: ‘EISCONN’ undeclared (first use in this function)
  152 |  case EISCONN: // Fall through
      |       ^~~~~~~
src/error.c:154:7: error: ‘EHOSTDOWN’ undeclared (first use in this function)
  154 |  case EHOSTDOWN:
      |       ^~~~~~~~~
src/error.c:156:7: error: ‘ENOPROTOOPT’ undeclared (first use in this function)
  156 |  case ENOPROTOOPT:
      |       ^~~~~~~~~~~
src/error.c:158:7: error: ‘EIO’ undeclared (first use in this function)
  158 |  case EIO:
      |       ^~~
src/error.c:160:7: error: ‘EINVAL’ undeclared (first use in this function)
  160 |  case EINVAL:
      |       ^~~~~~
src/error.c:162:7: error: ‘EHOSTUNREACH’ undeclared (first use in this function)
  162 |  case EHOSTUNREACH:
      |       ^~~~~~~~~~~~
src/error.c:164:7: error: ‘EOPNOTSUPP’ undeclared (first use in this function)
  164 |  case EOPNOTSUPP:
      |       ^~~~~~~~~~
src/error.c:165:7: error: ‘EPROTONOSUPPORT’ undeclared (first use in this function)
  165 |  case EPROTONOSUPPORT: // Fall through
      |       ^~~~~~~~~~~~~~~
src/error.c:167:7: error: ‘EBADFD’ undeclared (first use in this function)
  167 |  case EBADFD:
      |       ^~~~~~
src/error.c:169:7: error: ‘ENOMEM’ undeclared (first use in this function)
  169 |  case ENOMEM:
      |       ^~~~~~
src/error.c:171:7: error: ‘EBUSY’ undeclared (first use in this function)
  171 |  case EBUSY:
      |       ^~~~~
src/error.c:173:7: error: ‘EMLINK’ undeclared (first use in this function)
  173 |  case EMLINK:
      |       ^~~~~~
src/error.c:175:7: error: ‘ETIMEDOUT’ undeclared (first use in this function)
  175 |  case ETIMEDOUT:
      |       ^~~~~~~~~
src/error.c:177:7: error: ‘ECONNREFUSED’ undeclared (first use in this function)
  177 |  case ECONNREFUSED:
      |       ^~~~~~~~~~~~
src/error.c:179:7: error: ‘ECONNRESET’ undeclared (first use in this function)
  179 |  case ECONNRESET:
      |       ^~~~~~~~~~
src/error.c:181:7: error: ‘ECONNABORTED’ undeclared (first use in this function)
  181 |  case ECONNABORTED:
      |       ^~~~~~~~~~~~
src/error.c:183:7: error: ‘EPROTO’ undeclared (first use in this function)
  183 |  case EPROTO:
      |       ^~~~~~
src/error.c: In function ‘btd_error_le_conn_from_errno’:
src/error.c:193:7: error: ‘EINVAL’ undeclared (first use in this function)
  193 |  case EINVAL:
      |       ^~~~~~
src/error.c:195:7: error: ‘EHOSTUNREACH’ undeclared (first use in this function)
  195 |  case EHOSTUNREACH:
      |       ^~~~~~~~~~~~
src/error.c:197:7: error: ‘EOPNOTSUPP’ undeclared (first use in this function)
  197 |  case EOPNOTSUPP:
      |       ^~~~~~~~~~
src/error.c:198:7: error: ‘EPROTONOSUPPORT’ undeclared (first use in this function)
  198 |  case EPROTONOSUPPORT: // Fall through
      |       ^~~~~~~~~~~~~~~
src/error.c:200:7: error: ‘EALREADY’ undeclared (first use in this function)
  200 |  case EALREADY:
      |       ^~~~~~~~
src/error.c:201:7: error: ‘EISCONN’ undeclared (first use in this function)
  201 |  case EISCONN: // Fall through
      |       ^~~~~~~
src/error.c:203:7: error: ‘EBADFD’ undeclared (first use in this function)
  203 |  case EBADFD:
      |       ^~~~~~
src/error.c:205:7: error: ‘ENOMEM’ undeclared (first use in this function)
  205 |  case ENOMEM:
      |       ^~~~~~
src/error.c:207:7: error: ‘EBUSY’ undeclared (first use in this function)
  207 |  case EBUSY:
      |       ^~~~~
src/error.c:209:7: error: ‘ECONNREFUSED’ undeclared (first use in this function)
  209 |  case ECONNREFUSED:
      |       ^~~~~~~~~~~~
src/error.c:211:7: error: ‘EIO’ undeclared (first use in this function)
  211 |  case EIO:
      |       ^~~
src/error.c:213:7: error: ‘ETIMEDOUT’ undeclared (first use in this function)
  213 |  case ETIMEDOUT:
      |       ^~~~~~~~~
src/error.c:215:7: error: ‘EMLINK’ undeclared (first use in this function)
  215 |  case EMLINK:
      |       ^~~~~~
src/error.c:217:7: error: ‘ECONNRESET’ undeclared (first use in this function)
  217 |  case ECONNRESET:
      |       ^~~~~~~~~~
src/error.c:219:7: error: ‘ECONNABORTED’ undeclared (first use in this function)
  219 |  case ECONNABORTED:
      |       ^~~~~~~~~~~~
src/error.c:221:7: error: ‘EPROTO’ undeclared (first use in this function)
  221 |  case EPROTO:
      |       ^~~~~~
make[1]: *** [Makefile:9301: src/bluetoothd-error.o] Error 1
make: *** [Makefile:10406: check] Error 2


##############################
Test: Make Distcheck - FAIL
Desc: Run distcheck to check the distribution
Output:
../../src/error.c: In function ‘btd_error_bredr_conn_from_errno’:
../../src/error.c:151:7: error: ‘EALREADY’ undeclared (first use in this function)
  151 |  case EALREADY:
      |       ^~~~~~~~
../../src/error.c:151:7: note: each undeclared identifier is reported only once for each function it appears in
../../src/error.c:152:7: error: ‘EISCONN’ undeclared (first use in this function)
  152 |  case EISCONN: // Fall through
      |       ^~~~~~~
../../src/error.c:154:7: error: ‘EHOSTDOWN’ undeclared (first use in this function)
  154 |  case EHOSTDOWN:
      |       ^~~~~~~~~
../../src/error.c:156:7: error: ‘ENOPROTOOPT’ undeclared (first use in this function)
  156 |  case ENOPROTOOPT:
      |       ^~~~~~~~~~~
../../src/error.c:158:7: error: ‘EIO’ undeclared (first use in this function)
  158 |  case EIO:
      |       ^~~
../../src/error.c:160:7: error: ‘EINVAL’ undeclared (first use in this function)
  160 |  case EINVAL:
      |       ^~~~~~
../../src/error.c:162:7: error: ‘EHOSTUNREACH’ undeclared (first use in this function)
  162 |  case EHOSTUNREACH:
      |       ^~~~~~~~~~~~
../../src/error.c:164:7: error: ‘EOPNOTSUPP’ undeclared (first use in this function)
  164 |  case EOPNOTSUPP:
      |       ^~~~~~~~~~
../../src/error.c:165:7: error: ‘EPROTONOSUPPORT’ undeclared (first use in this function)
  165 |  case EPROTONOSUPPORT: // Fall through
      |       ^~~~~~~~~~~~~~~
../../src/error.c:167:7: error: ‘EBADFD’ undeclared (first use in this function)
  167 |  case EBADFD:
      |       ^~~~~~
../../src/error.c:169:7: error: ‘ENOMEM’ undeclared (first use in this function)
  169 |  case ENOMEM:
      |       ^~~~~~
../../src/error.c:171:7: error: ‘EBUSY’ undeclared (first use in this function)
  171 |  case EBUSY:
      |       ^~~~~
../../src/error.c:173:7: error: ‘EMLINK’ undeclared (first use in this function)
  173 |  case EMLINK:
      |       ^~~~~~
../../src/error.c:175:7: error: ‘ETIMEDOUT’ undeclared (first use in this function)
  175 |  case ETIMEDOUT:
      |       ^~~~~~~~~
../../src/error.c:177:7: error: ‘ECONNREFUSED’ undeclared (first use in this function)
  177 |  case ECONNREFUSED:
      |       ^~~~~~~~~~~~
../../src/error.c:179:7: error: ‘ECONNRESET’ undeclared (first use in this function)
  179 |  case ECONNRESET:
      |       ^~~~~~~~~~
../../src/error.c:181:7: error: ‘ECONNABORTED’ undeclared (first use in this function)
  181 |  case ECONNABORTED:
      |       ^~~~~~~~~~~~
../../src/error.c:183:7: error: ‘EPROTO’ undeclared (first use in this function)
  183 |  case EPROTO:
      |       ^~~~~~
../../src/error.c: In function ‘btd_error_le_conn_from_errno’:
../../src/error.c:193:7: error: ‘EINVAL’ undeclared (first use in this function)
  193 |  case EINVAL:
      |       ^~~~~~
../../src/error.c:195:7: error: ‘EHOSTUNREACH’ undeclared (first use in this function)
  195 |  case EHOSTUNREACH:
      |       ^~~~~~~~~~~~
../../src/error.c:197:7: error: ‘EOPNOTSUPP’ undeclared (first use in this function)
  197 |  case EOPNOTSUPP:
      |       ^~~~~~~~~~
../../src/error.c:198:7: error: ‘EPROTONOSUPPORT’ undeclared (first use in this function)
  198 |  case EPROTONOSUPPORT: // Fall through
      |       ^~~~~~~~~~~~~~~
../../src/error.c:200:7: error: ‘EALREADY’ undeclared (first use in this function)
  200 |  case EALREADY:
      |       ^~~~~~~~
../../src/error.c:201:7: error: ‘EISCONN’ undeclared (first use in this function)
  201 |  case EISCONN: // Fall through
      |       ^~~~~~~
../../src/error.c:203:7: error: ‘EBADFD’ undeclared (first use in this function)
  203 |  case EBADFD:
      |       ^~~~~~
../../src/error.c:205:7: error: ‘ENOMEM’ undeclared (first use in this function)
  205 |  case ENOMEM:
      |       ^~~~~~
../../src/error.c:207:7: error: ‘EBUSY’ undeclared (first use in this function)
  207 |  case EBUSY:
      |       ^~~~~
../../src/error.c:209:7: error: ‘ECONNREFUSED’ undeclared (first use in this function)
  209 |  case ECONNREFUSED:
      |       ^~~~~~~~~~~~
../../src/error.c:211:7: error: ‘EIO’ undeclared (first use in this function)
  211 |  case EIO:
      |       ^~~
../../src/error.c:213:7: error: ‘ETIMEDOUT’ undeclared (first use in this function)
  213 |  case ETIMEDOUT:
      |       ^~~~~~~~~
../../src/error.c:215:7: error: ‘EMLINK’ undeclared (first use in this function)
  215 |  case EMLINK:
      |       ^~~~~~
../../src/error.c:217:7: error: ‘ECONNRESET’ undeclared (first use in this function)
  217 |  case ECONNRESET:
      |       ^~~~~~~~~~
../../src/error.c:219:7: error: ‘ECONNABORTED’ undeclared (first use in this function)
  219 |  case ECONNABORTED:
      |       ^~~~~~~~~~~~
../../src/error.c:221:7: error: ‘EPROTO’ undeclared (first use in this function)
  221 |  case EPROTO:
      |       ^~~~~~
make[2]: *** [Makefile:9301: src/bluetoothd-error.o] Error 1
make[1]: *** [Makefile:4134: all] Error 2
make: *** [Makefile:10327: distcheck] Error 1


##############################
Test: Build w/ext ELL - Configure - PASS
Desc: Configure BlueZ source with '--enable-external-ell' configuration

##############################
Test: Build w/ext ELL - Make - FAIL
Desc: Build BlueZ source with '--enable-external-ell' configuration
Output:
src/error.c: In function ‘btd_error_bredr_conn_from_errno’:
src/error.c:151:7: error: ‘EALREADY’ undeclared (first use in this function)
  151 |  case EALREADY:
      |       ^~~~~~~~
src/error.c:151:7: note: each undeclared identifier is reported only once for each function it appears in
src/error.c:152:7: error: ‘EISCONN’ undeclared (first use in this function)
  152 |  case EISCONN: // Fall through
      |       ^~~~~~~
src/error.c:154:7: error: ‘EHOSTDOWN’ undeclared (first use in this function)
  154 |  case EHOSTDOWN:
      |       ^~~~~~~~~
src/error.c:156:7: error: ‘ENOPROTOOPT’ undeclared (first use in this function)
  156 |  case ENOPROTOOPT:
      |       ^~~~~~~~~~~
src/error.c:158:7: error: ‘EIO’ undeclared (first use in this function)
  158 |  case EIO:
      |       ^~~
src/error.c:160:7: error: ‘EINVAL’ undeclared (first use in this function)
  160 |  case EINVAL:
      |       ^~~~~~
src/error.c:162:7: error: ‘EHOSTUNREACH’ undeclared (first use in this function)
  162 |  case EHOSTUNREACH:
      |       ^~~~~~~~~~~~
src/error.c:164:7: error: ‘EOPNOTSUPP’ undeclared (first use in this function)
  164 |  case EOPNOTSUPP:
      |       ^~~~~~~~~~
src/error.c:165:7: error: ‘EPROTONOSUPPORT’ undeclared (first use in this function)
  165 |  case EPROTONOSUPPORT: // Fall through
      |       ^~~~~~~~~~~~~~~
src/error.c:167:7: error: ‘EBADFD’ undeclared (first use in this function)
  167 |  case EBADFD:
      |       ^~~~~~
src/error.c:169:7: error: ‘ENOMEM’ undeclared (first use in this function)
  169 |  case ENOMEM:
      |       ^~~~~~
src/error.c:171:7: error: ‘EBUSY’ undeclared (first use in this function)
  171 |  case EBUSY:
      |       ^~~~~
src/error.c:173:7: error: ‘EMLINK’ undeclared (first use in this function)
  173 |  case EMLINK:
      |       ^~~~~~
src/error.c:175:7: error: ‘ETIMEDOUT’ undeclared (first use in this function)
  175 |  case ETIMEDOUT:
      |       ^~~~~~~~~
src/error.c:177:7: error: ‘ECONNREFUSED’ undeclared (first use in this function)
  177 |  case ECONNREFUSED:
      |       ^~~~~~~~~~~~
src/error.c:179:7: error: ‘ECONNRESET’ undeclared (first use in this function)
  179 |  case ECONNRESET:
      |       ^~~~~~~~~~
src/error.c:181:7: error: ‘ECONNABORTED’ undeclared (first use in this function)
  181 |  case ECONNABORTED:
      |       ^~~~~~~~~~~~
src/error.c:183:7: error: ‘EPROTO’ undeclared (first use in this function)
  183 |  case EPROTO:
      |       ^~~~~~
src/error.c: In function ‘btd_error_le_conn_from_errno’:
src/error.c:193:7: error: ‘EINVAL’ undeclared (first use in this function)
  193 |  case EINVAL:
      |       ^~~~~~
src/error.c:195:7: error: ‘EHOSTUNREACH’ undeclared (first use in this function)
  195 |  case EHOSTUNREACH:
      |       ^~~~~~~~~~~~
src/error.c:197:7: error: ‘EOPNOTSUPP’ undeclared (first use in this function)
  197 |  case EOPNOTSUPP:
      |       ^~~~~~~~~~
src/error.c:198:7: error: ‘EPROTONOSUPPORT’ undeclared (first use in this function)
  198 |  case EPROTONOSUPPORT: // Fall through
      |       ^~~~~~~~~~~~~~~
src/error.c:200:7: error: ‘EALREADY’ undeclared (first use in this function)
  200 |  case EALREADY:
      |       ^~~~~~~~
src/error.c:201:7: error: ‘EISCONN’ undeclared (first use in this function)
  201 |  case EISCONN: // Fall through
      |       ^~~~~~~
src/error.c:203:7: error: ‘EBADFD’ undeclared (first use in this function)
  203 |  case EBADFD:
      |       ^~~~~~
src/error.c:205:7: error: ‘ENOMEM’ undeclared (first use in this function)
  205 |  case ENOMEM:
      |       ^~~~~~
src/error.c:207:7: error: ‘EBUSY’ undeclared (first use in this function)
  207 |  case EBUSY:
      |       ^~~~~
src/error.c:209:7: error: ‘ECONNREFUSED’ undeclared (first use in this function)
  209 |  case ECONNREFUSED:
      |       ^~~~~~~~~~~~
src/error.c:211:7: error: ‘EIO’ undeclared (first use in this function)
  211 |  case EIO:
      |       ^~~
src/error.c:213:7: error: ‘ETIMEDOUT’ undeclared (first use in this function)
  213 |  case ETIMEDOUT:
      |       ^~~~~~~~~
src/error.c:215:7: error: ‘EMLINK’ undeclared (first use in this function)
  215 |  case EMLINK:
      |       ^~~~~~
src/error.c:217:7: error: ‘ECONNRESET’ undeclared (first use in this function)
  217 |  case ECONNRESET:
      |       ^~~~~~~~~~
src/error.c:219:7: error: ‘ECONNABORTED’ undeclared (first use in this function)
  219 |  case ECONNABORTED:
      |       ^~~~~~~~~~~~
src/error.c:221:7: error: ‘EPROTO’ undeclared (first use in this function)
  221 |  case EPROTO:
      |       ^~~~~~
make[1]: *** [Makefile:9301: src/bluetoothd-error.o] Error 1
make: *** [Makefile:4134: all] Error 2




---
Regards,
Linux Bluetooth

[BlueZ PATCH v2 2/3] device: Include BtdError code in Connect() return

[Miao-chen Chou]

This replaces generic strerror message with context specific BtdError
code to better indicate the detailed failure reason so that the D-Bus
clients can optimize their application to work better with BlueZ, e.g.
introducing retry mechanism or building metrics.

Reviewed-by: Alain Michaud <alainm@chromium.org>
Reviewed-by: Howard Chung <howardchung@google.com>
---

(no changes since v1)

 src/device.c | 52 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 34 insertions(+), 18 deletions(-)

diff --git a/src/device.c b/src/device.c
index df440ce09..c9dc616a2 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1607,8 +1607,8 @@ void device_request_disconnect(struct btd_device *device, DBusMessage *msg)
 	}
 
 	if (device->connect) {
-		DBusMessage *reply = btd_error_failed(device->connect,
-								"Cancelled");
+		DBusMessage *reply = btd_error_failed_err(device->connect,
+						BTD_ERR_BREDR_CONN_CANCELED);
 		g_dbus_send_message(dbus_conn, reply);
 		dbus_message_unref(device->connect);
 		device->connect = NULL;
@@ -1802,7 +1802,8 @@ done:
 		}
 
 		g_dbus_send_message(dbus_conn,
-				btd_error_failed(dev->connect, strerror(-err)));
+			btd_error_failed_err(dev->connect,
+					btd_error_bredr_conn_from_errno(err)));
 	} else {
 		/* Start passive SDP discovery to update known services */
 		if (dev->bredr && !dev->svc_refreshed && dev->refresh_discovery)
@@ -2003,10 +2004,12 @@ static DBusMessage *connect_profiles(struct btd_device *dev, uint8_t bdaddr_type
 						dbus_message_get_sender(msg));
 
 	if (dev->pending || dev->connect || dev->browse)
-		return btd_error_in_progress(msg);
+		return btd_error_in_progress_err(msg, BTD_ERR_BREDR_CONN_BUSY);
 
-	if (!btd_adapter_get_powered(dev->adapter))
-		return btd_error_not_ready(msg);
+	if (!btd_adapter_get_powered(dev->adapter)) {
+		return btd_error_not_ready_err(msg,
+					BTD_ERR_BREDR_CONN_ADAPTER_NOT_POWERED);
+	}
 
 	btd_device_set_temporary(dev, false);
 
@@ -2019,10 +2022,12 @@ static DBusMessage *connect_profiles(struct btd_device *dev, uint8_t bdaddr_type
 			if (dbus_message_is_method_call(msg, DEVICE_INTERFACE,
 							"Connect") &&
 				find_service_with_state(dev->services,
-						BTD_SERVICE_STATE_CONNECTED))
+						BTD_SERVICE_STATE_CONNECTED)) {
 				return dbus_message_new_method_return(msg);
-			else
-				return btd_error_not_available(msg);
+			} else {
+				return btd_error_not_available_err(msg,
+					BTD_ERR_BREDR_CONN_PROFILE_UNAVAILABLE);
+			}
 		}
 
 		goto resolve_services;
@@ -2032,7 +2037,8 @@ static DBusMessage *connect_profiles(struct btd_device *dev, uint8_t bdaddr_type
 	if (err < 0) {
 		if (err == -EALREADY)
 			return dbus_message_new_method_return(msg);
-		return btd_error_failed(msg, strerror(-err));
+		return btd_error_failed_err(msg,
+					btd_error_bredr_conn_from_errno(err));
 	}
 
 	dev->connect = dbus_message_ref(msg);
@@ -2046,8 +2052,11 @@ resolve_services:
 		err = device_browse_sdp(dev, msg);
 	else
 		err = device_browse_gatt(dev, msg);
-	if (err < 0)
-		return btd_error_failed(msg, strerror(-err));
+	if (err < 0) {
+		return btd_error_failed_err(msg, bdaddr_type == BDADDR_BREDR ?
+			BTD_ERR_BREDR_CONN_SDP_SEARCH :
+			BTD_ERR_LE_CONN_GATT_BROWSE);
+	}
 
 	return NULL;
 }
@@ -2157,8 +2166,10 @@ static DBusMessage *connect_profile(DBusConnection *conn, DBusMessage *msg,
 	DBusMessage *reply;
 
 	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &pattern,
-							DBUS_TYPE_INVALID))
-		return btd_error_invalid_args(msg);
+							DBUS_TYPE_INVALID)) {
+		return btd_error_invalid_args_err(msg,
+					BTD_ERR_BREDR_CONN_INVALID_ARGUMENTS);
+	}
 
 	uuid = bt_name2string(pattern);
 	reply = connect_profiles(dev, BDADDR_BREDR, msg, uuid);
@@ -2541,7 +2552,11 @@ static void browse_request_complete(struct browse_req *req, uint8_t type,
 			if (err == 0)
 				goto done;
 		}
-		reply = btd_error_failed(req->msg, strerror(-err));
+		reply = (bdaddr_type == BDADDR_BREDR ?
+				btd_error_failed_err(req->msg,
+					btd_error_bredr_conn_from_errno(err)) :
+				btd_error_failed_err(req->msg,
+					btd_error_le_conn_from_errno(err)));
 		goto done;
 	}
 
@@ -3027,7 +3042,8 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type)
 	 */
 	if (device->connect) {
 		DBG("connection removed while Connect() is waiting reply");
-		reply = btd_error_failed(device->connect, "Disconnected early");
+		reply = btd_error_failed_err(device->connect,
+						BTD_ERR_BREDR_CONN_CANCELED);
 		g_dbus_send_message(dbus_conn, reply);
 		dbus_message_unref(device->connect);
 		device->connect = NULL;
@@ -5413,8 +5429,8 @@ done:
 
 	if (device->connect) {
 		if (err < 0)
-			reply = btd_error_failed(device->connect,
-							strerror(-err));
+			reply = btd_error_failed_err(device->connect,
+					btd_error_le_conn_from_errno(err));
 		else
 			reply = dbus_message_new_method_return(device->connect);
 
-- 
2.32.0.93.g670b81a890-goog

[BlueZ PATCH v2 3/3] client: Print error code for connect methods

[Miao-chen Chou]

The following steps were performed.
- Issuing repeated commands to connect the same BLE device.
- Verifying the print in bluetoothctl console

Reviewed-by: Alain Michaud <alainm@chromium.org>
Reviewed-by: Howard Chung <howardchung@google.com>
---

Changes in v2:
- Add documentation for error codes

 client/main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/client/main.c b/client/main.c
index da877b546..488a24bf6 100644
--- a/client/main.c
+++ b/client/main.c
@@ -1949,7 +1949,8 @@ static void connect_reply(DBusMessage *message, void *user_data)
 	dbus_error_init(&error);
 
 	if (dbus_set_error_from_message(&error, message) == TRUE) {
-		bt_shell_printf("Failed to connect: %s\n", error.name);
+		bt_shell_printf("Failed to connect: %s %s\n", error.name,
+				error.message);
 		dbus_error_free(&error);
 		return bt_shell_noninteractive_quit(EXIT_FAILURE);
 	}
-- 
2.32.0.93.g670b81a890-goog