2014-11-05 11:22:04

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH 0/7] android/handsfree-client: First set of HFP HF

In this set service record, slc creation and incoming/outgoing
connection support has been added.

This code has been tested on UPF49.

Lukasz Rymanowski (7):
android/handsfree-client: Typo fix in define name
android/handsfree-client: Add service record
android/handsfree-client: Add devices queue
android/handsfree-client: Add incoming connection handling
android/handsfree-client: Add SLC creation procedure
android/handsfree-client: Add support for outgoing connection
android/handsfree-client: Handle disconnect command

android/hal-msg.h | 2 +-
android/handsfree-client.c | 997 ++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 992 insertions(+), 7 deletions(-)

--
1.8.4



2014-11-08 23:05:52

by Lukasz Rymanowski

[permalink] [raw]
Subject: Re: [PATCH 4/7] android/handsfree-client: Add incoming connection handling

Hi Szymon,

On Sat, Nov 8, 2014 at 9:59 PM, Lukasz Rymanowski
<[email protected]> wrote:
> Hi Szymon,
>
> On Fri, Nov 7, 2014 at 8:00 PM, Szymon Janc <[email protected]> wrote:
>> Hi Łukasz,
>>
>> On Wednesday 05 of November 2014 12:22:08 Lukasz Rymanowski wrote:
>>> ---
>>> android/handsfree-client.c | 174
>>> ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 173
>>> insertions(+), 1 deletion(-)
>>>
>>> diff --git a/android/handsfree-client.c b/android/handsfree-client.c
>>> index aa3912b..6be9851 100644
>>> --- a/android/handsfree-client.c
>>> +++ b/android/handsfree-client.c
>>> @@ -36,6 +36,7 @@
>>> #include "lib/sdp_lib.h"
>>> #include "src/sdp-client.h"
>>> #include "src/shared/queue.h"
>>> +#include "btio/btio.h"
>>> #include "ipc.h"
>>> #include "ipc-common.h"
>>> #include "src/log.h"
>>> @@ -64,6 +65,11 @@
>>> HFP_HF_FEAT_RVC | HFP_HF_FEAT_ECS |\
>>> HFP_HF_FEAT_ECC)
>>>
>>> +struct device {
>>> + bdaddr_t bdaddr;
>>> + uint8_t state;
>>> +};
>>> +
>>> static bdaddr_t adapter_addr;
>>>
>>> static struct ipc *hal_ipc = NULL;
>>> @@ -71,6 +77,55 @@ static struct ipc *hal_ipc = NULL;
>>> static uint32_t hfp_hf_features = 0;
>>> static uint32_t hfp_hf_record_id = 0;
>>> static struct queue *devices = NULL;
>>> +static GIOChannel *hfp_hf_server = NULL;
>>> +
>>> +static bool match_by_bdaddr(const void *data, const void *user_data)
>>> +{
>>> + const bdaddr_t *addr1 = data;
>>> + const bdaddr_t *addr2 = user_data;
>>> +
>>> + return !bacmp(addr1, addr2);
>>> +}
>>> +
>>> +static struct device *find_device(const bdaddr_t *addr)
>>> +{
>>> + /* For now we support only one device */
>>
>> I'm not sure how this comment fits here.
>>
>>> + return queue_find(devices, match_by_bdaddr, addr);
>>> +}
>>> +
>>> +static struct device *device_create(const bdaddr_t *bdaddr)
>>> +{
>>> + struct device *dev;
>>> +
>>> + dev = g_malloc0(sizeof(struct device));
>>
>> This should be new0();
>>
>>> + if (!dev)
>>> + return NULL;
>>> +
>>> + if (!queue_push_tail(devices, dev)) {
>>> + error("hf-client: Could not push dev on the list");
>>> + free(dev);
>>> + return NULL;
>>> + }
>>> +
>>> + bacpy(&dev->bdaddr, bdaddr);
>>
>> Please initialize state here explicitly. Will make code easier to follow.
>>
>>> +
>>> + return dev;
>>> +}
>>> +
>>> +static struct device *get_device(const bdaddr_t *addr)
>>> +{
>>> + struct device *dev;
>>> +
>>> + dev = find_device(addr);
>>> + if (dev)
>>> + return dev;
>>> +
>>> + /* We do support only one device as for now */
>>> + if (queue_isempty(devices))
>>> + return device_create(addr);
>>> +
>>> + return NULL;
>>> +}
>>>
>>> static void handle_connect(const void *buf, uint16_t len)
>>> {
>>> @@ -192,6 +247,102 @@ static void handle_get_last_vc_tag_num(const void
>>> *buf, uint16_t len) HAL_STATUS_UNSUPPORTED);
>>> }
>>>
>>> +static void device_set_state(struct device *dev, uint8_t state)
>>> +{
>>> + struct hal_ev_hf_client_conn_state ev;
>>> + char address[18];
>>> +
>>> + memset(&ev, 0, sizeof(ev));
>>> +
>>> + if (dev->state == state)
>>> + return;
>>
>> Minor: this check could be done before clearing ev.
>>
>>> +
>>> + dev->state = state;
>>> +
>>> + ba2str(&dev->bdaddr, address);
>>> + DBG("device %s state %u", address, state);
>>> +
>>> + bdaddr2android(&dev->bdaddr, ev.bdaddr);
>>> + ev.state = state;
>>> +
>>> + ipc_send_notif(hal_ipc, HAL_SERVICE_ID_HANDSFREE_CLIENT,
>>> + HAL_EV_HF_CLIENT_CONN_STATE, sizeof(ev), &ev);
>>> +}
>>> +
>>> +static void (void *data)
>>
>> Please make this parameter proper type.
>>
>>> +{
>>> + struct device *dev = data;
>>> +
>>
>> I'd set state to disconnect here to be sure that framework always get proper
>> notification of disconnected device.
>>
>>> + queue_remove(devices, dev);
>>> + free(dev);
>>> +}
>>> +
>>> +static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
>>> +{
>>> + struct device *dev = user_data;
>>> +
>>> + DBG("");
>>> +
>>> + if (err) {
>>> + error("hf-client: connect failed (%s)", err->message);
>>> + goto failed;
>>> + }
>>> +
>>> + g_io_channel_set_close_on_unref(chan, FALSE);
>>> +
>>> + /* TODO Create SLC here. For now do nothing, link will be dropped */
>>> +
>>> + return;
>>> +
>>> +failed:
>>> + g_io_channel_shutdown(chan, TRUE, NULL);
>>> + device_destroy(dev);
>>> +}
>>> +
>>> +static void confirm_cb(GIOChannel *chan, gpointer data)
>>> +{
>>> + struct device *dev;
>>> + char address[18];
>>> + bdaddr_t bdaddr;
>>> + GError *err = NULL;
>>> +
>>> + bt_io_get(chan, &err,
>>> + BT_IO_OPT_DEST, address,
>>> + BT_IO_OPT_DEST_BDADDR, &bdaddr,
>>> + BT_IO_OPT_INVALID);
>>> + if (err) {
>>> + error("hf-client: confirm failed (%s)", err->message);
>>> + g_error_free(err);
>>> + goto drop;
>>> + }
>>> +
>>> + DBG("Incoming connection from %s", address);
>>> +
>>> + dev = get_device(&bdaddr);
>>> + if (!dev) {
>>> + error("hf-client: There is other AG connected");
>>> + goto drop;
>>> + }
>>> +
>>> + if (dev->state != HAL_HF_CLIENT_CONN_STATE_DISCONNECTED) {
>>> + error("hf-client: Connections is up or ongoing ?");
>>> + return;
>>
>> Shouldn't we drop connection here?
>>
> No, we assume that there is ongoing connection - so let it be.

Actually I think I should not return here, but let it go further. This
is because, it can only happen when we initiate outgoing connection
but in the meantime we got incoming connection request.
But, what I don't know here what we get from bt_io as a response for
outgoing conection.
Therefore for the time being I will drop connection here for the
moment and put TODO

>>> + }
>>> +
>>> + device_set_state(dev, HAL_HF_CLIENT_CONN_STATE_CONNECTING);
>>> +
>>> + if (!bt_io_accept(chan, connect_cb, dev, NULL, NULL)) {
>>> + error("hf-client: failed to accept connection");
>>
>> set_state(DISCONNECTED) is missing here. As mentioned above, maybe this should
>> be handled in device_destroy() as this seems to be missing in more places?
>>
> Right, will have it in destroy_device
>
>>> + device_destroy(dev);
>>> + goto drop;
>>> + }
>>> +
>>> + return;
>>> +
>>> +drop:
>>> + g_io_channel_shutdown(chan, TRUE, NULL);
>>> +}
>>> +
>>> static const struct ipc_handler cmd_handlers[] = {
>>> /* HAL_OP_HF_CLIENT_CONNECT */
>>> { handle_connect, false,
>>> @@ -304,6 +455,21 @@ static sdp_record_t *hfp_hf_record(void)
>>> static bool enable_hf_client(void)
>>> {
>>> sdp_record_t *rec;
>>> + GError *err = NULL;
>>> +
>>> + if (hfp_hf_server)
>>> + return false;
>>
>> Can this happen?
>
> Well , we don't need to be that strict here.
>>
>>> +
>>> + hfp_hf_server = bt_io_listen(NULL, confirm_cb, NULL, NULL, &err,
>>> + BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
>>> + BT_IO_OPT_CHANNEL, HFP_HF_CHANNEL,
>>> + BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
>>> + BT_IO_OPT_INVALID);
>>> + if (!hfp_hf_server) {
>>> + error("Failed to listen on Handsfree rfcomm: %s", err->message);
>>
>> prefix this with hf-client: to avoid confusion with AG.
>>
>>> + g_error_free(err);
>>> + return false;
>>> + }
>>>
>>> hfp_hf_features = HFP_HF_FEATURES;
>>>
>>> @@ -326,6 +492,12 @@ static bool enable_hf_client(void)
>>>
>>> static void cleanup_hfp_hf(void)
>>> {
>>> + if (hfp_hf_server) {
>>> + g_io_channel_shutdown(hfp_hf_server, TRUE, NULL);
>>> + g_io_channel_unref(hfp_hf_server);
>>> + hfp_hf_server = NULL;
>>> + }
>>> +
>>> if (hfp_hf_record_id > 0) {
>>> bt_adapter_remove_record(hfp_hf_record_id);
>>> hfp_hf_record_id = 0;
>>> @@ -366,7 +538,7 @@ void bt_hf_client_unregister(void)
>>>
>>> cleanup_hfp_hf();
>>>
>>> - queue_destroy(devices, free);
>>> + queue_destroy(devices, device_destroy);
>>> devices = NULL;
>>>
>>> ipc_unregister(hal_ipc, HAL_SERVICE_ID_HANDSFREE);
>>
>> --
>> BR
>> Szymon Janc
>
\Łukasz
>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-11-08 20:59:09

by Lukasz Rymanowski

[permalink] [raw]
Subject: Re: [PATCH 4/7] android/handsfree-client: Add incoming connection handling

Hi Szymon,

On Fri, Nov 7, 2014 at 8:00 PM, Szymon Janc <[email protected]> wrote:
> Hi Łukasz,
>
> On Wednesday 05 of November 2014 12:22:08 Lukasz Rymanowski wrote:
>> ---
>> android/handsfree-client.c | 174
>> ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 173
>> insertions(+), 1 deletion(-)
>>
>> diff --git a/android/handsfree-client.c b/android/handsfree-client.c
>> index aa3912b..6be9851 100644
>> --- a/android/handsfree-client.c
>> +++ b/android/handsfree-client.c
>> @@ -36,6 +36,7 @@
>> #include "lib/sdp_lib.h"
>> #include "src/sdp-client.h"
>> #include "src/shared/queue.h"
>> +#include "btio/btio.h"
>> #include "ipc.h"
>> #include "ipc-common.h"
>> #include "src/log.h"
>> @@ -64,6 +65,11 @@
>> HFP_HF_FEAT_RVC | HFP_HF_FEAT_ECS |\
>> HFP_HF_FEAT_ECC)
>>
>> +struct device {
>> + bdaddr_t bdaddr;
>> + uint8_t state;
>> +};
>> +
>> static bdaddr_t adapter_addr;
>>
>> static struct ipc *hal_ipc = NULL;
>> @@ -71,6 +77,55 @@ static struct ipc *hal_ipc = NULL;
>> static uint32_t hfp_hf_features = 0;
>> static uint32_t hfp_hf_record_id = 0;
>> static struct queue *devices = NULL;
>> +static GIOChannel *hfp_hf_server = NULL;
>> +
>> +static bool match_by_bdaddr(const void *data, const void *user_data)
>> +{
>> + const bdaddr_t *addr1 = data;
>> + const bdaddr_t *addr2 = user_data;
>> +
>> + return !bacmp(addr1, addr2);
>> +}
>> +
>> +static struct device *find_device(const bdaddr_t *addr)
>> +{
>> + /* For now we support only one device */
>
> I'm not sure how this comment fits here.
>
>> + return queue_find(devices, match_by_bdaddr, addr);
>> +}
>> +
>> +static struct device *device_create(const bdaddr_t *bdaddr)
>> +{
>> + struct device *dev;
>> +
>> + dev = g_malloc0(sizeof(struct device));
>
> This should be new0();
>
>> + if (!dev)
>> + return NULL;
>> +
>> + if (!queue_push_tail(devices, dev)) {
>> + error("hf-client: Could not push dev on the list");
>> + free(dev);
>> + return NULL;
>> + }
>> +
>> + bacpy(&dev->bdaddr, bdaddr);
>
> Please initialize state here explicitly. Will make code easier to follow.
>
>> +
>> + return dev;
>> +}
>> +
>> +static struct device *get_device(const bdaddr_t *addr)
>> +{
>> + struct device *dev;
>> +
>> + dev = find_device(addr);
>> + if (dev)
>> + return dev;
>> +
>> + /* We do support only one device as for now */
>> + if (queue_isempty(devices))
>> + return device_create(addr);
>> +
>> + return NULL;
>> +}
>>
>> static void handle_connect(const void *buf, uint16_t len)
>> {
>> @@ -192,6 +247,102 @@ static void handle_get_last_vc_tag_num(const void
>> *buf, uint16_t len) HAL_STATUS_UNSUPPORTED);
>> }
>>
>> +static void device_set_state(struct device *dev, uint8_t state)
>> +{
>> + struct hal_ev_hf_client_conn_state ev;
>> + char address[18];
>> +
>> + memset(&ev, 0, sizeof(ev));
>> +
>> + if (dev->state == state)
>> + return;
>
> Minor: this check could be done before clearing ev.
>
>> +
>> + dev->state = state;
>> +
>> + ba2str(&dev->bdaddr, address);
>> + DBG("device %s state %u", address, state);
>> +
>> + bdaddr2android(&dev->bdaddr, ev.bdaddr);
>> + ev.state = state;
>> +
>> + ipc_send_notif(hal_ipc, HAL_SERVICE_ID_HANDSFREE_CLIENT,
>> + HAL_EV_HF_CLIENT_CONN_STATE, sizeof(ev), &ev);
>> +}
>> +
>> +static void (void *data)
>
> Please make this parameter proper type.
>
>> +{
>> + struct device *dev = data;
>> +
>
> I'd set state to disconnect here to be sure that framework always get proper
> notification of disconnected device.
>
>> + queue_remove(devices, dev);
>> + free(dev);
>> +}
>> +
>> +static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
>> +{
>> + struct device *dev = user_data;
>> +
>> + DBG("");
>> +
>> + if (err) {
>> + error("hf-client: connect failed (%s)", err->message);
>> + goto failed;
>> + }
>> +
>> + g_io_channel_set_close_on_unref(chan, FALSE);
>> +
>> + /* TODO Create SLC here. For now do nothing, link will be dropped */
>> +
>> + return;
>> +
>> +failed:
>> + g_io_channel_shutdown(chan, TRUE, NULL);
>> + device_destroy(dev);
>> +}
>> +
>> +static void confirm_cb(GIOChannel *chan, gpointer data)
>> +{
>> + struct device *dev;
>> + char address[18];
>> + bdaddr_t bdaddr;
>> + GError *err = NULL;
>> +
>> + bt_io_get(chan, &err,
>> + BT_IO_OPT_DEST, address,
>> + BT_IO_OPT_DEST_BDADDR, &bdaddr,
>> + BT_IO_OPT_INVALID);
>> + if (err) {
>> + error("hf-client: confirm failed (%s)", err->message);
>> + g_error_free(err);
>> + goto drop;
>> + }
>> +
>> + DBG("Incoming connection from %s", address);
>> +
>> + dev = get_device(&bdaddr);
>> + if (!dev) {
>> + error("hf-client: There is other AG connected");
>> + goto drop;
>> + }
>> +
>> + if (dev->state != HAL_HF_CLIENT_CONN_STATE_DISCONNECTED) {
>> + error("hf-client: Connections is up or ongoing ?");
>> + return;
>
> Shouldn't we drop connection here?
>
No, we assume that there is ongoing connection - so let it be.
>> + }
>> +
>> + device_set_state(dev, HAL_HF_CLIENT_CONN_STATE_CONNECTING);
>> +
>> + if (!bt_io_accept(chan, connect_cb, dev, NULL, NULL)) {
>> + error("hf-client: failed to accept connection");
>
> set_state(DISCONNECTED) is missing here. As mentioned above, maybe this should
> be handled in device_destroy() as this seems to be missing in more places?
>
Right, will have it in destroy_device

>> + device_destroy(dev);
>> + goto drop;
>> + }
>> +
>> + return;
>> +
>> +drop:
>> + g_io_channel_shutdown(chan, TRUE, NULL);
>> +}
>> +
>> static const struct ipc_handler cmd_handlers[] = {
>> /* HAL_OP_HF_CLIENT_CONNECT */
>> { handle_connect, false,
>> @@ -304,6 +455,21 @@ static sdp_record_t *hfp_hf_record(void)
>> static bool enable_hf_client(void)
>> {
>> sdp_record_t *rec;
>> + GError *err = NULL;
>> +
>> + if (hfp_hf_server)
>> + return false;
>
> Can this happen?

Well , we don't need to be that strict here.
>
>> +
>> + hfp_hf_server = bt_io_listen(NULL, confirm_cb, NULL, NULL, &err,
>> + BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
>> + BT_IO_OPT_CHANNEL, HFP_HF_CHANNEL,
>> + BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
>> + BT_IO_OPT_INVALID);
>> + if (!hfp_hf_server) {
>> + error("Failed to listen on Handsfree rfcomm: %s", err->message);
>
> prefix this with hf-client: to avoid confusion with AG.
>
>> + g_error_free(err);
>> + return false;
>> + }
>>
>> hfp_hf_features = HFP_HF_FEATURES;
>>
>> @@ -326,6 +492,12 @@ static bool enable_hf_client(void)
>>
>> static void cleanup_hfp_hf(void)
>> {
>> + if (hfp_hf_server) {
>> + g_io_channel_shutdown(hfp_hf_server, TRUE, NULL);
>> + g_io_channel_unref(hfp_hf_server);
>> + hfp_hf_server = NULL;
>> + }
>> +
>> if (hfp_hf_record_id > 0) {
>> bt_adapter_remove_record(hfp_hf_record_id);
>> hfp_hf_record_id = 0;
>> @@ -366,7 +538,7 @@ void bt_hf_client_unregister(void)
>>
>> cleanup_hfp_hf();
>>
>> - queue_destroy(devices, free);
>> + queue_destroy(devices, device_destroy);
>> devices = NULL;
>>
>> ipc_unregister(hal_ipc, HAL_SERVICE_ID_HANDSFREE);
>
> --
> BR
> Szymon Janc

\Łukasz

> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-11-07 19:56:14

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH 7/7] android/handsfree-client: Handle disconnect command

Hi Łukasz,

On Wednesday 05 of November 2014 12:22:11 Lukasz Rymanowski wrote:
> ---
> android/handsfree-client.c | 27 +++++++++++++++++++++++++--
> 1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/android/handsfree-client.c b/android/handsfree-client.c
> index d90cef0..751a9f1 100644
> --- a/android/handsfree-client.c
> +++ b/android/handsfree-client.c
> @@ -195,9 +195,32 @@ static struct device *get_device(const bdaddr_t *addr)
>
> static void handle_disconnect(const void *buf, uint16_t len)
> {
> - DBG("Not Implemented");
> + const struct hal_cmd_hf_client_disconnect *cmd = buf;
> + struct device *dev;
> + uint32_t status;
> + bdaddr_t bdaddr;
> + char addr[18];
> +
> + DBG("");
> +
> + android2bdaddr(&cmd->bdaddr, &bdaddr);
> +
> + ba2str(&bdaddr, addr);
> + DBG("Disconnect %s", addr);
> +
> + dev = get_device(&bdaddr);
> + if (!dev) {
> + status = HAL_STATUS_FAILED;
> + goto done;
> + }
> +
> + status = hfp_hf_disconnect(dev->hf) ? HAL_STATUS_SUCCESS :
> + HAL_STATUS_FAILED;

Shouldn't 'connecting' and 'disconnecting' state be handled here?

> +
> +done:
> +
> ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_HANDSFREE_CLIENT,
> - HAL_OP_HF_CLIENT_DISCONNECT, HAL_STATUS_UNSUPPORTED);
> + HAL_OP_HF_CLIENT_DISCONNECT, status);
> }
>
> static void handle_connect_audio(const void *buf, uint16_t len)

--
BR
Szymon Janc

2014-11-07 19:50:45

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH 6/7] android/handsfree-client: Add support for outgoing connection

Hi Łukasz,

On Wednesday 05 of November 2014 12:22:10 Lukasz Rymanowski wrote:
> ---
> android/handsfree-client.c | 123
> ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 116
> insertions(+), 7 deletions(-)
>
> diff --git a/android/handsfree-client.c b/android/handsfree-client.c
> index def057d..d90cef0 100644
> --- a/android/handsfree-client.c
> +++ b/android/handsfree-client.c
> @@ -193,13 +193,6 @@ static struct device *get_device(const bdaddr_t *addr)
> return NULL;
> }
>
> -static void handle_connect(const void *buf, uint16_t len)
> -{
> - DBG("Not Implemented");
> - ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_HANDSFREE_CLIENT,
> - HAL_OP_HF_CLIENT_CONNECT, HAL_STATUS_UNSUPPORTED);
> -}
> -
> static void handle_disconnect(const void *buf, uint16_t len)
> {
> DBG("Not Implemented");
> @@ -833,6 +826,122 @@ failed:
> device_destroy(dev);
> }
>
> +static void sdp_hfp_search_cb(sdp_list_t *recs, int err, gpointer data)
> +{
> + sdp_list_t *protos, *classes;
> + struct device *dev = data;
> + GError *gerr = NULL;
> + GIOChannel *io;
> + uuid_t uuid;
> + int channel;
> +
> + DBG("");
> +
> + if (err < 0) {
> + error("hf-client: unable to get SDP record: %s",
> + strerror(-err));
> + goto failed;
> + }
> +
> + if (!recs || !recs->data) {
> + info("hf-client: no HFP SDP records found");
> + goto failed;
> + }
> +
> + if (sdp_get_service_classes(recs->data, &classes) < 0 || !classes) {
> + error("hf-client: unable to get service classes from record");
> + goto failed;
> + }
> +
> + if (sdp_get_access_protos(recs->data, &protos) < 0) {
> + error("hf-client: unable to get access protocols from record");
> + sdp_list_free(classes, free);
> + goto failed;
> + }

Why not get protos later, when you use it. Error path will simpler.

> +
> + /* TODO read remote version? */
> +
> + memcpy(&uuid, classes->data, sizeof(uuid));
> + sdp_list_free(classes, free);
> +
> + if (!sdp_uuid128_to_uuid(&uuid) || uuid.type != SDP_UUID16 ||
> + uuid.value.uuid16 != HANDSFREE_AGW_SVCLASS_ID) {
> + sdp_list_free(protos, NULL);
> + error("hf-client: invalid service record or not HFP");
> + goto failed;
> + }
> +
> + channel = sdp_get_proto_port(protos, RFCOMM_UUID);
> + sdp_list_foreach(protos, (sdp_list_func_t) sdp_list_free, NULL);
> + sdp_list_free(protos, NULL);
> + if (channel <= 0) {
> + error("hf-client: unable to get RFCOMM channel from record");
> + goto failed;
> + }
> +
> + io = bt_io_connect(connect_cb, dev, NULL, &gerr,
> + BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
> + BT_IO_OPT_DEST_BDADDR, &dev->bdaddr,
> + BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
> + BT_IO_OPT_CHANNEL, channel,
> + BT_IO_OPT_INVALID);
> + if (!io) {
> + error("hf-client: unable to connect: %s", gerr->message);
> + g_error_free(gerr);
> + goto failed;
> + }
> +
> + g_io_channel_unref(io);
> + return;
> +
> +failed:
> + device_destroy(dev);
> +}
> +
> +static int sdp_search_hfp(struct device *dev)
> +{
> + uuid_t uuid;
> +
> + sdp_uuid16_create(&uuid, HANDSFREE_AGW_SVCLASS_ID);
> +
> + return bt_search_service(&adapter_addr, &dev->bdaddr, &uuid,
> + sdp_hfp_search_cb, dev, NULL, 0);
> +}
> +
> +static void handle_connect(const void *buf, uint16_t len)
> +{
> + struct device *dev;
> + const struct hal_cmd_hf_client_connect *cmd = buf;
> + uint32_t status;
> + bdaddr_t bdaddr;
> + char addr[18];
> +
> + DBG("");
> +
> + android2bdaddr(&cmd->bdaddr, &bdaddr);
> +
> + ba2str(&bdaddr, addr);
> + DBG("connecting to %s", addr);
> +
> + dev = get_device(&bdaddr);
> + if (!dev) {
> + status = HAL_STATUS_FAILED;
> + goto done;
> + }

Shouldn't dev state be checked here? And possibly set to 'connecting' is
command succeed?

> +
> + if (sdp_search_hfp(dev) < 0) {
> + status = HAL_STATUS_FAILED;
> + device_destroy(dev);
> + goto done;
> + }
> +
> + status = HAL_STATUS_SUCCESS;
> +
> +done:
> + ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_HANDSFREE_CLIENT,
> + HAL_OP_HF_CLIENT_CONNECT, status);
> +}
> +
> static void confirm_cb(GIOChannel *chan, gpointer data)
> {
> struct device *dev;

--
BR
Szymon Janc

2014-11-07 19:36:08

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH 5/7] android/handsfree-client: Add SLC creation procedure

Hi Łukasz,

On Wednesday 05 of November 2014 12:22:09 Lukasz Rymanowski wrote:
> This patch implements SLC procedure for HFP HF.
> ---
> android/handsfree-client.c | 536
> ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 535
> insertions(+), 1 deletion(-)
>
> diff --git a/android/handsfree-client.c b/android/handsfree-client.c
> index 6be9851..def057d 100644
> --- a/android/handsfree-client.c
> +++ b/android/handsfree-client.c
> @@ -36,6 +36,7 @@
> #include "lib/sdp_lib.h"
> #include "src/sdp-client.h"
> #include "src/shared/queue.h"
> +#include "src/shared/hfp.h"
> #include "btio/btio.h"
> #include "ipc.h"
> #include "ipc-common.h"
> @@ -59,15 +60,70 @@
> #define HFP_HF_FEAT_HF_IND 0x00000100
> #define HFP_HF_FEAT_ESCO_S4_T2 0x00000200
>
> +#define HFP_AG_FEAT_3WAY 0x00000001
> +#define HFP_AG_FEAT_ECNR 0x00000002
> +#define HFP_AG_FEAT_VR 0x00000004
> +#define HFP_AG_FEAT_INBAND 0x00000008
> +#define HFP_AG_FEAT_VTAG 0x00000010
> +#define HFP_AG_FEAT_REJ_CALL 0x00000020
> +#define HFP_AG_FEAT_ECS 0x00000040
> +#define HFP_AG_FEAT_ECC 0x00000080
> +#define HFP_AG_FEAT_EXT_ERR 0x00000100
> +#define HFP_AG_FEAT_CODEC 0x00000200
>
> #define HFP_HF_FEATURES (HFP_HF_FEAT_ECNR | HFP_HF_FEAT_3WAY |\
> HFP_HF_FEAT_CLI | HFP_HF_FEAT_VR |\
> HFP_HF_FEAT_RVC | HFP_HF_FEAT_ECS |\
> HFP_HF_FEAT_ECC)
>
> +#define CVSD_OFFSET 0
> +#define MSBC_OFFSET 1
> +#define CODECS_COUNT (MSBC_OFFSET + 1)
> +
> +#define CODEC_ID_CVSD 0x01
> +#define CODEC_ID_MSBC 0x02
> +
> +enum hfp_indicator {
> + HFP_INDICATOR_SERVICE = 0,
> + HFP_INDICATOR_CALL,
> + HFP_INDICATOR_CALLSETUP,
> + HFP_INDICATOR_CALLHELD,
> + HFP_INDICATOR_SIGNAL,
> + HFP_INDICATOR_ROAM,
> + HFP_INDICATOR_BATTCHG,
> + HFP_INDICATOR_LAST
> +};
> +
> +struct indicator {
> + uint8_t index;
> + uint32_t min;
> + uint32_t max;
> + uint32_t val;
> +};
> +
> +struct hfp_codec {
> + uint8_t type;
> + bool local_supported;
> + bool remote_supported;
> +};
> +
> struct device {
> bdaddr_t bdaddr;
> + bool slc_created;
> + struct hfp_hf *hf;
> uint8_t state;
> +
> + uint32_t features;
> + struct hfp_codec codecs[2];
> +
> + struct indicator ag_ind[HFP_INDICATOR_LAST];
> +
> + uint32_t chld_features;
> +};
> +
> +static const struct hfp_codec codecs_defaults[] = {
> + { CODEC_ID_CVSD, true, false},
> + { CODEC_ID_MSBC, false, false},
> };
>
> static bdaddr_t adapter_addr;
> @@ -93,6 +149,14 @@ static struct device *find_device(const bdaddr_t *addr)
> return queue_find(devices, match_by_bdaddr, addr);
> }
>
> +static void init_codecs(struct device *dev)
> +{
> + memcpy(&dev->codecs, codecs_defaults, sizeof(dev->codecs));
> +
> + if (hfp_hf_features & HFP_HF_FEAT_CODEC)
> + dev->codecs[MSBC_OFFSET].local_supported = true;
> +}
> +
> static struct device *device_create(const bdaddr_t *bdaddr)
> {
> struct device *dev;
> @@ -109,6 +173,8 @@ static struct device *device_create(const bdaddr_t
> *bdaddr)
>
> bacpy(&dev->bdaddr, bdaddr);
>
> + init_codecs(dev);
> +
> return dev;
> }
>
> @@ -265,6 +331,9 @@ static void device_set_state(struct device *dev, uint8_t
> state) bdaddr2android(&dev->bdaddr, ev.bdaddr);
> ev.state = state;
>
> + ev.chld_feat = dev->chld_features;
> + ev.peer_feat = dev->features;
> +
> ipc_send_notif(hal_ipc, HAL_SERVICE_ID_HANDSFREE_CLIENT,
> HAL_EV_HF_CLIENT_CONN_STATE, sizeof(ev), &ev);
> }
> @@ -274,9 +343,462 @@ static void device_destroy(void *data)
> struct device *dev = data;
>
> queue_remove(devices, dev);
> +
> + if (dev->hf)
> + hfp_hf_unref(dev->hf);
> +
> free(dev);
> }
>
> +static void disconnect_watch(void *user_data)
> +{
> + DBG("");
> +
> + device_destroy(user_data);
> +}
> +
> +static void slc_error(struct device *dev)
> +{
> + error("hf-client: Could not create SLC - dropping connection");
> + hfp_hf_disconnect(dev->hf);
> +}
> +
> +static void set_chld_feat(struct device *dev, char *feat)
> +{
> + DBG(" %s", feat);
> +
> + if (strcmp(feat, "0") == 0)
> + dev->chld_features |= HAL_HF_CLIENT_CHLD_FEAT_REL;
> + else if (strcmp(feat, "1") == 0)
> + dev->chld_features |= HAL_HF_CLIENT_CHLD_FEAT_REL_ACC;
> + else if (strcmp(feat, "1x") == 0)
> + dev->chld_features |= HAL_HF_CLIENT_CHLD_FEAT_REL_X;
> + else if (strcmp(feat, "2") == 0)
> + dev->chld_features |= HAL_HF_CLIENT_CHLD_FEAT_HOLD_ACC;
> + else if (strcmp(feat, "2x") == 0)
> + dev->chld_features |= HAL_HF_CLIENT_CHLD_FEAT_PRIV_X;
> + else if (strcmp(feat, "3") == 0)
> + dev->chld_features |= HAL_HF_CLIENT_CHLD_FEAT_MERGE;
> + else if (strcmp(feat, "4") == 0)
> + dev->chld_features |= HAL_HF_CLIENT_CHLD_FEAT_MERGE_DETACH;
> +}
> +
> +static void get_local_codecs_string(struct device *dev, char *buf,
> + uint8_t len)
> +{
> + int i;
> + uint8_t offset;
> +
> + memset(buf, 0, len);
> + buf[0] = '\0';

memset already cleared buf.

> + offset = 0;
> +
> + for (i = 0; i < CODECS_COUNT; i++) {
> + if (!dev->codecs[i].local_supported)
> + continue;
> +
> + offset += sprintf(&buf[offset], "%d,", dev->codecs[i].type);

Shouldn't you verify if that fit in buf?

> + }
> +}
> +
> +static void slc_completed(struct device *dev)
> +{
> + DBG("");
> +
> + dev->slc_created = true;
> + device_set_state(dev, HAL_HF_CLIENT_CONN_STATE_SLC_CONNECTED);

Isn't this bool duplicating state==SLC_CONNECTED?

> +
> + /*
> + * TODO: Notify Android with indicators, register unsolicited result
> + * handlers
> + */
> +}
> +
> +static void slc_chld_cb(struct hfp_context *context, void *user_data)
> +{
> + struct device *dev = user_data;
> + char feat[3];
> +
> + if (!hfp_context_open_container(context))
> + goto failed;
> +
> + while (hfp_context_get_unquoted_string(context, feat, sizeof(feat)))
> + set_chld_feat(dev, feat);
> +
> + if (!hfp_context_close_container(context))
> + info("hf-client: incorrect AT format? Lets be relax on this");

As we discussed offline, I don't like this. We can be relaxed while connected
by imho strict on SLC. Also such message should be a code comment, not info
message :)

