2013-12-20 18:30:07

by Szymon Janc

[permalink] [raw]
Subject: [PATCH 1/4] android/bluetooth: Add support for timestamp device property

This allows to handle timestamp property request. Also this will be
usefull for devices info cache (clearing old devices).
---
android/bluetooth.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index e534fc1..2e75864 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -80,6 +80,8 @@ struct device {
uint32_t class;
int32_t rssi;

+ uint32_t timestamp;
+
GSList *uuids;
};

@@ -227,6 +229,8 @@ static void store_device_info(struct device *dev)
else
g_key_file_remove_key(key_file, addr, "Class", NULL);

+ g_key_file_set_integer(key_file, addr, "Timestamp", dev->timestamp);
+
if (dev->uuids) {
GSList *l;
int i;
@@ -291,6 +295,7 @@ static struct device *create_device(const bdaddr_t *bdaddr, uint8_t type)
bacpy(&dev->bdaddr, bdaddr);
dev->bdaddr_type = type;
dev->bond_state = HAL_BOND_STATE_NONE;
+ dev->timestamp = time(NULL);

/* use address for name, will be change if one is present
* eg. in EIR or set by set_property. */
@@ -1004,6 +1009,8 @@ static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,

ev->status = HAL_STATUS_SUCCESS;
bdaddr2android(bdaddr, ev->bdaddr);
+
+ dev->timestamp = time(NULL);
}

if (eir.class) {
@@ -1607,6 +1614,9 @@ static void create_device_from_info(GKeyFile *key_file, const char *peer)

dev->class = g_key_file_get_integer(key_file, peer, "Class", NULL);

+ dev->timestamp = g_key_file_get_integer(key_file, peer, "Timestamp",
+ NULL);
+
uuids = g_key_file_get_string_list(key_file, peer, "Services", NULL,
NULL);
if (uuids) {
@@ -2647,11 +2657,10 @@ static uint8_t get_device_version_info(struct device *dev)

static uint8_t get_device_timestamp(struct device *dev)
{
- DBG("Not implemented");
-
- /* TODO */
+ send_device_property(&dev->bdaddr, HAL_PROP_DEVICE_TIMESTAMP,
+ sizeof(dev->timestamp), &dev->timestamp);

- return HAL_STATUS_FAILED;
+ return HAL_STATUS_SUCCESS;
}

static void handle_get_remote_device_props_cmd(const void *buf, uint16_t len)
--
1.8.3.2



2013-12-23 14:14:33

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [PATCH 2/4] android/bluetooth: Add support for DUT mode configure command

Hi Marcel,

On Mon, Dec 23, 2013 at 05:55:28AM -0800, Marcel Holtmann wrote:
> Hi Andrei,
>
> >> This allows to enable and disable DUT mode.
> >
> > Hi Szymon, could you add some more info about the mode?
>
> please refer to the Bluetooth Core specification that explain Device Under Test (DUT) mode. It is a commonly used term. There is really no need to go all out and explain it here.
>

I was referring mostly to /sys/ entry in the kernel.

Best regards
Andrei Emeltchenko

2013-12-23 13:55:28

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 2/4] android/bluetooth: Add support for DUT mode configure command

Hi Andrei,

>> This allows to enable and disable DUT mode.
>
> Hi Szymon, could you add some more info about the mode?

please refer to the Bluetooth Core specification that explain Device Under Test (DUT) mode. It is a commonly used term. There is really no need to go all out and explain it here.

Regards

Marcel


2013-12-23 09:46:39

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 1/4] android/bluetooth: Add support for timestamp device property

Hi Szymon,

