Use packed struct instead of buf to improve readability.
---
emulator/btdev.c | 71 +++++++++++++++++++++++++++++---------------------------
1 file changed, 37 insertions(+), 34 deletions(-)
diff --git a/emulator/btdev.c b/emulator/btdev.c
index 4066d10..dfd953e 100644
--- a/emulator/btdev.c
+++ b/emulator/btdev.c
@@ -1088,12 +1088,13 @@ static void le_conn_complete(struct btdev *btdev,
const uint8_t *bdaddr, uint8_t bdaddr_type,
uint8_t status)
{
- char buf[1 + sizeof(struct bt_hci_evt_le_conn_complete)];
- struct bt_hci_evt_le_conn_complete *cc = (void *) &buf[1];
+ struct __packed {
+ uint8_t subevent;
+ struct bt_hci_evt_le_conn_complete cc;
+ } ev;
- memset(buf, 0, sizeof(buf));
- buf[0] = BT_HCI_EVT_LE_CONN_COMPLETE;
+ ev.subevent = BT_HCI_EVT_LE_CONN_COMPLETE;
if (!status) {
struct btdev *remote = find_btdev_by_bdaddr_type(bdaddr,
@@ -1104,27 +1105,27 @@ static void le_conn_complete(struct btdev *btdev,
remote->conn = btdev;
remote->le_adv_enable = 0;
- cc->status = status;
- cc->peer_addr_type = btdev->le_scan_own_addr_type;
- if (cc->peer_addr_type == 0x01)
- memcpy(cc->peer_addr, btdev->random_addr, 6);
+ ev.cc.status = status;
+ ev.cc.peer_addr_type = btdev->le_scan_own_addr_type;
+ if (ev.cc.peer_addr_type == 0x01)
+ memcpy(ev.cc.peer_addr, btdev->random_addr, 6);
else
- memcpy(cc->peer_addr, btdev->bdaddr, 6);
+ memcpy(ev.cc.peer_addr, btdev->bdaddr, 6);
- cc->role = 0x01;
- cc->handle = cpu_to_le16(42);
+ ev.cc.role = 0x01;
+ ev.cc.handle = cpu_to_le16(42);
- send_event(remote, BT_HCI_EVT_LE_META_EVENT, buf, sizeof(buf));
+ send_event(remote, BT_HCI_EVT_LE_META_EVENT, &ev, sizeof(ev));
- cc->handle = cpu_to_le16(42);
+ ev.cc.handle = cpu_to_le16(42);
}
- cc->status = status;
- cc->peer_addr_type = bdaddr_type;
- memcpy(cc->peer_addr, bdaddr, 6);
- cc->role = 0x00;
+ ev.cc.status = status;
+ ev.cc.peer_addr_type = bdaddr_type;
+ memcpy(ev.cc.peer_addr, bdaddr, 6);
+ ev.cc.role = 0x00;
- send_event(btdev, BT_HCI_EVT_LE_META_EVENT, buf, sizeof(buf));
+ send_event(btdev, BT_HCI_EVT_LE_META_EVENT, &ev, sizeof(ev));
}
static const uint8_t *scan_addr(const struct btdev *btdev)
@@ -1813,9 +1814,11 @@ static void le_set_scan_enable_complete(struct btdev *btdev)
static void le_read_remote_features_complete(struct btdev *btdev)
{
- char buf[1 + sizeof(struct bt_hci_evt_le_remote_features_complete)];
- struct bt_hci_evt_le_remote_features_complete *ev = (void *) &buf[1];
struct btdev *remote = btdev->conn;
+ struct __packed {
+ uint8_t subevent;
+ struct bt_hci_evt_le_remote_features_complete rfc;
+ } ev;
if (!remote) {
cmd_status(btdev, BT_HCI_ERR_UNKNOWN_CONN_ID,
@@ -1826,20 +1829,21 @@ static void le_read_remote_features_complete(struct btdev *btdev)
cmd_status(btdev, BT_HCI_ERR_SUCCESS,
BT_HCI_CMD_LE_READ_REMOTE_FEATURES);
- memset(buf, 0, sizeof(buf));
- buf[0] = BT_HCI_EVT_LE_REMOTE_FEATURES_COMPLETE;
- ev->status = BT_HCI_ERR_SUCCESS;
- ev->handle = cpu_to_le16(42);
- memcpy(ev->features, remote->le_features, 8);
+ ev.subevent = BT_HCI_EVT_LE_REMOTE_FEATURES_COMPLETE;
+ ev.rfc.status = BT_HCI_ERR_SUCCESS;
+ ev.rfc.handle = cpu_to_le16(42);
+ memcpy(&ev.rfc.features, remote->le_features, 8);
- send_event(btdev, BT_HCI_EVT_LE_META_EVENT, buf, sizeof(buf));
+ send_event(btdev, BT_HCI_EVT_LE_META_EVENT, &ev, sizeof(ev));
}
static void le_start_encrypt_complete(struct btdev *btdev, uint16_t ediv,
uint64_t rand)
{
- char buf[1 + sizeof(struct bt_hci_evt_le_long_term_key_request)];
- struct bt_hci_evt_le_long_term_key_request *ev = (void *) &buf[1];
+ struct __packed {
+ uint8_t subevent;
+ struct bt_hci_evt_le_long_term_key_request ev;
+ } ev;
struct btdev *remote = btdev->conn;
if (!remote) {
@@ -1850,13 +1854,12 @@ static void le_start_encrypt_complete(struct btdev *btdev, uint16_t ediv,
cmd_status(btdev, BT_HCI_ERR_SUCCESS, BT_HCI_CMD_LE_START_ENCRYPT);
- memset(buf, 0, sizeof(buf));
- buf[0] = BT_HCI_EVT_LE_LONG_TERM_KEY_REQUEST;
- ev->handle = cpu_to_le16(42);
- ev->ediv = ediv;
- ev->rand = rand;
+ ev.subevent = BT_HCI_EVT_LE_LONG_TERM_KEY_REQUEST;
+ ev.ev.handle = cpu_to_le16(42);
+ ev.ev.ediv = ediv;
+ ev.ev.rand = rand;
- send_event(remote, BT_HCI_EVT_LE_META_EVENT, buf, sizeof(buf));
+ send_event(remote, BT_HCI_EVT_LE_META_EVENT, &ev, sizeof(ev));
}
static void le_encrypt_complete(struct btdev *btdev)
--
1.9.1
Hi Gowtham,
>>>>> ---
>>>>> emulator/le.c | 45
>> ++++++++++++++++++++++++++++++++++++++++++--
>>>> -
>>>>> 1 file changed, 42 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/emulator/le.c b/emulator/le.c index dc51469..7f852a9
>>>>> 100644
>>>>> --- a/emulator/le.c
>>>>> +++ b/emulator/le.c
>>>>> @@ -258,9 +258,9 @@ static void reset_defaults(struct bt_le *hci)
>>>>> //hci->commands[28] |= 0x02; /* LE Long Term Key Request Reply
>>>> */
>>>>> //hci->commands[28] |= 0x04; /* LE Long Term Key Request
>>>> Negative Reply */
>>>>> hci->commands[28] |= 0x08; /* LE Read Supported States */
>>>>> - //hci->commands[28] |= 0x10; /* LE Receiver Test */
>>>>> - //hci->commands[28] |= 0x20; /* LE Transmitter Test */
>>>>> - //hci->commands[28] |= 0x40; /* LE Test End */
>>>>> + hci->commands[28] |= 0x10; /* LE Receiver Test */
>>>>> + hci->commands[28] |= 0x20; /* LE Transmitter Test */
>>>>> + hci->commands[28] |= 0x40; /* LE Test End */
>>>>> //hci->commands[33] |= 0x10; /* LE Remote Connection Parameter
>>>> Request Reply */
>>>>> //hci->commands[33] |= 0x20; /* LE Remote Connection Parameter
>>>> Request Negative Reply */
>>>>> hci->commands[33] |= 0x40; /* LE Set Data Length */
>>>>> @@ -1218,6 +1218,36 @@ static void
>>>> cmd_le_read_supported_states(struct bt_le *hci,
>>>>> &rsp, sizeof(rsp));
>>>>> }
>>>>>
>>>>> +static void cmd_le_receiver_test(struct bt_le *hci,
>>>>> + const void *data, uint8_t
>>> size)
>>>> {
>>>>> + uint8_t status;
>>>>> +
>>>>> + status = BT_HCI_ERR_SUCCESS;
>>>>> + cmd_complete(hci, BT_HCI_CMD_LE_RECEIVER_TEST,
>>>>> + &status, sizeof(status));
>>>>> +}
>>>>> +
>>>>> +static void cmd_le_transmitter_test(struct bt_le *hci,
>>>>> + const void *data, uint8_t
>>> size)
>>>> {
>>>>> + uint8_t status;
>>>>> +
>>>>> + status = BT_HCI_ERR_SUCCESS;
>>>>> + cmd_complete(hci, BT_HCI_CMD_LE_TRANSMITTER_TEST,
>>>>> + &status, sizeof(status));
>>>>> +}
>>>>> +
>>>>> +static void cmd_le_end_test(struct bt_le *hci, const void *data,
>>>>> +uint8_t size) {
>>>>> + struct bt_hci_rsp_le_test_end lte;
>>>>> +
>>>>> + lte.status = BT_HCI_ERR_SUCCESS;
>>>>> + lte.num_packets = 0;
>>>>> +
>>>>> + cmd_complete(hci, BT_HCI_CMD_LE_TEST_END, <e, sizeof(lte)); }
>>>>> +
>>>>
>>>> this is actually not enough. You need to put a bit of logic into this
>>> code. For
>>>> example it is either RX or TX test and not both at the same time. You
>>>> need
>>> to
>>>> return proper errors. And the test end command you also return the
>>>> correct value for num_packets. So if you do a receiver test against a
>>>> transmitter
>>> test
>>>> you are getting some useful values here. Otherwise there are just
>>>> empty stubs and we better not bother having them at all.
>>>>
>>>
>>> Thanks for the quick review.
>>> Below are the assumptions that I want to clarify:
>>> 1) Need to check rx_chan and tx_chan are in 0x00 to 0x27 range, if
>>> not return BT_HCI_ERR_INVALID_PARAMETERS.
>>
>> that is the first thing that needs to be done.
>>
>> For the TX test also test data length needs to be checked for 0x00 - 0x25.
> This
>> might actually depend on if data length extension is supported or not.
>>
>> Also the packet payload is limited to 0x00 - 0x07.
>>
>>> 2) Need to maintain a global variable (rx_test, tx_test) to track
>>> which test is going on.
>>
>> Yes, and command disallowed needs to be return if either of the tests is
>> active.
>>
>>> 3) When receiver test is called, a global variable (pkt_recv ) is
>>> incremented to track num of packets received.
>>>
>>> Or Should I maintain a structure for (2) & (3) instead of maintaining
>>> globals?
>>
>> The receiving side needs to track the num of packets received on that
>> specified channel. Meaning that if no other transceiver is testing, that
> should
>> return 0. If some other transceiver is active, it should increment with
> the
>> proper interval depending on how long the test is active.
>>
>> You might realise why I have not yet implemented these commands. They
>> require a bit of timing accuracy that we currently do not really have in
> the LE
>> only emulator.
>
> Thanks for the reply. Then I probably skip this one and work on the other
> commands as well.
> Listed below are few missing LE commands that I am planning to work on le.c
> which requires suggestion:
> 1) BT_HCI_CMD_READ_TX_POWER
> 2) BT_HCI_CMD_READ_RSSI
> 3) BT_HCI_CMD_LE_CONN_UPDATE
> 4) BT_HCI_CMD_LE_SET_HOST_CLASSIFICATION
> 5) BT_HCI_CMD_LE_READ_CHANNEL_MAP
>
>
> Apart from that is there any specific reason why BDADDR for LE is not
> created/used anywhere in le.c,
> whereas in btdev.c it was created and used wherever necessary?
> Because of this, LE emulator never gets up, since le.c is using
> 00:00:00:00:00:00 as BDADDR.
> Does it need to be fixed as in btdev?
the explicit intent is that emulator/le.c provide a LE only single mode controller without a public BD_ADDR. So this means that you have to use Set Static Address management command to first set an identity address.
> One more thing, I am not able to write/read the local name for LE
> emulator(le.c).
> The cmd itself it not supported right now. Is it good to be fixed?
LE only single mode controllers do not support HCI command for local name. That is a BR/EDR only feature.
Regards
Marcel
Hi Marcel,
> -----Original Message-----
> From: [email protected] [mailto:linux-bluetooth-
> [email protected]] On Behalf Of Marcel Holtmann
> Sent: Saturday, June 06, 2015 10:04 AM
> To: Gowtham Anandha Babu
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH 2/2] emulator/le: Add LE test commands
>
> Hi Gowtham,
>
> >>> ---
> >>> emulator/le.c | 45
> ++++++++++++++++++++++++++++++++++++++++++--
> >> -
> >>> 1 file changed, 42 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/emulator/le.c b/emulator/le.c index dc51469..7f852a9
> >>> 100644
> >>> --- a/emulator/le.c
> >>> +++ b/emulator/le.c
> >>> @@ -258,9 +258,9 @@ static void reset_defaults(struct bt_le *hci)
> >>> //hci->commands[28] |= 0x02; /* LE Long Term Key Request Reply
> >> */
> >>> //hci->commands[28] |= 0x04; /* LE Long Term Key Request
> >> Negative Reply */
> >>> hci->commands[28] |= 0x08; /* LE Read Supported States */
> >>> - //hci->commands[28] |= 0x10; /* LE Receiver Test */
> >>> - //hci->commands[28] |= 0x20; /* LE Transmitter Test */
> >>> - //hci->commands[28] |= 0x40; /* LE Test End */
> >>> + hci->commands[28] |= 0x10; /* LE Receiver Test */
> >>> + hci->commands[28] |= 0x20; /* LE Transmitter Test */
> >>> + hci->commands[28] |= 0x40; /* LE Test End */
> >>> //hci->commands[33] |= 0x10; /* LE Remote Connection Parameter
> >> Request Reply */
> >>> //hci->commands[33] |= 0x20; /* LE Remote Connection Parameter
> >> Request Negative Reply */
> >>> hci->commands[33] |= 0x40; /* LE Set Data Length */
> >>> @@ -1218,6 +1218,36 @@ static void
> >> cmd_le_read_supported_states(struct bt_le *hci,
> >>> &rsp, sizeof(rsp));
> >>> }
> >>>
> >>> +static void cmd_le_receiver_test(struct bt_le *hci,
> >>> + const void *data, uint8_t
> > size)
> >> {
> >>> + uint8_t status;
> >>> +
> >>> + status = BT_HCI_ERR_SUCCESS;
> >>> + cmd_complete(hci, BT_HCI_CMD_LE_RECEIVER_TEST,
> >>> + &status, sizeof(status));
> >>> +}
> >>> +
> >>> +static void cmd_le_transmitter_test(struct bt_le *hci,
> >>> + const void *data, uint8_t
> > size)
> >> {
> >>> + uint8_t status;
> >>> +
> >>> + status = BT_HCI_ERR_SUCCESS;
> >>> + cmd_complete(hci, BT_HCI_CMD_LE_TRANSMITTER_TEST,
> >>> + &status, sizeof(status));
> >>> +}
> >>> +
> >>> +static void cmd_le_end_test(struct bt_le *hci, const void *data,
> >>> +uint8_t size) {
> >>> + struct bt_hci_rsp_le_test_end lte;
> >>> +
> >>> + lte.status = BT_HCI_ERR_SUCCESS;
> >>> + lte.num_packets = 0;
> >>> +
> >>> + cmd_complete(hci, BT_HCI_CMD_LE_TEST_END, <e, sizeof(lte)); }
> >>> +
> >>
> >> this is actually not enough. You need to put a bit of logic into this
> > code. For
> >> example it is either RX or TX test and not both at the same time. You
> >> need
> > to
> >> return proper errors. And the test end command you also return the
> >> correct value for num_packets. So if you do a receiver test against a
> >> transmitter
> > test
> >> you are getting some useful values here. Otherwise there are just
> >> empty stubs and we better not bother having them at all.
> >>
> >
> > Thanks for the quick review.
> > Below are the assumptions that I want to clarify:
> > 1) Need to check rx_chan and tx_chan are in 0x00 to 0x27 range, if
> > not return BT_HCI_ERR_INVALID_PARAMETERS.
>
> that is the first thing that needs to be done.
>
> For the TX test also test data length needs to be checked for 0x00 - 0x25.
This
> might actually depend on if data length extension is supported or not.
>
> Also the packet payload is limited to 0x00 - 0x07.
>
> > 2) Need to maintain a global variable (rx_test, tx_test) to track
> > which test is going on.
>
> Yes, and command disallowed needs to be return if either of the tests is
> active.
>
> > 3) When receiver test is called, a global variable (pkt_recv ) is
> > incremented to track num of packets received.
> >
> > Or Should I maintain a structure for (2) & (3) instead of maintaining
> > globals?
>
> The receiving side needs to track the num of packets received on that
> specified channel. Meaning that if no other transceiver is testing, that
should
> return 0. If some other transceiver is active, it should increment with
the
> proper interval depending on how long the test is active.
>
> You might realise why I have not yet implemented these commands. They
> require a bit of timing accuracy that we currently do not really have in
the LE
> only emulator.
Thanks for the reply. Then I probably skip this one and work on the other
commands as well.
Listed below are few missing LE commands that I am planning to work on le.c
which requires suggestion:
1) BT_HCI_CMD_READ_TX_POWER
2) BT_HCI_CMD_READ_RSSI
3) BT_HCI_CMD_LE_CONN_UPDATE
4) BT_HCI_CMD_LE_SET_HOST_CLASSIFICATION
5) BT_HCI_CMD_LE_READ_CHANNEL_MAP
Apart from that is there any specific reason why BDADDR for LE is not
created/used anywhere in le.c,
whereas in btdev.c it was created and used wherever necessary?
Because of this, LE emulator never gets up, since le.c is using
00:00:00:00:00:00 as BDADDR.
Does it need to be fixed as in btdev?
One more thing, I am not able to write/read the local name for LE
emulator(le.c).
The cmd itself it not supported right now. Is it good to be fixed?
> >
> > 4) Finally while test end cmd,
> > If (tx_test)
> > num_packets = 0;
> > else if (rx_test) {
> > num_packets = pkt_recv;
> > pkt_recv = 0;
> > }
>
> Setting pkt_recv to 0 happens when starting the RX test.
>
> > Is there any tool to test the above commands?
> > If not I can add these commands to any of the tools (hcitool, I guess)
> > for testing (If it is worth to add in BlueZ, then I will send them as
> > patches too).
>
> I used hcitool cmd for it.
>
> Regards
>
> Marcel
>
Regards
Gowtham Anandha Babu
Hi Gowtham,
>>> ---
>>> emulator/le.c | 45 ++++++++++++++++++++++++++++++++++++++++++--
>> -
>>> 1 file changed, 42 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/emulator/le.c b/emulator/le.c index dc51469..7f852a9
>>> 100644
>>> --- a/emulator/le.c
>>> +++ b/emulator/le.c
>>> @@ -258,9 +258,9 @@ static void reset_defaults(struct bt_le *hci)
>>> //hci->commands[28] |= 0x02; /* LE Long Term Key Request Reply
>> */
>>> //hci->commands[28] |= 0x04; /* LE Long Term Key Request
>> Negative Reply */
>>> hci->commands[28] |= 0x08; /* LE Read Supported States */
>>> - //hci->commands[28] |= 0x10; /* LE Receiver Test */
>>> - //hci->commands[28] |= 0x20; /* LE Transmitter Test */
>>> - //hci->commands[28] |= 0x40; /* LE Test End */
>>> + hci->commands[28] |= 0x10; /* LE Receiver Test */
>>> + hci->commands[28] |= 0x20; /* LE Transmitter Test */
>>> + hci->commands[28] |= 0x40; /* LE Test End */
>>> //hci->commands[33] |= 0x10; /* LE Remote Connection Parameter
>> Request Reply */
>>> //hci->commands[33] |= 0x20; /* LE Remote Connection Parameter
>> Request Negative Reply */
>>> hci->commands[33] |= 0x40; /* LE Set Data Length */
>>> @@ -1218,6 +1218,36 @@ static void
>> cmd_le_read_supported_states(struct bt_le *hci,
>>> &rsp, sizeof(rsp));
>>> }
>>>
>>> +static void cmd_le_receiver_test(struct bt_le *hci,
>>> + const void *data, uint8_t
> size)
>> {
>>> + uint8_t status;
>>> +
>>> + status = BT_HCI_ERR_SUCCESS;
>>> + cmd_complete(hci, BT_HCI_CMD_LE_RECEIVER_TEST,
>>> + &status, sizeof(status));
>>> +}
>>> +
>>> +static void cmd_le_transmitter_test(struct bt_le *hci,
>>> + const void *data, uint8_t
> size)
>> {
>>> + uint8_t status;
>>> +
>>> + status = BT_HCI_ERR_SUCCESS;
>>> + cmd_complete(hci, BT_HCI_CMD_LE_TRANSMITTER_TEST,
>>> + &status, sizeof(status));
>>> +}
>>> +
>>> +static void cmd_le_end_test(struct bt_le *hci, const void *data,
>>> +uint8_t size) {
>>> + struct bt_hci_rsp_le_test_end lte;
>>> +
>>> + lte.status = BT_HCI_ERR_SUCCESS;
>>> + lte.num_packets = 0;
>>> +
>>> + cmd_complete(hci, BT_HCI_CMD_LE_TEST_END, <e, sizeof(lte)); }
>>> +
>>
>> this is actually not enough. You need to put a bit of logic into this
> code. For
>> example it is either RX or TX test and not both at the same time. You need
> to
>> return proper errors. And the test end command you also return the correct
>> value for num_packets. So if you do a receiver test against a transmitter
> test
>> you are getting some useful values here. Otherwise there are just empty
>> stubs and we better not bother having them at all.
>>
>
> Thanks for the quick review.
> Below are the assumptions that I want to clarify:
> 1) Need to check rx_chan and tx_chan are in 0x00 to 0x27 range, if not
> return BT_HCI_ERR_INVALID_PARAMETERS.
that is the first thing that needs to be done.
For the TX test also test data length needs to be checked for 0x00 - 0x25. This might actually depend on if data length extension is supported or not.
Also the packet payload is limited to 0x00 - 0x07.
> 2) Need to maintain a global variable (rx_test, tx_test) to track which test
> is going on.
Yes, and command disallowed needs to be return if either of the tests is active.
> 3) When receiver test is called, a global variable (pkt_recv ) is
> incremented to track num of packets received.
>
> Or Should I maintain a structure for (2) & (3) instead of maintaining
> globals?
The receiving side needs to track the num of packets received on that specified channel. Meaning that if no other transceiver is testing, that should return 0. If some other transceiver is active, it should increment with the proper interval depending on how long the test is active.
You might realise why I have not yet implemented these commands. They require a bit of timing accuracy that we currently do not really have in the LE only emulator.
>
> 4) Finally while test end cmd,
> If (tx_test)
> num_packets = 0;
> else if (rx_test) {
> num_packets = pkt_recv;
> pkt_recv = 0;
> }
Setting pkt_recv to 0 happens when starting the RX test.
> Is there any tool to test the above commands?
> If not I can add these commands to any of the tools (hcitool, I guess) for
> testing (If it is worth to add in BlueZ, then I will send them as patches
> too).
I used hcitool cmd for it.
Regards
Marcel
Hi Marcel,
> -----Original Message-----
> From: [email protected] [mailto:linux-bluetooth-
> [email protected]] On Behalf Of Marcel Holtmann
> Sent: Thursday, June 04, 2015 4:29 PM
> To: Gowtham Anandha Babu
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH 2/2] emulator/le: Add LE test commands
>
> Hi Gowtham,
>
> > ---
> > emulator/le.c | 45 ++++++++++++++++++++++++++++++++++++++++++--
> -
> > 1 file changed, 42 insertions(+), 3 deletions(-)
> >
> > diff --git a/emulator/le.c b/emulator/le.c index dc51469..7f852a9
> > 100644
> > --- a/emulator/le.c
> > +++ b/emulator/le.c
> > @@ -258,9 +258,9 @@ static void reset_defaults(struct bt_le *hci)
> > //hci->commands[28] |= 0x02; /* LE Long Term Key Request Reply
> */
> > //hci->commands[28] |= 0x04; /* LE Long Term Key Request
> Negative Reply */
> > hci->commands[28] |= 0x08; /* LE Read Supported States */
> > - //hci->commands[28] |= 0x10; /* LE Receiver Test */
> > - //hci->commands[28] |= 0x20; /* LE Transmitter Test */
> > - //hci->commands[28] |= 0x40; /* LE Test End */
> > + hci->commands[28] |= 0x10; /* LE Receiver Test */
> > + hci->commands[28] |= 0x20; /* LE Transmitter Test */
> > + hci->commands[28] |= 0x40; /* LE Test End */
> > //hci->commands[33] |= 0x10; /* LE Remote Connection Parameter
> Request Reply */
> > //hci->commands[33] |= 0x20; /* LE Remote Connection Parameter
> Request Negative Reply */
> > hci->commands[33] |= 0x40; /* LE Set Data Length */
> > @@ -1218,6 +1218,36 @@ static void
> cmd_le_read_supported_states(struct bt_le *hci,
> > &rsp, sizeof(rsp));
> > }
> >
> > +static void cmd_le_receiver_test(struct bt_le *hci,
> > + const void *data, uint8_t
size)
> {
> > + uint8_t status;
> > +
> > + status = BT_HCI_ERR_SUCCESS;
> > + cmd_complete(hci, BT_HCI_CMD_LE_RECEIVER_TEST,
> > + &status, sizeof(status));
> > +}
> > +
> > +static void cmd_le_transmitter_test(struct bt_le *hci,
> > + const void *data, uint8_t
size)
> {
> > + uint8_t status;
> > +
> > + status = BT_HCI_ERR_SUCCESS;
> > + cmd_complete(hci, BT_HCI_CMD_LE_TRANSMITTER_TEST,
> > + &status, sizeof(status));
> > +}
> > +
> > +static void cmd_le_end_test(struct bt_le *hci, const void *data,
> > +uint8_t size) {
> > + struct bt_hci_rsp_le_test_end lte;
> > +
> > + lte.status = BT_HCI_ERR_SUCCESS;
> > + lte.num_packets = 0;
> > +
> > + cmd_complete(hci, BT_HCI_CMD_LE_TEST_END, <e, sizeof(lte)); }
> > +
>
> this is actually not enough. You need to put a bit of logic into this
code. For
> example it is either RX or TX test and not both at the same time. You need
to
> return proper errors. And the test end command you also return the correct
> value for num_packets. So if you do a receiver test against a transmitter
test
> you are getting some useful values here. Otherwise there are just empty
> stubs and we better not bother having them at all.
>
Thanks for the quick review.
Below are the assumptions that I want to clarify:
1) Need to check rx_chan and tx_chan are in 0x00 to 0x27 range, if not
return BT_HCI_ERR_INVALID_PARAMETERS.
2) Need to maintain a global variable (rx_test, tx_test) to track which test
is going on.
3) When receiver test is called, a global variable (pkt_recv ) is
incremented to track num of packets received.
Or Should I maintain a structure for (2) & (3) instead of maintaining
globals?
4) Finally while test end cmd,
If (tx_test)
num_packets = 0;
else if (rx_test) {
num_packets = pkt_recv;
pkt_recv = 0;
}
Is there any tool to test the above commands?
If not I can add these commands to any of the tools (hcitool, I guess) for
testing (If it is worth to add in BlueZ, then I will send them as patches
too).
> The standalone LE emulation in btvirt is trying really hard to have proper
and
> correct error handling for LE only single mode controller.
>
> Regards
>
> Marcel
>
> --
> 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
Best Regards,
Gowtham Anandha Babu
Hi Gowtham,
> ---
> emulator/le.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/emulator/le.c b/emulator/le.c
> index dc51469..7f852a9 100644
> --- a/emulator/le.c
> +++ b/emulator/le.c
> @@ -258,9 +258,9 @@ static void reset_defaults(struct bt_le *hci)
> //hci->commands[28] |= 0x02; /* LE Long Term Key Request Reply */
> //hci->commands[28] |= 0x04; /* LE Long Term Key Request Negative Reply */
> hci->commands[28] |= 0x08; /* LE Read Supported States */
> - //hci->commands[28] |= 0x10; /* LE Receiver Test */
> - //hci->commands[28] |= 0x20; /* LE Transmitter Test */
> - //hci->commands[28] |= 0x40; /* LE Test End */
> + hci->commands[28] |= 0x10; /* LE Receiver Test */
> + hci->commands[28] |= 0x20; /* LE Transmitter Test */
> + hci->commands[28] |= 0x40; /* LE Test End */
> //hci->commands[33] |= 0x10; /* LE Remote Connection Parameter Request Reply */
> //hci->commands[33] |= 0x20; /* LE Remote Connection Parameter Request Negative Reply */
> hci->commands[33] |= 0x40; /* LE Set Data Length */
> @@ -1218,6 +1218,36 @@ static void cmd_le_read_supported_states(struct bt_le *hci,
> &rsp, sizeof(rsp));
> }
>
> +static void cmd_le_receiver_test(struct bt_le *hci,
> + const void *data, uint8_t size)
> +{
> + uint8_t status;
> +
> + status = BT_HCI_ERR_SUCCESS;
> + cmd_complete(hci, BT_HCI_CMD_LE_RECEIVER_TEST,
> + &status, sizeof(status));
> +}
> +
> +static void cmd_le_transmitter_test(struct bt_le *hci,
> + const void *data, uint8_t size)
> +{
> + uint8_t status;
> +
> + status = BT_HCI_ERR_SUCCESS;
> + cmd_complete(hci, BT_HCI_CMD_LE_TRANSMITTER_TEST,
> + &status, sizeof(status));
> +}
> +
> +static void cmd_le_end_test(struct bt_le *hci, const void *data, uint8_t size)
> +{
> + struct bt_hci_rsp_le_test_end lte;
> +
> + lte.status = BT_HCI_ERR_SUCCESS;
> + lte.num_packets = 0;
> +
> + cmd_complete(hci, BT_HCI_CMD_LE_TEST_END, <e, sizeof(lte));
> +}
> +
this is actually not enough. You need to put a bit of logic into this code. For example it is either RX or TX test and not both at the same time. You need to return proper errors. And the test end command you also return the correct value for num_packets. So if you do a receiver test against a transmitter test you are getting some useful values here. Otherwise there are just empty stubs and we better not bother having them at all.
The standalone LE emulation in btvirt is trying really hard to have proper and correct error handling for LE only single mode controller.
Regards
Marcel
---
emulator/le.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 42 insertions(+), 3 deletions(-)
diff --git a/emulator/le.c b/emulator/le.c
index dc51469..7f852a9 100644
--- a/emulator/le.c
+++ b/emulator/le.c
@@ -258,9 +258,9 @@ static void reset_defaults(struct bt_le *hci)
//hci->commands[28] |= 0x02; /* LE Long Term Key Request Reply */
//hci->commands[28] |= 0x04; /* LE Long Term Key Request Negative Reply */
hci->commands[28] |= 0x08; /* LE Read Supported States */
- //hci->commands[28] |= 0x10; /* LE Receiver Test */
- //hci->commands[28] |= 0x20; /* LE Transmitter Test */
- //hci->commands[28] |= 0x40; /* LE Test End */
+ hci->commands[28] |= 0x10; /* LE Receiver Test */
+ hci->commands[28] |= 0x20; /* LE Transmitter Test */
+ hci->commands[28] |= 0x40; /* LE Test End */
//hci->commands[33] |= 0x10; /* LE Remote Connection Parameter Request Reply */
//hci->commands[33] |= 0x20; /* LE Remote Connection Parameter Request Negative Reply */
hci->commands[33] |= 0x40; /* LE Set Data Length */
@@ -1218,6 +1218,36 @@ static void cmd_le_read_supported_states(struct bt_le *hci,
&rsp, sizeof(rsp));
}
+static void cmd_le_receiver_test(struct bt_le *hci,
+ const void *data, uint8_t size)
+{
+ uint8_t status;
+
+ status = BT_HCI_ERR_SUCCESS;
+ cmd_complete(hci, BT_HCI_CMD_LE_RECEIVER_TEST,
+ &status, sizeof(status));
+}
+
+static void cmd_le_transmitter_test(struct bt_le *hci,
+ const void *data, uint8_t size)
+{
+ uint8_t status;
+
+ status = BT_HCI_ERR_SUCCESS;
+ cmd_complete(hci, BT_HCI_CMD_LE_TRANSMITTER_TEST,
+ &status, sizeof(status));
+}
+
+static void cmd_le_end_test(struct bt_le *hci, const void *data, uint8_t size)
+{
+ struct bt_hci_rsp_le_test_end lte;
+
+ lte.status = BT_HCI_ERR_SUCCESS;
+ lte.num_packets = 0;
+
+ cmd_complete(hci, BT_HCI_CMD_LE_TEST_END, <e, sizeof(lte));
+}
+
static void cmd_le_set_data_length(struct bt_le *hci,
const void *data, uint8_t size)
{
@@ -1611,6 +1641,15 @@ static const struct {
{ BT_HCI_CMD_LE_READ_SUPPORTED_STATES,
cmd_le_read_supported_states, 0, true },
+ { BT_HCI_CMD_LE_RECEIVER_TEST,
+ cmd_le_receiver_test, 1, true },
+
+ { BT_HCI_CMD_LE_TRANSMITTER_TEST,
+ cmd_le_transmitter_test, 3, true },
+
+ { BT_HCI_CMD_LE_TEST_END,
+ cmd_le_end_test, 0, true },
+
{ BT_HCI_CMD_LE_SET_DATA_LENGTH,
cmd_le_set_data_length, 6, true },
{ BT_HCI_CMD_LE_READ_DEFAULT_DATA_LENGTH,
--
1.9.1