> +
> + return;
> +
> +failed:
> + error("hf-client: Error on CHLD response");
> + slc_error(dev);
> +}
> +
> +static void slc_chld_resp(enum hfp_result result, enum hfp_error cme_err,
> + void *user_data)
> +{
> + struct device *dev = user_data;
> +
> + DBG("");
> +
> + hfp_hf_unregister(dev->hf, "+CHLD");
> +
> + if (result != HFP_RESULT_OK) {
> + error("hf-client: CHLD error: %d", result);
> + slc_error(dev);
> + return;
> + }
> +
> + slc_completed(dev);
> +}
> +
> +static void slc_cmer_resp(enum hfp_result result, enum hfp_error cme_err,
> + void *user_data)
> +{
> + struct device *dev = user_data;
> +
> + DBG("");
> +
> + if (result != HFP_RESULT_OK) {
> + error("hf-client: CMER error: %d", result);
> + goto failed;
> + }
> +
> + /* Continue with SLC creation */
> + if (!(dev->features & HFP_AG_FEAT_3WAY)) {
> + slc_completed(dev);
> + return;
> + }
> +
> + if (!hfp_hf_register(dev->hf, slc_chld_cb, "+CHLD", dev, NULL)) {
> + error("hf-client: Could not register +CHLD");
> + goto failed;
> + }
> +
> + if (!hfp_hf_send_command(dev->hf, slc_chld_resp, dev, "AT+CHLD=?")) {
> + error("hf-client: Could not send AT+CHLD");
> + goto failed;
> + }
> +
> + return;
> +
> +failed:
> + slc_error(dev);
> +}
> +
> +static void set_indicator_value(uint8_t index, uint32_t val,
> + struct indicator *ag_ind)
> +{
> + int i;
> +
> + for (i = 0; i < HFP_INDICATOR_LAST; i++) {
> + if (index != ag_ind[i].index)
> + continue;
> +
> + ag_ind[i].val = val;
> + return;
> + }
> +}
> +
> +static void slc_cind_status_cb(struct hfp_context *context,
> + void *user_data)
> +{
> + struct device *dev = user_data;
> + uint8_t index = 1;
> +
> + DBG("");
> +
> + while (hfp_context_has_next(context)) {
> + uint32_t val;
> +
> + if (!hfp_context_get_number(context, &val)) {
> + error("hf-client: Error on CIND status response");
> + return;
> + }
> +
> + set_indicator_value(index++, val, dev->ag_ind);
> + }
> +}
> +
> +static void slc_cind_status_resp(enum hfp_result result,
> + enum hfp_error cme_err,
> + void *user_data)
> +{
> + struct device *dev = user_data;
> +
> + DBG("");
> +
> + hfp_hf_unregister(dev->hf, "+CIND");
> +
> + if (result != HFP_RESULT_OK) {
> + error("hf-client: CIND error: %d", result);
> + goto failed;
> + }
> +
> + /* Continue with SLC creation */
> + if (!hfp_hf_send_command(dev->hf, slc_cmer_resp, dev,
> + "AT+CMER=3,0,0,1")) {
> + error("hf-client: Counld not send AT+CMER");
> + goto failed;
> + }
> +
> + return;
> +
> +failed:
> + slc_error(dev);
> +}
> +
> +static void slc_cind_resp(enum hfp_result result, enum hfp_error cme_err,
> + void *user_data)
> +{
> + struct device *dev = user_data;
> +
> + DBG("");
> +
> + hfp_hf_unregister(dev->hf, "+CIND");
> +
> + if (result != HFP_RESULT_OK) {
> + error("hf-client: CIND error: %d", result);
> + goto failed;
> + }
> +
> + /* Continue with SLC creation */
> + if (!hfp_hf_register(dev->hf, slc_cind_status_cb, "+CIND", dev,
> + NULL)) {
> + error("hf-client: Counld not register +CIND");
> + goto failed;
> + }
> +
> + if (!hfp_hf_send_command(dev->hf, slc_cind_status_resp, dev,
> + "AT+CIND?")) {
> + error("hf-client: Counld not send AT+CIND?");
> + goto failed;
> + }
> +
> + return;
> +
> +failed:
> + slc_error(dev);
> +}
> +
> +static void set_indicator_parameters(uint8_t index, const char *indicator,
> + uint32_t min, uint32_t max,

just use unsigned int instead of uint32_t here.

> + struct indicator *ag_ind)
> +{
> + DBG("%s, %i", indicator, index);

index is unsigned -> %u.

> +
> + if (strcmp("service", indicator) == 0) {
> + ag_ind[HFP_INDICATOR_SERVICE].index = index;
> + ag_ind[HFP_INDICATOR_SERVICE].min = min;
> + ag_ind[HFP_INDICATOR_SERVICE].max = max;

We should verify that min/max are within spec allowed range.

> + return;
> + }
> +
> + if (strcmp("call", indicator) == 0) {
> + ag_ind[HFP_INDICATOR_CALL].index = index;
> + ag_ind[HFP_INDICATOR_CALL].min = min;
> + ag_ind[HFP_INDICATOR_CALL].max = max;
> + return;
> + }
> +
> + if (strcmp("callsetup", indicator) == 0) {
> + ag_ind[HFP_INDICATOR_CALLSETUP].index = index;
> + ag_ind[HFP_INDICATOR_CALLSETUP].min = min;
> + ag_ind[HFP_INDICATOR_CALLSETUP].max = max;
> + return;
> + }
> +
> + if (strcmp("callheld", indicator) == 0) {
> + ag_ind[HFP_INDICATOR_CALLHELD].index = index;
> + ag_ind[HFP_INDICATOR_CALLHELD].min = min;
> + ag_ind[HFP_INDICATOR_CALLHELD].max = max;
> + return;
> + }
> +
> + if (strcmp("signal", indicator) == 0) {
> + ag_ind[HFP_INDICATOR_SIGNAL].index = index;
> + ag_ind[HFP_INDICATOR_SIGNAL].min = min;
> + ag_ind[HFP_INDICATOR_SIGNAL].max = max;
> + return;
> + }
> +
> + if (strcmp("roam", indicator) == 0) {
> + ag_ind[HFP_INDICATOR_ROAM].index = index;
> + ag_ind[HFP_INDICATOR_ROAM].min = min;
> + ag_ind[HFP_INDICATOR_ROAM].max = max;
> + return;
> + }
> +
> + if (strcmp("battchg", indicator) == 0) {
> + ag_ind[HFP_INDICATOR_BATTCHG].index = index;
> + ag_ind[HFP_INDICATOR_BATTCHG].min = min;
> + ag_ind[HFP_INDICATOR_BATTCHG].max = max;
> + return;
> + }
> +
> + error("hf-client: Unknown indicator: %s", indicator);
> +}
> +
> +static void slc_cind_cb(struct hfp_context *context, void *user_data)
> +{
> + struct device *dev = user_data;
> + int index = 1;
> +
> + DBG("");
> +
> + while (hfp_context_has_next(context)) {
> + char name[255];
> + unsigned int min, max;
> +
> + /*e.g ("callsetup",(0-3))*/

Spaces /* foo */.

> + if (!hfp_context_open_container(context))
> + break;
> +
> + if (!hfp_context_get_string(context, name, sizeof(name))) {
> + error("hf-client: Could not get string");
> + goto failed;
> + }
> +
> + if (!hfp_context_open_container(context)) {
> + error("hf-client: Could not open container");
> + goto failed;
> + }
> +
> + if (!hfp_context_get_range(context, &min, &max)) {
> + if (!hfp_context_get_number(context, &min)) {
> + error("hf-client: Could not get number");
> + goto failed;
> + }
> +
> + if (!hfp_context_get_number(context, &max)) {
> + error("hf-client: Could not get number");
> + goto failed;
> + }
> + }
> +
> + if (!hfp_context_close_container(context)) {
> + error("hf-client: Could not close container");
> + goto failed;
> + }
> +
> + if (!hfp_context_close_container(context)) {
> + error("hf-client: Could not close container");
> + goto failed;
> + }
> +
> + set_indicator_parameters(index, name, min, max, dev->ag_ind);
> + index++;
> + }
> +
> + return;
> +
> +failed:
> + error("hf-client: Error on CIND response");
> + slc_error(dev);
> +}
> +
> +static void slc_bac_resp(enum hfp_result result, enum hfp_error cme_err,
> + void *user_data)
> +{
> + struct device *dev = user_data;
> +
> + DBG("");
> +
> + if (result != HFP_RESULT_OK)
> + goto failed;
> +
> + /* Continue with SLC creation */
> + if (!hfp_hf_register(dev->hf, slc_cind_cb, "+CIND", dev, NULL)) {
> + error("hf-client: Could not register for +CIND");
> + goto failed;
> + }
> +
> + if (!hfp_hf_send_command(dev->hf, slc_cind_resp, dev, "AT+CIND=?"))
> + goto failed;
> +
> + return;
> +
> +failed:
> + error("hf-client: Error on BAC response");
> + slc_error(dev);
> +}
> +
> +static bool send_supported_codecs(struct device *dev)
> +{
> + char codecs_string[8];
> + char bac[16];
> +
> + memset(bac, 0, sizeof(bac));
> +
> + strcpy(bac, "AT+BAC=");
> +
> + get_local_codecs_string(dev, codecs_string, sizeof(codecs_string));
> + strcat(bac, codecs_string);
> +
> + return hfp_hf_send_command(dev->hf, slc_bac_resp, dev, bac);
> +}
> +
> +static void slc_brsf_cb(struct hfp_context *context, void *user_data)
> +{
> + uint32_t feat;
> + struct device *dev = user_data;
> +
> + DBG("");
> +
> + if (hfp_context_get_number(context, &feat))

feat should be unsigned int otherwise this might do invalid write on 64bit
arch. In general I'd avoid using fixed size types unless this is necessary due
to API or protocol.

> + dev->features = feat;
> +}
> +
> +static void slc_brsf_resp(enum hfp_result result, enum hfp_error cme_err,
> + void *user_data)
> +{
> + struct device *dev = user_data;
> +
> + hfp_hf_unregister(dev->hf, "+BRSF");
> +
> + if (result != HFP_RESULT_OK) {
> + error("hf-client: BRSF error: %d", result);
> + goto failed;
> + }
> +
> + /* Continue with SLC creation */
> + if (dev->features & HFP_AG_FEAT_CODEC) {
> + if (send_supported_codecs(dev))
> + return;
> +
> + error("hf-client: Could not send BAC command");
> + goto failed;
> + }
> +
> + /* No WBS on remote side. Continue with indicators */
> + if (!hfp_hf_register(dev->hf, slc_cind_cb, "+CIND", dev, NULL)) {
> + error("hf-client: Could not register for +CIND");
> + goto failed;
> + }
> +
> + if (!hfp_hf_send_command(dev->hf, slc_cind_resp, dev, "AT+CIND=?")) {
> + error("hf-client: Could not send AT+CIND command");
> + goto failed;
> + }
> +
> + return;
> +
> +failed:
> + slc_error(dev);
> +}
> +
> +static bool create_slc(struct device *dev)
> +{
> + DBG("");
> +
> + if (!hfp_hf_register(dev->hf, slc_brsf_cb, "+BRSF", dev, NULL))
> + return false;
> +
> + return hfp_hf_send_command(dev->hf, slc_brsf_resp, dev, "AT+BRSF=%u",
> + hfp_hf_features);
> +}
> +
> static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
> {
> struct device *dev = user_data;
> @@ -288,9 +810,21 @@ static void connect_cb(GIOChannel *chan, GError *err,
> gpointer user_data) goto failed;
> }
>
> + dev->hf = hfp_hf_new(g_io_channel_unix_get_fd(chan));
> + if (!dev->hf) {
> + error("hf-client: Could not create hfp io");
> + goto failed;
> + }
> +
> g_io_channel_set_close_on_unref(chan, FALSE);
>
> - /* TODO Create SLC here. For now do nothing, link will be dropped */
> + hfp_hf_set_close_on_unref(dev->hf, true);
> + hfp_hf_set_disconnect_handler(dev->hf, disconnect_watch, dev, NULL);
> +
> + if (!create_slc(dev)) {
> + error("hf-client: Could not start SLC creation");
> + hfp_hf_disconnect(dev->hf);

goto failed; missing?

> + }
>
> return;

--
BR
Szymon Janc

2014-11-07 19:00:02

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH 4/7] android/handsfree-client: Add incoming connection handling

Hi Łukasz,

On Wednesday 05 of November 2014 12:22:08 Lukasz Rymanowski wrote:
> ---
> android/handsfree-client.c | 174
> ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 173
> insertions(+), 1 deletion(-)
>
> diff --git a/android/handsfree-client.c b/android/handsfree-client.c
> index aa3912b..6be9851 100644
> --- a/android/handsfree-client.c
> +++ b/android/handsfree-client.c
> @@ -36,6 +36,7 @@
> #include "lib/sdp_lib.h"
> #include "src/sdp-client.h"
> #include "src/shared/queue.h"
> +#include "btio/btio.h"
> #include "ipc.h"
> #include "ipc-common.h"
> #include "src/log.h"
> @@ -64,6 +65,11 @@
> HFP_HF_FEAT_RVC | HFP_HF_FEAT_ECS |\
> HFP_HF_FEAT_ECC)
>
> +struct device {
> + bdaddr_t bdaddr;
> + uint8_t state;
> +};
> +
> static bdaddr_t adapter_addr;
>
> static struct ipc *hal_ipc = NULL;
> @@ -71,6 +77,55 @@ static struct ipc *hal_ipc = NULL;
> static uint32_t hfp_hf_features = 0;
> static uint32_t hfp_hf_record_id = 0;
> static struct queue *devices = NULL;
> +static GIOChannel *hfp_hf_server = NULL;
> +
> +static bool match_by_bdaddr(const void *data, const void *user_data)
> +{
> + const bdaddr_t *addr1 = data;
> + const bdaddr_t *addr2 = user_data;
> +
> + return !bacmp(addr1, addr2);
> +}
> +
> +static struct device *find_device(const bdaddr_t *addr)
> +{
> + /* For now we support only one device */

I'm not sure how this comment fits here.

> + return queue_find(devices, match_by_bdaddr, addr);
> +}
> +
> +static struct device *device_create(const bdaddr_t *bdaddr)
> +{
> + struct device *dev;
> +
> + dev = g_malloc0(sizeof(struct device));

This should be new0();

> + if (!dev)
> + return NULL;
> +
> + if (!queue_push_tail(devices, dev)) {
> + error("hf-client: Could not push dev on the list");
> + free(dev);
> + return NULL;
> + }
> +
> + bacpy(&dev->bdaddr, bdaddr);

Please initialize state here explicitly. Will make code easier to follow.

> +
> + return dev;
> +}
> +
> +static struct device *get_device(const bdaddr_t *addr)
> +{
> + struct device *dev;
> +
> + dev = find_device(addr);
> + if (dev)
> + return dev;
> +
> + /* We do support only one device as for now */
> + if (queue_isempty(devices))
> + return device_create(addr);
> +
> + return NULL;
> +}
>
> static void handle_connect(const void *buf, uint16_t len)
> {
> @@ -192,6 +247,102 @@ static void handle_get_last_vc_tag_num(const void
> *buf, uint16_t len) HAL_STATUS_UNSUPPORTED);
> }
>
> +static void device_set_state(struct device *dev, uint8_t state)
> +{
> + struct hal_ev_hf_client_conn_state ev;
> + char address[18];
> +
> + memset(&ev, 0, sizeof(ev));
> +
> + if (dev->state == state)
> + return;

Minor: this check could be done before clearing ev.

> +
> + dev->state = state;
> +
> + ba2str(&dev->bdaddr, address);
> + DBG("device %s state %u", address, state);
> +
> + bdaddr2android(&dev->bdaddr, ev.bdaddr);
> + ev.state = state;
> +
> + ipc_send_notif(hal_ipc, HAL_SERVICE_ID_HANDSFREE_CLIENT,
> + HAL_EV_HF_CLIENT_CONN_STATE, sizeof(ev), &ev);
> +}
> +
> +static void (void *data)

