-
Notifications
You must be signed in to change notification settings - Fork 383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
nimble/controller: Add Channel Sounding setup phase #1765
base: master
Are you sure you want to change the base?
Conversation
fcafb89
to
6a71e3d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
look good, few nitpicks here and there
nimble/controller/src/ble_ll_cs.c
Outdated
cssm = connsm->cssm; | ||
|
||
/* Check if a disabled role is used in CS configs */ | ||
for (int i = 0; i < ARRAY_SIZE(cssm->config); i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: we typically declare variable at top of the function, also could be unsigned to avoid signed-unsigned comparison
|
||
/* Check if a disabled role is used in CS configs */ | ||
for (int i = 0; i < ARRAY_SIZE(cssm->config); i++) { | ||
struct ble_ll_cs_config *conf = &cssm->config[i]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
nimble/controller/src/ble_ll_cs.c
Outdated
} | ||
|
||
cssm->roles_enabled = cmd->role_enable; | ||
cssm->cs_sync_antenna_selection = cmd->cs_sync_antenna_selection; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if we are very consistent with this but maybe sm could be modified only after all validation is done? ie to avoid modifying it and stil returning error status
int | ||
ble_ll_cs_rx_fae_req(struct ble_ll_conn_sm *connsm, uint8_t *rspbuf) | ||
{ | ||
memcpy(rspbuf, connsm->cssm->local_fae_table, 72); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is quite a lot, is it guaranteed that there is enough linear space in underlying mbuf?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, there wasn't ;)
|
||
if (!rc) { | ||
swap_buf(out_data, ecb.cipher_text, BLE_ENC_BLOCK_SIZE); | ||
rc = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rc is already 0 here
nimble/controller/src/ble_ll_cs.c
Outdated
{ | ||
uint8_t config_id = connsm->cssm->config_req_id; | ||
uint8_t action = connsm->cssm->config_req_action; | ||
const struct ble_ll_cs_config *conf = &connsm->cssm->config[config_id]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is config_id already validated here?
return BLE_ERR_UNK_CONN_ID; | ||
} | ||
|
||
conf = &connsm->cssm->config[cmd->config_id]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
config_id should be validated before being used for indexing
A dedicated CS DRBG is used to coordinate the randomization of several transaction types during a CS procedure.
The BT specification provides sample data that can be used to test the correctness of a CS DRBG implementation.
ble_cs.c contains initial implementation that caused warnings when running command: newt test @apache-mynewt-nimble/nimble
Latest drafts of Atlanta spec contains Optional_TX_SNR_Capability field as a CS capability. Companion signal parameter was removed.
Initialize the CS state machine with the connection state machine.
Add lengths and opcodes of Channel Sounding LL CTRL PDUs.
Implement the LE CS Read Local Supported Capabilities command. Enable basic CS capabilities in local controller.
Implements: - LE CS Read Remote Supported Capabilities command, - LE CS Write Cached Remote Supported Capabilities command.
The HCI_LE_CS_Set_Default_Settings command is used by a Host to set default CS settings in the local Controller for the connection.
Implements: - LE CS Read Remote FAE Table command - LE CS Write Cached Remote FAE Table command
Implements: - LE CS Create Config command - LE CS Remove Config command
Implements LE CS Security Enable command.
No description provided.