* [PATCH 5/5] android/hal-a2dp: Use generic IPC message handling for events
From: Szymon Janc @ 2013-11-16 14:13 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384611208-18536-1-git-send-email-szymon.janc@tieto.com>
Register handlers on service init and unregister on cleanup.
---
android/hal-a2dp.c | 40 ++++++++++++++++++++--------------------
android/hal.h | 1 -
2 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/android/hal-a2dp.c b/android/hal-a2dp.c
index e9fadb7..baa4b10 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, int fd)
{
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, int fd)
{
struct hal_ev_a2dp_audio_state *ev = buf;
@@ -48,24 +48,19 @@ 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 */
+static const struct hal_ipc_handler ev_handlers[] = {
+ {
+ .handler = handle_conn_state,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_pan_conn_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 +100,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 +124,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 4/5] android/hal-pan: Use generic IPC message handling for events
From: Szymon Janc @ 2013-11-16 14:13 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384611208-18536-1-git-send-email-szymon.janc@tieto.com>
Register handlers on service init and unregister on cleanup.
---
android/hal-pan.c | 39 ++++++++++++++++++++-------------------
android/hal.h | 1 -
2 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/android/hal-pan.c b/android/hal-pan.c
index 2bc560e..dff2a44 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, int fd)
{
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, int fd)
{
struct hal_ev_pan_ctrl_state *ev = buf;
@@ -50,23 +50,19 @@ 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 */
+static const struct hal_ipc_handler ev_handlers[] = {
+ {
+ .handler = handle_conn_state,
+ .var_len = false,
+ .data_len = sizeof(struct 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 +139,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 +163,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 3/5] android/hal-hidhost: Use generic IPC message handling for events
From: Szymon Janc @ 2013-11-16 14:13 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384611208-18536-1-git-send-email-szymon.janc@tieto.com>
Register handlers on service init and unregister on cleanup.
---
android/hal-hidhost.c | 75 +++++++++++++++++++++++++++++----------------------
android/hal.h | 1 -
2 files changed, 43 insertions(+), 33 deletions(-)
diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
index 2ce17a3..27ce492 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, int fd)
{
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, int fd)
{
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, int fd)
{
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, int fd)
{
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, int fd)
{
struct hal_ev_hidhost_virtual_unplug *ev = buf;
@@ -87,33 +92,34 @@ 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 */
+static const struct hal_ipc_handler ev_handlers[] = {
+ {
+ .handler = handle_conn_state,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_hidhost_conn_state)
+ },
+ {
+ .handler = handle_info,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_hidhost_info),
+ },
+ {
+ .handler = handle_proto_mode,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_hidhost_proto_mode),
+ },
+ {
+ .handler = handle_get_report,
+ .var_len = true,
+ .data_len = sizeof(struct hal_ev_hidhost_get_report),
+ },
+ {
+ .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 +368,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 +392,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 2/5] android/hal-bluetooth: Register IPC message handlers
From: Szymon Janc @ 2013-11-16 14:13 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384611208-18536-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 | 223 ++++++++++++++++++++++++++++++------------------
android/hal.h | 1 -
2 files changed, 142 insertions(+), 82 deletions(-)
diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 078d537..2ec7c10 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, int fd)
{
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,24 +138,48 @@ static void device_hal_props_cleanup(bt_property_t *props, uint8_t num)
}
}
-static void handle_adapter_props_changed(void *buf, uint16_t len)
+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, int fd)
{
struct hal_ev_adapter_props_changed *ev = buf;
bt_property_t props[ev->num_props];
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, int fd)
{
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, int fd)
{
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, int fd)
{
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, int fd)
{
struct hal_ev_discovery_state_changed *ev = buf;
@@ -231,34 +246,38 @@ static void handle_discovery_state_changed(void *buf)
bt_hal_cbacks->discovery_state_changed_cb(ev->state);
}
-static void handle_device_found(void *buf, uint16_t len)
+static void handle_device_found(void *buf, uint16_t len, int fd)
{
struct hal_ev_device_found *ev = buf;
bt_property_t props[ev->num_props];
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);
device_hal_props_cleanup(props, ev->num_props);
}
-static void handle_device_state_changed(void *buf, uint16_t len)
+static void handle_device_state_changed(void *buf, uint16_t len, int fd)
{
struct hal_ev_remote_device_props *ev = buf;
bt_property_t props[ev->num_props];
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, int fd)
{
struct hal_ev_acl_state_changed *ev = buf;
bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
@@ -279,48 +298,82 @@ 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, int fd)
{
- 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, int fd)
+{
+ DBG("");
+
+ /* TODO */
+}
+
+/* handlers will be called from notification thread context */
+static const struct hal_ipc_handler ev_handlers[] = {
+ {
+ .handler = handle_adapter_state_changed,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_adapter_state_changed)
+ },
+ {
+ .handler = handle_adapter_props_changed,
+ .var_len = true,
+ .data_len = sizeof(struct hal_ev_adapter_props_changed) +
+ sizeof(struct hal_property),
+ },
+ {
+ .handler = handle_device_state_changed,
+ .var_len = true,
+ .data_len = sizeof(struct hal_ev_remote_device_props) +
+ sizeof(struct hal_property),
+ },
+ {
+ .handler = handle_device_found,
+ .var_len = true,
+ .data_len = sizeof(struct hal_ev_device_found) +
+ sizeof(struct hal_property),
+ },
+ {
+ .handler = handle_discovery_state_changed,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_discovery_state_changed),
+ },
+ {
+ .handler = handle_pin_request,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_pin_request),
+ },
+ {
+ .handler = handle_ssp_request,
+ .var_len = false,
+ .data_len = sizeof(handle_ssp_request),
+ },
+ {
+ .handler = handle_bond_state_change,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_bond_state_changed),
+ },
+ {
+ .handler = handle_acl_state_changed,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_acl_state_changed),
+ },
+ {
+ .handler = handle_dut_mode_receive,
+ .var_len = true,
+ .data_len = sizeof(struct hal_ev_dut_mode_receive),
+ },
+ {
+ .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 +386,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 +417,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 +455,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 1/5] android/hal: Add initial code for IPC message handlers
From: Szymon Janc @ 2013-11-16 14:13 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384611208-18536-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 | 115 ++++++++++++++++++++++++++++++++++++------------------
android/hal-ipc.h | 10 +++++
2 files changed, 87 insertions(+), 38 deletions(-)
diff --git a/android/hal-ipc.c b/android/hal-ipc.c
index 026e245..53af897 100644
--- a/android/hal-ipc.c
+++ b/android/hal-ipc.c
@@ -43,26 +43,84 @@ 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, int fd)
+{
+ 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 = 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, fd);
}
static void *notification_handler(void *data)
@@ -72,7 +130,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 +140,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 +165,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 +177,7 @@ static void *notification_handler(void *data)
}
}
- notification_dispatch(ev, fd);
+ handle_msg(buf, ret, fd);
}
close(notif_sk);
diff --git a/android/hal-ipc.h b/android/hal-ipc.h
index ea53e1c..9f867f8 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, int fd);
+ 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 0/5] android: IPC improvements
From: Szymon Janc @ 2013-11-16 14:13 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
Hi,
Changes since RFCv3:
- use correct email for commit author
- no longer RFC - as per my testing this works fine and already
start catching up IPC bugs which would not be introduced with those
patches present
--
BR
Szymon Janc
Szymon Janc (5):
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-a2dp.c | 40 ++++-----
android/hal-bluetooth.c | 223 ++++++++++++++++++++++++++++++------------------
android/hal-hidhost.c | 75 +++++++++-------
android/hal-ipc.c | 115 ++++++++++++++++---------
android/hal-ipc.h | 10 +++
android/hal-pan.c | 39 ++++-----
android/hal.h | 4 -
7 files changed, 312 insertions(+), 194 deletions(-)
--
1.8.4.2
^ permalink raw reply
* [PATCH 3/3] android: Refactor update_found_device function
From: Szymon Janc @ 2013-11-16 14:09 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384610986-17990-1-git-send-email-szymon.janc@tieto.com>
This makes function flow easier to follow and understand. Besides that
it also fix issue with sending to many bytes if some prop were not
present in EIR.
---
android/bluetooth.c | 141 +++++++++++++++++++++-------------------------------
1 file changed, 57 insertions(+), 84 deletions(-)
diff --git a/android/bluetooth.c b/android/bluetooth.c
index 6a7ba9d..5c5c61e 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -776,54 +776,32 @@ static void confirm_device_name(const bdaddr_t *addr, uint8_t addr_type)
error("Failed to send confirm name request");
}
-static int fill_device_props(struct hal_property *prop, bdaddr_t *addr,
- uint32_t cod, int8_t rssi, char *name)
-{
- uint8_t num_props = 0;
-
- /* fill Class of Device */
- if (cod) {
- prop->type = HAL_PROP_DEVICE_CLASS;
- prop->len = sizeof(cod);
- memcpy(prop->val, &cod, prop->len);
- prop = ((void *) prop) + sizeof(*prop) + sizeof(cod);
- num_props++;
- }
- /* fill RSSI */
- if (rssi) {
- prop->type = HAL_PROP_DEVICE_RSSI;
- prop->len = sizeof(rssi);
- memcpy(prop->val, &rssi, prop->len);
- prop = ((void *) prop) + sizeof(*prop) + sizeof(rssi);
- num_props++;
- }
+static int fill_hal_prop(void *buf, uint8_t type, uint16_t len,
+ const void *val)
+{
+ struct hal_property *prop = buf;
- /* fill name */
- if (name) {
- prop->type = HAL_PROP_DEVICE_NAME;
- prop->len = strlen(name);
- memcpy(prop->val, name, prop->len);
- prop = ((void *) prop) + sizeof(*prop) + prop->len;
- num_props++;
- }
+ prop->type = type;
+ prop->len = len;
+ memcpy(prop->val, val, len);
- return num_props;
+ return sizeof(*prop) + len;
}
static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
int8_t rssi, bool confirm,
const uint8_t *data, uint8_t data_len)
{
- bool is_new_dev = false;
- size_t props_size = 0;
- size_t buff_size = 0;
- void *buf;
+ uint8_t buf[BLUEZ_HAL_MTU];
+ bool new_dev = false;
struct eir_data eir;
- GSList *l;
- bdaddr_t *remote = NULL;
+ uint8_t *num_prop;
+ uint8_t opcode;
+ int size = 0;
int err;
+ memset(buf, 0, sizeof(buf));
memset(&eir, 0, sizeof(eir));
err = eir_parse(&eir, data, data_len);
@@ -832,75 +810,70 @@ static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
return;
}
- l = g_slist_find_custom(found_devices, bdaddr, bdaddr_cmp);
- if (l)
- remote = l->data;
-
- if (!remote) {
+ if (!g_slist_find_custom(found_devices, bdaddr, bdaddr_cmp)) {
+ bdaddr_t *new_bdaddr;
char addr[18];
- remote = g_new0(bdaddr_t, 1);
- bacpy(remote, bdaddr);
+ new_bdaddr = g_new0(bdaddr_t, 1);
+ bacpy(new_bdaddr, bdaddr);
- found_devices = g_slist_prepend(found_devices, remote);
- is_new_dev = true;
+ found_devices = g_slist_prepend(found_devices, new_bdaddr);
- ba2str(remote, addr);
+ ba2str(new_bdaddr, addr);
DBG("New device found: %s", addr);
- }
- props_size += sizeof(struct hal_property) + sizeof(eir.class);
- props_size += sizeof(struct hal_property) + sizeof(rssi);
-
- if (eir.name) {
- props_size += sizeof(struct hal_property) + strlen(eir.name);
- cache_device_name(remote, eir.name);
+ new_dev = true;
}
- if (is_new_dev) {
- struct hal_ev_device_found *ev = NULL;
- struct hal_property *prop = NULL;
-
- /* with new device we also send bdaddr prop */
- props_size += sizeof(struct hal_property) + sizeof(eir.addr);
+ if (new_dev) {
+ struct hal_ev_device_found *ev = (void *) buf;
+ bdaddr_t android_bdaddr;
- buff_size = sizeof(struct hal_ev_device_found) + props_size;
- buf = g_new0(char, buff_size);
- ev = buf;
- prop = ev->props;
+ size += sizeof(*ev);
- /* fill first prop with bdaddr */
- prop->type = HAL_PROP_DEVICE_ADDR;
- prop->len = sizeof(bdaddr_t);
- bdaddr2android(bdaddr, prop->val);
- prop = ((void *) prop) + sizeof(*prop) + sizeof(bdaddr_t);
- ev->num_props += 1;
+ num_prop = &ev->num_props;
+ opcode = HAL_EV_DEVICE_FOUND;
- /* fill eir, name, and cod props */
- ev->num_props += fill_device_props(prop, remote, eir.class,
- rssi, eir.name);
+ bdaddr2android(bdaddr, &android_bdaddr);
- ipc_send(notification_sk, HAL_SERVICE_ID_BLUETOOTH,
- HAL_EV_DEVICE_FOUND, buff_size, ev, -1);
- g_free(buf);
+ size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_ADDR,
+ sizeof(android_bdaddr), &android_bdaddr);
+ (*num_prop)++;
} else {
- struct hal_ev_remote_device_props *ev = NULL;
+ struct hal_ev_remote_device_props *ev = (void *) buf;
- buff_size = sizeof(*ev) + props_size;
- buf = g_new0(char, buff_size);
- ev = buf;
+ size += sizeof(*ev);
- ev->num_props = fill_device_props(ev->props, remote, eir.class,
- rssi, eir.name);
+ num_prop = &ev->num_props;
+ opcode = HAL_EV_REMOTE_DEVICE_PROPS;
ev->status = HAL_STATUS_SUCCESS;
bdaddr2android(bdaddr, ev->bdaddr);
+ }
+
+ if (eir.class) {
+ size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_CLASS,
+ sizeof(eir.class), &eir.class);
+ (*num_prop)++;
+ }
- ipc_send(notification_sk, HAL_SERVICE_ID_BLUETOOTH,
- HAL_EV_REMOTE_DEVICE_PROPS, buff_size, ev, -1);
- g_free(buf);
+ if (rssi) {
+ size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_RSSI,
+ sizeof(rssi), &rssi);
+ (*num_prop)++;
+ }
+
+ if (eir.name) {
+ cache_device_name(bdaddr, eir.name);
+
+ size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_NAME,
+ strlen(eir.name), eir.name);
+ (*num_prop)++;
}
+ ipc_send(notification_sk, HAL_SERVICE_ID_BLUETOOTH, opcode, size, buf,
+ -1);
+
if (confirm) {
char addr[18];
--
1.8.4.2
^ permalink raw reply related
* [PATCH 2/3] android: Fix sending invalid remote device property event
From: Szymon Janc @ 2013-11-16 14:09 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384610986-17990-1-git-send-email-szymon.janc@tieto.com>
Remote device property event has variable length, pass whole event
length to ipc_send, not only header.
---
android/bluetooth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/android/bluetooth.c b/android/bluetooth.c
index 4439cc6..6a7ba9d 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -605,7 +605,7 @@ static void send_remote_device_name_prop(const bdaddr_t *bdaddr)
memcpy(&ev->props[0].val, name, strlen(name));
ipc_send(notification_sk, HAL_SERVICE_ID_BLUETOOTH,
- HAL_EV_REMOTE_DEVICE_PROPS, sizeof(ev), ev, -1);
+ HAL_EV_REMOTE_DEVICE_PROPS, ev_len, ev, -1);
g_free(ev);
}
--
1.8.4.2
^ permalink raw reply related
* [PATCH 1/3] android: Fix sending remote device property if name is not present
From: Szymon Janc @ 2013-11-16 14:09 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
This fix missing bdaddr to string convertion if name was NULL. This
was resulting in using undefined dst value.
---
android/bluetooth.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/android/bluetooth.c b/android/bluetooth.c
index 83f20e2..4439cc6 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -589,8 +589,10 @@ static void send_remote_device_name_prop(const bdaddr_t *bdaddr)
/* Use cached name or bdaddr string */
name = get_device_name(bdaddr);
- if (!name)
+ if (!name) {
+ ba2str(bdaddr, dst);
name = dst;
+ }
ev_len = BASELEN_REMOTE_DEV_PROP + strlen(name);
ev = g_malloc0(ev_len);
--
1.8.4.2
^ permalink raw reply related
* Re: [PATCH] Staging: btmtk_usb: Add hdev parameter to hdev->send driver callback
From: Greg Kroah-Hartman @ 2013-11-15 23:39 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Dan Carpenter, Geert Uytterhoeven, devel, Johan Hedberg,
Yu-Chen Cho, linux-kernel@vger.kernel.org list,
linux-bluetooth@vger.kernel.org development, jay.hung
In-Reply-To: <324E99F5-B9B7-4E56-BE91-42964AFF5BA8@holtmann.org>
On Sat, Nov 16, 2013 at 07:36:31AM +0900, Marcel Holtmann wrote:
> Hi Greg,
>
> >>>>> while this is patch is correct, I do not really care about staging drivers that actually bluntly violate my copyright.
> >>>>>
> >>>>
> >>>> That's very cryptic.
> >>>>
> >>>> What is going on here? I googled it and I wasn't able to find what you
> >>>> are talking about. Care to give us a hint and what you want us to do
> >>>> here?
> >>>
> >>> the last time I checked, the majority of drivers/bluetooth/btusb.c has been written by myself. Now go and compare btusb.c to btmtk_usb.[ch].
> >>>
> >>>> I have also added Johan Hedberg to the CC list because he also helped
> >>>> break the build. Don't do that.
> >>>
> >>> Yes, we are doing exactly that. It is a staging driver. I could not care less if a staging drivers breaks the build or not.
> >>>
> >>> If anybody cares about this driver, then take the time to merge it upstream. It has never been submitted to linux-bluetooth mailing list.
> >>>
> >>> There are drivers that should have never been merged into staging.
> >>> This is one of them. Look for yourself and explain to me why this
> >>> driver is part of staging in the first place.
> >>
> >> Because it was sent to me by a developer?
> >
> > it is a problem when staging just becomes a dumping ground for drivers that the distributions find somewhere on the Internet or CD-ROMs. And then nobody has any intentions to clean up and integrate properly. This one did not even go through linux-bluetooth mailing list once. It was submitted right to staging. And then the submitter walked away.
>
> and if I quote the TODO file:
>
> TODO:
> - checkpatch.pl clean
> - determine if the driver should not be using a duplicate
> version of the usb-bluetooth interface code, but should
> be merged into the drivers/bluetooth/ directory and
> infrastructure instead.
> - review by the bluetooth developer community
>
> Please send any patches for this driver to Yu-Chen, Cho <acho@suse.com> and
> jay.hung@mediatek.com
>
> So from the submission we can assume that the submitter knew that this
> was duplicated code. The code also never got submitted for review to
> linux-bluetooth. And now 6 month later, none of the TODO items have
> been actually worked on.
>
> I do not know what your timeline is for removing drivers from staging,
> but this one seems to be a good candidate to get removed next.
6 months without any active contribution to getting it cleaned up and
merged is the timeline. Which this one fits, so yes, I will remove it
for 3.14, unless Jay or Cho is going to start doing work on this.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] Staging: btmtk_usb: Add hdev parameter to hdev->send driver callback
From: Marcel Holtmann @ 2013-11-15 22:36 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Dan Carpenter, Geert Uytterhoeven, devel, Johan Hedberg,
Yu-Chen Cho, linux-kernel@vger.kernel.org list,
linux-bluetooth@vger.kernel.org development, jay.hung
In-Reply-To: <6C5AC1E9-D9A0-473A-B47A-345018DFAC3C@holtmann.org>
Hi Greg,
>>>>> while this is patch is correct, I do not really care about staging drivers that actually bluntly violate my copyright.
>>>>>
>>>>
>>>> That's very cryptic.
>>>>
>>>> What is going on here? I googled it and I wasn't able to find what you
>>>> are talking about. Care to give us a hint and what you want us to do
>>>> here?
>>>
>>> the last time I checked, the majority of drivers/bluetooth/btusb.c has been written by myself. Now go and compare btusb.c to btmtk_usb.[ch].
>>>
>>>> I have also added Johan Hedberg to the CC list because he also helped
>>>> break the build. Don't do that.
>>>
>>> Yes, we are doing exactly that. It is a staging driver. I could not care less if a staging drivers breaks the build or not.
>>>
>>> If anybody cares about this driver, then take the time to merge it upstream. It has never been submitted to linux-bluetooth mailing list.
>>>
>>> There are drivers that should have never been merged into staging.
>>> This is one of them. Look for yourself and explain to me why this
>>> driver is part of staging in the first place.
>>
>> Because it was sent to me by a developer?
>
> it is a problem when staging just becomes a dumping ground for drivers that the distributions find somewhere on the Internet or CD-ROMs. And then nobody has any intentions to clean up and integrate properly. This one did not even go through linux-bluetooth mailing list once. It was submitted right to staging. And then the submitter walked away.
and if I quote the TODO file:
TODO:
- checkpatch.pl clean
- determine if the driver should not be using a duplicate
version of the usb-bluetooth interface code, but should
be merged into the drivers/bluetooth/ directory and
infrastructure instead.
- review by the bluetooth developer community
Please send any patches for this driver to Yu-Chen, Cho <acho@suse.com> and
jay.hung@mediatek.com
So from the submission we can assume that the submitter knew that this was duplicated code. The code also never got submitted for review to linux-bluetooth. And now 6 month later, none of the TODO items have been actually worked on.
I do not know what your timeline is for removing drivers from staging, but this one seems to be a good candidate to get removed next.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH] Staging: btmtk_usb: Add hdev parameter to hdev->send driver callback
From: Marcel Holtmann @ 2013-11-15 22:26 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Dan Carpenter, Geert Uytterhoeven, devel, Johan Hedberg,
Yu-Chen Cho, linux-kernel@vger.kernel.org list,
linux-bluetooth@vger.kernel.org development, jay.hung
In-Reply-To: <20131115212936.GC14897@kroah.com>
Hi Greg,
>>>> while this is patch is correct, I do not really care about staging drivers that actually bluntly violate my copyright.
>>>>
>>>
>>> That's very cryptic.
>>>
>>> What is going on here? I googled it and I wasn't able to find what you
>>> are talking about. Care to give us a hint and what you want us to do
>>> here?
>>
>> the last time I checked, the majority of drivers/bluetooth/btusb.c has been written by myself. Now go and compare btusb.c to btmtk_usb.[ch].
>>
>>> I have also added Johan Hedberg to the CC list because he also helped
>>> break the build. Don't do that.
>>
>> Yes, we are doing exactly that. It is a staging driver. I could not care less if a staging drivers breaks the build or not.
>>
>> If anybody cares about this driver, then take the time to merge it upstream. It has never been submitted to linux-bluetooth mailing list.
>>
>> There are drivers that should have never been merged into staging.
>> This is one of them. Look for yourself and explain to me why this
>> driver is part of staging in the first place.
>
> Because it was sent to me by a developer?
it is a problem when staging just becomes a dumping ground for drivers that the distributions find somewhere on the Internet or CD-ROMs. And then nobody has any intentions to clean up and integrate properly. This one did not even go through linux-bluetooth mailing list once. It was submitted right to staging. And then the submitter walked away.
> Seriously, what's the issue here, I didn't notice it was a fork of your
> code, sorry, I didn't check. I figured it would be eventually cleaned
> up properly and then it will be sent to linux-bluetooth for proper
> merging.
Kernel subsystem maintainers should not be responsible for fixing staging drivers. I did not even know that this driver existed in staging. I remember you saying that we can just ignore staging. The group of people looking after staging will take care of drivers that break.
Actually I am taken personal offense when someone takes my code, removes my copyright and slaps their copyright notice on top of it. Yes, I am looking at you Mediatek. I feel completely in my right to say that I am not touching this driver or care if it breaks.
And of course there is the technical problem that this driver as it is should not exist in the first place. We do not need duplicated code. The only difference between btusb.c and btmtk_usb.c is the driver init for loading firmware and poking at vendor specific registers. The Bluetooth subsystem has a vendor specific driver setup stage for exactly this. That should be used instead of forking the driver.
> Yu-Chen, what's the satus of getting this cleaned up "properly"? You
> haven't really done anything on this since I took the driver in May.
My comment above stands, distributions do not seem to care :(
The Realtek Bluetooth driver is the same mess. I rejected it for the same reasons, but so far nobody made the effort to clean it up and build it as mini-driver of btusb.c.
Only our Intel guys managed to get that one done properly for their hardware. So yes, it can be done. I am not talking about unicorns here ;)
Regards
Marcel
^ permalink raw reply
* Re: [PATCH] Staging: btmtk_usb: Add hdev parameter to hdev->send driver callback
From: Greg Kroah-Hartman @ 2013-11-15 21:29 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Dan Carpenter, Geert Uytterhoeven, devel, Johan Hedberg,
Yu-Chen Cho, linux-kernel@vger.kernel.org list,
linux-bluetooth@vger.kernel.org development
In-Reply-To: <F615BA13-BEC1-457B-8002-BC6C149039EA@holtmann.org>
On Fri, Nov 15, 2013 at 10:26:41PM +0900, Marcel Holtmann wrote:
> Hi Dan,
>
> >> while this is patch is correct, I do not really care about staging drivers that actually bluntly violate my copyright.
> >>
> >
> > That's very cryptic.
> >
> > What is going on here? I googled it and I wasn't able to find what you
> > are talking about. Care to give us a hint and what you want us to do
> > here?
>
> the last time I checked, the majority of drivers/bluetooth/btusb.c has been written by myself. Now go and compare btusb.c to btmtk_usb.[ch].
>
> > I have also added Johan Hedberg to the CC list because he also helped
> > break the build. Don't do that.
>
> Yes, we are doing exactly that. It is a staging driver. I could not care less if a staging drivers breaks the build or not.
>
> If anybody cares about this driver, then take the time to merge it upstream. It has never been submitted to linux-bluetooth mailing list.
>
> There are drivers that should have never been merged into staging.
> This is one of them. Look for yourself and explain to me why this
> driver is part of staging in the first place.
Because it was sent to me by a developer?
Seriously, what's the issue here, I didn't notice it was a fork of your
code, sorry, I didn't check. I figured it would be eventually cleaned
up properly and then it will be sent to linux-bluetooth for proper
merging.
Yu-Chen, what's the satus of getting this cleaned up "properly"? You
haven't really done anything on this since I took the driver in May.
thanks,
greg k-h
^ permalink raw reply
* [RFC v3 5/5] android/hal-a2dp: Use generic IPC message handling for events
From: Szymon Janc @ 2013-11-15 20:49 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384548578-18710-1-git-send-email-szymon.janc@tieto.com>
Register handlers on service init and unregister on cleanup.
---
android/hal-a2dp.c | 40 ++++++++++++++++++++--------------------
android/hal.h | 1 -
2 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/android/hal-a2dp.c b/android/hal-a2dp.c
index e9fadb7..baa4b10 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, int fd)
{
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, int fd)
{
struct hal_ev_a2dp_audio_state *ev = buf;
@@ -48,24 +48,19 @@ 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 */
+static const struct hal_ipc_handler ev_handlers[] = {
+ {
+ .handler = handle_conn_state,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_pan_conn_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 +100,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 +124,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.3
^ permalink raw reply related
* [RFC v3 4/5] android/hal-pan: Use generic IPC message handling for events
From: Szymon Janc @ 2013-11-15 20:49 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384548578-18710-1-git-send-email-szymon.janc@tieto.com>
Register handlers on service init and unregister on cleanup.
---
android/hal-pan.c | 39 ++++++++++++++++++++-------------------
android/hal.h | 1 -
2 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/android/hal-pan.c b/android/hal-pan.c
index 2bc560e..dff2a44 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, int fd)
{
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, int fd)
{
struct hal_ev_pan_ctrl_state *ev = buf;
@@ -50,23 +50,19 @@ 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 */
+static const struct hal_ipc_handler ev_handlers[] = {
+ {
+ .handler = handle_conn_state,
+ .var_len = false,
+ .data_len = sizeof(struct 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 +139,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 +163,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.3
^ permalink raw reply related
* [RFC v3 3/5] android/hal-hidhost: Use generic IPC message handling for events
From: Szymon Janc @ 2013-11-15 20:49 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384548578-18710-1-git-send-email-szymon.janc@tieto.com>
Register handlers on service init and unregister on cleanup.
---
android/hal-hidhost.c | 75 +++++++++++++++++++++++++++++----------------------
android/hal.h | 1 -
2 files changed, 43 insertions(+), 33 deletions(-)
diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
index 2ce17a3..27ce492 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, int fd)
{
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, int fd)
{
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, int fd)
{
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, int fd)
{
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, int fd)
{
struct hal_ev_hidhost_virtual_unplug *ev = buf;
@@ -87,33 +92,34 @@ 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 */
+static const struct hal_ipc_handler ev_handlers[] = {
+ {
+ .handler = handle_conn_state,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_hidhost_conn_state)
+ },
+ {
+ .handler = handle_info,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_hidhost_info),
+ },
+ {
+ .handler = handle_proto_mode,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_hidhost_proto_mode),
+ },
+ {
+ .handler = handle_get_report,
+ .var_len = true,
+ .data_len = sizeof(struct hal_ev_hidhost_get_report),
+ },
+ {
+ .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 +368,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 +392,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.3
^ permalink raw reply related
* [RFC v3 2/5] android/hal-bluetooth: Register IPC message handlers
From: Szymon Janc @ 2013-11-15 20:49 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384548578-18710-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 | 223 ++++++++++++++++++++++++++++++------------------
android/hal.h | 1 -
2 files changed, 142 insertions(+), 82 deletions(-)
diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 078d537..2ec7c10 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, int fd)
{
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,24 +138,48 @@ static void device_hal_props_cleanup(bt_property_t *props, uint8_t num)
}
}
-static void handle_adapter_props_changed(void *buf, uint16_t len)
+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, int fd)
{
struct hal_ev_adapter_props_changed *ev = buf;
bt_property_t props[ev->num_props];
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, int fd)
{
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, int fd)
{
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, int fd)
{
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, int fd)
{
struct hal_ev_discovery_state_changed *ev = buf;
@@ -231,34 +246,38 @@ static void handle_discovery_state_changed(void *buf)
bt_hal_cbacks->discovery_state_changed_cb(ev->state);
}
-static void handle_device_found(void *buf, uint16_t len)
+static void handle_device_found(void *buf, uint16_t len, int fd)
{
struct hal_ev_device_found *ev = buf;
bt_property_t props[ev->num_props];
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);
device_hal_props_cleanup(props, ev->num_props);
}
-static void handle_device_state_changed(void *buf, uint16_t len)
+static void handle_device_state_changed(void *buf, uint16_t len, int fd)
{
struct hal_ev_remote_device_props *ev = buf;
bt_property_t props[ev->num_props];
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, int fd)
{
struct hal_ev_acl_state_changed *ev = buf;
bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
@@ -279,48 +298,82 @@ 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, int fd)
{
- 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, int fd)
+{
+ DBG("");
+
+ /* TODO */
+}
+
+/* handlers will be called from notification thread context */
+static const struct hal_ipc_handler ev_handlers[] = {
+ {
+ .handler = handle_adapter_state_changed,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_adapter_state_changed)
+ },
+ {
+ .handler = handle_adapter_props_changed,
+ .var_len = true,
+ .data_len = sizeof(struct hal_ev_adapter_props_changed) +
+ sizeof(struct hal_property),
+ },
+ {
+ .handler = handle_device_state_changed,
+ .var_len = true,
+ .data_len = sizeof(struct hal_ev_remote_device_props) +
+ sizeof(struct hal_property),
+ },
+ {
+ .handler = handle_device_found,
+ .var_len = true,
+ .data_len = sizeof(struct hal_ev_device_found) +
+ sizeof(struct hal_property),
+ },
+ {
+ .handler = handle_discovery_state_changed,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_discovery_state_changed),
+ },
+ {
+ .handler = handle_pin_request,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_pin_request),
+ },
+ {
+ .handler = handle_ssp_request,
+ .var_len = false,
+ .data_len = sizeof(handle_ssp_request),
+ },
+ {
+ .handler = handle_bond_state_change,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_bond_state_changed),
+ },
+ {
+ .handler = handle_acl_state_changed,
+ .var_len = false,
+ .data_len = sizeof(struct hal_ev_acl_state_changed),
+ },
+ {
+ .handler = handle_dut_mode_receive,
+ .var_len = true,
+ .data_len = sizeof(struct hal_ev_dut_mode_receive),
+ },
+ {
+ .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 +386,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 +417,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 +455,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.3
^ permalink raw reply related
* [RFC v3 1/5] android/hal: Add initial code for IPC message handlers
From: Szymon Janc @ 2013-11-15 20:49 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384548578-18710-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 | 115 ++++++++++++++++++++++++++++++++++++------------------
android/hal-ipc.h | 10 +++++
2 files changed, 87 insertions(+), 38 deletions(-)
diff --git a/android/hal-ipc.c b/android/hal-ipc.c
index 026e245..53af897 100644
--- a/android/hal-ipc.c
+++ b/android/hal-ipc.c
@@ -43,26 +43,84 @@ 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, int fd)
+{
+ 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 = 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, fd);
}
static void *notification_handler(void *data)
@@ -72,7 +130,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 +140,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 +165,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 +177,7 @@ static void *notification_handler(void *data)
}
}
- notification_dispatch(ev, fd);
+ handle_msg(buf, ret, fd);
}
close(notif_sk);
diff --git a/android/hal-ipc.h b/android/hal-ipc.h
index ea53e1c..9f867f8 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, int fd);
+ 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.3
^ permalink raw reply related
* [RFC v3 0/5] android: IPC improvements
From: Szymon Janc @ 2013-11-15 20:49 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
Hi,
Third version of RFC, changes since v2:
- as suggested by Johan and Luiz handlers are called from IPC code,
not from message code, this allows for informative logs while reporting
errors
- fixed few bugs
Comments and testing are welcome!
--
BR
Szymon Janc
Szymon Janc (5):
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-a2dp.c | 40 ++++-----
android/hal-bluetooth.c | 223 ++++++++++++++++++++++++++++++------------------
android/hal-hidhost.c | 75 +++++++++-------
android/hal-ipc.c | 115 ++++++++++++++++---------
android/hal-ipc.h | 10 +++
android/hal-pan.c | 39 ++++-----
android/hal.h | 4 -
7 files changed, 312 insertions(+), 194 deletions(-)
--
1.8.4.3
^ permalink raw reply
* [PATCH] android: Refactor update_found_device function
From: Szymon Janc @ 2013-11-15 20:40 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
This makes function flow easier to follow and understand. Besides that
it also fix issue with sending to many bytes if some prop were not
present in EIR.
---
android/bluetooth.c | 141 +++++++++++++++++++++-------------------------------
1 file changed, 57 insertions(+), 84 deletions(-)
diff --git a/android/bluetooth.c b/android/bluetooth.c
index 83f20e2..e3190d1 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -774,54 +774,32 @@ static void confirm_device_name(const bdaddr_t *addr, uint8_t addr_type)
error("Failed to send confirm name request");
}
-static int fill_device_props(struct hal_property *prop, bdaddr_t *addr,
- uint32_t cod, int8_t rssi, char *name)
-{
- uint8_t num_props = 0;
-
- /* fill Class of Device */
- if (cod) {
- prop->type = HAL_PROP_DEVICE_CLASS;
- prop->len = sizeof(cod);
- memcpy(prop->val, &cod, prop->len);
- prop = ((void *) prop) + sizeof(*prop) + sizeof(cod);
- num_props++;
- }
- /* fill RSSI */
- if (rssi) {
- prop->type = HAL_PROP_DEVICE_RSSI;
- prop->len = sizeof(rssi);
- memcpy(prop->val, &rssi, prop->len);
- prop = ((void *) prop) + sizeof(*prop) + sizeof(rssi);
- num_props++;
- }
+static int fill_hal_prop(void *buf, uint8_t type, uint16_t len,
+ const void *val)
+{
+ struct hal_property *prop = buf;
- /* fill name */
- if (name) {
- prop->type = HAL_PROP_DEVICE_NAME;
- prop->len = strlen(name);
- memcpy(prop->val, name, prop->len);
- prop = ((void *) prop) + sizeof(*prop) + prop->len;
- num_props++;
- }
+ prop->type = type;
+ prop->len = len;
+ memcpy(prop->val, val, len);
- return num_props;
+ return sizeof(*prop) + len;
}
static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
int8_t rssi, bool confirm,
const uint8_t *data, uint8_t data_len)
{
- bool is_new_dev = false;
- size_t props_size = 0;
- size_t buff_size = 0;
- void *buf;
+ uint8_t buf[BLUEZ_HAL_MTU];
+ bool new_dev = false;
struct eir_data eir;
- GSList *l;
- bdaddr_t *remote = NULL;
+ uint8_t *num_prop;
+ uint8_t opcode;
+ int size = 0;
int err;
+ memset(buf, 0, sizeof(buf));
memset(&eir, 0, sizeof(eir));
err = eir_parse(&eir, data, data_len);
@@ -830,75 +808,70 @@ static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
return;
}
- l = g_slist_find_custom(found_devices, bdaddr, bdaddr_cmp);
- if (l)
- remote = l->data;
-
- if (!remote) {
+ if (!g_slist_find_custom(found_devices, bdaddr, bdaddr_cmp)) {
+ bdaddr_t *new_bdaddr;
char addr[18];
- remote = g_new0(bdaddr_t, 1);
- bacpy(remote, bdaddr);
+ new_bdaddr = g_new0(bdaddr_t, 1);
+ bacpy(new_bdaddr, bdaddr);
- found_devices = g_slist_prepend(found_devices, remote);
- is_new_dev = true;
+ found_devices = g_slist_prepend(found_devices, new_bdaddr);
- ba2str(remote, addr);
+ ba2str(new_bdaddr, addr);
DBG("New device found: %s", addr);
- }
- props_size += sizeof(struct hal_property) + sizeof(eir.class);
- props_size += sizeof(struct hal_property) + sizeof(rssi);
-
- if (eir.name) {
- props_size += sizeof(struct hal_property) + strlen(eir.name);
- cache_device_name(remote, eir.name);
+ new_dev = true;
}
- if (is_new_dev) {
- struct hal_ev_device_found *ev = NULL;
- struct hal_property *prop = NULL;
-
- /* with new device we also send bdaddr prop */
- props_size += sizeof(struct hal_property) + sizeof(eir.addr);
+ if (new_dev) {
+ struct hal_ev_device_found *ev = (void *) buf;
+ bdaddr_t android_bdaddr;
- buff_size = sizeof(struct hal_ev_device_found) + props_size;
- buf = g_new0(char, buff_size);
- ev = buf;
- prop = ev->props;
+ size += sizeof(*ev);
- /* fill first prop with bdaddr */
- prop->type = HAL_PROP_DEVICE_ADDR;
- prop->len = sizeof(bdaddr_t);
- bdaddr2android(bdaddr, prop->val);
- prop = ((void *) prop) + sizeof(*prop) + sizeof(bdaddr_t);
- ev->num_props += 1;
+ num_prop = &ev->num_props;
+ opcode = HAL_EV_DEVICE_FOUND;
- /* fill eir, name, and cod props */
- ev->num_props += fill_device_props(prop, remote, eir.class,
- rssi, eir.name);
+ bdaddr2android(bdaddr, &android_bdaddr);
- ipc_send(notification_sk, HAL_SERVICE_ID_BLUETOOTH,
- HAL_EV_DEVICE_FOUND, buff_size, ev, -1);
- g_free(buf);
+ size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_ADDR,
+ sizeof(android_bdaddr), &android_bdaddr);
+ (*num_prop)++;
} else {
- struct hal_ev_remote_device_props *ev = NULL;
+ struct hal_ev_remote_device_props *ev = (void *) buf;
- buff_size = sizeof(*ev) + props_size;
- buf = g_new0(char, buff_size);
- ev = buf;
+ size += sizeof(*ev);
- ev->num_props = fill_device_props(ev->props, remote, eir.class,
- rssi, eir.name);
+ num_prop = &ev->num_props;
+ opcode = HAL_EV_REMOTE_DEVICE_PROPS;
ev->status = HAL_STATUS_SUCCESS;
bdaddr2android(bdaddr, ev->bdaddr);
+ }
+
+ if (eir.class) {
+ size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_CLASS,
+ sizeof(eir.class), &eir.class);
+ (*num_prop)++;
+ }
- ipc_send(notification_sk, HAL_SERVICE_ID_BLUETOOTH,
- HAL_EV_REMOTE_DEVICE_PROPS, buff_size, ev, -1);
- g_free(buf);
+ if (rssi) {
+ size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_RSSI,
+ sizeof(rssi), &rssi);
+ (*num_prop)++;
+ }
+
+ if (eir.name) {
+ cache_device_name(bdaddr, eir.name);
+
+ size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_NAME,
+ strlen(eir.name), eir.name);
+ (*num_prop)++;
}
+ ipc_send(notification_sk, HAL_SERVICE_ID_BLUETOOTH, opcode, size, buf,
+ -1);
+
if (confirm) {
char addr[18];
--
1.8.4.3
^ permalink raw reply related
* Re: [RFC BlueZ v1] doc: Add GATT API
From: Scott James Remnant @ 2013-11-15 19:47 UTC (permalink / raw)
To: Claudio Takahasi; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CAKT1EBc2d3VkiUF705aNY79YDmL5ZRX75fg=a4a_ZfTPt4zoDA@mail.gmail.com>
On Tue, Nov 12, 2013 at 10:49 AM, Claudio Takahasi
<claudio.takahasi@openbossa.org> wrote:
> On Mon, Nov 11, 2013 at 3:56 PM, Scott James Remnant <keybuk@google.com> wrote:
>> How will service changed be handled? How will BlueZ track the set of
>> applications, and the set of services etc. defined by those
>> applications in a manner that keeps handles consistent? How will it
>> handle generating the Services Changed notification in the cases where
>> the set of applications and/or services change, or the handles change?
>
> We implemented a hash of declarations. Using the "Id" provided in the
> options dictionary (see RegisterAgent) we are able to identity if the
> external service changed its attributes.
> However, I don' t think we will upstream this approach soon, Marcel
> wants a simpler approach: always send ServiceChanged.
>
While this is probably "spec sufficient", and probably sufficient for
passing qualification, I'm not sure that this is necessarily the best
approach since this means that BlueZ when acting as a GATT Server
wouldn't be behaving quite the same as, say, a commodity BTLE device.
>>> +Characteristic hierarchy
>>> +========================
>> :
>>> +Service org.bluez
>>> +Interface org.bluez.Characteristic1 [Experimental]
>>> +Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/serviceXX/charYYYY
>>
>> This would also need a "Permissions" property akin to the one you have
>> for Descriptors - characteristics can be "not accessible", read-only,
>> write-only, read/write - and can also require authorization,
>> authentication, encryption and minimum encryption key sizes - as with
>> descriptors.
>
> It is implemented already, there is an optional "Flags" property :
> "array{string} Flags [read-only, optional]"
>
Flags seemed to correspond to the Flags characteristic descriptor and
not the simple permissions of the characteristic itself.
>>> + array{byte} Value [read-write]
>>> +
>>> + Cached Value of the characteristic. If present, the
>>> + value will be cached by bluetoothd and updated when the
>>> + PropertiesChanged signal is emitted.
>>> +
>>> + External services must emit this signal when the
>>> + characteristic supports notification/indication, so
>>> + that clients can be notified of the new value.
>>
>> The PropertiesChanged signal explains how Notification will be handled
>> - but how will Indication? How will a service receive the Indication
>> Confirmation from the remote devices?
>
> The bluetoothd core manages the Confirmation. In my opinion clients
> listening for PropertiesChanged don' t need to know the difference
> between notification and indication.
> Allow an external client to manage the Confirmation will insert
> additional complexity without giving real benefits.
>
I'm thinking of the opposite way around - not the clients, but the services.
If I implement a service over the D-Bus API, and a characteristic
supports Indication, then is it not important that the service be
informed when the clients confirm the Indication that is sent out?
Otherwise it makes Indications identical to Notifications when
implementing a service using the BlueZ D-Bus API, which may cause
issues with implementing certain profiles.
>>> +Application Agent hierarchy
>>> +===========================
>>> +
>>> +Service unique name
>>> +Interface org.bluez.ApplicationAgent1 [Experimental]
>>> +Object path freely definable
>>> +
>>
>> "Agent" seems unnnecessary here - if the object is an Application,
>> then org.bluez.Application1 would be a decent enough name. Thus an
>> "Application" consists of multiple Services, each of which consists of
>> multiple Characteristics, each of which has multiple Descriptors
>
> IMO "Agent" gives a better association with its functionality, it
> reminds me org.bluez.Agent1.
> Let's wait the opinion of the others developers...
>
I was more thinking that we have "AgentManager" -> "Agent",
"ProfileManager" -> "Profile", "ServiceManager" -> "Service" ... all
of those use the agent-style pattern, but only "Pairing" Agent is
called Agent.
But honestly, bike shedding ;-) I'm only complaining because calling
it "ApplicationAgent" would slightly screw up my naming convention
inside Chromium
Scott
--
Scott James Remnant | Chrome OS Systems | keybuk@google.com | Google
^ permalink raw reply
* Re: pull request: bluetooth 2013-11-11
From: John W. Linville @ 2013-11-15 19:19 UTC (permalink / raw)
To: Gustavo Padovan, linux-wireless, linux-bluetooth, linux-kernel
In-Reply-To: <20131115151554.GB1627@joana>
On Fri, Nov 15, 2013 at 01:15:54PM -0200, Gustavo Padovan wrote:
> Hi John,
>
> 2013-11-11 John W. Linville <linville@tuxdriver.com>:
>
> > On Mon, Nov 11, 2013 at 04:27:38PM -0200, Gustavo Padovan wrote:
> > > Hi John,
> > >
> > > A few fixes for 3.13. There is 3 fixes to the RFCOMM protocol. One crash fix to
> > > L2CAP. A simple fix to a bad behaviour in the SMP protocol, and last, an
> > > revert, that I sent in the last pull request but doesn't seem to be in your
> > > tree.
> >
> > Are you looking in the wireless-next tree? It seems to be there, no?
>
> Yes, it is there. I'm resending this pull request without it:
>
> Please pull or let me know of any problems! Thanks
>
> Gustavo
>
> ---
>
> The following changes since commit 8ce9beac4661f576ea0d518b9f086bb52a171a37:
>
> drivers: net: wireless: b43: Fix possible NULL ptr dereference (2013-10-18 13:41:11 -0400)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth for-upstream
>
> for you to fetch changes up to 86ca9eac31d0d8c4fe61b5726e6d63197bc435a6:
>
> Bluetooth: Fix rejecting SMP security request in slave role (2013-11-13 11:36:54 -0200)
Pulling now...
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* Re: [PATCH v5 0/4] Fixes for incoming connection and bonding
From: Johan Hedberg @ 2013-11-15 18:24 UTC (permalink / raw)
To: Lukasz Rymanowski; +Cc: linux-bluetooth
In-Reply-To: <1384538641-1873-1-git-send-email-lukasz.rymanowski@tieto.com>
Hi Lukasz,
On Fri, Nov 15, 2013, Lukasz Rymanowski wrote:
> V5:
> Fixes according to Johan comments
>
> V4:
> Added device name caching in device list
> Changes according to Johan comments
>
> V3:
> Removed adapter bonding state
> Added device list in order to track bonding state per device.
> Fix for incoming legacy paring. We act here same as Bluedroid.
>
> V2:
> Bonding state added.
>
> V1:
> With those patches it is possible to bond from remote device and in addition
> you will see the name of remote device on Android pairing request pop up.
>
>
> Lukasz Rymanowski (4):
> android: Update bond state on incoming bonding
> android: Cache device name on device list.
> android: Update bond state on auth and connect failed
> android: Change TODO with explaining comment
>
> android/bluetooth.c | 212 ++++++++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 173 insertions(+), 39 deletions(-)
All patches have been applied. Thanks.
I did need to do some (minor) coding style tweaks for 2/4 though.
Johan
^ permalink raw reply
* [PATCH v5 4/4] android: Change TODO with explaining comment
From: Lukasz Rymanowski @ 2013-11-15 18:04 UTC (permalink / raw)
To: linux-bluetooth; +Cc: johan.hedberg, Lukasz Rymanowski
In-Reply-To: <1384538641-1873-1-git-send-email-lukasz.rymanowski@tieto.com>
---
android/bluetooth.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/android/bluetooth.c b/android/bluetooth.c
index 37ac7b1..095f44a 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -645,7 +645,10 @@ static void send_ssp_request(const bdaddr_t *addr, uint8_t variant,
{
struct hal_ev_ssp_request ev;
- /* TODO name and CoD of remote devices should probably be cached */
+ /* It is ok to have empty name and CoD of remote devices here since
+ * those information has been already provided on device_connected event
+ * or during device scaning. Android will use that instead.
+ */
memset(&ev, 0, sizeof(ev));
bdaddr2android(addr, ev.bdaddr);
ev.pairing_variant = variant;
--
1.8.4
^ permalink raw reply related
* [PATCH v5 3/4] android: Update bond state on auth and connect failed
From: Lukasz Rymanowski @ 2013-11-15 18:04 UTC (permalink / raw)
To: linux-bluetooth; +Cc: johan.hedberg, Lukasz Rymanowski
In-Reply-To: <1384538641-1873-1-git-send-email-lukasz.rymanowski@tieto.com>
---
android/bluetooth.c | 57 ++++++++++++++++++++++++++++++++---------------------
1 file changed, 35 insertions(+), 22 deletions(-)
diff --git a/android/bluetooth.c b/android/bluetooth.c
index 853f5a8..37ac7b1 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -987,16 +987,51 @@ static void mgmt_device_disconnected_event(uint16_t index, uint16_t length,
HAL_EV_ACL_STATE_CHANGED, sizeof(hal_ev), &hal_ev, -1);
}
+static uint8_t status_mgmt2hal(uint8_t mgmt)
+{
+ switch (mgmt) {
+ case MGMT_STATUS_SUCCESS:
+ return HAL_STATUS_SUCCESS;
+ case MGMT_STATUS_NO_RESOURCES:
+ return HAL_STATUS_NOMEM;
+ case MGMT_STATUS_BUSY:
+ return HAL_STATUS_BUSY;
+ case MGMT_STATUS_NOT_SUPPORTED:
+ return HAL_STATUS_UNSUPPORTED;
+ case MGMT_STATUS_INVALID_PARAMS:
+ return HAL_STATUS_INVALID;
+ case MGMT_STATUS_AUTH_FAILED:
+ return HAL_STATUS_AUTH_FAILURE;
+ case MGMT_STATUS_NOT_CONNECTED:
+ return HAL_STATUS_REMOTE_DEVICE_DOWN;
+ default:
+ return HAL_STATUS_FAILED;
+ }
+}
+
static void mgmt_connect_failed_event(uint16_t index, uint16_t length,
const void *param, void *user_data)
{
+ const struct mgmt_ev_connect_failed *ev = param;
+
DBG("");
+
+ /* In case security mode 3 pairing we will get connect failed event
+ * in case e.g wrong PIN code entered. Let's check if device is
+ * bonding, if so update bond state */
+ set_device_bond_state(&ev->addr.bdaddr, status_mgmt2hal(ev->status),
+ HAL_BOND_STATE_NONE);
}
static void mgmt_auth_failed_event(uint16_t index, uint16_t length,
const void *param, void *user_data)
{
+ const struct mgmt_ev_auth_failed *ev = param;
+
DBG("");
+
+ set_device_bond_state(&ev->addr.bdaddr, status_mgmt2hal(ev->status),
+ HAL_BOND_STATE_NONE);
}
static void mgmt_device_unpaired_event(uint16_t index, uint16_t length,
@@ -1924,28 +1959,6 @@ static uint8_t set_property(void *buf, uint16_t len)
}
}
-static uint8_t status_mgmt2hal(uint8_t mgmt)
-{
- switch (mgmt) {
- case MGMT_STATUS_SUCCESS:
- return HAL_STATUS_SUCCESS;
- case MGMT_STATUS_NO_RESOURCES:
- return HAL_STATUS_NOMEM;
- case MGMT_STATUS_BUSY:
- return HAL_STATUS_BUSY;
- case MGMT_STATUS_NOT_SUPPORTED:
- return HAL_STATUS_UNSUPPORTED;
- case MGMT_STATUS_INVALID_PARAMS:
- return HAL_STATUS_INVALID;
- case MGMT_STATUS_AUTH_FAILED:
- return HAL_STATUS_AUTH_FAILURE;
- case MGMT_STATUS_NOT_CONNECTED:
- return HAL_STATUS_REMOTE_DEVICE_DOWN;
- default:
- return HAL_STATUS_FAILED;
- }
-}
-
static void pair_device_complete(uint8_t status, uint16_t length,
const void *param, void *user_data)
{
--
1.8.4
^ 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