* autopair corner cases
From: Bastien Nocera @ 2013-11-19 12:31 UTC (permalink / raw)
To: linux-bluetooth
Heya,
There's a bunch of corner cases that the autopair plugin doesn't handle
and that we used to handle in gnome-bluetooth with BlueZ 4.x.
1) First is the case of the PS3 BD Remote that will reject
authentication when you try to pair to it. gnome-bluetooth knows not to
pair with it.
> <!-- Sony PlayStation 3 Remote Control -->
> <device oui="00:19:C1:" name="BD Remote Control" pin="NULL"/>
> <device oui="00:1E:3D:" name="BD Remote Control" pin="NULL"/>
> <device oui="00:06:F5:" name="BD Remote Control" pin="NULL"/>
Is there a way to say "we can't actually pair" when the client requested
pairing already? Or is that considered a security problem?
2) The second case is pairing this "funny" keyboard that's the iCade
controller. In gnome-bluetooth, we had special code to generate only
joystick movements for the pairing, rather than hard to determine
buttons, so we'd end up with a 6-digit pin using only 1 through 4.
> <!-- ION iCade Game Controller -->
> <device name="iCade" type="keyboard" pin="ICADE"/>
3) We have a whole list of GPS that don't use present themselves as
anything special apart from the name. Most use "0000", but some use
things like "NAVMAN" or "12345678"
> <!-- http://bugzilla.gnome.org/show_bug.cgi?id=560315#c20 -->
> <device oui="00:02:5B:" name="Pharos iGPS-BT" pin="12345678"/>
>
> <!-- https://bugzilla.gnome.org/show_bug.cgi?id=613698 -->
> <device oui="00:0C:A5:" name="NAVMAN GPS ONE" pin="NAVMAN"/>
4) Audio devices will mostly already be supported by the autopair code
(yay!), though we have a few stragglers, most notably this speaker that
can use random pincode, as long as they're only 4 digits in length:
> <!-- http://bugzilla.gnome.org/show_bug.cgi?id=583651 -->
> <device type="audio" oui="00:1A:80:" name="CMT-DH5BT" pin="max:4"/>
5) Printers are missing from the list, that should be an easy fix.
I'll try and send patches for cases 2 through 4, using a static table in
the autopair plugin. If that's not acceptable, I'll create a new
higher-priority plugin that handles those corner cases.
Cheers
^ permalink raw reply
* [PATCH 5/5] android/hidhost: Free all connected devices in profile cleanup call
From: Ravi kumar Veeramally @ 2013-11-19 12:21 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Ravi kumar Veeramally
In-Reply-To: <1384863676-12358-1-git-send-email-ravikumar.veeramally@linux.intel.com>
---
android/hidhost.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/android/hidhost.c b/android/hidhost.c
index 05a3fe4..20dedc4 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -1226,6 +1226,14 @@ bool bt_hid_register(int sk, const bdaddr_t *addr)
return true;
}
+static void free_hid_devices(gpointer data, gpointer user_data)
+{
+ struct hid_device *dev = data;
+
+ bt_hid_notify_state(dev, HAL_HIDHOST_STATE_DISCONNECTED);
+ hid_device_free(dev);
+}
+
void bt_hid_unregister(void)
{
DBG("");
@@ -1233,6 +1241,8 @@ void bt_hid_unregister(void)
if (notification_sk < 0)
return;
+ g_slist_foreach(devices, free_hid_devices, NULL);
+
notification_sk = -1;
if (ctrl_io) {
--
1.8.3.2
^ permalink raw reply related
* [PATCH 4/5] android/hidhost: Handle error case properly in interrupt_connect_cb
From: Ravi kumar Veeramally @ 2013-11-19 12:21 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Ravi kumar Veeramally
In-Reply-To: <1384863676-12358-1-git-send-email-ravikumar.veeramally@linux.intel.com>
---
android/hidhost.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/android/hidhost.c b/android/hidhost.c
index a929a94..05a3fe4 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -522,7 +522,6 @@ static int uhid_create(struct hid_device *dev)
dev->uhid_fd = open(UHID_DEVICE_FILE, O_RDWR | O_CLOEXEC);
if (dev->uhid_fd < 0) {
error("Failed to open uHID device: %s", strerror(errno));
- bt_hid_notify_state(dev, HAL_HIDHOST_STATE_NO_HID);
return -errno;
}
@@ -541,7 +540,6 @@ static int uhid_create(struct hid_device *dev)
error("Failed to create uHID device: %s", strerror(errno));
close(dev->uhid_fd);
dev->uhid_fd = -1;
- bt_hid_notify_state(dev, HAL_HIDHOST_STATE_NO_HID);
return -errno;
}
@@ -559,16 +557,20 @@ static void interrupt_connect_cb(GIOChannel *chan, GError *conn_err,
gpointer user_data)
{
struct hid_device *dev = user_data;
+ uint8_t state;
DBG("");
if (conn_err) {
error("%s", conn_err->message);
+ state = HAL_HIDHOST_STATE_FAILED;
goto failed;
}
- if (uhid_create(dev) < 0)
+ if (uhid_create(dev) < 0) {
+ state = HAL_HIDHOST_STATE_NO_HID;
goto failed;
+ }
dev->intr_watch = g_io_add_watch(dev->intr_io,
G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
@@ -579,6 +581,7 @@ static void interrupt_connect_cb(GIOChannel *chan, GError *conn_err,
return;
failed:
+ bt_hid_notify_state(dev, state);
hid_device_free(dev);
}
--
1.8.3.2
^ permalink raw reply related
* [PATCH 3/5] android: Handle multiple init(register) and cleanup(unregister) calls properly
From: Ravi kumar Veeramally @ 2013-11-19 12:21 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Ravi kumar Veeramally
In-Reply-To: <1384863676-12358-1-git-send-email-ravikumar.veeramally@linux.intel.com>
This can be tested with haltest.
---
android/a2dp.c | 6 ++++++
android/bluetooth.c | 6 ++++++
android/hidhost.c | 6 ++++++
android/pan.c | 6 ++++++
4 files changed, 24 insertions(+)
diff --git a/android/a2dp.c b/android/a2dp.c
index 74d0082..28a1250 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -332,6 +332,9 @@ bool bt_a2dp_register(int sk, const bdaddr_t *addr)
DBG("");
+ if (notification_sk > 0)
+ return false;
+
bacpy(&adapter_addr, addr);
server = bt_io_listen(connect_cb, NULL, NULL, NULL, &err,
@@ -365,6 +368,9 @@ void bt_a2dp_unregister(void)
{
DBG("");
+ if (notification_sk < 0)
+ return;
+
notification_sk = -1;
bt_adapter_remove_record(record_id);
diff --git a/android/bluetooth.c b/android/bluetooth.c
index 7dc2ec3..7805d42 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -2275,6 +2275,9 @@ bool bt_bluetooth_register(int sk)
{
DBG("");
+ if (notification_sk > 0)
+ return false;
+
notification_sk = sk;
return true;
@@ -2284,5 +2287,8 @@ void bt_bluetooth_unregister(void)
{
DBG("");
+ if (notification_sk < 0)
+ return;
+
notification_sk = -1;
}
diff --git a/android/hidhost.c b/android/hidhost.c
index 842b8ad..a929a94 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -1190,6 +1190,9 @@ bool bt_hid_register(int sk, const bdaddr_t *addr)
DBG("");
+ if (notification_sk > 0)
+ return false;
+
bacpy(&adapter_addr, addr);
ctrl_io = bt_io_listen(connect_cb, NULL, NULL, NULL, &err,
@@ -1224,6 +1227,9 @@ void bt_hid_unregister(void)
{
DBG("");
+ if (notification_sk < 0)
+ return;
+
notification_sk = -1;
if (ctrl_io) {
diff --git a/android/pan.c b/android/pan.c
index fba86b8..970a8a7 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -95,6 +95,9 @@ bool bt_pan_register(int sk, const bdaddr_t *addr)
{
DBG("");
+ if (notification_sk > 0)
+ return false;
+
notification_sk = sk;
return true;
@@ -104,5 +107,8 @@ void bt_pan_unregister(void)
{
DBG("");
+ if (notification_sk < 0)
+ return;
+
notification_sk = -1;
}
--
1.8.3.2
^ permalink raw reply related
* [PATCH 2/5] android/hal-pan: Return error in case of unsupported PAN roles
From: Ravi kumar Veeramally @ 2013-11-19 12:21 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Ravi kumar Veeramally
In-Reply-To: <1384863676-12358-1-git-send-email-ravikumar.veeramally@linux.intel.com>
---
android/hal-pan.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/android/hal-pan.c b/android/hal-pan.c
index 2bc560e..30facd4 100644
--- a/android/hal-pan.c
+++ b/android/hal-pan.c
@@ -77,6 +77,9 @@ static bt_status_t pan_enable(int local_role)
if (!interface_ready())
return BT_STATUS_NOT_READY;
+ if (!(local_role == BTPAN_ROLE_PANU || local_role == BTPAN_ROLE_PANNAP))
+ return BT_STATUS_UNSUPPORTED;
+
cmd.local_role = local_role;
return hal_ipc_cmd(HAL_SERVICE_ID_PAN, HAL_OP_PAN_ENABLE,
@@ -112,6 +115,14 @@ static bt_status_t pan_connect(const bt_bdaddr_t *bd_addr, int local_role,
if (!interface_ready())
return BT_STATUS_NOT_READY;
+ if (!((local_role == BTPAN_ROLE_PANNAP &&
+ remote_role == BTPAN_ROLE_PANU) ||
+ (local_role == BTPAN_ROLE_PANU &&
+ remote_role == BTPAN_ROLE_PANNAP) ||
+ (local_role == BTPAN_ROLE_PANU &&
+ remote_role == BTPAN_ROLE_PANU)))
+ return BT_STATUS_UNSUPPORTED;
+
memcpy(cmd.bdaddr, bd_addr, sizeof(cmd.bdaddr));
cmd.local_role = local_role;
cmd.remote_role = remote_role;
--
1.8.3.2
^ permalink raw reply related
* [PATCH 1/5] android/pan: Fix wrong struct parameter in disconnect function call
From: Ravi kumar Veeramally @ 2013-11-19 12:21 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Ravi kumar Veeramally
---
android/pan.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/android/pan.c b/android/pan.c
index 2a11f46..fba86b8 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -58,7 +58,8 @@ static uint8_t bt_pan_connect(struct hal_cmd_pan_connect *cmd, uint16_t len)
return HAL_STATUS_FAILED;
}
-static uint8_t bt_pan_disconnect(struct hal_cmd_pan_connect *cmd, uint16_t len)
+static uint8_t bt_pan_disconnect(struct hal_cmd_pan_disconnect *cmd,
+ uint16_t len)
{
DBG("Not Implemented");
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH 1/2] android/a2sp: trivial: Use NULL for zero pointer
From: Johan Hedberg @ 2013-11-19 10:17 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1384853264-10924-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
On Tue, Nov 19, 2013, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> ---
> android/a2dp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Both patches have been applied (after fixing the obvious typo in the
subject). Thanks.
Johan
^ permalink raw reply
* Re: [PATCH] profile: Add missing browse group to MAS, MNS and SYNC records
From: Johan Hedberg @ 2013-11-19 10:15 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1384854391-13274-1-git-send-email-szymon.janc@tieto.com>
Hi Szymon,
On Tue, Nov 19, 2013, Szymon Janc wrote:
> This fix not updating UUIDs when those services are registered.
> ---
> src/profile.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
Applied. Thanks.
Johan
^ permalink raw reply
* [PATCH] profile: Add missing browse group to MAS, MNS and SYNC records
From: Szymon Janc @ 2013-11-19 9:46 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
This fix not updating UUIDs when those services are registered.
---
src/profile.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/src/profile.c b/src/profile.c
index d8cbe6a..baadd99 100644
--- a/src/profile.c
+++ b/src/profile.c
@@ -417,6 +417,11 @@
</sequence> \
</sequence> \
</attribute> \
+ <attribute id=\"0x0005\"> \
+ <sequence> \
+ <uuid value=\"0x1002\" /> \
+ </sequence> \
+ </attribute> \
<attribute id=\"0x0009\"> \
<sequence> \
<sequence> \
@@ -458,6 +463,11 @@
</sequence> \
</sequence> \
</attribute> \
+ <attribute id=\"0x0005\"> \
+ <sequence> \
+ <uuid value=\"0x1002\" /> \
+ </sequence> \
+ </attribute> \
<attribute id=\"0x0009\"> \
<sequence> \
<sequence> \
@@ -496,6 +506,11 @@
</sequence> \
</sequence> \
</attribute> \
+ <attribute id=\"0x0005\"> \
+ <sequence> \
+ <uuid value=\"0x1002\" /> \
+ </sequence> \
+ </attribute> \
<attribute id=\"0x0009\"> \
<sequence> \
<sequence> \
--
1.8.4.3
^ permalink raw reply related
* [PATCH 2/2] android/a2dp: Use sdp_data_free for freeing sdp data types
From: Andrei Emeltchenko @ 2013-11-19 9:27 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1384853264-10924-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
android/a2dp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/android/a2dp.c b/android/a2dp.c
index 7f16be8..74d0082 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -312,8 +312,8 @@ static sdp_record_t *a2dp_record(void)
sdp_set_info_attr(record, "Audio Source", NULL, NULL);
- free(psm);
- free(version);
+ sdp_data_free(psm);
+ sdp_data_free(version);
sdp_list_free(proto[0], NULL);
sdp_list_free(proto[1], NULL);
sdp_list_free(apseq, NULL);
--
1.7.10.4
^ permalink raw reply related
* [PATCH 1/2] android/a2sp: trivial: Use NULL for zero pointer
From: Andrei Emeltchenko @ 2013-11-19 9:27 UTC (permalink / raw)
To: linux-bluetooth
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
android/a2dp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/android/a2dp.c b/android/a2dp.c
index b0cb9b5..7f16be8 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -310,7 +310,7 @@ static sdp_record_t *a2dp_record(void)
features = sdp_data_alloc(SDP_UINT16, &feat);
sdp_attr_add(record, SDP_ATTR_SUPPORTED_FEATURES, features);
- sdp_set_info_attr(record, "Audio Source", 0, 0);
+ sdp_set_info_attr(record, "Audio Source", NULL, NULL);
free(psm);
free(version);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCHv4 02/17] android: trivial: Add comment making code consistent
From: Johan Hedberg @ 2013-11-19 9:22 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1384780854-20970-3-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
On Mon, Nov 18, 2013, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> ---
> android/hal-msg.h | 2 ++
> 1 file changed, 2 insertions(+)
The first two patches have been applied. Thanks.
Johan
^ permalink raw reply
* [PATCH] Don't care about power status and adapter power on after rfkill unblock
From: Wu zheng @ 2013-11-19 7:01 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Wu zheng
Don't care about the power status of BT adapter before rfkill block.
(BT adapter is power on or off)
When rfkill unblock, BT will set the adapter power on.
It results in the following issue.
Connman can't control BT adapter power on/off rightly.
When using connman down BT, connman will send the dbus command
of power off and set rfkill block BT.
Power off BT: adapter power off -> rfkill block BT.
When using connman up BT, connman will set rfkill unblock BT
and send the dbus command of power on.
Power on BT: rfkill unblock adapter power on -> adapter power on.
When connman enable BT, BT adapter will power on after rfkill unblock.
After then, connman will receive the message of adapter property change.
The message is received before connman finish the message BT up to Bluez.
Thefore, the BT power status of connman is different with BT power status
in the message of adapter property change.
It will result in that connman send the dbus message of power off
to Bluez and power down BT adapter again.
---
src/adapter.c | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index d904a56..bed195b 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -138,6 +138,7 @@ struct btd_adapter {
char *short_name; /* controller short name */
uint32_t supported_settings; /* controller supported settings */
uint32_t current_settings; /* current controller settings */
+ gboolean current_powered; /* current power status */
char *path; /* adapter object path */
uint8_t major_class; /* configured major class */
@@ -1845,6 +1846,7 @@ static void property_set_mode_complete(uint8_t status, uint16_t length,
{
struct property_set_data *data = user_data;
struct btd_adapter *adapter = data->adapter;
+ uint32_t settings;
DBG("%s (0x%02x)", mgmt_errstr(status), status);
@@ -1866,6 +1868,13 @@ static void property_set_mode_complete(uint8_t status, uint16_t length,
g_dbus_pending_property_success(data->id);
+ settings = bt_get_le32(param);
+
+ if (settings & MGMT_SETTING_POWERED)
+ adapter->current_powered = TRUE;
+ else
+ adapter->current_powered = FALSE;
+
/*
* The parameters are identical and also the task that is
* required in both cases. So it is safe to just call the
@@ -4527,6 +4536,9 @@ int btd_adapter_restore_powered(struct btd_adapter *adapter)
if (adapter->current_settings & MGMT_SETTING_POWERED)
return 0;
+ if (adapter->current_powered == FALSE)
+ return 0;
+
set_mode(adapter, MGMT_OP_SET_POWERED, 0x01);
return 0;
@@ -5637,6 +5649,7 @@ static int adapter_register(struct btd_adapter *adapter)
load_connections(adapter);
adapter->initialized = TRUE;
+ adapter->current_powered = FALSE;
if (main_opts.did_source) {
/* DeviceID record is added by sdpd-server before any other
--
1.7.7
^ permalink raw reply related
* [PATCH] Don't care about power status and adapter power on after rfkill unblock
From: wu.zheng @ 2013-11-19 6:55 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Wu zheng
From: Wu zheng <wu.zheng@intel.com>
Don't care about the power status of BT adapter before rfkill block.
(BT adapter is power on or off)
When rfkill unblock, BT will set the adapter power on.
It results in the following issue.
Connman can't control BT adapter power on/off rightly.
When using connman down BT, connman will send the dbus command
of power off and set rfkill block BT.
Power off BT: adapter power off -> rfkill blcok BT.
When using connman up BT, connman will set rfkill unblock BT
and send the dbus command of power on.
Power on BT: rfkill unblock adapter power on -> adapter power on.
When connman enable BT, BT adapter will power on after rfkill unblock.
After then, connman will receive the message of adapter property change.
The message is received before connman finish the message BT up to Bluez.
Thefore, the BT power status of connman is different with BT power status
in the message of adapter property change.
It will result in that connman send the dbus message of power off
to Bluez and power down BT adapter again.
---
src/adapter.c | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index d904a56..bed195b 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -138,6 +138,7 @@ struct btd_adapter {
char *short_name; /* controller short name */
uint32_t supported_settings; /* controller supported settings */
uint32_t current_settings; /* current controller settings */
+ gboolean current_powered; /* current power status */
char *path; /* adapter object path */
uint8_t major_class; /* configured major class */
@@ -1845,6 +1846,7 @@ static void property_set_mode_complete(uint8_t status, uint16_t length,
{
struct property_set_data *data = user_data;
struct btd_adapter *adapter = data->adapter;
+ uint32_t settings;
DBG("%s (0x%02x)", mgmt_errstr(status), status);
@@ -1866,6 +1868,13 @@ static void property_set_mode_complete(uint8_t status, uint16_t length,
g_dbus_pending_property_success(data->id);
+ settings = bt_get_le32(param);
+
+ if (settings & MGMT_SETTING_POWERED)
+ adapter->current_powered = TRUE;
+ else
+ adapter->current_powered = FALSE;
+
/*
* The parameters are identical and also the task that is
* required in both cases. So it is safe to just call the
@@ -4527,6 +4536,9 @@ int btd_adapter_restore_powered(struct btd_adapter *adapter)
if (adapter->current_settings & MGMT_SETTING_POWERED)
return 0;
+ if (adapter->current_powered == FALSE)
+ return 0;
+
set_mode(adapter, MGMT_OP_SET_POWERED, 0x01);
return 0;
@@ -5637,6 +5649,7 @@ static int adapter_register(struct btd_adapter *adapter)
load_connections(adapter);
adapter->initialized = TRUE;
+ adapter->current_powered = FALSE;
if (main_opts.did_source) {
/* DeviceID record is added by sdpd-server before any other
--
1.7.7
^ permalink raw reply related
* Re: [RFC v2 01/15] Bluetooth: Refactor hci_disconn_complete_evt
From: Andre Guedes @ 2013-11-18 18:40 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <227F5D6B-6B76-428E-9893-37ED1562F833@holtmann.org>
Hi Marcel,
On 10/29/2013 07:52 PM, Marcel Holtmann wrote:
> Hi Andre,
>
>> hci_disconn_complete_evt() logic is more complicated than what it
>> should be, making it hard to follow and add new features.
>>
>> So this patch does some code refactoring by handling the error cases
>> in the beginning of the function and by moving the main flow into the
>> first level of function scope. No change is done in the event handling
>> logic itself.
>>
>> Besides organizing this messy code, this patch makes easier to add
>> code for handling LE auto connection (which will be added in a further
>> patch).
>>
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>> net/bluetooth/hci_event.c | 62 +++++++++++++++++++++++++----------------------
>> 1 file changed, 33 insertions(+), 29 deletions(-)
>>
>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
>> index 5935f74..8b7cd37 100644
>> --- a/net/bluetooth/hci_event.c
>> +++ b/net/bluetooth/hci_event.c
>> @@ -1783,6 +1783,8 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
>> {
>> struct hci_ev_disconn_complete *ev = (void *) skb->data;
>> struct hci_conn *conn;
>> + u8 type;
>> + bool send_mgmt_event = false;
>>
>> BT_DBG("%s status 0x%2.2x", hdev->name, ev->status);
>>
>> @@ -1792,44 +1794,46 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
>> if (!conn)
>> goto unlock;
>>
>> - if (ev->status == 0)
>> - conn->state = BT_CLOSED;
>> -
>> if (test_and_clear_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags) &&
>> - (conn->type == ACL_LINK || conn->type == LE_LINK)) {
>> - if (ev->status) {
>> + (conn->type == ACL_LINK || conn->type == LE_LINK))
>> + send_mgmt_event = true;
>> +
>> + if (ev->status) {
>> + if (send_mgmt_event)
>> mgmt_disconnect_failed(hdev, &conn->dst, conn->type,
>> conn->dst_type, ev->status);
>> - } else {
>> - u8 reason = hci_to_mgmt_reason(ev->reason);
>> -
>> - mgmt_device_disconnected(hdev, &conn->dst, conn->type,
>> - conn->dst_type, reason);
>> - }
>> + return;
>> }
>
> so I get the feeling that mgmt_disconnect_failed should check the link type all by itself. Since we are handing it over to the function anyway. And then this send_mgmt_event could be just removed.
>
>> - if (ev->status == 0) {
>> - u8 type = conn->type;
>> + conn->state = BT_CLOSED;
>>
>> - if (type == ACL_LINK && conn->flush_key)
>> - hci_remove_link_key(hdev, &conn->dst);
>> - hci_proto_disconn_cfm(conn, ev->reason);
>> - hci_conn_del(conn);
>> + if (send_mgmt_event) {
>> + u8 reason = hci_to_mgmt_reason(ev->reason);
>>
>> - /* Re-enable advertising if necessary, since it might
>> - * have been disabled by the connection. From the
>> - * HCI_LE_Set_Advertise_Enable command description in
>> - * the core specification (v4.0):
>> - * "The Controller shall continue advertising until the Host
>> - * issues an LE_Set_Advertise_Enable command with
>> - * Advertising_Enable set to 0x00 (Advertising is disabled)
>> - * or until a connection is created or until the Advertising
>> - * is timed out due to Directed Advertising."
>> - */
>> - if (type == LE_LINK)
>> - mgmt_reenable_advertising(hdev);
>> + mgmt_device_disconnected(hdev, &conn->dst, conn->type,
>> + conn->dst_type, reason);
>> }
>
> Same here. Just make sure that mgmt_device_disconneted checks the link type by itself and call it unconditional.
>
>>
>> + if (conn->type == ACL_LINK && conn->flush_key)
>> + hci_remove_link_key(hdev, &conn->dst);
>> +
>> + type = conn->type;
>> + hci_proto_disconn_cfm(conn, ev->reason);
>> + hci_conn_del(conn);
>> +
>> + /* Re-enable advertising if necessary, since it might
>> + * have been disabled by the connection. From the
>> + * HCI_LE_Set_Advertise_Enable command description in
>> + * the core specification (v4.0):
>> + * "The Controller shall continue advertising until the Host
>> + * issues an LE_Set_Advertise_Enable command with
>> + * Advertising_Enable set to 0x00 (Advertising is disabled)
>> + * or until a connection is created or until the Advertising
>> + * is timed out due to Directed Advertising."
>> + */
>> + if (type == LE_LINK)
>> + mgmt_reenable_advertising(hdev);
>> +
>> unlock:
>> hci_dev_unlock(hdev);
>> }
These comments were implemented in the patch set "[RFC 0/5] Disconnect
complete refactoring". That patch set is already pushed upstream.
I'm currently working on a new version of this patch set. All your other
comments will be addressed in v3.
Thanks,
Andre
^ permalink raw reply
* Re: [RFC v2 10/15] Bluetooth: Temporarily stop background scanning on discovery
From: Andre Guedes @ 2013-11-18 18:40 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <8E33B37F-71D9-4516-8883-080DDEC81132@holtmann.org>
Hi Marcel,
On 10/29/2013 08:19 PM, Marcel Holtmann wrote:
> Hi Andre,
>
>> If the user send a mgmt start discovery command while the background
>> scanning is running, we should temporarily stop it. Once the discovery
>> finishes, we start the background scanning again.
>>
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>> net/bluetooth/hci_core.c | 5 +++++
>> net/bluetooth/mgmt.c | 12 ++++++++----
>> 2 files changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index 79debc3..44d3f99 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -1496,6 +1496,11 @@ void hci_discovery_set_state(struct hci_dev *hdev, int state)
>>
>> switch (state) {
>> case DISCOVERY_STOPPED:
>> + /* Check the background scanning since it may have been
>> + * temporarily stopped by the start discovery command.
>> + */
>> + hci_check_background_scan(hdev);
>> +
>
> I do not know what check here means. For me this is reenable_background_scan or trigger_background_scan or start_background_scan, but not some magic check.
>
>> if (hdev->discovery.state != DISCOVERY_STARTING)
>> mgmt_discovering(hdev, 0);
>> break;
>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>> index 22cf547..0e329e5 100644
>> --- a/net/bluetooth/mgmt.c
>> +++ b/net/bluetooth/mgmt.c
>> @@ -3280,11 +3280,15 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
>> goto failed;
>> }
>>
>> + /* If controller is scanning, it means the background scanning
>> + * is running. Thus, we should temporarily stop it in order to
>> + * set the discovery scanning parameters.
>> + */
>> if (test_bit(HCI_LE_SCAN, &hdev->dev_flags)) {
>> - err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY,
>> - MGMT_STATUS_BUSY);
>> - mgmt_pending_remove(cmd);
>> - goto failed;
>> + memset(&enable_cp, 0, sizeof(enable_cp));
>> + enable_cp.enable = LE_SCAN_DISABLE;
>> + hci_req_add(&req, HCI_OP_LE_SET_SCAN_ENABLE,
>> + sizeof(enable_cp), &enable_cp);
>> }
>
> I think we need a new hdev->dev_flags for HCI_LE_BG_SCAN. Magically assuming that it is a background scan is dangerous.
A few lines above (the diff doesn't show it), we check if discovery is
running. Since discovery is not running and controller is scanning, we
assume that it is the background scanning. So, we might not want to
create the HCI_LE_BG_SCAN flag since it is not really necessary by now.
Regards,
Andre
^ permalink raw reply
* [PATCH v2 6/6] android/hal: Check if command socket was shutdown by peer
From: Szymon Janc @ 2013-11-18 14:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384785305-17084-1-git-send-email-szymon.janc@tieto.com>
This will allow to print proper error before exiting.
---
android/hal-ipc.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/android/hal-ipc.c b/android/hal-ipc.c
index 5d622e1..b19704a 100644
--- a/android/hal-ipc.c
+++ b/android/hal-ipc.c
@@ -356,6 +356,12 @@ int hal_ipc_cmd(uint8_t service_id, uint8_t opcode, uint16_t len, void *param,
exit(EXIT_FAILURE);
}
+ /* socket was shutdown */
+ if (ret == 0) {
+ error("Command socket closed, aborting");
+ exit(EXIT_FAILURE);
+ }
+
memset(&msg, 0, sizeof(msg));
memset(&cmd, 0, sizeof(cmd));
--
1.8.4.2
^ permalink raw reply related
* [PATCH v2 5/6] android/hal-a2dp: Use generic IPC message handling for events
From: Szymon Janc @ 2013-11-18 14:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384785305-17084-1-git-send-email-szymon.janc@tieto.com>
Register handlers on service init and unregister on cleanup.
---
android/hal-a2dp.c | 41 +++++++++++++++++++++--------------------
android/hal.h | 1 -
2 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/android/hal-a2dp.c b/android/hal-a2dp.c
index e9fadb7..7d91151 100644
--- a/android/hal-a2dp.c
+++ b/android/hal-a2dp.c
@@ -31,7 +31,7 @@ static bool interface_ready(void)
return cbs != NULL;
}
-static void handle_conn_state(void *buf)
+static void handle_conn_state(void *buf, uint16_t len)
{
struct hal_ev_a2dp_conn_state *ev = buf;
@@ -40,7 +40,7 @@ static void handle_conn_state(void *buf)
(bt_bdaddr_t *) (ev->bdaddr));
}
-static void handle_audio_state(void *buf)
+static void handle_audio_state(void *buf, uint16_t len)
{
struct hal_ev_a2dp_audio_state *ev = buf;
@@ -48,24 +48,20 @@ static void handle_audio_state(void *buf)
cbs->audio_state_cb(ev->state, (bt_bdaddr_t *)(ev->bdaddr));
}
-/* will be called from notification thread context */
-void bt_notify_a2dp(uint8_t opcode, void *buf, uint16_t len)
-{
- if (!interface_ready())
- return;
-
- switch (opcode) {
- case HAL_EV_A2DP_CONN_STATE:
- handle_conn_state(buf);
- break;
- case HAL_EV_A2DP_AUDIO_STATE:
- handle_audio_state(buf);
- break;
- default:
- DBG("Unhandled callback opcode=0x%x", opcode);
- break;
- }
-}
+/* handlers will be called from notification thread context,
+ * index in table equals to 'opcode - HAL_MINIMUM_EVENT' */
+static const struct hal_ipc_handler ev_handlers[] = {
+ { /* HAL_EV_A2DP_CONN_STATE */
+ .handler = handle_conn_state,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_pan_conn_state),
+ },
+ { /* HAL_EV_A2DP_AUDIO_STATE */
+ .handler = handle_audio_state,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_pan_ctrl_state),
+ },
+};
static bt_status_t a2dp_connect(bt_bdaddr_t *bd_addr)
{
@@ -105,6 +101,9 @@ static bt_status_t init(btav_callbacks_t *callbacks)
cbs = callbacks;
+ hal_ipc_register(HAL_SERVICE_ID_A2DP, ev_handlers,
+ sizeof(ev_handlers)/sizeof(ev_handlers[0]));
+
cmd.service_id = HAL_SERVICE_ID_A2DP;
return hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
@@ -126,6 +125,8 @@ static void cleanup()
hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_UNREGISTER_MODULE,
sizeof(cmd), &cmd, 0, NULL, NULL);
+
+ hal_ipc_unregister(HAL_SERVICE_ID_A2DP);
}
static btav_interface_t iface = {
diff --git a/android/hal.h b/android/hal.h
index 6bd4c5a..b475411 100644
--- a/android/hal.h
+++ b/android/hal.h
@@ -28,4 +28,3 @@ btav_interface_t *bt_get_a2dp_interface(void);
void bt_thread_associate(void);
void bt_thread_disassociate(void);
-void bt_notify_a2dp(uint8_t opcode, void *buf, uint16_t len);
--
1.8.4.2
^ permalink raw reply related
* [PATCH v2 4/6] android/hal-pan: Use generic IPC message handling for events
From: Szymon Janc @ 2013-11-18 14:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384785305-17084-1-git-send-email-szymon.janc@tieto.com>
Register handlers on service init and unregister on cleanup.
---
android/hal-pan.c | 40 +++++++++++++++++++++-------------------
android/hal.h | 1 -
2 files changed, 21 insertions(+), 20 deletions(-)
diff --git a/android/hal-pan.c b/android/hal-pan.c
index 2bc560e..0ca9e12 100644
--- a/android/hal-pan.c
+++ b/android/hal-pan.c
@@ -31,7 +31,7 @@ static bool interface_ready(void)
return cbs != NULL;
}
-static void handle_conn_state(void *buf)
+static void handle_conn_state(void *buf, uint16_t len)
{
struct hal_ev_pan_conn_state *ev = buf;
@@ -41,7 +41,7 @@ static void handle_conn_state(void *buf)
ev->local_role, ev->remote_role);
}
-static void handle_ctrl_state(void *buf)
+static void handle_ctrl_state(void *buf, uint16_t len)
{
struct hal_ev_pan_ctrl_state *ev = buf;
@@ -50,23 +50,20 @@ static void handle_ctrl_state(void *buf)
ev->local_role, (char *)ev->name);
}
-void bt_notify_pan(uint8_t opcode, void *buf, uint16_t len)
-{
- if (!interface_ready())
- return;
-
- switch (opcode) {
- case HAL_EV_PAN_CONN_STATE:
- handle_conn_state(buf);
- break;
- case HAL_EV_PAN_CTRL_STATE:
- handle_ctrl_state(buf);
- break;
- default:
- DBG("Unhandled callback opcode=0x%x", opcode);
- break;
- }
-}
+/* handlers will be called from notification thread context,
+ * index in table equals to 'opcode - HAL_MINIMUM_EVENT' */
+static const struct hal_ipc_handler ev_handlers[] = {
+ { /* HAL_EV_PAN_CTRL_STATE */
+ .handler = handle_conn_state,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_pan_conn_state),
+ },
+ { /* HAL_EV_PAN_CONN_STATE */
+ .handler = handle_ctrl_state,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_pan_ctrl_state),
+ },
+};
static bt_status_t pan_enable(int local_role)
{
@@ -143,6 +140,9 @@ static bt_status_t pan_init(const btpan_callbacks_t *callbacks)
cbs = callbacks;
+ hal_ipc_register(HAL_SERVICE_ID_PAN, ev_handlers,
+ sizeof(ev_handlers)/sizeof(ev_handlers[0]));
+
cmd.service_id = HAL_SERVICE_ID_PAN;
return hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
@@ -164,6 +164,8 @@ static void pan_cleanup()
hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_UNREGISTER_MODULE,
sizeof(cmd), &cmd, 0, NULL, NULL);
+
+ hal_ipc_unregister(HAL_SERVICE_ID_PAN);
}
static btpan_interface_t pan_if = {
diff --git a/android/hal.h b/android/hal.h
index 58426a9..6bd4c5a 100644
--- a/android/hal.h
+++ b/android/hal.h
@@ -29,4 +29,3 @@ btav_interface_t *bt_get_a2dp_interface(void);
void bt_thread_associate(void);
void bt_thread_disassociate(void);
void bt_notify_a2dp(uint8_t opcode, void *buf, uint16_t len);
-void bt_notify_pan(uint8_t opcode, void *buf, uint16_t len);
--
1.8.4.2
^ permalink raw reply related
* [PATCH v2 3/6] android/hal-hidhost: Use generic IPC message handling for events
From: Szymon Janc @ 2013-11-18 14:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384785305-17084-1-git-send-email-szymon.janc@tieto.com>
Register handlers on service init and unregister on cleanup.
---
android/hal-hidhost.c | 76 +++++++++++++++++++++++++++++----------------------
android/hal.h | 1 -
2 files changed, 44 insertions(+), 33 deletions(-)
diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
index 2ce17a3..0573006 100644
--- a/android/hal-hidhost.c
+++ b/android/hal-hidhost.c
@@ -32,7 +32,7 @@ static bool interface_ready(void)
return cbacks != NULL;
}
-static void handle_conn_state(void *buf)
+static void handle_conn_state(void *buf, uint16_t len)
{
struct hal_ev_hidhost_conn_state *ev = buf;
@@ -41,7 +41,7 @@ static void handle_conn_state(void *buf)
ev->state);
}
-static void handle_info(void *buf)
+static void handle_info(void *buf, uint16_t len)
{
struct hal_ev_hidhost_info *ev = buf;
bthh_hid_info_t info;
@@ -60,7 +60,7 @@ static void handle_info(void *buf)
cbacks->hid_info_cb((bt_bdaddr_t *) ev->bdaddr, info);
}
-static void handle_proto_mode(void *buf)
+static void handle_proto_mode(void *buf, uint16_t len)
{
struct hal_ev_hidhost_proto_mode *ev = buf;
@@ -69,16 +69,21 @@ static void handle_proto_mode(void *buf)
ev->status, ev->mode);
}
-static void handle_get_report(void *buf)
+static void handle_get_report(void *buf, uint16_t len)
{
struct hal_ev_hidhost_get_report *ev = buf;
+ if (len != sizeof(*ev) + ev->len) {
+ error("invalid get report event, aborting");
+ exit(EXIT_FAILURE);
+ }
+
if (cbacks->get_report_cb)
cbacks->get_report_cb((bt_bdaddr_t *) ev->bdaddr, ev->status,
ev->data, ev->len);
}
-static void handle_virtual_unplug(void *buf)
+static void handle_virtual_unplug(void *buf, uint16_t len)
{
struct hal_ev_hidhost_virtual_unplug *ev = buf;
@@ -87,33 +92,35 @@ static void handle_virtual_unplug(void *buf)
ev->status);
}
-/* will be called from notification thread context */
-void bt_notify_hidhost(uint8_t opcode, void *buf, uint16_t len)
-{
- if (!interface_ready())
- return;
-
- switch (opcode) {
- case HAL_EV_HIDHOST_CONN_STATE:
- handle_conn_state(buf);
- break;
- case HAL_EV_HIDHOST_INFO:
- handle_info(buf);
- break;
- case HAL_EV_HIDHOST_PROTO_MODE:
- handle_proto_mode(buf);
- break;
- case HAL_EV_HIDHOST_GET_REPORT:
- handle_get_report(buf);
- break;
- case HAL_EV_HIDHOST_VIRTUAL_UNPLUG:
- handle_virtual_unplug(buf);
- break;
- default:
- DBG("Unhandled callback opcode=0x%x", opcode);
- break;
- }
-}
+/* handlers will be called from notification thread context,
+ * index in table equals to 'opcode - HAL_MINIMUM_EVENT' */
+static const struct hal_ipc_handler ev_handlers[] = {
+ { /* HAL_EV_HIDHOST_CONN_STATE */
+ .handler = handle_conn_state,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_hidhost_conn_state)
+ },
+ { /* HAL_EV_HIDHOST_INFO */
+ .handler = handle_info,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_hidhost_info),
+ },
+ { /* HAL_EV_HIDHOST_PROTO_MODE */
+ .handler = handle_proto_mode,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_hidhost_proto_mode),
+ },
+ { /* HAL_EV_HIDHOST_GET_REPORT */
+ .handler = handle_get_report,
+ .var_len = true,
+ .data_len = sizeof(struct hal_ev_hidhost_get_report),
+ },
+ { /* HAL_EV_HIDHOST_VIRTUAL_UNPLUG */
+ .handler = handle_virtual_unplug,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_hidhost_virtual_unplug),
+ },
+};
static bt_status_t hidhost_connect(bt_bdaddr_t *bd_addr)
{
@@ -362,6 +369,9 @@ static bt_status_t init(bthh_callbacks_t *callbacks)
/* store reference to user callbacks */
cbacks = callbacks;
+ hal_ipc_register(HAL_SERVICE_ID_HIDHOST, ev_handlers,
+ sizeof(ev_handlers)/sizeof(ev_handlers[0]));
+
cmd.service_id = HAL_SERVICE_ID_HIDHOST;
return hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
@@ -383,6 +393,8 @@ static void cleanup(void)
hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_UNREGISTER_MODULE,
sizeof(cmd), &cmd, 0, NULL, NULL);
+
+ hal_ipc_unregister(HAL_SERVICE_ID_HIDHOST);
}
static bthh_interface_t hidhost_if = {
diff --git a/android/hal.h b/android/hal.h
index 67dad5d..58426a9 100644
--- a/android/hal.h
+++ b/android/hal.h
@@ -28,6 +28,5 @@ btav_interface_t *bt_get_a2dp_interface(void);
void bt_thread_associate(void);
void bt_thread_disassociate(void);
-void bt_notify_hidhost(uint8_t opcode, void *buf, uint16_t len);
void bt_notify_a2dp(uint8_t opcode, void *buf, uint16_t len);
void bt_notify_pan(uint8_t opcode, void *buf, uint16_t len);
--
1.8.4.2
^ permalink raw reply related
* [PATCH v2 2/6] android/hal-bluetooth: Register IPC message handlers
From: Szymon Janc @ 2013-11-18 14:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384785305-17084-1-git-send-email-szymon.janc@tieto.com>
Register handlers on service init. Since this requires all handlers to
be registered (unknown opcode is considered IPC error) missing handlers
stubs are provided.
---
android/hal-bluetooth.c | 218 +++++++++++++++++++++++++++++++-----------------
android/hal.h | 1 -
2 files changed, 140 insertions(+), 79 deletions(-)
diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 078d537..17015db 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -35,7 +35,7 @@ static const bt_callbacks_t *bt_hal_cbacks = NULL;
*pe = *((uint8_t *) (hal_prop->val)); \
} while (0)
-static void handle_adapter_state_changed(void *buf)
+static void handle_adapter_state_changed(void *buf, uint16_t len)
{
struct hal_ev_adapter_state_changed *ev = buf;
@@ -45,38 +45,35 @@ static void handle_adapter_state_changed(void *buf)
bt_hal_cbacks->adapter_state_changed_cb(ev->state);
}
-static void adapter_props_to_hal(bt_property_t *send_props,
- struct hal_property *hal_prop,
- uint8_t num_props, void *buff_end)
+static void adapter_props_to_hal(bt_property_t *send_props, void *buf,
+ uint8_t num_props)
{
- void *p = hal_prop;
+ struct hal_property *prop = buf;
uint8_t i;
for (i = 0; i < num_props; i++) {
- if (p + sizeof(*hal_prop) + hal_prop->len > buff_end) {
- error("invalid adapter properties event, aborting");
- exit(EXIT_FAILURE);
- }
-
- send_props[i].type = hal_prop->type;
+ send_props[i].type = prop->type;
- switch (hal_prop->type) {
+ switch (prop->type) {
case HAL_PROP_ADAPTER_TYPE:
- create_enum_prop(send_props[i], hal_prop,
+ create_enum_prop(send_props[i], prop,
bt_device_type_t);
break;
case HAL_PROP_ADAPTER_SCAN_MODE:
- create_enum_prop(send_props[i], hal_prop,
+ create_enum_prop(send_props[i], prop,
bt_scan_mode_t);
break;
case HAL_PROP_ADAPTER_SERVICE_REC:
default:
- send_props[i].len = hal_prop->len;
- send_props[i].val = hal_prop->val;
+ send_props[i].len = prop->len;
+ send_props[i].val = prop->val;
break;
}
DBG("prop[%d]: %s", i, btproperty2str(&send_props[i]));
+
+ buf += sizeof(*prop) + prop->len;
+ prop = buf;
}
}
@@ -96,36 +93,30 @@ static void adapter_hal_props_cleanup(bt_property_t *props, uint8_t num)
}
}
-static void device_props_to_hal(bt_property_t *send_props,
- struct hal_property *hal_prop,
- uint8_t num_props, void *buff_end)
+static void device_props_to_hal(bt_property_t *send_props, void *buf,
+ uint8_t num_props)
{
- void *p = hal_prop;
+ struct hal_property *prop = buf;
uint8_t i;
for (i = 0; i < num_props; i++) {
- if (p + sizeof(*hal_prop) + hal_prop->len > buff_end) {
- error("invalid adapter properties event, aborting");
- exit(EXIT_FAILURE);
- }
+ send_props[i].type = prop->type;
- send_props[i].type = hal_prop->type;
-
- switch (hal_prop->type) {
+ switch (prop->type) {
case HAL_PROP_DEVICE_TYPE:
- create_enum_prop(send_props[i], hal_prop,
+ create_enum_prop(send_props[i], prop,
bt_device_type_t);
break;
case HAL_PROP_DEVICE_SERVICE_REC:
case HAL_PROP_DEVICE_VERSION_INFO:
default:
- send_props[i].len = hal_prop->len;
- send_props[i].val = hal_prop->val;
+ send_props[i].len = prop->len;
+ send_props[i].val = prop->val;
break;
}
- p += sizeof(*hal_prop) + hal_prop->len;
- hal_prop = p;
+ buf += sizeof(*prop) + prop->len;
+ prop = buf;
DBG("prop[%d]: %s", i, btproperty2str(&send_props[i]));
}
@@ -147,6 +138,28 @@ static void device_hal_props_cleanup(bt_property_t *props, uint8_t num)
}
}
+static void check_props(int num, const struct hal_property *prop, uint16_t len)
+{
+ int i;
+
+ for (i = 0; i < num; i++) {
+ if (sizeof(*prop) + prop->len > len) {
+ error("invalid properties (%zu > %u), aborting",
+ sizeof(*prop) + prop->len, len);
+ exit(EXIT_FAILURE);
+ }
+
+ len -= sizeof(*prop) + prop->len;
+ prop = ((void *) prop) + sizeof(*prop) + prop->len;
+ }
+
+ if (!len)
+ return;
+
+ error("invalid properties length (%u bytes left), aborting", len);
+ exit(EXIT_FAILURE);
+}
+
static void handle_adapter_props_changed(void *buf, uint16_t len)
{
struct hal_ev_adapter_props_changed *ev = buf;
@@ -154,17 +167,19 @@ static void handle_adapter_props_changed(void *buf, uint16_t len)
DBG("");
+ check_props(ev->num_props, ev->props, len - sizeof(*ev));
+
if (!bt_hal_cbacks->adapter_properties_cb)
return;
- adapter_props_to_hal(props, ev->props, ev->num_props, buf + len);
+ adapter_props_to_hal(props, ev->props, ev->num_props);
bt_hal_cbacks->adapter_properties_cb(ev->status, ev->num_props, props);
adapter_hal_props_cleanup(props, ev->num_props);
}
-static void handle_bond_state_change(void *buf)
+static void handle_bond_state_change(void *buf, uint16_t len)
{
struct hal_ev_bond_state_changed *ev = buf;
bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
@@ -176,7 +191,7 @@ static void handle_bond_state_change(void *buf)
ev->state);
}
-static void handle_pin_request(void *buf)
+static void handle_pin_request(void *buf, uint16_t len)
{
struct hal_ev_pin_request *ev = buf;
/* Those are declared as packed, so it's safe to assign pointers */
@@ -189,7 +204,7 @@ static void handle_pin_request(void *buf)
bt_hal_cbacks->pin_request_cb(addr, name, ev->class_of_dev);
}
-static void handle_ssp_request(void *buf)
+static void handle_ssp_request(void *buf, uint16_t len)
{
struct hal_ev_ssp_request *ev = buf;
/* Those are declared as packed, so it's safe to assign pointers */
@@ -221,7 +236,7 @@ static bool interface_ready(void)
return bt_hal_cbacks != NULL;
}
-static void handle_discovery_state_changed(void *buf)
+static void handle_discovery_state_changed(void *buf, uint16_t len)
{
struct hal_ev_discovery_state_changed *ev = buf;
@@ -238,10 +253,12 @@ static void handle_device_found(void *buf, uint16_t len)
DBG("");
+ check_props(ev->num_props, ev->props, len - sizeof(*ev));
+
if (!bt_hal_cbacks->device_found_cb)
return;
- device_props_to_hal(props, ev->props, ev->num_props, buf + len);
+ device_props_to_hal(props, ev->props, ev->num_props);
bt_hal_cbacks->device_found_cb(ev->num_props, props);
@@ -255,10 +272,12 @@ static void handle_device_state_changed(void *buf, uint16_t len)
DBG("");
+ check_props(ev->num_props, ev->props, len - sizeof(*ev));
+
if (!bt_hal_cbacks->remote_device_properties_cb)
return;
- device_props_to_hal(props, ev->props, ev->num_props, buf + len);
+ device_props_to_hal(props, ev->props, ev->num_props);
bt_hal_cbacks->remote_device_properties_cb(ev->status,
(bt_bdaddr_t *)ev->bdaddr,
@@ -267,7 +286,7 @@ static void handle_device_state_changed(void *buf, uint16_t len)
device_hal_props_cleanup(props, ev->num_props);
}
-static void handle_acl_state_changed(void *buf)
+static void handle_acl_state_changed(void *buf, uint16_t len)
{
struct hal_ev_acl_state_changed *ev = buf;
bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
@@ -279,48 +298,83 @@ static void handle_acl_state_changed(void *buf)
ev->state);
}
-/* will be called from notification thread context */
-void bt_notify_adapter(uint8_t opcode, void *buf, uint16_t len)
+static void handle_dut_mode_receive(void *buf, uint16_t len)
{
- if (!interface_ready())
- return;
-
- DBG("opcode 0x%x", opcode);
+ DBG("");
- switch (opcode) {
- case HAL_EV_ADAPTER_STATE_CHANGED:
- handle_adapter_state_changed(buf);
- break;
- case HAL_EV_ADAPTER_PROPS_CHANGED:
- handle_adapter_props_changed(buf, len);
- break;
- case HAL_EV_DISCOVERY_STATE_CHANGED:
- handle_discovery_state_changed(buf);
- break;
- case HAL_EV_DEVICE_FOUND:
- handle_device_found(buf, len);
- break;
- case HAL_EV_REMOTE_DEVICE_PROPS:
- handle_device_state_changed(buf, len);
- break;
- case HAL_EV_BOND_STATE_CHANGED:
- handle_bond_state_change(buf);
- break;
- case HAL_EV_PIN_REQUEST:
- handle_pin_request(buf);
- break;
- case HAL_EV_SSP_REQUEST:
- handle_ssp_request(buf);
- break;
- case HAL_EV_ACL_STATE_CHANGED:
- handle_acl_state_changed(buf);
- break;
- default:
- DBG("Unhandled callback opcode=0x%x", opcode);
- break;
- }
+ /* TODO */
}
+static void handle_le_test_mode(void *buf, uint16_t len)
+{
+ DBG("");
+
+ /* TODO */
+}
+
+/* handlers will be called from notification thread context,
+ * index in table equals to 'opcode - HAL_MINIMUM_EVENT' */
+static const struct hal_ipc_handler ev_handlers[] = {
+ { /* HAL_EV_ADAPTER_STATE_CHANGED */
+ .handler = handle_adapter_state_changed,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_adapter_state_changed)
+ },
+ { /* HAL_EV_ADAPTER_PROPS_CHANGED */
+ .handler = handle_adapter_props_changed,
+ .var_len = true,
+ .data_len = sizeof(struct hal_ev_adapter_props_changed) +
+ sizeof(struct hal_property),
+ },
+ { /* HAL_EV_REMOTE_DEVICE_PROPS */
+ .handler = handle_device_state_changed,
+ .var_len = true,
+ .data_len = sizeof(struct hal_ev_remote_device_props) +
+ sizeof(struct hal_property),
+ },
+ { /* HAL_EV_DEVICE_FOUND */
+ .handler = handle_device_found,
+ .var_len = true,
+ .data_len = sizeof(struct hal_ev_device_found) +
+ sizeof(struct hal_property),
+ },
+ { /* HAL_EV_DISCOVERY_STATE_CHANGED */
+ .handler = handle_discovery_state_changed,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_discovery_state_changed),
+ },
+ { /* HAL_EV_PIN_REQUEST */
+ .handler = handle_pin_request,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_pin_request),
+ },
+ { /* HAL_EV_SSP_REQUEST */
+ .handler = handle_ssp_request,
+ .var_len = false,
+ .data_len = sizeof(handle_ssp_request),
+ },
+ { /* HAL_EV_BOND_STATE_CHANGED */
+ .handler = handle_bond_state_change,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_bond_state_changed),
+ },
+ { /* HAL_EV_ACL_STATE_CHANGED */
+ .handler = handle_acl_state_changed,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_acl_state_changed),
+ },
+ { /* HAL_EV_DUT_MODE_RECEIVE */
+ .handler = handle_dut_mode_receive,
+ .var_len = true,
+ .data_len = sizeof(struct hal_ev_dut_mode_receive),
+ },
+ { /* HAL_EV_LE_TEST_MODE */
+ .handler = handle_le_test_mode,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_le_test_mode),
+ },
+};
+
static int init(bt_callbacks_t *callbacks)
{
struct hal_cmd_register_module cmd;
@@ -333,6 +387,9 @@ static int init(bt_callbacks_t *callbacks)
bt_hal_cbacks = callbacks;
+ hal_ipc_register(HAL_SERVICE_ID_BLUETOOTH, ev_handlers,
+ sizeof(ev_handlers)/sizeof(ev_handlers[0]));
+
if (!hal_ipc_init()) {
bt_hal_cbacks = NULL;
return BT_STATUS_FAIL;
@@ -361,6 +418,9 @@ static int init(bt_callbacks_t *callbacks)
fail:
hal_ipc_cleanup();
bt_hal_cbacks = NULL;
+
+ hal_ipc_unregister(HAL_SERVICE_ID_BLUETOOTH);
+
return status;
}
@@ -396,6 +456,8 @@ static void cleanup(void)
hal_ipc_cleanup();
bt_hal_cbacks = NULL;
+
+ hal_ipc_unregister(HAL_SERVICE_ID_BLUETOOTH);
}
static int get_adapter_properties(void)
diff --git a/android/hal.h b/android/hal.h
index 72090fe..67dad5d 100644
--- a/android/hal.h
+++ b/android/hal.h
@@ -26,7 +26,6 @@ bthh_interface_t *bt_get_hidhost_interface(void);
btpan_interface_t *bt_get_pan_interface(void);
btav_interface_t *bt_get_a2dp_interface(void);
-void bt_notify_adapter(uint8_t opcode, void *buf, uint16_t len);
void bt_thread_associate(void);
void bt_thread_disassociate(void);
void bt_notify_hidhost(uint8_t opcode, void *buf, uint16_t len);
--
1.8.4.2
^ permalink raw reply related
* [PATCH v2 1/6] android/hal: Add initial code for IPC message handlers
From: Szymon Janc @ 2013-11-18 14:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384785305-17084-1-git-send-email-szymon.janc@tieto.com>
This will allow to register and unregister handlers for IPC messages
Basic sanity check will be done in common code. Commands with variable
length will be verified against minimum size only.
---
android/hal-ipc.c | 117 ++++++++++++++++++++++++++++++++++++------------------
android/hal-ipc.h | 10 +++++
2 files changed, 89 insertions(+), 38 deletions(-)
diff --git a/android/hal-ipc.c b/android/hal-ipc.c
index 5155e04..5d622e1 100644
--- a/android/hal-ipc.c
+++ b/android/hal-ipc.c
@@ -43,26 +43,86 @@ static pthread_mutex_t cmd_sk_mutex = PTHREAD_MUTEX_INITIALIZER;
static pthread_t notif_th = 0;
-static void notification_dispatch(struct hal_hdr *msg, int fd)
+struct service_handler {
+ const struct hal_ipc_handler *handler;
+ uint8_t size;
+};
+
+static struct service_handler services[HAL_SERVICE_ID_MAX + 1];
+
+void hal_ipc_register(uint8_t service, const struct hal_ipc_handler *handlers,
+ uint8_t size)
+{
+ services[service].handler = handlers;
+ services[service].size = size;
+}
+
+void hal_ipc_unregister(uint8_t service)
{
- switch (msg->service_id) {
- case HAL_SERVICE_ID_BLUETOOTH:
- bt_notify_adapter(msg->opcode, msg->payload, msg->len);
- break;
- case HAL_SERVICE_ID_HIDHOST:
- bt_notify_hidhost(msg->opcode, msg->payload, msg->len);
- break;
- case HAL_SERVICE_ID_A2DP:
- bt_notify_a2dp(msg->opcode, msg->payload, msg->len);
- break;
- case HAL_SERVICE_ID_PAN:
- bt_notify_pan(msg->opcode, msg->payload, msg->len);
- break;
- default:
- DBG("Unhandled notification service=%d opcode=0x%x",
+ services[service].handler = NULL;
+ services[service].size = 0;
+}
+
+static void handle_msg(void *buf, ssize_t len)
+{
+ struct hal_hdr *msg = buf;
+ const struct hal_ipc_handler *handler;
+ uint8_t opcode;
+
+ if (len < (ssize_t) sizeof(*msg)) {
+ error("IPC: message too small (%zd bytes), aborting", len);
+ exit(EXIT_FAILURE);
+ }
+
+ if (len != (ssize_t) (sizeof(*msg) + msg->len)) {
+ error("IPC: message malformed (%zd bytes), aborting", len);
+ exit(EXIT_FAILURE);
+ }
+
+ /* if service is valid */
+ if (msg->service_id > HAL_SERVICE_ID_MAX) {
+ error("IPC: unknown service (0x%x), aborting",
+ msg->service_id);
+ exit(EXIT_FAILURE);
+ }
+
+ /* if service is registered */
+ if (!services[msg->service_id].handler) {
+ error("IPC: unregistered service (0x%x), aborting",
+ msg->service_id);
+ exit(EXIT_FAILURE);
+ }
+
+ /* if opcode fit valid range */
+ if (msg->opcode < HAL_MINIMUM_EVENT) {
+ error("IPC: invalid opcode for service 0x%x (0x%x), aborting",
msg->service_id, msg->opcode);
- break;
+ exit(EXIT_FAILURE);
+ }
+
+ /* opcode is used as table offset and must be adjusted as events start
+ * with HAL_MINIMUM_EVENT offset */
+ opcode = msg->opcode - HAL_MINIMUM_EVENT;
+
+ /* if opcode is valid */
+ if (opcode >= services[msg->service_id].size) {
+ error("IPC: invalid opcode for service 0x%x (0x%x), aborting",
+ msg->service_id, msg->opcode);
+ exit(EXIT_FAILURE);
+ }
+
+ handler = &services[msg->service_id].handler[opcode];
+
+ /* if payload size is valid */
+ if ((handler->var_len && handler->data_len > msg->len) ||
+ (!handler->var_len && handler->data_len != msg->len)) {
+ error("IPC: message size invalid for service 0x%x opcode 0x%x "
+ "(%u bytes), aborting",
+ msg->service_id, msg->opcode, msg->len);
+ exit(EXIT_FAILURE);
}
+
+ handler->handler(msg->payload, msg->len);
}
static void *notification_handler(void *data)
@@ -72,7 +132,6 @@ static void *notification_handler(void *data)
struct cmsghdr *cmsg;
char cmsgbuf[CMSG_SPACE(sizeof(int))];
char buf[BLUEZ_HAL_MTU];
- struct hal_hdr *ev = (void *) buf;
ssize_t ret;
int fd;
@@ -83,7 +142,7 @@ static void *notification_handler(void *data)
memset(buf, 0, sizeof(buf));
memset(cmsgbuf, 0, sizeof(cmsgbuf));
- iv.iov_base = ev;
+ iv.iov_base = buf;
iv.iov_len = sizeof(buf);
msg.msg_iov = &iv;
@@ -108,24 +167,6 @@ static void *notification_handler(void *data)
exit(EXIT_FAILURE);
}
- if (ret < (ssize_t) sizeof(*ev)) {
- error("Too small notification (%zd bytes), aborting",
- ret);
- exit(EXIT_FAILURE);
- }
-
- if (ev->opcode < HAL_MINIMUM_EVENT) {
- error("Invalid notification (0x%x), aborting",
- ev->opcode);
- exit(EXIT_FAILURE);
- }
-
- if (ret != (ssize_t) (sizeof(*ev) + ev->len)) {
- error("Malformed notification(%zd bytes), aborting",
- ret);
- exit(EXIT_FAILURE);
- }
-
fd = -1;
/* Receive auxiliary data in msg */
@@ -138,7 +179,7 @@ static void *notification_handler(void *data)
}
}
- notification_dispatch(ev, fd);
+ handle_msg(buf, ret);
}
close(notif_sk);
diff --git a/android/hal-ipc.h b/android/hal-ipc.h
index ea53e1c..2fbf30f 100644
--- a/android/hal-ipc.h
+++ b/android/hal-ipc.h
@@ -15,8 +15,18 @@
*
*/
+struct hal_ipc_handler {
+ void (*handler) (void *buf, uint16_t len);
+ bool var_len;
+ size_t data_len;
+};
+
bool hal_ipc_init(void);
void hal_ipc_cleanup(void);
int hal_ipc_cmd(uint8_t service_id, uint8_t opcode, uint16_t len, void *param,
size_t *rsp_len, void *rsp, int *fd);
+
+void hal_ipc_register(uint8_t service, const struct hal_ipc_handler *handlers,
+ uint8_t size);
+void hal_ipc_unregister(uint8_t service);
--
1.8.4.2
^ permalink raw reply related
* [PATCH v2 0/6] android: IPC improvements
From: Szymon Janc @ 2013-11-18 14:34 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
Hi,
Changes since v1:
- remove fd parameter from handler proptotype - this is only needed by one
callback in health HAL and getter function can be addded later on if needed
- improve comments - each handler has comment with opcode define for easier
code reading
- some bugfixes
--
BR
Szymon Janc
Szymon Janc (6):
android/hal: Add initial code for IPC message handlers
android/hal-bluetooth: Register IPC message handlers
android/hal-hidhost: Use generic IPC message handling for events
android/hal-pan: Use generic IPC message handling for events
android/hal-a2dp: Use generic IPC message handling for events
android/hal: Check if command socket was shutdown by peer
android/hal-a2dp.c | 41 ++++-----
android/hal-bluetooth.c | 218 +++++++++++++++++++++++++++++++-----------------
android/hal-hidhost.c | 76 ++++++++++-------
android/hal-ipc.c | 123 ++++++++++++++++++---------
android/hal-ipc.h | 10 +++
android/hal-pan.c | 40 ++++-----
android/hal.h | 4 -
7 files changed, 321 insertions(+), 191 deletions(-)
--
1.8.4.2
^ permalink raw reply
* Re: [PATCH BlueZ] audio/AVCTP: Use A/V_RemoteControl UUID for incoming connections
From: Johan Hedberg @ 2013-11-18 14:27 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1384782134-17118-1-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
On Mon, Nov 18, 2013, Luiz Augusto von Dentz wrote:
> By the time and incoming connection arrives it is not possible to
> determine the role, so this changes the UUID to A/V_RemoteControl which
> according to assigned number page bellow is the profile UUID for AVRCP:
>
> https://www.bluetooth.org/en-us/specification/assigned-numbers/service-discovery
> ---
> profiles/audio/avctp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Applied. Thanks.
Johan
^ permalink raw reply
* [PATCH BlueZ] audio/AVCTP: Use A/V_RemoteControl UUID for incoming connections
From: Luiz Augusto von Dentz @ 2013-11-18 13:42 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
By the time and incoming connection arrives it is not possible to
determine the role, so this changes the UUID to A/V_RemoteControl which
according to assigned number page bellow is the profile UUID for AVRCP:
https://www.bluetooth.org/en-us/specification/assigned-numbers/service-discovery
---
profiles/audio/avctp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index dac7a66..476f61a 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -1311,7 +1311,7 @@ static void avctp_control_confirm(struct avctp *session, GIOChannel *chan,
dst = device_get_address(dev);
session->auth_id = btd_request_authorization(src, dst,
- AVRCP_TARGET_UUID,
+ AVRCP_REMOTE_UUID,
auth_cb, session);
if (session->auth_id == 0)
goto drop;
--
1.8.3.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox