[BlueZ PATCH v7 0/4] Detailed error string

[BlueZ PATCH v7 0/4] Detailed error string

[Miao-chen Chou]

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 v7:
- Fix nits

Changes in v6:
- Rephrase error string

Changes in v5:
- Replace error code with error string

Changes in v4:
- Address make errors.

Changes in v3:
- Correct error-codes.txt.

Changes in v2:
- Add documentation for error codes

Miao-chen Chou (4):
  Add errors.txt to describe errors of D-Bus method returns
  BR/EDR and LE connection failure reasons
  Include detailed error string in Connect() return
  Print error code for connect methods

 client/main.c  |   3 +-
 doc/errors.txt | 233 +++++++++++++++++++++++++++++++++++++++++++++++++
 src/device.c   |  46 ++++++----
 src/error.c    | 100 +++++++++++++++++++++
 src/error.h    |  57 ++++++++++++
 5 files changed, 422 insertions(+), 17 deletions(-)
 create mode 100644 doc/errors.txt

-- 
2.33.0.800.g4c38ced690-goog

[BlueZ PATCH v7 1/4] doc: Add errors.txt to describe errors of D-Bus method returns

[Miao-chen Chou]

---

(no changes since v5)

Changes in v5:
- Remove the use of error codes.

 doc/errors.txt | 233 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 233 insertions(+)
 create mode 100644 doc/errors.txt

diff --git a/doc/errors.txt b/doc/errors.txt
new file mode 100644
index 000000000..047610c74
--- /dev/null
+++ b/doc/errors.txt
@@ -0,0 +1,233 @@
+D-Bus Method Return Error Codes
+===============================
+
+The motivation of having detailed error 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
+===================================
+	errno:	EALREADY, EISCONN
+
+	Either the profile is already connected or ACL connection is in place.
+
+BR/EDR connection page timeout
+==============================
+	errno:	EHOSTDOWN
+
+	Failed due to page timeout.
+
+BR/EDR connection profile unavailable
+=====================================
+	errno:	ENOPROTOOPT
+
+	Failed to find connectable services or the target service.
+
+BR/EDR connection SDP search
+============================
+	errno:	none
+
+	Failed to complete the SDP search.
+
+BR/EDR connection create socket
+===============================
+	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
+===================================
+	errno:	EHOSTUNREACH
+
+	Failed due to invalid arguments.
+
+BR/EDR connection not powered
+=============================
+	errno:	EHOSTUNREACH
+
+	Failed due to adapter not powered.
+
+BR/EDR connection not supported
+===============================
+	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
+============================
+	errno:	EBADFD
+
+	Failed due to the socket is in bad state.
+
+BR/EDR connection memory allocation
+===================================
+	errno:	ENOMEM
+
+	Failed to allocate memory in either host stack or controller.
+
+BR/EDR connection busy
+======================
+	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
+=============================================
+	errno:	EMLINK
+
+	Failed due to reaching the concurrent connection limit to a device.
+
+BR/EDR connection timeout
+=========================
+	errno:	ETIMEDOUT
+
+	Failed due to connection timeout
+
+BR/EDR connection refused
+=========================
+	errno:	ECONNREFUSED
+
+	Refused by the remote device due to limited resource, security reason
+	or unacceptable address type.
+
+BR/EDR connection aborted by remote
+===================================
+	errno:	ECONNRESET
+
+	Terminated by the remote device due to limited resource or power off.
+
+BR/EDR connection aborted by local
+==================================
+	errno:	ECONNABORTED
+
+	Aborted by the local host.
+
+BR/EDR connection LMP protocol error
+====================================
+	errno:	EPROTO
+
+	Failed due to LMP protocol error.
+
+BR/EDR connection canceled
+==========================
+	errno:	none
+
+	Failed due to cancellation caused by adapter drop, unexpected device
+	drop, orincoming disconnection request before connection request is
+	completed.
+
+BR/EDR connection unknown error
+===============================
+	errno:	ENOSYS
+
+	Failed due to unknown reason.
+
+LE connection invalid arguments
+===============================
+	errno:	EINVAL
+
+	Failed due to invalid arguments.
+
+LE connection not powered
+=========================
+	errno:	EHOSTUNREACH
+
+	Failed due to adapter not powered.
+
+LE connection not supported
+===========================
+	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
+===============================
+	errno: EALREADY, EISCONN
+
+	Either the BT IO is already connected or LE link connection in place.
+
+LE connection bad socket
+========================
+	errno: EBADFD
+
+	Failed due to the socket is in bad state.
+
+LE connection memory allocation
+===============================
+	errno: ENOMEM
+
+	Failed to allocate memory in either host stack or controller.
+
+LE connection busy
+==================
+	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
+=====================
+	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
+===========================
+	errno:	EIO
+
+	Failed to create or connect to BT IO socket. This can also indicate
+	hardware failure in the controller.
+
+LE connection timeout
+=====================
+	errno:	ETIMEDOUT
+
+	Failed due to connection timeout
+
+LE connection concurrent connection limit
+=========================================
+	errno:	EMLINK
+
+	Failed due to reaching the synchronous connection limit to a device.
+
+LE connection abort by remote
+=============================
+	errno:	ECONNRESET
+
+	Aborted by the remote device due to limited resource or power off.
+
+LE connection abort by local
+============================
+	errno:	ECONNABORTED
+
+	Aborted by the local host.
+
+LE connection link layer protocol error
+=======================================
+	errno:	EPROTO
+
+	Failed due to link layer protocol error.
+
+LE connection GATT browsing
+===========================
+	errno:	none
+
+	Failed to complete the GATT browsing.
+
+LE connection unknown error
+===========================
+	errno:	ENOSYS
+
+	Failed due to unknown reason.
-- 
2.33.0.800.g4c38ced690-goog

