2013-11-07 09:19:50

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH 1/2 v2] android: Use BASELEN define for property changed

---
android/adapter.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/android/adapter.c b/android/adapter.c
index 1d462c8..45a40f9 100644
--- a/android/adapter.c
+++ b/android/adapter.c
@@ -47,6 +47,8 @@

/* Default to DisplayYesNo */
#define DEFAULT_IO_CAPABILITY 0x01
+#define BASELEN_PROP_CHANGED sizeof(struct hal_ev_adapter_props_changed) \
+ + (sizeof(struct hal_property))

static GIOChannel *notification_io = NULL;
/* This list contains addresses which are asked for records */
@@ -90,7 +92,7 @@ static void adapter_name_changed(const uint8_t *name)
{
struct hal_ev_adapter_props_changed *ev;
size_t len = strlen((const char *) name);
- uint8_t buf[sizeof(*ev) + sizeof(struct hal_property) + len];
+ uint8_t buf[BASELEN_PROP_CHANGED + len];

memset(buf, 0, sizeof(buf));
ev = (void *) buf;
--
1.8.4



2013-11-07 15:10:01

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 2/2] android: Add set/get for discovery timeout

Hi Lukasz,

On Thu, Nov 07, 2013, Lukasz Rymanowski wrote:
> Android handles discoverable timeout in Settings app, however still
> expects BT stack to maintain this value so we should do it as well.
> Otherwise we will get some unexpected behaviour.
> For now we keep discovery_timeout only during runtime, but we need to move
> it to some local storage once we will have it.
>
> Note: That since Android Settings up handles timer there is no reason to
> use discovery timer we have in kernel.
> ---
> android/adapter.c | 38 +++++++++++++++++++++++++++++++++++---
> 1 file changed, 35 insertions(+), 3 deletions(-)

You'll need to rebase this one due to the patches from Szymon that were
recently applied (right now it applies but doesn't compile).

Johan

2013-11-07 15:09:32

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 1/2 v2] android: Use BASELEN define for property changed

Hi Lukasz,

On Thu, Nov 07, 2013, Lukasz Rymanowski wrote:
> ---
> android/adapter.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)

This patch has been applied. Thanks.

Johan

2013-11-07 09:19:51

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH 2/2] android: Add set/get for discovery timeout

Android handles discoverable timeout in Settings app, however still
expects BT stack to maintain this value so we should do it as well.
Otherwise we will get some unexpected behaviour.
For now we keep discovery_timeout only during runtime, but we need to move
it to some local storage once we will have it.

Note: That since Android Settings up handles timer there is no reason to
use discovery timer we have in kernel.
---
android/adapter.c | 38 +++++++++++++++++++++++++++++++++++---
1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/android/adapter.c b/android/adapter.c
index 45a40f9..92e51b2 100644
--- a/android/adapter.c
+++ b/android/adapter.c
@@ -47,6 +47,9 @@

/* Default to DisplayYesNo */
#define DEFAULT_IO_CAPABILITY 0x01
+/* Default discoverable timeout 120sec as in Android */
+#define DEFAULT_DISCOVERABLE_TIMEOUT 120
+
#define BASELEN_PROP_CHANGED sizeof(struct hal_ev_adapter_props_changed) \
+ (sizeof(struct hal_property))

@@ -69,6 +72,7 @@ struct bt_adapter {
uint32_t current_settings;

bool discovering;
+ uint32_t discoverable_timeout;
};

struct browse_req {
@@ -1103,6 +1107,17 @@ static uint8_t set_adapter_name(uint8_t *name, uint16_t len)
return HAL_STATUS_FAILED;
}

