User Directed Commissioning: simplify shell commands (project-chip#10364

)

* UDC: persist device name, IP and port in cache, simplify shell commands

* fix CI

* enable ip commissioning from tv-app

* address comment on shell command names

examples/platform/linux/AppMain.cpp

@@ -305,7 +305,7 @@ void ChipLinuxAppMainLoop()
 #if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
     InitCommissioner();
 #if defined(ENABLE_CHIP_SHELL)
-    chip::Shell::RegisterDiscoverCommands(&gCommissioner);
+    chip::Shell::RegisterControllerCommands(&gCommissioner);
 #endif // defined(ENABLE_CHIP_SHELL)
 #endif // CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
 

examples/platform/linux/ControllerShellCommands.cpp

@@ -46,7 +46,7 @@ static CHIP_ERROR ResetUDC(bool printHeader)
 
     if (printHeader)
     {
-        streamer_printf(sout, "resetudc:        ");
+        streamer_printf(sout, "udc-reset:        ");
     }
 
     gCommissioner->GetUserDirectedCommissioningServer()->ResetUDCClientProcessingStates();
@@ -56,6 +56,22 @@ static CHIP_ERROR ResetUDC(bool printHeader)
     return CHIP_NO_ERROR;
 }
 
+static CHIP_ERROR PrintUDC(bool printHeader)
+{
+    streamer_t * sout = streamer_get();
+
+    if (printHeader)
+    {
+        streamer_printf(sout, "udc-print:        \r\n");
+    }
+
+    gCommissioner->GetUserDirectedCommissioningServer()->PrintUDCClients();
+
+    streamer_printf(sout, "done\r\n");
+
+    return CHIP_NO_ERROR;
+}
+
 static CHIP_ERROR discover(bool printHeader)
 {
     streamer_t * sout = streamer_get();
@@ -132,13 +148,12 @@ class PairingCommand : public chip::Controller::DevicePairingDelegate, public ch
     CHIP_ERROR UpdateNetworkAddress();
 };
 
-NodeId gRemoteId   = kTestDeviceNodeId;
-uint64_t gFabricId = 0;
+NodeId gRemoteId = kTestDeviceNodeId;
 PairingCommand gPairingCommand;
 
 CHIP_ERROR PairingCommand::UpdateNetworkAddress()
 {
-    ChipLogProgress(chipTool, "Mdns: Updating NodeId: %" PRIx64 " FabricId: %" PRIx64 " ...", gRemoteId, gFabricId);
+    ChipLogProgress(chipTool, "Mdns: Updating NodeId: %" PRIx64 " ...", gRemoteId);
     return gCommissioner->UpdateDevice(gRemoteId);
 }
 
@@ -197,8 +212,7 @@ void PairingCommand::OnCommissioningComplete(NodeId nodeId, CHIP_ERROR err)
     }
 }
 