Please make this parameter proper type.

> +{
> + struct device *dev = data;
> +

I'd set state to disconnect here to be sure that framework always get proper
notification of disconnected device.

> + queue_remove(devices, dev);
> + free(dev);
> +}
> +
> +static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
> +{
> + struct device *dev = user_data;
> +
> + DBG("");
> +
> + if (err) {
> + error("hf-client: connect failed (%s)", err->message);
> + goto failed;
> + }
> +
> + g_io_channel_set_close_on_unref(chan, FALSE);
> +
> + /* TODO Create SLC here. For now do nothing, link will be dropped */
> +
> + return;
> +
> +failed:
> + g_io_channel_shutdown(chan, TRUE, NULL);
> + device_destroy(dev);
> +}
> +
> +static void confirm_cb(GIOChannel *chan, gpointer data)
> +{
> + struct device *dev;
> + char address[18];
> + bdaddr_t bdaddr;
> + GError *err = NULL;
> +
> + bt_io_get(chan, &err,
> + BT_IO_OPT_DEST, address,
> + BT_IO_OPT_DEST_BDADDR, &bdaddr,
> + BT_IO_OPT_INVALID);
> + if (err) {
> + error("hf-client: confirm failed (%s)", err->message);
> + g_error_free(err);
> + goto drop;
> + }
> +
> + DBG("Incoming connection from %s", address);
> +
> + dev = get_device(&bdaddr);
> + if (!dev) {
> + error("hf-client: There is other AG connected");
> + goto drop;
> + }
> +
> + if (dev->state != HAL_HF_CLIENT_CONN_STATE_DISCONNECTED) {
> + error("hf-client: Connections is up or ongoing ?");
> + return;

Shouldn't we drop connection here?

> + }
> +
> + device_set_state(dev, HAL_HF_CLIENT_CONN_STATE_CONNECTING);
> +
> + if (!bt_io_accept(chan, connect_cb, dev, NULL, NULL)) {
> + error("hf-client: failed to accept connection");

set_state(DISCONNECTED) is missing here. As mentioned above, maybe this should
be handled in device_destroy() as this seems to be missing in more places?

> + device_destroy(dev);
> + goto drop;
> + }
> +
> + return;
> +
> +drop:
> + g_io_channel_shutdown(chan, TRUE, NULL);
> +}
> +
> static const struct ipc_handler cmd_handlers[] = {
> /* HAL_OP_HF_CLIENT_CONNECT */
> { handle_connect, false,
> @@ -304,6 +455,21 @@ static sdp_record_t *hfp_hf_record(void)
> static bool enable_hf_client(void)
> {
> sdp_record_t *rec;
> + GError *err = NULL;
> +
> + if (hfp_hf_server)
> + return false;

Can this happen?

> +
> + hfp_hf_server = bt_io_listen(NULL, confirm_cb, NULL, NULL, &err,
> + BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
> + BT_IO_OPT_CHANNEL, HFP_HF_CHANNEL,
> + BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
> + BT_IO_OPT_INVALID);
> + if (!hfp_hf_server) {
> + error("Failed to listen on Handsfree rfcomm: %s", err->message);

prefix this with hf-client: to avoid confusion with AG.

> + g_error_free(err);
> + return false;
> + }
>
> hfp_hf_features = HFP_HF_FEATURES;
>
> @@ -326,6 +492,12 @@ static bool enable_hf_client(void)
>
> static void cleanup_hfp_hf(void)
> {
> + if (hfp_hf_server) {
> + g_io_channel_shutdown(hfp_hf_server, TRUE, NULL);
> + g_io_channel_unref(hfp_hf_server);
> + hfp_hf_server = NULL;
> + }
> +
> if (hfp_hf_record_id > 0) {
> bt_adapter_remove_record(hfp_hf_record_id);
> hfp_hf_record_id = 0;
> @@ -366,7 +538,7 @@ void bt_hf_client_unregister(void)
>
> cleanup_hfp_hf();
>
> - queue_destroy(devices, free);
> + queue_destroy(devices, device_destroy);
> devices = NULL;
>
> ipc_unregister(hal_ipc, HAL_SERVICE_ID_HANDSFREE);

--
BR
Szymon Janc

2014-11-05 11:22:11

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH 7/7] android/handsfree-client: Handle disconnect command

---
android/handsfree-client.c | 27 +++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/android/handsfree-client.c b/android/handsfree-client.c
index d90cef0..751a9f1 100644
--- a/android/handsfree-client.c
+++ b/android/handsfree-client.c
@@ -195,9 +195,32 @@ static struct device *get_device(const bdaddr_t *addr)

static void handle_disconnect(const void *buf, uint16_t len)
{
- DBG("Not Implemented");
+ const struct hal_cmd_hf_client_disconnect *cmd = buf;
+ struct device *dev;
+ uint32_t status;
+ bdaddr_t bdaddr;
+ char addr[18];
+
+ DBG("");
+
+ android2bdaddr(&cmd->bdaddr, &bdaddr);
+
+ ba2str(&bdaddr, addr);
+ DBG("Disconnect %s", addr);
+
+ dev = get_device(&bdaddr);
+ if (!dev) {
+ status = HAL_STATUS_FAILED;
+ goto done;
+ }
+
+ status = hfp_hf_disconnect(dev->hf) ? HAL_STATUS_SUCCESS :
+ HAL_STATUS_FAILED;
+
+done:
+
ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_HANDSFREE_CLIENT,
- HAL_OP_HF_CLIENT_DISCONNECT, HAL_STATUS_UNSUPPORTED);
+ HAL_OP_HF_CLIENT_DISCONNECT, status);
}

static void handle_connect_audio(const void *buf, uint16_t len)
--
1.8.4


2014-11-05 11:22:10

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH 6/7] android/handsfree-client: Add support for outgoing connection

---
android/handsfree-client.c | 123 ++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 116 insertions(+), 7 deletions(-)

diff --git a/android/handsfree-client.c b/android/handsfree-client.c
index def057d..d90cef0 100644
--- a/android/handsfree-client.c
+++ b/android/handsfree-client.c
@@ -193,13 +193,6 @@ static struct device *get_device(const bdaddr_t *addr)
return NULL;
}

