2014-06-17 11:19:26

by Ravi kumar Veeramally

[permalink] [raw]
Subject: [PATCH_v4 0/4] Initiates L2CAP Control Channel

v4: Fixed Szymon's comments(minor ones).

v3: Ignore.

v2: Fixed Szymon's comments (provided utility to convert
wrong HAL channel type defines, minor issues).

v1: Patchset is initial implementaion for MCL connection. It can
be tested with PTS, register application in source or sink role
and run source or sink L2CAP control channel tests. Initiate
channel connection from haltest, only MCAP MCL gets connected.

SINK
-----------
hl register_application bluez-android Bluez bluez-hdp health-device-profile 1 BTHL_MDEP_ROLE_SINK 4100 BTHL_CHANNEL_TYPE_RELIABLE pulse-oximeter

hl connect_channel 1 00:1b:dc:05:b5:54 0

SOURCE
-----------
hl register_application bluez-android Bluez bluez-hdp health-device-profile 1 BTHL_MDEP_ROLE_SOURCE 4100 BTHL_CHANNEL_TYPE_RELIABLE pulse-oximeter

hl connect_channel 1 00:1b:dc:05:b5:54 0

Ravi kumar Veeramally (4):
profiles/health/hdp: Fix memory leak in SDP record preparation
anrdoid/health: Fix wrong channel type defines
android/health: Initial connect channel implementation
android/health: Notify channel connection status

android/health.c | 454 ++++++++++++++++++++++++++++++++++++++++++++-
profiles/health/hdp_util.c | 2 +
2 files changed, 453 insertions(+), 3 deletions(-)

--
1.9.1



2014-06-17 12:28:08

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH_v4 0/4] Initiates L2CAP Control Channel

Hi Ravi,

On Tuesday 17 of June 2014 14:19:26 Ravi kumar Veeramally wrote:
> v4: Fixed Szymon's comments(minor ones).
>
> v3: Ignore.
>
> v2: Fixed Szymon's comments (provided utility to convert
> wrong HAL channel type defines, minor issues).
>
> v1: Patchset is initial implementaion for MCL connection. It can
> be tested with PTS, register application in source or sink role
> and run source or sink L2CAP control channel tests. Initiate
> channel connection from haltest, only MCAP MCL gets connected.
>
> SINK
> -----------
> hl register_application bluez-android Bluez bluez-hdp health-device-profile 1 BTHL_MDEP_ROLE_SINK 4100 BTHL_CHANNEL_TYPE_RELIABLE pulse-oximeter
>
> hl connect_channel 1 00:1b:dc:05:b5:54 0
>
> SOURCE
> -----------
> hl register_application bluez-android Bluez bluez-hdp health-device-profile 1 BTHL_MDEP_ROLE_SOURCE 4100 BTHL_CHANNEL_TYPE_RELIABLE pulse-oximeter
>
> hl connect_channel 1 00:1b:dc:05:b5:54 0
>
> Ravi kumar Veeramally (4):
> profiles/health/hdp: Fix memory leak in SDP record preparation
> anrdoid/health: Fix wrong channel type defines
> android/health: Initial connect channel implementation
> android/health: Notify channel connection status
>
> android/health.c | 454 ++++++++++++++++++++++++++++++++++++++++++++-
> profiles/health/hdp_util.c | 2 +
> 2 files changed, 453 insertions(+), 3 deletions(-)
>

All patches applied, thanks.

--
Best regards,
Szymon Janc

2014-06-17 11:19:29

by Ravi kumar Veeramally

[permalink] [raw]
Subject: [PATCH_v4 3/4] android/health: Initial connect channel implementation

Fetches remote sdp record and initiates MCL connection.
---
android/health.c | 418 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 416 insertions(+), 2 deletions(-)

