From: Mikel Astiz <[email protected]>
Some use-cases require distinguishing the reason behind an ACL disconnection. Particularly, IVI use-cases regarding HFP and A2DP need to know if a disconnection was due to a timeout, in order to know which should be the reconnection strategy.
This information needs to be exposed to some component implementing the specific policy of each application. Typically neither BlueZ or oFono (in case of HFP) would be the case. This means the disconnect reason needs to be exposed in D-Bus.
In this patch series the proposed approach is to add a signal in the device API, given that we already have some low-level disconnection-related API there. Another approach would be to support it in the specific HFP and A2DP interfaces.
In addition, the kernel doesn't provide this information when using the management interface. Either the management interface is extended (the corresponding patch will be sent soon) or this field needs to be reported through a RFCOMM socket option after a POLLHUP disconnect event. This second approach would require to handle it separatedly in userspace for HFP and A2DP (and potentially for other profiles), and besides, assuming BlueZ is responsible for exposing the field in D-Bus, it would be necessary to hold a copy of the RFCOMM socket.
Mikel Astiz (5):
doc: Add D-Bus signal to provide disconnect reason
Use defines instead of magic numbers
Propagate disconnect reason to adapter
Propagate disconnect reason to device
Send D-Bus signal to propagate disconnect reason
doc/device-api.txt | 14 ++++++++++++++
lib/mgmt.h | 5 +++++
plugins/hciops.c | 2 +-
plugins/mgmtops.c | 10 ++++++++--
src/adapter.c | 7 ++++---
src/adapter.h | 2 +-
src/device.c | 51 +++++++++++++++++++++++++++++----------------------
src/device.h | 3 ++-
src/event.c | 4 ++--
src/event.h | 3 ++-
10 files changed, 68 insertions(+), 33 deletions(-)
--
1.7.7.6
Hi Luiz,
On Thu, May 17, 2012 at 8:41 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Mike,
>
> On Wed, May 16, 2012 at 5:28 PM, Mike <[email protected]> wrote:
>>> I do agree that anything that could cause IOP problems should be
>>> handled inside BlueZ. And the immediate reconnection try after a link
>>> loss does sound an example of this kind. So I would be fine if BlueZ
>>> internally handles this, but I still think we need to expose this in
>>> D-Bus somehow.
>>
>> The reason this is per model is that there is no spec on this and
>> hence it will be very implementation specific. ?If you are a desktop
>> hooked up to AC, you really don't care, but an embedded device will
>> need to implement something that meets its battery constraints. ?As
>> well, the efforts of bluez will be fruitless if the rest of the system
>> goes into suspend. ?In my case, my device goes to suspend as soon as
>> possible (and wakes if there is BT UART traffic). ?Hence, timers that
>> implement re-connection need to let the system know when the next
>> attempt will occur.
>
> Yes, but that can be implemented in audio.conf, so you can enter the
> number of retries and the interval between them that you want per
> platform, but I think 99% will just pick the default. Btw, you need to
> elaborate a bit more about your requirements, this work is focuses on
> IVI system where I don't expect the system to go into suspend that
> often.
I'll admit that my system, being embedded, is likely to always need
some custom bits that aren't in mainline. The point I'm trying to
make is that it'd be helpful if the implementation considered the fact
that not all devices want to stay awake all the time. Hence, it would
be good if there was some D-Bus notification that was explicit about
entering link loss recovery. When my device is asleep, it only wakes
up if the user presses buttons or there is BT HCI traffic. The simple
solution is to not enter suspend when link loss recovery is occurring,
but the rest of the system needs to know what bluetoothd is doing then
to prevent that. I'm not using Android, but I would assume that this
would equally apply there with wakelocks (this is in essence what I'm
doing, just in userspace).
Regardless, it's good that this is being worked on. It seems like
some complaints I find about BlueZ various places complain about it
not "just working" -- ignoring the fact that the BT spec doesn't call
this stuff out and it actually takes effort!
Mike
Hi Mike,
On Wed, May 16, 2012 at 5:28 PM, Mike <[email protected]> wrote:
>> I do agree that anything that could cause IOP problems should be
>> handled inside BlueZ. And the immediate reconnection try after a link
>> loss does sound an example of this kind. So I would be fine if BlueZ
>> internally handles this, but I still think we need to expose this in
>> D-Bus somehow.
>
> The reason this is per model is that there is no spec on this and
> hence it will be very implementation specific. ?If you are a desktop
> hooked up to AC, you really don't care, but an embedded device will
> need to implement something that meets its battery constraints. ?As
> well, the efforts of bluez will be fruitless if the rest of the system
> goes into suspend. ?In my case, my device goes to suspend as soon as
> possible (and wakes if there is BT UART traffic). ?Hence, timers that
> implement re-connection need to let the system know when the next
> attempt will occur.
Yes, but that can be implemented in audio.conf, so you can enter the
number of retries and the interval between them that you want per
platform, but I think 99% will just pick the default. Btw, you need to
elaborate a bit more about your requirements, this work is focuses on
IVI system where I don't expect the system to go into suspend that
often.
--
Luiz Augusto von Dentz
Hi Luiz, Mikel,
On Wed, May 16, 2012 at 2:12 AM, Mikel Astiz <[email protected]> wrote:
> Hi Luiz,
>
>>> From my point of view, the exact policy of which devices, how many of
>>> them, using which priorities, which profiles, etc. is manufacturer and
>>> model-specific. I don't think these policies should be embedded in
>>> BlueZ. After all, we already have a nice D-Bus API to handle these
>>> use-cases, and only the disconnect reason seems to be missing.
>>
>> Disconnect reason doesn't solve all the problems, specially in multi
>> profile case when just one profile has disconnected but the link is
>> still active. Besides that I would be very interested to know why each
>> model has to have a different behavior this goes completely in
>> opposite of having a common stack with a well defined behavior.
>
> The ACL disconnect case might not cover all use-cases, but it does
> cover the most important one: the link loss. Exposing if a specific
> profile has disconnected cleanly is IMO way too much.
You must know if the profile disconnected cleanly. The spec is very
clear that a user must initiate re-connection in the event of a clean
disconnect (via any means, rebooting a device, clicking a button,
etc). Somewhere there must be state stored that knows if the profile
was previously connected and if it needs to reconnect it.
>> When you say you want this specific per model all I can think of is
>> how much IOP problems it can cause and in the end I see no benefit for
>> the user, we should aim for re-connect as quick as possible and that
>> should be enough for any model.
>
> Let's not make a big deal out of this. We are talking about small
> differences in policies, such as how many devices and profiles are
> connected, and according to which priorities.
>
> I do agree that anything that could cause IOP problems should be
> handled inside BlueZ. And the immediate reconnection try after a link
> loss does sound an example of this kind. So I would be fine if BlueZ
> internally handles this, but I still think we need to expose this in
> D-Bus somehow.
The reason this is per model is that there is no spec on this and
hence it will be very implementation specific. If you are a desktop
hooked up to AC, you really don't care, but an embedded device will
need to implement something that meets its battery constraints. As
well, the efforts of bluez will be fruitless if the rest of the system
goes into suspend. In my case, my device goes to suspend as soon as
possible (and wakes if there is BT UART traffic). Hence, timers that
implement re-connection need to let the system know when the next
attempt will occur.
> One alternative to my original proposal -adding a Disconnected(uint8
> reason) signal- would be to use the state property of each interface
> (HFP, A2DP). If the autoreconnect policy is enabled, a link loss would
> generate the transition "connected"->"connecting" (or
> "playing"->"connecting"). Never setting the state to "disconnected"
> would effectively be equivalent as reporting a link loss.
That could appear awkward during the interval between connection attempts.
Mike
> Any thoughts on this second approach? Any other proposal?
Hi Luiz,
>> From my point of view, the exact policy of which devices, how many of
>> them, using which priorities, which profiles, etc. is manufacturer and
>> model-specific. I don't think these policies should be embedded in
>> BlueZ. After all, we already have a nice D-Bus API to handle these
>> use-cases, and only the disconnect reason seems to be missing.
>
> Disconnect reason doesn't solve all the problems, specially in multi
> profile case when just one profile has disconnected but the link is
> still active. Besides that I would be very interested to know why each
> model has to have a different behavior this goes completely in
> opposite of having a common stack with a well defined behavior.
The ACL disconnect case might not cover all use-cases, but it does
cover the most important one: the link loss. Exposing if a specific
profile has disconnected cleanly is IMO way too much.
> When you say you want this specific per model all I can think of is
> how much IOP problems it can cause and in the end I see no benefit for
> the user, we should aim for re-connect as quick as possible and that
> should be enough for any model.
Let's not make a big deal out of this. We are talking about small
differences in policies, such as how many devices and profiles are
connected, and according to which priorities.
I do agree that anything that could cause IOP problems should be
handled inside BlueZ. And the immediate reconnection try after a link
loss does sound an example of this kind. So I would be fine if BlueZ
internally handles this, but I still think we need to expose this in
D-Bus somehow.
One alternative to my original proposal -adding a Disconnected(uint8
reason) signal- would be to use the state property of each interface
(HFP, A2DP). If the autoreconnect policy is enabled, a link loss would
generate the transition "connected"->"connecting" (or
"playing"->"connecting"). Never setting the state to "disconnected"
would effectively be equivalent as reporting a link loss.
Any thoughts on this second approach? Any other proposal?
Cheers,
Mikel
Hi Mikel,
On Tue, May 15, 2012 at 11:45 AM, Mikel Astiz <[email protected]> wrote:
> We might be discussing about different reconnection policies. You are
> focusing on the short-term reconnection strategy, where several
> attempts can be made in order to reconnect as soon as possible and
> improve user experience. For this I agree BlueZ could handle the case.
>
> However this RFC was motivated by the overall connection policy
> (perhaps "reconnect" was not the right term here). A use-case can be a
> car with a paired phone and HFP is connected. The user leaves the car
> while still turned on (someone else could be driving) and a timeout
> would eventually occur. Next morning, when the user enters the car,
> his phone should be connected. On the contrary, if the user
> disconnected the HFP from the phone, this reconnection should not be
> triggered.
Yep, but that is a reboot case where you probably have to attempt to
connect to each device that was not disconnected cleanly.
> From my point of view, the exact policy of which devices, how many of
> them, using which priorities, which profiles, etc. is manufacturer and
> model-specific. I don't think these policies should be embedded in
> BlueZ. After all, we already have a nice D-Bus API to handle these
> use-cases, and only the disconnect reason seems to be missing.
Disconnect reason doesn't solve all the problems, specially in multi
profile case when just one profile has disconnected but the link is
still active. Besides that I would be very interested to know why each
model has to have a different behavior this goes completely in
opposite of having a common stack with a well defined behavior.
When you say you want this specific per model all I can think of is
how much IOP problems it can cause and in the end I see no benefit for
the user, we should aim for re-connect as quick as possible and that
should be enough for any model.
--
Luiz Augusto von Dentz
Hi Luiz,
> So if it is recommended by the white paper I don't see why it should
> be manufacture/model specific, note that only the number of attempts
> to re-connect is implementation dependent, but we can have an option
> in audio.conf where the platform configure the number of attempts and
> perhaps the interval between them if we cannot find a good default.
We might be discussing about different reconnection policies. You are
focusing on the short-term reconnection strategy, where several
attempts can be made in order to reconnect as soon as possible and
improve user experience. For this I agree BlueZ could handle the case.
However this RFC was motivated by the overall connection policy
(perhaps "reconnect" was not the right term here). A use-case can be a
car with a paired phone and HFP is connected. The user leaves the car
while still turned on (someone else could be driving) and a timeout
would eventually occur. Next morning, when the user enters the car,
his phone should be connected. On the contrary, if the user
disconnected the HFP from the phone, this reconnection should not be
triggered.
>From my point of view, the exact policy of which devices, how many of
them, using which priorities, which profiles, etc. is manufacturer and
model-specific. I don't think these policies should be embedded in
BlueZ. After all, we already have a nice D-Bus API to handle these
use-cases, and only the disconnect reason seems to be missing.
Cheers,
Mikel
Hi Mikel,
On Tue, May 15, 2012 at 9:46 AM, Mikel Astiz <[email protected]> wrote:
> The connection/reconnection policy is manufacturer and model-specific.
> Therefore I find it difficult that we could include this inside
> bluetoothd.
Well the Simultaneous Use of HFP, A2DP and AVRCP has a recommendation
to re-connect:
Recommendation 9:
An automatic re-connection policy can be used to re-connect the
signaling connections after link loss. In this case only the RD side
should reconnect. The number of attempts and periodicity are
implementation dependant, but it is recommended to have a relatively
small limit on the number of attempts to avoid battery drainage.
Motivation 9:
Since a link loss is an unexpected behavior for the user, an automatic
action of attempting to reconnect improves the user experience. MP
should not try to reconnect to avoid collisions.
So if it is recommended by the white paper I don't see why it should
be manufacture/model specific, note that only the number of attempts
to re-connect is implementation dependent, but we can have an option
in audio.conf where the platform configure the number of attempts and
perhaps the interval between them if we cannot find a good default.
> Having said that, I would be happy to review any proposal.
>
>> Btw, none of this is useful without tuning the link supervision
>> timeout because the current one used by BlueZ iirc is 20 seconds, far
>> too big for being able to recover the audio.
>
> That should definitely be tuned. However it's not always about
> recovering audio, but knowing to which phone the car should connect.
> With HFP there might be no audio and no voicecall during connection.
Still 20 seconds to detect link-loss will be noticeable by the user,
which is the whole point of the recommendation.
--
Luiz Augusto von Dentz
Hi Luiz,
On Mon, May 14, 2012 at 7:00 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Mikel,
>
> On Mon, May 14, 2012 at 6:32 PM, Mikel Astiz <[email protected]> wrote:
>> From: Mikel Astiz <[email protected]>
>>
>> Some use-cases require distinguishing the reason behind an ACL disconnection. Particularly, IVI use-cases regarding HFP and A2DP need to know if a disconnection was due to a timeout, in order to know which should be the reconnection strategy.
>
> Afaik there is only one use case which is link loss aka out of range,
> because of that carkit/headset policy is normally clean disconnect do
> not reconnect, non-clean disconnect do reconnect.
Right, with a few exceptions including clean disconnection during
head-unit shutdown.
>>
>> This information needs to be exposed to some component implementing the specific policy of each application. Typically neither BlueZ or oFono (in case of HFP) would be the case. This means the disconnect reason needs to be exposed in D-Bus.
>>
>> In this patch series the proposed approach is to add a signal in the device API, given that we already have some low-level disconnection-related API there. Another approach would be to support it in the specific HFP and A2DP interfaces.
>
> The disconnect reason might no be necessary, have you thought about
> implementing the reconnection logic directly in bluetoothd? This way
> we can test this without the UI, and we know when for example A2DP has
> disconnect without CLOSE command or things like that and afaik we can
> query the error from the socket.
The connection/reconnection policy is manufacturer and model-specific.
Therefore I find it difficult that we could include this inside
bluetoothd.
Having said that, I would be happy to review any proposal.
> Btw, none of this is useful without tuning the link supervision
> timeout because the current one used by BlueZ iirc is 20 seconds, far
> too big for being able to recover the audio.
That should definitely be tuned. However it's not always about
recovering audio, but knowing to which phone the car should connect.
With HFP there might be no audio and no voicecall during connection.
Cheers,
Mikel
Hi Mikel,
On Mon, May 14, 2012 at 6:32 PM, Mikel Astiz <[email protected]> wrote:
> From: Mikel Astiz <[email protected]>
>
> Some use-cases require distinguishing the reason behind an ACL disconnection. Particularly, IVI use-cases regarding HFP and A2DP need to know if a disconnection was due to a timeout, in order to know which should be the reconnection strategy.
Afaik there is only one use case which is link loss aka out of range,
because of that carkit/headset policy is normally clean disconnect do
not reconnect, non-clean disconnect do reconnect.
>
> This information needs to be exposed to some component implementing the specific policy of each application. Typically neither BlueZ or oFono (in case of HFP) would be the case. This means the disconnect reason needs to be exposed in D-Bus.
>
> In this patch series the proposed approach is to add a signal in the device API, given that we already have some low-level disconnection-related API there. Another approach would be to support it in the specific HFP and A2DP interfaces.
The disconnect reason might no be necessary, have you thought about
implementing the reconnection logic directly in bluetoothd? This way
we can test this without the UI, and we know when for example A2DP has
disconnect without CLOSE command or things like that and afaik we can
query the error from the socket.
Btw, none of this is useful without tuning the link supervision
timeout because the current one used by BlueZ iirc is 20 seconds, far
too big for being able to recover the audio.
--
Luiz Augusto von Dentz
From: Mikel Astiz <[email protected]>
Internally propagate disconnect reason in order to make it public in the
D-Bus API.
---
src/adapter.c | 2 +-
src/device.c | 3 ++-
src/device.h | 3 ++-
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index b9a90dd..4ed32a7 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -3001,7 +3001,7 @@ void adapter_remove_connection(struct btd_adapter *adapter,
return;
}
- device_remove_connection(device, connection);
+ device_remove_connection(device, connection, reason);
adapter->connections = g_slist_remove(adapter->connections, device);
diff --git a/src/device.c b/src/device.c
index 82dc2ca..b2b9d54 100644
--- a/src/device.c
+++ b/src/device.c
@@ -915,7 +915,8 @@ void device_add_connection(struct btd_device *device, DBusConnection *conn)
DBUS_TYPE_BOOLEAN, &device->connected);
}
-void device_remove_connection(struct btd_device *device, DBusConnection *conn)
+void device_remove_connection(struct btd_device *device, DBusConnection *conn,
+ uint8_t reason)
{
if (!device->connected) {
char addr[18];
diff --git a/src/device.h b/src/device.h
index 51140c7..f77d3aa 100644
--- a/src/device.h
+++ b/src/device.h
@@ -91,7 +91,8 @@ gboolean device_is_authenticating(struct btd_device *device);
gboolean device_is_authorizing(struct btd_device *device);
void device_set_authorizing(struct btd_device *device, gboolean auth);
void device_add_connection(struct btd_device *device, DBusConnection *conn);
-void device_remove_connection(struct btd_device *device, DBusConnection *conn);
+void device_remove_connection(struct btd_device *device, DBusConnection *conn,
+ uint8_t reason);
void device_request_disconnect(struct btd_device *device, DBusMessage *msg);
typedef void (*disconnect_watch) (struct btd_device *device, gboolean removal,
--
1.7.7.6
From: Mikel Astiz <[email protected]>
Propagate the disconnection reason through D-Bus, allowing other
components to react accordingly.
---
src/device.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/device.c b/src/device.c
index b2b9d54..8c124d1 100644
--- a/src/device.c
+++ b/src/device.c
@@ -891,6 +891,7 @@ static GDBusMethodTable device_methods[] = {
static GDBusSignalTable device_signals[] = {
{ "PropertyChanged", "sv" },
{ "DisconnectRequested", "" },
+ { "Disconnected", "y" },
{ }
};
@@ -942,6 +943,11 @@ void device_remove_connection(struct btd_device *device, DBusConnection *conn,
if (device_is_paired(device) && !device_is_bonded(device))
device_set_paired(device, FALSE);
+ g_dbus_emit_signal(conn, device->path,
+ DEVICE_INTERFACE, "Disconnected",
+ DBUS_TYPE_BYTE, &reason,
+ DBUS_TYPE_INVALID);
+
emit_property_changed(conn, device->path,
DEVICE_INTERFACE, "Connected",
DBUS_TYPE_BOOLEAN, &device->connected);
--
1.7.7.6
From: Mikel Astiz <[email protected]>
This signal is needed to propagate the reason in case of disconnection.
Some use-cases require this information, for example if there is a
policy to automatically reconnect in case of connection timeout.
---
doc/device-api.txt | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/doc/device-api.txt b/doc/device-api.txt
index 4a170e4..58b0957 100644
--- a/doc/device-api.txt
+++ b/doc/device-api.txt
@@ -108,6 +108,20 @@ Signals PropertyChanged(string name, variant value)
disconnection to a remote device has been requested.
The actual disconnection will happen 2 seconds later.
+ Disconnected(uint8 reason)
+
+ This signal will be sent when the low-level ACL link
+ is disconnected, either because is was explicitly
+ requested (locally initiated) or not.
+
+ The actual reason of the disconnection is given by
+ the first parameter, which is a 8-bit error code as
+ defined in the HCI specification (or 0 if the
+ disconnection is due to a previous request).
+
+ This signal will be sent just before the corresponding
+ state property change to "disconnected".
+
NodeCreated(object node)
Parameter is object path of created device node.
--
1.7.7.6
From: Mikel Astiz <[email protected]>
Internally propagate disconnect reason in order to make it public in the
D-Bus API.
---
lib/mgmt.h | 5 +++++
plugins/hciops.c | 2 +-
plugins/mgmtops.c | 10 ++++++++--
src/adapter.c | 5 +++--
src/adapter.h | 2 +-
src/event.c | 4 ++--
src/event.h | 3 ++-
7 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/lib/mgmt.h b/lib/mgmt.h
index a58915b..39d4e71 100644
--- a/lib/mgmt.h
+++ b/lib/mgmt.h
@@ -374,6 +374,11 @@ struct mgmt_ev_device_disconnected {
struct mgmt_addr_info addr;
} __packed;
+struct mgmt_ev_device_disconnected_2 {
+ struct mgmt_addr_info addr;
+ uint8_t reason;
+} __packed;
+
#define MGMT_EV_CONNECT_FAILED 0x000D
struct mgmt_ev_connect_failed {
struct mgmt_addr_info addr;
diff --git a/plugins/hciops.c b/plugins/hciops.c
index d74f2ea..ab2edb5 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -2364,7 +2364,7 @@ static inline void disconn_complete(int index, void *ptr)
dev->connections = g_slist_remove(dev->connections, conn);
- btd_event_disconn_complete(&dev->bdaddr, &conn->bdaddr);
+ btd_event_disconn_complete(&dev->bdaddr, &conn->bdaddr, evt->reason);
conn_free(conn);
}
diff --git a/plugins/mgmtops.c b/plugins/mgmtops.c
index 22fa329..47c73b6 100644
--- a/plugins/mgmtops.c
+++ b/plugins/mgmtops.c
@@ -511,12 +511,18 @@ static void mgmt_device_disconnected(int sk, uint16_t index, void *buf,
struct mgmt_addr_info *ev = buf;
struct controller_info *info;
char addr[18];
+ uint8_t reason;
if (len < sizeof(*ev)) {
error("Too small device_disconnected event");
return;
}
+ if (len < sizeof(struct mgmt_ev_device_disconnected_2))
+ reason = 0;
+ else
+ reason = ((struct mgmt_ev_device_disconnected_2 *)buf)->reason;
+
ba2str(&ev->bdaddr, addr);
DBG("hci%u device %s disconnected", index, addr);
@@ -528,7 +534,7 @@ static void mgmt_device_disconnected(int sk, uint16_t index, void *buf,
info = &controllers[index];
- btd_event_disconn_complete(&info->bdaddr, &ev->bdaddr);
+ btd_event_disconn_complete(&info->bdaddr, &ev->bdaddr, reason);
}
static void mgmt_connect_failed(int sk, uint16_t index, void *buf, size_t len)
@@ -1108,7 +1114,7 @@ static void disconnect_complete(int sk, uint16_t index, uint8_t status,
info = &controllers[index];
- btd_event_disconn_complete(&info->bdaddr, &rp->addr.bdaddr);
+ btd_event_disconn_complete(&info->bdaddr, &rp->addr.bdaddr, 0);
bonding_complete(info, &rp->addr.bdaddr, HCI_CONNECTION_TERMINATED);
}
diff --git a/src/adapter.c b/src/adapter.c
index 93b55e8..b9a90dd 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2304,7 +2304,8 @@ int btd_adapter_stop(struct btd_adapter *adapter)
while (adapter->connections) {
struct btd_device *device = adapter->connections->data;
- adapter_remove_connection(adapter, device);
+ adapter_remove_connection(adapter, device,
+ HCI_OE_USER_ENDED_CONNECTION);
}
if (adapter->scan_mode == (SCAN_PAGE | SCAN_INQUIRY))
@@ -2991,7 +2992,7 @@ void adapter_add_connection(struct btd_adapter *adapter,
}
void adapter_remove_connection(struct btd_adapter *adapter,
- struct btd_device *device)
+ struct btd_device *device, uint8_t reason)
{
DBG("");
diff --git a/src/adapter.h b/src/adapter.h
index b7ea62b..a9043de 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -128,7 +128,7 @@ struct agent *adapter_get_agent(struct btd_adapter *adapter);
void adapter_add_connection(struct btd_adapter *adapter,
struct btd_device *device);
void adapter_remove_connection(struct btd_adapter *adapter,
- struct btd_device *device);
+ struct btd_device *device, uint8_t reason);
gboolean adapter_has_discov_sessions(struct btd_adapter *adapter);
struct btd_adapter *btd_adapter_ref(struct btd_adapter *adapter);
diff --git a/src/event.c b/src/event.c
index 514744b..0fcc2ec 100644
--- a/src/event.c
+++ b/src/event.c
@@ -499,7 +499,7 @@ void btd_event_conn_failed(bdaddr_t *local, bdaddr_t *peer, uint8_t status)
adapter_remove_device(conn, adapter, device, TRUE);
}
-void btd_event_disconn_complete(bdaddr_t *local, bdaddr_t *peer)
+void btd_event_disconn_complete(bdaddr_t *local, bdaddr_t *peer, uint8_t reason)
{
struct btd_adapter *adapter;
struct btd_device *device;
@@ -512,7 +512,7 @@ void btd_event_disconn_complete(bdaddr_t *local, bdaddr_t *peer)
if (!device)
return;
- adapter_remove_connection(adapter, device);
+ adapter_remove_connection(adapter, device, reason);
}
void btd_event_device_blocked(bdaddr_t *local, bdaddr_t *peer)
diff --git a/src/event.h b/src/event.h
index dfc158d..cf2c091 100644
--- a/src/event.h
+++ b/src/event.h
@@ -32,7 +32,8 @@ void btd_event_remote_name(bdaddr_t *local, bdaddr_t *peer, char *name);
void btd_event_conn_complete(bdaddr_t *local, bdaddr_t *peer, uint8_t bdaddr_type,
char *name, uint8_t *dev_class);
void btd_event_conn_failed(bdaddr_t *local, bdaddr_t *peer, uint8_t status);
-void btd_event_disconn_complete(bdaddr_t *local, bdaddr_t *peer);
+void btd_event_disconn_complete(bdaddr_t *local, bdaddr_t *peer,
+ uint8_t reason);
void btd_event_simple_pairing_complete(bdaddr_t *local, bdaddr_t *peer, uint8_t status);
void btd_event_returned_link_key(bdaddr_t *local, bdaddr_t *peer);
int btd_event_user_confirm(bdaddr_t *sba, bdaddr_t *dba, uint32_t passkey);
--
1.7.7.6
From: Mikel Astiz <[email protected]>
The error-handling code should use the defines instead of magic numbers.
---
src/device.c | 42 +++++++++++++++++++++---------------------
1 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/src/device.c b/src/device.c
index 2a8812e..82dc2ca 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2303,48 +2303,48 @@ static DBusMessage *new_authentication_return(DBusMessage *msg, int status)
case 0x00: /* success */
return dbus_message_new_method_return(msg);
- case 0x04: /* page timeout */
+ case HCI_PAGE_TIMEOUT:
return dbus_message_new_error(msg,
ERROR_INTERFACE ".ConnectionAttemptFailed",
"Page Timeout");
- case 0x08: /* connection timeout */
+ case HCI_CONNECTION_TIMEOUT:
return dbus_message_new_error(msg,
ERROR_INTERFACE ".ConnectionAttemptFailed",
"Connection Timeout");
- case 0x10: /* connection accept timeout */
- case 0x22: /* LMP response timeout */
- case 0x28: /* instant passed - is this a timeout? */
+ case HCI_HOST_TIMEOUT:
+ case HCI_LMP_RESPONSE_TIMEOUT:
+ case HCI_INSTANT_PASSED: /* is this a timeout? */
return dbus_message_new_error(msg,
ERROR_INTERFACE ".AuthenticationTimeout",
"Authentication Timeout");
- case 0x17: /* too frequent pairing attempts */
+ case HCI_REPEATED_ATTEMPTS: /* too frequent pairing attempts */
return dbus_message_new_error(msg,
ERROR_INTERFACE ".RepeatedAttempts",
"Repeated Attempts");
- case 0x06:
- case 0x18: /* pairing not allowed (e.g. gw rejected attempt) */
+ case HCI_PIN_OR_KEY_MISSING:
+ case HCI_PAIRING_NOT_ALLOWED: /* e.g. gw rejected attempt */
return dbus_message_new_error(msg,
ERROR_INTERFACE ".AuthenticationRejected",
"Authentication Rejected");
- case 0x07: /* memory capacity */
- case 0x09: /* connection limit */
- case 0x0a: /* synchronous connection limit */
- case 0x0d: /* limited resources */
- case 0x13: /* user ended the connection */
- case 0x14: /* terminated due to low resources */
- case 0x16: /* connection terminated */
+ case HCI_MEMORY_FULL:
+ case HCI_MAX_NUMBER_OF_CONNECTIONS:
+ case HCI_MAX_NUMBER_OF_SCO_CONNECTIONS:
+ case HCI_REJECTED_LIMITED_RESOURCES:
+ case HCI_OE_USER_ENDED_CONNECTION:
+ case HCI_OE_LOW_RESOURCES:
+ case HCI_CONNECTION_TERMINATED:
return dbus_message_new_error(msg,
ERROR_INTERFACE ".AuthenticationCanceled",
"Authentication Canceled");
- case 0x05: /* authentication failure */
- case 0x0E: /* rejected due to security reasons - is this auth failure? */
- case 0x25: /* encryption mode not acceptable - is this auth failure? */
- case 0x26: /* link key cannot be changed - is this auth failure? */
- case 0x29: /* pairing with unit key unsupported - is this auth failure? */
- case 0x2f: /* insufficient security - is this auth failure? */
+ case HCI_AUTHENTICATION_FAILURE:
+ case HCI_REJECTED_SECURITY: /* is this auth failure? */
+ case HCI_ENCRYPTION_MODE_NOT_ACCEPTED: /* is this auth failure? */
+ case HCI_UNIT_LINK_KEY_USED: /* is this auth failure? */
+ case HCI_PAIRING_NOT_SUPPORTED: /* is this auth failure? */
+ case HCI_INSUFFICIENT_SECURITY: /* is this auth failure? */
default:
return dbus_message_new_error(msg,
ERROR_INTERFACE ".AuthenticationFailed",
--
1.7.7.6
Hi Mikel,
On Wed, Jul 18, 2012, Mikel Astiz wrote:
> On Mon, May 14, 2012 at 5:33 PM, Mikel Astiz <[email protected]> wrote:
> > From: Mikel Astiz <[email protected]>
> >
> > The error-handling code should use the defines instead of magic numbers.
> > ---
> > src/device.c | 42 +++++++++++++++++++++---------------------
> > 1 files changed, 21 insertions(+), 21 deletions(-)
>
> Ping.
>
> This was included inside a RFC series but has actually no dependency.
Thanks for the reminder. The patch has now been applied.
The usefulness of the patch is quite short-lived though since we've
pretty much stopped using HCI related values in userspace with the
introduction of mgmt. Looking at the code base it seems like several
places need fixing as the status value passed from mgmt.c to core is not
an HCI status but MGMT_STATUS_*. All places in the kernel should be
converting HCI status values with mgmt_status() before putting them into
a mgmt event. If you find some place that doesn't do that send a patch
please.
Johan
Hi,
On Mon, May 14, 2012 at 5:33 PM, Mikel Astiz <[email protected]> wrote:
> From: Mikel Astiz <[email protected]>
>
> The error-handling code should use the defines instead of magic numbers.
> ---
> src/device.c | 42 +++++++++++++++++++++---------------------
> 1 files changed, 21 insertions(+), 21 deletions(-)
Ping.
This was included inside a RFC series but has actually no dependency.
Cheers,
Mikel