From 4459c54069bb8573579aa4e84c3c6cb6ea82b1cf Mon Sep 17 00:00:00 2001 From: Esko Dijk Date: Fri, 6 Sep 2024 04:59:39 +0200 Subject: [PATCH] [tcat][ble] fixes to connection state mgmt and Disconnect cmd (#10619) - Fixes to connection state management and handling of Disconnect command TLV - specifically, this now ensures that TCAT remains on (started) after a commissioner disconnects. Earlier, there was the problem that the 2nd commissioner couldn't connect anymore. - specifically, in ble_secure.cpp the check for if (mTcatAgent.IsEnabled()) is removed, since the err = mTcatAgent.Connected(mTls) will already check this and raise an error if not enabled. If not enabled, the Device is in a wrong state to handle TCAT Commissioner commands so now it closes the connection right away. That's better than to leave the Commissioner in limbo on the TLS connection. The Commissioner can now retry again and all will be well again. - timeout of at most 10 seconds on UDP write operation in simulation mode (if longer, the TCAT device isn't reachable and the Commissioner now shows the error to the user.) Earlier, it got stuck forever. - Corrects some copy/paste errors in API definitions in comments; adds comments where needed to explain. - adds whitespace at some places to align format with rest of code - improved some of the --debug output for the UDP simulation mode of the TCAT Commissioner. --- src/core/meshcop/tcat_agent.cpp | 6 ++++- src/core/meshcop/tcat_agent.hpp | 26 +++++++++---------- src/core/radio/ble_secure.cpp | 20 +++++++------- src/core/radio/ble_secure.hpp | 8 +++--- tools/tcat_ble_client/bbtc.py | 7 ++--- .../tcat_ble_client/ble/ble_stream_secure.py | 1 + tools/tcat_ble_client/ble/udp_stream.py | 18 ++++++++----- tools/tcat_ble_client/cli/base_commands.py | 4 +-- 8 files changed, 51 insertions(+), 39 deletions(-) diff --git a/src/core/meshcop/tcat_agent.cpp b/src/core/meshcop/tcat_agent.cpp index 8f2c7c2e7..847a005c6 100644 --- a/src/core/meshcop/tcat_agent.cpp +++ b/src/core/meshcop/tcat_agent.cpp @@ -393,7 +393,8 @@ Error TcatAgent::HandleSingleTlv(const Message &aIncomingMessage, Message &aOutg switch (tlv.GetType()) { case kTlvDisconnect: - error = kErrorAbort; + error = kErrorAbort; + response = true; // true - to avoid response-with-status being sent. break; case kTlvSetActiveOperationalDataset: @@ -415,9 +416,11 @@ Error TcatAgent::HandleSingleTlv(const Message &aIncomingMessage, Message &aOutg response = true; error = kErrorNone; break; + case kTlvDecommission: error = HandleDecomission(); break; + case kTlvPing: error = HandlePing(aIncomingMessage, aOutgoingMessage, offset, length, response); break; @@ -433,6 +436,7 @@ Error TcatAgent::HandleSingleTlv(const Message &aIncomingMessage, Message &aOutg case kTlvGetProvisioningURL: error = HandleGetProvisioningUrl(aOutgoingMessage, response); break; + default: error = kErrorInvalidCommand; } diff --git a/src/core/meshcop/tcat_agent.hpp b/src/core/meshcop/tcat_agent.hpp index a76008513..15578ffbc 100644 --- a/src/core/meshcop/tcat_agent.hpp +++ b/src/core/meshcop/tcat_agent.hpp @@ -272,7 +272,7 @@ public: }; /** - * Initializes the Joiner object. + * Initializes the TCAT agent object. * * @param[in] aInstance A reference to the OpenThread instance. * @@ -280,20 +280,20 @@ public: explicit TcatAgent(Instance &aInstance); /** - * Enables the TCAT protocol. + * Enables the TCAT agent. * * @param[in] aAppDataReceiveCallback A pointer to a function that is called when the user data is received. * @param[in] aHandler A pointer to a function that is called when the join operation completes. * @param[in] aContext A context pointer. * - * @retval kErrorNone Successfully started the TCAT agent. - * @retval kErrorInvalidArgs The aVendorInfo is invalid. + * @retval kErrorNone Successfully started the TCAT agent. + * @retval kErrorFailed Failed to start due to missing vendor info. * */ Error Start(AppDataReceiveCallback aAppDataReceiveCallback, JoinCallback aHandler, void *aContext); /** - * Stops the TCAT protocol. + * Stops the TCAT agent. * */ void Stop(void); @@ -318,19 +318,19 @@ public: /** * Indicates whether or not the TCAT agent is connected. * - * @retval TRUE The TCAT agent is connected. + * @retval TRUE The TCAT agent is connected with a TCAT commissioner. * @retval FALSE The TCAT agent is not connected. * */ bool IsConnected(void) const { return mState == kStateConnected; } /** - * Indicates whether or not a command class is authorized. + * Indicates whether or not a TCAT command class is authorized for use. * - * @param[in] aCommandClass Command class to subject for authorization check. + * @param[in] aCommandClass Command class to subject to authorization check. * - * @retval TRUE The command class is authorized. - * @retval FALSE The command class is not authorized. + * @retval TRUE The command class is authorized for use by the present TCAT commissioner. + * @retval FALSE The command class is not authorized for use. * */ bool IsCommandClassAuthorized(CommandClass aCommandClass) const; @@ -338,11 +338,11 @@ public: /** * Gets TCAT advertisement data. * - * @param[out] aLen Advertisement length. + * @param[out] aLen Advertisement data length (up to OT_TCAT_ADVERTISEMENT_MAX_LEN). * @param[out] aAdvertisementData Advertisement data. * - * @retval kErrorNone Successfully started the TCAT agent. - * @retval kErrorInvalidArgs The aVendorInfo is invalid or provided incorrect parameters. + * @retval kErrorNone Successfully retrieved the TCAT advertisement data. + * @retval kErrorInvalidArgs The data could not be retrieved, or aAdvertisementData is null. * */ Error GetAdvertisementData(uint16_t &aLen, uint8_t *aAdvertisementData); diff --git a/src/core/radio/ble_secure.cpp b/src/core/radio/ble_secure.cpp index e18c3fd07..f1047ef01 100644 --- a/src/core/radio/ble_secure.cpp +++ b/src/core/radio/ble_secure.cpp @@ -330,21 +330,19 @@ void BleSecure::HandleTlsConnectEvent(MeshCoP::SecureTransport::ConnectEvent aEv { if (aEvent == MeshCoP::SecureTransport::kConnected) { + Error err; + if (mReceivedMessage == nullptr) { mReceivedMessage = Get().Allocate(Message::kTypeBle); } + err = mTcatAgent.Connected(mTls); - if (mTcatAgent.IsEnabled()) + if (err != kErrorNone) { - Error err = mTcatAgent.Connected(mTls); - - if (err != kErrorNone) - { - mTls.Close(); - LogWarn("Rejected TCAT Commissioner, error: %s", ErrorToString(err)); - ExitNow(); - } + mTls.Disconnect(); // must not use Close(), so that next Commissioner can connect + LogWarn("Rejected TCAT Commissioner, error: %s", ErrorToString(err)); + ExitNow(); } } else @@ -449,8 +447,10 @@ void BleSecure::HandleTlsReceive(uint8_t *aBuf, uint16_t aLength) if (error == kErrorAbort) { + // kErrorAbort indicates that a Disconnect command TLV has been received. Disconnect(); - Stop(); + // BleSecure is not stopped here, it must remain active in advertising state and + // must be ready to receive a next TCAT commissioner. ExitNow(); } } diff --git a/src/core/radio/ble_secure.hpp b/src/core/radio/ble_secure.hpp index d2ef628d7..7be1201d1 100644 --- a/src/core/radio/ble_secure.hpp +++ b/src/core/radio/ble_secure.hpp @@ -169,13 +169,13 @@ public: bool IsTcatEnabled(void) const { return mTcatAgent.IsEnabled(); } /** - * Indicates whether or not a TCAT command class is authorized. + * Indicates whether or not a TCAT command class is authorized for use. * * @param[in] aInstance A pointer to an OpenThread instance. - * @param[in] aCommandClass A command class to check. + * @param[in] aCommandClass A command class to subject to authorization check. * - * @retval TRUE The command class is authorized. - * @retval FALSE The command class is not authorized. + * @retval TRUE The command class is authorized for use by the present TCAT commissioner. + * @retval FALSE The command class is not authorized for use. * */ bool IsCommandClassAuthorized(CommandClass aCommandClass) const diff --git a/tools/tcat_ble_client/bbtc.py b/tools/tcat_ble_client/bbtc.py index a9be1f539..5ad735943 100755 --- a/tools/tcat_ble_client/bbtc.py +++ b/tools/tcat_ble_client/bbtc.py @@ -102,9 +102,6 @@ async def main(): while True: user_input = await loop.run_in_executor(None, lambda: input('> ')) if user_input.lower() == 'exit': - print('Disconnecting...') - if ble_sstream is not None: - await ble_sstream.close() break try: result: CommandResult = await cli.evaluate_input(user_input) @@ -113,6 +110,10 @@ async def main(): except Exception as e: logger.error(e) + print('Disconnecting...') + if ble_sstream is not None: + await ble_sstream.close() + async def get_device_by_args(args): device = None diff --git a/tools/tcat_ble_client/ble/ble_stream_secure.py b/tools/tcat_ble_client/ble/ble_stream_secure.py index de2b42d8a..97ac0d653 100644 --- a/tools/tcat_ble_client/ble/ble_stream_secure.py +++ b/tools/tcat_ble_client/ble/ble_stream_secure.py @@ -140,6 +140,7 @@ class BleStreamSecure: async def close(self): if self.ssl_object.session is not None: + logger.debug('sending Disconnect command TLV') data = TLV(TcatTLVType.DISCONNECT.value, bytes()).to_bytes() await self.send(data) diff --git a/tools/tcat_ble_client/ble/udp_stream.py b/tools/tcat_ble_client/ble/udp_stream.py index c6ba599a0..aef5fa60a 100644 --- a/tools/tcat_ble_client/ble/udp_stream.py +++ b/tools/tcat_ble_client/ble/udp_stream.py @@ -28,25 +28,31 @@ import logging import socket +import select logger = logging.getLogger(__name__) class UdpStream: BASE_PORT = 10000 + MAX_SERVER_TIMEOUT_SEC = 10 def __init__(self, address, node_id): self.__receive_buffer = b'' self.__last_recv_time = None self.socket = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) + self.socket.setblocking(False) self.address = (address, self.BASE_PORT + node_id) async def send(self, data): - logger.debug(f'sending {data}') - self.socket.sendto(data, self.address) - return len(data) + logger.debug(f'sending {len(data)} bytes: {data}') + return self.socket.sendto(data, self.address) async def recv(self, bufsize): - message = self.socket.recv(bufsize) - logger.debug(f'retrieved {message}') - return message + ready = select.select([self.socket], [], [], self.MAX_SERVER_TIMEOUT_SEC) + if ready[0]: + data = self.socket.recv(bufsize) + logger.debug(f'received {len(data)} bytes: {data}') + return data + else: + raise Exception('simulation UdpStream recv timeout - likely, TCAT is stopped on TCAT Device') diff --git a/tools/tcat_ble_client/cli/base_commands.py b/tools/tcat_ble_client/cli/base_commands.py index 5bef2a021..669f78ccf 100644 --- a/tools/tcat_ble_client/cli/base_commands.py +++ b/tools/tcat_ble_client/cli/base_commands.py @@ -181,7 +181,7 @@ class PingCommand(Command): data = TLV(TcatTLVType.PING.value, to_send).to_bytes() elapsed_time = time() response = await bless.send_with_resp(data) - elapsed_time = time() - elapsed_time + elapsed_time = 1e3 * (time() - elapsed_time) if not response: return @@ -189,7 +189,7 @@ class PingCommand(Command): if tlv_response.value != to_send: print("Received malformed response.") - print(f"Roundtrip time {elapsed_time} s.") + print(f"Roundtrip time: {elapsed_time} ms") return CommandResultTLV(tlv_response)