diff --git a/android/health.c b/android/health.c
index f0e7335..33fc0c2 100644
--- a/android/health.c
+++ b/android/health.c
@@ -36,9 +36,13 @@
#include "lib/bluetooth.h"
#include "lib/sdp.h"
#include "lib/sdp_lib.h"
+#include "lib/uuid.h"
+#include "lib/l2cap.h"
#include "src/log.h"
#include "src/shared/util.h"
#include "src/shared/queue.h"
+#include "src/uuid-helper.h"
+#include "src/sdp-client.h"

#include "hal-msg.h"
#include "ipc-common.h"
@@ -72,6 +76,28 @@ struct mdep_cfg {
uint8_t id; /* mdep id */
};

+struct health_device {
+ bdaddr_t dst;
+ uint16_t app_id;
+
+ struct mcap_mcl *mcl;
+ bool mcl_conn;
+
+ struct queue *channels; /* data channels */
+
+ uint16_t ccpsm;
+ uint16_t dcpsm;
+};
+
+struct health_channel {
+ uint8_t mdep_id;
+ uint8_t type;
+
+ struct health_device *dev;
+
+ uint16_t id; /* channel id */
+};
+
struct health_app {
char *app_name;
char *provider_name;
@@ -81,8 +107,54 @@ struct health_app {
struct queue *mdeps;

uint16_t id; /* app id */
+ struct queue *devices;
};

+static void free_health_channel(void *data)
+{
+ struct health_channel *channel = data;
+
+ if (!channel)
+ return;
+
+ free(channel);
+}
+
+static void destroy_channel(void *data)
+{
+ struct health_channel *channel = data;
+
+ if (!channel)
+ return;
+
+ /* TODO: Notify channel connection status DESTROYED */
+ queue_remove(channel->dev->channels, channel);
+ free_health_channel(channel);
+}
+
+static void unref_mcl(struct health_device *dev)
+{
+ if (!dev || !dev->mcl)
+ return;
+
+ mcap_close_mcl(dev->mcl, FALSE);
+ mcap_mcl_unref(dev->mcl);
+ dev->mcl = NULL;
+ dev->mcl_conn = false;
+}
+
+static void free_health_device(void *data)
+{
+ struct health_device *dev = data;
+
+ if (!dev)
+ return;
+
+ unref_mcl(dev);
+ queue_destroy(dev->channels, free_health_channel);
+ free(dev);
+}
+
static void free_mdep_cfg(void *data)
{
struct mdep_cfg *cfg = data;
@@ -106,6 +178,7 @@ static void free_health_app(void *data)
free(app->service_name);
free(app->service_descr);
queue_destroy(app->mdeps, free_mdep_cfg);
+ queue_destroy(app->devices, free_health_device);
free(app);
}

@@ -130,6 +203,14 @@ static bool mdep_by_mdep_role(const void *data, const void *user_data)
return mdep->role == role;
}

+static bool mdep_by_mdep_id(const void *data, const void *user_data)
+{
+ const struct mdep_cfg *mdep = data;
+ uint16_t mdep_id = PTR_TO_INT(user_data);
+
+ return mdep->id == mdep_id;
+}
+
static bool app_by_app_id(const void *data, const void *user_data)
{
const struct health_app *app = data;
@@ -760,12 +841,345 @@ static void bt_health_unregister_app(const void *buf, uint16_t len)
HAL_OP_HEALTH_UNREG_APP, HAL_STATUS_SUCCESS);
}

