Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds Telemetry 2 (T2) reporting for BLE RCU pairing attempts and consolidates telemetry reporting categories by replacing separate BLE/RF4CE report types with a single RCU type.
Changes:
- Rename telemetry report category from
RF4CE/BLEtoRCUand update TR-181 trigger constants accordingly. - Thread BlueZ pairing retry counters and pairing outcome details up through the BLE pairing state machine and controller layers.
- Emit a new T2 marker (
ctrlm.rcu.pairing.attempt) with a JSON payload describing pairing method, result, retries, discovered devices, and error message.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/telemetry/ctrlm_telemetry.h | Consolidates telemetry report types to include RCU instead of separate BLE/RF4CE. |
| src/telemetry/ctrlm_telemetry.cpp | Updates report trigger/string mapping to use the new RCU report type. |
| src/telemetry/ctrlm_telemetry_markers.h | Adds marker definition and schema documentation for RCU pairing attempt telemetry. |
| src/ctrlm_tr181.h | Replaces BLE/RF4CE telemetry report TR-181 keys with a unified rcu key. |
| src/ble/hal/blercu/bluez/blercudevice.cpp | Extends pairing error signal to include retry counters and max retries. |
| src/ble/hal/blercu/bluez/blercuadapter.cpp | Propagates pairing error retry details from device to adapter slots. |
| src/ble/hal/blercu/bluez/blercuadapter_p.h | Updates adapter pairing error handler signature to include retry details. |
| src/ble/hal/blercu/blercupairingstatemachine.h | Introduces pairing outcome tracking types/fields and public getters. |
| src/ble/hal/blercu/blercupairingstatemachine.cpp | Records pairing method/outcome, discovered devices, retry counts, and error details. |
| src/ble/hal/blercu/blercudevice.h | Updates device pairing error slot signature to include retry counters. |
| src/ble/hal/blercu/blercucontroller.h | Adds BleRcuPairingOutcome struct and slot for pairing outcome notifications. |
| src/ble/hal/blercu/blercucontroller.cpp | Emits BleRcuPairingOutcome on success/failure with method/result/devices/errors. |
| src/ble/hal/blercu/blercuadapter.h | Updates adapter pairing error slot signature to include retry counters. |
| src/ble/ctrlm_ble_rcu_interface.h | Adds handler registration for pairing outcome notifications. |
| src/ble/ctrlm_ble_rcu_interface.cpp | Wires controller pairing outcome into the interface slot fan-out. |
| src/ble/ctrlm_ble_network.h | Adds a BLE pairing-attempt aggregation struct for telemetry emission. |
| src/ble/ctrlm_ble_network.cpp | Queues pairing outcome to main thread and emits the T2 marker JSON. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
b'## WARNING: A Blackduck scan failure has been waived A prior failure has been upvoted
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 11 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ea2cfbc to
b00781c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
src/ble/hal/blercu/bluez/blercudevice.cpp:253
pair()setsm_maxPairingRetriesbut does not resetm_pairingRetryCnt. If a previous pairing attempt succeeded after one or more retries,m_pairingRetryCntremains non-zero and subsequent pairing attempts will start “part-way through” the retry budget (and emit incorrect retry counts). Resetm_pairingRetryCnt(and optionallym_maxPairingRetries) when starting a new pairing attempt and/or on successful pairing reply.
void BleRcuDeviceBluez::pair(int timeout, int retries)
{
m_maxPairingRetries = retries;
m_deviceProxy->Pair(
PendingReply<>(m_isAlive, std::bind(&BleRcuDeviceBluez::onPairRequestReply, this, std::placeholders::_1)));
// set the flag, it may be cleared if the call fails
m_isPairing = true;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| bool ctrlm_telemetry_event_t<std::string>::event() const { | ||
| XLOGD_TELEMETRY("telemetry event <%s, %s>", this->marker.c_str(), this->value.c_str()); | ||
| return(t2_event_s((char *)this->marker.c_str(), (char *)this->value.c_str()) == T2ERROR_SUCCESS); | ||
| if (value.length() > CTRLM_TELEMETRY_MAX_EVENT_SIZE_BYTES) { |
There was a problem hiding this comment.
I'm not sure if this is going to work as intended. This should be explicitly tested. For the voice session telemetry, the size applied to the block of data in the entire 15 minute window. Since this is using the accumulate feature, I'm not sure what the agreement is with respect to the size of each individual event.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Reason for change: Test Procedure: Risks: Signed-off-by: Kelvin Lu <Kelvin_Lu@comcast.com>
14c5cce to
a699481
Compare
No description provided.