Enable TestDiscovery again (followup of #14598) (#14977)

examples/chip-tool/BUILD.gn

@@ -69,6 +69,7 @@ static_library("chip-tool-utils") {
 
   public_deps = [
     "${chip_root}/src/app/server",
+    "${chip_root}/src/app/tests/suites/commands/delay",
     "${chip_root}/src/app/tests/suites/commands/discovery",
     "${chip_root}/src/app/tests/suites/commands/log",
     "${chip_root}/src/app/tests/suites/commands/system",

examples/chip-tool/commands/tests/TestCommand.cpp

@@ -56,17 +56,6 @@ void TestCommand::OnDeviceConnectionFailureFn(void * context, PeerId peerId, CHI
     command->ContinueOnChipMainThread();
 }
 
-void TestCommand::OnWaitForMsFn(chip::System::Layer * systemLayer, void * context)
-{
-    auto * command = static_cast<TestCommand *>(context);
-    command->NextTest();
-}
-
-CHIP_ERROR TestCommand::Wait(chip::System::Clock::Timeout duration)
-{
-    return chip::DeviceLayer::SystemLayer().StartTimer(duration, OnWaitForMsFn, this);
-}
-
 void TestCommand::Exit(std::string message)
 {
     ChipLogError(chipTool, " ***** Test Failure: %s\n", message.c_str());

examples/chip-tool/commands/tests/TestCommand.h

@@ -19,6 +19,7 @@
 #pragma once
 
 #include "../common/CHIPCommand.h"
+#include <app/tests/suites/commands/delay/DelayCommands.h>
 #include <app/tests/suites/commands/discovery/DiscoveryCommands.h>
 #include <app/tests/suites/commands/log/LogCommands.h>
 #include <app/tests/suites/commands/system/SystemCommands.h>
@@ -36,7 +37,8 @@ class TestCommand : public CHIPCommand,
                     public PICSChecker,
                     public LogCommands,
                     public DiscoveryCommands,
-                    public SystemCommands
+                    public SystemCommands,
+                    public DelayCommands
 {
 public:
     TestCommand(const char * commandName, CredentialIssuerCommands * credsIssuerConfig) :
@@ -56,18 +58,16 @@ class TestCommand : public CHIPCommand,
 
     virtual void NextTest() = 0;
 
-    /////////// GlobalCommands Interface /////////
-    CHIP_ERROR Wait(chip::System::Clock::Timeout ms);
-    CHIP_ERROR WaitForMs(uint16_t ms) { return Wait(chip::System::Clock::Milliseconds32(ms)); }
-    CHIP_ERROR WaitForCommissionee();
-
 protected:
+    /////////// DelayCommands Interface /////////
+    CHIP_ERROR WaitForCommissionee() override;
+    void OnWaitForMs() override { NextTest(); };
+
     std::map<std::string, ChipDevice *> mDevices;
     chip::NodeId mNodeId;
 
     static void OnDeviceConnectedFn(void * context, chip::OperationalDeviceProxy * device);
     static void OnDeviceConnectionFailureFn(void * context, PeerId peerId, CHIP_ERROR error);
-    static void OnWaitForMsFn(chip::System::Layer * systemLayer, void * context);
 
     CHIP_ERROR ContinueOnChipMainThread() override { return WaitForMs(0); };
 

examples/chip-tool/templates/tests.js

@@ -250,6 +250,7 @@ function getTests()
     'TestClusterComplexTypes',
     'TestConstraints',
     'TestDelayCommands',
+    'TestDiscovery',
     'TestLogCommands',
     'TestSaveAs',
     'TestConfigVariables',

examples/placeholder/linux/BUILD.gn

@@ -41,6 +41,7 @@ executable("chip-${chip_tests_zap_config}") {
   deps = [
     ":configuration",
     "${chip_root}/examples/platform/linux:app-main",
+    "${chip_root}/src/app/tests/suites/commands/delay",
     "${chip_root}/src/app/tests/suites/commands/discovery",
     "${chip_root}/src/app/tests/suites/commands/log",
     "${chip_root}/src/app/tests/suites/pics",

examples/placeholder/linux/include/TestCommand.h

@@ -23,6 +23,7 @@
 #include <app/ConcreteAttributePath.h>
 #include <app/ConcreteCommandPath.h>
 
+#include <app/tests/suites/commands/delay/DelayCommands.h>
 #include <app/tests/suites/commands/discovery/DiscoveryCommands.h>
 #include <app/tests/suites/commands/log/LogCommands.h>
 #include <app/tests/suites/include/PICSChecker.h>
@@ -35,18 +36,14 @@ constexpr const char kIdentityAlpha[] = "";
 constexpr const char kIdentityBeta[]  = "";
 constexpr const char kIdentityGamma[] = "";
 
-class TestCommand : public PICSChecker, public LogCommands, public DiscoveryCommands
+class TestCommand : public PICSChecker, public LogCommands, public DiscoveryCommands, public DelayCommands
 {
 public:
     TestCommand(const char * commandName) : mCommandPath(0, 0, 0), mAttributePath(0, 0, 0) {}
     virtual ~TestCommand() {}
 
     virtual void NextTest() = 0;
-    CHIP_ERROR WaitMS(chip::System::Clock::Timeout ms)
-    {
-        return chip::DeviceLayer::SystemLayer().StartTimer(ms, OnWaitForMsFn, this);
-    }
-    CHIP_ERROR WaitForMs(uint16_t ms) { return WaitMS(chip::System::Clock::Milliseconds32(ms)); }
+
     void SetCommandExitStatus(CHIP_ERROR status)
     {
         chip::DeviceLayer::PlatformMgr().StopEventLoopTask();
@@ -85,12 +82,6 @@ class TestCommand : public PICSChecker, public LogCommands, public DiscoveryComm
         chip::DeviceLayer::PlatformMgr().RemoveEventHandler(OnPlatformEvent, context);
     }
 
-    CHIP_ERROR WaitForCommissioning()
-    {
-        isRunning = false;
-        return chip::DeviceLayer::PlatformMgr().AddEventHandler(OnPlatformEvent, reinterpret_cast<intptr_t>(this));
-    }
-
     static void OnPlatformEvent(const chip::DeviceLayer::ChipDeviceEvent * event, intptr_t arg)
     {
         switch (event->Type)
@@ -131,11 +122,6 @@ class TestCommand : public PICSChecker, public LogCommands, public DiscoveryComm
         mCommandPath   = chip::app::ConcreteCommandPath(0, 0, 0);
         mAttributePath = chip::app::ConcreteAttributePath(0, 0, 0);
     }
-    static void OnWaitForMsFn(chip::System::Layer * systemLayer, void * context)
-    {
-        auto * command = static_cast<TestCommand *>(context);
-        command->NextTest();
-    }
 
     std::atomic_bool isRunning{ true };
 
@@ -145,4 +131,13 @@ class TestCommand : public PICSChecker, public LogCommands, public DiscoveryComm
     chip::Optional<chip::EndpointId> mEndpointId;
     void SetIdentity(const char * name){};
     void Wait(){};
+
+    /////////// DelayCommands Interface /////////
+    void OnWaitForMs() override { NextTest(); }
+
+    CHIP_ERROR WaitForCommissioning() override
+    {
+        isRunning = false;
+        return chip::DeviceLayer::PlatformMgr().AddEventHandler(OnPlatformEvent, reinterpret_cast<intptr_t>(this));
+    }
 };

scripts/tests/chiptest/accessories.py

@@ -91,6 +91,18 @@ def factoryReset(self, name):
             return accessory.factoryReset()
         return False
 
+    def waitForCommissionableAdvertisement(self, name):
+        accessory = self.__accessories[name]
+        if accessory:
+            return accessory.waitForCommissionableAdvertisement()
+        return False
+
+    def waitForOperationalAdvertisement(self, name):
+        accessory = self.__accessories[name]
+        if accessory:
+            return accessory.waitForOperationalAdvertisement()
+        return False
+
     def ping(self):
         return True
 
@@ -101,6 +113,10 @@ def __startXMLRPCServer(self):
         self.server.register_function(self.stop, 'stop')
         self.server.register_function(self.reboot, 'reboot')
         self.server.register_function(self.factoryReset, 'factoryReset')
+        self.server.register_function(
+            self.waitForCommissionableAdvertisement, 'waitForCommissionableAdvertisement')
+        self.server.register_function(
+            self.waitForOperationalAdvertisement, 'waitForOperationalAdvertisement')
         self.server.register_function(self.ping, 'ping')
 
         self.server_thread = threading.Thread(target=self.__handle_request)

scripts/tests/chiptest/runner.py

@@ -47,8 +47,11 @@ def __init__(self, level, capture_delegate=None, name=None):
 
         self.start()
 
-    def CapturedLogContains(self, txt: str):
-        return any(txt in l for l in self.captured_logs)
+    def CapturedLogContains(self, txt: str, index=0):
+        for i, line in enumerate(self.captured_logs[index:]):
+            if txt in line:
+                return True, i
+        return False, len(self.captured_logs)
 
     def FindLastMatchingLine(self, matcher):
         for l in reversed(self.captured_logs):

scripts/tests/chiptest/test_definition.py

@@ -32,18 +32,21 @@
 class App:
     def __init__(self, runner, command):
         self.process = None
+        self.outpipe = None
         self.runner = runner
         self.command = command
         self.stopped = False
+        self.lastLogIndex = 0
 
     def start(self, discriminator):
         if not self.process:
             self.process = None
             process, outpipe, errpipe = self.__startServer(
                 self.runner, self.command, discriminator)
-            self.__waitForServerReady(process, outpipe)
+            self.waitForAnyAdvertisement(process, outpipe)
             self.__updateSetUpCode(outpipe)
             self.process = process
+            self.outpipe = outpipe
             self.stopped = False
             return True
         return False
@@ -54,6 +57,7 @@ def stop(self):
             self.process.kill()
             self.process.wait(10)
             self.process = None
+            self.outpipe = None
             return True
         return False
 
@@ -71,6 +75,19 @@ def factoryReset(self):
 
         return True
 
+    def waitForAnyAdvertisement(self, process, outpipe):
+        self.__waitFor("mDNS service published:", process, outpipe)
+
+    def waitForCommissionableAdvertisement(self):
+        self.__waitFor("mDNS service published: _matterc._udp",
+                       self.process, self.outpipe)
+        return True
+
+    def waitForOperationalAdvertisement(self):
+        self.__waitFor("mDNS service published: _matter._tcp",
+                       self.process, self.outpipe)
+        return True
+
     def poll(self):
         # When the server is manually stopped, process polling is overriden so the other
         # processes that depends on the accessory beeing alive does not stop.
@@ -92,21 +109,25 @@ def __startServer(self, runner, command, discriminator):
         app_cmd = command + ['--discriminator', str(discriminator)]
         return runner.RunSubprocess(app_cmd, name='APP ', wait=False)
 
-    def __waitForServerReady(self, server_process, outpipe):
-        logging.debug('Waiting for server to listen.')
+    def __waitFor(self, waitForString, server_process, outpipe):
+        logging.debug('Waiting for %s' % waitForString)
+
         start_time = time.time()
-        server_is_listening = outpipe.CapturedLogContains("Server Listening")
-        while not server_is_listening:
+        ready, self.lastLogIndex = outpipe.CapturedLogContains(
+            waitForString, self.lastLogIndex)
+        while not ready:
             if server_process.poll() is not None:
-                died_str = 'Server died during startup, returncode %d' % server_process.returncode
+                died_str = 'Server died while waiting for %s, returncode %d' % (
+                    waitForString, server_process.returncode)
                 logging.error(died_str)
                 raise Exception(died_str)
             if time.time() - start_time > 10:
-                raise Exception('Timeout for server listening')
+                raise Exception('Timeout while waiting for %s' % waitForString)
             time.sleep(0.1)
-            server_is_listening = outpipe.CapturedLogContains(
-                "Server Listening")
-        logging.debug('Server is listening. Can proceed.')
+            ready, self.lastLogIndex = outpipe.CapturedLogContains(
+                waitForString, self.lastLogIndex)
+
+        logging.debug('Success waiting for: %s' % waitForString)
 
     def __updateSetUpCode(self, outpipe):
         qrLine = outpipe.FindLastMatchingLine('.*SetupQRCode: *\\[(.*)]')

src/app/tests/suites/TestDiscovery.yaml

@@ -24,7 +24,7 @@ config:
         defaultValue: 65521
     productId:
         type: INT16U
-        defaultValue: 32768
+        defaultValue: 32769
     deviceType:
         type: INT16U
         defaultValue: 5
@@ -51,13 +51,9 @@ tests:
               - name: "CommissioningTimeout"
                 value: 120
 
-    - label: "Wait 1000ms for the mdns advertisement to be published"
+    - label: "Wait Commissionable advertisement"
       cluster: "DelayCommands"
-      command: "WaitForMs"
-      arguments:
-          values:
-              - name: "ms"
-                value: 1000
+      command: "WaitForCommissionableAdvertisement"
 
     - label: "Check Instance Name"
       cluster: "DiscoveryCommands"
@@ -292,13 +288,9 @@ tests:
               - name: "CommissioningTimeout"
                 value: 120
 
-    - label: "Wait 1000ms for the mdns advertisement to be published"
+    - label: "Wait Commissionable advertisement"
       cluster: "DelayCommands"
-      command: "WaitForMs"
-      arguments:
-          values:
-              - name: "ms"
-                value: 1000
+      command: "WaitForCommissionableAdvertisement"
 
     - label: "Check Instance Name"
       cluster: "DiscoveryCommands"

src/app/tests/suites/commands/delay/BUILD.gn

@@ -0,0 +1,33 @@
+# Copyright (c) 2022 Project CHIP Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import("//build_overrides/build.gni")
+import("//build_overrides/chip.gni")
+
+static_library("delay") {
+  output_name = "libDelayCommands"
+
+  sources = [
+    "DelayCommands.cpp",
+    "DelayCommands.h",
+  ]
+
+  cflags = [ "-Wconversion" ]
+
+  public_deps = [
+    "${chip_root}/src/lib/support",
+    "${chip_root}/src/platform",
+    "${chip_root}/src/system",
+  ]
+}