+static int get_prot_desc_entry(sdp_data_t *entry, int type, guint16 *val)
+{
+ sdp_data_t *iter;
+ int proto;
+
+ if (!entry || !SDP_IS_SEQ(entry->dtd))
+ return -1;
+
+ iter = entry->val.dataseq;
+ if (!(iter->dtd & SDP_UUID_UNSPEC))
+ return -1;
+
+ proto = sdp_uuid_to_proto(&iter->val.uuid);
+ if (proto != type)
+ return -1;
+
+ if (!val)
+ return 0;
+
+ iter = iter->next;
+ if (iter->dtd != SDP_UINT16)
+ return -1;
+
+ *val = iter->val.uint16;
+
+ return 0;
+}
+
+static int get_prot_desc_list(const sdp_record_t *rec, uint16_t *psm,
+ uint16_t *version)
+{
+ sdp_data_t *pdl, *p0, *p1;
+
+ if (!psm && !version)
+ return -1;
+
+ pdl = sdp_data_get(rec, SDP_ATTR_PROTO_DESC_LIST);
+ if (!pdl || !SDP_IS_SEQ(pdl->dtd))
+ return -1;
+
+ p0 = pdl->val.dataseq;
+ if (get_prot_desc_entry(p0, L2CAP_UUID, psm) < 0)
+ return -1;
+
+ p1 = p0->next;
+ if (get_prot_desc_entry(p1, MCAP_CTRL_UUID, version) < 0)
+ return -1;
+
+ return 0;
+}
+
+static int get_ccpsm(sdp_list_t *recs, uint16_t *ccpsm)
+{
+ sdp_list_t *l;
+
+ for (l = recs; l; l = l->next) {
+ sdp_record_t *rec = l->data;
+
+ if (!get_prot_desc_list(rec, ccpsm, NULL))
+ return 0;
+ }
+
+ return -1;
+}
+
+static int get_add_prot_desc_list(const sdp_record_t *rec, uint16_t *psm)
+{
+ sdp_data_t *pdl, *p0, *p1;
+
+ if (!psm)
+ return -1;
+
+ pdl = sdp_data_get(rec, SDP_ATTR_ADD_PROTO_DESC_LIST);
+ if (!pdl || pdl->dtd != SDP_SEQ8)
+ return -1;
+
+ pdl = pdl->val.dataseq;
+ if (pdl->dtd != SDP_SEQ8)
+ return -1;
+
+ p0 = pdl->val.dataseq;
+
+ if (get_prot_desc_entry(p0, L2CAP_UUID, psm) < 0)
+ return -1;
+
+ p1 = p0->next;
+ if (get_prot_desc_entry(p1, MCAP_DATA_UUID, NULL) < 0)
+ return -1;
+
+ return 0;
+}
+
+static int get_dcpsm(sdp_list_t *recs, uint16_t *dcpsm)
+{
+ sdp_list_t *l;
+
+ for (l = recs; l; l = l->next) {
+ sdp_record_t *rec = l->data;
+
+ if (!get_add_prot_desc_list(rec, dcpsm))
+ return 0;
+ }
+
+ return -1;
+}
+
+static void mcap_mdl_connected_cb(struct mcap_mdl *mdl, void *data)
+{
+ DBG("Not Implemeneted");
+}
+
+static void mcap_mdl_closed_cb(struct mcap_mdl *mdl, void *data)
+{
+ DBG("Not Implemeneted");
+}
+
+static void mcap_mdl_deleted_cb(struct mcap_mdl *mdl, void *data)
+{
+ DBG("Not Implemeneted");
+}
+
+static void mcap_mdl_aborted_cb(struct mcap_mdl *mdl, void *data)
+{
+ DBG("Not Implemeneted");
+}
+
+static void mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
+ uint16_t mdlid, uint8_t *conf, void *data)
+{
+ DBG("Not Implemeneted");
+}
+
+static void mcap_mdl_reconn_req_cb(struct mcap_mdl *mdl, void *data)
+{
+ DBG("Not Implemeneted");
+}
+
+static void create_mcl_cb(struct mcap_mcl *mcl, GError *err, gpointer data)
+{
+ struct health_channel *channel = data;
+ gboolean ret;
+ GError *gerr = NULL;
+
+ DBG("");
+
+ if (err) {
+ error("error creating MCL : %s", err->message);
+ goto fail;
+ }
+
+ if (!channel->dev->mcl)
+ channel->dev->mcl = mcap_mcl_ref(mcl);
+
+ channel->dev->mcl_conn = true;
+ DBG("MCL connected");
+
+ ret = mcap_mcl_set_cb(channel->dev->mcl, channel, &gerr,
+ MCAP_MDL_CB_CONNECTED, mcap_mdl_connected_cb,
+ MCAP_MDL_CB_CLOSED, mcap_mdl_closed_cb,
+ MCAP_MDL_CB_DELETED, mcap_mdl_deleted_cb,
+ MCAP_MDL_CB_ABORTED, mcap_mdl_aborted_cb,
+ MCAP_MDL_CB_REMOTE_CONN_REQ, mcap_mdl_conn_req_cb,
+ MCAP_MDL_CB_REMOTE_RECONN_REQ, mcap_mdl_reconn_req_cb,
+ MCAP_MDL_CB_INVALID);
+ if (!ret) {
+ error("error setting mdl callbacks on mcl");
+
+ if (gerr)
+ g_error_free(gerr);
+
+ goto fail;
+ }
+
+ /* TODO : create mdl */
+ return;
+
+fail:
+ destroy_channel(channel);
+}
+
+static void search_cb(sdp_list_t *recs, int err, gpointer data)
+{
+ struct health_channel *channel = data;
+ GError *gerr = NULL;
+
+ DBG("");
+
+ if (err < 0 || !recs) {
+ error("Error getting remote SDP records");
+ goto fail;
+ }
+
+ if (get_ccpsm(recs, &channel->dev->ccpsm) < 0) {
+ error("Can't get remote PSM for control channel");
+ goto fail;
+ }
+
+ if (get_dcpsm(recs, &channel->dev->dcpsm) < 0) {
+ error("Can't get remote PSM for data channel");
+ goto fail;
+ }
+
+ if (!mcap_create_mcl(mcap, &channel->dev->dst, channel->dev->ccpsm,
+ create_mcl_cb, channel, NULL, &gerr)) {
+ error("error creating mcl %s", gerr->message);
+
+ if (gerr)
+ g_error_free(gerr);
+
+ goto fail;
+ }
+
+ /* TODO: send channel state CONNECTING */
+ return;
+
+fail:
+ /* TODO: send channel state DESTROYED*/
+
+ queue_remove(channel->dev->channels, channel);
+ free_health_channel(channel);
+}
+
+static int connect_mcl(struct health_channel *channel)
+{
+ uuid_t uuid;
+
+ DBG("");
+
+ bt_string2uuid(&uuid, HDP_UUID);
+
+ return bt_search_service(&adapter_addr, &channel->dev->dst, &uuid,
+ search_cb, channel, NULL, 0);
+}
+
+static struct health_device *create_device(uint16_t app_id,
+ const uint8_t addr[])
+{
+ struct health_device *dev;
+
+ dev = new0(struct health_device, 1);
+ if (!dev)
+ return NULL;
+
+ android2bdaddr(&addr, &dev->dst);
+ dev->app_id = app_id;
+
+ return dev;
+}
+
+static struct health_channel *create_channel(uint16_t app_id,
+ uint8_t mdep_index)
+{
+ struct health_app *app;
+ struct mdep_cfg *mdep;
+ struct health_channel *channel;
+ uint8_t index;
+ static unsigned int channel_id = 1;
+
+ app = queue_find(apps, app_by_app_id, INT_TO_PTR(app_id));
+ if (!app)
+ return NULL;
+
+ index = mdep_index + 1;
+ mdep = queue_find(app->mdeps, mdep_by_mdep_id, INT_TO_PTR(index));
+ if (!mdep)
+ return NULL;
+
+ channel = new0(struct health_channel, 1);
+ if (!channel)
+ return NULL;
+
+ channel->mdep_id = mdep_index;
+ channel->type = mdep->channel_type;
+ channel->id = channel_id++;
+
+ return channel;
+}
+
static void bt_health_connect_channel(const void *buf, uint16_t len)
{
- DBG("Not implemented");
+ const struct hal_cmd_health_connect_channel *cmd = buf;
+ struct hal_rsp_health_connect_channel rsp;
+ struct health_app *app;
+ struct health_device *dev = NULL;
+ struct health_channel *channel = NULL;
+
+ DBG("");
+
+ app = queue_find(apps, app_by_app_id, INT_TO_PTR(cmd->app_id));
+ if (!app)
+ goto fail;
+
+ dev = create_device(cmd->app_id, cmd->bdaddr);
+ if (!dev)
+ goto fail;

+ channel = create_channel(cmd->app_id, cmd->mdep_index);
+ if (!channel)
+ goto fail;
+
+ channel->dev = dev;
+
+ if (!app->devices) {
+ app->devices = queue_new();
+ if (!app->devices)
+ goto fail;
+ }
+
+ if (!queue_push_tail(app->devices, dev))
+ goto fail;
+
+ if (!dev->channels) {
+ dev->channels = queue_new();
+ if (!dev->channels)
+ goto fail;
+ }
+
+ if (!queue_push_tail(dev->channels, channel)) {
+ queue_remove(app->devices, dev);
+ goto fail;
+ }
+
+ if (connect_mcl(channel) < 0) {
+ error("error retrieving HDP SDP record");
+ queue_remove(app->devices, dev);
+ goto fail;
+ }
+
+ rsp.channel_id = channel->id;
+ ipc_send_rsp_full(hal_ipc, HAL_SERVICE_ID_HEALTH,
+ HAL_OP_HEALTH_CONNECT_CHANNEL,
+ sizeof(rsp), &rsp, -1);
+ return;
+
+fail:
+ free_health_channel(channel);
+ free_health_device(dev);
ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_HEALTH,
- HAL_OP_HEALTH_CONNECT_CHANNEL, HAL_STATUS_UNSUPPORTED);
+ HAL_OP_HEALTH_CONNECT_CHANNEL, HAL_STATUS_FAILED);
}

