[PATCH BlueZ v4 0/8] Better connect debug and error reporting

[PATCH BlueZ v4 0/8] Better connect debug and error reporting

[Bastien Nocera]

Completely redone from v3, following discussion on the mailing-list:
https://patchwork.kernel.org/project/bluetooth/list/?series=964591&archive=both

Bastien Nocera (8):
  error: Rename device error helper to match retval
  error: Add helper to return DbusMessage errors from errno
  device: Use new DbusMessage error helper
  error: Make errno to string helpers private
  device: Better error when no BR/EDR profiles can be connected to
  device: Better "Connect" debug
  device: Better error when the link key is missing
  device: Better error when the adapter is not powered

 doc/org.bluez.Device.rst |  5 +++
 src/device.c             | 79 +++++++++++++++++++++++++---------------
 src/error.c              | 71 ++++++++++++++++++++++++++++++------
 src/error.h              | 12 +++---
 4 files changed, 118 insertions(+), 49 deletions(-)

-- 
2.50.0

[PATCH BlueZ v4 1/8] error: Rename device error helper to match retval

[Bastien Nocera]

Rename the helper to show that it outputs a string, and not an error
DbusMessage as the other functions do.
---
 src/device.c | 10 +++++-----
 src/error.c  |  4 ++--
 src/error.h  |  4 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/device.c b/src/device.c
index e4d5d79ee18b..709052d301b7 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2341,7 +2341,7 @@ done:
 
 		g_dbus_send_message(dbus_conn,
 			btd_error_failed(dev->connect,
-					btd_error_bredr_conn_from_errno(err)));
+					btd_error_str_bredr_conn_from_errno(err)));
 	} else {
 		/* Start passive SDP discovery to update known services */
 		if (dev->bredr && !dev->svc_refreshed && dev->refresh_discovery)
@@ -2699,7 +2699,7 @@ static DBusMessage *connect_profiles(struct btd_device *dev, uint8_t bdaddr_type
 		if (err == -EALREADY)
 			return dbus_message_new_method_return(msg);
 		return btd_error_failed(msg,
-					btd_error_bredr_conn_from_errno(err));
+					btd_error_str_bredr_conn_from_errno(err));
 	}
 
 	dev->connect = dbus_message_ref(msg);
@@ -3004,8 +3004,8 @@ static void browse_request_complete(struct browse_req *req, uint8_t type,
 		}
 		reply = btd_error_failed(req->msg,
 				bdaddr_type == BDADDR_BREDR ?
-				btd_error_bredr_conn_from_errno(err) :
-				btd_error_le_conn_from_errno(err));
+				btd_error_str_bredr_conn_from_errno(err) :
+				btd_error_str_le_conn_from_errno(err));
 		goto done;
 	}
 
@@ -6283,7 +6283,7 @@ done:
 	if (device->connect) {
 		if (err < 0)
 			reply = btd_error_failed(device->connect,
-					btd_error_le_conn_from_errno(err));
+					btd_error_str_le_conn_from_errno(err));
 		else
 			reply = dbus_message_new_method_return(device->connect);
 
diff --git a/src/error.c b/src/error.c
index f179e6cda0e8..d82c2f2e1520 100644
--- a/src/error.c
+++ b/src/error.c
@@ -134,7 +134,7 @@ DBusMessage *btd_error_failed(DBusMessage *msg, const char *str)
 					".Failed", "%s", str);
 }
 
-const char *btd_error_bredr_conn_from_errno(int errno_code)
+const char *btd_error_str_bredr_conn_from_errno(int errno_code)
 {
 	switch (-errno_code) {
 	case EALREADY:
@@ -178,7 +178,7 @@ const char *btd_error_bredr_conn_from_errno(int errno_code)
 	}
 }
 
-const char *btd_error_le_conn_from_errno(int errno_code)
+const char *btd_error_str_le_conn_from_errno(int errno_code)
 {
 	switch (-errno_code) {
 	case EINVAL:
diff --git a/src/error.h b/src/error.h
index 83206cb48a66..5b49400fbb92 100644
--- a/src/error.h
+++ b/src/error.h
@@ -87,5 +87,5 @@ 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);
+const char *btd_error_str_bredr_conn_from_errno(int errno_code);
+const char *btd_error_str_le_conn_from_errno(int errno_code);
-- 
2.50.0

[PATCH BlueZ v4 2/8] error: Add helper to return DbusMessage errors from errno

[Bastien Nocera]

This will allow us to start adding more precise errors instead of
"Failed" with a string if we're parsing an errno.
---
 src/error.c | 12 ++++++++++++
 src/error.h |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/src/error.c b/src/error.c
index d82c2f2e1520..0e57a07548a9 100644
--- a/src/error.c
+++ b/src/error.c
@@ -217,3 +217,15 @@ const char *btd_error_str_le_conn_from_errno(int errno_code)
 		return ERR_LE_CONN_UNKNOWN;
 	}
 }