-static void handle_connect(const void *buf, uint16_t len)
-{
- DBG("Not Implemented");
- ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_HANDSFREE_CLIENT,
- HAL_OP_HF_CLIENT_CONNECT, HAL_STATUS_UNSUPPORTED);
-}
-
static void handle_disconnect(const void *buf, uint16_t len)
{
DBG("Not Implemented");
@@ -833,6 +826,122 @@ failed:
device_destroy(dev);
}

+static void sdp_hfp_search_cb(sdp_list_t *recs, int err, gpointer data)
+{
+ sdp_list_t *protos, *classes;
+ struct device *dev = data;
+ GError *gerr = NULL;
+ GIOChannel *io;
+ uuid_t uuid;
+ int channel;
+
+ DBG("");
+
+ if (err < 0) {
+ error("hf-client: unable to get SDP record: %s",
+ strerror(-err));
+ goto failed;
+ }
+
+ if (!recs || !recs->data) {
+ info("hf-client: no HFP SDP records found");
+ goto failed;
+ }
+
+ if (sdp_get_service_classes(recs->data, &classes) < 0 || !classes) {
+ error("hf-client: unable to get service classes from record");
+ goto failed;
+ }
+
+ if (sdp_get_access_protos(recs->data, &protos) < 0) {
+ error("hf-client: unable to get access protocols from record");
+ sdp_list_free(classes, free);
+ goto failed;
+ }
+
+ /* TODO read remote version? */
+
+ memcpy(&uuid, classes->data, sizeof(uuid));
+ sdp_list_free(classes, free);
+
+ if (!sdp_uuid128_to_uuid(&uuid) || uuid.type != SDP_UUID16 ||
+ uuid.value.uuid16 != HANDSFREE_AGW_SVCLASS_ID) {
+ sdp_list_free(protos, NULL);
+ error("hf-client: invalid service record or not HFP");
+ goto failed;
+ }
+
+ channel = sdp_get_proto_port(protos, RFCOMM_UUID);
+ sdp_list_foreach(protos, (sdp_list_func_t) sdp_list_free, NULL);
+ sdp_list_free(protos, NULL);
+ if (channel <= 0) {
+ error("hf-client: unable to get RFCOMM channel from record");
+ goto failed;
+ }
+
+ io = bt_io_connect(connect_cb, dev, NULL, &gerr,
+ BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
+ BT_IO_OPT_DEST_BDADDR, &dev->bdaddr,
+ BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
+ BT_IO_OPT_CHANNEL, channel,
+ BT_IO_OPT_INVALID);
+ if (!io) {
+ error("hf-client: unable to connect: %s", gerr->message);
+ g_error_free(gerr);
+ goto failed;
+ }
+
+ g_io_channel_unref(io);
+ return;
+
+failed:
+ device_destroy(dev);
+}
+
+static int sdp_search_hfp(struct device *dev)
+{
+ uuid_t uuid;
+
+ sdp_uuid16_create(&uuid, HANDSFREE_AGW_SVCLASS_ID);
+
+ return bt_search_service(&adapter_addr, &dev->bdaddr, &uuid,
+ sdp_hfp_search_cb, dev, NULL, 0);
+}
+
+static void handle_connect(const void *buf, uint16_t len)
+{
+ struct device *dev;
+ const struct hal_cmd_hf_client_connect *cmd = buf;
+ uint32_t status;
+ bdaddr_t bdaddr;
+ char addr[18];
+
+ DBG("");
+
+ android2bdaddr(&cmd->bdaddr, &bdaddr);
+
+ ba2str(&bdaddr, addr);
+ DBG("connecting to %s", addr);
+
+ dev = get_device(&bdaddr);
+ if (!dev) {
+ status = HAL_STATUS_FAILED;
+ goto done;
+ }
+
+ if (sdp_search_hfp(dev) < 0) {
+ status = HAL_STATUS_FAILED;
+ device_destroy(dev);
+ goto done;
+ }
+
+ status = HAL_STATUS_SUCCESS;
+
+done:
+ ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_HANDSFREE_CLIENT,
+ HAL_OP_HF_CLIENT_CONNECT, status);
+}
+
static void confirm_cb(GIOChannel *chan, gpointer data)
{
struct device *dev;
--
1.8.4


2014-11-05 11:22:09

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH 5/7] android/handsfree-client: Add SLC creation procedure

