diff --git a/include/openthread/commissioner.h b/include/openthread/commissioner.h index 5e4f3ddde..d632fd0ff 100644 --- a/include/openthread/commissioner.h +++ b/include/openthread/commissioner.h @@ -203,6 +203,29 @@ otError otCommissionerStart(otInstance * aInstance, */ otError otCommissionerStop(otInstance *aInstance); +/** + * This function returns the Commissioner Id. + * + * @param[in] aInstance A pointer to an OpenThread instance. + * + * @returns The Commissioner Id. + * + */ +const char *otCommissionerGetId(otInstance *aInstance); + +/** + * This function sets the Commissioner Id. + * + * @param[in] aInstance A pointer to an OpenThread instance. + * @param[in] aId A pointer to a string character array. Must be null terminated. + * + * @retval OT_ERROR_NONE Successfully set the Commissioner Id. + * @retval OT_ERROR_INVALID_ARGS Given name is too long. + * @retval OT_ERROR_INVALID_STATE The commissioner is active and id cannot be changed. + * + */ +otError otCommissionerSetId(otInstance *aInstance, const char *aId); + /** * This function adds a Joiner entry. * diff --git a/include/openthread/instance.h b/include/openthread/instance.h index 1054c90e1..659d36453 100644 --- a/include/openthread/instance.h +++ b/include/openthread/instance.h @@ -53,7 +53,7 @@ extern "C" { * @note This number versions both OpenThread platform and user APIs. * */ -#define OPENTHREAD_API_VERSION (204) +#define OPENTHREAD_API_VERSION (205) /** * @addtogroup api-instance diff --git a/src/cli/README_COMMISSIONER.md b/src/cli/README_COMMISSIONER.md index d94c12526..013b4167d 100644 --- a/src/cli/README_COMMISSIONER.md +++ b/src/cli/README_COMMISSIONER.md @@ -200,6 +200,27 @@ Get current commissioner session id. Done ``` +### id + +Usage: `commissioner id` + +Get the commissioner id. + +```bash +> commissioner id +OpenThread Commissioner +Done +``` + +### id \ + +Set the commissioner id. + +```bash +> commissioner id "Custom Commissioner Id" +Done +``` + ### start Usage: `commissioner start` diff --git a/src/cli/cli_commissioner.cpp b/src/cli/cli_commissioner.cpp index 5db0425db..bbbcfceb2 100644 --- a/src/cli/cli_commissioner.cpp +++ b/src/cli/cli_commissioner.cpp @@ -332,6 +332,23 @@ template <> otError Commissioner::Process(Arg aArgs[]) return OT_ERROR_NONE; } +template <> otError Commissioner::Process(Arg aArgs[]) +{ + otError error; + + if (aArgs[0].IsEmpty()) + { + OutputLine("%s", otCommissionerGetId(GetInstancePtr())); + error = OT_ERROR_NONE; + } + else + { + error = otCommissionerSetId(GetInstancePtr(), aArgs[0].GetCString()); + } + + return error; +} + template <> otError Commissioner::Process(Arg aArgs[]) { OT_UNUSED_VARIABLE(aArgs); @@ -427,10 +444,9 @@ otError Commissioner::Process(Arg aArgs[]) } static constexpr Command kCommands[] = { - CmdEntry("announce"), CmdEntry("energy"), CmdEntry("joiner"), - CmdEntry("mgmtget"), CmdEntry("mgmtset"), CmdEntry("panid"), - CmdEntry("provisioningurl"), CmdEntry("sessionid"), CmdEntry("start"), - CmdEntry("state"), CmdEntry("stop"), + CmdEntry("announce"), CmdEntry("energy"), CmdEntry("id"), CmdEntry("joiner"), + CmdEntry("mgmtget"), CmdEntry("mgmtset"), CmdEntry("panid"), CmdEntry("provisioningurl"), + CmdEntry("sessionid"), CmdEntry("start"), CmdEntry("state"), CmdEntry("stop"), }; #undef CmdEntry diff --git a/src/core/api/commissioner_api.cpp b/src/core/api/commissioner_api.cpp index 4f5aa4bcc..9c5b57590 100644 --- a/src/core/api/commissioner_api.cpp +++ b/src/core/api/commissioner_api.cpp @@ -50,6 +50,16 @@ otError otCommissionerStart(otInstance * aInstance, return AsCoreType(aInstance).Get().Start(aStateCallback, aJoinerCallback, aCallbackContext); } +const char *otCommissionerGetId(otInstance *aInstance) +{ + return AsCoreType(aInstance).Get().GetId(); +} + +otError otCommissionerSetId(otInstance *aInstance, const char *aId) +{ + return AsCoreType(aInstance).Get().SetId(aId); +} + otError otCommissionerStop(otInstance *aInstance) { return AsCoreType(aInstance).Get().Stop(); diff --git a/src/core/meshcop/commissioner.cpp b/src/core/meshcop/commissioner.cpp index a15d991a8..bb9a7a482 100644 --- a/src/core/meshcop/commissioner.cpp +++ b/src/core/meshcop/commissioner.cpp @@ -86,6 +86,8 @@ Commissioner::Commissioner(Instance &aInstance) mCommissionerAloc.mScopeOverride = Ip6::Address::kRealmLocalScope; mCommissionerAloc.mScopeOverrideValid = true; + IgnoreError(SetId("OpenThread Commissioner")); + mProvisioningUrl[0] = '\0'; } @@ -318,6 +320,8 @@ Error Commissioner::Start(StateCallback aStateCallback, JoinerCallback aJoinerCa SuccessOrExit(error = SendPetition()); SetState(kStatePetition); + LogInfo("start commissioner %s", mCommissionerId); + exit: if ((error != kErrorNone) && (error != kErrorAlready)) { @@ -367,6 +371,28 @@ exit: return error; } +Error Commissioner::SetId(const char *aId) +{ + Error error = kErrorNone; + uint8_t len; + + VerifyOrExit(IsDisabled(), error = kErrorInvalidState); + VerifyOrExit(aId != nullptr); + VerifyOrExit(IsValidUtf8String(aId), error = kErrorInvalidArgs); + + len = static_cast(StringLength(aId, sizeof(mCommissionerId))); + + // CommissionerIdTlv::SetCommissionerId trims the string to the maximum array size. + // Prevent this from happening returning an error. + VerifyOrExit(len < CommissionerIdTlv::kMaxLength, error = kErrorInvalidArgs); + + memcpy(mCommissionerId, aId, len); + mCommissionerId[len] = '\0'; + +exit: + return error; +} + void Commissioner::ComputeBloomFilter(SteeringData &aSteeringData) const { Mac::ExtAddress joinerId; @@ -815,7 +841,7 @@ Error Commissioner::SendPetition(void) Error error = kErrorNone; Coap::Message * message = nullptr; Tmf::MessageInfo messageInfo(GetInstance()); - CommissionerIdTlv commissionerId; + CommissionerIdTlv commissionerIdTlv; mTransmitAttempts++; @@ -824,11 +850,10 @@ Error Commissioner::SendPetition(void) SuccessOrExit(error = message->InitAsConfirmablePost(UriPath::kLeaderPetition)); SuccessOrExit(error = message->SetPayloadMarker()); - commissionerId.Init(); - commissionerId.SetCommissionerId("OpenThread Commissioner"); - - SuccessOrExit(error = commissionerId.AppendTo(*message)); + commissionerIdTlv.Init(); + commissionerIdTlv.SetCommissionerId(mCommissionerId); + SuccessOrExit(error = commissionerIdTlv.AppendTo(*message)); SuccessOrExit(error = messageInfo.SetSockAddrToRlocPeerAddrToLeaderAloc()); SuccessOrExit( error = Get().SendMessage(*message, messageInfo, Commissioner::HandleLeaderPetitionResponse, this)); diff --git a/src/core/meshcop/commissioner.hpp b/src/core/meshcop/commissioner.hpp index 90639e99b..4c1d19046 100644 --- a/src/core/meshcop/commissioner.hpp +++ b/src/core/meshcop/commissioner.hpp @@ -251,6 +251,26 @@ public: */ Error Stop(void) { return Stop(kSendKeepAliveToResign); } + /** + * This method returns the Commissioner Id. + * + * @returns The Commissioner Id. + * + */ + const char *GetId(void) const { return mCommissionerId; } + + /** + * This method sets the Commissioner Id. + * + * @param[in] aId A pointer to a string character array. Must be null terminated. + * + * @retval kErrorNone Successfully set the Commissioner Id. + * @retval kErrorInvalidArgs Given name is too long. + * @retval kErrorInvalidState The commissioner is active and id cannot be changed. + * + */ + Error SetId(const char *aId); + /** * This method clears all Joiner entries. * @@ -606,6 +626,7 @@ private: Ip6::Netif::UnicastAddress mCommissionerAloc; char mProvisioningUrl[OT_PROVISIONING_URL_MAX_SIZE + 1]; // + 1 is for null char at end of string. + char mCommissionerId[CommissionerIdTlv::kMaxLength + 1]; State mState; diff --git a/tests/scripts/expect/cli-commissioner-multiple-ftds.exp b/tests/scripts/expect/cli-commissioner-multiple-ftds.exp new file mode 100755 index 000000000..7d43522c4 --- /dev/null +++ b/tests/scripts/expect/cli-commissioner-multiple-ftds.exp @@ -0,0 +1,139 @@ +#!/usr/bin/expect -f +# +# Copyright (c) 2022, The OpenThread Authors. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# 1. Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# 3. Neither the name of the copyright holder nor the +# names of its contributors may be used to endorse or promote products +# derived from this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. +# + +source "tests/scripts/expect/_common.exp" +source "tests/scripts/expect/_multinode.exp" + +spawn_node 3 +spawn_node 2 +spawn_node 1 + +setup_leader +setup_node 3 "rdn" "router" +setup_node 2 "rdn" "router" + +sleep 2 + +########################################### +# Verify topology and commissioners + +switch_node 3 +send "state\n" +expect "router" +send "commissioner state\n" +expect "disabled" + +switch_node 2 +send "state\n" +expect "router" +send "commissioner state\n" +expect "disabled" + +switch_node 1 +send "state\n" +expect "leader" +send "commissioner state\n" +expect "active" + +########################################### +# Starting a commissioner on the same partition with the same ID makes +# the active commissioner resign and become disabled. + +switch_node 2 +send "commissioner start\n" +expect "Done" +wait_for "commissioner state" "active" +switch_node 1 +sleep 5 +wait_for "commissioner state" "disabled" + +switch_node 3 +send "commissioner start\n" +expect "Done" +wait_for "commissioner state" "active" +sleep 5 +switch_node 2 +wait_for "commissioner state" "disabled" + +switch_node 1 +send "commissioner start\n" +expect "Done" +wait_for "commissioner state" "active" +sleep 5 +switch_node 3 +wait_for "commissioner state" "disabled" + +########################################### +# Starting another commissioner on the same partition, using a different ID avoids +# the previous problem of overriding the active commissioner. + +switch_node 2 +send "commissioner id COMMISSIONER_2\n" +expect "Done" +send "commissioner start\n" +expect "Done" +wait_for "commissioner state" "petitioning" +wait_for "commissioner state" "disabled" + +switch_node 1 +send "commissioner state\n" +expect "active" + +switch_node 3 +send "commissioner id COMMISSIONER_3\n" +expect "Done" +send "commissioner start\n" +expect "Done" +wait_for "commissioner state" "petitioning" +wait_for "commissioner state" "disabled" + +switch_node 1 +send "commissioner state\n" +expect "active" + +########################################### +# Stop active commissioner and start another one +send "commissioner stop\n" + +switch_node 2 +send "commissioner id COMMISSIONER_2\n" +expect "Done" +send "commissioner start\n" +expect "Done" +wait_for "commissioner state" "petitioning" +wait_for "commissioner state" "active" + +switch_node 1 +send "commissioner start\n" +expect "Done" +wait_for "commissioner state" "petitioning" +wait_for "commissioner state" "disabled" + +########################################### +dispose_all diff --git a/tests/scripts/expect/cli-commissioner.exp b/tests/scripts/expect/cli-commissioner.exp index 2f8df3388..2c6737e86 100755 --- a/tests/scripts/expect/cli-commissioner.exp +++ b/tests/scripts/expect/cli-commissioner.exp @@ -41,16 +41,34 @@ send "commissioner joiner remove $eui64\n" expect_line "Done" wait_for "netdata steeringdata check $eui64" "NotFound" +# Stop commissioner so node 2 can start a new one with different id +send "commissioner stop\n" +expect "Done" + switch_node 2 send "commissioner state\n" expect "disabled" expect_line "Done" +send "commissioner id\n" +expect "OpenThread Commissioner" +expect_line "Done" +send "commissioner id reallyAndUnnecessaryLongOpenthreadComisionerCustomIdShouldNotFit\n" +expect "Error 7: InvalidArgs" +send "commissioner id reallyAndUnnecessaryLongOpenthreadCommissionerCustomIdShouldFit\n" +expect "Done" +send "commissioner id customId\n" +expect "Done" +send "commissioner id\n" +expect "customId" +expect_line "Done" send "commissioner start\n" expect_line "Done" expect "Commissioner: active" send "commissioner state\n" expect "active" expect_line "Done" +send "commissioner id AnotherCustomId\n" +expect_line "Error 13: InvalidState" send "commissioner provisioningurl openthread.io\n" expect_line "Done" send "commissioner joiner add * J01NME 1\n"