2013-11-05 08:53:41

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH] android/daemon: Send update name notification on mgmt evt

From: Andrei Emeltchenko <[email protected]>

Wehen receiving mgmt event local_name_changed we shall send notification
to the HAL that local name is changed.
---
android/adapter.c | 46 +++++++++++++++++++++++-----------------------
1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/android/adapter.c b/android/adapter.c
index 383cf07..b6f6096 100644
--- a/android/adapter.c
+++ b/android/adapter.c
@@ -66,6 +66,28 @@ struct bt_adapter {
static struct bt_adapter *adapter;
static GSList *found_devices = NULL;

+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];
+
+ memset(buf, 0, sizeof(buf));
+ ev = (void *) buf;
+
+ ev->num_props = 1;
+ ev->status = HAL_STATUS_SUCCESS;
+ ev->props[0].type = HAL_PROP_ADAPTER_NAME;
+ /* Android expects value without NULL terminator */
+ ev->props[0].len = len;
+ memcpy(ev->props->val, name, len);
+
+ DBG("Adapter name changed to: %s", name);
+
+ ipc_send(notification_io, HAL_SERVICE_ID_BLUETOOTH,
+ HAL_EV_ADAPTER_PROPS_CHANGED, sizeof(buf), ev, -1);
+}
+
static void mgmt_local_name_changed_event(uint16_t index, uint16_t length,
const void *param, void *user_data)
{
@@ -84,7 +106,7 @@ static void mgmt_local_name_changed_event(uint16_t index, uint16_t length,
g_free(adapter->name);
adapter->name = g_strdup((const char *) rp->name);

- /* TODO Update services if needed */
+ adapter_name_changed(rp->name);
}

static void powered_changed(void)
@@ -142,28 +164,6 @@ static void scan_mode_changed(void)
g_free(ev);
}

-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];
-
- memset(buf, 0, sizeof(buf));
- ev = (void *) buf;
-
- ev->num_props = 1;
- ev->status = HAL_STATUS_SUCCESS;
- ev->props[0].type = HAL_PROP_ADAPTER_NAME;
- /* Android expects value without NULL terminator */
- ev->props[0].len = len;
- memcpy(ev->props->val, name, len);
-
- DBG("Adapter name changed to: %s", name);
-
- ipc_send(notification_io, HAL_SERVICE_ID_BLUETOOTH,
- HAL_EV_ADAPTER_PROPS_CHANGED, sizeof(buf), ev, -1);
-}
-
static void settings_changed(uint32_t settings)
{
uint32_t changed_mask;
--
1.7.10.4



2013-11-05 12:28:44

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] android/daemon: Save adapter name on complete event

Hi Andrei,

> Saving adapter name was missing from set name complete event.
> Refactor code to function and reuse it in both places where
> name is changed.
> ---
> android/adapter.c | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/android/adapter.c b/android/adapter.c
> index b6f6096..6b0dc8b 100644
> --- a/android/adapter.c
> +++ b/android/adapter.c
> @@ -88,6 +88,17 @@ static void adapter_name_changed(const uint8_t *name)
> HAL_EV_ADAPTER_PROPS_CHANGED, sizeof(buf), ev, -1);
> }
>
> +static void adapter_set_name(const uint8_t *name)
> +{
> + if (!g_strcmp0(adapter->name, (const char *) name))
> + return;
> +
> + DBG("Cnage name: %s -> %s", adapter->name, name);
> +
> + g_free(adapter->name);
> + adapter->name = g_strdup((const char *) name);
> +}
> +
> static void mgmt_local_name_changed_event(uint16_t index, uint16_t length,
> const void *param, void *user_data)
> {
> @@ -98,13 +109,7 @@ static void mgmt_local_name_changed_event(uint16_t index, uint16_t length,
> return;
> }
>
> - if (!g_strcmp0(adapter->name, (const char *) rp->name))
> - return;
> -
> - DBG("name: %s", rp->name);
> -
> - g_free(adapter->name);
> - adapter->name = g_strdup((const char *) rp->name);
> + adapter_set_name(rp->name);
>
> adapter_name_changed(rp->name);
> }
> @@ -879,6 +884,8 @@ static void set_adapter_name_complete(uint8_t status, uint16_t length,
> return;
> }
>
> + adapter_set_name(rp->name);
> +
> adapter_name_changed(rp->name);