-static CHIP_ERROR pairOnNetwork(bool printHeader, uint64_t fabric, uint32_t pincode, uint16_t disc,
-                                chip::Transport::PeerAddress address)
+static CHIP_ERROR pairOnNetwork(bool printHeader, uint32_t pincode, uint16_t disc, chip::Transport::PeerAddress address)
 {
     streamer_t * sout = streamer_get();
 
@@ -217,34 +231,76 @@ static CHIP_ERROR pairOnNetwork(bool printHeader, uint64_t fabric, uint32_t pinc
 
     return CHIP_NO_ERROR;
 }
+
+static CHIP_ERROR pairUDC(bool printHeader, uint32_t pincode, size_t index)
+{
+    streamer_t * sout = streamer_get();
+
+    if (printHeader)
+    {
+        streamer_printf(sout, "udc-commission %ld %ld\r\n", pincode, index);
+    }
+
+    UDCClientState * state = gCommissioner->GetUserDirectedCommissioningServer()->GetUDCClients().GetUDCClientState(index);
+    if (state == nullptr)
+    {
+        streamer_printf(sout, "udc client[%d] null \r\n", index);
+    }
+    else
+    {
+        Transport::PeerAddress peerAddress = state->GetPeerAddress();
+
+        state->SetUDCClientProcessingState(UDCClientProcessingState::kCommissioningNode);
+
+        RendezvousParameters params = RendezvousParameters()
+                                          .SetSetupPINCode(pincode)
+                                          .SetDiscriminator(state->GetLongDiscriminator())
+                                          .SetPeerAddress(peerAddress);
+
+        gCommissioner->RegisterDeviceAddressUpdateDelegate(&gPairingCommand);
+        gCommissioner->RegisterPairingDelegate(&gPairingCommand);
+        gCommissioner->PairDevice(gRemoteId, params);
+
+        streamer_printf(sout, "done\r\n");
+    }
+    return CHIP_NO_ERROR;
+}
 #endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY
 
 static CHIP_ERROR PrintAllCommands()
 {
     streamer_t * sout = streamer_get();
-    streamer_printf(sout, "  help                       Usage: discover <subcommand>\r\n");
+    streamer_printf(sout, "  help                       Usage: controller <subcommand>\r\n");
 #if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY
     streamer_printf(
-        sout, "  resetudc                   Clear all pending UDC sessions from this UDC server. Usage: commission resetudc\r\n");
+        sout, "  udc-reset                   Clear all pending UDC sessions from this UDC server. Usage: controller udc-reset\r\n");
+    streamer_printf(
+        sout, "  udc-print                   Print all pending UDC sessions from this UDC server. Usage: controller udc-print\r\n");
 #endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY
 #if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
-    streamer_printf(sout, "  commissionable                   Discover all commissionable nodes. Usage: commission discover\r\n");
+    streamer_printf(sout,
+                    "  udc-commission <pincode> <udc-entry>     Commission given udc-entry using given pincode. Usage: controller "
+                    "udc-commission 34567890 0\r\n");
     streamer_printf(
         sout,
-        "  commissionable-instance <name>   Discover all commissionable node with given instance name. Usage: commission "
-        "commissionable-instance DC514873944A5CFF\r\n");
-    streamer_printf(sout,
-                    "  display                    Display all discovered commissionable nodes. Usage: commission display\r\n");
-    streamer_printf(sout,
-                    "  onnetwork <fabric> <pincode> <disc> <IP> <port>   Pair given device. Usage: commission onnetwork 2222 "
-                    "20202021 3840 127.0.0.1 5540\r\n");
+        "  discover-commissionable          Discover all commissionable nodes. Usage: controller discover-commissionable\r\n");
+    streamer_printf(
+        sout,
+        "  discover-commissionable-instance <name>   Discover all commissionable node with given instance name. Usage: controller "
+        "discover-commissionable-instance DC514873944A5CFF\r\n");
+    streamer_printf(
+        sout, "  discover-display           Display all discovered commissionable nodes. Usage: controller discover-display\r\n");
+    streamer_printf(
+        sout,
+        "  commission-onnetwork <pincode> <disc> <IP> <port>   Pair given device. Usage: controller commission-onnetwork "
+        "20202021 3840 127.0.0.1 5540\r\n");
 #endif // CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
     streamer_printf(sout, "\r\n");
 
     return CHIP_NO_ERROR;
 }
 
-static CHIP_ERROR DiscoverHandler(int argc, char ** argv)
+static CHIP_ERROR ControllerHandler(int argc, char ** argv)
 {
     CHIP_ERROR error = CHIP_NO_ERROR;
 
@@ -253,42 +309,57 @@ static CHIP_ERROR DiscoverHandler(int argc, char ** argv)
         return PrintAllCommands();
     }
 #if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY
-    else if (strcmp(argv[0], "resetudc") == 0)
+    else if (strcmp(argv[0], "udc-reset") == 0)
     {
         return error = ResetUDC(true);
     }
+    else if (strcmp(argv[0], "udc-print") == 0)
+    {
+        return error = PrintUDC(true);
+    }
 #endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY
 #if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
-    else if (strcmp(argv[0], "commissionable") == 0)
+    else if (strcmp(argv[0], "discover-commissionable") == 0)
     {
         return error = discover(true);
     }
-    else if (strcmp(argv[0], "commissionable-instance") == 0)
+    else if (strcmp(argv[0], "discover-commissionable-instance") == 0)
     {
         return error = discover(true, argv[1]);
     }
-    else if (strcmp(argv[0], "display") == 0)
+    else if (strcmp(argv[0], "discover-display") == 0)
     {
         return error = display(true);
     }