static void bt_health_destroy_channel(const void *buf, uint16_t len)
--
1.9.1


2014-06-17 11:19:28

by Ravi kumar Veeramally

[permalink] [raw]
Subject: [PATCH_v4 2/4] anrdoid/health: Fix wrong channel type defines

Enums in bt_hl.h are defined in this order BTHL_CHANNEL_TYPE_RELIABLE,
BTHL_CHANNEL_TYPE_STREAMING, BTHL_CHANNEL_TYPE_ANY. But HDP_SPEC_V11(3.4)
has no-preference(any)-0, reliable-1, stream-2, reserved(0x03-0xff).
Providing utility to solve this.
---
android/health.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/android/health.c b/android/health.c
index 7e2c5d6..f0e7335 100644
--- a/android/health.c
+++ b/android/health.c
@@ -52,6 +52,10 @@
#define HDP_VERSION 0x0101
#define DATA_EXCHANGE_SPEC_11073 0x01

+#define CHANNEL_TYPE_ANY 0x00
+#define CHANNEL_TYPE_RELIABLE 0x01
+#define CHANNEL_TYPE_STREAM 0x02
+
static bdaddr_t adapter_addr;
static struct ipc *hal_ipc = NULL;
static struct queue *apps = NULL;
@@ -639,6 +643,18 @@ fail:
HAL_STATUS_FAILED);
}

