From 926dc828d276f4290c9baee08f7934bbfe161d3f Mon Sep 17 00:00:00 2001 From: Esko Dijk Date: Mon, 18 May 2026 14:50:30 +0200 Subject: [PATCH] [tcat][ble] improve internal state management of ble simulation platform (#12913) This adds sIsEnabled for more realistic BLE simulation platform behavior. Also improves internal disconnection state management, per review comments. Adds asynchronous calling of otPlatBleGapOnDisconnected() to avoid doing the 'disconnected' callback twice, per review comments. A unit test is extended to verify that the callback happens once. For test_platform.cpp 2 instances of '#ifdef' are fixed to '#if'. --- examples/platforms/simulation/ble.c | 66 +++++++++++++++++++++-------- include/openthread/platform/ble.h | 4 ++ src/core/radio/ble_secure.cpp | 14 ++++-- tests/unit/test_platform.cpp | 8 ++-- tests/unit/test_tcat.cpp | 16 +++++-- 5 files changed, 80 insertions(+), 28 deletions(-) diff --git a/examples/platforms/simulation/ble.c b/examples/platforms/simulation/ble.c index e7f893b43..7a57c5284 100644 --- a/examples/platforms/simulation/ble.c +++ b/examples/platforms/simulation/ble.c @@ -49,6 +49,7 @@ static uint8_t sBleBuffer[PLAT_BLE_MSG_DATA_MAX]; static int sFd = -1; static bool sIsConnected = false; static bool sIsDisconnecting = false; +static bool sIsEnabled = false; static const uint16_t kPortBase = 10000; static uint16_t sPort = 0; @@ -108,28 +109,31 @@ otError otPlatBleGetAdvertisementBuffer(otInstance *aInstance, uint8_t **aAdvert otError otPlatBleEnable(otInstance *aInstance) { OT_UNUSED_VARIABLE(aInstance); - initFds(); + if (!sIsEnabled) + { + initFds(); + sIsEnabled = true; + sIsConnected = false; + sIsDisconnecting = false; + } return OT_ERROR_NONE; } otError otPlatBleDisable(otInstance *aInstance) { OT_UNUSED_VARIABLE(aInstance); - deinitFds(); - sIsConnected = false; - sIsDisconnecting = false; + if (sIsEnabled) + { + deinitFds(); + sIsEnabled = false; + } return OT_ERROR_NONE; } otError otPlatBleGapAdvStart(otInstance *aInstance, uint16_t aInterval) { OT_UNUSED_VARIABLE(aInstance); - if (sIsDisconnecting) // finalize the disconnection of the TCAT client - { - sIsConnected = false; - sIsDisconnecting = false; - } - if (sIsConnected) + if (sIsConnected || !sIsEnabled) { return OT_ERROR_INVALID_STATE; } @@ -140,21 +144,24 @@ otError otPlatBleGapAdvStart(otInstance *aInstance, uint16_t aInterval) otError otPlatBleGapAdvStop(otInstance *aInstance) { OT_UNUSED_VARIABLE(aInstance); + if (!sIsEnabled) + { + return OT_ERROR_INVALID_STATE; + } otLogDebgPlat("BLE adv stop"); return OT_ERROR_NONE; } otError otPlatBleGapDisconnect(otInstance *aInstance) { - if (!sIsConnected && !sIsDisconnecting) + OT_UNUSED_VARIABLE(aInstance); + if (!sIsConnected) { return OT_ERROR_INVALID_STATE; } - if (!sIsDisconnecting) // check, to avoid reentrant calls - { - sIsDisconnecting = true; - otPlatBleGapOnDisconnected(aInstance, 0); - } + // Only flag the disconnection here. The 'disconnected' event is delivered asynchronously + // by platformBleProcess() (via otPlatBleGapOnDisconnected), per API contract. + sIsDisconnecting = true; return OT_ERROR_NONE; } @@ -189,7 +196,6 @@ void platformBleDeinit(void) { deinitFds(); } void platformBleUpdateFdSet(fd_set *aReadFdSet, fd_set *aWriteFdSet, struct timeval *aTimeout, int *aMaxFd) { - OT_UNUSED_VARIABLE(aTimeout); OT_UNUSED_VARIABLE(aWriteFdSet); if (aReadFdSet != NULL && sFd != -1) @@ -201,6 +207,13 @@ void platformBleUpdateFdSet(fd_set *aReadFdSet, fd_set *aWriteFdSet, struct time *aMaxFd = sFd; } } + + // A pending disconnection must be delivered promptly; ensure the main loop does not block. + if (sIsDisconnecting && aTimeout != NULL) + { + aTimeout->tv_sec = 0; + aTimeout->tv_usec = 0; + } } void platformBleProcess(otInstance *aInstance, const fd_set *aReadFdSet, const fd_set *aWriteFdSet) @@ -209,6 +222,14 @@ void platformBleProcess(otInstance *aInstance, const fd_set *aReadFdSet, const f otEXPECT(sFd != -1); + // Deliver a pending disconnection (requested earlier via otPlatBleGapDisconnect) + if (sIsDisconnecting) + { + sIsConnected = false; + otPlatBleGapOnDisconnected(aInstance, 0); + sIsDisconnecting = false; + } + if (FD_ISSET(sFd, aReadFdSet)) { socklen_t len = sizeof(sSockaddr); @@ -244,6 +265,7 @@ void platformBleProcess(otInstance *aInstance, const fd_set *aReadFdSet, const f DieNow(OT_EXIT_FAILURE); } } + exit: return; } @@ -287,6 +309,10 @@ otError otPlatBleGapAdvSetData(otInstance *aInstance, uint8_t *aAdvertisementDat OT_UNUSED_VARIABLE(aInstance); OT_UNUSED_VARIABLE(aAdvertisementData); OT_UNUSED_VARIABLE(aAdvertisementLen); + if (!sIsEnabled) + { + return OT_ERROR_INVALID_STATE; + } return OT_ERROR_NONE; } @@ -295,13 +321,17 @@ otError otPlatBleGapAdvUpdateData(otInstance *aInstance, uint8_t *aAdvertisement OT_UNUSED_VARIABLE(aInstance); OT_UNUSED_VARIABLE(aAdvertisementData); OT_UNUSED_VARIABLE(aAdvertisementLen); + if (!sIsEnabled) + { + return OT_ERROR_INVALID_STATE; + } return OT_ERROR_NONE; } bool otPlatBleSupportsMultiRadio(otInstance *aInstance) { OT_UNUSED_VARIABLE(aInstance); - return false; + return true; } #endif // OPENTHREAD_CONFIG_BLE_TCAT_ENABLE diff --git a/include/openthread/platform/ble.h b/include/openthread/platform/ble.h index ab189f782..1ade0e4d5 100644 --- a/include/openthread/platform/ble.h +++ b/include/openthread/platform/ble.h @@ -276,6 +276,10 @@ extern void otPlatBleGapOnDisconnected(otInstance *aInstance, uint16_t aConnecti * The BLE device shall use the Remote User Terminated Connection (0x13) reason * code when disconnecting from the peer BLE device. * + * This function only triggers the disconnection procedure. When OT_ERROR_NONE is returned, + * the platform MUST report completion of the disconnection asynchronously, by invoking + * otPlatBleGapOnDisconnected(). + * * @param[in] aInstance The OpenThread instance structure. * * @retval OT_ERROR_NONE Disconnection procedure has been started. diff --git a/src/core/radio/ble_secure.cpp b/src/core/radio/ble_secure.cpp index 63dc272a8..50a392497 100644 --- a/src/core/radio/ble_secure.cpp +++ b/src/core/radio/ble_secure.cpp @@ -180,14 +180,22 @@ void BleSecure::Disconnect(void) if (mBleState == kConnected) { - // request platform to close BLE. Once this closing is done, #HandleBleDisconnected will - // be called by the platform, which will call BleSecure::Disconnect again. - IgnoreError(otPlatBleGapDisconnect(&GetInstance())); + // Request the platform to close the BLE connection. When the platform signals completion + // (asynchronously) it calls otPlatBleGapOnDisconnected() -> #HandleBleDisconnected(), which + // in turn calls this method again - now with mBleState no longer kConnected - to invoke below + // mConnectCallback and update the advertisement data. We therefore return early here when + // the platform accepted the request, to avoid invoking the callback (and updating the + // advertisement data) twice. If the platform did not start the disconnection, no completion + // callback will follow and we handle mConnectCallback directly below. + VerifyOrExit(otPlatBleGapDisconnect(&GetInstance()) != kErrorNone); } mConnectCallback.InvokeIfSet(&GetInstance(), false, false); // Update advertisement data IgnoreError(NotifyAdvertisementChanged()); + +exit: + return; } Error BleSecure::NotifyAdvertisementChanged(void) diff --git a/tests/unit/test_platform.cpp b/tests/unit/test_platform.cpp index d29e3748d..d81f4f319 100644 --- a/tests/unit/test_platform.cpp +++ b/tests/unit/test_platform.cpp @@ -35,7 +35,7 @@ #include #include -#ifdef OPENTHREAD_CONFIG_BLE_TCAT_ENABLE +#if OPENTHREAD_CONFIG_BLE_TCAT_ENABLE #include #include #endif @@ -800,7 +800,7 @@ OT_TOOL_WEAK otPlatMcuPowerState otPlatGetMcuPowerState(otInstance *aInstance) { OT_TOOL_WEAK otError otPlatSetMcuPowerState(otInstance *aInstance, otPlatMcuPowerState aState) { return OT_ERROR_NONE; } #endif // OPENTHREAD_CONFIG_NCP_ENABLE_MCU_POWER_STATE_CONTROL -#ifdef OPENTHREAD_CONFIG_BLE_TCAT_ENABLE +#if OPENTHREAD_CONFIG_BLE_TCAT_ENABLE uint8_t sPlatBleLastAdvSetData[OT_TCAT_ADVERTISEMENT_MAX_LEN]; uint16_t sPlatBleLastAdvSetDataLen = 0; @@ -842,7 +842,9 @@ otError otPlatBleGapAdvStop(otInstance *aInstance) otError otPlatBleGapDisconnect(otInstance *aInstance) { - OT_UNUSED_VARIABLE(aInstance); + // Honor the platform contract: report completion of the disconnection via the callback. + // Normally this call is done asynchronously, but for unit testing purposes we make the reentrant call. + otPlatBleGapOnDisconnected(aInstance, 0); return OT_ERROR_NONE; } diff --git a/tests/unit/test_tcat.cpp b/tests/unit/test_tcat.cpp index a6f8a7e16..c9ab997d1 100644 --- a/tests/unit/test_tcat.cpp +++ b/tests/unit/test_tcat.cpp @@ -194,6 +194,7 @@ public: TestBleSecure(void) : mIsConnected(false) , mIsBleConnectionOpen(false) + , mConnectCallbackCount(0) { } @@ -201,14 +202,18 @@ public: { mIsConnected = aConnected; mIsBleConnectionOpen = aBleConnectionOpen; + mConnectCallbackCount++; } - bool IsConnected(void) const { return mIsConnected; } - bool IsBleConnectionOpen(void) const { return mIsBleConnectionOpen; } + bool IsConnected(void) const { return mIsConnected; } + bool IsBleConnectionOpen(void) const { return mIsBleConnectionOpen; } + uint32_t GetConnectCallbackCount(void) const { return mConnectCallbackCount; } + void ResetConnectCallbackCount(void) { mConnectCallbackCount = 0; } private: - bool mIsConnected; - bool mIsBleConnectionOpen; + bool mIsConnected; + bool mIsBleConnectionOpen; + uint32_t mConnectCallbackCount; }; static void HandleBleSecureConnect(otInstance *aInstance, bool aConnected, bool aBleConnectionOpen, void *aContext) @@ -306,8 +311,11 @@ void TestTcatConnectionAndCertAttributes(void) // Validate connection callbacks when calling `otBleSecureDisconnect()` otPlatBleGapOnConnected(instance, kConnectionId); VerifyOrQuit(!ble.IsConnected() && ble.IsBleConnectionOpen()); + ble.ResetConnectCallbackCount(); otBleSecureDisconnect(instance); VerifyOrQuit(!ble.IsConnected() && !ble.IsBleConnectionOpen()); + // Regression test: a locally-initiated disconnect must invoke the connect callback exactly once. + VerifyOrQuit(ble.GetConnectCallbackCount() == 1); // Validate TLS connection can be started (as client) only when peer is BLE-connected otPlatBleGapOnConnected(instance, kConnectionId);