[BlueZ PATCH v7 2/4] 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

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

Changes in v7:
- Fix nits.

Changes in v6:
- Rephrase error strings.

Changes in v5:
- Replace uint16_t error code with string

Changes in v4:
- Address make errors

Changes in v3:
- Separate error-code.txt into its own commit

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

 src/error.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/error.h |  57 ++++++++++++++++++++++++++++++
 2 files changed, 157 insertions(+)

diff --git a/src/error.c b/src/error.c
index dd85cd288..29c071067 100644
--- a/src/error.c
+++ b/src/error.c
@@ -14,6 +14,8 @@
 #include <config.h>
 #endif
 
+#include <errno.h>
+#include <stdio.h>
 #include "gdbus/gdbus.h"
 
 #include "error.h"
@@ -66,12 +68,24 @@ DBusMessage *btd_error_in_progress(DBusMessage *msg)
 					"In Progress");
 }
 
+DBusMessage *btd_error_in_progress_str(DBusMessage *msg, const char *str)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
+					"%s", str);
+}
+
 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_str(DBusMessage *msg, const char *str)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
+					"%s", str);
+}
+
 DBusMessage *btd_error_does_not_exist(DBusMessage *msg)
 {
 	return g_dbus_create_error(msg, ERROR_INTERFACE ".DoesNotExist",
@@ -108,8 +122,94 @@ DBusMessage *btd_error_not_ready(DBusMessage *msg)
 					"Resource Not Ready");
 }
 
+DBusMessage *btd_error_not_ready_str(DBusMessage *msg, const char *str)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE ".NotReady",
+					"%s", str);
+}
+
 DBusMessage *btd_error_failed(DBusMessage *msg, const char *str)
 {
 	return g_dbus_create_error(msg, ERROR_INTERFACE
 					".Failed", "%s", str);
 }