can we feed adapter_set_name and adapter_name_change into a single function. Especially since this is not public API and will never be, please make sure consolidate these things properly. Copies from src/adapter.c are pointless since it does not have to handle any special cases like external hostnamed daemon or plugins.

With most of these function, we need to make to store the value and send out a notification. We never do anything else. So lets keep this simple. No need to spaghetti code.

Regards

Marcel


2013-11-05 10:33:31

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH] android/daemon: Save adapter name on complete event

Hi Andrei,

On Tue, Nov 05, 2013, Andrei Emeltchenko wrote:
> Saving adapter name was missing from set name complete event.
> Refactor code to function and reuse it in both places where
> name is changed.
> ---
> android/adapter.c | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)

Applied. Thanks.

Johan

2013-11-05 10:08:48

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH] android/daemon: Save adapter name on complete event

From: Andrei Emeltchenko <[email protected]>

Saving adapter name was missing from set name complete event.
Refactor code to function and reuse it in both places where
name is changed.
---
android/adapter.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/android/adapter.c b/android/adapter.c
index b6f6096..6b0dc8b 100644
--- a/android/adapter.c
+++ b/android/adapter.c
@@ -88,6 +88,17 @@ static void adapter_name_changed(const uint8_t *name)
HAL_EV_ADAPTER_PROPS_CHANGED, sizeof(buf), ev, -1);
}

+static void adapter_set_name(const uint8_t *name)
+{
+ if (!g_strcmp0(adapter->name, (const char *) name))
+ return;
+
+ DBG("Cnage name: %s -> %s", adapter->name, name);
+
+ g_free(adapter->name);
+ adapter->name = g_strdup((const char *) name);
+}
+
static void mgmt_local_name_changed_event(uint16_t index, uint16_t length,
const void *param, void *user_data)
{
@@ -98,13 +109,7 @@ static void mgmt_local_name_changed_event(uint16_t index, uint16_t length,
return;
}

- if (!g_strcmp0(adapter->name, (const char *) rp->name))
- return;
-
- DBG("name: %s", rp->name);
-
- g_free(adapter->name);
- adapter->name = g_strdup((const char *) rp->name);
+ adapter_set_name(rp->name);

adapter_name_changed(rp->name);
}
@@ -879,6 +884,8 @@ static void set_adapter_name_complete(uint8_t status, uint16_t length,
return;
}

+ adapter_set_name(rp->name);
+
adapter_name_changed(rp->name);
}

--
1.7.10.4


2013-11-05 09:05:16

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH] android/daemon: Send update name notification on mgmt evt

Hi Andrei,

On Tue, Nov 5, 2013 at 10:53 AM, Andrei Emeltchenko
<[email protected]> wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> Wehen receiving mgmt event local_name_changed we shall send notification
> to the HAL that local name is changed.
> ---
> android/adapter.c | 46 +++++++++++++++++++++++-----------------------
> 1 file changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/android/adapter.c b/android/adapter.c
> index 383cf07..b6f6096 100644
> --- a/android/adapter.c
> +++ b/android/adapter.c
> @@ -66,6 +66,28 @@ struct bt_adapter {
> static struct bt_adapter *adapter;
> static GSList *found_devices = NULL;
>
> +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];
> +
> + memset(buf, 0, sizeof(buf));
> + ev = (void *) buf;
> +
> + ev->num_props = 1;
> + ev->status = HAL_STATUS_SUCCESS;
> + ev->props[0].type = HAL_PROP_ADAPTER_NAME;
> + /* Android expects value without NULL terminator */
> + ev->props[0].len = len;
> + memcpy(ev->props->val, name, len);
> +
> + DBG("Adapter name changed to: %s", name);
> +
> + ipc_send(notification_io, HAL_SERVICE_ID_BLUETOOTH,
> + HAL_EV_ADAPTER_PROPS_CHANGED, sizeof(buf), ev, -1);
> +}
> +
> static void mgmt_local_name_changed_event(uint16_t index, uint16_t length,
> const void *param, void *user_data)
> {
> @@ -84,7 +106,7 @@ static void mgmt_local_name_changed_event(uint16_t index, uint16_t length,
> g_free(adapter->name);
> adapter->name = g_strdup((const char *) rp->name);
>
> - /* TODO Update services if needed */
> + adapter_name_changed(rp->name);
> }

This is okay, but you should call this function from
set_adapter_name_complete to make sure we are synchronized with the
upper stack.