[petition] return ID of existing active commissioner if one exists (#36)

This commit returns the commissioner ID when petition failed due to an
existing active commissioner in the Thread Network.

include/commissioner/commissioner.hpp

@@ -160,28 +160,39 @@ class Commissioner
     /**
      * The response handler of a general TMF request.
      *
-     * @param[in] aError  A error code.
+     * @param[in] aError  An error code.
      */
     using ErrorHandler = std::function<void(Error aError)>;
 
     /**
      * The response handler of a general TMF request.
      *
      * @param[in] aResponseData  A response data. nullable.
-     * @param[in] aError         A error code.
+     * @param[in] aError         An error code.
      *
      * @note @p aResponseData is guaranteed to be not null only when @p aError == Error::kNone.
      *       Otherwise, @p aResponseData should never be accessed.
      */
-    template <class T> using Handler = std::function<void(const T *aResponseData, Error aError)>;
+    template <typename T> using Handler = std::function<void(const T *aResponseData, Error aError)>;
+
+    /**
+     * The petition result handler.
+     *
+     * @param[in] aExistingCommissionerId  The Existing commissioner Id. nullable.
+     * @param[in] aError                   An error code.
+     *
+     * @note There is an exiting active commissioner if @p aError != Error::kNone
+     *       and @p aExistingCommissionerId is not null.
+     */
+    using PetitionHandler = std::function<void(const std::string *aExistingCommissionerId, Error aError)>;
 
     /**
      * The MGMT_PANID_CONFLICT.ans handler.
      *
      * @param[in] aPeerAddr     A peer address that sent this MGMT_PANID_CONFLICT.ans request.
      * @param[in] aChannelMask  A channel mask the peer scanned at.
      * @param[in] aPanId        A PAN ID that has a conflict.
-     * @param[in] aError        A error code.
+     * @param[in] aError        An error code.
      *
      * @note @p aPeerAddr, aChannelMask and aPanId are guaranteed to
      *       be not null only when @p aError == Error::kNone. Otherwise,
@@ -196,7 +207,7 @@ class Commissioner
      * @param[in] aPeerAddr     A peer address that sent this MGMT_PANID_CONFLICT.ans request.
      * @param[in] aChannelMask  A channel mask the peer scanned at.
      * @param[in] aEnergyList   A list of measured energy level in dBm.
-     * @param[in] aError        A error code.
+     * @param[in] aError        An error code.
      *
      * @note @p aPeerAddr, aChannelMask and aEnergyList are guaranteed to
      *       be not null only when @p aError == Error::kNone. Otherwise,
@@ -422,7 +433,7 @@ class Commissioner
      *
      * @note The commissioner will be first connected to the network if it is not.
      */
-    virtual void Petition(ErrorHandler aHandler, const std::string &aAddr, uint16_t aPort) = 0;
+    virtual void Petition(PetitionHandler aHandler, const std::string &aAddr, uint16_t aPort) = 0;
 
     /**
      * @brief Synchronously petition to a Thread network.
@@ -431,12 +442,18 @@ class Commissioner
      * If succeed, a keep-alive message will be periodically sent to keep itself active.
      * It will not return until errors happened, timeouted or succeed.
      *
-     * @param[in] aAddr  A border agent address.
-     * @param[in] aPort  A border agent port.
+     * @param[out] aExistingCommissionerId  The existing active commissioner ID.
+     * @param[in]  aAddr                    A border agent address.
+     * @param[in]  aPort                    A border agent port.
      *
      * @return Error::kNone, succeed; otherwise, failed;
+     *
+     * @note @p aExistingCommissionerId is valid only when return value
+     *       is not Error::kNone and itself is not empty. Otherwise,
+     *       its content should never be accessed.
+     *
      */
-    virtual Error Petition(const std::string &aAddr, uint16_t aPort) = 0;
+    virtual Error Petition(std::string &aExistingCommissionerId, const std::string &aAddr, uint16_t aPort) = 0;
 
     /**
      * @brief Asynchronously resign from the commissioner role.

include/commissioner/error.hpp

@@ -65,7 +65,7 @@ enum class Error : int
 /**
  * @brief Convert Error to printable string.
  *
- * @param aError  A error code.
+ * @param aError  An error code.
  *
  * @return The printable string of the error.
  */

src/app/cli/interpreter.cpp

@@ -316,13 +316,18 @@ Interpreter::Value Interpreter::ProcessStart(const Expression &aExpr)
 {
     Error       error = Error::kInvalidArgs;
     std::string msg;
+    std::string existingCommissionerId;
     uint16_t    port;
 
     VerifyOrExit(aExpr.size() >= 3, msg = Usage(aExpr));
     SuccessOrExit(ParseInteger(port, aExpr[2]), msg = aExpr[2]);
-    SuccessOrExit(error = mCommissioner->Start(aExpr[1], port));
+    SuccessOrExit(error = mCommissioner->Start(existingCommissionerId, aExpr[1], port));
 
 exit:
+    if (!existingCommissionerId.empty())
+    {
+        msg = "there is an existing active commissioner: " + existingCommissionerId;
+    }
     return {error, msg};
 }
 

src/app/commissioner_app.cpp

@@ -135,11 +135,14 @@ const BorderAgent *CommissionerApp::GetBorderAgent(const std::string &aNetworkNa
     return nullptr;
 }
 
-Error CommissionerApp::Start(const std::string &aBorderAgentAddr, uint16_t aBorderAgentPort)
+Error CommissionerApp::Start(std::string &      aExistingCommissionerId,
+                             const std::string &aBorderAgentAddr,
+                             uint16_t           aBorderAgentPort)
 {
     Error error = Error::kNone;
 
-    SuccessOrExit(error = mCommissioner->Petition(aBorderAgentAddr, aBorderAgentPort));
+    // We need to report the already active commissioner ID if one exists.
+    SuccessOrExit(error = mCommissioner->Petition(aExistingCommissionerId, aBorderAgentAddr, aBorderAgentPort));
     SuccessOrExit(error = PullNetworkData());
 
 exit:

src/app/commissioner_app.hpp

@@ -82,7 +82,7 @@ class CommissionerApp
     // Set @p aNetworkName to empty to match any network name.
     const BorderAgent *GetBorderAgent(const std::string &aNetworkName);
 
-    Error Start(const std::string &aBorderAgentAddr, uint16_t aBorderAgentPort);
+    Error Start(std::string &aExistingCommissionerId, const std::string &aBorderAgentAddr, uint16_t aBorderAgentPort);
     void  Stop();
 
     void AbortRequests();

src/library/commissioner_impl.cpp

@@ -290,12 +290,12 @@ void CommissionerImpl::Stop()
     event_base_loopbreak(mEventBase);
 }
 
-void CommissionerImpl::Petition(ErrorHandler aHandler, const std::string &aAddr, uint16_t aPort)
+void CommissionerImpl::Petition(PetitionHandler aHandler, const std::string &aAddr, uint16_t aPort)
 {
     auto onConnected = [this, aHandler](Error aError) {
         if (aError != Error::kNone)
         {
-            aHandler(aError);
+            aHandler(nullptr, aError);
         }
         else
         {
@@ -1101,16 +1101,18 @@ void CommissionerImpl::SetEnergyReportHandler(EnergyReportHandler aHandler)
     mEnergyReportHandler = aHandler;
 }
 
-void CommissionerImpl::SendPetition(ErrorHandler aHandler)
+void CommissionerImpl::SendPetition(PetitionHandler aHandler)
 {
     Error         error = Error::kNone;
     coap::Request request{coap::Type::kConfirmable, coap::Code::kPost};
 
     auto onResponse = [this, aHandler](const coap::Response *aResponse, Error aError) {
         Error       error = Error::kNone;
         tlv::TlvSet tlvSet;
-        tlv::TlvPtr stateTlv     = nullptr;
-        tlv::TlvPtr sessionIdTlv = nullptr;
+        tlv::TlvPtr stateTlv          = nullptr;
+        tlv::TlvPtr sessionIdTlv      = nullptr;
+        tlv::TlvPtr commissionerIdTlv = nullptr;
+        std::string existingCommissionerId;
 
         SuccessOrExit(error = aError);
 
@@ -1121,7 +1123,15 @@ void CommissionerImpl::SendPetition(ErrorHandler aHandler)
         VerifyOrExit(stateTlv != nullptr, error = Error::kNotFound);
         VerifyOrExit(stateTlv->IsValid(), error = Error::kBadFormat);
 
-        VerifyOrExit(stateTlv->GetValueAsInt8() == tlv::kStateAccept, error = Error::kReject);
+        if (stateTlv->GetValueAsInt8() != tlv::kStateAccept)
+        {
+            commissionerIdTlv = tlvSet[tlv::Type::kCommissionerId];
+            if (commissionerIdTlv != nullptr && commissionerIdTlv->IsValid())
+            {
+                existingCommissionerId = commissionerIdTlv->GetValueAsString();
+            }
+            ExitNow(error = Error::kReject);
+        }
 
         sessionIdTlv = tlvSet[tlv::Type::kCommissionerSessionId];
         VerifyOrExit(sessionIdTlv != nullptr, error = Error::kNotFound);
@@ -1141,7 +1151,7 @@ void CommissionerImpl::SendPetition(ErrorHandler aHandler)
         {
             mState = State::kDisabled;
         }
-        aHandler(error);
+        aHandler(existingCommissionerId.empty() ? nullptr : &existingCommissionerId, error);
     };
 
     VerifyOrExit(mState == State::kDisabled, error = Error::kInvalidState);
@@ -1163,7 +1173,7 @@ void CommissionerImpl::SendPetition(ErrorHandler aHandler)
 exit:
     if (error != Error::kNone)
     {
-        aHandler(error);
+        aHandler(nullptr, error);
     }
 }
 

src/library/commissioner_impl.hpp

@@ -108,8 +108,8 @@ class CommissionerImpl : public Commissioner
 
     void Disconnect() override;
 
-    void  Petition(ErrorHandler aHandler, const std::string &aAddr, uint16_t aPort) override;
-    Error Petition(const std::string &, uint16_t) override { return Error::kNotImplemented; }
+    void  Petition(PetitionHandler aHandler, const std::string &aAddr, uint16_t aPort) override;
+    Error Petition(std::string &, const std::string &, uint16_t) override { return Error::kNotImplemented; }
 
     void  Resign(ErrorHandler aHandler) override;
     Error Resign() override { return Error::kNotImplemented; }
@@ -234,7 +234,7 @@ class CommissionerImpl : public Commissioner
     static Error     EncodeCommissionerDataset(coap::Request &aRequest, const CommissionerDataset &aDataset);
     static ByteArray GetCommissionerDatasetTlvs(uint16_t aDatasetFlags);
 
-    void SendPetition(ErrorHandler aHandler);
+    void SendPetition(PetitionHandler aHandler);
 
     // Set @p aKeepAlive to false to resign the commissioner role.
     void SendKeepAlive(Timer &aTimer, bool aKeepAlive = true);

src/library/commissioner_safe.cpp

@@ -190,15 +190,21 @@ void CommissionerSafe::AbortRequests()
     PushAsyncRequest([=]() { mImpl.AbortRequests(); });
 }
 
-void CommissionerSafe::Petition(ErrorHandler aHandler, const std::string &aAddr, uint16_t aPort)
+void CommissionerSafe::Petition(PetitionHandler aHandler, const std::string &aAddr, uint16_t aPort)
 {
     PushAsyncRequest([=]() { mImpl.Petition(aHandler, aAddr, aPort); });
 }
 
-Error CommissionerSafe::Petition(const std::string &aAddr, uint16_t aPort)
+Error CommissionerSafe::Petition(std::string &aExistingCommissionerId, const std::string &aAddr, uint16_t aPort)
 {
     std::promise<Error> pro;
-    auto                wait = [&pro](Error aError) { pro.set_value(aError); };
+    auto wait = [&pro, &aExistingCommissionerId](const std::string *existingCommissionerId, Error error) {
+        if (existingCommissionerId != nullptr)
+        {
+            aExistingCommissionerId = *existingCommissionerId;
+        }
+        pro.set_value(error);
+    };
 
     Petition(wait, aAddr, aPort);
     return pro.get_future().get();

src/library/commissioner_safe.hpp

@@ -104,8 +104,8 @@ class CommissionerSafe : public Commissioner
 
     void Disconnect() override;
 
-    void  Petition(ErrorHandler aHandler, const std::string &aAddr, uint16_t aPort) override;
-    Error Petition(const std::string &aAddr, uint16_t aPort) override;
+    void  Petition(PetitionHandler aHandler, const std::string &aAddr, uint16_t aPort) override;
+    Error Petition(std::string &aExistingCommissionerId, const std::string &aAddr, uint16_t aPort) override;
 
     void  Resign(ErrorHandler aHandler) override;
     Error Resign() override;

tests/unit/test_commissioner_impl.cpp

@@ -58,7 +58,9 @@ TEST_CASE("commissioner-impl-not-implemented-APIs", "[comm-impl]")
     std::list<BorderAgent> baList;
     REQUIRE(commImpl.Discover(baList) == Error::kNotImplemented);
     REQUIRE(commImpl.Connect("::1", 5684) == Error::kNotImplemented);
-    REQUIRE(commImpl.Petition("::1", 5684) == Error::kNotImplemented);
+
+    std::string existingCommissionerId;
+    REQUIRE(commImpl.Petition(existingCommissionerId, "::1", 5684) == Error::kNotImplemented);
     REQUIRE(commImpl.Resign() == Error::kNotImplemented);
 
     CommissionerDataset commDataset;