+
+const char *btd_error_bredr_conn_from_errno(int errno_code)
+{
+	switch (-errno_code) {
+	case EALREADY:
+	case EISCONN:
+		return ERR_BREDR_CONN_ALREADY_CONNECTED;
+	case EHOSTDOWN:
+		return ERR_BREDR_CONN_PAGE_TIMEOUT;
+	case ENOPROTOOPT:
+		return ERR_BREDR_CONN_PROFILE_UNAVAILABLE;
+	case EIO:
+		return ERR_BREDR_CONN_CREATE_SOCKET;
+	case EINVAL:
+		return ERR_BREDR_CONN_INVALID_ARGUMENTS;
+	case EHOSTUNREACH:
+		return ERR_BREDR_CONN_ADAPTER_NOT_POWERED;
+	case EOPNOTSUPP:
+	case EPROTONOSUPPORT:
+		return ERR_BREDR_CONN_NOT_SUPPORTED;
+	case EBADFD:
+		return ERR_BREDR_CONN_BAD_SOCKET;
+	case ENOMEM:
+		return ERR_BREDR_CONN_MEMORY_ALLOC;
+	case EBUSY:
+		return ERR_BREDR_CONN_BUSY;
+	case EMLINK:
+		return ERR_BREDR_CONN_CNCR_CONNECT_LIMIT;
+	case ETIMEDOUT:
+		return ERR_BREDR_CONN_TIMEOUT;
+	case ECONNREFUSED:
+		return ERR_BREDR_CONN_REFUSED;
+	case ECONNRESET:
+		return ERR_BREDR_CONN_ABORT_BY_REMOTE;
+	case ECONNABORTED:
+		return ERR_BREDR_CONN_ABORT_BY_LOCAL;
+	case EPROTO:
+		return ERR_BREDR_CONN_LMP_PROTO_ERROR;
+	default:
+		return ERR_BREDR_CONN_UNKNOWN;
+	}
+}
+
+const char *btd_error_le_conn_from_errno(int errno_code)
+{
+	switch (-errno_code) {
+	case EINVAL:
+		return ERR_LE_CONN_INVALID_ARGUMENTS;
+	case EHOSTUNREACH:
+		return ERR_LE_CONN_ADAPTER_NOT_POWERED;
+	case EOPNOTSUPP:
+	case EPROTONOSUPPORT:
+		return ERR_LE_CONN_NOT_SUPPORTED;
+	case EALREADY:
+	case EISCONN:
+		return ERR_LE_CONN_ALREADY_CONNECTED;
+	case EBADFD:
+		return ERR_LE_CONN_BAD_SOCKET;
+	case ENOMEM:
+		return ERR_LE_CONN_MEMORY_ALLOC;
+	case EBUSY:
+		return ERR_LE_CONN_BUSY;
+	case ECONNREFUSED:
+		return ERR_LE_CONN_REFUSED;
+	case EIO:
+		return ERR_LE_CONN_CREATE_SOCKET;
+	case ETIMEDOUT:
+		return ERR_LE_CONN_TIMEOUT;
+	case EMLINK:
+		return ERR_LE_CONN_SYNC_CONNECT_LIMIT;
+	case ECONNRESET:
+		return ERR_LE_CONN_ABORT_BY_REMOTE;
+	case ECONNABORTED:
+		return ERR_LE_CONN_ABORT_BY_LOCAL;
+	case EPROTO:
+		return ERR_LE_CONN_LL_PROTO_ERROR;
+	default:
+		return ERR_LE_CONN_UNKNOWN;
+	}
+}
diff --git a/src/error.h b/src/error.h
index 7fcddaf4a..dded9f93d 100644
--- a/src/error.h
+++ b/src/error.h
@@ -11,9 +11,60 @@
  */
 
 #include <dbus/dbus.h>
+#include <stdint.h>
 
 #define ERROR_INTERFACE "org.bluez.Error"
 
