[commissioner] add command line option to specify commissioner ID (#7520)

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.
  *

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

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 \<name\>
+
+Set the commissioner id.
+
+```bash
+> commissioner id "Custom Commissioner Id"
+Done
+```
+
 ### start
 
 Usage: `commissioner start`

src/cli/cli_commissioner.cpp

@@ -332,6 +332,23 @@ template <> otError Commissioner::Process<Cmd("sessionid")>(Arg aArgs[])
     return OT_ERROR_NONE;
 }
 
+template <> otError Commissioner::Process<Cmd("id")>(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<Cmd("start")>(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

src/core/api/commissioner_api.cpp

@@ -50,6 +50,16 @@ otError otCommissionerStart(otInstance *                 aInstance,
     return AsCoreType(aInstance).Get<MeshCoP::Commissioner>().Start(aStateCallback, aJoinerCallback, aCallbackContext);
 }
 
+const char *otCommissionerGetId(otInstance *aInstance)
+{
+    return AsCoreType(aInstance).Get<MeshCoP::Commissioner>().GetId();
+}
+
+otError otCommissionerSetId(otInstance *aInstance, const char *aId)
+{
+    return AsCoreType(aInstance).Get<MeshCoP::Commissioner>().SetId(aId);
+}
+
 otError otCommissionerStop(otInstance *aInstance)
 {
     return AsCoreType(aInstance).Get<MeshCoP::Commissioner>().Stop();

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<uint8_t>(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<Tmf::Agent>().SendMessage(*message, messageInfo, Commissioner::HandleLeaderPetitionResponse, this));

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;
 

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

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"