Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions subsys/bluetooth/controller/Kconfig.ll_sw_split
Original file line number Diff line number Diff line change
Expand Up @@ -1154,6 +1154,20 @@ config BT_CTLR_CONN_RSSI_EVENT
help
Generate events for connection RSSI measurement.

config BT_CTLR_CHAN_METRICS_EVENT
bool "Channel Metrics event"
help
Generate events for channel usage metrics.

config BT_CTLR_CHAN_METRICS_BAD_COUNT
int "Channel bad count threshold"
depends on BT_CTLR_CHAN_METRICS_EVENT
Comment on lines +1157 to +1164
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could consider doing so that BT_CTLR_CHAN_METRICS_BAD_COUNT defaults to 0 and omit BT_CTLR_CHAN_METRICS_EVENT, and then you can just use a single Kconfig option for this purpose. If you want to have a bool, you could also do

config BT_CTLR_CHAN_METRICS_EVENT
        defbool BT_CTLR_CHAN_METRICS_BAD_COUNT > 0

config BT_CTLR_CHAN_METRICS_BAD_COUNT
	int "Channel bad count threshold"

It is also OK as is if you prefer that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer the current way because this can be extended in the future to measure (or have threshold for) other details like CRC error counts, or retransmission counts etc,

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure :) Just FYI, you would still be able to extend with my suggestion assuming that BT_CTLR_CHAN_METRICS_BAD_COUNT still needs to be minimum 1 for the feature to be enabled

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, say, RSSI metrics per channel is added. Then, CONFIG_BT_CTLR_CHAN_METRICS_BAD_COUNT=0 and CONFIG_BT_CTLR_CHAN_METRICS_LOW_RSSI_COUNT=3 to generate event on 3 consecutive low RSSI measurement on the channel.

range 1 $(UINT8_MAX)
default 3
help
Set the number of consecutive failures on transmission acknowledgment that will generate
a channel metrics event.

config BT_CTLR_ALLOW_SAME_PEER_CONN
bool "Allow connection requests from same peer"
depends on BT_MAX_CONN > 1
Expand Down
11 changes: 11 additions & 0 deletions subsys/bluetooth/controller/hci/hci.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#include "ll_sw/pdu.h"

