Bluetooth: Controller: Adds path loss monitoring functionality #92450
+208
−8
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2793,6 +2793,55 @@ static void le_set_phy(struct net_buf *buf, struct net_buf **evt) | |||||
| *evt = cmd_status(status); | ||||||
| } | ||||||
| #endif /* CONFIG_BT_CTLR_PHY */ | ||||||
|
|
||||||
|
|
||||||
| #if defined(CONFIG_BT_CTLR_LE_PATH_LOSS_MONITORING) | ||||||
|
|
||||||
| #define PATH_LOSS_FACTOR_INVALID 0xFF | ||||||
| static void le_path_loss_set_parameters(struct net_buf *buf, struct net_buf **evt) | ||||||
| { | ||||||
| struct bt_hci_cp_le_set_path_loss_reporting_parameters *cmd = (void *)buf->data; | ||||||
| struct bt_hci_rp_le_set_path_loss_reporting_parameters *rp; | ||||||
|
|
||||||
| uint16_t handle = sys_le16_to_cpu(cmd->handle); | ||||||
| uint16_t min_time_spent = sys_le16_to_cpu(cmd->min_time_spent); | ||||||
|
|
||||||
| rp = hci_cmd_complete(evt, sizeof(*rp)); | ||||||
| rp->handle = handle; | ||||||
|
|
||||||
| if (cmd->high_threshold + cmd->high_hysteresis > PATH_LOSS_FACTOR_INVALID || | ||||||
| cmd->low_threshold < cmd->low_hysteresis || | ||||||
| cmd->low_threshold > cmd->high_threshold || | ||||||
| cmd->low_threshold + cmd->low_hysteresis > cmd->high_threshold - cmd->high_hysteresis) { | ||||||
| rp->status = BT_HCI_ERR_INVALID_PARAM; | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| rp->status = ll_conn_set_path_loss_parameters(handle, cmd->high_threshold, | ||||||
| cmd->high_hysteresis, cmd->low_threshold, | ||||||
| cmd->low_hysteresis, min_time_spent); | ||||||
| } | ||||||
|
|
||||||
| #define PATH_LOSS_ENABLED 0x01 | ||||||
| #define PATH_LOSS_DISABLED 0x00 | ||||||
| static void le_path_loss_enable(struct net_buf *buf, struct net_buf **evt) | ||||||
| { | ||||||
| struct bt_hci_cp_le_set_path_loss_reporting_enable *cmd = (void *)buf->data; | ||||||
| struct bt_hci_rp_le_set_path_loss_reporting_enable *rp; | ||||||
|
|
||||||
| uint16_t handle = sys_le16_to_cpu(cmd->handle); | ||||||
|
|
||||||
| rp = hci_cmd_complete(evt, sizeof(*rp)); | ||||||
| rp->handle = handle; | ||||||
|
|
||||||
| if (cmd->enable != PATH_LOSS_ENABLED && | ||||||
| cmd->enable != PATH_LOSS_DISABLED) { | ||||||
| rp->status = BT_HCI_ERR_INVALID_PARAM; | ||||||
| } | ||||||
|
|
||||||
| rp->status = ll_conn_set_path_loss_reporting(handle, cmd->enable); | ||||||
| } | ||||||
| #endif /* CONFIG_BT_CTLR_LE_PATH_LOSS_MONITORING */ | ||||||
| #endif /* CONFIG_BT_CONN */ | ||||||
|
|
||||||
| #if defined(CONFIG_BT_CTLR_PRIVACY) | ||||||
|
|
@@ -4767,6 +4816,17 @@ static int controller_cmd_handle(uint16_t ocf, struct net_buf *cmd, | |||||
| le_set_phy(cmd, evt); | ||||||
| break; | ||||||
| #endif /* CONFIG_BT_CTLR_PHY */ | ||||||
|
|
||||||
| #if defined(CONFIG_BT_CTLR_LE_PATH_LOSS_MONITORING) | ||||||
| case BT_OCF(BT_HCI_OP_LE_SET_PATH_LOSS_REPORTING_PARAMETERS): | ||||||
| le_path_loss_set_parameters(cmd, evt); | ||||||
| break; | ||||||
|
|
||||||
| case BT_OCF(BT_HCI_OP_LE_SET_PATH_LOSS_REPORTING_ENABLE): | ||||||
| le_path_loss_enable(cmd, evt); | ||||||
| break; | ||||||
|
|
||||||
| #endif /* CONFIG_BT_CTLR_LE_PATH_LOSS_MONITORING */ | ||||||
| #endif /* CONFIG_BT_CONN */ | ||||||
|
|
||||||
| #if defined(CONFIG_BT_CTLR_ADV_EXT) | ||||||
|
|
@@ -8645,6 +8705,28 @@ static void le_req_peer_sca_complete(struct pdu_data *pdu, uint16_t handle, | |||||
| sep->sca = scau->sca; | ||||||
| } | ||||||
| #endif /* CONFIG_BT_CTLR_SCA_UPDATE */ | ||||||
| #if defined(CONFIG_BT_CTLR_LE_PATH_LOSS_MONITORING) | ||||||
| static void le_path_loss_threshold(struct pdu_data *pdu, | ||||||
| uint16_t handle, | ||||||
| struct net_buf *buf) | ||||||
| { | ||||||
| struct bt_hci_evt_le_path_loss_threshold *sep; | ||||||
| struct node_rx_path_loss *pl_pdu; | ||||||
|
|
||||||
| pl_pdu = (void *)pdu; | ||||||
|
|
||||||
| if (!(event_mask & BT_EVT_MASK_LE_META_EVENT) || | ||||||
|
There was a problem hiding this comment.
Suggested change
There was a problem hiding this comment. I'd prefer to leave it as is, since it's similar to how it's done in the other functions above. Examples: zephyr/subsys/bluetooth/controller/hci/hci.c Line 8633 in fdd02c5 zephyr/subsys/bluetooth/controller/hci/hci.c Line 8608 in fdd02c5 There was a problem hiding this comment. I'll defer that to the maintainer of the controller. The reason why I mentioned here is that it's in violation of rule 85 of the Zephyr coding guidelines that does not allow using integers as controlling expression https://docs.zephyrproject.org/latest/contribute/coding_guidelines/index.html : https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_14_04.c There was a problem hiding this comment. @cvinayak Perhaps we could make a Github issue and fix up this file to be compliant after this PR? Then we can get all the changes done at once. There was a problem hiding this comment. Fixing it for a single file won't do much :D It's pretty much in every file and everywhere, since it has been the coding standard for many years before we adaptoed the aforementioned rule. I'm personally slowly going through some of the coding style violations in the LE Audio code, but there are thousands of cases for each rule :) |
||||||
| !(le_event_mask & BT_EVT_MASK_LE_PATH_LOSS_THRESHOLD)) { | ||||||
|
There was a problem hiding this comment.
Suggested change
There was a problem hiding this comment. Same suggestion as above, since this would stick out from the other, similar functions. |
||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| sep = meta_evt(buf, BT_HCI_EVT_LE_PATH_LOSS_THRESHOLD, sizeof(*sep)); | ||||||
|
|
||||||
| sep->handle = sys_cpu_to_le16(handle); | ||||||
| sep->current_path_loss = pl_pdu->current_path_loss; | ||||||
| sep->zone_entered = pl_pdu->zone_entered; | ||||||
| } | ||||||
| #endif /* CONFIG_BT_CTLR_LE_PATH_LOSS_MONITORING */ | ||||||
| #endif /* CONFIG_BT_CONN */ | ||||||
|
|
||||||
| #if defined(CONFIG_BT_HCI_MESH_EXT) | ||||||
|
|
@@ -8847,6 +8929,12 @@ static void encode_control(struct node_rx_pdu *node_rx, | |||||
| #endif /* CONFIG_BT_CTLR_DF_VS_CONN_IQ_REPORT_16_BITS_IQ_SAMPLES */ | ||||||
| return; | ||||||
| #endif /* CONFIG_BT_CTLR_DF_CONN_CTE_RX */ | ||||||
|
|
||||||
| #if defined(CONFIG_BT_CTLR_LE_PATH_LOSS_MONITORING) | ||||||
| case NODE_RX_TYPE_PATH_LOSS: | ||||||
| le_path_loss_threshold(pdu_data, handle, buf); | ||||||
| return; | ||||||
| #endif /* CONFIG_BT_CTLR_LE_PATH_LOSS_MONITORING */ | ||||||
| #endif /* CONFIG_BT_CONN */ | ||||||
|
|
||||||
| #if defined(CONFIG_BT_CTLR_ADV_INDICATION) | ||||||
|
|
@@ -9332,6 +9420,10 @@ uint8_t hci_get_class(struct node_rx_pdu *node_rx) | |||||
| #endif /* CONFIG_BT_CTLR_PHY */ | ||||||
|
|
||||||
| return HCI_CLASS_EVT_CONNECTION; | ||||||
| #if defined(CONFIG_BT_CTLR_LE_PATH_LOSS_MONITORING) | ||||||
| case NODE_RX_TYPE_PATH_LOSS: | ||||||
| return HCI_CLASS_EVT_REQUIRED; | ||||||
| #endif /* CONFIG_BT_CTLR_LE_PATH_LOSS_MONITORING */ | ||||||
| #endif /* CONFIG_BT_CONN */ | ||||||
|
|
||||||
| #if defined(CONFIG_BT_CTLR_SYNC_ISO) || defined(CONFIG_BT_CTLR_CONN_ISO) | ||||||
|
|
||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -138,3 +138,13 @@ void ull_conn_resume_rx_data(struct ll_conn *conn); | |
| * @brief Check if the lower link layer transmit queue is empty | ||
| */ | ||
| uint8_t ull_is_lll_tx_queue_empty(struct ll_conn *conn); | ||
|
|
||
| /** | ||
| * @brief Set path loss parameters | ||
| */ | ||
| void ull_path_loss_set_parameters(void); | ||
|
|
||
| /** | ||
| * @brief Enable path loss reporting | ||
| */ | ||
| void ull_enable_path_loss_reporting(void); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -301,3 +301,8 @@ struct node_rx_sca { | |
| uint8_t status; | ||
| uint8_t sca; | ||
| }; | ||
|
|
||
| struct node_rx_path_loss { | ||
| uint8_t current_path_loss; | ||
| uint8_t zone_entered; | ||
| }; | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 particularly related to this PR, as it's basically done nowhere in the controller, but
cmd->handleis only 12 bits. If we receive a a handle of0xFFFFwe should returnBT_HCI_ERR_INVALID_PARAM, right?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.
Very good point. Maybe it's worth making a Github issue and fixing this file up? 👍
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.
Sure, could be a nice first-time issue as it's just fixing a missing check several places :) Chances are that it's being handled somewhere else that I'm not aware of @cvinayak ?