Return-Path: <cyrus@holtmann.org>
Message-ID: <4C6D6ED1.4070905@openbossa.org>
Date: Thu, 19 Aug 2010 13:50:09 -0400
From: Anderson Briglia <anderson.briglia@openbossa.org>
MIME-Version: 1.0
To: Claudio Takahasi <claudio.takahasi@openbossa.org>
CC: linux-bluetooth@vger.kernel.org
Subject: Re: [PATCH] hciconfig: add LE_SET_ADVERTISE_ENABLE cmd
References: <1282232587-14551-1-git-send-email-anderson.briglia@openbossa.org> <AANLkTinVA+hkzmnTGFEp-veWaEkjywq-Z9OLmqpihA=E@mail.gmail.com>
In-Reply-To: <AANLkTinVA+hkzmnTGFEp-veWaEkjywq-Z9OLmqpihA=E@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Sender: linux-bluetooth-owner@vger.kernel.org
List-ID: <linux-bluetooth.vger.kernel.org>

Thanks for reviewing. I'll update and send again.

On 08/19/2010 12:36 PM, Claudio Takahasi wrote:
> Hi Briglia,
>
> On Thu, Aug 19, 2010 at 12:43 PM, Anderson Briglia
> <anderson.briglia@openbossa.org>  wrote:
>    
>> This patch implements two new hciconfig commands: leadv and noleadv.
>> These new hciconfig flags are responsible to LE_SET_ADVERTISE_ENABLE
>> command implementation.
>> ---
>>   lib/hci.c         |    1 +
>>   lib/hci.h         |    1 +
>>   tools/hciconfig.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 48 insertions(+), 0 deletions(-)
>>
>> diff --git a/lib/hci.c b/lib/hci.c
>> index 3304daa..64ac5f2 100644
>> --- a/lib/hci.c
>> +++ b/lib/hci.c
>> @@ -188,6 +188,7 @@ static hci_map dev_flags_map[] = {
>>         { "INQUIRY", HCI_INQUIRY },
>>         { "AUTH",    HCI_AUTH    },
>>         { "ENCRYPT", HCI_ENCRYPT },
>> +       { "LEADV",   HCI_LE_ADV  },
>>         { NULL }
>>   };
>>
>> diff --git a/lib/hci.h b/lib/hci.h
>> index 512dab9..8b9d33f 100644
>> --- a/lib/hci.h
>> +++ b/lib/hci.h
>> @@ -73,6 +73,7 @@ enum {
>>         HCI_INQUIRY,
>>
>>         HCI_RAW,
>> +       HCI_LE_ADV,
>>   };
>>      
> I prefer this small change in another commit, it needs your previous
> kernel patch to work/test properly.
>
>    
>>   /* HCI ioctl defines */
>> diff --git a/tools/hciconfig.c b/tools/hciconfig.c
>> index 87dd127..ba12a8a 100644
>> --- a/tools/hciconfig.c
>> +++ b/tools/hciconfig.c
>> @@ -149,6 +149,50 @@ static void cmd_scan(int ctl, int hdev, char *opt)
>>         }
>>   }
>>
>> +static void cmd_le_adv(int ctl, int hdev, char *opt)
>> +{
>> +
>>      
> No empty line here!
>
>    
>> +       struct hci_request rq;
>> +       le_set_advertise_enable_cp advertise_cp;
>> +       uint8_t status;
>> +       uint8_t enable;
>>      
> "enable" can be declared in the same line of "status"
>
>    
>> +       int dd, ret;
>> +
>> +       if (hdev<  0)
>> +               hdev = hci_get_route(NULL);
>> +
>> +       dd = hci_open_dev(hdev);
>> +       if (dd<  0) {
>> +               perror("Could not open device");
>> +               exit(1);
>> +       }
>> +
>> +       enable = 0x01;
>> +       if (!strcmp(opt, "noleadv"))
>> +               enable = 0x00;
>> +
>> +       memset(&advertise_cp, 0, sizeof(advertise_cp));
>> +       advertise_cp.enable = enable;
>> +
>> +       memset(&rq, 0, sizeof(rq));
>> +       rq.ogf = OGF_LE_CTL;
>> +       rq.ocf = OCF_LE_SET_ADVERTISE_ENABLE;
>> +       rq.cparam =&advertise_cp;
>> +       rq.clen = LE_SET_ADVERTISE_ENABLE_CP_SIZE;
>> +       rq.rparam =&status;
>> +       rq.rlen = 1;
>> +
>> +       ret = hci_send_req(dd,&rq, 100);
>> +
>>      
> No empty line here.
>    
>> +       if (status || ret<  0) {
>> +               errno = EIO;
>>      
> No need to set errno. hci_send_req sets errno when needed.
>
> Claudio
>
>    
>> +               fprintf(stderr, "Can't set advertise mode on hci%d: %s (%d)\n",
>> +                                               hdev, strerror(errno), errno);
>> +       }
>> +
>> +       hci_close_dev(dd);
>> +}
>> +
>>   static void cmd_iac(int ctl, int hdev, char *opt)
>>   {
>>         int s = hci_open_dev(hdev);
>> @@ -1728,6 +1772,8 @@ static struct {
>>         { "revision",   cmd_revision,   0,              "Display revision information" },
>>         { "block",      cmd_block,      "<bdaddr>",     "Add a device to the blacklist" },
>>         { "unblock",    cmd_unblock,    "<bdaddr>",     "Remove a device from the blacklist" },
>> +       { "leadv",      cmd_le_adv,     0,              "Enable LE advertising" },
>> +       { "noleadv",    cmd_le_adv,     0,              "Disable LE advertising" },
>>         { NULL, NULL, 0 }
>>   };
>>
>> --
>> 1.7.0.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>