+/* BR/EDR connection failure reasons */
+#define ERR_BREDR_CONN_ALREADY_CONNECTED	"br-connection-already-"\
+						"connected"
+#define ERR_BREDR_CONN_PAGE_TIMEOUT		"br-connection-page-timeout"
+#define ERR_BREDR_CONN_PROFILE_UNAVAILABLE	"br-connection-profile-"\
+						"unavailable"
+#define ERR_BREDR_CONN_SDP_SEARCH		"br-connection-sdp-search"
+#define ERR_BREDR_CONN_CREATE_SOCKET		"br-connection-create-socket"
+#define ERR_BREDR_CONN_INVALID_ARGUMENTS	"br-connection-invalid-"\
+						"argument"
+#define ERR_BREDR_CONN_ADAPTER_NOT_POWERED	"br-connection-adapter-not-"\
+						"powered"
+#define ERR_BREDR_CONN_NOT_SUPPORTED		"br-connection-not-suuported"
+#define ERR_BREDR_CONN_BAD_SOCKET		"br-connection-bad-socket"
+#define ERR_BREDR_CONN_MEMORY_ALLOC		"br-connection-memory-"\
+						"allocation"
+#define ERR_BREDR_CONN_BUSY			"br-connection-busy"
+#define ERR_BREDR_CONN_CNCR_CONNECT_LIMIT	"br-connection-concurrent-"\
+						"connection-limit"
+#define ERR_BREDR_CONN_TIMEOUT			"br-connection-timeout"
+#define ERR_BREDR_CONN_REFUSED			"br-connection-refused"
+#define ERR_BREDR_CONN_ABORT_BY_REMOTE		"br-connection-aborted-by-"\
+						"remote"
+#define ERR_BREDR_CONN_ABORT_BY_LOCAL		"br-connection-aborted-by-"\
+						"local"
+#define ERR_BREDR_CONN_LMP_PROTO_ERROR		"br-connection-lmp-protocol-"\
+						"error"
+#define ERR_BREDR_CONN_CANCELED			"br-connection-canceled"
+#define ERR_BREDR_CONN_UNKNOWN			"br-connection-unknown"
+
+/* LE connection failure reasons */
+#define ERR_LE_CONN_INVALID_ARGUMENTS	"le-connection-invalid-arguments"
+#define ERR_LE_CONN_ADAPTER_NOT_POWERED	"le-connection-adapter-not-powered"
+#define ERR_LE_CONN_NOT_SUPPORTED	"le-connection-not-supported"
+#define ERR_LE_CONN_ALREADY_CONNECTED	"le-connection-already-connected"
+#define ERR_LE_CONN_BAD_SOCKET		"le-connection-bad-socket"
+#define ERR_LE_CONN_MEMORY_ALLOC	"le-connection-memory-allocation"
+#define ERR_LE_CONN_BUSY		"le-connection-busy"
+#define ERR_LE_CONN_REFUSED		"le-connection-refused"
+#define ERR_LE_CONN_CREATE_SOCKET	"le-connection-create-socket"
+#define ERR_LE_CONN_TIMEOUT		"le-connection-timeout"
+#define ERR_LE_CONN_SYNC_CONNECT_LIMIT	"le-connection-concurrent-connection-"\
+					"limit"
+#define ERR_LE_CONN_ABORT_BY_REMOTE	"le-connection-abort-by-remote"
+#define ERR_LE_CONN_ABORT_BY_LOCAL	"le-connection-abort-by-local"
+#define ERR_LE_CONN_LL_PROTO_ERROR	"le-connection-link-layer-protocol-"\
+					"error"
+#define ERR_LE_CONN_GATT_BROWSE		"le-connection-gatt-browsing"
+#define ERR_LE_CONN_UNKNOWN		"le-connection-unknown"
+
 DBusMessage *btd_error_invalid_args(DBusMessage *msg);
 DBusMessage *btd_error_invalid_args_str(DBusMessage *msg, const char *str);
 DBusMessage *btd_error_busy(DBusMessage *msg);
@@ -22,11 +73,17 @@ 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_str(DBusMessage *msg, const char *str);
 DBusMessage *btd_error_in_progress(DBusMessage *msg);
+DBusMessage *btd_error_in_progress_str(DBusMessage *msg, const char *str);
 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_str(DBusMessage *msg, const char *str);
 DBusMessage *btd_error_failed(DBusMessage *msg, const char *str);
+
+const char *btd_error_bredr_conn_from_errno(int errno_code);
+const char *btd_error_le_conn_from_errno(int errno_code);
-- 
2.33.0.800.g4c38ced690-goog

[BlueZ PATCH v7 3/4] device: Include detailed error string in Connect() return

[Miao-chen Chou]

This replaces generic strerror message with context detailed error
string 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 v5)

Changes in v5:
- Address the changes from error code to string

 src/device.c | 46 ++++++++++++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/src/device.c b/src/device.c
index b29aa195d..230274ee1 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1605,7 +1605,7 @@ void device_request_disconnect(struct btd_device *device, DBusMessage *msg)
 
 	if (device->connect) {
 		DBusMessage *reply = btd_error_failed(device->connect,
-								"Cancelled");
+						ERR_BREDR_CONN_CANCELED);
 		g_dbus_send_message(dbus_conn, reply);
 		dbus_message_unref(device->connect);
 		device->connect = NULL;