+static uint8_t android2channel_type(uint8_t type)
+{
+ switch (type) {
+ case HAL_HEALTH_CHANNEL_TYPE_RELIABLE:
+ return CHANNEL_TYPE_RELIABLE;
+ case HAL_HEALTH_CHANNEL_TYPE_STREAMING:
+ return CHANNEL_TYPE_STREAM;
+ default:
+ return CHANNEL_TYPE_ANY;
+ }
+}
+
static void bt_health_mdep_cfg_data(const void *buf, uint16_t len)
{
const struct hal_cmd_health_mdep *cmd = buf;
@@ -662,7 +678,7 @@ static void bt_health_mdep_cfg_data(const void *buf, uint16_t len)

mdep->role = cmd->role;
mdep->data_type = cmd->data_type;
- mdep->channel_type = cmd->channel_type;
+ mdep->channel_type = android2channel_type(cmd->channel_type);
mdep->id = queue_length(app->mdeps) + 1;

if (cmd->descr_len > 0) {
--
1.9.1


2014-06-17 11:19:30

by Ravi kumar Veeramally

[permalink] [raw]
Subject: [PATCH_v4 4/4] android/health: Notify channel connection status

---
android/health.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/android/health.c b/android/health.c
index 33fc0c2..585d8a4 100644
--- a/android/health.c
+++ b/android/health.c
@@ -195,6 +195,24 @@ static void send_app_reg_notify(struct health_app *app, uint8_t state)
HAL_EV_HEALTH_APP_REG_STATE, sizeof(ev), &ev);
}