#include "ll_sw/lll.h"
#include "ll_sw/lll_chan.h"
#include "lll/lll_adv_types.h"
#include "ll_sw/lll_adv.h"
#include "lll/lll_adv_pdu.h"
Expand Down Expand Up @@ -8902,6 +8903,12 @@ static void encode_control(struct node_rx_pdu *node_rx,
return;
#endif /* CONFIG_BT_CTLR_CONN_RSSI_EVENT */

#if defined(CONFIG_BT_CTLR_CHAN_METRICS_EVENT)
case NODE_RX_TYPE_CHAN_METRICS:
lll_chan_metrics_print();
return;
#endif /* CONFIG_BT_CTLR_CHAN_METRICS_EVENT */

#if defined(CONFIG_BT_CTLR_PERIPHERAL_ISO)
case NODE_RX_TYPE_CIS_REQUEST:
le_cis_request(pdu_data, node_rx, buf);
Expand Down Expand Up @@ -9407,6 +9414,10 @@ uint8_t hci_get_class(struct node_rx_pdu *node_rx)
case NODE_RX_TYPE_RSSI:
#endif /* CONFIG_BT_CTLR_CONN_RSSI_EVENT */

#if defined(CONFIG_BT_CTLR_CHAN_METRICS_EVENT)
case NODE_RX_TYPE_CHAN_METRICS:
#endif /* CONFIG_BT_CTLR_CHAN_METRICS_EVENT */

#if defined(CONFIG_BT_CTLR_LE_PING)
case NODE_RX_TYPE_APTO:
#endif /* CONFIG_BT_CTLR_LE_PING */
Expand Down
1 change: 1 addition & 0 deletions subsys/bluetooth/controller/ll_sw/lll.h
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ enum node_rx_type {
NODE_RX_TYPE_CHAN_SEL_ALGO,
NODE_RX_TYPE_PHY_UPDATE,
NODE_RX_TYPE_RSSI,
NODE_RX_TYPE_CHAN_METRICS,
NODE_RX_TYPE_PROFILE,
NODE_RX_TYPE_ADV_INDICATION,
NODE_RX_TYPE_SCAN_INDICATION,
Expand Down
150 changes: 150 additions & 0 deletions subsys/bluetooth/controller/ll_sw/lll_chan.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <stdint.h>
#include <stdbool.h>
#include <string.h>

#include "hal/ccm.h"
#include "hal/radio.h"
Expand Down Expand Up @@ -331,6 +332,155 @@ static uint8_t chan_d(uint8_t n)
}
#endif /* CONFIG_BT_CTLR_ISO */

#if defined(CONFIG_BT_CTLR_CHAN_METRICS_EVENT)
#include "hal/cpu.h"

#define CHM_SURVEY_CHAN_COUNT_MAX 37U

static struct {
struct chan_metrics_chan {
uint16_t count; /* Total metrics count */
uint16_t prev; /* Previous total metrics count when good tx acknowledgment */
uint16_t good; /* Total good tx acknowledgments */
} chan[CHM_SURVEY_CHAN_COUNT_MAX];

/* Differential state, context-safe, to notify upper layer */
uint8_t req;
uint8_t ack;
} chan_metrics;

void lll_chan_metrics_init(void)
{
(void)memset((void *)&chan_metrics, 0U, sizeof(chan_metrics));
}

bool lll_chan_metrics_is_notify(void)
{
return chan_metrics.req != chan_metrics.ack;
}

bool lll_chan_metrics_notify_clear(void)
{
if (lll_chan_metrics_is_notify()) {
chan_metrics.ack = chan_metrics.req;

return true;
}

return false;
}

static bool chan_metrics_notify_set(void)
{
uint8_t req;

req = chan_metrics.req + 1U;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could potentially overflow

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is intentional, the follow check ensured the assignment does not rollover.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I follow.

if chan_metrics.req is 255, then req becomes 0, and then chan_metrics.req becomes 0 if req != chan_metrics.ack.

If overflow is intentional and support (although I'm not fully following the logic here), then please add a comment stating that. We should avoid having implicit over- and underflows IMO

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (req != chan_metrics.ack) {
chan_metrics.req = req;
cpu_dmb(); /* data memory barrier */

return true;
}

return false;
}

static struct chan_metrics_chan *chan_metrics_count_inc(uint8_t chan_idx)
{
struct chan_metrics_chan *chan;

LL_ASSERT(chan_idx < CHM_SURVEY_CHAN_COUNT_MAX);

/* Mitigate metrics overflow, half the collected good and count value */
chan = &chan_metrics.chan[chan_idx];
if (chan->count == UINT16_MAX) {
uint16_t diff;

/* Current consecutive bad versus good difference */
diff = chan->count - chan->prev;

/* Half the count and keep the current consecutive bad versus good difference.
* We do not want to rollover `UINT16_MAX`, hence we normalize the `count` and
* `good`, but keep the absolute `diff` between `total` and `prev`.
Copy link

Copilot AI Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment mentions 'absolute diff between total and prev' but the variable is called count, not total. Update the comment to use consistent variable names.

Suggested change
* `good`, but keep the absolute `diff` between `total` and `prev`.
* `good`, but keep the absolute `diff` between `count` and `prev`.

Copilot uses AI. Check for mistakes.

*/
chan->count >>= 1;
if (diff < chan->count) {
chan->prev = chan->count - diff;
} else {
chan->prev = 0U;
}

/* Half the good count */
chan->good >>= 1;
}

/* Increment the channel metrics count */
chan->count++;

return chan;
}

void lll_chan_metrics_chan_bad(uint8_t chan_idx)
{
struct chan_metrics_chan *chan;
uint16_t diff;

/* Increment the per channel metrics count */
chan = chan_metrics_count_inc(chan_idx);

/* Notify beyond a bad versus good difference */
diff = chan->count - chan->prev;
if (diff > CONFIG_BT_CTLR_CHAN_METRICS_BAD_COUNT) {
(void)chan_metrics_notify_set();

/* Reset consecutive bad versus good difference */
chan->prev = chan->count;
}
}

void lll_chan_metrics_chan_good(uint8_t chan_idx)
{
struct chan_metrics_chan *chan;

/* Increment the per channel metrics count */
chan = chan_metrics_count_inc(chan_idx);

/* Reset the consecutive bad versus good difference */
chan->prev = chan->count;

/* Increment good count */
chan->good++;
}

void lll_chan_metrics_print(void)
{
const uint8_t max = 100U;

printk("%s:\n", __func__);
printk("chan #: (actual / expected reception) percentage -\n");
Copy link

Copilot AI Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment 'expected reception' is misleading since count represents total attempts (both good and bad), not expected receptions. Consider changing to 'total attempts' for clarity.

Suggested change
printk("chan #: (actual / expected reception) percentage -\n");
printk("chan #: (actual / total attempts) percentage -\n");

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected reception represents best the expected reception count (for the attempts)

for (uint8_t i = 0; i < CHM_SURVEY_CHAN_COUNT_MAX; i++) {
const struct chan_metrics_chan *chan;
uint8_t cnt;
char c;

chan = &chan_metrics.chan[i];
if (chan->count != 0U) {
cnt = chan->good * max / chan->count;
c = '*';
} else {
cnt = max;
c = '-';
}

printk("%02d: (%05u / %05u) %03u - ", i, chan->good, chan->count, cnt);
Copy link

Copilot AI Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The magic numbers 02d, 05u, and 03u in the printf format string should be defined as constants to improve maintainability and make the output format more consistent.

Copilot uses AI. Check for mistakes.

for (uint8_t j = 0; j < cnt; j++) {
printk("%c", c);
}
printk("\n");
}
}
#endif /* CONFIG_BT_CTLR_CHAN_METRICS_EVENT */

#if defined(CONFIG_BT_CTLR_TEST)
/* Refer to Bluetooth Specification v5.2 Vol 6, Part C, Section 3 LE Channel
* Selection algorithm #2 sample data
Expand Down
41 changes: 41 additions & 0 deletions subsys/bluetooth/controller/ll_sw/lll_chan.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@
* SPDX-License-Identifier: Apache-2.0
*/

#if defined(CONFIG_BT_CTLR_CHAN_METRICS_EVENT)
#define LLL_CHAN_METRICS
#else /* !CONFIG_BT_CTLR_CHAN_METRICS_EVENT */
#define LLL_CHAN_METRICS static __attribute__((always_inline)) inline
#endif /* !CONFIG_BT_CTLR_CHAN_METRICS_EVENT */

uint8_t lll_chan_sel_1(uint8_t *chan_use, uint8_t hop, uint16_t latency,
uint8_t *chan_map, uint8_t chan_count);

Expand All @@ -18,4 +24,39 @@ uint8_t lll_chan_iso_subevent(uint16_t chan_id, const uint8_t *chan_map,
uint8_t chan_count, uint16_t *prn_subevent_lu,
uint16_t *remap_idx);

LLL_CHAN_METRICS void lll_chan_metrics_init(void);
LLL_CHAN_METRICS void lll_chan_metrics_chan_bad(uint8_t chan_idx);
LLL_CHAN_METRICS void lll_chan_metrics_chan_good(uint8_t chan_idx);
LLL_CHAN_METRICS bool lll_chan_metrics_is_notify(void);
LLL_CHAN_METRICS bool lll_chan_metrics_notify_clear(void);
LLL_CHAN_METRICS void lll_chan_metrics_print(void);

void lll_chan_sel_2_ut(void);

#if !defined(CONFIG_BT_CTLR_CHAN_METRICS_EVENT)
LLL_CHAN_METRICS void lll_chan_metrics_init(void)
{
}

LLL_CHAN_METRICS void lll_chan_metrics_chan_bad(uint8_t chan_idx)
{
}

LLL_CHAN_METRICS void lll_chan_metrics_chan_good(uint8_t chan_idx)
{
}

LLL_CHAN_METRICS bool lll_chan_metrics_is_notify(void)
{
return false;
}

LLL_CHAN_METRICS bool lll_chan_metrics_notify_clear(void)
{
return false;
}

LLL_CHAN_METRICS void lll_chan_metrics_print(void)
{
}
#endif /* !CONFIG_BT_CTLR_CHAN_METRICS_EVENT */
13 changes: 13 additions & 0 deletions subsys/bluetooth/controller/ll_sw/lll_conn.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,20 @@ struct lll_conn {
uint8_t initiated:1;
uint8_t cancelled:1;
uint8_t forced:1;

#if defined(CONFIG_BT_CTLR_CHAN_METRICS_EVENT)
uint8_t chan_curr;
#endif /* CONFIG_BT_CTLR_CHAN_METRICS_EVENT */
};

struct {
uint8_t initiated:1;
uint8_t cancelled:1;
uint8_t forced:1;

#if defined(CONFIG_BT_CTLR_CHAN_METRICS_EVENT)
uint8_t chan_curr;
#endif /* CONFIG_BT_CTLR_CHAN_METRICS_EVENT */
} central;

#if defined(CONFIG_BT_PERIPHERAL)
Expand All @@ -111,6 +119,11 @@ struct lll_conn {
uint8_t phy_rx_event:3;
#endif /* CONFIG_BT_CTLR_PHY */

#if defined(CONFIG_BT_CTLR_CHAN_METRICS_EVENT)
uint8_t chan_curr;
uint8_t chan_prev;
#endif /* CONFIG_BT_CTLR_CHAN_METRICS_EVENT */

uint32_t window_widening_periodic_us;
uint32_t window_widening_max_us;
uint32_t window_widening_prepare_us;
Expand Down
1 change: 0 additions & 1 deletion subsys/bluetooth/controller/ll_sw/nordic/lll/lll.c
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,6 @@ void lll_chan_set(uint32_t chan)
radio_whiten_iv_set(chan);
}


uint32_t lll_radio_is_idle(void)
{
return radio_is_idle();
Expand Down
4 changes: 4 additions & 0 deletions subsys/bluetooth/controller/ll_sw/nordic/lll/lll_central.c
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,10 @@ static int prepare_cb(struct lll_prepare_param *p)
sys_get_le24(lll->crc_init));
lll_chan_set(data_chan_use);

#if defined(CONFIG_BT_CTLR_CHAN_METRICS_EVENT)
lll->central.chan_curr = data_chan_use;
#endif /* CONFIG_BT_CTLR_CHAN_METRICS_EVENT */

lll_conn_tx_pkt_set(lll, pdu_data_tx);

radio_isr_set(lll_conn_isr_tx, lll);
Expand Down
32 changes: 32 additions & 0 deletions subsys/bluetooth/controller/ll_sw/nordic/lll/lll_conn.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "lll_df_types.h"
#include "lll_df.h"
#include "lll_conn.h"
#include "lll_chan.h"

#include "lll_internal.h"
#include "lll_df_internal.h"
Expand Down Expand Up @@ -389,6 +390,16 @@ void lll_conn_isr_rx(void *param)
goto lll_conn_isr_rx_exit;
}

#if defined(CONFIG_BT_CTLR_CHAN_METRICS_EVENT)
/* Increase bad channel count for previously a CRC error */
if (crc_expire != 0U) {
lll_chan_metrics_chan_bad(lll->chan_curr);
}

/* Increase good channel count for successful reception */
lll_chan_metrics_chan_good(lll->chan_curr);
#endif /* CONFIG_BT_CTLR_CHAN_METRICS_EVENT */

/* Reset CRC expiry counter */
crc_expire = 0U;

Expand All @@ -403,8 +414,22 @@ void lll_conn_isr_rx(void *param)
/* CRC error countdown */
crc_expire--;
is_done = (crc_expire == 0U);

#if defined(CONFIG_BT_CTLR_CHAN_METRICS_EVENT)
/* Increase bad channel count for consecutive CRC error */
if (is_done != 0U) {
lll_chan_metrics_chan_bad(lll->chan_curr);
}
#endif /* CONFIG_BT_CTLR_CHAN_METRICS_EVENT */
}

