diff --git a/include/commissioner/commissioner.hpp b/include/commissioner/commissioner.hpp index 408bf939f..3f9327e29 100644 --- a/include/commissioner/commissioner.hpp +++ b/include/commissioner/commissioner.hpp @@ -160,7 +160,7 @@ 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; @@ -168,12 +168,23 @@ class Commissioner * 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 using Handler = std::function; + template using Handler = std::function; + + /** + * 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; /** * The MGMT_PANID_CONFLICT.ans handler. @@ -181,7 +192,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] 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. diff --git a/include/commissioner/error.hpp b/include/commissioner/error.hpp index b6ad2a661..e9f58fea2 100644 --- a/include/commissioner/error.hpp +++ b/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. */ diff --git a/src/app/cli/interpreter.cpp b/src/app/cli/interpreter.cpp index ed8bdf718..aa01f896a 100644 --- a/src/app/cli/interpreter.cpp +++ b/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}; } diff --git a/src/app/commissioner_app.cpp b/src/app/commissioner_app.cpp index d8de83c77..224a738d5 100644 --- a/src/app/commissioner_app.cpp +++ b/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: diff --git a/src/app/commissioner_app.hpp b/src/app/commissioner_app.hpp index ad83f49e4..eb3044d01 100644 --- a/src/app/commissioner_app.hpp +++ b/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(); diff --git a/src/library/commissioner_impl.cpp b/src/library/commissioner_impl.cpp index 85aae8476..c0295589b 100644 --- a/src/library/commissioner_impl.cpp +++ b/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,7 +1101,7 @@ 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}; @@ -1109,8 +1109,10 @@ void CommissionerImpl::SendPetition(ErrorHandler aHandler) 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); } } diff --git a/src/library/commissioner_impl.hpp b/src/library/commissioner_impl.hpp index 3a9a811f3..44a8f4b00 100644 --- a/src/library/commissioner_impl.hpp +++ b/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); diff --git a/src/library/commissioner_safe.cpp b/src/library/commissioner_safe.cpp index e27008c7a..c05340d3f 100644 --- a/src/library/commissioner_safe.cpp +++ b/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 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(); diff --git a/src/library/commissioner_safe.hpp b/src/library/commissioner_safe.hpp index c5b8968e1..d13e02bcc 100644 --- a/src/library/commissioner_safe.hpp +++ b/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; diff --git a/tests/unit/test_commissioner_impl.cpp b/tests/unit/test_commissioner_impl.cpp index 07d302e81..df2c31775 100644 --- a/tests/unit/test_commissioner_impl.cpp +++ b/tests/unit/test_commissioner_impl.cpp @@ -58,7 +58,9 @@ TEST_CASE("commissioner-impl-not-implemented-APIs", "[comm-impl]") std::list 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;