From 223e92d030fa56533a7eeb026b30a87af41bde29 Mon Sep 17 00:00:00 2001 From: klu339 Date: Tue, 31 Mar 2026 14:45:46 +0000 Subject: [PATCH 1/5] RDKEMW-15315: T2 RCU pairing --- src/ble/ctrlm_ble_network.cpp | 72 ++++++++++ src/ble/ctrlm_ble_network.h | 18 +++ src/ble/ctrlm_ble_rcu_interface.cpp | 6 + src/ble/ctrlm_ble_rcu_interface.h | 5 + src/ble/hal/blercu/blercuadapter.h | 4 +- src/ble/hal/blercu/blercucontroller.cpp | 51 +++++++ src/ble/hal/blercu/blercucontroller.h | 18 +++ src/ble/hal/blercu/blercudevice.h | 5 +- .../hal/blercu/blercupairingstatemachine.cpp | 125 +++++++++++++++++- .../hal/blercu/blercupairingstatemachine.h | 45 ++++++- src/ble/hal/blercu/bluez/blercuadapter.cpp | 10 +- src/ble/hal/blercu/bluez/blercuadapter_p.h | 2 +- src/ble/hal/blercu/bluez/blercudevice.cpp | 3 +- src/ctrlm_tr181.h | 3 +- src/telemetry/ctrlm_telemetry.cpp | 9 +- src/telemetry/ctrlm_telemetry.h | 3 +- src/telemetry/ctrlm_telemetry_markers.h | 22 +++ 17 files changed, 376 insertions(+), 25 deletions(-) diff --git a/src/ble/ctrlm_ble_network.cpp b/src/ble/ctrlm_ble_network.cpp index 03e1322f..810e6dc3 100644 --- a/src/ble/ctrlm_ble_network.cpp +++ b/src/ble/ctrlm_ble_network.cpp @@ -47,6 +47,7 @@ #include #include #include "ctrlm_rcp_ipc_iarm_thunder.h" +#include "ctrlm_telemetry.h" using namespace std; @@ -370,6 +371,9 @@ ctrlm_hal_result_t ctrlm_obj_network_ble_t::hal_init_request(GThread *ctrlm_main ble_rcu_interface_->addRcuKeypressHandler(Slot(is_alive_, std::bind(&ctrlm_obj_network_ble_t::ind_keypress, this, std::placeholders::_1))); + ble_rcu_interface_->addRcuPairingOutcomeHandler(Slot(is_alive_, + std::bind(&ctrlm_obj_network_ble_t::ind_rcu_pairing_outcome, this, std::placeholders::_1))); + ble_rcu_interface_->startKeyMonitorThread(); @@ -1673,6 +1677,74 @@ void ctrlm_obj_network_ble_t::req_process_unpair(void *data, int size) { // BEGIN - Process Indications from HAL to the network in CTRLM Main thread context // ------------------------------------------------------------------------------------------------------------------------------------------------------------------ +void ctrlm_obj_network_ble_t::ind_rcu_pairing_outcome(const BleRcuPairingOutcome &outcome) { + // Queue to the ctrlm main thread for duration computation and telemetry emission + auto copy = std::make_shared(outcome); + ctrlm_main_queue_handler_push_new( + CTRLM_HANDLER_NETWORK, + (ctrlm_msg_handler_network_t)&ctrlm_obj_network_ble_t::ind_process_rcu_pairing_outcome, + copy, + NULL, + id_); +} + +void ctrlm_obj_network_ble_t::ind_process_rcu_pairing_outcome(void *data, int size) { + THREAD_ID_VALIDATE(); + const BleRcuPairingOutcome *outcome = static_cast(data); + if (!outcome) { + XLOGD_ERROR("pairing outcome data is NULL"); + return; + } + + ctrlm_ble_pair_attempt_t attempt; + attempt.method = outcome->method; + attempt.result = outcome->result; + attempt.discovered = outcome->discovered; + attempt.bluez_retries = outcome->bluezRetries; + attempt.paired_mac = outcome->pairedMac; + attempt.error_msg = outcome->bluezError; + + XLOGD_INFO("BLE pairing attempt complete: method=%s result=%s paired_mac=%s " + "bluez_retries=%d bluez_msg=%s discovered=%zu", + attempt.method.c_str(), attempt.result.c_str(), attempt.paired_mac.c_str(), + attempt.bluez_retries, attempt.error_msg.c_str(), attempt.discovered.size()); + +#ifdef TELEMETRY_SUPPORT + // Serialize to JSON compatible with the ctrlm.ble.pairing.attempt T2 marker + json_t *jroot = json_object(); + if (jroot) { + json_object_set_new(jroot, "version", json_integer(MARKER_RCU_PAIRING_ATTEMPT_VERSION)); + json_object_set_new(jroot, "method", json_string(attempt.method.c_str())); + json_object_set_new(jroot, "result", json_string(attempt.result.c_str())); + json_object_set_new(jroot, "paired_mac", json_string(attempt.paired_mac.c_str())); + json_object_set_new(jroot, "bluez_retries", json_integer(attempt.bluez_retries)); + json_object_set_new(jroot, "bluez_msg", json_string(attempt.error_msg.c_str())); + + json_t *jdiscovered = json_array(); + if (jdiscovered) { + for (const auto &dev : attempt.discovered) { + json_t *jdev = json_object(); + if (jdev) { + json_object_set_new(jdev, "mac", json_string(dev.first.c_str())); + json_object_set_new(jdev, "name", json_string(dev.second.c_str())); + json_array_append_new(jdiscovered, jdev); + } + } + json_object_set_new(jroot, "discovered", jdiscovered); + } + + char *json_str = json_dumps(jroot, JSON_COMPACT); + XLOGD_WARN("KLU339 %s", json_str); + if (json_str) { + ctrlm_telemetry_event_t ev(MARKER_RCU_PAIRING_ATTEMPT, std::string(json_str)); + ctrlm_telemetry_t::get_instance()->event(ctrlm_telemetry_report_t::RCU, ev); + free(json_str); + } + json_decref(jroot); + } +#endif // TELEMETRY_SUPPORT +} + void ctrlm_obj_network_ble_t::ind_rcu_status(ctrlm_hal_ble_RcuStatusData_t *params) { // push to the main queue and process it synchronously there diff --git a/src/ble/ctrlm_ble_network.h b/src/ble/ctrlm_ble_network.h index 1182889a..c0702980 100644 --- a/src/ble/ctrlm_ble_network.h +++ b/src/ble/ctrlm_ble_network.h @@ -92,6 +92,22 @@ typedef struct { } ctrlm_ble_upgrade_image_info_t; +/////////////////////////////////////////////////////////////////////////////////////////// + +/** + * @brief BLE RCU Pairing Attempt Metrics + * + * Aggregates the outcome of a single BLE pairing attempt for telemetry emission. + */ +struct ctrlm_ble_pair_attempt_t { + std::string method; // pairing method string (auto_timeout, ir_code, mac_hash, mac_list) + std::string result; // "success" or failure reason string + std::vector> discovered; // {mac_str, was_paired} + int bluez_retries; + std::string paired_mac; // empty on failure + std::string error_msg; +}; + /////////////////////////////////////////////////////////////////////////////////////////// /** @@ -134,11 +150,13 @@ class ctrlm_obj_network_ble_t : public ctrlm_obj_network_t { void ind_rcu_paired(ctrlm_hal_ble_IndPaired_params_t *params); void ind_rcu_unpaired(ctrlm_hal_ble_IndUnPaired_params_t *params); void ind_keypress(ctrlm_hal_ble_IndKeypress_params_t *params); + void ind_rcu_pairing_outcome(const BleRcuPairingOutcome &outcome); void ind_process_rcu_status(void *data, int size); void ind_process_paired(void *data, int size); void ind_process_unpaired(void *data, int size); void ind_process_keypress(void *data, int size); + void ind_process_rcu_pairing_outcome(void *data, int size); virtual void req_process_network_status(void *data, int size); virtual void req_process_controller_status(void *data, int size); diff --git a/src/ble/ctrlm_ble_rcu_interface.cpp b/src/ble/ctrlm_ble_rcu_interface.cpp index 6abecad1..abec9dc0 100644 --- a/src/ble/ctrlm_ble_rcu_interface.cpp +++ b/src/ble/ctrlm_ble_rcu_interface.cpp @@ -185,6 +185,12 @@ void ctrlm_ble_rcu_interface_t::initialize() m_rcuUnpairedSlots.invoke(¶ms); }; m_controller->addManagedDeviceRemovedSlot(Slot(m_isAlive, deviceRemovedSlot)); + + auto pairingOutcomeSlot = [this](const BleRcuPairingOutcome &outcome) + { + m_rcuPairingOutcomeSlots.invoke(outcome); + }; + m_controller->addPairingOutcomeSlot(Slot(m_isAlive, pairingOutcomeSlot)); } voice_params_par_t params; diff --git a/src/ble/ctrlm_ble_rcu_interface.h b/src/ble/ctrlm_ble_rcu_interface.h index a6526e1b..0dc1cc38 100644 --- a/src/ble/ctrlm_ble_rcu_interface.h +++ b/src/ble/ctrlm_ble_rcu_interface.h @@ -146,12 +146,17 @@ class ctrlm_ble_rcu_interface_t { m_rcuKeypressSlots.addSlot(func); } + inline void addRcuPairingOutcomeHandler(const Slot &func) + { + m_rcuPairingOutcomeSlots.addSlot(func); + } Slots m_rcuStatusChangedSlots; Slots m_rcuPairedSlots; Slots m_rcuUnpairedSlots; Slots m_rcuKeypressSlots; + Slots m_rcuPairingOutcomeSlots; private: std::shared_ptr m_isAlive; diff --git a/src/ble/hal/blercu/blercuadapter.h b/src/ble/hal/blercu/blercuadapter.h index cd9fc9e8..d795acb8 100644 --- a/src/ble/hal/blercu/blercuadapter.h +++ b/src/ble/hal/blercu/blercuadapter.h @@ -107,7 +107,7 @@ class BleRcuAdapter { m_deviceNameChangedSlots.addSlot(func); } - inline void addDevicePairingErrorSlot(const Slot &func) + inline void addDevicePairingErrorSlot(const Slot &func) { m_devicePairingErrorSlots.addSlot(func); } @@ -135,7 +135,7 @@ class BleRcuAdapter Slots m_deviceFoundSlots; Slots m_deviceRemovedSlots; Slots m_deviceNameChangedSlots; - Slots m_devicePairingErrorSlots; + Slots m_devicePairingErrorSlots; Slots m_devicePairingChangedSlots; Slots m_deviceReadyChangedSlots; diff --git a/src/ble/hal/blercu/blercucontroller.cpp b/src/ble/hal/blercu/blercucontroller.cpp index eb1b39ac..7a5c0891 100644 --- a/src/ble/hal/blercu/blercucontroller.cpp +++ b/src/ble/hal/blercu/blercucontroller.cpp @@ -41,6 +41,34 @@ using namespace std; +static const char* pairingMethodStr(BleRcuPairingStateMachine::PairingMethod m) +{ + switch (m) { + case BleRcuPairingStateMachine::AUTO_TIMEOUT: return "auto_timeout"; + case BleRcuPairingStateMachine::MAC_LIST: return "mac_list"; + case BleRcuPairingStateMachine::IR_CODE: return "ir_code"; + case BleRcuPairingStateMachine::MAC_HASH: return "mac_hash"; + default: return "unknown"; + } +} + +static const char* failureReasonStr(BleRcuPairingStateMachine::FailureReason r) +{ + switch (r) { + case BleRcuPairingStateMachine::SUCCESS: return "success"; + case BleRcuPairingStateMachine::FAIL_DISCOVERY_TIMEOUT: return "discovery_timeout"; + case BleRcuPairingStateMachine::FAIL_DISCOVERY_STOPPED: return "discovery_stopped"; + case BleRcuPairingStateMachine::FAIL_DISCOVERY_STOP_TIMEOUT:return "discovery_stop_timeout"; + case BleRcuPairingStateMachine::FAIL_PAIRING_TIMEOUT: return "pairing_timeout"; + case BleRcuPairingStateMachine::FAIL_BLUEZ_ERROR: return "bluez_error"; + case BleRcuPairingStateMachine::FAIL_ADAPTER_OFF: return "adapter_off"; + case BleRcuPairingStateMachine::FAIL_DEVICE_UNPAIRED: return "device_unpaired"; + case BleRcuPairingStateMachine::FAIL_DEVICE_REMOVED: return "device_removed"; + case BleRcuPairingStateMachine::FAIL_CANCELLED: return "cancelled"; + default: return "unknown"; + } +} + class BleRcuController_userdata { public: @@ -662,6 +690,17 @@ void BleRcuControllerImpl::onFinishedPairing() m_state = Complete; m_stateChangedSlots.invoke(m_state); + + // emit pairing outcome telemetry + BleRcuPairingOutcome outcome; + outcome.method = pairingMethodStr(m_pairingStateMachine.pairingMethod()); + outcome.result = failureReasonStr(BleRcuPairingStateMachine::SUCCESS); + outcome.bluezRetries = m_pairingStateMachine.bluezRetries(); + outcome.pairedMac = m_pairingStateMachine.pairedMac().toString(); + for (const auto &dev : m_pairingStateMachine.discoveredDevices()) { + outcome.discovered.emplace_back(dev.mac.toString(), dev.name); + } + m_pairingOutcomeSlots.invoke(outcome); } // ----------------------------------------------------------------------------- @@ -701,6 +740,18 @@ void BleRcuControllerImpl::onFailedPairing() m_state = Failed; m_stateChangedSlots.invoke(m_state); + + // emit pairing outcome telemetry + BleRcuPairingOutcome outcome; + outcome.method = pairingMethodStr(m_pairingStateMachine.pairingMethod()); + outcome.result = failureReasonStr(m_pairingStateMachine.failureReason()); + outcome.bluezRetries = m_pairingStateMachine.bluezRetries(); + outcome.pairedMac = m_pairingStateMachine.pairedMac().toString(); + outcome.bluezError = m_pairingStateMachine.bluezError(); + for (const auto &dev : m_pairingStateMachine.discoveredDevices()) { + outcome.discovered.emplace_back(dev.mac.toString(), dev.name); + } + m_pairingOutcomeSlots.invoke(outcome); } diff --git a/src/ble/hal/blercu/blercucontroller.h b/src/ble/hal/blercu/blercucontroller.h index a0b678cf..f8c6ebe5 100644 --- a/src/ble/hal/blercu/blercucontroller.h +++ b/src/ble/hal/blercu/blercucontroller.h @@ -36,10 +36,23 @@ #include #include +#include +#include +#include class BleRcuDevice; +struct BleRcuPairingOutcome +{ + std::string method; // "auto_timeout" | "ir_code" | "mac_hash" | "mac_list" + std::string result; // "success" | failure-reason string + std::vector> discovered; // {mac_str, was_paired} + int bluezRetries; + std::string pairedMac; // empty on failure + std::string bluezError; +}; + class BleRcuController { @@ -102,12 +115,17 @@ class BleRcuController { m_stateChangedSlots.addSlot(func); } + inline void addPairingOutcomeSlot(const Slot &func) + { + m_pairingOutcomeSlots.addSlot(func); + } protected: Slots m_managedDeviceAddedSlots; Slots m_managedDeviceRemovedSlots; Slots m_pairingStateChangedSlots; Slots m_stateChangedSlots; + Slots m_pairingOutcomeSlots; }; #endif // !defined(BLERCUCONTROLLER_H) diff --git a/src/ble/hal/blercu/blercudevice.h b/src/ble/hal/blercu/blercudevice.h index 8ab7cc44..c2ed4531 100644 --- a/src/ble/hal/blercu/blercudevice.h +++ b/src/ble/hal/blercu/blercudevice.h @@ -187,17 +187,18 @@ class BleRcuDevice { m_readyChangedSlots.addSlot(func); } - inline void addPairingErrorSlot(const Slot &func) + inline void addPairingErrorSlot(const Slot &func) { m_pairingErrorSlots.addSlot(func); } + protected: Slots m_connectedChangedSlots; Slots m_pairedChangedSlots; Slots m_nameChangedSlots; Slots m_readyChangedSlots; - Slots m_pairingErrorSlots; + Slots m_pairingErrorSlots; }; diff --git a/src/ble/hal/blercu/blercupairingstatemachine.cpp b/src/ble/hal/blercu/blercupairingstatemachine.cpp index bb769b41..37c0e8ee 100644 --- a/src/ble/hal/blercu/blercupairingstatemachine.cpp +++ b/src/ble/hal/blercu/blercupairingstatemachine.cpp @@ -52,6 +52,9 @@ BleRcuPairingStateMachine::BleRcuPairingStateMachine(const shared_ptraddPoweredChangedSlot(Slot(m_isAlive, std::bind(&BleRcuPairingStateMachine::onAdapterPoweredChanged, this, std::placeholders::_1))); - m_adapter->addDevicePairingErrorSlot(Slot(m_isAlive, - std::bind(&BleRcuPairingStateMachine::onDevicePairingError, this, std::placeholders::_1, std::placeholders::_2))); + m_adapter->addDevicePairingErrorSlot(Slot(m_isAlive, + std::bind(&BleRcuPairingStateMachine::onDevicePairingError, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, std::placeholders::_4))); } BleRcuPairingStateMachine::~BleRcuPairingStateMachine() @@ -207,6 +210,36 @@ bool BleRcuPairingStateMachine::isAutoPairing() const return isRunning() && m_isAutoPairing; } +BleRcuPairingStateMachine::PairingMethod BleRcuPairingStateMachine::pairingMethod() const +{ + return m_pairingMethod; +} + +BleRcuPairingStateMachine::FailureReason BleRcuPairingStateMachine::failureReason() const +{ + return m_failureReason; +} + +std::vector BleRcuPairingStateMachine::discoveredDevices() const +{ + return m_discoveredDevices; +} + +int BleRcuPairingStateMachine::bluezRetries() const +{ + return m_bluezRetries; +} + +BleAddress BleRcuPairingStateMachine::pairedMac() const +{ + return m_pairedMac; +} + +std::string BleRcuPairingStateMachine::bluezError() const +{ + return m_bluezErrorMsg; +} + // ----------------------------------------------------------------------------- /*! @@ -240,6 +273,14 @@ void BleRcuPairingStateMachine::startAutoWithTimeout(int timeoutMs) m_targetedPairingNames.push_back(name); } + // reset telemetry tracking state + m_pairingMethod = AUTO_TIMEOUT; + m_failureReason = SUCCESS; + m_discoveredDevices.clear(); + m_bluezRetries = 0; + m_pairedMac.clear(); + m_bluezErrorMsg.clear(); + // start the state machine m_stateMachine.start(); @@ -291,6 +332,14 @@ void BleRcuPairingStateMachine::startWithCode(uint8_t pairingCode) m_targetedPairingNames.push_back(std::regex(nameWithCode, std::regex_constants::ECMAScript)); } + // reset telemetry tracking state + m_pairingMethod = IR_CODE; + m_failureReason = SUCCESS; + m_discoveredDevices.clear(); + m_bluezRetries = 0; + m_pairedMac.clear(); + m_bluezErrorMsg.clear(); + // start the state machine m_stateMachine.start(); @@ -331,6 +380,14 @@ void BleRcuPairingStateMachine::startWithMacHash(uint8_t macHash) // clear the maps, we are trying to pair to a specific device using a hash of the MAC address m_targetedPairingNames.clear(); + // reset telemetry tracking state + m_pairingMethod = MAC_HASH; + m_failureReason = SUCCESS; + m_discoveredDevices.clear(); + m_bluezRetries = 0; + m_pairedMac.clear(); + m_bluezErrorMsg.clear(); + // start the state machine m_stateMachine.start(); @@ -370,6 +427,14 @@ void BleRcuPairingStateMachine::start(const BleAddress &target, const string &na m_targetedPairingNames.clear(); m_targetedPairingNames.push_back(std::regex(name, std::regex_constants::ECMAScript)); + // reset telemetry tracking state + m_pairingMethod = AUTO_TIMEOUT; + m_failureReason = SUCCESS; + m_discoveredDevices.clear(); + m_bluezRetries = 0; + m_pairedMac.clear(); + m_bluezErrorMsg.clear(); + // start the state machine m_stateMachine.start(); @@ -404,6 +469,14 @@ void BleRcuPairingStateMachine::startWithMacList(const std::vector & // set the list of addresses to filter for m_pairingMacList = macList; + // reset telemetry tracking state + m_pairingMethod = MAC_LIST; + m_failureReason = SUCCESS; + m_discoveredDevices.clear(); + m_bluezRetries = 0; + m_pairedMac.clear(); + m_bluezErrorMsg.clear(); + // start the state machine m_stateMachine.start(); @@ -436,6 +509,9 @@ void BleRcuPairingStateMachine::stop() XLOGD_INFO("cancelling pairing"); + // record the cancellation reason before posting the event + m_failureReason = FAIL_CANCELLED; + // post a cancel event and let the state-machine clean up m_stateMachine.postEvent(CancelRequestEvent); } @@ -518,12 +594,16 @@ void BleRcuPairingStateMachine::onStateTransition(int oldState, int newState) if (newState == FinishedState) { if (oldState == UnpairingState) { XLOGD_AUTOMATION_WARN("timed-out in un-pairing phase (failed rcu may be left paired)"); + m_failureReason = FAIL_PAIRING_TIMEOUT; } else if (oldState == StartingDiscoveryState) { XLOGD_AUTOMATION_ERROR("timed-out waiting for discovery started signal"); + m_failureReason = FAIL_DISCOVERY_TIMEOUT; } else if (oldState == DiscoveringState) { XLOGD_AUTOMATION_ERROR("timed-out in discovery phase (didn't find target rcu device to pair to)"); + m_failureReason = FAIL_DISCOVERY_TIMEOUT; } else if (oldState == StoppingDiscoveryState) { XLOGD_AUTOMATION_ERROR("timed-out waiting for discovery to stop (suggesting something has gone wrong inside bluez)"); + m_failureReason = FAIL_DISCOVERY_STOP_TIMEOUT; } } else if (newState == UnpairingState) { if (oldState == EnablePairableState || oldState == PairingState) { @@ -604,6 +684,9 @@ void BleRcuPairingStateMachine::onDiscoveryChanged(bool discovering) if (discovering) { m_stateMachine.postEvent(DiscoveryStartedEvent); } else { + if (m_stateMachine.inState(DiscoverySuperState)) { + m_failureReason = FAIL_DISCOVERY_STOPPED; + } m_stateMachine.postEvent(DiscoveryStoppedEvent); } } @@ -797,6 +880,7 @@ void BleRcuPairingStateMachine::onEnteredUnpairingState() // remove (unpair) the target device because we've failed :-( if (m_adapter->removeDevice(m_targetAddress) == false) { + m_failureReason = FAIL_PAIRING_TIMEOUT; m_stateMachine.postEvent(DeviceUnpairedEvent); } } @@ -964,6 +1048,17 @@ void BleRcuPairingStateMachine::onDeviceFound(const BleAddress &address, XLOGD_DEBUG("device added %s %s (target %s)", address.toString().c_str(), name.c_str(), m_targetAddress.toString().c_str()); + bool alreadyRecorded = false; + for (const auto &dev : m_discoveredDevices) { + if (dev.name == name) { alreadyRecorded = true; break; } + } + if (!alreadyRecorded) { + DiscoveredDevice dev; + dev.mac = address; + dev.name = name; + m_discoveredDevices.push_back(dev); + } + processDevice(address, name); } @@ -985,6 +1080,11 @@ void BleRcuPairingStateMachine::onDeviceRemoved(const BleAddress &address) // check if the device removed is the one we're targeting if (!m_targetAddress.isNull() && (m_targetAddress == address)) { + if (m_stateMachine.inState(UnpairingState)) { + m_failureReason = FAIL_PAIRING_TIMEOUT; + } else { + m_failureReason = FAIL_DEVICE_REMOVED; + } m_stateMachine.postEvent(DeviceRemovedEvent); } } @@ -1021,12 +1121,24 @@ void BleRcuPairingStateMachine::onDeviceNameChanged(const BleAddress &address, */ void BleRcuPairingStateMachine::onDevicePairingError(const BleAddress &address, - const string &error) + const string &error, + int retryCnt, + int maxRetryCnt) { if (!m_stateMachine.isRunning()) { return; } + m_failureReason = FAIL_BLUEZ_ERROR; + m_bluezRetries = retryCnt; + m_bluezErrorMsg = error; + m_pairedMac = address; + + if (retryCnt < maxRetryCnt) { + // Still retrying so don't stop pairing and timers yet + return; + } + XLOGD_ERROR("Device (%s) pairing failed, shutting down state machine...", address.toString().c_str()); // stop the pairing and setup timeout timers @@ -1059,6 +1171,11 @@ void BleRcuPairingStateMachine::onDevicePairingChanged(const BleAddress &address if (paired) { m_stateMachine.postEvent(DevicePairedEvent); } else { + if (m_stateMachine.inState(UnpairingState)) { + m_failureReason = FAIL_PAIRING_TIMEOUT; + } else { + m_failureReason = FAIL_DEVICE_UNPAIRED; + } m_stateMachine.postEvent(DeviceUnpairedEvent); } } @@ -1086,6 +1203,7 @@ void BleRcuPairingStateMachine::onDeviceReadyChanged(const BleAddress &address, if (ready) { m_pairingSuccesses++; m_pairingSucceeded = true; + m_pairedMac = address; m_stateMachine.postEvent(DeviceReadyEvent); } } @@ -1107,6 +1225,7 @@ void BleRcuPairingStateMachine::onAdapterPoweredChanged(bool powered) } if (!powered) { + m_failureReason = FAIL_ADAPTER_OFF; m_stateMachine.postEvent(AdapterPoweredOffEvent); } } diff --git a/src/ble/hal/blercu/blercupairingstatemachine.h b/src/ble/hal/blercu/blercupairingstatemachine.h index d21b46b1..50be05ad 100644 --- a/src/ble/hal/blercu/blercupairingstatemachine.h +++ b/src/ble/hal/blercu/blercupairingstatemachine.h @@ -40,13 +40,38 @@ #include #include - class BleRcuAdapter; class ConfigSettings; class BleRcuPairingStateMachine { +public: + enum PairingMethod { + AUTO_TIMEOUT, + MAC_LIST, + IR_CODE, + MAC_HASH + }; + + enum FailureReason { + SUCCESS = 0, + FAIL_DISCOVERY_TIMEOUT, + FAIL_DISCOVERY_STOPPED, + FAIL_DISCOVERY_STOP_TIMEOUT, + FAIL_PAIRING_TIMEOUT, + FAIL_BLUEZ_ERROR, + FAIL_ADAPTER_OFF, + FAIL_DEVICE_UNPAIRED, + FAIL_DEVICE_REMOVED, + FAIL_CANCELLED + }; + + struct DiscoveredDevice { + BleAddress mac; + std::string name; + }; + public: enum State { RunningSuperState, @@ -73,6 +98,14 @@ class BleRcuPairingStateMachine bool isAutoPairing() const; int pairingCode() const; +// Pairing outcome getters (valid after the state machine stops) + PairingMethod pairingMethod() const; + FailureReason failureReason() const; + std::vector discoveredDevices() const; + int bluezRetries() const; + BleAddress pairedMac() const; + std::string bluezError() const; + // public slots: void start(const BleAddress &target, const std::string &name); void startAutoWithTimeout(int timeoutMs); @@ -116,7 +149,7 @@ class BleRcuPairingStateMachine void onDeviceNameChanged(const BleAddress &address, const std::string &name); void onDevicePairingChanged(const BleAddress &address, bool paired); void onDeviceReadyChanged(const BleAddress &address, bool ready); - void onDevicePairingError(const BleAddress &address, const std::string &error); + void onDevicePairingError(const BleAddress &address, const std::string &error, int retryCnt, int maxRetryCnt); void onAdapterPoweredChanged(bool powered); @@ -174,6 +207,14 @@ class BleRcuPairingStateMachine int m_pairingSuccesses; bool m_pairingSucceeded; + // Pairing outcome tracking + PairingMethod m_pairingMethod; + FailureReason m_failureReason; + std::vector m_discoveredDevices; + int m_bluezRetries; + BleAddress m_pairedMac; + std::string m_bluezErrorMsg; + BtrMgrAdapter m_btrMgrAdapter; bool discoveryStartedExternally = false; BtrMgrAdapter::OperationType lastOperationType = BtrMgrAdapter::unknownOperation; diff --git a/src/ble/hal/blercu/bluez/blercuadapter.cpp b/src/ble/hal/blercu/bluez/blercuadapter.cpp index 56949f40..14cb0b5c 100644 --- a/src/ble/hal/blercu/bluez/blercuadapter.cpp +++ b/src/ble/hal/blercu/bluez/blercuadapter.cpp @@ -1245,8 +1245,8 @@ bool BleRcuAdapterBluez::addDevice(const BleAddress &address) XLOGD_INFO("requesting bluez pair %s", device->address().toString().c_str()); - device->addPairingErrorSlot(Slot(m_isAlive, - std::bind(&BleRcuAdapterBluez::onDevicePairingError, this, address, std::placeholders::_1))); + device->addPairingErrorSlot(Slot(m_isAlive, + std::bind(&BleRcuAdapterBluez::onDevicePairingError, this, address, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3))); device->pair(0); @@ -1688,9 +1688,11 @@ void BleRcuAdapterBluez::onDeviceNameChanged(const BleAddress &address, */ void BleRcuAdapterBluez::onDevicePairingError(const BleAddress &address, - const std::string &error) + const std::string &error, + int retryCnt, + int maxRetryCnt) { - m_devicePairingErrorSlots.invoke(address, error); + m_devicePairingErrorSlots.invoke(address, error, retryCnt, maxRetryCnt); } // ----------------------------------------------------------------------------- diff --git a/src/ble/hal/blercu/bluez/blercuadapter_p.h b/src/ble/hal/blercu/bluez/blercuadapter_p.h index aeb35f5c..d5124ed2 100644 --- a/src/ble/hal/blercu/bluez/blercuadapter_p.h +++ b/src/ble/hal/blercu/bluez/blercuadapter_p.h @@ -113,7 +113,7 @@ class BleRcuAdapterBluez : public BleRcuAdapter const std::vector &interfaces); void onDeviceNameChanged(const BleAddress &address, const std::string &name); - void onDevicePairingError(const BleAddress &address, const std::string &error); + void onDevicePairingError(const BleAddress &address, const std::string &error, int retryCnt, int maxRetryCnt); void onDevicePairedChanged(const BleAddress &address, bool paired); void onDeviceReadyChanged(const BleAddress &address, bool ready); diff --git a/src/ble/hal/blercu/bluez/blercudevice.cpp b/src/ble/hal/blercu/bluez/blercudevice.cpp index 08aa0287..2c2e02a7 100644 --- a/src/ble/hal/blercu/bluez/blercudevice.cpp +++ b/src/ble/hal/blercu/bluez/blercudevice.cpp @@ -272,8 +272,9 @@ void BleRcuDeviceBluez::onPairRequestReply(PendingReply<> *reply) pair(0); } else { m_pairingRetryCnt = 0; - m_pairingErrorSlots.invoke(reply->errorMessage()); } + int retryCnt = m_pairingRetryCnt; + m_pairingErrorSlots.invoke(reply->errorMessage(), retryCnt, m_maxPairingRetries); } else { XLOGD_DEBUG("%s pairing request successful", m_address.toString().c_str()); } diff --git a/src/ctrlm_tr181.h b/src/ctrlm_tr181.h index 8fdb0b34..d04ea769 100644 --- a/src/ctrlm_tr181.h +++ b/src/ctrlm_tr181.h @@ -59,8 +59,7 @@ #define CTRLM_TR181_VOICE_PARAMS_VSDK_CONFIGURATION "Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.Voice.VSDKConfiguration" #define CTRLM_TR181_VOICE_PARAMS_KEYWORD_SENSITIVITY "Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.Voice.KeywordSensitivity" #define CTRLM_TR181_TELEMETRY_REPORT_GLOBAL "Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.ctrlm.telemetry_report.global" -#define CTRLM_TR181_TELEMETRY_REPORT_RF4CE "Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.ctrlm.telemetry_report.rf4ce" -#define CTRLM_TR181_TELEMETRY_REPORT_BLE "Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.ctrlm.telemetry_report.ble" +#define CTRLM_TR181_TELEMETRY_REPORT_RCU "Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.ctrlm.telemetry_report.rcu" #define CTRLM_TR181_TELEMETRY_REPORT_IP "Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.ctrlm.telemetry_report.ip" #define CTRLM_TR181_TELEMETRY_REPORT_VOICE "Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.ctrlm.telemetry_report.voice" #define CTRLM_RT181_POWER_RFC_PWRMGR2 "Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.Power.PwrMgr2.Enable" diff --git a/src/telemetry/ctrlm_telemetry.cpp b/src/telemetry/ctrlm_telemetry.cpp index 3918444f..398bd369 100644 --- a/src/telemetry/ctrlm_telemetry.cpp +++ b/src/telemetry/ctrlm_telemetry.cpp @@ -60,8 +60,7 @@ ctrlm_telemetry_t::ctrlm_telemetry_t() { this->timeout_id = 0; this->set_duration(this->reporting_interval); this->event_reported[ctrlm_telemetry_report_t::GLOBAL] = false; - this->event_reported[ctrlm_telemetry_report_t::RF4CE] = false; - this->event_reported[ctrlm_telemetry_report_t::BLE] = false; + this->event_reported[ctrlm_telemetry_report_t::RCU] = false; this->event_reported[ctrlm_telemetry_report_t::IP] = false; this->event_reported[ctrlm_telemetry_report_t::VOICE] = false; } @@ -112,8 +111,7 @@ void ctrlm_telemetry_t::report() { const char *ctrlm_telemetry_t::get_report_trigger(ctrlm_telemetry_report_t report) { switch(report) { case ctrlm_telemetry_report_t::GLOBAL: return(CTRLM_TR181_TELEMETRY_REPORT_GLOBAL); - case ctrlm_telemetry_report_t::RF4CE: return(CTRLM_TR181_TELEMETRY_REPORT_RF4CE); - case ctrlm_telemetry_report_t::BLE: return(CTRLM_TR181_TELEMETRY_REPORT_BLE); + case ctrlm_telemetry_report_t::RCU: return(CTRLM_TR181_TELEMETRY_REPORT_RCU); case ctrlm_telemetry_report_t::IP: return(CTRLM_TR181_TELEMETRY_REPORT_IP); case ctrlm_telemetry_report_t::VOICE: return(CTRLM_TR181_TELEMETRY_REPORT_VOICE); } @@ -123,8 +121,7 @@ const char *ctrlm_telemetry_t::get_report_trigger(ctrlm_telemetry_report_t repor const char *ctrlm_telemetry_t::get_report_str(ctrlm_telemetry_report_t report) { switch(report) { case ctrlm_telemetry_report_t::GLOBAL: return("GLOBAL"); - case ctrlm_telemetry_report_t::RF4CE: return("RF4CE"); - case ctrlm_telemetry_report_t::BLE: return("BLE"); + case ctrlm_telemetry_report_t::RCU: return("RCU"); case ctrlm_telemetry_report_t::IP: return("IP"); case ctrlm_telemetry_report_t::VOICE: return("VOICE"); } diff --git a/src/telemetry/ctrlm_telemetry.h b/src/telemetry/ctrlm_telemetry.h index 8b5db4fb..9024ebe2 100644 --- a/src/telemetry/ctrlm_telemetry.h +++ b/src/telemetry/ctrlm_telemetry.h @@ -28,8 +28,7 @@ */ typedef enum { GLOBAL, - RF4CE, - BLE, + RCU, IP, VOICE } ctrlm_telemetry_report_t; diff --git a/src/telemetry/ctrlm_telemetry_markers.h b/src/telemetry/ctrlm_telemetry_markers.h index 9c0ba3a3..14f73226 100644 --- a/src/telemetry/ctrlm_telemetry_markers.h +++ b/src/telemetry/ctrlm_telemetry_markers.h @@ -125,4 +125,26 @@ // End Voice Markers // +// +// RCU Markers +// + +// BLE RCU Pairing Attempt Marker +// Value format: JSON object string with the fields below. +// {"method":,"result":,"paired_mac":,"bluez_retries":,"error_msg":,"discovered":[{"mac":,"name":}...]} +// - pairing method: "auto_timeout" | "ir_code" | "mac_hash" | "mac_list" +// - "success" or failure reason: "discovery_timeout" | "discovery_stopped" | "discovery_stop_timeout" | +// "pairing_timeout" | "bluez_error" | "adapter_off" | "device_unpaired" | "device_removed" | "cancelled" +// - MAC address string of paired device, empty string on failure +// - number of bluez pair() retries made before success or final error +// - error message of bluez layer +// - array of devices seen during discovery; each has "mac" and "name" +// - Version of the marker format. +#define MARKER_RCU_PAIRING_ATTEMPT "ctrlm.rcu.pairing.attempt" +#define MARKER_RCU_PAIRING_ATTEMPT_VERSION 1 + +// +// End BLE Markers +// + #endif From b8347e5c4cb4e04dc7a652b44f0253aefe5bd136 Mon Sep 17 00:00:00 2001 From: Kelvin Lu <119349872+klu339@users.noreply.github.com> Date: Tue, 31 Mar 2026 14:41:32 -0400 Subject: [PATCH 2/5] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/ble/ctrlm_ble_network.cpp | 4 ++-- src/ble/hal/blercu/blercucontroller.h | 2 +- src/telemetry/ctrlm_telemetry_markers.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ble/ctrlm_ble_network.cpp b/src/ble/ctrlm_ble_network.cpp index 810e6dc3..752397a7 100644 --- a/src/ble/ctrlm_ble_network.cpp +++ b/src/ble/ctrlm_ble_network.cpp @@ -1718,7 +1718,7 @@ void ctrlm_obj_network_ble_t::ind_process_rcu_pairing_outcome(void *data, int si json_object_set_new(jroot, "result", json_string(attempt.result.c_str())); json_object_set_new(jroot, "paired_mac", json_string(attempt.paired_mac.c_str())); json_object_set_new(jroot, "bluez_retries", json_integer(attempt.bluez_retries)); - json_object_set_new(jroot, "bluez_msg", json_string(attempt.error_msg.c_str())); + json_object_set_new(jroot, "error_msg", json_string(attempt.error_msg.c_str())); json_t *jdiscovered = json_array(); if (jdiscovered) { @@ -1734,7 +1734,7 @@ void ctrlm_obj_network_ble_t::ind_process_rcu_pairing_outcome(void *data, int si } char *json_str = json_dumps(jroot, JSON_COMPACT); - XLOGD_WARN("KLU339 %s", json_str); + XLOGD_DEBUG("Serialized RCU pairing attempt telemetry payload"); if (json_str) { ctrlm_telemetry_event_t ev(MARKER_RCU_PAIRING_ATTEMPT, std::string(json_str)); ctrlm_telemetry_t::get_instance()->event(ctrlm_telemetry_report_t::RCU, ev); diff --git a/src/ble/hal/blercu/blercucontroller.h b/src/ble/hal/blercu/blercucontroller.h index f8c6ebe5..49963156 100644 --- a/src/ble/hal/blercu/blercucontroller.h +++ b/src/ble/hal/blercu/blercucontroller.h @@ -47,7 +47,7 @@ struct BleRcuPairingOutcome { std::string method; // "auto_timeout" | "ir_code" | "mac_hash" | "mac_list" std::string result; // "success" | failure-reason string - std::vector> discovered; // {mac_str, was_paired} + std::vector> discovered; // {mac, name} int bluezRetries; std::string pairedMac; // empty on failure std::string bluezError; diff --git a/src/telemetry/ctrlm_telemetry_markers.h b/src/telemetry/ctrlm_telemetry_markers.h index 14f73226..e44cdc82 100644 --- a/src/telemetry/ctrlm_telemetry_markers.h +++ b/src/telemetry/ctrlm_telemetry_markers.h @@ -144,7 +144,7 @@ #define MARKER_RCU_PAIRING_ATTEMPT_VERSION 1 // -// End BLE Markers +// End RCU Markers // #endif From 96bbf48ba119a4cfd83ad425d4639d40965d40a7 Mon Sep 17 00:00:00 2001 From: klu339 Date: Tue, 31 Mar 2026 20:13:11 +0000 Subject: [PATCH 3/5] RDKEMW-15135: Copilot fixes and array of bluez errors --- src/ble/ctrlm_ble_network.cpp | 19 ++++++++++--------- src/ble/ctrlm_ble_network.h | 4 ++-- src/ble/hal/blercu/blercucontroller.cpp | 2 +- src/ble/hal/blercu/blercucontroller.h | 2 +- .../hal/blercu/blercupairingstatemachine.cpp | 3 +-- src/ble/hal/blercu/bluez/blercudevice.cpp | 4 +++- src/telemetry/ctrlm_telemetry_markers.h | 2 +- 7 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/ble/ctrlm_ble_network.cpp b/src/ble/ctrlm_ble_network.cpp index 752397a7..02136b0b 100644 --- a/src/ble/ctrlm_ble_network.cpp +++ b/src/ble/ctrlm_ble_network.cpp @@ -1702,15 +1702,10 @@ void ctrlm_obj_network_ble_t::ind_process_rcu_pairing_outcome(void *data, int si attempt.discovered = outcome->discovered; attempt.bluez_retries = outcome->bluezRetries; attempt.paired_mac = outcome->pairedMac; - attempt.error_msg = outcome->bluezError; - - XLOGD_INFO("BLE pairing attempt complete: method=%s result=%s paired_mac=%s " - "bluez_retries=%d bluez_msg=%s discovered=%zu", - attempt.method.c_str(), attempt.result.c_str(), attempt.paired_mac.c_str(), - attempt.bluez_retries, attempt.error_msg.c_str(), attempt.discovered.size()); + attempt.bluez_msgs = outcome->bluezError; #ifdef TELEMETRY_SUPPORT - // Serialize to JSON compatible with the ctrlm.ble.pairing.attempt T2 marker + // Serialize to JSON compatible with the ctrlm.rcu.pairing.attempt T2 marker json_t *jroot = json_object(); if (jroot) { json_object_set_new(jroot, "version", json_integer(MARKER_RCU_PAIRING_ATTEMPT_VERSION)); @@ -1718,7 +1713,14 @@ void ctrlm_obj_network_ble_t::ind_process_rcu_pairing_outcome(void *data, int si json_object_set_new(jroot, "result", json_string(attempt.result.c_str())); json_object_set_new(jroot, "paired_mac", json_string(attempt.paired_mac.c_str())); json_object_set_new(jroot, "bluez_retries", json_integer(attempt.bluez_retries)); - json_object_set_new(jroot, "error_msg", json_string(attempt.error_msg.c_str())); + + json_t *jbluez_errors = json_array(); + if (jbluez_errors) { + for (const auto &msg : attempt.bluez_msgs) { + json_array_append_new(jbluez_errors, json_string(msg.c_str())); + } + json_object_set_new(jroot, "bluez_msg", jbluez_errors); + } json_t *jdiscovered = json_array(); if (jdiscovered) { @@ -1734,7 +1736,6 @@ void ctrlm_obj_network_ble_t::ind_process_rcu_pairing_outcome(void *data, int si } char *json_str = json_dumps(jroot, JSON_COMPACT); - XLOGD_DEBUG("Serialized RCU pairing attempt telemetry payload"); if (json_str) { ctrlm_telemetry_event_t ev(MARKER_RCU_PAIRING_ATTEMPT, std::string(json_str)); ctrlm_telemetry_t::get_instance()->event(ctrlm_telemetry_report_t::RCU, ev); diff --git a/src/ble/ctrlm_ble_network.h b/src/ble/ctrlm_ble_network.h index c0702980..deebc2dd 100644 --- a/src/ble/ctrlm_ble_network.h +++ b/src/ble/ctrlm_ble_network.h @@ -102,10 +102,10 @@ typedef struct { struct ctrlm_ble_pair_attempt_t { std::string method; // pairing method string (auto_timeout, ir_code, mac_hash, mac_list) std::string result; // "success" or failure reason string - std::vector> discovered; // {mac_str, was_paired} + std::vector> discovered; // {mac_str, rcu_name} int bluez_retries; std::string paired_mac; // empty on failure - std::string error_msg; + std::vector bluez_msgs; }; /////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/ble/hal/blercu/blercucontroller.cpp b/src/ble/hal/blercu/blercucontroller.cpp index 7a5c0891..77b7ba0e 100644 --- a/src/ble/hal/blercu/blercucontroller.cpp +++ b/src/ble/hal/blercu/blercucontroller.cpp @@ -747,7 +747,7 @@ void BleRcuControllerImpl::onFailedPairing() outcome.result = failureReasonStr(m_pairingStateMachine.failureReason()); outcome.bluezRetries = m_pairingStateMachine.bluezRetries(); outcome.pairedMac = m_pairingStateMachine.pairedMac().toString(); - outcome.bluezError = m_pairingStateMachine.bluezError(); + outcome.bluezError.emplace_back(m_pairingStateMachine.bluezError()); for (const auto &dev : m_pairingStateMachine.discoveredDevices()) { outcome.discovered.emplace_back(dev.mac.toString(), dev.name); } diff --git a/src/ble/hal/blercu/blercucontroller.h b/src/ble/hal/blercu/blercucontroller.h index 49963156..2a88f03c 100644 --- a/src/ble/hal/blercu/blercucontroller.h +++ b/src/ble/hal/blercu/blercucontroller.h @@ -50,7 +50,7 @@ struct BleRcuPairingOutcome std::vector> discovered; // {mac, name} int bluezRetries; std::string pairedMac; // empty on failure - std::string bluezError; + std::vector bluezError; }; diff --git a/src/ble/hal/blercu/blercupairingstatemachine.cpp b/src/ble/hal/blercu/blercupairingstatemachine.cpp index 37c0e8ee..21f99981 100644 --- a/src/ble/hal/blercu/blercupairingstatemachine.cpp +++ b/src/ble/hal/blercu/blercupairingstatemachine.cpp @@ -1050,7 +1050,7 @@ void BleRcuPairingStateMachine::onDeviceFound(const BleAddress &address, bool alreadyRecorded = false; for (const auto &dev : m_discoveredDevices) { - if (dev.name == name) { alreadyRecorded = true; break; } + if (dev.mac == address) { alreadyRecorded = true; break; } } if (!alreadyRecorded) { DiscoveredDevice dev; @@ -1132,7 +1132,6 @@ void BleRcuPairingStateMachine::onDevicePairingError(const BleAddress &address, m_failureReason = FAIL_BLUEZ_ERROR; m_bluezRetries = retryCnt; m_bluezErrorMsg = error; - m_pairedMac = address; if (retryCnt < maxRetryCnt) { // Still retrying so don't stop pairing and timers yet diff --git a/src/ble/hal/blercu/bluez/blercudevice.cpp b/src/ble/hal/blercu/bluez/blercudevice.cpp index 2c2e02a7..8abd11f8 100644 --- a/src/ble/hal/blercu/bluez/blercudevice.cpp +++ b/src/ble/hal/blercu/bluez/blercudevice.cpp @@ -266,15 +266,17 @@ void BleRcuDeviceBluez::onPairRequestReply(PendingReply<> *reply) XLOGD_ERROR("%s pairing request failed with error: <%s>", m_address.toString().c_str(), reply->errorMessage().c_str()); + bool resetCnt = false; if (m_pairingRetryCnt < m_maxPairingRetries) { m_pairingRetryCnt++; XLOGD_INFO("Retrying pairing, attempt %d out of %d ", m_pairingRetryCnt, m_maxPairingRetries); pair(0); } else { - m_pairingRetryCnt = 0; + resetCnt = true; } int retryCnt = m_pairingRetryCnt; m_pairingErrorSlots.invoke(reply->errorMessage(), retryCnt, m_maxPairingRetries); + m_pairingRetryCnt = resetCnt ? 0 : m_pairingRetryCnt; } else { XLOGD_DEBUG("%s pairing request successful", m_address.toString().c_str()); } diff --git a/src/telemetry/ctrlm_telemetry_markers.h b/src/telemetry/ctrlm_telemetry_markers.h index e44cdc82..5b96d5d0 100644 --- a/src/telemetry/ctrlm_telemetry_markers.h +++ b/src/telemetry/ctrlm_telemetry_markers.h @@ -137,7 +137,7 @@ // "pairing_timeout" | "bluez_error" | "adapter_off" | "device_unpaired" | "device_removed" | "cancelled" // - MAC address string of paired device, empty string on failure // - number of bluez pair() retries made before success or final error -// - error message of bluez layer +// - array of error message of bluez layer, empty if no errors // - array of devices seen during discovery; each has "mac" and "name" // - Version of the marker format. #define MARKER_RCU_PAIRING_ATTEMPT "ctrlm.rcu.pairing.attempt" From 1cf2e858c8d41f2939636f0088301bd03db0a67b Mon Sep 17 00:00:00 2001 From: klu339 Date: Wed, 1 Apr 2026 13:50:09 -0400 Subject: [PATCH 4/5] RDKEMW-15135: Fix vector msg and clean up --- src/ble/ctrlm_ble_network.cpp | 20 ++++++------------- src/ble/ctrlm_ble_network.h | 16 --------------- src/ble/hal/blercu/blercucontroller.cpp | 2 +- .../hal/blercu/blercupairingstatemachine.cpp | 4 ++-- .../hal/blercu/blercupairingstatemachine.h | 4 ++-- src/ble/hal/blercu/bluez/blercudevice.cpp | 8 +++----- src/telemetry/ctrlm_telemetry.cpp | 2 +- 7 files changed, 15 insertions(+), 41 deletions(-) diff --git a/src/ble/ctrlm_ble_network.cpp b/src/ble/ctrlm_ble_network.cpp index 02136b0b..fdc5a023 100644 --- a/src/ble/ctrlm_ble_network.cpp +++ b/src/ble/ctrlm_ble_network.cpp @@ -1696,27 +1696,19 @@ void ctrlm_obj_network_ble_t::ind_process_rcu_pairing_outcome(void *data, int si return; } - ctrlm_ble_pair_attempt_t attempt; - attempt.method = outcome->method; - attempt.result = outcome->result; - attempt.discovered = outcome->discovered; - attempt.bluez_retries = outcome->bluezRetries; - attempt.paired_mac = outcome->pairedMac; - attempt.bluez_msgs = outcome->bluezError; - #ifdef TELEMETRY_SUPPORT // Serialize to JSON compatible with the ctrlm.rcu.pairing.attempt T2 marker json_t *jroot = json_object(); if (jroot) { json_object_set_new(jroot, "version", json_integer(MARKER_RCU_PAIRING_ATTEMPT_VERSION)); - json_object_set_new(jroot, "method", json_string(attempt.method.c_str())); - json_object_set_new(jroot, "result", json_string(attempt.result.c_str())); - json_object_set_new(jroot, "paired_mac", json_string(attempt.paired_mac.c_str())); - json_object_set_new(jroot, "bluez_retries", json_integer(attempt.bluez_retries)); + json_object_set_new(jroot, "method", json_string(outcome->method.c_str())); + json_object_set_new(jroot, "result", json_string(outcome->result.c_str())); + json_object_set_new(jroot, "paired_mac", json_string(outcome->pairedMac.c_str())); + json_object_set_new(jroot, "bluez_retries", json_integer(outcome->bluezRetries)); json_t *jbluez_errors = json_array(); if (jbluez_errors) { - for (const auto &msg : attempt.bluez_msgs) { + for (const auto &msg : outcome->bluezError) { json_array_append_new(jbluez_errors, json_string(msg.c_str())); } json_object_set_new(jroot, "bluez_msg", jbluez_errors); @@ -1724,7 +1716,7 @@ void ctrlm_obj_network_ble_t::ind_process_rcu_pairing_outcome(void *data, int si json_t *jdiscovered = json_array(); if (jdiscovered) { - for (const auto &dev : attempt.discovered) { + for (const auto &dev : outcome->discovered) { json_t *jdev = json_object(); if (jdev) { json_object_set_new(jdev, "mac", json_string(dev.first.c_str())); diff --git a/src/ble/ctrlm_ble_network.h b/src/ble/ctrlm_ble_network.h index deebc2dd..9e4f99b4 100644 --- a/src/ble/ctrlm_ble_network.h +++ b/src/ble/ctrlm_ble_network.h @@ -92,22 +92,6 @@ typedef struct { } ctrlm_ble_upgrade_image_info_t; -/////////////////////////////////////////////////////////////////////////////////////////// - -/** - * @brief BLE RCU Pairing Attempt Metrics - * - * Aggregates the outcome of a single BLE pairing attempt for telemetry emission. - */ -struct ctrlm_ble_pair_attempt_t { - std::string method; // pairing method string (auto_timeout, ir_code, mac_hash, mac_list) - std::string result; // "success" or failure reason string - std::vector> discovered; // {mac_str, rcu_name} - int bluez_retries; - std::string paired_mac; // empty on failure - std::vector bluez_msgs; -}; - /////////////////////////////////////////////////////////////////////////////////////////// /** diff --git a/src/ble/hal/blercu/blercucontroller.cpp b/src/ble/hal/blercu/blercucontroller.cpp index 77b7ba0e..aca5bc81 100644 --- a/src/ble/hal/blercu/blercucontroller.cpp +++ b/src/ble/hal/blercu/blercucontroller.cpp @@ -747,7 +747,7 @@ void BleRcuControllerImpl::onFailedPairing() outcome.result = failureReasonStr(m_pairingStateMachine.failureReason()); outcome.bluezRetries = m_pairingStateMachine.bluezRetries(); outcome.pairedMac = m_pairingStateMachine.pairedMac().toString(); - outcome.bluezError.emplace_back(m_pairingStateMachine.bluezError()); + outcome.bluezError = m_pairingStateMachine.bluezError(); for (const auto &dev : m_pairingStateMachine.discoveredDevices()) { outcome.discovered.emplace_back(dev.mac.toString(), dev.name); } diff --git a/src/ble/hal/blercu/blercupairingstatemachine.cpp b/src/ble/hal/blercu/blercupairingstatemachine.cpp index 21f99981..34a64979 100644 --- a/src/ble/hal/blercu/blercupairingstatemachine.cpp +++ b/src/ble/hal/blercu/blercupairingstatemachine.cpp @@ -235,7 +235,7 @@ BleAddress BleRcuPairingStateMachine::pairedMac() const return m_pairedMac; } -std::string BleRcuPairingStateMachine::bluezError() const +std::vector BleRcuPairingStateMachine::bluezError() const { return m_bluezErrorMsg; } @@ -1131,7 +1131,7 @@ void BleRcuPairingStateMachine::onDevicePairingError(const BleAddress &address, m_failureReason = FAIL_BLUEZ_ERROR; m_bluezRetries = retryCnt; - m_bluezErrorMsg = error; + m_bluezErrorMsg.push_back(error); if (retryCnt < maxRetryCnt) { // Still retrying so don't stop pairing and timers yet diff --git a/src/ble/hal/blercu/blercupairingstatemachine.h b/src/ble/hal/blercu/blercupairingstatemachine.h index 50be05ad..c37b610c 100644 --- a/src/ble/hal/blercu/blercupairingstatemachine.h +++ b/src/ble/hal/blercu/blercupairingstatemachine.h @@ -104,7 +104,7 @@ class BleRcuPairingStateMachine std::vector discoveredDevices() const; int bluezRetries() const; BleAddress pairedMac() const; - std::string bluezError() const; + std::vector bluezError() const; // public slots: void start(const BleAddress &target, const std::string &name); @@ -213,7 +213,7 @@ class BleRcuPairingStateMachine std::vector m_discoveredDevices; int m_bluezRetries; BleAddress m_pairedMac; - std::string m_bluezErrorMsg; + std::vector m_bluezErrorMsg; BtrMgrAdapter m_btrMgrAdapter; bool discoveryStartedExternally = false; diff --git a/src/ble/hal/blercu/bluez/blercudevice.cpp b/src/ble/hal/blercu/bluez/blercudevice.cpp index 8abd11f8..2e352bd3 100644 --- a/src/ble/hal/blercu/bluez/blercudevice.cpp +++ b/src/ble/hal/blercu/bluez/blercudevice.cpp @@ -266,17 +266,15 @@ void BleRcuDeviceBluez::onPairRequestReply(PendingReply<> *reply) XLOGD_ERROR("%s pairing request failed with error: <%s>", m_address.toString().c_str(), reply->errorMessage().c_str()); - bool resetCnt = false; if (m_pairingRetryCnt < m_maxPairingRetries) { m_pairingRetryCnt++; XLOGD_INFO("Retrying pairing, attempt %d out of %d ", m_pairingRetryCnt, m_maxPairingRetries); + m_pairingErrorSlots.invoke(reply->errorMessage(), m_pairingRetryCnt, m_maxPairingRetries); pair(0); } else { - resetCnt = true; + m_pairingErrorSlots.invoke(reply->errorMessage(), m_pairingRetryCnt, m_maxPairingRetries); + m_pairingRetryCnt = 0; } - int retryCnt = m_pairingRetryCnt; - m_pairingErrorSlots.invoke(reply->errorMessage(), retryCnt, m_maxPairingRetries); - m_pairingRetryCnt = resetCnt ? 0 : m_pairingRetryCnt; } else { XLOGD_DEBUG("%s pairing request successful", m_address.toString().c_str()); } diff --git a/src/telemetry/ctrlm_telemetry.cpp b/src/telemetry/ctrlm_telemetry.cpp index 398bd369..32a6f946 100644 --- a/src/telemetry/ctrlm_telemetry.cpp +++ b/src/telemetry/ctrlm_telemetry.cpp @@ -60,7 +60,7 @@ ctrlm_telemetry_t::ctrlm_telemetry_t() { this->timeout_id = 0; this->set_duration(this->reporting_interval); this->event_reported[ctrlm_telemetry_report_t::GLOBAL] = false; - this->event_reported[ctrlm_telemetry_report_t::RCU] = false; + this->event_reported[ctrlm_telemetry_report_t::RCU] = false; this->event_reported[ctrlm_telemetry_report_t::IP] = false; this->event_reported[ctrlm_telemetry_report_t::VOICE] = false; } From 056752e93ef63ab45fa53f8bc084cf95e5b7f3bd Mon Sep 17 00:00:00 2001 From: klu339 Date: Wed, 1 Apr 2026 20:38:50 +0000 Subject: [PATCH 5/5] RDKEMW-15135: another round of Copilot review comments --- src/ble/ctrlm_ble_network.cpp | 4 ++++ src/ble/hal/blercu/blercupairingstatemachine.cpp | 15 +++++++++++---- src/telemetry/ctrlm_telemetry_markers.h | 2 +- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/ble/ctrlm_ble_network.cpp b/src/ble/ctrlm_ble_network.cpp index fdc5a023..5f67e80e 100644 --- a/src/ble/ctrlm_ble_network.cpp +++ b/src/ble/ctrlm_ble_network.cpp @@ -1691,6 +1691,10 @@ void ctrlm_obj_network_ble_t::ind_rcu_pairing_outcome(const BleRcuPairingOutcome void ctrlm_obj_network_ble_t::ind_process_rcu_pairing_outcome(void *data, int size) { THREAD_ID_VALIDATE(); const BleRcuPairingOutcome *outcome = static_cast(data); + if (sizeof(BleRcuPairingOutcome) != size) { + XLOGD_ERROR("Invalid size!"); + return; + } if (!outcome) { XLOGD_ERROR("pairing outcome data is NULL"); return; diff --git a/src/ble/hal/blercu/blercupairingstatemachine.cpp b/src/ble/hal/blercu/blercupairingstatemachine.cpp index 34a64979..e00d868e 100644 --- a/src/ble/hal/blercu/blercupairingstatemachine.cpp +++ b/src/ble/hal/blercu/blercupairingstatemachine.cpp @@ -591,19 +591,21 @@ void BleRcuPairingStateMachine::onStateExit(int state) */ void BleRcuPairingStateMachine::onStateTransition(int oldState, int newState) { + FailureReason newFailureReason = SUCCESS; + if (newState == FinishedState) { if (oldState == UnpairingState) { XLOGD_AUTOMATION_WARN("timed-out in un-pairing phase (failed rcu may be left paired)"); - m_failureReason = FAIL_PAIRING_TIMEOUT; + newFailureReason = FAIL_PAIRING_TIMEOUT; } else if (oldState == StartingDiscoveryState) { XLOGD_AUTOMATION_ERROR("timed-out waiting for discovery started signal"); - m_failureReason = FAIL_DISCOVERY_TIMEOUT; + newFailureReason = FAIL_DISCOVERY_TIMEOUT; } else if (oldState == DiscoveringState) { XLOGD_AUTOMATION_ERROR("timed-out in discovery phase (didn't find target rcu device to pair to)"); - m_failureReason = FAIL_DISCOVERY_TIMEOUT; + newFailureReason = FAIL_DISCOVERY_TIMEOUT; } else if (oldState == StoppingDiscoveryState) { XLOGD_AUTOMATION_ERROR("timed-out waiting for discovery to stop (suggesting something has gone wrong inside bluez)"); - m_failureReason = FAIL_DISCOVERY_STOP_TIMEOUT; + newFailureReason = FAIL_DISCOVERY_STOP_TIMEOUT; } } else if (newState == UnpairingState) { if (oldState == EnablePairableState || oldState == PairingState) { @@ -612,6 +614,11 @@ void BleRcuPairingStateMachine::onStateTransition(int oldState, int newState) XLOGD_AUTOMATION_WARN("timed-out in setup phase (rcu didn't response to all requests within %dms)", m_setupTimeout); } } + + // Change the failure reason only if it hasn't already been set elsewhere + if (newFailureReason != SUCCESS && m_failureReason == SUCCESS) { + m_failureReason = newFailureReason; + } } void BleRcuPairingStateMachine::onEnteredStoppingDiscoveryStartedExternally() diff --git a/src/telemetry/ctrlm_telemetry_markers.h b/src/telemetry/ctrlm_telemetry_markers.h index 5b96d5d0..44abec35 100644 --- a/src/telemetry/ctrlm_telemetry_markers.h +++ b/src/telemetry/ctrlm_telemetry_markers.h @@ -131,7 +131,7 @@ // BLE RCU Pairing Attempt Marker // Value format: JSON object string with the fields below. -// {"method":,"result":,"paired_mac":,"bluez_retries":,"error_msg":,"discovered":[{"mac":,"name":}...]} +// {"method":,"result":,"paired_mac":,"bluez_retries":,"bluez_msg":,"discovered":[{"mac":,"name":}...]} // - pairing method: "auto_timeout" | "ir_code" | "mac_hash" | "mac_list" // - "success" or failure reason: "discovery_timeout" | "discovery_stopped" | "discovery_stop_timeout" | // "pairing_timeout" | "bluez_error" | "adapter_off" | "device_unpaired" | "device_removed" | "cancelled"