-    else if (strcmp(argv[0], "onnetwork") == 0)
+    else if (strcmp(argv[0], "commission-onnetwork") == 0)
     {
-        // onnetwork fabric pincode disc IP port
-        if (argc < 6)
+        // onnetwork pincode disc IP port
+        if (argc < 5)
         {
             return PrintAllCommands();
         }
         char * eptr;
-        gFabricId        = (uint64_t) strtol(argv[1], &eptr, 10);
-        uint32_t pincode = (uint32_t) strtol(argv[2], &eptr, 10);
-        uint16_t disc    = (uint16_t) strtol(argv[3], &eptr, 10);
+        uint32_t pincode = (uint32_t) strtol(argv[1], &eptr, 10);
+        uint16_t disc    = (uint16_t) strtol(argv[2], &eptr, 10);
 
         chip::Inet::IPAddress address;
-        chip::Inet::IPAddress::FromString(argv[4], address);
+        chip::Inet::IPAddress::FromString(argv[3], address);
 
-        uint16_t port = (uint16_t) strtol(argv[5], &eptr, 10);
+        uint16_t port = (uint16_t) strtol(argv[4], &eptr, 10);
 
-        return error = pairOnNetwork(true, gFabricId, pincode, disc, chip::Transport::PeerAddress::UDP(address, port));
+        return error = pairOnNetwork(true, pincode, disc, chip::Transport::PeerAddress::UDP(address, port));
+    }
+    else if (strcmp(argv[0], "udc-commission") == 0)
+    {
+        // udc-commission pincode index
+        if (argc < 3)
+        {
+            return PrintAllCommands();
+        }
+        char * eptr;
+        uint32_t pincode = (uint32_t) strtol(argv[1], &eptr, 10);
+        size_t index     = (size_t) strtol(argv[2], &eptr, 10);
+        return error     = pairUDC(true, pincode, index);
     }
 #endif // CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
     else
@@ -298,11 +369,11 @@ static CHIP_ERROR DiscoverHandler(int argc, char ** argv)
     return error;
 }
 
-void RegisterDiscoverCommands(chip::Controller::DeviceCommissioner * commissioner)
+void RegisterControllerCommands(chip::Controller::DeviceCommissioner * commissioner)
 {
     gCommissioner                              = commissioner;
-    static const shell_command_t sDeviceComand = { &DiscoverHandler, "discover",
-                                                   "Discover commands. Usage: discover [command_name]" };
+    static const shell_command_t sDeviceComand = { &ControllerHandler, "controller",
+                                                   "Controller commands. Usage: controller [command_name]" };
 
     // Register the root `device` command with the top-level shell.
     Engine::Root().RegisterCommands(&sDeviceComand, 1);

examples/platform/linux/ControllerShellCommands.h

@@ -25,7 +25,7 @@
 namespace chip {
 namespace Shell {
 
-void RegisterDiscoverCommands(chip::Controller::DeviceCommissioner * commissioner);
+void RegisterControllerCommands(chip::Controller::DeviceCommissioner * commissioner);
 
 } // namespace Shell
 } // namespace chip

examples/tv-app/linux/args.gni

@@ -25,3 +25,4 @@ chip_project_config_include_dirs =
 chip_project_config_include_dirs += [ "${chip_root}/config/standalone" ]
 
 chip_build_libshell = true
+chip_use_clusters_for_ip_commissioning = true

examples/tv-casting-app/linux/args.gni

@@ -23,3 +23,5 @@ chip_system_project_config_include = "<SystemProjectConfig.h>"
 chip_project_config_include_dirs =
     [ "${chip_root}/examples/tv-casting-app/tv-casting-common/include" ]
 chip_project_config_include_dirs += [ "${chip_root}/config/standalone" ]
+
+chip_build_libshell = true

src/controller/CHIPDeviceController.cpp

@@ -1580,9 +1580,17 @@ void DeviceCommissioner::FindCommissionableNode(char * instanceName)
     DiscoverCommissionableNodes(filter);
 }
 
-void DeviceCommissioner::OnUserDirectedCommissioningRequest(const Dnssd::DiscoveredNodeData & nodeData)
+void DeviceCommissioner::OnUserDirectedCommissioningRequest(UDCClientState state)
 {
-    ChipLogDetail(Controller, "------PROMPT USER!! OnUserDirectedCommissioningRequest instance=%s", nodeData.instanceName);
+    ChipLogDetail(Controller, "------PROMPT USER!! OnUserDirectedCommissioningRequest instance=%s deviceName=%s",
+                  state.GetInstanceName(), state.GetDeviceName());
+
+    if (mUdcServer != nullptr)
+    {
+        mUdcServer->PrintUDCClients();
+    }
+
+    ChipLogDetail(Controller, "------To Accept Enter: discover udc-commission <pincode> <udc-client-index>");
 }
 
 void DeviceCommissioner::OnNodeDiscoveryComplete(const chip::Dnssd::DiscoveredNodeData & nodeData)

src/controller/CHIPDeviceController.h

@@ -554,7 +554,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
      * @param nodeData DNS-SD node information for the client requesting commissioning
      *
      */