+static uint8_t set_discoverable_timeout(uint8_t *timeout)
+{
+ /* Android handles discoverable timeout in Settings app.
+ * There is no need to use kernel feature for that.
+ * Just need to store this value here */
+
+ /* TODO: This should be in some storage */
+ memcpy(&adapter->discoverable_timeout, timeout, sizeof(uint32_t));
+
+ return HAL_STATUS_SUCCESS;
+}
static void read_info_complete(uint8_t status, uint16_t length, const void *param,
void *user_data)
{
@@ -1171,6 +1186,8 @@ void bt_adapter_init(uint16_t index, struct mgmt *mgmt, bt_adapter_ready cb)
adapter->index = index;
adapter->discovering = false;
adapter->ready = cb;
+ /* TODO: Read it from some storage */
+ adapter->discoverable_timeout = DEFAULT_DISCOVERABLE_TIMEOUT;

if (mgmt_send(mgmt, MGMT_OP_READ_INFO, index, 0, NULL,
read_info_complete, NULL, NULL) > 0)
@@ -1288,11 +1305,25 @@ static bool get_devices(void)

static bool get_discoverable_timeout(void)
{
- DBG("Not implemented");
+ struct hal_ev_adapter_props_changed *ev;
+ int len = sizeof(uint32_t);
+ uint8_t buf[BASELEN_PROP_CHANGED + len];

- /* TODO: Add implementation */

- return false;
+ memset(buf, 0, sizeof(buf));
+ ev = (void *) buf;
+
+ ev->num_props = 1;
+ ev->status = HAL_STATUS_SUCCESS;
+
+ ev->props[0].type = HAL_PROP_ADAPTER_DISC_TIMEOUT;
+ ev->props[0].len = len;
+ memcpy(&ev->props[0].val, &adapter->discoverable_timeout, sizeof(uint32_t));
+
+ ipc_send(notification_io, HAL_SERVICE_ID_BLUETOOTH,
+ HAL_EV_ADAPTER_PROPS_CHANGED, sizeof(buf), ev, -1);
+
+ return true;
}

static bool get_property(void *buf, uint16_t len)
@@ -1443,6 +1474,7 @@ static uint8_t set_property(void *buf, uint16_t len)
case HAL_PROP_ADAPTER_NAME:
return set_adapter_name(cmd->val, cmd->len);
case HAL_PROP_ADAPTER_DISC_TIMEOUT:
+ return set_discoverable_timeout(cmd->val);
default:
DBG("Unhandled property type 0x%x", cmd->type);
return HAL_STATUS_FAILED;
--
1.8.4


2013-11-07 08:14:18

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 2/2] android: Add set/get for discovery timeout

Hi Johan,

>> static bool get_discoverable_timeout(void)
>> {
>> - DBG("Not implemented");
>> + struct hal_ev_adapter_props_changed *ev;
>> + int len;
>>
>> - /* TODO: Add implementation */
>> + len = sizeof(*ev) + sizeof(struct hal_property) + sizeof(uint32_t);
>>
>
> The dynamic memory allocation seems a bit overkill here since the size is
> not very big and can be determined even at compile time. I'd consider
> doing something like:
>
> uint8_t buf[sizeof(struct hal_ev_adapter_props_changed) +
> sizeof(struct hal_property) + sizeof(uint32_t)];
> struct hal_ev_adapter_props_changed *ev = (void *) buf;

actually having some #define for the base len would be easier to read.

uint8_t buf[BASELEN + sizeof(uint32_t));

>
> ...
>
> ipc_send(..., sizeof(buf), buf, ...);

Of course an alternative is to use alloca to just allocate on the stack. Then no free is actually needed.

Regards

Marcel


2013-11-07 08:07:58

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 2/2] android: Add set/get for discovery timeout

Hi Lukasz,

On Wed, Nov 06, 2013, Lukasz Rymanowski wrote:
> static bool get_discoverable_timeout(void)
> {
> - DBG("Not implemented");
> + struct hal_ev_adapter_props_changed *ev;
> + int len;
>
> - /* TODO: Add implementation */
> + len = sizeof(*ev) + sizeof(struct hal_property) + sizeof(uint32_t);
>

The dynamic memory allocation seems a bit overkill here since the size is
not very big and can be determined even at compile time. I'd consider
doing something like:

uint8_t buf[sizeof(struct hal_ev_adapter_props_changed) +
sizeof(struct hal_property) + sizeof(uint32_t)];
struct hal_ev_adapter_props_changed *ev = (void *) buf;

...

ipc_send(..., sizeof(buf), buf, ...);

Johan