This patch implements SLC procedure for HFP HF.
---
android/handsfree-client.c | 536 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 535 insertions(+), 1 deletion(-)

diff --git a/android/handsfree-client.c b/android/handsfree-client.c
index 6be9851..def057d 100644
--- a/android/handsfree-client.c
+++ b/android/handsfree-client.c
@@ -36,6 +36,7 @@
#include "lib/sdp_lib.h"
#include "src/sdp-client.h"
#include "src/shared/queue.h"
+#include "src/shared/hfp.h"
#include "btio/btio.h"
#include "ipc.h"
#include "ipc-common.h"
@@ -59,15 +60,70 @@
#define HFP_HF_FEAT_HF_IND 0x00000100
#define HFP_HF_FEAT_ESCO_S4_T2 0x00000200

+#define HFP_AG_FEAT_3WAY 0x00000001
+#define HFP_AG_FEAT_ECNR 0x00000002
+#define HFP_AG_FEAT_VR 0x00000004
+#define HFP_AG_FEAT_INBAND 0x00000008
+#define HFP_AG_FEAT_VTAG 0x00000010
+#define HFP_AG_FEAT_REJ_CALL 0x00000020
+#define HFP_AG_FEAT_ECS 0x00000040
+#define HFP_AG_FEAT_ECC 0x00000080
+#define HFP_AG_FEAT_EXT_ERR 0x00000100
+#define HFP_AG_FEAT_CODEC 0x00000200

#define HFP_HF_FEATURES (HFP_HF_FEAT_ECNR | HFP_HF_FEAT_3WAY |\
HFP_HF_FEAT_CLI | HFP_HF_FEAT_VR |\
HFP_HF_FEAT_RVC | HFP_HF_FEAT_ECS |\
HFP_HF_FEAT_ECC)

+#define CVSD_OFFSET 0
+#define MSBC_OFFSET 1
+#define CODECS_COUNT (MSBC_OFFSET + 1)
+
+#define CODEC_ID_CVSD 0x01
+#define CODEC_ID_MSBC 0x02
+
+enum hfp_indicator {
+ HFP_INDICATOR_SERVICE = 0,
+ HFP_INDICATOR_CALL,
+ HFP_INDICATOR_CALLSETUP,
+ HFP_INDICATOR_CALLHELD,
+ HFP_INDICATOR_SIGNAL,
+ HFP_INDICATOR_ROAM,
+ HFP_INDICATOR_BATTCHG,
+ HFP_INDICATOR_LAST
+};
+
+struct indicator {
+ uint8_t index;
+ uint32_t min;
+ uint32_t max;
+ uint32_t val;
+};
+
+struct hfp_codec {
+ uint8_t type;
+ bool local_supported;
+ bool remote_supported;
+};
+
struct device {
bdaddr_t bdaddr;
+ bool slc_created;
+ struct hfp_hf *hf;
uint8_t state;
+
+ uint32_t features;
+ struct hfp_codec codecs[2];
+
+ struct indicator ag_ind[HFP_INDICATOR_LAST];
+
+ uint32_t chld_features;
+};
+
+static const struct hfp_codec codecs_defaults[] = {
+ { CODEC_ID_CVSD, true, false},
+ { CODEC_ID_MSBC, false, false},
};

static bdaddr_t adapter_addr;
@@ -93,6 +149,14 @@ static struct device *find_device(const bdaddr_t *addr)
return queue_find(devices, match_by_bdaddr, addr);
}

+static void init_codecs(struct device *dev)
+{
+ memcpy(&dev->codecs, codecs_defaults, sizeof(dev->codecs));
+
+ if (hfp_hf_features & HFP_HF_FEAT_CODEC)
+ dev->codecs[MSBC_OFFSET].local_supported = true;
+}
+
static struct device *device_create(const bdaddr_t *bdaddr)
{
struct device *dev;
@@ -109,6 +173,8 @@ static struct device *device_create(const bdaddr_t *bdaddr)

bacpy(&dev->bdaddr, bdaddr);

+ init_codecs(dev);
+
return dev;
}

@@ -265,6 +331,9 @@ static void device_set_state(struct device *dev, uint8_t state)
bdaddr2android(&dev->bdaddr, ev.bdaddr);
ev.state = state;

+ ev.chld_feat = dev->chld_features;
+ ev.peer_feat = dev->features;
+
ipc_send_notif(hal_ipc, HAL_SERVICE_ID_HANDSFREE_CLIENT,
HAL_EV_HF_CLIENT_CONN_STATE, sizeof(ev), &ev);
}
@@ -274,9 +343,462 @@ static void device_destroy(void *data)
struct device *dev = data;

queue_remove(devices, dev);
+
+ if (dev->hf)
+ hfp_hf_unref(dev->hf);
+
free(dev);
}