-    void OnUserDirectedCommissioningRequest(const Dnssd::DiscoveredNodeData & nodeData) override;
+    void OnUserDirectedCommissioningRequest(UDCClientState state) override;
 
     /**
      * @brief

src/lib/dnssd/Resolver.h

@@ -118,6 +118,7 @@ struct DiscoveredNodeData
         mrpRetryIntervalIdle   = kUndefinedRetryInterval;
         mrpRetryIntervalActive = kUndefinedRetryInterval;
         numIPs                 = 0;
+        port                   = 0;
         for (int i = 0; i < kMaxIPAddresses; ++i)
         {
             ipAddress[i] = chip::Inet::IPAddress::Any;

src/protocols/user_directed_commissioning/UDCClientState.h

@@ -63,13 +63,18 @@ class UDCClientState
     UDCClientState & operator=(const UDCClientState &) = default;
     UDCClientState & operator=(UDCClientState &&) = default;
 
-    const PeerAddress & GetPeerAddress() const { return mPeerAddress; }
-    PeerAddress & GetPeerAddress() { return mPeerAddress; }
+    const PeerAddress GetPeerAddress() const { return mPeerAddress; }
     void SetPeerAddress(const PeerAddress & address) { mPeerAddress = address; }
 
     const char * GetInstanceName() const { return mInstanceName; }
     void SetInstanceName(const char * instanceName) { strncpy(mInstanceName, instanceName, sizeof(mInstanceName)); }
 
+    const char * GetDeviceName() const { return mDeviceName; }
+    void SetDeviceName(const char * deviceName) { strncpy(mDeviceName, deviceName, sizeof(mDeviceName)); }
+
+    uint16_t GetLongDiscriminator() const { return mLongDiscriminator; }
+    void SetLongDiscriminator(uint16_t value) { mLongDiscriminator = value; }
+
     UDCClientProcessingState GetUDCClientProcessingState() const { return mUDCClientProcessingState; }
     void SetUDCClientProcessingState(UDCClientProcessingState state) { mUDCClientProcessingState = state; }
 
@@ -95,6 +100,8 @@ class UDCClientState
 private:
     PeerAddress mPeerAddress;
     char mInstanceName[Dnssd::Commissionable::kInstanceNameMaxLength + 1];
+    char mDeviceName[Dnssd::kMaxDeviceNameLen + 1];
+    uint16_t mLongDiscriminator = 0;
     UDCClientProcessingState mUDCClientProcessingState;
     uint64_t mExpirationTimeMs = 0;
 };

src/protocols/user_directed_commissioning/UDCClients.h

@@ -84,6 +84,30 @@ class UDCClients
         return err;
     }
 
+    /**
+     * Get a UDC Client state given a Peer address.
+     *
+     * @param address is the connection to find (based on address)
+     *
+     * @return the state found, nullptr if not found
+     */
+    CHECK_RETURN_VALUE
+    UDCClientState * GetUDCClientState(size_t index)
+    {
+        if (index >= kMaxClientCount)
+        {
+            return nullptr;
+        }
+
+        const uint64_t currentTime = mTimeSource.GetCurrentMonotonicTimeMs();
+        UDCClientState state       = mStates[index];
+        if (!state.IsInitialized(currentTime))
+        {
+            return nullptr;
+        }
+        return &mStates[index];
+    }
+
     /**
      * Get a UDC Client state given a Peer address.
      *

src/protocols/user_directed_commissioning/UserDirectedCommissioning.h

@@ -42,7 +42,7 @@ namespace Protocols {
 namespace UserDirectedCommissioning {
 
 // Cache contains 16 clients. This may need to be tweaked.
-constexpr size_t kMaxUDCClients = 16;
+constexpr uint8_t kMaxUDCClients = 16;
 
 /**
  * User Directed Commissioning Protocol Message Types
@@ -82,7 +82,7 @@ class DLL_EXPORT UserConfirmationProvider
      * @param nodeData DNS-SD node information for the client requesting commissioning
      *
      */
-    virtual void OnUserDirectedCommissioningRequest(const Dnssd::DiscoveredNodeData & nodeData) = 0;
+    virtual void OnUserDirectedCommissioningRequest(UDCClientState state) = 0;
 
     virtual ~UserConfirmationProvider() = default;
 };
@@ -185,6 +185,12 @@ class DLL_EXPORT UserDirectedCommissioningServer : public TransportMgrDelegate
      */
     UDCClients<kMaxUDCClients> GetUDCClients() { return mUdcClients; }
 
+    /**
+     * Print the cache of UDC Clients
+     *
+     */
+    void PrintUDCClients();
+
 private:
     InstanceNameResolver * mInstanceNameResolver         = nullptr;
     UserConfirmationProvider * mUserConfirmationProvider = nullptr;