[PATCH BlueZ 0/3] Propagate disconnection reason

[PATCH BlueZ 0/3] Propagate disconnection reason

[Frédéric Danis]

Currently a client application is informed of the disconnection by the
update of the Connected property to false.
This sends a Disconnected signal with the disconnection reason before
the property is updated.

This will help client application to know the reason for the
disconnection and to take appropriate action.

bluetoothctl is updated to display the disconnection reason on reception
of the signal.

This can be tested in bluetoothctl by disconnecting a device, which
generates:
[SIGNAL] org.bluez.Device1.Disconnected disconnection-local-host

Frédéric Danis (3):
  src/device: Add Disconnected signal to propagate disconnection reason
  doc/device: Add Disconnected signal
  client: Display disconnection reason

 client/main.c            | 20 ++++++++++++++++++++
 doc/org.bluez.Device.rst | 17 +++++++++++++++++
 src/adapter.c            | 13 ++++++++-----
 src/device.c             | 37 +++++++++++++++++++++++++++++++++++--
 src/device.h             |  3 ++-
 5 files changed, 82 insertions(+), 8 deletions(-)

-- 
2.43.0

[PATCH BlueZ 1/3] src/device: Add Disconnected signal to propagate disconnection reason

[Frédéric Danis]

Currently a client application is informed of the disconnection by the
update of the Connected property to false.
This sends a Disconnected signal with the disconnection reason before
the property is updated.

This helps client application to know the reason for the disconnection
and to take appropriate action.
---
 src/adapter.c | 13 ++++++++-----
 src/device.c  | 37 +++++++++++++++++++++++++++++++++++--
 src/device.h  |  3 ++-
 3 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index fd425e6d2..a10721489 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -7549,7 +7549,8 @@ struct agent *adapter_get_agent(struct btd_adapter *adapter)
 
 static void adapter_remove_connection(struct btd_adapter *adapter,
 						struct btd_device *device,
-						uint8_t bdaddr_type)
+						uint8_t bdaddr_type,
+						uint8_t reason)
 {
 	bool remove_device = false;
 
@@ -7560,7 +7561,7 @@ static void adapter_remove_connection(struct btd_adapter *adapter,
 		return;
 	}
 
-	device_remove_connection(device, bdaddr_type, &remove_device);
+	device_remove_connection(device, bdaddr_type, &remove_device, reason);
 
 	device_cancel_authentication(device, TRUE);
 
@@ -7601,9 +7602,11 @@ static void adapter_stop(struct btd_adapter *adapter)
 		struct btd_device *device = adapter->connections->data;
 		uint8_t addr_type = btd_device_get_bdaddr_type(device);
 
-		adapter_remove_connection(adapter, device, BDADDR_BREDR);
+		adapter_remove_connection(adapter, device, BDADDR_BREDR,
+						MGMT_DEV_DISCONN_UNKNOWN);
 		if (addr_type != BDADDR_BREDR)
-			adapter_remove_connection(adapter, device, addr_type);
+			adapter_remove_connection(adapter, device, addr_type,
+						MGMT_DEV_DISCONN_UNKNOWN);
 	}
 
 	g_dbus_emit_property_changed(dbus_conn, adapter->path,
@@ -8551,7 +8554,7 @@ static void dev_disconnected(struct btd_adapter *adapter,
 
 	device = btd_adapter_find_device(adapter, &addr->bdaddr, addr->type);
 	if (device) {
-		adapter_remove_connection(adapter, device, addr->type);
+		adapter_remove_connection(adapter, device, addr->type, reason);
 		disconnect_notify(device, reason);
 	}
 
diff --git a/src/device.c b/src/device.c
index d230af0a8..16e880f71 100644
--- a/src/device.c
+++ b/src/device.c
@@ -3417,6 +3417,12 @@ static const GDBusMethodTable device_methods[] = {
 	{ }
 };
 
+static const GDBusSignalTable device_signals[] = {
+	{ GDBUS_SIGNAL("Disconnected",
+			GDBUS_ARGS({ "reason", "s" })) },
+	{ }
+};
+
 static gboolean
 dev_property_get_prefer_bearer(const GDBusPropertyTable *property,
 				DBusMessageIter *iter, void *data)
@@ -3637,13 +3643,34 @@ static void set_temporary_timer(struct btd_device *dev, unsigned int timeout)
 								dev, NULL);
 }
 