+static void disconnect_watch(void *user_data)
+{
+ DBG("");
+
+ device_destroy(user_data);
+}
+
+static void slc_error(struct device *dev)
+{
+ error("hf-client: Could not create SLC - dropping connection");
+ hfp_hf_disconnect(dev->hf);
+}
+
+static void set_chld_feat(struct device *dev, char *feat)
+{
+ DBG(" %s", feat);
+
+ if (strcmp(feat, "0") == 0)
+ dev->chld_features |= HAL_HF_CLIENT_CHLD_FEAT_REL;
+ else if (strcmp(feat, "1") == 0)
+ dev->chld_features |= HAL_HF_CLIENT_CHLD_FEAT_REL_ACC;
+ else if (strcmp(feat, "1x") == 0)
+ dev->chld_features |= HAL_HF_CLIENT_CHLD_FEAT_REL_X;
+ else if (strcmp(feat, "2") == 0)
+ dev->chld_features |= HAL_HF_CLIENT_CHLD_FEAT_HOLD_ACC;
+ else if (strcmp(feat, "2x") == 0)
+ dev->chld_features |= HAL_HF_CLIENT_CHLD_FEAT_PRIV_X;
+ else if (strcmp(feat, "3") == 0)
+ dev->chld_features |= HAL_HF_CLIENT_CHLD_FEAT_MERGE;
+ else if (strcmp(feat, "4") == 0)
+ dev->chld_features |= HAL_HF_CLIENT_CHLD_FEAT_MERGE_DETACH;
+}
+
+static void get_local_codecs_string(struct device *dev, char *buf,
+ uint8_t len)
+{
+ int i;
+ uint8_t offset;
+
+ memset(buf, 0, len);
+ buf[0] = '\0';
+ offset = 0;
+
+ for (i = 0; i < CODECS_COUNT; i++) {
+ if (!dev->codecs[i].local_supported)
+ continue;
+
+ offset += sprintf(&buf[offset], "%d,", dev->codecs[i].type);
+ }
+}
+
+static void slc_completed(struct device *dev)
+{
+ DBG("");
+
+ dev->slc_created = true;
+ device_set_state(dev, HAL_HF_CLIENT_CONN_STATE_SLC_CONNECTED);
+
+ /*
+ * TODO: Notify Android with indicators, register unsolicited result
+ * handlers
+ */
+}
+
+static void slc_chld_cb(struct hfp_context *context, void *user_data)
+{
+ struct device *dev = user_data;
+ char feat[3];
+
+ if (!hfp_context_open_container(context))
+ goto failed;
+
+ while (hfp_context_get_unquoted_string(context, feat, sizeof(feat)))
+ set_chld_feat(dev, feat);
+
+ if (!hfp_context_close_container(context))
+ info("hf-client: incorrect AT format? Lets be relax on this");
+
+ return;
+
+failed:
+ error("hf-client: Error on CHLD response");
+ slc_error(dev);
+}
+
+static void slc_chld_resp(enum hfp_result result, enum hfp_error cme_err,
+ void *user_data)
+{
+ struct device *dev = user_data;
+
+ DBG("");
+
+ hfp_hf_unregister(dev->hf, "+CHLD");
+
+ if (result != HFP_RESULT_OK) {
+ error("hf-client: CHLD error: %d", result);
+ slc_error(dev);
+ return;
+ }
+
+ slc_completed(dev);
+}
+
+static void slc_cmer_resp(enum hfp_result result, enum hfp_error cme_err,
+ void *user_data)
+{
+ struct device *dev = user_data;
+
+ DBG("");
+
+ if (result != HFP_RESULT_OK) {
+ error("hf-client: CMER error: %d", result);
+ goto failed;
+ }
+
+ /* Continue with SLC creation */
+ if (!(dev->features & HFP_AG_FEAT_3WAY)) {
+ slc_completed(dev);
+ return;
+ }
+
+ if (!hfp_hf_register(dev->hf, slc_chld_cb, "+CHLD", dev, NULL)) {
+ error("hf-client: Could not register +CHLD");
+ goto failed;
+ }
+
+ if (!hfp_hf_send_command(dev->hf, slc_chld_resp, dev, "AT+CHLD=?")) {
+ error("hf-client: Could not send AT+CHLD");
+ goto failed;
+ }
+
+ return;
+
+failed:
+ slc_error(dev);
+}
+
+static void set_indicator_value(uint8_t index, uint32_t val,
+ struct indicator *ag_ind)
+{
+ int i;
+
+ for (i = 0; i < HFP_INDICATOR_LAST; i++) {
+ if (index != ag_ind[i].index)
+ continue;
+
+ ag_ind[i].val = val;
+ return;
+ }
+}
+
+static void slc_cind_status_cb(struct hfp_context *context,
+ void *user_data)
+{
+ struct device *dev = user_data;
+ uint8_t index = 1;
+
+ DBG("");
+
+ while (hfp_context_has_next(context)) {
+ uint32_t val;
+
+ if (!hfp_context_get_number(context, &val)) {
+ error("hf-client: Error on CIND status response");
+ return;
+ }
+
+ set_indicator_value(index++, val, dev->ag_ind);
+ }
+}
+
+static void slc_cind_status_resp(enum hfp_result result,
+ enum hfp_error cme_err,
+ void *user_data)
+{
+ struct device *dev = user_data;
+
+ DBG("");
+
+ hfp_hf_unregister(dev->hf, "+CIND");
+
+ if (result != HFP_RESULT_OK) {
+ error("hf-client: CIND error: %d", result);
+ goto failed;
+ }
+
+ /* Continue with SLC creation */
+ if (!hfp_hf_send_command(dev->hf, slc_cmer_resp, dev,
+ "AT+CMER=3,0,0,1")) {
+ error("hf-client: Counld not send AT+CMER");
+ goto failed;
+ }
+
+ return;
+
+failed:
+ slc_error(dev);
+}
+
+static void slc_cind_resp(enum hfp_result result, enum hfp_error cme_err,
+ void *user_data)
+{
+ struct device *dev = user_data;
+
+ DBG("");
+
+ hfp_hf_unregister(dev->hf, "+CIND");
+
+ if (result != HFP_RESULT_OK) {
+ error("hf-client: CIND error: %d", result);
+ goto failed;
+ }
+
+ /* Continue with SLC creation */
+ if (!hfp_hf_register(dev->hf, slc_cind_status_cb, "+CIND", dev,
+ NULL)) {
+ error("hf-client: Counld not register +CIND");
+ goto failed;
+ }
+
+ if (!hfp_hf_send_command(dev->hf, slc_cind_status_resp, dev,
+ "AT+CIND?")) {
+ error("hf-client: Counld not send AT+CIND?");
+ goto failed;
+ }
+
+ return;
+
+failed:
+ slc_error(dev);
+}
+
+static void set_indicator_parameters(uint8_t index, const char *indicator,
+ uint32_t min, uint32_t max,
+ struct indicator *ag_ind)
+{
+ DBG("%s, %i", indicator, index);
+
+ if (strcmp("service", indicator) == 0) {
+ ag_ind[HFP_INDICATOR_SERVICE].index = index;
+ ag_ind[HFP_INDICATOR_SERVICE].min = min;
+ ag_ind[HFP_INDICATOR_SERVICE].max = max;
+ return;
+ }
+
+ if (strcmp("call", indicator) == 0) {
+ ag_ind[HFP_INDICATOR_CALL].index = index;
+ ag_ind[HFP_INDICATOR_CALL].min = min;
+ ag_ind[HFP_INDICATOR_CALL].max = max;
+ return;
+ }
+
+ if (strcmp("callsetup", indicator) == 0) {
+ ag_ind[HFP_INDICATOR_CALLSETUP].index = index;
+ ag_ind[HFP_INDICATOR_CALLSETUP].min = min;
+ ag_ind[HFP_INDICATOR_CALLSETUP].max = max;
+ return;
+ }
+
+ if (strcmp("callheld", indicator) == 0) {
+ ag_ind[HFP_INDICATOR_CALLHELD].index = index;
+ ag_ind[HFP_INDICATOR_CALLHELD].min = min;
+ ag_ind[HFP_INDICATOR_CALLHELD].max = max;
+ return;
+ }
+
+ if (strcmp("signal", indicator) == 0) {
+ ag_ind[HFP_INDICATOR_SIGNAL].index = index;
+ ag_ind[HFP_INDICATOR_SIGNAL].min = min;
+ ag_ind[HFP_INDICATOR_SIGNAL].max = max;
+ return;
+ }
+
+ if (strcmp("roam", indicator) == 0) {
+ ag_ind[HFP_INDICATOR_ROAM].index = index;
+ ag_ind[HFP_INDICATOR_ROAM].min = min;
+ ag_ind[HFP_INDICATOR_ROAM].max = max;
+ return;
+ }
+
+ if (strcmp("battchg", indicator) == 0) {
+ ag_ind[HFP_INDICATOR_BATTCHG].index = index;
+ ag_ind[HFP_INDICATOR_BATTCHG].min = min;
+ ag_ind[HFP_INDICATOR_BATTCHG].max = max;
+ return;
+ }
+
+ error("hf-client: Unknown indicator: %s", indicator);
+}
+
+static void slc_cind_cb(struct hfp_context *context, void *user_data)
+{
+ struct device *dev = user_data;
+ int index = 1;
+
+ DBG("");
+
+ while (hfp_context_has_next(context)) {
+ char name[255];
+ unsigned int min, max;
+
+ /*e.g ("callsetup",(0-3))*/
+ if (!hfp_context_open_container(context))
+ break;
+
+ if (!hfp_context_get_string(context, name, sizeof(name))) {
+ error("hf-client: Could not get string");
+ goto failed;
+ }
+
+ if (!hfp_context_open_container(context)) {
+ error("hf-client: Could not open container");
+ goto failed;
+ }
+
+ if (!hfp_context_get_range(context, &min, &max)) {
+ if (!hfp_context_get_number(context, &min)) {
+ error("hf-client: Could not get number");
+ goto failed;
+ }
+
+ if (!hfp_context_get_number(context, &max)) {
+ error("hf-client: Could not get number");
+ goto failed;
+ }
+ }
+
+ if (!hfp_context_close_container(context)) {
+ error("hf-client: Could not close container");
+ goto failed;
+ }
+
+ if (!hfp_context_close_container(context)) {
+ error("hf-client: Could not close container");
+ goto failed;
+ }
+
+ set_indicator_parameters(index, name, min, max, dev->ag_ind);
+ index++;
+ }
+
+ return;
+
+failed:
+ error("hf-client: Error on CIND response");
+ slc_error(dev);
+}
+
+static void slc_bac_resp(enum hfp_result result, enum hfp_error cme_err,
+ void *user_data)
+{
+ struct device *dev = user_data;
+
+ DBG("");
+
+ if (result != HFP_RESULT_OK)
+ goto failed;
+
+ /* Continue with SLC creation */
+ if (!hfp_hf_register(dev->hf, slc_cind_cb, "+CIND", dev, NULL)) {
+ error("hf-client: Could not register for +CIND");
+ goto failed;
+ }
+
+ if (!hfp_hf_send_command(dev->hf, slc_cind_resp, dev, "AT+CIND=?"))
+ goto failed;
+
+ return;
+
+failed:
+ error("hf-client: Error on BAC response");
+ slc_error(dev);
+}
+
+static bool send_supported_codecs(struct device *dev)
+{
+ char codecs_string[8];
+ char bac[16];
+
+ memset(bac, 0, sizeof(bac));
+
+ strcpy(bac, "AT+BAC=");
+
+ get_local_codecs_string(dev, codecs_string, sizeof(codecs_string));
+ strcat(bac, codecs_string);
+
+ return hfp_hf_send_command(dev->hf, slc_bac_resp, dev, bac);
+}
+
+static void slc_brsf_cb(struct hfp_context *context, void *user_data)
+{
+ uint32_t feat;
+ struct device *dev = user_data;
+
+ DBG("");
+
+ if (hfp_context_get_number(context, &feat))
+ dev->features = feat;
+}
+
+static void slc_brsf_resp(enum hfp_result result, enum hfp_error cme_err,
+ void *user_data)
+{
+ struct device *dev = user_data;
+
+ hfp_hf_unregister(dev->hf, "+BRSF");
+
+ if (result != HFP_RESULT_OK) {
+ error("hf-client: BRSF error: %d", result);
+ goto failed;
+ }
+
+ /* Continue with SLC creation */
+ if (dev->features & HFP_AG_FEAT_CODEC) {
+ if (send_supported_codecs(dev))
+ return;
+
+ error("hf-client: Could not send BAC command");
+ goto failed;
+ }
+
+ /* No WBS on remote side. Continue with indicators */
+ if (!hfp_hf_register(dev->hf, slc_cind_cb, "+CIND", dev, NULL)) {
+ error("hf-client: Could not register for +CIND");
+ goto failed;
+ }
+
+ if (!hfp_hf_send_command(dev->hf, slc_cind_resp, dev, "AT+CIND=?")) {
+ error("hf-client: Could not send AT+CIND command");
+ goto failed;
+ }
+
+ return;
+
+failed:
+ slc_error(dev);
+}
+
+static bool create_slc(struct device *dev)
+{
+ DBG("");
+
+ if (!hfp_hf_register(dev->hf, slc_brsf_cb, "+BRSF", dev, NULL))
+ return false;
+
+ return hfp_hf_send_command(dev->hf, slc_brsf_resp, dev, "AT+BRSF=%u",
+ hfp_hf_features);
+}
+
static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
{
struct device *dev = user_data;
@@ -288,9 +810,21 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
goto failed;
}

+ dev->hf = hfp_hf_new(g_io_channel_unix_get_fd(chan));
+ if (!dev->hf) {
+ error("hf-client: Could not create hfp io");
+ goto failed;
+ }
+
g_io_channel_set_close_on_unref(chan, FALSE);

- /* TODO Create SLC here. For now do nothing, link will be dropped */
+ hfp_hf_set_close_on_unref(dev->hf, true);
+ hfp_hf_set_disconnect_handler(dev->hf, disconnect_watch, dev, NULL);
+
+ if (!create_slc(dev)) {
+ error("hf-client: Could not start SLC creation");
+ hfp_hf_disconnect(dev->hf);
+ }

return;

--
1.8.4


2014-11-05 11:22:08

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH 4/7] android/handsfree-client: Add incoming connection handling

---
android/handsfree-client.c | 174 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 173 insertions(+), 1 deletion(-)

diff --git a/android/handsfree-client.c b/android/handsfree-client.c
index aa3912b..6be9851 100644
--- a/android/handsfree-client.c
+++ b/android/handsfree-client.c
@@ -36,6 +36,7 @@
#include "lib/sdp_lib.h"
#include "src/sdp-client.h"
#include "src/shared/queue.h"
+#include "btio/btio.h"
#include "ipc.h"
#include "ipc-common.h"
#include "src/log.h"
@@ -64,6 +65,11 @@
HFP_HF_FEAT_RVC | HFP_HF_FEAT_ECS |\
HFP_HF_FEAT_ECC)

+struct device {
+ bdaddr_t bdaddr;
+ uint8_t state;
+};
+
static bdaddr_t adapter_addr;

static struct ipc *hal_ipc = NULL;
@@ -71,6 +77,55 @@ static struct ipc *hal_ipc = NULL;
static uint32_t hfp_hf_features = 0;
static uint32_t hfp_hf_record_id = 0;
static struct queue *devices = NULL;
+static GIOChannel *hfp_hf_server = NULL;
+
+static bool match_by_bdaddr(const void *data, const void *user_data)
+{
+ const bdaddr_t *addr1 = data;
+ const bdaddr_t *addr2 = user_data;
+
+ return !bacmp(addr1, addr2);
+}
+
+static struct device *find_device(const bdaddr_t *addr)
+{
+ /* For now we support only one device */
+ return queue_find(devices, match_by_bdaddr, addr);
+}
+
+static struct device *device_create(const bdaddr_t *bdaddr)
+{
+ struct device *dev;
+
+ dev = g_malloc0(sizeof(struct device));
+ if (!dev)
+ return NULL;
+
+ if (!queue_push_tail(devices, dev)) {
+ error("hf-client: Could not push dev on the list");
+ free(dev);
+ return NULL;
+ }
+
+ bacpy(&dev->bdaddr, bdaddr);
+
+ return dev;
+}
+
+static struct device *get_device(const bdaddr_t *addr)
+{
+ struct device *dev;
+
+ dev = find_device(addr);
+ if (dev)
+ return dev;
+
+ /* We do support only one device as for now */
+ if (queue_isempty(devices))
+ return device_create(addr);
+
+ return NULL;
+}

static void handle_connect(const void *buf, uint16_t len)
{
@@ -192,6 +247,102 @@ static void handle_get_last_vc_tag_num(const void *buf, uint16_t len)
HAL_STATUS_UNSUPPORTED);
}

