* [PATCH v2 1/3] android: Update bond state on incoming bonding
From: Lukasz Rymanowski @ 2013-11-12 12:51 UTC (permalink / raw)
To: linux-bluetooth; +Cc: johan.hedberg, szymon.janc, Lukasz Rymanowski
In-Reply-To: <1384260700-16621-1-git-send-email-lukasz.rymanowski@tieto.com>
Before sending any ssp request or pin code request up to HAL library we
need to send bond state change with bonding state. Otherwise incoming
bonding is impossible.
Add bonding tracking to adapter.
---
android/adapter.c | 50 +++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 45 insertions(+), 5 deletions(-)
diff --git a/android/adapter.c b/android/adapter.c
index 65b3170..f92301e 100644
--- a/android/adapter.c
+++ b/android/adapter.c
@@ -74,6 +74,8 @@ struct bt_adapter {
bool discovering;
uint32_t discoverable_timeout;
+
+ bool bonding;
};
struct browse_req {
@@ -486,6 +488,7 @@ static void new_link_key_callback(uint16_t index, uint16_t length,
send_bond_state_change(&addr->bdaddr, HAL_STATUS_SUCCESS,
HAL_BOND_STATE_BONDED);
+ adapter->bonding = false;
browse_remote_sdp(&addr->bdaddr);
}
@@ -501,6 +504,13 @@ static void pin_code_request_callback(uint16_t index, uint16_t length,
return;
}
+ if (!adapter->bonding) {
+ adapter->bonding = true;
+ /* Update bonding state */
+ send_bond_state_change(&ev->addr.bdaddr, HAL_STATUS_SUCCESS,
+ HAL_BOND_STATE_BONDING);
+ }
+
ba2str(&ev->addr.bdaddr, dst);
DBG("%s type %u secure %u", dst, ev->addr.type, ev->secure);
@@ -542,6 +552,13 @@ static void user_confirm_request_callback(uint16_t index, uint16_t length,
ba2str(&ev->addr.bdaddr, dst);
DBG("%s confirm_hint %u", dst, ev->confirm_hint);
+ if (!adapter->bonding) {
+ adapter->bonding = true;
+ /* Update bonding state */
+ send_bond_state_change(&ev->addr.bdaddr, HAL_STATUS_SUCCESS,
+ HAL_BOND_STATE_BONDING);
+ }
+
if (ev->confirm_hint)
send_ssp_request(&ev->addr.bdaddr, HAL_SSP_VARIANT_CONSENT, 0);
else
@@ -563,6 +580,13 @@ static void user_passkey_request_callback(uint16_t index, uint16_t length,
ba2str(&ev->addr.bdaddr, dst);
DBG("%s", dst);
+ if (!adapter->bonding) {
+ adapter->bonding = true;
+ /* Update bonding state */
+ send_bond_state_change(&ev->addr.bdaddr, HAL_STATUS_SUCCESS,
+ HAL_BOND_STATE_BONDING);
+ }
+
send_ssp_request(&ev->addr.bdaddr, HAL_SSP_VARIANT_ENTRY, 0);
}
@@ -581,9 +605,17 @@ static void user_passkey_notify_callback(uint16_t index, uint16_t length,
DBG("%s entered %u", dst, ev->entered);
/* HAL seems to not support entered characters */
- if (!ev->entered)
- send_ssp_request(&ev->addr.bdaddr, HAL_SSP_VARIANT_NOTIF,
- ev->passkey);
+ if (ev->entered)
+ return;
+
+ if (!adapter->bonding) {
+ adapter->bonding=true;
+ /* Update bonding state */
+ send_bond_state_change(&ev->addr.bdaddr, HAL_STATUS_SUCCESS,
+ HAL_BOND_STATE_BONDING);
+ }
+
+ send_ssp_request(&ev->addr.bdaddr, HAL_SSP_VARIANT_NOTIF, ev->passkey);
}
static void mgmt_discovering_event(uint16_t index, uint16_t length,
@@ -1166,6 +1198,7 @@ void bt_adapter_init(uint16_t index, struct mgmt *mgmt, bt_adapter_ready cb)
adapter->ready = cb;
/* TODO: Read it from some storage */
adapter->discoverable_timeout = DEFAULT_DISCOVERABLE_TIMEOUT;
+ adapter->bonding = false;
if (mgmt_send(mgmt, MGMT_OP_READ_INFO, index, 0, NULL,
read_info_complete, NULL, NULL) > 0)
@@ -1486,6 +1519,7 @@ static void pair_device_complete(uint8_t status, uint16_t length,
if (status == MGMT_STATUS_SUCCESS)
return;
+ adapter->bonding = false;
send_bond_state_change(&rp->addr.bdaddr, status_mgmt2hal(status),
HAL_BOND_STATE_NONE);
}
@@ -1504,6 +1538,7 @@ static bool create_bond(void *buf, uint16_t len)
NULL) == 0)
return false;
+ adapter->bonding = true;
send_bond_state_change(&cp.addr.bdaddr, HAL_STATUS_SUCCESS,
HAL_BOND_STATE_BONDING);
@@ -1514,13 +1549,18 @@ static bool cancel_bond(void *buf, uint16_t len)
{
struct hal_cmd_cancel_bond *cmd = buf;
struct mgmt_addr_info cp;
+ bool result;
cp.type = BDADDR_BREDR;
android2bdaddr(cmd->bdaddr, &cp.bdaddr);
- return mgmt_reply(adapter->mgmt, MGMT_OP_CANCEL_PAIR_DEVICE,
+ result = mgmt_reply(adapter->mgmt, MGMT_OP_CANCEL_PAIR_DEVICE,
adapter->index, sizeof(cp), &cp, NULL, NULL,
NULL) > 0;
+ if (result)
+ adapter->bonding = false;
+
+ return result;
}
static void unpair_device_complete(uint8_t status, uint16_t length,
@@ -1737,7 +1777,7 @@ void bt_adapter_handle_cmd(int sk, uint8_t opcode, void *buf, uint16_t len)
break;
case HAL_OP_CREATE_BOND:
- if (!create_bond(buf, len))
+ if (adapter->bonding || !create_bond(buf, len))
goto error;
break;
--
1.8.4
^ permalink raw reply related
* [PATCH v2 2/3] android: Update HAL with device info on incoming connection
From: Lukasz Rymanowski @ 2013-11-12 12:51 UTC (permalink / raw)
To: linux-bluetooth; +Cc: johan.hedberg, szymon.janc, Lukasz Rymanowski
In-Reply-To: <1384260700-16621-1-git-send-email-lukasz.rymanowski@tieto.com>
Make sure Android have information about connecting remote device. This
is needed for example to show device name on incoming bonding request.
---
android/adapter.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/android/adapter.c b/android/adapter.c
index f92301e..a59ab4e 100644
--- a/android/adapter.c
+++ b/android/adapter.c
@@ -857,9 +857,8 @@ static void mgmt_device_connected_event(uint16_t index, uint16_t length,
return;
}
- /* TODO: Update device */
-
- /* TODO: Check Set bonding state */
+ update_found_device(&ev->addr.bdaddr, ev->addr.type, 0, false,
+ &ev->eir[0], btohs(ev->eir_len));
hal_ev.status = HAL_STATUS_SUCCESS;
hal_ev.state = HAL_ACL_STATE_CONNECTED;
--
1.8.4
^ permalink raw reply related
* [PATCH v2 3/3] android: Change TODO with explaining comment
From: Lukasz Rymanowski @ 2013-11-12 12:51 UTC (permalink / raw)
To: linux-bluetooth; +Cc: johan.hedberg, szymon.janc, Lukasz Rymanowski
In-Reply-To: <1384260700-16621-1-git-send-email-lukasz.rymanowski@tieto.com>
---
android/adapter.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/android/adapter.c b/android/adapter.c
index a59ab4e..ac1ebc2 100644
--- a/android/adapter.c
+++ b/android/adapter.c
@@ -515,7 +515,10 @@ static void pin_code_request_callback(uint16_t index, uint16_t length,
DBG("%s type %u secure %u", dst, ev->addr.type, ev->secure);
- /* 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(&hal_ev, 0, sizeof(hal_ev));
bdaddr2android(&ev->addr.bdaddr, hal_ev.bdaddr);
@@ -528,7 +531,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
* Re: [PATCH v2 1/3] android: Update bond state on incoming bonding
From: Johan Hedberg @ 2013-11-12 13:02 UTC (permalink / raw)
To: Lukasz Rymanowski; +Cc: linux-bluetooth, szymon.janc
In-Reply-To: <1384260700-16621-2-git-send-email-lukasz.rymanowski@tieto.com>
Hi Lukasz,
On Tue, Nov 12, 2013, Lukasz Rymanowski wrote:
> Before sending any ssp request or pin code request up to HAL library we
> need to send bond state change with bonding state. Otherwise incoming
> bonding is impossible.
> Add bonding tracking to adapter.
> ---
> android/adapter.c | 50 +++++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 45 insertions(+), 5 deletions(-)
>
> diff --git a/android/adapter.c b/android/adapter.c
> index 65b3170..f92301e 100644
> --- a/android/adapter.c
> +++ b/android/adapter.c
> @@ -74,6 +74,8 @@ struct bt_adapter {
>
> bool discovering;
> uint32_t discoverable_timeout;
> +
> + bool bonding;
> };
>
> struct browse_req {
> @@ -486,6 +488,7 @@ static void new_link_key_callback(uint16_t index, uint16_t length,
> send_bond_state_change(&addr->bdaddr, HAL_STATUS_SUCCESS,
> HAL_BOND_STATE_BONDED);
>
> + adapter->bonding = false;
> browse_remote_sdp(&addr->bdaddr);
> }
>
> @@ -501,6 +504,13 @@ static void pin_code_request_callback(uint16_t index, uint16_t length,
> return;
> }
>
> + if (!adapter->bonding) {
> + adapter->bonding = true;
> + /* Update bonding state */
> + send_bond_state_change(&ev->addr.bdaddr, HAL_STATUS_SUCCESS,
> + HAL_BOND_STATE_BONDING);
> + }
Is there something explicitly blocking multiple parallel bonding
attempts (to different remote devices) in Android? If not this should
really be a list or hash-table with entries identifiable using the
remote bdaddr. In fact since we need this anyway for our LE address type
cache I don't see why it would hurt to have the infrastructure ready for
it now.
Johan
^ permalink raw reply
* [PATCH] android/hidhost: Fix error handling issue incase of G_IO_HUP
From: Ravi kumar Veeramally @ 2013-11-12 13:05 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Ravi kumar Veeramally
Incase of G_IO_HUP on GIOCondtion when hid device disconnected,
GIOCondition is combination of G_IO_IN and G_IO_HUP. Current code
tries to read as soon as it finds G_IO_IN in condition.
Apparently there is no data to read and loop continues.
---
android/hidhost.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/android/hidhost.c b/android/hidhost.c
index 491eacd..cba80a6 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -282,26 +282,27 @@ static gboolean intr_watch_cb(GIOChannel *chan, GIOCondition cond,
{
struct hid_device *dev = data;
+ if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL))
+ goto error;
+
if (cond & G_IO_IN)
return intr_io_watch_cb(chan, data);
+error:
+ bt_hid_notify_state(dev, HAL_HIDHOST_STATE_DISCONNECTED);
+
/* Checking for ctrl_watch avoids a double g_io_channel_shutdown since
* it's likely that ctrl_watch_cb has been queued for dispatching in
* this mainloop iteration */
if ((cond & (G_IO_HUP | G_IO_ERR)) && dev->ctrl_watch)
g_io_channel_shutdown(chan, TRUE, NULL);
- dev->intr_watch = 0;
-
- if (dev->intr_io) {
- g_io_channel_unref(dev->intr_io);
- dev->intr_io = NULL;
- }
-
/* Close control channel */
if (dev->ctrl_io && !(cond & G_IO_NVAL))
g_io_channel_shutdown(dev->ctrl_io, TRUE, NULL);
+ hid_device_free(dev);
+
return FALSE;
}
@@ -442,12 +443,14 @@ static gboolean ctrl_watch_cb(GIOChannel *chan, GIOCondition cond,
gpointer data)
{
struct hid_device *dev = data;
- char address[18];
+
+ if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL))
+ goto error;
if (cond & G_IO_IN)
return ctrl_io_watch_cb(chan, data);
- ba2str(&dev->dst, address);
+error:
bt_hid_notify_state(dev, HAL_HIDHOST_STATE_DISCONNECTED);
/* Checking for intr_watch avoids a double g_io_channel_shutdown since
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH v2 3/3] android: Change TODO with explaining comment
From: Johan Hedberg @ 2013-11-12 13:09 UTC (permalink / raw)
To: Lukasz Rymanowski; +Cc: linux-bluetooth, szymon.janc
In-Reply-To: <1384260700-16621-4-git-send-email-lukasz.rymanowski@tieto.com>
Hi Lukasz,
On Tue, Nov 12, 2013, Lukasz Rymanowski wrote:
> ---
> android/adapter.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/android/adapter.c b/android/adapter.c
> index a59ab4e..ac1ebc2 100644
> --- a/android/adapter.c
> +++ b/android/adapter.c
> @@ -515,7 +515,10 @@ static void pin_code_request_callback(uint16_t index, uint16_t length,
>
> DBG("%s type %u secure %u", dst, ev->addr.type, ev->secure);
>
> - /* 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(&hal_ev, 0, sizeof(hal_ev));
> bdaddr2android(&ev->addr.bdaddr, hal_ev.bdaddr);
Have you considered security mode 3 devices? There you will get a pin
code request before you get a device_connected. You can even try this
yourself by doing "hciconfig hci0 auth" before attempting to pair. That
said, I'm not sure if there is anything we can do better regarding such
devices, but at least we should keep this use case in mind.
Johan
^ permalink raw reply
* Re: [PATCH v2 1/5] android: Add and remove sdp records and uuids
From: Johan Hedberg @ 2013-11-12 13:24 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth, Marcin Kraglak
In-Reply-To: <1384257985-26319-2-git-send-email-szymon.janc@tieto.com>
Hi,
On Tue, Nov 12, 2013, Szymon Janc wrote:
> +static void add_uuid(uint8_t svc_hint, uint128_t *uuid)
> +{
> + struct mgmt_cp_add_uuid cp;
> +
> + htob128(uuid, (uint128_t *) cp.uuid);
> + cp.svc_hint = svc_hint;
> +
> + mgmt_send(adapter->mgmt, MGMT_OP_ADD_UUID,
> + adapter->index, sizeof(cp), &cp,
> + add_uuid_complete, NULL, NULL);
> +}
> +
> +int bt_adapter_add_record(uint16_t uuid, sdp_record_t *rec, uint8_t svc_hint)
Is it really so that we can't infer the UUID by looking inside
sdp_record_t? (e.g. just use rec->svclass).
> +{
> + uint128_t uint128;
> +
> + if (g_slist_find(adapter->uuids, GUINT_TO_POINTER(uuid))) {
> + DBG("UUID 0x%x already added", uuid);
> + return -1;
> + }
Shouldn't this be return -EALREADY?
> + adapter->uuids = g_slist_prepend(adapter->uuids,
> + GUINT_TO_POINTER(uuid));
> +
> + uuid16_to_uint128(uuid, &uint128);
> +
> + add_uuid(svc_hint, &uint128);
If we only support adding UUID-16, then how about just providing
uint16_t to add/remove_uuid and do the conversion inside these
functions?
> +void bt_adapter_remove_record(uint16_t uuid, int handle)
The type that sdp_record_t uses for the handle is uint32_t so the
parameter here should reflect that.
Johan
^ permalink raw reply
* [PATCH BlueZ] core: Do not attempt to connect if adapter is not powered
From: Luiz Augusto von Dentz @ 2013-11-12 13:26 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
If the adapter is not yet powered do not attempt to connect, the same
applies if the error -EHOSTUNREACH is returned by the kernel.
---
src/device.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/device.c b/src/device.c
index 60efd62..e58c16c 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1156,7 +1156,7 @@ static void device_profile_connected(struct btd_device *dev,
if (dev->pending == NULL)
return;
- if (!dev->connected && err == -EHOSTDOWN)
+ if (!dev->connected && (err == -EHOSTDOWN || err == -EHOSTUNREACH))
goto done;
pending = dev->pending->data;
@@ -1322,6 +1322,9 @@ static DBusMessage *connect_profiles(struct btd_device *dev, DBusMessage *msg,
if (dev->pending || dev->connect || dev->browse)
return btd_error_in_progress(msg);
+ if (!btd_adapter_get_powered(dev->adapter))
+ return btd_error_not_ready(msg);
+
device_set_temporary(dev, FALSE);
if (!dev->svc_resolved)
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2 2/3] android: Update HAL with device info on incoming connection
From: Johan Hedberg @ 2013-11-12 13:27 UTC (permalink / raw)
To: Lukasz Rymanowski; +Cc: linux-bluetooth, szymon.janc
In-Reply-To: <1384260700-16621-3-git-send-email-lukasz.rymanowski@tieto.com>
Hi Lukasz,
On Tue, Nov 12, 2013, Lukasz Rymanowski wrote:
> Make sure Android have information about connecting remote device. This
> is needed for example to show device name on incoming bonding request.
> ---
> android/adapter.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/android/adapter.c b/android/adapter.c
> index f92301e..a59ab4e 100644
> --- a/android/adapter.c
> +++ b/android/adapter.c
> @@ -857,9 +857,8 @@ static void mgmt_device_connected_event(uint16_t index, uint16_t length,
> return;
> }
>
> - /* TODO: Update device */
> -
> - /* TODO: Check Set bonding state */
> + update_found_device(&ev->addr.bdaddr, ev->addr.type, 0, false,
> + &ev->eir[0], btohs(ev->eir_len));
>
> hal_ev.status = HAL_STATUS_SUCCESS;
> hal_ev.state = HAL_ACL_STATE_CONNECTED;
This patch has been applied. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH] android/hidhost: Fix uhid create failure case
From: Johan Hedberg @ 2013-11-12 13:28 UTC (permalink / raw)
To: Ravi kumar Veeramally; +Cc: linux-bluetooth
In-Reply-To: <1384249639-10915-1-git-send-email-ravikumar.veeramally@linux.intel.com>
Hi Ravi,
On Tue, Nov 12, 2013, Ravi kumar Veeramally wrote:
> If uhid open or create fails then notify state and free the device.
> Otherwise it replies bogus success return value on connect request.
> ---
> android/hidhost.c | 19 +++++--------------
> 1 file changed, 5 insertions(+), 14 deletions(-)
Applied. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH] android/hidhost: Fix error handling issue incase of G_IO_HUP
From: Johan Hedberg @ 2013-11-12 13:28 UTC (permalink / raw)
To: Ravi kumar Veeramally; +Cc: linux-bluetooth
In-Reply-To: <1384261515-28725-1-git-send-email-ravikumar.veeramally@linux.intel.com>
Hi Ravi,
On Tue, Nov 12, 2013, Ravi kumar Veeramally wrote:
> Incase of G_IO_HUP on GIOCondtion when hid device disconnected,
> GIOCondition is combination of G_IO_IN and G_IO_HUP. Current code
> tries to read as soon as it finds G_IO_IN in condition.
> Apparently there is no data to read and loop continues.
> ---
> android/hidhost.c | 21 ++++++++++++---------
> 1 file changed, 12 insertions(+), 9 deletions(-)
Applied. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH 1/4] android/pan: Add PAN related defines and event struct to hsl-msg header
From: Johan Hedberg @ 2013-11-12 13:33 UTC (permalink / raw)
To: Ravi kumar Veeramally; +Cc: linux-bluetooth
In-Reply-To: <1384181621-8070-2-git-send-email-ravikumar.veeramally@linux.intel.com>
Hi Ravi,
On Mon, Nov 11, 2013, Ravi kumar Veeramally wrote:
> +#define HAL_EV_PAN_CONN_STATE 0x81
> +struct hal_ev_pan_conn_state {
> + uint8_t state;
> + uint8_t status;
> + uint8_t bdaddr[6];
> + uint8_t local_role;
> + uint8_t remote_role;
> +} __attribute__((packed));
> +
> +#define HAL_EV_PAN_CTRL_STATE 0x82
> +struct hal_ev_pan_ctrl_state {
> + uint8_t state;
> + uint8_t status;
> + uint8_t local_role;
> + uint8_t name[17];
> +} __attribute__((packed));
These are in different in hal-ipc-api.txt:
Opcode 0x81 - Control State notification
Opcode 0x82 - Connection State notification
You really need to start paying more attention to details like this. As
long as you don't do it it means I need to spend more time verifying
every detail in your patches, which in turn means that it takes longer
before your patches get reviewed and eventually go upstream.
Johan
^ permalink raw reply
* Re: [PATCH 1/4] android/pan: Add PAN related defines and event struct to hsl-msg header
From: Ravi Kumar Veeramally @ 2013-11-12 13:43 UTC (permalink / raw)
To: linux-bluetooth, Johan Hedberg
In-Reply-To: <20131112133328.GG22931@x220.p-661hnu-f1>
Hi Johan,
On 11/12/2013 03:33 PM, Johan Hedberg wrote:
> Hi Ravi,
>
> On Mon, Nov 11, 2013, Ravi kumar Veeramally wrote:
>> +#define HAL_EV_PAN_CONN_STATE 0x81
>> +struct hal_ev_pan_conn_state {
>> + uint8_t state;
>> + uint8_t status;
>> + uint8_t bdaddr[6];
>> + uint8_t local_role;
>> + uint8_t remote_role;
>> +} __attribute__((packed));
>> +
>> +#define HAL_EV_PAN_CTRL_STATE 0x82
>> +struct hal_ev_pan_ctrl_state {
>> + uint8_t state;
>> + uint8_t status;
>> + uint8_t local_role;
>> + uint8_t name[17];
>> +} __attribute__((packed));
> These are in different in hal-ipc-api.txt:
>
> Opcode 0x81 - Control State notification
> Opcode 0x82 - Connection State notification
>
> You really need to start paying more attention to details like this. As
> long as you don't do it it means I need to spend more time verifying
> every detail in your patches, which in turn means that it takes longer
> before your patches get reviewed and eventually go upstream.
>
> Johan
>
Really sorry about that and wasting your time. I will keep that in mind.
Sorry once again.
Regards,
Ravi.
^ permalink raw reply
* Re: [PATCH 1/7] android: Remove bt_adapter structure
From: Johan Hedberg @ 2013-11-12 13:47 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth, Szymon Janc
In-Reply-To: <1384215857-19679-2-git-send-email-szymon.janc@tieto.com>
Hi Szymon,
On Tue, Nov 12, 2013, Szymon Janc wrote:
> Only one controller is used so there is no need to keep extra
> abstraction for it.
> ---
> android/adapter.c | 271 +++++++++++++++++++++++++-----------------------------
> 1 file changed, 123 insertions(+), 148 deletions(-)
>
> diff --git a/android/adapter.c b/android/adapter.c
> index 65b3170..e161f9d 100644
> --- a/android/adapter.c
> +++ b/android/adapter.c
> @@ -58,23 +58,17 @@ static int notification_sk = -1;
> /* This list contains addresses which are asked for records */
> static GSList *browse_reqs;
>
> -struct bt_adapter {
> - uint16_t index;
> - struct mgmt *mgmt;
> +static uint16_t adapter_index = MGMT_INDEX_NONE;
> +static struct mgmt *mgmt_if = NULL;
> +static bt_adapter_ready adapter_ready = NULL;
> +static bdaddr_t adapter_bdaddr;
> +static uint32_t adapter_dev_class = 0;
> +static char *adapter_name = NULL;
> +static uint32_t current_settings = 0;
To be honest I'm not really liking this loss of some kind of
organization/grouping of these variables. Even if we don't dynamically
allocate a struct I'd rather have these collected up in their own static
struct, something like:
static struct adapter {
uint16_t index;
...
...
} adapter;
and then later e.g.:
{
if (adapter.index == MGMT_INDEX_NONE)
return;
...
}
This way you also get an automatic prefix for all the adapter variables
and don't need to include it explicitly in each variable name.
Things which are not directly tied together with the adapter (like
struct mgmt) you can possibly leave outside of the struct as separate
variables though.
Johan
^ permalink raw reply
* Re: [PATCH 2/3] android/hidhost: Remove deprecated idle opcode from ipc document
From: Ravi Kumar Veeramally @ 2013-11-12 14:00 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <5281F502.9080506@linux.intel.com>
Hi Marcel,
Any comments on my reply?
Thanks,
Ravi.
On 11/12/2013 11:29 AM, Ravi Kumar Veeramally wrote:
> Hi Marcel,
>
> On 11/12/2013 01:05 AM, Marcel Holtmann wrote:
>> Hi Ravi,
>>
>>> Idle time is deprecated in HID 1_1. So remove it from ipc document
>>> and update GET_REPORT and VIRTUAL_UNPLUG opcode values.
>>> ---
>>> android/hal-ipc-api.txt | 10 ++--------
>>> android/hal-msg.h | 4 ++--
>>> 2 files changed, 4 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
>>> index 91ea280..57f4c13 100644
>>> --- a/android/hal-ipc-api.txt
>>> +++ b/android/hal-ipc-api.txt
>>> @@ -614,20 +614,14 @@ Notifications:
>>> 0x01 = Boot
>>> 0xff = Unsupported
>>>
>>> - Opcode 0x84 - Idle Time notification
>>> -
>>> - Notification parameters: Remote address (6 octets)
>>> - Status (1 octet)
>>> - Idle time (2 octets)
>> so what does HID_1.1 actually mean? I still see this notification in
>> bt_hh.h.
>>
>> Regards
>>
>> Marcel
>>
> In HID_SPEC_V11 (3.1) Get Idle and Set Idle are deprecated. And even
> in bt_hh.h
> get_idle and set_idle interfaces are not available. Bluedroid had
> implementation
> of those two in bluedroid/btif/src/btif_hh.c but in interface struct
> those are commented out.
> I didn't find the call back call there. That's why I updated ipc-doc.
>
> Please let me know if I miss anything.
>
> Thanks,
> Ravi.
>
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [PATCH_v2 0/4] Implement missing PAN interface and notifications
From: Ravi kumar Veeramally @ 2013-11-12 14:20 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Ravi kumar Veeramally
v2: Fixed wrong number of opcode values as per Johan's comments.
v1: Patch set implements missing cleanup interface and adds event structs
to hal-msg.h and implements connection and control state notifications.
Ravi kumar Veeramally (4):
android/pan: Add PAN related defines and event struct to hsl-msg
header
android/pan: Add PAN cleanup interface implementation
android/pan: Add notify method to PAN notifications
android/pan: Handle connection and control state notifications
android/hal-msg.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
android/hal-pan.c | 46 +++++++++++++++++++++++++++++++++++++++++++---
android/hal.h | 1 +
3 files changed, 88 insertions(+), 3 deletions(-)
--
1.8.3.2
^ permalink raw reply
* [PATCH_v2 1/4] android/pan: Add PAN related defines and event struct to hsl-msg header
From: Ravi kumar Veeramally @ 2013-11-12 14:20 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Ravi kumar Veeramally
In-Reply-To: <1384266042-6344-1-git-send-email-ravikumar.veeramally@linux.intel.com>
---
android/hal-msg.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/android/hal-msg.h b/android/hal-msg.h
index 569c8ea..4dfd555 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -317,6 +317,33 @@ struct hal_cmd_a2dp_disconnect {
/* PAN HAL API */
+/* PAN Roles */
+#define HAL_PAN_ROLE_NONE 0x00
+#define HAL_PAN_ROLE_NAP 0x01
+#define HAL_PAN_ROLE_PANU 0x02
+
+/* PAN Control states */
+#define HAL_PAN_CTRL_ENABLED 0x00
+#define HAL_PAN_CTRL_DISABLED 0x01
+
+/* PAN Connection states */
+#define HAL_PAN_STATE_CONNECTED 0x00
+#define HAL_PAN_STATE_CONNECTING 0x01
+#define HAL_PAN_STATE_DISCONNECTED 0x02
+#define HAL_PAN_STATE_DISCONNECTING 0x03
+
+/* PAN status values */
+#define HAL_PAN_STATUS_FAIL 0x01
+#define HAL_PAN_STATUS_NOT_READY 0x02
+#define HAL_PAN_STATUS_NO_MEMORY 0x03
+#define HAL_PAN_STATUS_BUSY 0x04
+#define HAL_PAN_STATUS_DONE 0x05
+#define HAL_PAN_STATUS_UNSUPORTED 0x06
+#define HAL_PAN_STATUS_INVAL 0x07
+#define HAL_PAN_STATUS_UNHANDLED 0x08
+#define HAL_PAN_STATUS_AUTH_FAILED 0x09
+#define HAL_PAN_STATUS_DEVICE_DOWN 0x0A
+
#define HAL_OP_PAN_ENABLE 0x01
struct hal_cmd_pan_enable {
uint8_t local_role;
@@ -486,6 +513,23 @@ struct hal_ev_hidhost_virtual_unplug {
uint8_t status;
} __attribute__((packed));
+#define HAL_EV_PAN_CTRL_STATE 0x81
+struct hal_ev_pan_ctrl_state {
+ uint8_t state;
+ uint8_t status;
+ uint8_t local_role;
+ uint8_t name[17];
+} __attribute__((packed));
+
+#define HAL_EV_PAN_CONN_STATE 0x82
+struct hal_ev_pan_conn_state {
+ uint8_t state;
+ uint8_t status;
+ uint8_t bdaddr[6];
+ uint8_t local_role;
+ uint8_t remote_role;
+} __attribute__((packed));
+
#define HAL_EV_A2DP_CONNECTION_STATE 0x81
struct hal_ev_a2dp_connection_state {
uint8_t state;
--
1.8.3.2
^ permalink raw reply related
* [PATCH_v2 2/4] android/pan: Add PAN cleanup interface implementation
From: Ravi kumar Veeramally @ 2013-11-12 14:20 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Ravi kumar Veeramally
In-Reply-To: <1384266042-6344-1-git-send-email-ravikumar.veeramally@linux.intel.com>
---
android/hal-pan.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/android/hal-pan.c b/android/hal-pan.c
index cacc6c0..bec179f 100644
--- a/android/hal-pan.c
+++ b/android/hal-pan.c
@@ -114,16 +114,19 @@ static bt_status_t pan_init(const btpan_callbacks_t *callbacks)
static void pan_cleanup()
{
+ struct hal_cmd_register_module cmd;
+
DBG("");
if (!interface_ready())
return;
- /* TODO: disable service */
+ cbs = NULL;
- /* TODO: stop PAN thread */
+ cmd.service_id = HAL_SERVICE_ID_PAN;
- cbs = NULL;
+ hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_UNREGISTER_MODULE,
+ sizeof(cmd), &cmd, 0, NULL, NULL);
}
static btpan_interface_t pan_if = {
--
1.8.3.2
^ permalink raw reply related
* [PATCH_v2 3/4] android/pan: Add notify method to PAN notifications
From: Ravi kumar Veeramally @ 2013-11-12 14:20 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Ravi kumar Veeramally
In-Reply-To: <1384266042-6344-1-git-send-email-ravikumar.veeramally@linux.intel.com>
---
android/hal-pan.c | 6 ++++++
android/hal.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/android/hal-pan.c b/android/hal-pan.c
index bec179f..78d526c 100644
--- a/android/hal-pan.c
+++ b/android/hal-pan.c
@@ -31,6 +31,12 @@ static bool interface_ready(void)
return cbs != NULL;
}
+void bt_notify_pan(uint16_t opcode, void *buf, uint16_t len)
+{
+ if (!interface_ready())
+ return;
+}
+
static bt_status_t pan_enable(int local_role)
{
struct hal_cmd_pan_enable cmd;
diff --git a/android/hal.h b/android/hal.h
index 2ce7932..be5d491 100644
--- a/android/hal.h
+++ b/android/hal.h
@@ -31,3 +31,4 @@ void bt_thread_associate(void);
void bt_thread_disassociate(void);
void bt_notify_hidhost(uint16_t opcode, void *buf, uint16_t len);
void bt_notify_a2dp(uint16_t opcode, void *buf, uint16_t len);
+void bt_notify_pan(uint16_t opcode, void *buf, uint16_t len);
--
1.8.3.2
^ permalink raw reply related
* [PATCH_v2 4/4] android/pan: Handle connection and control state notifications
From: Ravi kumar Veeramally @ 2013-11-12 14:20 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Ravi kumar Veeramally
In-Reply-To: <1384266042-6344-1-git-send-email-ravikumar.veeramally@linux.intel.com>
---
android/hal-pan.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/android/hal-pan.c b/android/hal-pan.c
index 78d526c..595cde9 100644
--- a/android/hal-pan.c
+++ b/android/hal-pan.c
@@ -31,10 +31,41 @@ static bool interface_ready(void)
return cbs != NULL;
}
+static void handle_conn_state(void *buf)
+{
+ struct hal_ev_pan_conn_state *ev = buf;
+
+ if (cbs->connection_state_cb)
+ cbs->connection_state_cb(ev->state, ev->status,
+ (bt_bdaddr_t *) ev->bdaddr,
+ ev->local_role, ev->remote_role);
+}
+
+static void handle_ctrl_state(void *buf)
+{
+ struct hal_ev_pan_ctrl_state *ev = buf;
+
+ if (cbs->control_state_cb)
+ cbs->control_state_cb(ev->state, ev->status,
+ ev->local_role, (char *)ev->name);
+}
+
void bt_notify_pan(uint16_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;
+ }
}
static bt_status_t pan_enable(int local_role)
--
1.8.3.2
^ permalink raw reply related
* [PATCH v2 BlueZ] core: Do not attempt to connect if adapter is not powered
From: Luiz Augusto von Dentz @ 2013-11-12 14:32 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
If the adapter is not yet powered do not attempt to connect, also
abort any connection attempt in case of error -EHOSTUNREACH is returned
by the kernel which also means the adapter is not up or in case of
-ECONNABORTED which means the adapter was powered down during the
connection attempt.
---
v2: Add better description, add comments what each error code means and
treat -ECONNABORTED.
src/device.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/device.c b/src/device.c
index 60efd62..662ba26 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1156,8 +1156,15 @@ static void device_profile_connected(struct btd_device *dev,
if (dev->pending == NULL)
return;
- if (!dev->connected && err == -EHOSTDOWN)
- goto done;
+ if (!dev->connected) {
+ switch (-err) {
+ case EHOSTDOWN: /* page timeout */
+ case EHOSTUNREACH: /* adapter not powered */
+ case ECONNABORTED: /* adapter powered down */
+ goto done;
+ }
+ }
+
pending = dev->pending->data;
l = find_service_with_profile(dev->pending, profile);
@@ -1322,6 +1329,9 @@ static DBusMessage *connect_profiles(struct btd_device *dev, DBusMessage *msg,
if (dev->pending || dev->connect || dev->browse)
return btd_error_in_progress(msg);
+ if (!btd_adapter_get_powered(dev->adapter))
+ return btd_error_not_ready(msg);
+
device_set_temporary(dev, FALSE);
if (!dev->svc_resolved)
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2 BlueZ] core: Do not attempt to connect if adapter is not powered
From: Johan Hedberg @ 2013-11-12 14:52 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1384266749-309-1-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
On Tue, Nov 12, 2013, Luiz Augusto von Dentz wrote:
> If the adapter is not yet powered do not attempt to connect, also
> abort any connection attempt in case of error -EHOSTUNREACH is returned
> by the kernel which also means the adapter is not up or in case of
> -ECONNABORTED which means the adapter was powered down during the
> connection attempt.
> ---
> v2: Add better description, add comments what each error code means and
> treat -ECONNABORTED.
>
> src/device.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
Applied. Thanks.
Johan
^ permalink raw reply
* [PATCH_v2 1/3] android/hidhost: Handle uhid output and feature events
From: Ravi kumar Veeramally @ 2013-11-12 15:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Ravi kumar Veeramally
Data read on uhid events output and feature has to be send through
SET_REPORT request to HID device.
---
android/hidhost.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/android/hidhost.c b/android/hidhost.c
index cba80a6..556e5a5 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -155,7 +155,30 @@ static void hid_device_free(struct hid_device *dev)
static void handle_uhid_event(struct hid_device *dev, struct uhid_event *ev)
{
- DBG("UHID_OUTPUT UHID_FEATURE unsupported");
+ int fd, i;
+ uint8_t *req = NULL;
+ uint8_t req_size = 0;
+
+ if (!(dev->ctrl_io))
+ return;
+
+ req_size = 1 + (ev->u.output.size / 2);
+ req = g_try_malloc0(req_size);
+ if (!req)
+ return;
+
+ req[0] = HID_MSG_SET_REPORT | ev->u.output.rtype;
+ for (i = 0; i < (req_size - 1); i++)
+ sscanf((char *) &(ev->u.output.data)[i * 2],
+ "%hhx", &(req + 1)[i]);
+
+ fd = g_io_channel_unix_get_fd(dev->ctrl_io);
+
+ if (write(fd, req, req_size) < 0)
+ error("error writing set_report: %s (%d)",
+ strerror(errno), errno);
+
+ g_free(req);
}
static gboolean uhid_event_cb(GIOChannel *io, GIOCondition cond,
--
1.8.3.2
^ permalink raw reply related
* [PATCH_v2 2/3] android/hidhost: Remove deprecated idle opcode from ipc document
From: Ravi kumar Veeramally @ 2013-11-12 15:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Ravi kumar Veeramally
In-Reply-To: <1384268835-7570-1-git-send-email-ravikumar.veeramally@linux.intel.com>
Idle time is deprecated in HID_SPEC1_1. So get and set idle time api's
are removed and not implemented. But callback is left out in Android
bt_hh.h. Generally this callback needs to be called when HAL requests
get and set idle time calls with status. So the method calls itself
removed, no point to implement this callback.
Also update GET_REPORT and VIRTUAL_UNPLUG opcode values.
---
android/hal-ipc-api.txt | 10 ++--------
android/hal-msg.h | 4 ++--
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
index 91ea280..57f4c13 100644
--- a/android/hal-ipc-api.txt
+++ b/android/hal-ipc-api.txt
@@ -614,20 +614,14 @@ Notifications:
0x01 = Boot
0xff = Unsupported
- Opcode 0x84 - Idle Time notification
-
- Notification parameters: Remote address (6 octets)
- Status (1 octet)
- Idle time (2 octets)
-
- Opcode 0x85 - Get Report notification
+ Opcode 0x84 - Get Report notification
Notification parameters: Remote address (6 octets)
Status (1 octet)
Report length (2 octets)
Report data (variable)
- Opcode 0x86 - Virtual Unplug notification
+ Opcode 0x85 - Virtual Unplug notification
Notification parameters: Remote address (6 octets)
Status (1 octet)
diff --git a/android/hal-msg.h b/android/hal-msg.h
index 4dfd555..847cc1f 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -499,7 +499,7 @@ struct hal_ev_hidhost_proto_mode {
uint8_t mode;
} __attribute__((packed));
-#define HAL_EV_HIDHOST_GET_REPORT 0x85
+#define HAL_EV_HIDHOST_GET_REPORT 0x84
struct hal_ev_hidhost_get_report {
uint8_t bdaddr[6];
uint8_t status;
@@ -507,7 +507,7 @@ struct hal_ev_hidhost_get_report {
uint8_t data[0];
} __attribute__((packed));
-#define HAL_EV_HIDHOST_VIRTUAL_UNPLUG 0x86
+#define HAL_EV_HIDHOST_VIRTUAL_UNPLUG 0x85
struct hal_ev_hidhost_virtual_unplug {
uint8_t bdaddr[6];
uint8_t status;
--
1.8.3.2
^ permalink raw reply related
* [PATCH_v2 3/3] android/hidhost: Set info request from HAL is not supported
From: Ravi kumar Veeramally @ 2013-11-12 15:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Ravi kumar Veeramally
In-Reply-To: <1384268835-7570-1-git-send-email-ravikumar.veeramally@linux.intel.com>
Data from hal_cmd_hidhost_set_info is usefull only when we create
UHID device. Once device is created all the transactions will be
done through the fd. There is no way to use this information
once device is created with HID internals.
---
android/hidhost.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/android/hidhost.c b/android/hidhost.c
index 556e5a5..95a98c3 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -839,9 +839,13 @@ static uint8_t bt_hid_virtual_unplug(struct hal_cmd_hidhost_virtual_unplug *cmd,
static uint8_t bt_hid_info(struct hal_cmd_hidhost_set_info *cmd, uint16_t len)
{
- DBG("Not Implemented");
+ /* Data from hal_cmd_hidhost_set_info is usefull only when we create
+ * UHID device. Once device is created all the transactions will be
+ * done through the fd. There is no way to use this information
+ * once device is created with HID internals. */
+ DBG("Not supported");
- return HAL_STATUS_FAILED;
+ return HAL_STATUS_UNSUPPORTED;
}
static uint8_t bt_hid_get_protocol(struct hal_cmd_hidhost_get_protocol *cmd,
--
1.8.3.2
^ 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