+
+DBusMessage *btd_error_bredr_conn_from_errno(DBusMessage *msg, int errno_code)
+{
+	return btd_error_failed(msg,
+				btd_error_str_bredr_conn_from_errno(errno_code));
+}
+
+DBusMessage *btd_error_le_conn_from_errno(DBusMessage *msg, int errno_code)
+{
+	return btd_error_failed(msg,
+				btd_error_str_le_conn_from_errno(errno_code));
+}
diff --git a/src/error.h b/src/error.h
index 5b49400fbb92..828cf5923f6f 100644
--- a/src/error.h
+++ b/src/error.h
@@ -86,6 +86,8 @@ 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);
+DBusMessage *btd_error_bredr_conn_from_errno(DBusMessage *msg, int errno_code);
+DBusMessage *btd_error_le_conn_from_errno(DBusMessage *msg, int errno_code);
 
 const char *btd_error_str_bredr_conn_from_errno(int errno_code);
 const char *btd_error_str_le_conn_from_errno(int errno_code);
-- 
2.50.0

[PATCH BlueZ v4 3/8] device: Use new DbusMessage error helper

[Bastien Nocera]

---
 src/device.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/src/device.c b/src/device.c
index 709052d301b7..5c80fedd7dc6 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2340,8 +2340,7 @@ done:
 		}
 
 		g_dbus_send_message(dbus_conn,
-			btd_error_failed(dev->connect,
-					btd_error_str_bredr_conn_from_errno(err)));
+			btd_error_bredr_conn_from_errno(dev->connect, err));
 	} else {
 		/* Start passive SDP discovery to update known services */
 		if (dev->bredr && !dev->svc_refreshed && dev->refresh_discovery)
@@ -2698,8 +2697,7 @@ 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,
-					btd_error_str_bredr_conn_from_errno(err));
+		return btd_error_bredr_conn_from_errno(msg, err);
 	}
 
 	dev->connect = dbus_message_ref(msg);
@@ -3002,10 +3000,10 @@ static void browse_request_complete(struct browse_req *req, uint8_t type,
 			if (err == 0)
 				goto done;
 		}
-		reply = btd_error_failed(req->msg,
-				bdaddr_type == BDADDR_BREDR ?
-				btd_error_str_bredr_conn_from_errno(err) :
-				btd_error_str_le_conn_from_errno(err));
+		if (bdaddr_type == BDADDR_BREDR)
+			reply = btd_error_bredr_conn_from_errno(req->msg, err);
+		else
+			reply = btd_error_le_conn_from_errno(req->msg, err);
 		goto done;
 	}
 
@@ -6282,8 +6280,7 @@ done:
 
 	if (device->connect) {
 		if (err < 0)
-			reply = btd_error_failed(device->connect,
-					btd_error_str_le_conn_from_errno(err));
+			reply = btd_error_le_conn_from_errno(device->connect, err);
 		else
 			reply = dbus_message_new_method_return(device->connect);
 
-- 
2.50.0

[PATCH BlueZ v4 4/8] error: Make errno to string helpers private

[Bastien Nocera]

We're not using them outside the error code itself.
---
 src/error.c | 4 ++--
 src/error.h | 3 ---
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/error.c b/src/error.c
index 0e57a07548a9..74b8f80e63c5 100644
--- a/src/error.c
+++ b/src/error.c
@@ -134,7 +134,7 @@ DBusMessage *btd_error_failed(DBusMessage *msg, const char *str)
 					".Failed", "%s", str);
 }
 
-const char *btd_error_str_bredr_conn_from_errno(int errno_code)
+static const char *btd_error_str_bredr_conn_from_errno(int errno_code)
 {
 	switch (-errno_code) {
 	case EALREADY:
@@ -178,7 +178,7 @@ const char *btd_error_str_bredr_conn_from_errno(int errno_code)
 	}
 }
 
-const char *btd_error_str_le_conn_from_errno(int errno_code)
+static const char *btd_error_str_le_conn_from_errno(int errno_code)
 {
 	switch (-errno_code) {
 	case EINVAL:
diff --git a/src/error.h b/src/error.h
index 828cf5923f6f..0fa3975cff7c 100644
--- a/src/error.h
+++ b/src/error.h
@@ -88,6 +88,3 @@ DBusMessage *btd_error_not_ready_str(DBusMessage *msg, const char *str);
 DBusMessage *btd_error_failed(DBusMessage *msg, const char *str);
 DBusMessage *btd_error_bredr_conn_from_errno(DBusMessage *msg, int errno_code);
 DBusMessage *btd_error_le_conn_from_errno(DBusMessage *msg, int errno_code);
-
-const char *btd_error_str_bredr_conn_from_errno(int errno_code);
-const char *btd_error_str_le_conn_from_errno(int errno_code);
-- 
2.50.0

[PATCH BlueZ v4 5/8] device: Better error when no BR/EDR profiles can be connected to

[Bastien Nocera]

Add a more precise error when there are no BR/EDR profiles to connect
to. This is useful when trying to connect to a particular device, and
there are no relevant profiles to connect to on the bearer.
---
 doc/org.bluez.Device.rst |  2 ++
 src/device.c             |  3 +--
 src/error.c              | 19 +++++++++++++++----
 src/error.h              |  3 +--
 4 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/doc/org.bluez.Device.rst b/doc/org.bluez.Device.rst
index 646e2c77ec2d..b36a49eabdd5 100644
--- a/doc/org.bluez.Device.rst
+++ b/doc/org.bluez.Device.rst
@@ -49,6 +49,7 @@ Possible errors:
 :org.bluez.Error.Failed:
 :org.bluez.Error.InProgress:
 :org.bluez.Error.AlreadyConnected:
+:org.bluez.Error.ProfileUnavailable:
 
 void Disconnect()
 `````````````````
@@ -80,6 +81,7 @@ Possible errors:
 :org.bluez.Error.InProgress:
 :org.bluez.Error.InvalidArguments:
 :org.bluez.Error.NotAvailable:
+:org.bluez.Error.ProfileUnavailable:
 :org.bluez.Error.NotReady:
 
 void DisconnectProfile(string uuid)
diff --git a/src/device.c b/src/device.c
index 5c80fedd7dc6..99c0aa67ec0c 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2685,8 +2685,7 @@ static DBusMessage *connect_profiles(struct btd_device *dev, uint8_t bdaddr_type
 						BTD_SERVICE_STATE_CONNECTED)) {
 				return dbus_message_new_method_return(msg);
 			} else {
-				return btd_error_not_available_str(msg,
-					ERR_BREDR_CONN_PROFILE_UNAVAILABLE);
+				return btd_error_profile_unavailable(msg);
 			}
 		}
 
diff --git a/src/error.c b/src/error.c
index 74b8f80e63c5..8070bc6107ae 100644
--- a/src/error.c
+++ b/src/error.c
@@ -128,6 +128,14 @@ DBusMessage *btd_error_not_ready_str(DBusMessage *msg, const char *str)
 					"%s", str);
 }
 
+DBusMessage *btd_error_profile_unavailable(DBusMessage *msg)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE
+					".ProfileUnavailable",
+					"Exhausted the list of BR/EDR "
+					"profiles to connect to");
+}
+
 DBusMessage *btd_error_failed(DBusMessage *msg, const char *str)
 {
 	return g_dbus_create_error(msg, ERROR_INTERFACE
@@ -142,8 +150,6 @@ static const char *btd_error_str_bredr_conn_from_errno(int errno_code)
 		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:
@@ -220,8 +226,13 @@ static const char *btd_error_str_le_conn_from_errno(int errno_code)
 
 DBusMessage *btd_error_bredr_conn_from_errno(DBusMessage *msg, int errno_code)
 {
-	return btd_error_failed(msg,
-				btd_error_str_bredr_conn_from_errno(errno_code));
+	switch (-errno_code) {
+	case ENOPROTOOPT:
+		return btd_error_profile_unavailable(msg);
+	default:
+		return btd_error_failed(msg,
+					btd_error_str_bredr_conn_from_errno(errno_code));
+	}
 }
 
 DBusMessage *btd_error_le_conn_from_errno(DBusMessage *msg, int errno_code)
diff --git a/src/error.h b/src/error.h
index 0fa3975cff7c..f4ad81e5daa5 100644
--- a/src/error.h
+++ b/src/error.h
@@ -19,8 +19,6 @@
 #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-"\
@@ -85,6 +83,7 @@ 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_profile_unavailable(DBusMessage *msg);
 DBusMessage *btd_error_failed(DBusMessage *msg, const char *str);
 DBusMessage *btd_error_bredr_conn_from_errno(DBusMessage *msg, int errno_code);
 DBusMessage *btd_error_le_conn_from_errno(DBusMessage *msg, int errno_code);
-- 
2.50.0

[PATCH BlueZ v4 6/8] device: Better "Connect" debug

[Bastien Nocera]

Output clearer debug information so that it's possible to follow the
decisions made by the bluetoothd daemon when a client such as
bluetoothctl or the GNOME Bluetooth settings ask it to connect to a
device.
---
 src/device.c | 41 +++++++++++++++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 8 deletions(-)

diff --git a/src/device.c b/src/device.c
index 99c0aa67ec0c..d7a859f9df3f 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2683,6 +2683,7 @@ static DBusMessage *connect_profiles(struct btd_device *dev, uint8_t bdaddr_type
 							"Connect") &&
 				find_service_with_state(dev->services,
 						BTD_SERVICE_STATE_CONNECTED)) {
+				DBG("Already connected to services");
 				return dbus_message_new_method_return(msg);
 			} else {
 				return btd_error_profile_unavailable(msg);
@@ -2694,8 +2695,10 @@ static DBusMessage *connect_profiles(struct btd_device *dev, uint8_t bdaddr_type
 
 	err = connect_next(dev);
 	if (err < 0) {
-		if (err == -EALREADY)
+		if (err == -EALREADY) {
+			DBG("Already connected");
 			return dbus_message_new_method_return(msg);
+		}
 		return btd_error_bredr_conn_from_errno(msg, err);
 	}
 
@@ -2718,14 +2721,24 @@ resolve_services:
 	return NULL;
 }
 
+static const char *bdaddr_type_strs[] = {
+	"BR/EDR",
+	"LE public",
+	"LE random"
+};
+
 static DBusMessage *dev_connect(DBusConnection *conn, DBusMessage *msg,
 							void *user_data)
 {
 	struct btd_device *dev = user_data;
 	uint8_t bdaddr_type;
 
-	if (dev->bonding)
+	DBG("Calling \"Connect\" for device %s", dev->path);
+
+	if (dev->bonding) {
+		DBG("Bonding in progress");
 		return btd_error_in_progress(msg);
+	}
 
 	if (dev->bredr_state.connected) {
 		/*
@@ -2734,23 +2747,35 @@ static DBusMessage *dev_connect(DBusConnection *conn, DBusMessage *msg,
 		 */
 		if (dev->bredr_state.svc_resolved &&
 			find_service_with_state(dev->services,
-						BTD_SERVICE_STATE_CONNECTED))
+						BTD_SERVICE_STATE_CONNECTED)) {
 			bdaddr_type = dev->bdaddr_type;
-		else
+			DBG("Selecting address type %s, as BR/EDR services are resolved "
+			    " and connected", bdaddr_type_strs[dev->bdaddr_type]);
+		} else {
 			bdaddr_type = BDADDR_BREDR;
-	} else if (dev->le_state.connected && dev->bredr)
+			DBG("Selecting address type BR/EDR, as services not resolved "
+			    "or not connected");
+		}
+	} else if (dev->le_state.connected && dev->bredr) {
 		bdaddr_type = BDADDR_BREDR;
-	else
+		DBG("Selecting address type BR/EDR, as LE already connected");
+	} else {
 		bdaddr_type = select_conn_bearer(dev);
+		DBG("Selecting address type %s", bdaddr_type_strs[dev->bdaddr_type]);
+	}
 
 	if (bdaddr_type != BDADDR_BREDR) {
 		int err;
 
-		if (dev->connect)
+		if (dev->connect) {
+			DBG("Device already connecting");
 			return btd_error_in_progress(msg);
+		}
 
-		if (dev->le_state.connected)
+		if (dev->le_state.connected) {
+			DBG("Device already connected through LE");
 			return dbus_message_new_method_return(msg);
+		}
 
 		btd_device_set_temporary(dev, false);
 
-- 
2.50.0

[PATCH BlueZ v4 7/8] device: Better error when the link key is missing

[Bastien Nocera]

Add a more precise error when the link key is missing for a
profile or device we're trying to connect to.
---
 doc/org.bluez.Device.rst |  2 ++
 src/device.c             | 12 ++++++------
 src/error.c              | 27 ++++++++++++++++++++++-----
 src/error.h              |  3 +--
 4 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/doc/org.bluez.Device.rst b/doc/org.bluez.Device.rst
index b36a49eabdd5..d4de78a47239 100644
--- a/doc/org.bluez.Device.rst
+++ b/doc/org.bluez.Device.rst
@@ -50,6 +50,8 @@ Possible errors:
 :org.bluez.Error.InProgress:
 :org.bluez.Error.AlreadyConnected:
 :org.bluez.Error.ProfileUnavailable:
+:org.bluez.Error.BrConnectionKeyMissing:
+:org.bluez.Error.LeConnectionKeyMissing:
 
 void Disconnect()
 `````````````````
diff --git a/src/device.c b/src/device.c
index d7a859f9df3f..3bad7fb2c77c 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1982,14 +1982,14 @@ void device_request_disconnect(struct btd_device *device, DBusMessage *msg)
 	}
 
 	if (device->connect) {
-		const char *err_str;
 		DBusMessage *reply;
 
-		if (device->bonding_status == MGMT_STATUS_AUTH_FAILED)
-			err_str = ERR_BREDR_CONN_KEY_MISSING;
-		else
-			err_str = ERR_BREDR_CONN_CANCELED;
-		reply = btd_error_failed(device->connect, err_str);
+		if (device->bonding_status == MGMT_STATUS_AUTH_FAILED) {
+			reply = btd_error_br_connection_key_missing(device->connect);
+		} else {
+			reply = btd_error_failed(device->connect,
+						ERR_BREDR_CONN_CANCELED);
+		}
 		g_dbus_send_message(dbus_conn, reply);
 		dbus_message_unref(device->connect);
 		device->bonding_status = 0;
diff --git a/src/error.c b/src/error.c
index 8070bc6107ae..596ea4280991 100644
--- a/src/error.c
+++ b/src/error.c
@@ -136,6 +136,20 @@ DBusMessage *btd_error_profile_unavailable(DBusMessage *msg)
 					"profiles to connect to");
 }
 