+static void send_channel_state_notify(struct health_channel *channel,
+ uint8_t state, int fd)
+{
+ struct hal_ev_health_channel_state ev;
+
+ DBG("");
+
+ bdaddr2android(&channel->dev->dst, ev.bdaddr);
+ ev.app_id = channel->dev->app_id;
+ ev.mdep_index = channel->mdep_id;
+ ev.channel_id = channel->id;
+ ev.channel_state = state;
+
+ ipc_send_notif_with_fd(hal_ipc, HAL_SERVICE_ID_HEALTH,
+ HAL_EV_HEALTH_CHANNEL_STATE,
+ sizeof(ev), &ev, fd);
+}
+
static bool mdep_by_mdep_role(const void *data, const void *user_data)
{
const struct mdep_cfg *mdep = data;
@@ -1053,11 +1071,11 @@ static void search_cb(sdp_list_t *recs, int err, gpointer data)
goto fail;
}

- /* TODO: send channel state CONNECTING */
+ send_channel_state_notify(channel, HAL_HEALTH_CHANNEL_CONNECTING, -1);
return;

fail:
- /* TODO: send channel state DESTROYED*/
+ send_channel_state_notify(channel, HAL_HEALTH_CHANNEL_DESTROYED, -1);

queue_remove(channel->dev->channels, channel);
free_health_channel(channel);
--
1.9.1


2014-06-17 11:19:27

by Ravi kumar Veeramally

[permalink] [raw]
Subject: [PATCH_v4 1/4] profiles/health/hdp: Fix memory leak in SDP record preparation

Sdp record preparation part is copied to android/health.c from
profiles/health/hdp_utils.c. Memory leak is noticed while testing. Memory
summay is from android daemon, but code snippet is same. It is already fixed
in android/health.c while submitting android related patches.

==12515== 286 (16 direct, 270 indirect) bytes in 1 blocks are definitely lost in loss record 158 of 165
==12515== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==12515== by 0x44AC45: sdp_list_append (sdp.c:1743)
==12515== by 0x4398F9: register_features (health.c:381)
==12515== by 0x4091CC: queue_foreach (queue.c:219)
==12515== by 0x43A31F: bt_health_mdep_cfg_data (health.c:398)
==12515== by 0x418B50: cmd_watch_cb (ipc.c:144)
==12515== by 0x4E7FCE4: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4000.0)
==12515== by 0x4E80047: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4000.0)
==12515== by 0x4E80309: g_main_loop_run (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4000.0)
==12515== by 0x4044F2: main (main.c:538)
---
profiles/health/hdp_util.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/profiles/health/hdp_util.c b/profiles/health/hdp_util.c
index 58770b5..47c464e 100644
--- a/profiles/health/hdp_util.c
+++ b/profiles/health/hdp_util.c
@@ -652,6 +652,8 @@ static gboolean register_service_sup_features(GSList *app_list,
return FALSE;
}

+ sdp_list_free(sup_features, free_hdp_list);
+
return TRUE;
}

--
1.9.1