Re: [PATCH] HCI Commands for LE White List

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

Hi Sumit,

Could you please split this patch into minor functional patches?

On Thu, Jan 6, 2011 at 12:52 AM, Sumit Kumar BAJPAI
<sumitkumar.bajpai@xxxxxxxxxxxxxx> wrote:
> Added HCI commands for LE White List support.
> These LE White List Commands can be tested from
> hcitool.
>
>
> ---
> Âlib/hci.c    | Â107 +++++++++++++++++++++++++++++++
> Âlib/hci_lib.h  |  Â4 +
> Âtools/hcitool.c | Â186 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> Â3 files changed, 297 insertions(+), 0 deletions(-)
> Âmode change 100644 => 100755 lib/hci.c
> Âmode change 100644 => 100755 lib/hci_lib.h
>
> diff --git a/lib/hci.c b/lib/hci.c
> old mode 100644
> new mode 100755
> index 048fda4..5a0275c
> --- a/lib/hci.c
> +++ b/lib/hci.c
> @@ -1291,6 +1291,113 @@ int hci_disconnect(int dd, uint16_t handle, uint8_t reason, int to)
> Â Â Â Âreturn 0;
> Â}
>
> +int hci_le_add_to_white_list(int dd, bdaddr_t addr, uint8_t type)
Use pointer for bdaddr_t
address types have defined values in the BT spec, declare an enum in hci.h

