[tcat][ble] fixes to connection state mgmt and Disconnect cmd; API/fu…

…nction header comments updated/corrected.

- 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

@@ -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;
         }

src/core/meshcop/tcat_agent.hpp

@@ -272,28 +272,28 @@ class TcatAgent : public InstanceLocator, private NonCopyable
     };
 
     /**
-     * Initializes the Joiner object.
+     * Initializes the TCAT agent object.
      *
      * @param[in]  aInstance     A reference to the OpenThread instance.
      *
      */
     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,31 +318,31 @@ class TcatAgent : public InstanceLocator, private NonCopyable
     /**
      * 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;
 
     /**
      * 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);

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<MessagePool>().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();
                 }
             }

src/core/radio/ble_secure.hpp

@@ -169,13 +169,13 @@ class BleSecure : public InstanceLocator, private NonCopyable
     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

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

tools/tcat_ble_client/ble/ble_stream_secure.py

@@ -140,6 +140,7 @@ async def send_with_resp(self, bytes):
 
     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)
 

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')

tools/tcat_ble_client/cli/base_commands.py

@@ -181,15 +181,15 @@ async def execute_default(self, args, context):
         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
 
         tlv_response = TLV.from_bytes(response)
         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)