+static void device_set_state(struct device *dev, uint8_t state)
+{
+ struct hal_ev_hf_client_conn_state ev;
+ char address[18];
+
+ memset(&ev, 0, sizeof(ev));
+
+ if (dev->state == state)
+ return;
+
+ dev->state = state;
+
+ ba2str(&dev->bdaddr, address);
+ DBG("device %s state %u", address, state);
+
+ bdaddr2android(&dev->bdaddr, ev.bdaddr);
+ ev.state = state;
+
+ ipc_send_notif(hal_ipc, HAL_SERVICE_ID_HANDSFREE_CLIENT,
+ HAL_EV_HF_CLIENT_CONN_STATE, sizeof(ev), &ev);
+}
+
+static void device_destroy(void *data)
+{
+ struct device *dev = data;
+
+ queue_remove(devices, dev);
+ free(dev);
+}
+
+static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
+{
+ struct device *dev = user_data;
+
+ DBG("");
+
+ if (err) {
+ error("hf-client: connect failed (%s)", err->message);
+ goto failed;
+ }
+
+ g_io_channel_set_close_on_unref(chan, FALSE);
+
+ /* TODO Create SLC here. For now do nothing, link will be dropped */
+
+ return;
+
+failed:
+ g_io_channel_shutdown(chan, TRUE, NULL);
+ device_destroy(dev);
+}
+
+static void confirm_cb(GIOChannel *chan, gpointer data)
+{
+ struct device *dev;
+ char address[18];
+ bdaddr_t bdaddr;
+ GError *err = NULL;
+
+ bt_io_get(chan, &err,
+ BT_IO_OPT_DEST, address,
+ BT_IO_OPT_DEST_BDADDR, &bdaddr,
+ BT_IO_OPT_INVALID);
+ if (err) {
+ error("hf-client: confirm failed (%s)", err->message);
+ g_error_free(err);
+ goto drop;
+ }
+
+ DBG("Incoming connection from %s", address);
+
+ dev = get_device(&bdaddr);
+ if (!dev) {
+ error("hf-client: There is other AG connected");
+ goto drop;
+ }
+
+ if (dev->state != HAL_HF_CLIENT_CONN_STATE_DISCONNECTED) {
+ error("hf-client: Connections is up or ongoing ?");
+ return;
+ }
+
+ device_set_state(dev, HAL_HF_CLIENT_CONN_STATE_CONNECTING);
+
+ if (!bt_io_accept(chan, connect_cb, dev, NULL, NULL)) {
+ error("hf-client: failed to accept connection");
+ device_destroy(dev);
+ goto drop;
+ }
+
+ return;
+
+drop:
+ g_io_channel_shutdown(chan, TRUE, NULL);
+}
+
static const struct ipc_handler cmd_handlers[] = {
/* HAL_OP_HF_CLIENT_CONNECT */
{ handle_connect, false,
@@ -304,6 +455,21 @@ static sdp_record_t *hfp_hf_record(void)
static bool enable_hf_client(void)
{
sdp_record_t *rec;
+ GError *err = NULL;
+
+ if (hfp_hf_server)
+ return false;
+
+ hfp_hf_server = bt_io_listen(NULL, confirm_cb, NULL, NULL, &err,
+ BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
+ BT_IO_OPT_CHANNEL, HFP_HF_CHANNEL,
+ BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
+ BT_IO_OPT_INVALID);
+ if (!hfp_hf_server) {
+ error("Failed to listen on Handsfree rfcomm: %s", err->message);
+ g_error_free(err);
+ return false;
+ }

hfp_hf_features = HFP_HF_FEATURES;

@@ -326,6 +492,12 @@ static bool enable_hf_client(void)

static void cleanup_hfp_hf(void)
{
+ if (hfp_hf_server) {
+ g_io_channel_shutdown(hfp_hf_server, TRUE, NULL);
+ g_io_channel_unref(hfp_hf_server);
+ hfp_hf_server = NULL;
+ }
+
if (hfp_hf_record_id > 0) {
bt_adapter_remove_record(hfp_hf_record_id);
hfp_hf_record_id = 0;
@@ -366,7 +538,7 @@ void bt_hf_client_unregister(void)

cleanup_hfp_hf();

- queue_destroy(devices, free);
+ queue_destroy(devices, device_destroy);
devices = NULL;

ipc_unregister(hal_ipc, HAL_SERVICE_ID_HANDSFREE);
--
1.8.4


2014-11-05 11:22:07

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH 3/7] android/handsfree-client: Add devices queue

This patch adds devices queue eventhough we are going to support only
one HF device at once
---
android/handsfree-client.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/android/handsfree-client.c b/android/handsfree-client.c
index b63aa13..aa3912b 100644
--- a/android/handsfree-client.c
+++ b/android/handsfree-client.c
@@ -35,6 +35,7 @@
#include "lib/sdp.h"
#include "lib/sdp_lib.h"
#include "src/sdp-client.h"
+#include "src/shared/queue.h"
#include "ipc.h"
#include "ipc-common.h"
#include "src/log.h"
@@ -69,6 +70,7 @@ static struct ipc *hal_ipc = NULL;

static uint32_t hfp_hf_features = 0;
static uint32_t hfp_hf_record_id = 0;
+static struct queue *devices = NULL;

static void handle_connect(const void *buf, uint16_t len)
{
@@ -334,16 +336,28 @@ bool bt_hf_client_register(struct ipc *ipc, const bdaddr_t *addr)
{
DBG("");

+ devices = queue_new();
+ if (!devices) {
+ error("hf-client: Could not create devices list");
+ goto failed;
+ }
+
bacpy(&adapter_addr, addr);

if (!enable_hf_client())
- return false;
+ goto failed;

hal_ipc = ipc;
ipc_register(hal_ipc, HAL_SERVICE_ID_HANDSFREE_CLIENT, cmd_handlers,
G_N_ELEMENTS(cmd_handlers));

return true;
+
+failed:
+ queue_destroy(devices, free);
+ devices = NULL;
+
+ return false;
}

void bt_hf_client_unregister(void)
@@ -352,6 +366,9 @@ void bt_hf_client_unregister(void)

cleanup_hfp_hf();

+ queue_destroy(devices, free);
+ devices = NULL;
+
ipc_unregister(hal_ipc, HAL_SERVICE_ID_HANDSFREE);
hal_ipc = NULL;
}
--
1.8.4


2014-11-05 11:22:06

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH 2/7] android/handsfree-client: Add service record

This patch adds service record for HFP 1.6 HF role

Note that we do not fix codec negotiation feature as this will be later
controled by android property
---
android/handsfree-client.c | 130 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 130 insertions(+)

diff --git a/android/handsfree-client.c b/android/handsfree-client.c
index 773ef76..b63aa13 100644
--- a/android/handsfree-client.c
+++ b/android/handsfree-client.c
@@ -32,18 +32,44 @@
#include <glib.h>

#include "lib/bluetooth.h"
+#include "lib/sdp.h"
+#include "lib/sdp_lib.h"
+#include "src/sdp-client.h"
#include "ipc.h"
#include "ipc-common.h"
#include "src/log.h"
#include "utils.h"

+#include "bluetooth.h"
#include "hal-msg.h"
#include "handsfree-client.h"

+#define HFP_HF_CHANNEL 7
+
+#define HFP_HF_FEAT_ECNR 0x00000001
+#define HFP_HF_FEAT_3WAY 0x00000002
+#define HFP_HF_FEAT_CLI 0x00000004
+#define HFP_HF_FEAT_VR 0x00000008
+#define HFP_HF_FEAT_RVC 0x00000010
+#define HFP_HF_FEAT_ECS 0x00000020
+#define HFP_HF_FEAT_ECC 0x00000040
+#define HFP_HF_FEAT_CODEC 0x00000080
+#define HFP_HF_FEAT_HF_IND 0x00000100
+#define HFP_HF_FEAT_ESCO_S4_T2 0x00000200
+
+
+#define HFP_HF_FEATURES (HFP_HF_FEAT_ECNR | HFP_HF_FEAT_3WAY |\
+ HFP_HF_FEAT_CLI | HFP_HF_FEAT_VR |\
+ HFP_HF_FEAT_RVC | HFP_HF_FEAT_ECS |\
+ HFP_HF_FEAT_ECC)
+
static bdaddr_t adapter_addr;

static struct ipc *hal_ipc = NULL;

+static uint32_t hfp_hf_features = 0;
+static uint32_t hfp_hf_record_id = 0;
+
static void handle_connect(const void *buf, uint16_t len)
{
DBG("Not Implemented");
@@ -205,12 +231,114 @@ static const struct ipc_handler cmd_handlers[] = {
{ handle_get_last_vc_tag_num, false, 0 },
};

+static sdp_record_t *hfp_hf_record(void)
+{
+ sdp_list_t *svclass_id, *pfseq, *apseq, *root;
+ uuid_t root_uuid, svclass_uuid, ga_svclass_uuid;
+ uuid_t l2cap_uuid, rfcomm_uuid;
+ sdp_profile_desc_t profile;
+ sdp_list_t *aproto, *proto[2];
+ sdp_record_t *record;
+ sdp_data_t *channel, *features;
+ uint16_t sdpfeat;
+ uint8_t ch = HFP_HF_CHANNEL;
+
+ record = sdp_record_alloc();
+ if (!record)
+ return NULL;
+
+ sdp_uuid16_create(&root_uuid, PUBLIC_BROWSE_GROUP);
+ root = sdp_list_append(NULL, &root_uuid);
+ sdp_set_browse_groups(record, root);
+
+ sdp_uuid16_create(&svclass_uuid, HANDSFREE_SVCLASS_ID);
+ svclass_id = sdp_list_append(NULL, &svclass_uuid);
+ sdp_uuid16_create(&ga_svclass_uuid, GENERIC_AUDIO_SVCLASS_ID);
+ svclass_id = sdp_list_append(svclass_id, &ga_svclass_uuid);
+ sdp_set_service_classes(record, svclass_id);
+
+ sdp_uuid16_create(&profile.uuid, HANDSFREE_PROFILE_ID);
+ profile.version = 0x0106;
+ pfseq = sdp_list_append(NULL, &profile);
+ sdp_set_profile_descs(record, pfseq);
+
+ sdp_uuid16_create(&l2cap_uuid, L2CAP_UUID);
+ proto[0] = sdp_list_append(NULL, &l2cap_uuid);
+ apseq = sdp_list_append(NULL, proto[0]);
+
+ sdp_uuid16_create(&rfcomm_uuid, RFCOMM_UUID);
+ proto[1] = sdp_list_append(NULL, &rfcomm_uuid);
+ channel = sdp_data_alloc(SDP_UINT8, &ch);
+ proto[1] = sdp_list_append(proto[1], channel);
+ apseq = sdp_list_append(apseq, proto[1]);
+
+ /* Codec Negotiation bit in SDP feature is different then in BRSF */
+ sdpfeat = hfp_hf_features & 0x0000003F;
+ if (hfp_hf_features & HFP_HF_FEAT_CODEC)
+ sdpfeat |= 0x00000020;
+ else
+ sdpfeat &= ~0x00000020;
+
+ features = sdp_data_alloc(SDP_UINT16, &sdpfeat);
+ sdp_attr_add(record, SDP_ATTR_SUPPORTED_FEATURES, features);
+
+ aproto = sdp_list_append(NULL, apseq);
+ sdp_set_access_protos(record, aproto);
+
+ sdp_set_info_attr(record, "Hands-Free unit", NULL, NULL);
+
+ sdp_data_free(channel);
+ sdp_list_free(proto[0], NULL);
+ sdp_list_free(proto[1], NULL);
+ sdp_list_free(apseq, NULL);
+ sdp_list_free(pfseq, NULL);
+ sdp_list_free(aproto, NULL);
+ sdp_list_free(root, NULL);
+ sdp_list_free(svclass_id, NULL);
+
+ return record;
+}
+
+static bool enable_hf_client(void)
+{
+ sdp_record_t *rec;
+
+ hfp_hf_features = HFP_HF_FEATURES;
+
+ rec = hfp_hf_record();
+ if (!rec) {
+ error("hf-client: Could not create service record");
+ return false;
+ }
+
+ if (bt_adapter_add_record(rec, 0) < 0) {
+ error("hf-client: Failed to register service record");
+ sdp_record_free(rec);
+ return false;
+ }
+
+ hfp_hf_record_id = rec->handle;
+
+ return true;
+}
+
+static void cleanup_hfp_hf(void)
+{
+ if (hfp_hf_record_id > 0) {
+ bt_adapter_remove_record(hfp_hf_record_id);
+ hfp_hf_record_id = 0;
+ }
+}
+
bool bt_hf_client_register(struct ipc *ipc, const bdaddr_t *addr)
{
DBG("");

bacpy(&adapter_addr, addr);

+ if (!enable_hf_client())
+ return false;
+
hal_ipc = ipc;
ipc_register(hal_ipc, HAL_SERVICE_ID_HANDSFREE_CLIENT, cmd_handlers,
G_N_ELEMENTS(cmd_handlers));
@@ -222,6 +350,8 @@ void bt_hf_client_unregister(void)
{
DBG("");

+ cleanup_hfp_hf();
+
ipc_unregister(hal_ipc, HAL_SERVICE_ID_HANDSFREE);
hal_ipc = NULL;
}
--
1.8.4


2014-11-05 11:22:05

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH 1/7] android/handsfree-client: Typo fix in define name

---
android/hal-msg.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/android/hal-msg.h b/android/hal-msg.h
index bcb73b2..651865e 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -1059,7 +1059,7 @@ struct hal_cmd_hf_client_dial_memory {
#define HAL_HF_CLIENT_ACTION_CHUP 0x08
#define HAL_HF_CLIENT_ACTION_BRTH_0 0x09
#define HAL_HF_CLIENT_ACTION_BRTH_1 0x0a
-#define HAL_HF_CLIENT_ACTION_BRTH_02 0x0b
+#define HAL_HF_CLIENT_ACTION_BRTH_2 0x0b

#define HAL_OP_HF_CLIENT_CALL_ACTION 0x0a
struct hal_cmd_hf_client_call_action {
--
1.8.4