@@ -1799,7 +1799,8 @@ done:
 		}
 
 		g_dbus_send_message(dbus_conn,
-				btd_error_failed(dev->connect, strerror(-err)));
+			btd_error_failed(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)
@@ -2000,10 +2001,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_str(msg, 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_str(msg,
+					ERR_BREDR_CONN_ADAPTER_NOT_POWERED);
+	}
 
 	btd_device_set_temporary(dev, false);
 
@@ -2016,10 +2019,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_str(msg,
+					ERR_BREDR_CONN_PROFILE_UNAVAILABLE);
+			}
 		}
 
 		goto resolve_services;
@@ -2029,7 +2034,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(msg,
+					btd_error_bredr_conn_from_errno(err));
 	}
 
 	dev->connect = dbus_message_ref(msg);
@@ -2043,8 +2049,10 @@ 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(msg, bdaddr_type == BDADDR_BREDR ?
+			ERR_BREDR_CONN_SDP_SEARCH : ERR_LE_CONN_GATT_BROWSE);
+	}
 
 	return NULL;
 }
@@ -2154,8 +2162,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_str(msg,
+					ERR_BREDR_CONN_INVALID_ARGUMENTS);
+	}
 
 	uuid = bt_name2string(pattern);
 	reply = connect_profiles(dev, BDADDR_BREDR, msg, uuid);
@@ -2538,7 +2548,10 @@ 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 = btd_error_failed(req->msg,
+				bdaddr_type == BDADDR_BREDR ?
+				btd_error_bredr_conn_from_errno(err) :
+				btd_error_le_conn_from_errno(err));
 		goto done;
 	}
 
@@ -3030,7 +3043,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(device->connect,
+						ERR_BREDR_CONN_CANCELED);
 		g_dbus_send_message(dbus_conn, reply);
 		dbus_message_unref(device->connect);
 		device->connect = NULL;
@@ -5454,7 +5468,7 @@ done:
 	if (device->connect) {
 		if (err < 0)
 			reply = btd_error_failed(device->connect,
-							strerror(-err));
+					btd_error_le_conn_from_errno(err));
 		else
 			reply = dbus_message_new_method_return(device->connect);
 
-- 
2.33.0.800.g4c38ced690-goog

[BlueZ PATCH v7 4/4] 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 v7:
- Fix nits

Changes in v6:
- Rephrase error string

Changes in v5:
- Replace error code with error string

Changes in v4:
- Address make errors.

Changes in v3:
- Correct error-codes.txt.

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 c1a62edb7..61650c289 100644
--- a/client/main.c
+++ b/client/main.c
@@ -1966,7 +1966,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.33.0.800.g4c38ced690-goog

Re: [BlueZ PATCH v7 0/4] Detailed error string

[Luiz Augusto von Dentz]

Hi Miao,

On Thu, Sep 30, 2021 at 3:46 PM Miao-chen Chou <mcchou@chromium.org> wrote:
>
> 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 v7:
> - Fix nits
>
> Changes in v6:
> - Rephrase error string
>
> Changes in v5:
> - Replace error code with error string
>
> Changes in v4:
> - Address make errors.
>
> Changes in v3:
> - Correct error-codes.txt.
>
> Changes in v2:
> - Add documentation for error codes
>
> Miao-chen Chou (4):
>   Add errors.txt to describe errors of D-Bus method returns
>   BR/EDR and LE connection failure reasons
>   Include detailed error string in Connect() return
>   Print error code for connect methods
>
>  client/main.c  |   3 +-
>  doc/errors.txt | 233 +++++++++++++++++++++++++++++++++++++++++++++++++
>  src/device.c   |  46 ++++++----
>  src/error.c    | 100 +++++++++++++++++++++
>  src/error.h    |  57 ++++++++++++
>  5 files changed, 422 insertions(+), 17 deletions(-)
>  create mode 100644 doc/errors.txt
>
> --
> 2.33.0.800.g4c38ced690-goog

Applied, thanks.

-- 
Luiz Augusto von Dentz