On Mon, Dec 23, 2013 at 9:53 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Szymon,
>
> On Fri, Dec 20, 2013 at 8:30 PM, Szymon Janc <[email protected]> wrote:
>> This allows to handle timestamp property request. Also this will be
>> usefull for devices info cache (clearing old devices).
>> ---
>> android/bluetooth.c | 17 +++++++++++++----
>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/android/bluetooth.c b/android/bluetooth.c
>> index e534fc1..2e75864 100644
>> --- a/android/bluetooth.c
>> +++ b/android/bluetooth.c
>> @@ -80,6 +80,8 @@ struct device {
>> uint32_t class;
>> int32_t rssi;
>>
>> + uint32_t timestamp;
>> +
>> GSList *uuids;
>> };
>>
>> @@ -227,6 +229,8 @@ static void store_device_info(struct device *dev)
>> else
>> g_key_file_remove_key(key_file, addr, "Class", NULL);
>>
>> + g_key_file_set_integer(key_file, addr, "Timestamp", dev->timestamp);
>> +
>> if (dev->uuids) {
>> GSList *l;
>> int i;
>> @@ -291,6 +295,7 @@ static struct device *create_device(const bdaddr_t *bdaddr, uint8_t type)
>> bacpy(&dev->bdaddr, bdaddr);
>> dev->bdaddr_type = type;
>> dev->bond_state = HAL_BOND_STATE_NONE;
>> + dev->timestamp = time(NULL);
>>
>> /* use address for name, will be change if one is present
>> * eg. in EIR or set by set_property. */
>> @@ -1004,6 +1009,8 @@ static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
>>
>> ev->status = HAL_STATUS_SUCCESS;
>> bdaddr2android(bdaddr, ev->bdaddr);
>> +
>> + dev->timestamp = time(NULL);
>> }
>>
>> if (eir.class) {
>> @@ -1607,6 +1614,9 @@ static void create_device_from_info(GKeyFile *key_file, const char *peer)
>>
>> dev->class = g_key_file_get_integer(key_file, peer, "Class", NULL);
>>
>> + dev->timestamp = g_key_file_get_integer(key_file, peer, "Timestamp",
>> + NULL);
>> +
>> uuids = g_key_file_get_string_list(key_file, peer, "Services", NULL,
>> NULL);
>> if (uuids) {
>> @@ -2647,11 +2657,10 @@ static uint8_t get_device_version_info(struct device *dev)
>>
>> static uint8_t get_device_timestamp(struct device *dev)
>> {
>> - DBG("Not implemented");
>> -
>> - /* TODO */
>> + send_device_property(&dev->bdaddr, HAL_PROP_DEVICE_TIMESTAMP,
>> + sizeof(dev->timestamp), &dev->timestamp);
>>
>> - return HAL_STATUS_FAILED;
>> + return HAL_STATUS_SUCCESS;
>> }
>>
>
> I guess it probably make sense to update the timestamp also whenever a
> connection completes or does bluedroid ignores it?

As discussed in irc this seems to be way HAL is defined, so Im
applying this one and patch 4, patches 2 and 3 needs some changes so
Im expecting a v2 soon.


--
Luiz Augusto von Dentz

2013-12-23 08:46:09

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [PATCH 2/4] android/bluetooth: Add support for DUT mode configure command

On Fri, Dec 20, 2013 at 07:30:08PM +0100, Szymon Janc wrote:
> This allows to enable and disable DUT mode.

Hi Szymon, could you add some more info about the mode?

Best regards
Andrei Emeltchenko

> ---
> android/bluetooth.c | 29 ++++++++++++++++++++++++++---
> 1 file changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/android/bluetooth.c b/android/bluetooth.c
> index 2e75864..59f0810 100644
> --- a/android/bluetooth.c
> +++ b/android/bluetooth.c
> @@ -50,6 +50,8 @@
> #include "utils.h"
> #include "bluetooth.h"
>
> +#define DUT_MODE_FILE "/sys/kernel/debug/bluetooth/hci%u/dut_mode"
> +
> #define DEVICE_ID_SOURCE 0x0002 /* USB */
> #define DEVICE_ID_VENDOR 0x1d6b /* Linux Foundation */
> #define DEVICE_ID_PRODUCT 0x0247 /* BlueZ for Android */
> @@ -2872,13 +2874,34 @@ failed:
> static void handle_dut_mode_conf_cmd(const void *buf, uint16_t len)
> {
> const struct hal_cmd_dut_mode_conf *cmd = buf;
> + char path[FILENAME_MAX];
> + uint8_t status;
> + int fd, ret;
>
> DBG("enable %u", cmd->enable);
>
> - /* TODO */
> + snprintf(path, sizeof(path), DUT_MODE_FILE, adapter.index);
>
> - ipc_send_rsp(HAL_SERVICE_ID_BLUETOOTH, HAL_OP_DUT_MODE_CONF,
> - HAL_STATUS_FAILED);
> + fd = open(path, O_WRONLY);
> + if (fd < 0) {
> + status = HAL_STATUS_FAILED;
> + goto failed;
> + }
> +
> + if (cmd->enable)
> + ret = write(fd, "1", sizeof("1"));
> + else
> + ret = write(fd, "0", sizeof("0"));
> +
> + if (ret < 0)
> + status = HAL_STATUS_FAILED;
> + else
> + status = HAL_STATUS_SUCCESS;
> +
> + close(fd);
> +
> +failed:
> + ipc_send_rsp(HAL_SERVICE_ID_BLUETOOTH, HAL_OP_DUT_MODE_CONF, status);
> }
>
> static void handle_dut_mode_send_cmd(const void *buf, uint16_t len)
> --
> 1.8.3.2
>
> --
> 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

2013-12-23 07:53:18

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 1/4] android/bluetooth: Add support for timestamp device property

Hi Szymon,

On Fri, Dec 20, 2013 at 8:30 PM, Szymon Janc <[email protected]> wrote:
> This allows to handle timestamp property request. Also this will be
> usefull for devices info cache (clearing old devices).
> ---
> android/bluetooth.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/android/bluetooth.c b/android/bluetooth.c
> index e534fc1..2e75864 100644
> --- a/android/bluetooth.c
> +++ b/android/bluetooth.c
> @@ -80,6 +80,8 @@ struct device {
> uint32_t class;
> int32_t rssi;
>
> + uint32_t timestamp;
> +
> GSList *uuids;
> };
>
> @@ -227,6 +229,8 @@ static void store_device_info(struct device *dev)
> else
> g_key_file_remove_key(key_file, addr, "Class", NULL);
>
> + g_key_file_set_integer(key_file, addr, "Timestamp", dev->timestamp);
> +
> if (dev->uuids) {
> GSList *l;
> int i;
> @@ -291,6 +295,7 @@ static struct device *create_device(const bdaddr_t *bdaddr, uint8_t type)
> bacpy(&dev->bdaddr, bdaddr);
> dev->bdaddr_type = type;
> dev->bond_state = HAL_BOND_STATE_NONE;
> + dev->timestamp = time(NULL);
>
> /* use address for name, will be change if one is present
> * eg. in EIR or set by set_property. */
> @@ -1004,6 +1009,8 @@ static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
>
> ev->status = HAL_STATUS_SUCCESS;
> bdaddr2android(bdaddr, ev->bdaddr);
> +
> + dev->timestamp = time(NULL);
> }
>
> if (eir.class) {
> @@ -1607,6 +1614,9 @@ static void create_device_from_info(GKeyFile *key_file, const char *peer)
>
> dev->class = g_key_file_get_integer(key_file, peer, "Class", NULL);
>
> + dev->timestamp = g_key_file_get_integer(key_file, peer, "Timestamp",
> + NULL);
> +
> uuids = g_key_file_get_string_list(key_file, peer, "Services", NULL,
> NULL);
> if (uuids) {
> @@ -2647,11 +2657,10 @@ static uint8_t get_device_version_info(struct device *dev)
>
> static uint8_t get_device_timestamp(struct device *dev)
> {
> - DBG("Not implemented");
> -
> - /* TODO */
> + send_device_property(&dev->bdaddr, HAL_PROP_DEVICE_TIMESTAMP,
> + sizeof(dev->timestamp), &dev->timestamp);
>
> - return HAL_STATUS_FAILED;
> + return HAL_STATUS_SUCCESS;
> }
>

I guess it probably make sense to update the timestamp also whenever a
connection completes or does bluedroid ignores it?

Luiz Augusto von Dentz

2013-12-20 18:30:10

by Szymon Janc

[permalink] [raw]
Subject: [PATCH 4/4] android: Add shortcommings section to README

This sections lists unimplemented methods, callbacks or properties
with few words of comments why feature is missing.
---
android/README | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)

diff --git a/android/README b/android/README
index 68c3e9f..6d0e242 100644
--- a/android/README
+++ b/android/README
@@ -91,3 +91,40 @@ use provided android/system-emulator, which takes care of launching daemon
automatically on HAL library initialization. To deinitialize HAL library and
stop daemon type 'bluetooth cleanup'. Type 'help' for more information. Tab
completion is also supported.
+
+===========================
+Implementation shortcomings
+===========================
+
+It is possible that some of HAL functionality is missing implementation due to
+reasons like feature feasibility or necessity for latest Android Framework.
+This sections provides list of such deficiencies. Note that HAL library is
+always expected to fully implement HAL API so missing implementation might
+happen only in daemon.
+
+HAL Bluetooth
+=============
+methods:
+dut_mode_send never called from Android Framework
+le_test_mode never called from Android Framework
+get_remote_service_record never called from Android Framework
+
+callbacks:
+dut_mode_recv_cb
+le_test_mode_cb
+
+properties:
+BT_PROPERTY_SERVICE_RECORD not supported for adapter and device, for
+ device this property is to be returned as
+ response to get_remote_service_record,
+ not sure what to return on get_property
+ calls (records of all services?)
+
+BT_PROPERTY_REMOTE_VERSION_INFO information required by this property (LMP
+ information) are not accessible from mgmt
+ interface, also marking this property as
+ settable is probably a typo in HAL header
+
+Socket HAL
+==========
+Support only for BTSOCK_RFCOMM socket type.
--
1.8.3.2


2013-12-20 18:30:08

by Szymon Janc

[permalink] [raw]
Subject: [PATCH 2/4] android/bluetooth: Add support for DUT mode configure command

This allows to enable and disable DUT mode.
---
android/bluetooth.c | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 2e75864..59f0810 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -50,6 +50,8 @@
#include "utils.h"
#include "bluetooth.h"

+#define DUT_MODE_FILE "/sys/kernel/debug/bluetooth/hci%u/dut_mode"
+
#define DEVICE_ID_SOURCE 0x0002 /* USB */
#define DEVICE_ID_VENDOR 0x1d6b /* Linux Foundation */
#define DEVICE_ID_PRODUCT 0x0247 /* BlueZ for Android */
@@ -2872,13 +2874,34 @@ failed:
static void handle_dut_mode_conf_cmd(const void *buf, uint16_t len)
{
const struct hal_cmd_dut_mode_conf *cmd = buf;
+ char path[FILENAME_MAX];
+ uint8_t status;
+ int fd, ret;

DBG("enable %u", cmd->enable);

- /* TODO */
+ snprintf(path, sizeof(path), DUT_MODE_FILE, adapter.index);

- ipc_send_rsp(HAL_SERVICE_ID_BLUETOOTH, HAL_OP_DUT_MODE_CONF,
- HAL_STATUS_FAILED);
+ fd = open(path, O_WRONLY);
+ if (fd < 0) {
+ status = HAL_STATUS_FAILED;
+ goto failed;
+ }
+
+ if (cmd->enable)
+ ret = write(fd, "1", sizeof("1"));
+ else
+ ret = write(fd, "0", sizeof("0"));
+
+ if (ret < 0)
+ status = HAL_STATUS_FAILED;
+ else
+ status = HAL_STATUS_SUCCESS;
+
+ close(fd);
+
+failed:
+ ipc_send_rsp(HAL_SERVICE_ID_BLUETOOTH, HAL_OP_DUT_MODE_CONF, status);
}

static void handle_dut_mode_send_cmd(const void *buf, uint16_t len)
--
1.8.3.2


2013-12-20 18:30:09

by Szymon Janc

[permalink] [raw]
Subject: [PATCH 3/4] android/bluetooth: Print error on unimplemented functions

Functions or callbacks that are not implemented due to being bogus or
not feasible now prints error messages instead of debugs.
---
android/bluetooth.c | 22 +++++++---------------
1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 59f0810..5598ad8 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -2059,9 +2059,7 @@ static uint8_t get_adapter_type(void)

static uint8_t get_adapter_service_rec(void)
{
- DBG("Not implemented");
-
- /* TODO: Add implementation */
+ error("get adapter BT_PROPERTY_SERVICE_RECORD not supported");

return HAL_STATUS_FAILED;
}
@@ -2619,9 +2617,7 @@ static uint8_t get_device_type(struct device *dev)

static uint8_t get_device_service_rec(struct device *dev)
{
- DBG("Not implemented");
-
- /* TODO */
+ error("get device BT_PROPERTY_SERVICE_RECORD not supported");

return HAL_STATUS_FAILED;
}
@@ -2650,9 +2646,7 @@ static uint8_t get_device_rssi(struct device *dev)

static uint8_t get_device_version_info(struct device *dev)
{
- DBG("Not implemented");
-
- /* TODO */
+ error("get device BT_PROPERTY_REMOTE_VERSION_INFO not supported");

return HAL_STATUS_FAILED;
}
@@ -2768,9 +2762,7 @@ static uint8_t set_device_friendly_name(struct device *dev, const uint8_t *val,

static uint8_t set_device_version_info(struct device *dev)
{
- DBG("Not implemented");
-
- /* TODO */
+ error("set device BT_PROPERTY_REMOTE_VERSION_INFO not supported");

return HAL_STATUS_FAILED;
}
@@ -2816,7 +2808,7 @@ failed:

static void handle_get_remote_service_rec_cmd(const void *buf, uint16_t len)
{
- /* TODO */
+ error("get_remote_service_record not supported");

ipc_send_rsp(HAL_SERVICE_ID_BLUETOOTH, HAL_OP_GET_REMOTE_SERVICE_REC,
HAL_STATUS_FAILED);
@@ -2914,7 +2906,7 @@ static void handle_dut_mode_send_cmd(const void *buf, uint16_t len)
return;
}

- DBG("opcode %u", cmd->opcode);
+ error("dut_mode_send not supported");

/* TODO */

@@ -2932,7 +2924,7 @@ static void handle_le_test_mode_cmd(const void *buf, uint16_t len)
return;
}

- DBG("opcode %u", cmd->opcode);
+ error("le_test_mode not supported");

/* TODO */

--
1.8.3.2