#if defined(CONFIG_BT_CTLR_CHAN_METRICS_EVENT) && defined(CONFIG_BT_PERIPHERAL)
/* Peripheral tx-rx starts using current channel */
if (lll->role == BT_HCI_ROLE_PERIPHERAL) {
lll->periph.chan_prev = lll->periph.chan_curr;
}
#endif /* CONFIG_BT_CTLR_CHAN_METRICS_EVENT && CONFIG_BT_PERIPHERAL */

#if defined(CONFIG_BT_CTLR_DF_CONN_CTE_RX) && defined(CONFIG_BT_CTLR_LE_ENC)
if (lll->enc_rx) {
struct pdu_data *pdu_scratch;
Expand Down Expand Up @@ -1110,6 +1135,13 @@ static inline int isr_rx_pdu(struct lll_conn *lll, struct pdu_data *pdu_data_rx,
struct node_tx *tx;
memq_link_t *link;

#if defined(CONFIG_BT_CTLR_CHAN_METRICS_EVENT) && defined(CONFIG_BT_PERIPHERAL)
/* Previous event used channel is good as ack-ed by the peer */
if (lll->role == BT_HCI_ROLE_PERIPHERAL) {
lll_chan_metrics_chan_good(lll->periph.chan_prev);
}
#endif /* CONFIG_BT_CTLR_CHAN_METRICS_EVENT && CONFIG_BT_PERIPHERAL */

/* Increment sequence number */
lll->sn++;

Expand Down
Loading