+static const char *disconnect_reason(uint8_t reason)
+{
+	switch (reason) {
+	case MGMT_DEV_DISCONN_UNKNOWN:
+		return "disconnection-unknown";
+	case MGMT_DEV_DISCONN_TIMEOUT:
+		return "disconnection-timeout";
+	case MGMT_DEV_DISCONN_LOCAL_HOST:
+		return "disconnection-local-host";
+	case MGMT_DEV_DISCONN_REMOTE:
+		return "disconnection-remote";
+	case MGMT_DEV_DISCONN_LOCAL_HOST_SUSPEND:
+		return "disconnection-local-suspend";
+	default:
+		warn("Unknown disocnnection value: %u", reason);
+		return "disconnection-unknown";
+	}
+}
+
 void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type,
-								bool *remove)
+								bool *remove,
+								uint8_t reason)
 {
 	struct bearer_state *state = get_state(device, bdaddr_type);
 	DBusMessage *reply;
 	bool remove_device = false;
 	bool paired_status_updated = false;
+	const char *str_reason;
 
 	if (!state->connected)
 		return;
@@ -3708,6 +3735,12 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type,
 	g_slist_free_full(device->eir_uuids, g_free);
 	device->eir_uuids = NULL;
 
+	str_reason = disconnect_reason(reason);
+	g_dbus_emit_signal(dbus_conn, device->path, DEVICE_INTERFACE,
+						"Disconnected",
+						DBUS_TYPE_STRING, &str_reason,
+						DBUS_TYPE_INVALID);
+
 	g_dbus_emit_property_changed(dbus_conn, device->path,
 						DEVICE_INTERFACE, "Connected");
 
@@ -4611,7 +4644,7 @@ static struct btd_device *device_new(struct btd_adapter *adapter,
 
 	if (g_dbus_register_interface(dbus_conn,
 					device->path, DEVICE_INTERFACE,
-					device_methods, NULL,
+					device_methods, device_signals,
 					device_properties, device,
 					device_free) == FALSE) {
 		error("Unable to register device interface for %s", address);
diff --git a/src/device.h b/src/device.h
index a35bb1386..4eebcebe9 100644
--- a/src/device.h
+++ b/src/device.h
@@ -134,7 +134,8 @@ gboolean device_is_authenticating(struct btd_device *device);
 void device_add_connection(struct btd_device *dev, uint8_t bdaddr_type,
 							uint32_t flags);
 void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type,
-								bool *remove);
+							bool *remove,
+							uint8_t reason);
 void device_request_disconnect(struct btd_device *device, DBusMessage *msg);
 bool device_is_disconnecting(struct btd_device *device);
 void device_set_ltk(struct btd_device *device, const uint8_t val[16],
-- 
2.43.0

RE: Propagate disconnection reason

[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=964261

---Test result---

Test Summary:
CheckPatch                    PENDING   0.26 seconds
GitLint                       PENDING   0.33 seconds
BuildEll                      PASS      19.99 seconds
BluezMake                     PASS      2726.44 seconds
MakeCheck                     PASS      20.36 seconds
MakeDistcheck                 PASS      197.77 seconds
CheckValgrind                 PASS      272.16 seconds
CheckSmatch                   PASS      297.58 seconds
bluezmakeextell               PASS      126.66 seconds
IncrementalBuild              PENDING   0.32 seconds
ScanBuild                     PASS      899.13 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

[PATCH BlueZ 2/3] doc/device: Add Disconnected signal

[Frédéric Danis]

---
 doc/org.bluez.Device.rst | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/doc/org.bluez.Device.rst b/doc/org.bluez.Device.rst
index 80501eddd..6229f95ad 100644
--- a/doc/org.bluez.Device.rst
+++ b/doc/org.bluez.Device.rst
@@ -155,6 +155,23 @@ array{array{byte}} GetServiceRecords() [experimental]
 	:org.bluez.Error.NotConnected:
 	:org.bluez.Error.DoesNotExist:
 
+Signals
+-------
+
+void Disconnected(string reason)
+````````````````````````````````
+
+	This signal is launched when a device is disconnected with the reason of
+	the disconnection.
+
+	Possible reasons:
+
+	:disconnection-unknown:
+	:disconnection-timeout:
+	:disconnection-local-host:
+	:disconnection-remote:
+	:disconnection-local-suspend:
+
 Properties
 ----------
 
-- 
2.43.0

Re: [PATCH BlueZ 2/3] doc/device: Add Disconnected signal

[Luiz Augusto von Dentz]

Hi Frédéric,

On Mon, May 19, 2025 at 12:18 PM Frédéric Danis
<frederic.danis@collabora.com> wrote:
>
> ---
>  doc/org.bluez.Device.rst | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/doc/org.bluez.Device.rst b/doc/org.bluez.Device.rst
> index 80501eddd..6229f95ad 100644
> --- a/doc/org.bluez.Device.rst
> +++ b/doc/org.bluez.Device.rst
> @@ -155,6 +155,23 @@ array{array{byte}} GetServiceRecords() [experimental]
>         :org.bluez.Error.NotConnected:
>         :org.bluez.Error.DoesNotExist:
>
> +Signals
> +-------
> +
> +void Disconnected(string reason)
> +````````````````````````````````
> +
> +       This signal is launched when a device is disconnected with the reason of
> +       the disconnection.
> +
> +       Possible reasons:
> +
> +       :disconnection-unknown:
> +       :disconnection-timeout:
> +       :disconnection-local-host:
> +       :disconnection-remote:
> +       :disconnection-local-suspend:

Perhaps it would be better to use to the actual HCI code instead of
converting it to string, since I suspect application using this signal
may want to recover the actual error to do some sort of reconnecting
policy, etc, or having them both in case the client just wants to
print it.

> +
>  Properties
>  ----------
>
> --
> 2.43.0
>
>


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 2/3] doc/device: Add Disconnected signal

[Bastien Nocera]

On Mon, 2025-05-19 at 12:44 -0400, Luiz Augusto von Dentz wrote:
> Hi Frédéric,
> 
> On Mon, May 19, 2025 at 12:18 PM Frédéric Danis
> <frederic.danis@collabora.com> wrote:
> > 
> > ---
> >  doc/org.bluez.Device.rst | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/doc/org.bluez.Device.rst b/doc/org.bluez.Device.rst
> > index 80501eddd..6229f95ad 100644
> > --- a/doc/org.bluez.Device.rst
> > +++ b/doc/org.bluez.Device.rst
> > @@ -155,6 +155,23 @@ array{array{byte}} GetServiceRecords()
> > [experimental]
> >         :org.bluez.Error.NotConnected:
> >         :org.bluez.Error.DoesNotExist:
> > 
> > +Signals
> > +-------
> > +
> > +void Disconnected(string reason)
> > +````````````````````````````````
> > +
> > +       This signal is launched when a device is disconnected with
> > the reason of
> > +       the disconnection.
> > +
> > +       Possible reasons:
> > +
> > +       :disconnection-unknown:
> > +       :disconnection-timeout:
> > +       :disconnection-local-host:
> > +       :disconnection-remote:
> > +       :disconnection-local-suspend:
> 
> Perhaps it would be better to use to the actual HCI code instead of
> converting it to string, since I suspect application using this
> signal
> may want to recover the actual error to do some sort of reconnecting
> policy, etc, or having them both in case the client just wants to
> print it.

If there are applications using those signals (I'm guessing, Bluetooth
settings apps), whatever the format of the error, could we have an
expected behaviour associated with individual error types?

Re: [PATCH BlueZ 2/3] doc/device: Add Disconnected signal

[Frédéric Danis]

Hi Bastien,

On 19/05/2025 21:22, Bastien Nocera wrote:
> On Mon, 2025-05-19 at 12:44 -0400, Luiz Augusto von Dentz wrote:
>> Hi Frédéric,
>>
>> On Mon, May 19, 2025 at 12:18 PM Frédéric Danis
>> <frederic.danis@collabora.com> wrote:
>>> ---
>>>   doc/org.bluez.Device.rst | 17 +++++++++++++++++
>>>   1 file changed, 17 insertions(+)
>>>
>>> diff --git a/doc/org.bluez.Device.rst b/doc/org.bluez.Device.rst
>>> index 80501eddd..6229f95ad 100644
>>> --- a/doc/org.bluez.Device.rst
>>> +++ b/doc/org.bluez.Device.rst
>>> @@ -155,6 +155,23 @@ array{array{byte}} GetServiceRecords()
>>> [experimental]
>>>          :org.bluez.Error.NotConnected:
>>>          :org.bluez.Error.DoesNotExist:
>>>
>>> +Signals
>>> +-------
>>> +
>>> +void Disconnected(string reason)
>>> +````````````````````````````````
>>> +
>>> +       This signal is launched when a device is disconnected with
>>> the reason of
>>> +       the disconnection.
>>> +
>>> +       Possible reasons:
>>> +
>>> +       :disconnection-unknown:
>>> +       :disconnection-timeout:
>>> +       :disconnection-local-host:
>>> +       :disconnection-remote:
>>> +       :disconnection-local-suspend:
>> Perhaps it would be better to use to the actual HCI code instead of
>> converting it to string, since I suspect application using this
>> signal
>> may want to recover the actual error to do some sort of reconnecting
>> policy, etc, or having them both in case the client just wants to
>> print it.
> If there are applications using those signals (I'm guessing, Bluetooth
> settings apps), whatever the format of the error, could we have an
> expected behaviour associated with individual error types?

This could be used by client apps like Bluetooth setting to try to
reconnect to the device in case of timeout or unknown disconnection,
or to try to connect to another device depending on internal policy.

-- 
Frédéric Danis
Senior Software Engineer

Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, United Kingdom
Registered in England & Wales, no. 5513718

Re: [PATCH BlueZ 2/3] doc/device: Add Disconnected signal

[Bastien Nocera]

On Tue, 2025-05-20 at 11:32 +0200, Frédéric Danis wrote:
> Hi Bastien,
> 
> On 19/05/2025 21:22, Bastien Nocera wrote:
> > On Mon, 2025-05-19 at 12:44 -0400, Luiz Augusto von Dentz wrote:
> > > Hi Frédéric,
> > > 
> > > On Mon, May 19, 2025 at 12:18 PM Frédéric Danis
> > > <frederic.danis@collabora.com> wrote:
> > > > ---
> > > >   doc/org.bluez.Device.rst | 17 +++++++++++++++++
> > > >   1 file changed, 17 insertions(+)
> > > > 
> > > > diff --git a/doc/org.bluez.Device.rst
> > > > b/doc/org.bluez.Device.rst
> > > > index 80501eddd..6229f95ad 100644
> > > > --- a/doc/org.bluez.Device.rst
> > > > +++ b/doc/org.bluez.Device.rst
> > > > @@ -155,6 +155,23 @@ array{array{byte}} GetServiceRecords()
> > > > [experimental]
> > > >          :org.bluez.Error.NotConnected:
> > > >          :org.bluez.Error.DoesNotExist:
> > > > 
> > > > +Signals
> > > > +-------
> > > > +
> > > > +void Disconnected(string reason)
> > > > +````````````````````````````````
> > > > +
> > > > +       This signal is launched when a device is disconnected
> > > > with
> > > > the reason of
> > > > +       the disconnection.
> > > > +
> > > > +       Possible reasons:
> > > > +
> > > > +       :disconnection-unknown:
> > > > +       :disconnection-timeout:
> > > > +       :disconnection-local-host:
> > > > +       :disconnection-remote:
> > > > +       :disconnection-local-suspend:
> > > Perhaps it would be better to use to the actual HCI code instead
> > > of
> > > converting it to string, since I suspect application using this
> > > signal
> > > may want to recover the actual error to do some sort of
> > > reconnecting
> > > policy, etc, or having them both in case the client just wants to
> > > print it.
> > If there are applications using those signals (I'm guessing,
> > Bluetooth
> > settings apps), whatever the format of the error, could we have an
> > expected behaviour associated with individual error types?
> 
> This could be used by client apps like Bluetooth setting to try to
> reconnect to the device in case of timeout or unknown disconnection,
> or to try to connect to another device depending on internal policy.

I meant having that information in the docs :)

Cheers

Re: [PATCH BlueZ 2/3] doc/device: Add Disconnected signal

[Frédéric Danis]

Hi Luiz,

On 19/05/2025 18:44, Luiz Augusto von Dentz wrote:
> Hi Frédéric,
>
> On Mon, May 19, 2025 at 12:18 PM Frédéric Danis
> <frederic.danis@collabora.com> wrote:
>> ---
>>   doc/org.bluez.Device.rst | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/doc/org.bluez.Device.rst b/doc/org.bluez.Device.rst
>> index 80501eddd..6229f95ad 100644
>> --- a/doc/org.bluez.Device.rst
>> +++ b/doc/org.bluez.Device.rst
>> @@ -155,6 +155,23 @@ array{array{byte}} GetServiceRecords() [experimental]
>>          :org.bluez.Error.NotConnected:
>>          :org.bluez.Error.DoesNotExist:
>>
>> +Signals
>> +-------
>> +
>> +void Disconnected(string reason)
>> +````````````````````````````````
>> +
>> +       This signal is launched when a device is disconnected with the reason of
>> +       the disconnection.
>> +
>> +       Possible reasons:
>> +
>> +       :disconnection-unknown:
>> +       :disconnection-timeout:
>> +       :disconnection-local-host:
>> +       :disconnection-remote:
>> +       :disconnection-local-suspend:
> Perhaps it would be better to use to the actual HCI code instead of
> converting it to string, since I suspect application using this signal
> may want to recover the actual error to do some sort of reconnecting
> policy, etc, or having them both in case the client just wants to
> print it.

I will update the patch to use the numerical value.

But, the reason provided by MGMT_EV_DEVICE_DISCONNECTED is not the
HCI code but a mgmt value translated in net/bluetooth/hci_event.c
(https://github.com/bluez/bluetooth-next/blob/master/net/bluetooth/hci_event.c#L3366)

-- 
Frédéric Danis
Senior Software Engineer

Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, United Kingdom
Registered in England & Wales, no. 5513718

[PATCH BlueZ 3/3] client: Display disconnection reason

[Frédéric Danis]

The new org.bluez.Device1.Disconnected signal propagates the
disconnection reason.
---
 client/main.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/client/main.c b/client/main.c
index 57d71f2b6..79d0707bb 100644
--- a/client/main.c
+++ b/client/main.c
@@ -709,6 +709,26 @@ static void property_changed(GDBusProxy *proxy, const char *name,
 static void message_handler(DBusConnection *connection,
 					DBusMessage *message, void *user_data)
 {
+	if (!strcmp(dbus_message_get_member(message), "Disconnected")) {
+		DBusMessageIter iter;
+		const char *reason;
+
+		if (!dbus_message_iter_init(message, &iter))
+			goto failed;
+
+		if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+			goto failed;
+
+		dbus_message_iter_get_basic(&iter, &reason);
+
+		bt_shell_printf("[SIGNAL] %s.%s %s\n",
+					dbus_message_get_interface(message),
+					dbus_message_get_member(message),
+					reason);
+		return;
+	}
+
+failed:
 	bt_shell_printf("[SIGNAL] %s.%s\n", dbus_message_get_interface(message),
 					dbus_message_get_member(message));
 }
-- 
2.43.0