+DBusMessage *btd_error_br_connection_key_missing(DBusMessage *msg)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE
+					".BrConnectionKeyMissing",
+					"BR/EDR Link Key missing");
+}
+
+static DBusMessage *btd_error_le_connection_key_missing(DBusMessage *msg)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE
+					".LeConnectionKeyMissing",
+					"LE Link Key missing");
+}
+
 DBusMessage *btd_error_failed(DBusMessage *msg, const char *str)
 {
 	return g_dbus_create_error(msg, ERROR_INTERFACE
@@ -177,8 +191,6 @@ static const char *btd_error_str_bredr_conn_from_errno(int errno_code)
 		return ERR_BREDR_CONN_ABORT_BY_LOCAL;
 	case EPROTO:
 		return ERR_BREDR_CONN_LMP_PROTO_ERROR;
-	case EBADE:
-		return ERR_BREDR_CONN_KEY_MISSING;
 	default:
 		return ERR_BREDR_CONN_UNKNOWN;
 	}
@@ -217,8 +229,6 @@ static const char *btd_error_str_le_conn_from_errno(int errno_code)
 		return ERR_LE_CONN_ABORT_BY_LOCAL;
 	case EPROTO:
 		return ERR_LE_CONN_LL_PROTO_ERROR;
-	case EBADE:
-		return ERR_LE_CONN_KEY_MISSING;
 	default:
 		return ERR_LE_CONN_UNKNOWN;
 	}
@@ -227,6 +237,8 @@ static const char *btd_error_str_le_conn_from_errno(int errno_code)
 DBusMessage *btd_error_bredr_conn_from_errno(DBusMessage *msg, int errno_code)
 {
 	switch (-errno_code) {
+	case EBADE:
+		return btd_error_br_connection_key_missing(msg);
 	case ENOPROTOOPT:
 		return btd_error_profile_unavailable(msg);
 	default:
@@ -237,6 +249,11 @@ DBusMessage *btd_error_bredr_conn_from_errno(DBusMessage *msg, int errno_code)
 
 DBusMessage *btd_error_le_conn_from_errno(DBusMessage *msg, int errno_code)
 {
-	return btd_error_failed(msg,
+	switch (-errno_code) {
+	case EBADE:
+		return btd_error_le_connection_key_missing(msg);
+	default:
+		return btd_error_failed(msg,
 				btd_error_str_le_conn_from_errno(errno_code));
+	}
 }
diff --git a/src/error.h b/src/error.h
index f4ad81e5daa5..96b7a7122457 100644
--- a/src/error.h
+++ b/src/error.h
@@ -41,7 +41,6 @@
 #define ERR_BREDR_CONN_LMP_PROTO_ERROR		"br-connection-lmp-protocol-"\
 						"error"
 #define ERR_BREDR_CONN_CANCELED			"br-connection-canceled"
-#define ERR_BREDR_CONN_KEY_MISSING		"br-connection-key-missing"
 #define ERR_BREDR_CONN_UNKNOWN			"br-connection-unknown"
 
 /* LE connection failure reasons */
@@ -62,7 +61,6 @@
 #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_KEY_MISSING		"le-connection-key-missing"
 #define ERR_LE_CONN_UNKNOWN		"le-connection-unknown"
 
 DBusMessage *btd_error_invalid_args(DBusMessage *msg);
@@ -84,6 +82,7 @@ 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_profile_unavailable(DBusMessage *msg);
+DBusMessage *btd_error_br_connection_key_missing(DBusMessage *msg);
 DBusMessage *btd_error_failed(DBusMessage *msg, const char *str);
 DBusMessage *btd_error_bredr_conn_from_errno(DBusMessage *msg, int errno_code);
 DBusMessage *btd_error_le_conn_from_errno(DBusMessage *msg, int errno_code);
-- 
2.50.0

[PATCH BlueZ v4 8/8] device: Better error when the adapter is not powered

[Bastien Nocera]

---
 doc/org.bluez.Device.rst |  1 +
 src/device.c             |  6 ++----
 src/error.c              | 15 +++++++++++----
 src/error.h              |  1 +
 4 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/doc/org.bluez.Device.rst b/doc/org.bluez.Device.rst
index d4de78a47239..bf8816332070 100644
--- a/doc/org.bluez.Device.rst
+++ b/doc/org.bluez.Device.rst
@@ -52,6 +52,7 @@ Possible errors:
 :org.bluez.Error.ProfileUnavailable:
 :org.bluez.Error.BrConnectionKeyMissing:
 :org.bluez.Error.LeConnectionKeyMissing:
+:org.bluez.Error.AdapterNotPowered:
 
 void Disconnect()
 `````````````````
diff --git a/src/device.c b/src/device.c
index 3bad7fb2c77c..a48f702bb6a9 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2666,10 +2666,8 @@ static DBusMessage *connect_profiles(struct btd_device *dev, uint8_t bdaddr_type
 	if (dev->pending || dev->connect || dev->browse)
 		return btd_error_in_progress_str(msg, ERR_BREDR_CONN_BUSY);
 
-	if (!btd_adapter_get_powered(dev->adapter)) {
-		return btd_error_not_ready_str(msg,
-					ERR_BREDR_CONN_ADAPTER_NOT_POWERED);
-	}
+	if (!btd_adapter_get_powered(dev->adapter))
+		return btd_error_adapter_not_powered(msg);
 
 	btd_device_set_temporary(dev, false);
 
diff --git a/src/error.c b/src/error.c
index 596ea4280991..8f99e274bdcd 100644
--- a/src/error.c
+++ b/src/error.c
@@ -150,6 +150,13 @@ static DBusMessage *btd_error_le_connection_key_missing(DBusMessage *msg)
 					"LE Link Key missing");
 }
 
+DBusMessage *btd_error_adapter_not_powered(DBusMessage *msg)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE
+					".AdapterNotPowered",
+					"Adapter not powered");
+}
+
 DBusMessage *btd_error_failed(DBusMessage *msg, const char *str)
 {
 	return g_dbus_create_error(msg, ERROR_INTERFACE
@@ -168,8 +175,6 @@ static const char *btd_error_str_bredr_conn_from_errno(int errno_code)
 		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;
@@ -201,8 +206,6 @@ static const char *btd_error_str_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;
@@ -239,6 +242,8 @@ DBusMessage *btd_error_bredr_conn_from_errno(DBusMessage *msg, int errno_code)
 	switch (-errno_code) {
 	case EBADE:
 		return btd_error_br_connection_key_missing(msg);
+	case EHOSTUNREACH:
+		return btd_error_adapter_not_powered(msg);
 	case ENOPROTOOPT:
 		return btd_error_profile_unavailable(msg);
 	default:
@@ -252,6 +257,8 @@ DBusMessage *btd_error_le_conn_from_errno(DBusMessage *msg, int errno_code)
 	switch (-errno_code) {
 	case EBADE:
 		return btd_error_le_connection_key_missing(msg);
+	case EHOSTUNREACH:
+		return btd_error_adapter_not_powered(msg);
 	default:
 		return btd_error_failed(msg,
 				btd_error_str_le_conn_from_errno(errno_code));
diff --git a/src/error.h b/src/error.h
index 96b7a7122457..d51e84ed6679 100644
--- a/src/error.h
+++ b/src/error.h
@@ -83,6 +83,7 @@ DBusMessage *btd_error_not_ready(DBusMessage *msg);
 DBusMessage *btd_error_not_ready_str(DBusMessage *msg, const char *str);
 DBusMessage *btd_error_profile_unavailable(DBusMessage *msg);
 DBusMessage *btd_error_br_connection_key_missing(DBusMessage *msg);
+DBusMessage *btd_error_adapter_not_powered(DBusMessage *msg);
 DBusMessage *btd_error_failed(DBusMessage *msg, const char *str);
 DBusMessage *btd_error_bredr_conn_from_errno(DBusMessage *msg, int errno_code);
 DBusMessage *btd_error_le_conn_from_errno(DBusMessage *msg, int errno_code);
-- 
2.50.0

RE: Better connect debug and error reporting

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1261 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=977659

---Test result---

Test Summary:
CheckPatch                    PENDING   0.38 seconds
GitLint                       PENDING   0.36 seconds
BuildEll                      PASS      22.05 seconds
BluezMake                     PASS      2777.64 seconds
MakeCheck                     PASS      20.08 seconds
MakeDistcheck                 PASS      216.08 seconds
CheckValgrind                 PASS      284.01 seconds
CheckSmatch                   PASS      310.02 seconds
bluezmakeextell               PASS      133.02 seconds
IncrementalBuild              PENDING   0.32 seconds
ScanBuild                     PASS      961.27 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

Re: [PATCH BlueZ v4 6/8] device: Better "Connect" debug

[Luiz Augusto von Dentz]

Hi Bastien,

On Tue, Jul 1, 2025 at 6:20 AM Bastien Nocera <hadess@hadess.net> wrote:
>
> Output clearer debug information so that it's possible to follow the
> decisions made by the bluetoothd daemon when a client such as
> bluetoothctl or the GNOME Bluetooth settings ask it to connect to a
> device.

Well this will only output to syslog though, so the client won't
necessarily see any of this, and this actually requires the daemon to
be running with debug enabled.

> ---
>  src/device.c | 41 +++++++++++++++++++++++++++++++++--------
>  1 file changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/src/device.c b/src/device.c
> index 99c0aa67ec0c..d7a859f9df3f 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -2683,6 +2683,7 @@ static DBusMessage *connect_profiles(struct btd_device *dev, uint8_t bdaddr_type
>                                                         "Connect") &&
>                                 find_service_with_state(dev->services,
>                                                 BTD_SERVICE_STATE_CONNECTED)) {
> +                               DBG("Already connected to services");
>                                 return dbus_message_new_method_return(msg);
>                         } else {
>                                 return btd_error_profile_unavailable(msg);
> @@ -2694,8 +2695,10 @@ static DBusMessage *connect_profiles(struct btd_device *dev, uint8_t bdaddr_type
>
>         err = connect_next(dev);
>         if (err < 0) {
> -               if (err == -EALREADY)
> +               if (err == -EALREADY) {
> +                       DBG("Already connected");
>                         return dbus_message_new_method_return(msg);
> +               }
>                 return btd_error_bredr_conn_from_errno(msg, err);
>         }
>
> @@ -2718,14 +2721,24 @@ resolve_services:
>         return NULL;
>  }
>
> +static const char *bdaddr_type_strs[] = {
> +       "BR/EDR",
> +       "LE public",
> +       "LE random"
> +};
> +
>  static DBusMessage *dev_connect(DBusConnection *conn, DBusMessage *msg,
>                                                         void *user_data)
>  {
>         struct btd_device *dev = user_data;
>         uint8_t bdaddr_type;
>
> -       if (dev->bonding)
> +       DBG("Calling \"Connect\" for device %s", dev->path);

We do have D-Bus message logging so this seems excessive to me.

> +
> +       if (dev->bonding) {
> +               DBG("Bonding in progress");
>                 return btd_error_in_progress(msg);
> +       }
>
>         if (dev->bredr_state.connected) {
>                 /*
> @@ -2734,23 +2747,35 @@ static DBusMessage *dev_connect(DBusConnection *conn, DBusMessage *msg,
>                  */
>                 if (dev->bredr_state.svc_resolved &&
>                         find_service_with_state(dev->services,
> -                                               BTD_SERVICE_STATE_CONNECTED))
> +                                               BTD_SERVICE_STATE_CONNECTED)) {
>                         bdaddr_type = dev->bdaddr_type;
> -               else
> +                       DBG("Selecting address type %s, as BR/EDR services are resolved "
> +                           " and connected", bdaddr_type_strs[dev->bdaddr_type]);
> +               } else {
>                         bdaddr_type = BDADDR_BREDR;
> -       } else if (dev->le_state.connected && dev->bredr)
> +                       DBG("Selecting address type BR/EDR, as services not resolved "
> +                           "or not connected");
> +               }
> +       } else if (dev->le_state.connected && dev->bredr) {
>                 bdaddr_type = BDADDR_BREDR;
> -       else
> +               DBG("Selecting address type BR/EDR, as LE already connected");
> +       } else {
>                 bdaddr_type = select_conn_bearer(dev);
> +               DBG("Selecting address type %s", bdaddr_type_strs[dev->bdaddr_type]);

Don't really like this many calls for something so simple, beside we
now have the likes of PreferredBearer for the client to indicate what
bearer it wants to connect, otherwise if we have to explain every
decision we make we basically need to log the entire code, so we
better log what not why.

> +       }
>
>         if (bdaddr_type != BDADDR_BREDR) {
>                 int err;
>
> -               if (dev->connect)
> +               if (dev->connect) {
> +                       DBG("Device already connecting");
>                         return btd_error_in_progress(msg);
> +               }
>
> -               if (dev->le_state.connected)
> +               if (dev->le_state.connected) {
> +                       DBG("Device already connected through LE");
>                         return dbus_message_new_method_return(msg);
> +               }
>
>                 btd_device_set_temporary(dev, false);
>
> --
> 2.50.0
>
>


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ v4 7/8] device: Better error when the link key is missing

[Luiz Augusto von Dentz]

Hi Bastien,

On Tue, Jul 1, 2025 at 6:20 AM Bastien Nocera <hadess@hadess.net> wrote:
>
> Add a more precise error when the link key is missing for a
> profile or device we're trying to connect to.
> ---
>  doc/org.bluez.Device.rst |  2 ++
>  src/device.c             | 12 ++++++------
>  src/error.c              | 27 ++++++++++++++++++++++-----
>  src/error.h              |  3 +--
>  4 files changed, 31 insertions(+), 13 deletions(-)
>
> diff --git a/doc/org.bluez.Device.rst b/doc/org.bluez.Device.rst
> index b36a49eabdd5..d4de78a47239 100644
> --- a/doc/org.bluez.Device.rst
> +++ b/doc/org.bluez.Device.rst
> @@ -50,6 +50,8 @@ Possible errors:
>  :org.bluez.Error.InProgress:
>  :org.bluez.Error.AlreadyConnected:
>  :org.bluez.Error.ProfileUnavailable:
> +:org.bluez.Error.BrConnectionKeyMissing:
> +:org.bluez.Error.LeConnectionKeyMissing:

Id go with org.bluez.Error.BREDR.KeyMissing and org.bluez.Error.LE.KeyMissing

>
>  void Disconnect()
>  `````````````````
> diff --git a/src/device.c b/src/device.c
> index d7a859f9df3f..3bad7fb2c77c 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -1982,14 +1982,14 @@ void device_request_disconnect(struct btd_device *device, DBusMessage *msg)
>         }
>
>         if (device->connect) {
> -               const char *err_str;
>                 DBusMessage *reply;
>
> -               if (device->bonding_status == MGMT_STATUS_AUTH_FAILED)
> -                       err_str = ERR_BREDR_CONN_KEY_MISSING;
> -               else
> -                       err_str = ERR_BREDR_CONN_CANCELED;
> -               reply = btd_error_failed(device->connect, err_str);
> +               if (device->bonding_status == MGMT_STATUS_AUTH_FAILED) {
> +                       reply = btd_error_br_connection_key_missing(device->connect);
> +               } else {
> +                       reply = btd_error_failed(device->connect,
> +                                               ERR_BREDR_CONN_CANCELED);
> +               }
>                 g_dbus_send_message(dbus_conn, reply);
>                 dbus_message_unref(device->connect);
>                 device->bonding_status = 0;
> diff --git a/src/error.c b/src/error.c
> index 8070bc6107ae..596ea4280991 100644
> --- a/src/error.c
> +++ b/src/error.c
> @@ -136,6 +136,20 @@ DBusMessage *btd_error_profile_unavailable(DBusMessage *msg)
>                                         "profiles to connect to");
>  }
>
> +DBusMessage *btd_error_br_connection_key_missing(DBusMessage *msg)
> +{
> +       return g_dbus_create_error(msg, ERROR_INTERFACE
> +                                       ".BrConnectionKeyMissing",
> +                                       "BR/EDR Link Key missing");
> +}
> +
> +static DBusMessage *btd_error_le_connection_key_missing(DBusMessage *msg)
> +{
> +       return g_dbus_create_error(msg, ERROR_INTERFACE
> +                                       ".LeConnectionKeyMissing",
> +                                       "LE Link Key missing");
> +}
> +
>  DBusMessage *btd_error_failed(DBusMessage *msg, const char *str)
>  {
>         return g_dbus_create_error(msg, ERROR_INTERFACE
> @@ -177,8 +191,6 @@ static const char *btd_error_str_bredr_conn_from_errno(int errno_code)
>                 return ERR_BREDR_CONN_ABORT_BY_LOCAL;
>         case EPROTO:
>                 return ERR_BREDR_CONN_LMP_PROTO_ERROR;
> -       case EBADE:
> -               return ERR_BREDR_CONN_KEY_MISSING;
>         default:
>                 return ERR_BREDR_CONN_UNKNOWN;
>         }
> @@ -217,8 +229,6 @@ static const char *btd_error_str_le_conn_from_errno(int errno_code)
>                 return ERR_LE_CONN_ABORT_BY_LOCAL;
>         case EPROTO:
>                 return ERR_LE_CONN_LL_PROTO_ERROR;
> -       case EBADE:
> -               return ERR_LE_CONN_KEY_MISSING;
>         default:
>                 return ERR_LE_CONN_UNKNOWN;
>         }
> @@ -227,6 +237,8 @@ static const char *btd_error_str_le_conn_from_errno(int errno_code)
>  DBusMessage *btd_error_bredr_conn_from_errno(DBusMessage *msg, int errno_code)
>  {
>         switch (-errno_code) {
> +       case EBADE:
> +               return btd_error_br_connection_key_missing(msg);
>         case ENOPROTOOPT:
>                 return btd_error_profile_unavailable(msg);
>         default:
> @@ -237,6 +249,11 @@ DBusMessage *btd_error_bredr_conn_from_errno(DBusMessage *msg, int errno_code)
>
>  DBusMessage *btd_error_le_conn_from_errno(DBusMessage *msg, int errno_code)
>  {
> -       return btd_error_failed(msg,
> +       switch (-errno_code) {
> +       case EBADE:
> +               return btd_error_le_connection_key_missing(msg);
> +       default:
> +               return btd_error_failed(msg,
>                                 btd_error_str_le_conn_from_errno(errno_code));
> +       }
>  }
> diff --git a/src/error.h b/src/error.h
> index f4ad81e5daa5..96b7a7122457 100644
> --- a/src/error.h
> +++ b/src/error.h
> @@ -41,7 +41,6 @@
>  #define ERR_BREDR_CONN_LMP_PROTO_ERROR         "br-connection-lmp-protocol-"\
>                                                 "error"
>  #define ERR_BREDR_CONN_CANCELED                        "br-connection-canceled"
> -#define ERR_BREDR_CONN_KEY_MISSING             "br-connection-key-missing"
>  #define ERR_BREDR_CONN_UNKNOWN                 "br-connection-unknown"
>
>  /* LE connection failure reasons */
> @@ -62,7 +61,6 @@
>  #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_KEY_MISSING                "le-connection-key-missing"
>  #define ERR_LE_CONN_UNKNOWN            "le-connection-unknown"
>
>  DBusMessage *btd_error_invalid_args(DBusMessage *msg);
> @@ -84,6 +82,7 @@ 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_profile_unavailable(DBusMessage *msg);
> +DBusMessage *btd_error_br_connection_key_missing(DBusMessage *msg);
>  DBusMessage *btd_error_failed(DBusMessage *msg, const char *str);
>  DBusMessage *btd_error_bredr_conn_from_errno(DBusMessage *msg, int errno_code);
>  DBusMessage *btd_error_le_conn_from_errno(DBusMessage *msg, int errno_code);
> --
> 2.50.0
>
>


-- 
Luiz Augusto von Dentz