> +{
> + Â Â Â struct hci_request rq;
> + Â Â Â le_add_device_to_white_list_cp param_cp;
It seems that you copied this declaration from set scan parameters
function. Use only "cp", it is short for command parameters.

> + Â Â Â uint8_t status;
> +
> + Â Â Â memset(&param_cp, 0, sizeof(param_cp));
> + Â Â Â param_cp.bdaddr_type = type;
> + Â Â Â param_cp.bdaddr= addr;
> +
> + Â Â Â memset(&rq, 0, sizeof(rq));
> + Â Â Â rq.ogf = OGF_LE_CTL;
> + Â Â Â rq.ocf = OCF_LE_ADD_DEVICE_TO_WHITE_LIST;
> + Â Â Â rq.cparam = &param_cp;
> + Â Â Â rq.clen = LE_ADD_DEVICE_TO_WHITE_LIST_CP_SIZE;
> + Â Â Â rq.rparam = &status;
> + Â Â Â rq.rlen = 1;
> +
> + Â Â Â if (hci_send_req(dd, &rq, 100) < 0)
> + Â Â Â Â Â Â Â return -1;
Use 1000, we are noticing timeout on some LE hardwares.

> +
> + Â Â Â if (status) {
> + Â Â Â Â Â Â Â errno = EIO;
> + Â Â Â Â Â Â Â return -1;
> + Â Â Â }
> +
> + Â Â Â return 0;
> +}
> +
> +int hci_le_remove_from_white_list(int dd, bdaddr_t addr, uint8_t type)
> +{
Same comment about address type and bdaddr_t pointer

> + Â Â Â struct hci_request rq;
> + Â Â Â le_remove_device_from_white_list_cp param_cp;
Same comment about param_cp: s/param_cp/cp

> + Â Â Â uint8_t status;
> +
> + Â Â Â memset(&param_cp, 0, sizeof(param_cp));
> + Â Â Â param_cp.bdaddr_type = type;
> + Â Â Â param_cp.bdaddr= addr;
> +
> + Â Â Â memset(&rq, 0, sizeof(rq));
> + Â Â Â rq.ogf = OGF_LE_CTL;
> + Â Â Â rq.ocf = OCF_LE_REMOVE_DEVICE_FROM_WHITE_LIST;
> + Â Â Â rq.cparam = &param_cp;
> + Â Â Â rq.clen = LE_REMOVE_DEVICE_FROM_WHITE_LIST_CP_SIZE;
> + Â Â Â rq.rparam = &status;
> + Â Â Â rq.rlen = 1;
> +
> + Â Â Â if (hci_send_req(dd, &rq, 100) < 0)
> + Â Â Â Â Â Â Â return -1;
Use 1000

> +
> + Â Â Â if (status) {
> + Â Â Â Â Â Â Â errno = EIO;
> + Â Â Â Â Â Â Â return -1;
> + Â Â Â }
> +
> + Â Â Â return 0;
> +}
> +
> +int hci_le_read_white_list_size(int dd)
> +{
> + Â Â Â struct hci_request rq;
> + Â Â Â le_read_white_list_size_rp param_cp;
Rename param_cp to rp

> +
> + Â Â Â memset(&param_cp, 0, sizeof(param_cp));
> + Â Â Â param_cp.size=0;
> +
> + Â Â Â memset(&rq, 0, sizeof(rq));
> + Â Â Â rq.ogf = OGF_LE_CTL;
> + Â Â Â rq.ocf = OCF_LE_READ_WHITE_LIST_SIZE;
> + Â Â Â rq.rparam = &param_cp;
> + Â Â Â rq.rlen = LE_READ_WHITE_LIST_SIZE_RP_SIZE;
> +
> + Â Â Â if (hci_send_req(dd, &rq, 100) < 0)
> + Â Â Â Â Â Â Â return -1;
> +
> + Â Â Â if (param_cp.status) {
> + Â Â Â Â Â Â Â errno = EIO;
> + Â Â Â Â Â Â Â return -1;
> + Â Â Â }
> +
> + Â Â Â printf("LE White list size= %d\n", param_cp.size);
printf? the size should be returned to the caller.
Suggestion: add a pointer in the function parameters
if (size)
    *size = param_cp.size;

> +
> + Â Â Â return 0;
> +}
> +
> +int hci_le_clear_white_list(int dd)
> +{
> + Â Â Â struct hci_request rq;
> + Â Â Â uint8_t status;
> +
> + Â Â Â memset(&rq, 0, sizeof(rq));
> + Â Â Â rq.ogf = OGF_LE_CTL;
> + Â Â Â rq.ocf = OCF_LE_CLEAR_WHITE_LIST;
> + Â Â Â rq.rparam = &status;
> + Â Â Â rq.rlen = 1;
> +
> + Â Â Â if (hci_send_req(dd, &rq, 100) < 0)
Use 1000

> + Â Â Â Â Â Â Â return -1;
> +
> + Â Â Â if (status) {
> + Â Â Â Â Â Â Â errno = EIO;
> + Â Â Â Â Â Â Â return -1;
> + Â Â Â }
> +
> + Â Â Â return 0;
> +}
> +
> Âint hci_read_local_name(int dd, int len, char *name, int to)
> Â{
> Â Â Â Âread_local_name_rp rp;
> diff --git a/lib/hci_lib.h b/lib/hci_lib.h
> old mode 100644
> new mode 100755
> index b63a2a4..ed74dfc
> --- a/lib/hci_lib.h
> +++ b/lib/hci_lib.h
> @@ -127,6 +127,10 @@ int hci_le_create_conn(int dd, uint16_t interval, uint16_t window,
> Â Â Â Â Â Â Â Âuint16_t latency, uint16_t supervision_timeout,
> Â Â Â Â Â Â Â Âuint16_t min_ce_length, uint16_t max_ce_length,
> Â Â Â Â Â Â Â Âuint16_t *handle, int to);
> +int hci_le_add_to_white_list(int dd, bdaddr_t addr, uint8_t type);
> +int hci_le_remove_from_white_list(int dd, bdaddr_t addr, uint8_t type);
> +int hci_le_read_white_list_size(int dd);
> +int hci_le_clear_white_list(int dd);
>
> Âint hci_for_each_dev(int flag, int(*func)(int dd, int dev_id, long arg), long arg);
> Âint hci_get_route(bdaddr_t *bdaddr);
> diff --git a/tools/hcitool.c b/tools/hcitool.c
> index d50adaf..387c47c 100644
> --- a/tools/hcitool.c
> +++ b/tools/hcitool.c
> @@ -2471,6 +2471,188 @@ static void cmd_lecc(int dev_id, int argc, char **argv)
> Â Â Â Âhci_close_dev(dd);
> Â}
>
> +static struct option leaddwl_options[] = {
> + Â Â Â { "help", Â Â Â 0, 0, 'h' },
> + Â Â Â { 0, 0, 0, 0 }
> +};
> +
> +static const char *leaddwl_help =
> + Â Â Â "Usage:\n"
> + Â Â Â "\tleaddwl <bdaddr>\n";
> +
> +static void cmd_leaddwl(int dev_id, int argc,char **argv)
> +{
> + Â Â Â int err, opt, dd;
> + Â Â Â bdaddr_t bdaddr;
> + Â Â Â uint8_t bdaddr_type;
> +
> + Â Â Â for_each_opt(opt, leaddwl_options, NULL) {
> + Â Â Â Â Â Â Â switch (opt) {
> + Â Â Â Â Â Â Â default:
> + Â Â Â Â Â Â Â Â Â Â Â printf("%s", leaddwl_help);
> + Â Â Â Â Â Â Â Â Â Â Â return;
> + Â Â Â Â Â Â Â }
> + Â Â Â }
> + Â Â Â helper_arg(1, 1, &argc, &argv, leaddwl_help);
> +
> + Â Â Â if (dev_id < 0)
> + Â Â Â Â Â Â Â dev_id = hci_get_route(NULL);
> +
> + Â Â Â dd = hci_open_dev(dev_id);
> + Â Â Â if (dd < 0) {
> + Â Â Â Â Â Â Â perror("Could not open device");
> + Â Â Â Â Â Â Â exit(1);
> + Â Â Â }
> +
> + Â Â Â str2ba(argv[1], &bdaddr);
> + Â Â Â bdaddr_type = 0x00;
Use enum

> +
> + Â Â Â err = hci_le_add_to_white_list(dd, bdaddr ,bdaddr_type);
Missing space before bdaddr_type

> + Â Â Â if (err < 0) {
> + Â Â Â Â Â Â Â perror("Cant add to white list");
> + Â Â Â Â Â Â Â exit(1);
> + Â Â Â }
wrong coding style

> + Â Â Â else {
> + Â Â Â Â Â Â Â printf("Device added to white list");
> + Â Â Â }
"{" not necessary

> +
> + Â Â Â hci_close_dev(dd);
Move this line after "hci_le_add_to_white_list()" call, otherwise dd
will not be closed if the cmd fails

> +
remove empty line
> +}
> +
> +static struct option lermwl_options[] = {
> + Â Â Â { "help", Â Â Â 0, 0, 'h' },
> + Â Â Â { 0, 0, 0, 0 }
> +};
> +
> +static const char *lermwl_help =
> + Â Â Â "Usage:\n"
> + Â Â Â "\tlermwl <bdaddr>\n";
> +
> +static void cmd_lermwl(int dev_id, int argc,char **argv)
> +{
> + Â Â Â int err, opt, dd;
> + Â Â Â bdaddr_t bdaddr;
> + Â Â Â uint8_t bdaddr_type;
> +
> + Â Â Â for_each_opt(opt, lermwl_options, NULL) {
> + Â Â Â Â Â Â Â switch (opt) {
> + Â Â Â Â Â Â Â default:
> + Â Â Â Â Â Â Â Â Â Â Â printf("%s", lermwl_help);
> + Â Â Â Â Â Â Â Â Â Â Â return;
> + Â Â Â Â Â Â Â }
> + Â Â Â }
> + Â Â Â helper_arg(1, 1, &argc, &argv, lermwl_help);
> +
> + Â Â Â if (dev_id < 0)
> + Â Â Â Â Â Â Â dev_id = hci_get_route(NULL);
> +
> + Â Â Â dd = hci_open_dev(dev_id);
> + Â Â Â if (dd < 0) {
> + Â Â Â Â Â Â Â perror("Could not open device");
> + Â Â Â Â Â Â Â exit(1);
> + Â Â Â }
> +
> + Â Â Â str2ba(argv[1], &bdaddr);
> + Â Â Â bdaddr_type = 0x00;
> +
> + Â Â Â err = hci_le_remove_from_white_list(dd, bdaddr ,bdaddr_type);
> + Â Â Â if (err < 0) {
> + Â Â Â Â Â Â Â perror("Cant remove from white list");
> + Â Â Â Â Â Â Â exit(1);
> + Â Â Â }
> + Â Â Â Âelse {
> + Â Â Â Â Â Â Â printf("Device removed from white list");
> + Â Â Â }
> +
> + Â Â Â hci_close_dev(dd);
Same coding style issue and close "dd"

> +
> +}
> +
> +static struct option lerdwlsz_options[] = {
> + Â Â Â { "help", Â Â Â 0, 0, 'h' },
> + Â Â Â { 0, 0, 0, 0 }
> +};
> +
> +static const char *lerdwlsz_help =
> + Â Â Â "Usage:\n"
> + Â Â Â "\tlerdwlsz\n";
> +
> +static void cmd_lerdwlsz(int dev_id, int argc,char **argv)
> +{
> + Â Â Â int err,dd, opt;
> +
> + Â Â Â for_each_opt(opt, lerdwlsz_options, NULL) {
> + Â Â Â Â Â Â Â switch (opt) {
> + Â Â Â Â Â Â Â default:
> + Â Â Â Â Â Â Â Â Â Â Â printf("%s", lerdwlsz_help);
> + Â Â Â Â Â Â Â Â Â Â Â return;
> + Â Â Â Â Â Â Â }
> + Â Â Â }
Add empty line here
> + Â Â Â helper_arg(0, 0, &argc, &argv, lermwl_help);
> +
> + Â Â Â if (dev_id < 0)
> + Â Â Â Â Â Â Â dev_id = hci_get_route(NULL);
> +
> + Â Â Â dd = hci_open_dev(dev_id);
> + Â Â Â if (dd < 0) {
> + Â Â Â Â Â Â Â perror("Could not open device");
> + Â Â Â Â Â Â Â exit(1);
> + Â Â Â }
> +
> + Â Â Â err = hci_le_read_white_list_size(dd);
> + Â Â Â if (err < 0) {
> + Â Â Â Â Â Â Â perror("Cant read white list size");
> + Â Â Â Â Â Â Â exit(1);
> + Â Â Â }
> +
> + Â Â Â hci_close_dev(dd);
Missing close dd if the cmd fails
Remove empty line here
> +
> +}
> +
> +
Remove empty line here
> +static struct option leclrwl_options[] = {
> + Â Â Â { "help", Â Â Â 0, 0, 'h' },
> + Â Â Â { 0, 0, 0, 0 }
> +};
> +
> +static const char *leclrwl_help =
> + Â Â Â "Usage:\n"
> + Â Â Â "\tleclrwl\n";
> +
> +static void cmd_leclrwl(int dev_id, int argc,char **argv)
> +{
> + Â Â Â int err,dd, opt;
> +
> + Â Â Â for_each_opt(opt, leclrwl_options, NULL) {
> + Â Â Â Â Â Â Â switch (opt) {
> + Â Â Â Â Â Â Â default:
> + Â Â Â Â Â Â Â Â Â Â Â printf("%s", leclrwl_help);
> + Â Â Â Â Â Â Â Â Â Â Â return;
> + Â Â Â Â Â Â Â }
> + Â Â Â }
> + Â Â Â helper_arg(0, 0, &argc, &argv, leclrwl_help);
> +
> + Â Â Â if (dev_id < 0)
> + Â Â Â Â Â Â Â dev_id = hci_get_route(NULL);
> +
> + Â Â Â dd = hci_open_dev(dev_id);
> + Â Â Â if (dd < 0) {
> + Â Â Â Â Â Â Â perror("Could not open device");
> + Â Â Â Â Â Â Â exit(1);
> + Â Â Â }
> +
> + Â Â Â err = hci_le_clear_white_list(dd);
> + Â Â Â if (err < 0) {
> + Â Â Â Â Â Â Â perror("Cant clear white list");
> + Â Â Â Â Â Â Â exit(1);
> + Â Â Â }
> +
> + Â Â Â hci_close_dev(dd);
Missing close dd when the cmd fails
Remove empty line here

Claudio
> +
> +}
> +
> +
Remove empty line here
> Âstatic struct option ledc_options[] = {
> Â Â Â Â{ "help", Â Â Â 0, 0, 'h' },
> Â Â Â Â{ 0, 0, 0, 0 }
> @@ -2547,6 +2729,10 @@ static struct {
> Â Â Â Â{ "clkoff", cmd_clkoff, "Read clock offset" Â Â Â Â Â Â Â Â Â Â},
> Â Â Â Â{ "clock", Âcmd_clock, Â"Read local or remote clock" Â Â Â Â Â },
> Â Â Â Â{ "lescan", cmd_lescan, "Start LE scan" Â Â Â Â Â Â Â Â Â Â Â Â},
> + Â Â Â { "leaddwl", cmd_leaddwl, "Add this device to white list" Â Â Â Â Â},
> + Â Â Â { "lermwl", cmd_lermwl, "Remove this device from white list" Â },
> + Â Â Â { "lerdwlsz", Âcmd_lerdwlsz, Â"Read white list size" Â Â Â Â Â Â Â },
> + Â Â Â { "leclrwl", Â cmd_leclrwl, Â "Clear white list" Â Â Â Â Â Â Â Â Â },
> Â Â Â Â{ "lecc", Â cmd_lecc, Â "Create a LE Connection", Â Â Â Â Â Â Â},
> Â Â Â Â{ "ledc", Â cmd_ledc, Â "Disconnect a LE Connection", Â Â Â Â Â},
> Â Â Â Â{ NULL, NULL, 0 }
> --
> 1.6.5
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux