Merge branch 'master' into idm-4.2-troubleshoot

raul-marquez-csa · May 22, 2024 · 1b265c0 · 1b265c0
2 parents c87de26 + b5d13a7
commit 1b265c0
Show file tree

Hide file tree

Showing 26 changed files with 1,175 additions and 1,458 deletions.
diff --git a/.gn b/.gn
@@ -25,6 +25,7 @@ script_executable = "python3"
 
 default_args = {
   pw_unit_test_AUTOMATIC_RUNNER = "$dir_pigweed/targets/host/run_test"
+  pw_unit_test_CONFIG = "//config/pw_unit_test:define_overrides"
 
   pw_build_PIP_CONSTRAINTS = [ "//scripts/setup/constraints.txt" ]
   pw_build_PIP_REQUIREMENTS = [ "//scripts/setup/requirements.build.txt" ]

diff --git a/config/pw_unit_test/BUILD.gn b/config/pw_unit_test/BUILD.gn
@@ -0,0 +1,32 @@
+# Copyright (c) 2024 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")
+import("//build_overrides/pigweed.gni")
+
+import("${chip_root}/build/chip/tests.gni")
+
+import("$dir_pw_build/target_types.gni")
+pw_source_set("define_overrides") {
+  public_configs = [ ":define_options" ]
+}
+
+config("define_options") {
+  if (chip_fake_platform && chip_link_tests) {
+    defines = [ "PW_UNIT_TEST_CONFIG_MEMORY_POOL_SIZE=65536" ]
+  } else {
+    defines = [ "PW_UNIT_TEST_CONFIG_MEMORY_POOL_SIZE=16384" ]
+  }
+}
diff --git a/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter b/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter
@@ -3752,7 +3752,7 @@ cluster ActivatedCarbonFilterMonitoring = 114 {
 }
 
 /** This cluster is used to configure a boolean sensor. */
-provisional cluster BooleanStateConfiguration = 128 {
+cluster BooleanStateConfiguration = 128 {
   revision 1;
 
   bitmap AlarmModeBitmap : bitmap8 {
@@ -3810,7 +3810,7 @@ provisional cluster BooleanStateConfiguration = 128 {
 }
 
 /** This cluster is used to configure a valve. */
-provisional cluster ValveConfigurationAndControl = 129 {
+cluster ValveConfigurationAndControl = 129 {
   revision 1;
 
   enum StatusCodeEnum : enum8 {

diff --git a/src/app/zap-templates/zcl/data-model/chip/boolean-state-configuration-cluster.xml b/src/app/zap-templates/zcl/data-model/chip/boolean-state-configuration-cluster.xml
@@ -28,7 +28,7 @@ limitations under the License.
     <field name="GeneralFault" mask="0x1"/>
   </bitmap>
 
-  <cluster apiMaturity="provisional">
+  <cluster>
     <domain>General</domain>
     <name>Boolean State Configuration</name>
     <code>0x0080</code>

diff --git a/src/app/zap-templates/zcl/data-model/chip/valve-configuration-and-control-cluster.xml b/src/app/zap-templates/zcl/data-model/chip/valve-configuration-and-control-cluster.xml
@@ -40,7 +40,7 @@ limitations under the License.
      <item name="FailureDueToFault" value="0x02"/>
   </enum>
 
-  <cluster apiMaturity="provisional">
+  <cluster>
     <domain>HVAC</domain>
     <name>Valve Configuration and Control</name>
     <code>0x0081</code>

diff --git a/src/controller/data_model/controller-clusters.matter b/src/controller/data_model/controller-clusters.matter
@@ -3988,7 +3988,7 @@ cluster ActivatedCarbonFilterMonitoring = 114 {
 }
 
 /** This cluster is used to configure a boolean sensor. */
-provisional cluster BooleanStateConfiguration = 128 {
+cluster BooleanStateConfiguration = 128 {
   revision 1;
 
   bitmap AlarmModeBitmap : bitmap8 {
@@ -4046,7 +4046,7 @@ provisional cluster BooleanStateConfiguration = 128 {
 }
 
 /** This cluster is used to configure a valve. */
-provisional cluster ValveConfigurationAndControl = 129 {
+cluster ValveConfigurationAndControl = 129 {
   revision 1;
 
   enum StatusCodeEnum : enum8 {

diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm
@@ -881,24 +881,21 @@ - (MTRDeviceController * _Nullable)maybeInitializeOTAProvider:(MTRDeviceControll
 
 - (void)resetOperationalAdvertising
 {
-    if (!_advertiseOperational) {
-        // No need to reset anything; we are not advertising the things that
-        // would need to get reset.
-        return;
-    }
+    assertChipStackLockedByCurrentThread();
 
-    std::lock_guard lock(_controllersLock);
-    if (_controllers.count != 0) {
-        // We have a running controller.  That means we likely need to reset
-        // operational advertising for that controller.
-        dispatch_async(_chipWorkQueue, ^{
-            // StartServer() is the only API we have for resetting DNS-SD
-            // advertising.  It sure would be nice if there were a "restart"
-            // that was a no-op if the DNS-SD server was not already
-            // running.
-            app::DnssdServer::Instance().StartServer();
-        });
+    // If we're not advertising, then there's no need to reset anything.
+    VerifyOrReturn(_advertiseOperational);
+
+    // If there are no running controllers there will be no advertisements to reset.
+    {
+        std::lock_guard lock(_controllersLock);
+        VerifyOrReturn(_controllers.count > 0);
     }
+
+    // StartServer() is the only API we have for resetting DNS-SD advertising.
+    // It sure would be nice if there were a "restart" that was a no-op if the
+    // DNS-SD server was not already running.
+    app::DnssdServer::Instance().StartServer();
 }
 
 - (void)controllerShuttingDown:(MTRDeviceController *)controller
@@ -1165,6 +1162,45 @@ - (MTRDeviceController * _Nullable)initializeController:(MTRDeviceController *)c
                                   error:error];
 }
 
+- (void)setMessageReliabilityProtocolIdleRetransmitMs:(nullable NSNumber *)idleRetransmitMs
+                                   activeRetransmitMs:(nullable NSNumber *)activeRetransmitMs
+                                    activeThresholdMs:(nullable NSNumber *)activeThresholdMs
+                          additionalRetransmitDelayMs:(nullable NSNumber *)additionalRetransmitDelayMs
+{
+    [self _assertCurrentQueueIsNotMatterQueue];
+    dispatch_async(_chipWorkQueue, ^{
+        bool resetAdvertising;
+        if (idleRetransmitMs == nil && activeRetransmitMs == nil && activeThresholdMs == nil && additionalRetransmitDelayMs == nil) {
+            Messaging::ReliableMessageMgr::SetAdditionalMRPBackoffTime(NullOptional);
+            resetAdvertising = ReliableMessageProtocolConfig::SetLocalMRPConfig(NullOptional);
+        } else {
+            if (additionalRetransmitDelayMs != nil) {
+                System::Clock::Timeout additionalBackoff(additionalRetransmitDelayMs.unsignedLongValue);
+                Messaging::ReliableMessageMgr::SetAdditionalMRPBackoffTime(MakeOptional(additionalBackoff));
+            }
+
+            // Get current MRP parameters, then override the things we were asked to
+            // override.
+            ReliableMessageProtocolConfig mrpConfig = GetLocalMRPConfig().ValueOr(GetDefaultMRPConfig());
+            if (idleRetransmitMs != nil) {
+                mrpConfig.mIdleRetransTimeout = System::Clock::Milliseconds32(idleRetransmitMs.unsignedLongValue);
+            }
+            if (activeRetransmitMs != nil) {
+                mrpConfig.mActiveRetransTimeout = System::Clock::Milliseconds32(activeRetransmitMs.unsignedLongValue);
+            }
+            if (activeThresholdMs != nil) {
+                mrpConfig.mActiveThresholdTime = System::Clock::Milliseconds32(activeThresholdMs.unsignedLongValue);
+            }
+
+            resetAdvertising = ReliableMessageProtocolConfig::SetLocalMRPConfig(MakeOptional(mrpConfig));
+        }
+
+        if (resetAdvertising) {
+            [self resetOperationalAdvertising];
+        }
+    });
+}
+
 - (PersistentStorageDelegate *)storageDelegate
 {
     return _persistentStorageDelegate;
@@ -1327,33 +1363,8 @@ void MTRSetMessageReliabilityParameters(NSNumber * _Nullable idleRetransmitMs,
     NSNumber * _Nullable activeThresholdMs,
     NSNumber * _Nullable additionalRetransmitDelayMs)
 {
-    bool resetAdvertising = false;
-    if (idleRetransmitMs == nil && activeRetransmitMs == nil && activeThresholdMs == nil && additionalRetransmitDelayMs == nil) {
-        Messaging::ReliableMessageMgr::SetAdditionalMRPBackoffTime(NullOptional);
-        resetAdvertising = ReliableMessageProtocolConfig::SetLocalMRPConfig(NullOptional);
-    } else {
-        if (additionalRetransmitDelayMs != nil) {
-            System::Clock::Timeout additionalBackoff(additionalRetransmitDelayMs.unsignedLongValue);
-            Messaging::ReliableMessageMgr::SetAdditionalMRPBackoffTime(MakeOptional(additionalBackoff));
-        }
-
-        // Get current MRP parameters, then override the things we were asked to
-        // override.
-        ReliableMessageProtocolConfig mrpConfig = GetLocalMRPConfig().ValueOr(GetDefaultMRPConfig());
-        if (idleRetransmitMs != nil) {
-            mrpConfig.mIdleRetransTimeout = System::Clock::Milliseconds32(idleRetransmitMs.unsignedLongValue);
-        }
-        if (activeRetransmitMs != nil) {
-            mrpConfig.mActiveRetransTimeout = System::Clock::Milliseconds32(activeRetransmitMs.unsignedLongValue);
-        }
-        if (activeThresholdMs != nil) {
-            mrpConfig.mActiveThresholdTime = System::Clock::Milliseconds32(activeThresholdMs.unsignedLongValue);
-        }
-
-        resetAdvertising = ReliableMessageProtocolConfig::SetLocalMRPConfig(MakeOptional(mrpConfig));
-    }
-
-    if (resetAdvertising) {
-        [[MTRDeviceControllerFactory sharedInstance] resetOperationalAdvertising];
-    }
+    [MTRDeviceControllerFactory.sharedInstance setMessageReliabilityProtocolIdleRetransmitMs:idleRetransmitMs
+                                                                          activeRetransmitMs:activeThresholdMs
+                                                                           activeThresholdMs:activeThresholdMs
+                                                                 additionalRetransmitDelayMs:additionalRetransmitDelayMs];
 }
diff --git a/src/darwin/Framework/CHIPTests/MTRControllerTests.m b/src/darwin/Framework/CHIPTests/MTRControllerTests.m
@@ -1552,4 +1552,11 @@ - (void)testControllerCATs
     XCTAssertFalse([factory isRunning]);
 }
 
+- (void)testSetMRPParameters
+{
+    // Can be called before starting the factory
+    XCTAssertFalse(MTRDeviceControllerFactory.sharedInstance.running);
+    MTRSetMessageReliabilityParameters(@2000, @2000, @2000, @2000);
+}
+
 @end
diff --git a/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m b/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m
@@ -2035,6 +2035,22 @@ - (void)testControllerServer
     [controllerServer shutdown];
 }
 
+- (void)testSetMRPParametersWithRunningController
+{
+    NSError * error;
+    __auto_type * storageDelegate = [[MTRTestPerControllerStorage alloc] initWithControllerID:[NSUUID UUID]];
+    MTRDeviceController * controller = [self startControllerWithRootKeys:[[MTRTestKeys alloc] init]
+                                                         operationalKeys:[[MTRTestKeys alloc] init]
+                                                                fabricID:@555
+                                                                  nodeID:@888
+                                                                 storage:storageDelegate
+                                                                   error:&error];
+    XCTAssertNotNil(controller);
+    XCTAssertTrue(controller.running);
+    MTRSetMessageReliabilityParameters(@2000, @2000, @2000, @2000);
+    [controller shutdown];
+}
+
 static NSString * const kLocalTestUserDefaultDomain = @"org.csa-iot.matter.darwintest";
 static NSString * const kLocalTestUserDefaultSubscriptionPoolSizeOverrideKey = @"subscriptionPoolSizeOverride";
 

diff --git a/src/lib/core/CHIPError.h b/src/lib/core/CHIPError.h
@@ -132,7 +132,7 @@ class ChipError
      *  The result is valid only if CanEncapsulate() is true.
      */
     constexpr ChipError(Range range, ValueType value) : ChipError(range, value, /*file=*/nullptr, /*line=*/0) {}
-#if __cplusplus >= 202002L
+#if CHIP_CONFIG_ERROR_SOURCE && __cplusplus >= 202002L
     constexpr ChipError(Range range, ValueType value, const char * file, unsigned int line,
                         std::source_location location = std::source_location::current()) :
         mError(MakeInteger(range, (value & MakeMask(0, kValueLength)))) CHIP_INITIALIZE_ERROR_SOURCE(file, line, location)
@@ -141,7 +141,7 @@ class ChipError
     constexpr ChipError(Range range, ValueType value, const char * file, unsigned int line) :
         mError(MakeInteger(range, (value & MakeMask(0, kValueLength)))) CHIP_INITIALIZE_ERROR_SOURCE(file, line, /*loc=*/nullptr)
     {}
-#endif // __cplusplus >= 202002L
+#endif // CHIP_CONFIG_ERROR_SOURCE && __cplusplus >= 202002L
 
     /**
      * Construct a CHIP_ERROR for SdkPart @a part with @a code.
@@ -150,7 +150,7 @@ class ChipError
      *  The macro version CHIP_SDK_ERROR checks that the numeric value is constant and well-formed.
      */
     constexpr ChipError(SdkPart part, uint8_t code) : ChipError(part, code, /*file=*/nullptr, /*line=*/0) {}
-#if __cplusplus >= 202002L
+#if CHIP_CONFIG_ERROR_SOURCE && __cplusplus >= 202002L
     constexpr ChipError(SdkPart part, uint8_t code, const char * file, unsigned int line,
                         std::source_location location = std::source_location::current()) :
         mError(MakeInteger(part, code)) CHIP_INITIALIZE_ERROR_SOURCE(file, line, location)
@@ -159,7 +159,7 @@ class ChipError
     constexpr ChipError(SdkPart part, uint8_t code, const char * file, unsigned int line) :
         mError(MakeInteger(part, code)) CHIP_INITIALIZE_ERROR_SOURCE(file, line, /*loc=*/nullptr)
     {}
-#endif // __cplusplus >= 202002L
+#endif // CHIP_CONFIG_ERROR_SOURCE && __cplusplus >= 202002L
 
     /**
      * Construct a CHIP_ERROR constant for SdkPart @a part with @a code at the current source line.
@@ -180,7 +180,7 @@ class ChipError
      *  This is intended to be used only in foreign function interfaces.
      */
     explicit constexpr ChipError(StorageType error) : ChipError(error, /*file=*/nullptr, /*line=*/0) {}
-#if __cplusplus >= 202002L
+#if CHIP_CONFIG_ERROR_SOURCE && __cplusplus >= 202002L
     explicit constexpr ChipError(StorageType error, const char * file, unsigned int line,
                                  std::source_location location = std::source_location::current()) :
         mError(error) CHIP_INITIALIZE_ERROR_SOURCE(file, line, location)
@@ -189,7 +189,7 @@ class ChipError
     explicit constexpr ChipError(StorageType error, const char * file, unsigned int line) :
         mError(error) CHIP_INITIALIZE_ERROR_SOURCE(file, line, /*loc=*/nullptr)
     {}
-#endif // __cplusplus >= 202002L
+#endif // CHIP_CONFIG_ERROR_SOURCE && __cplusplus >= 202002L
 
 #undef CHIP_INITIALIZE_ERROR_SOURCE
 

diff --git a/src/messaging/tests/BUILD.gn b/src/messaging/tests/BUILD.gn
@@ -14,7 +14,6 @@
 
 import("//build_overrides/build.gni")
 import("//build_overrides/chip.gni")
-import("//build_overrides/nlunit_test.gni")
 
 import("${chip_root}/build/chip/chip_test_suite.gni")
 import("${chip_root}/src/app/icd/icd.gni")
@@ -38,15 +37,9 @@ static_library("helpers") {
     "${chip_root}/src/transport",
     "${chip_root}/src/transport/tests:helpers",
   ]
-
-  # MessagingContext exposes nl-test compatible setup/teardown functions, specifically
-  # they return nltest specific SUCCESS/FAILURE constants hence this dependency.
-  #
-  # Once all tests are moved to pw_unittest/gtest, this dependency should be removed
-  public_deps = [ "${nlunit_test_root}:nlunit-test" ]
 }
 
-chip_test_suite_using_nltest("tests") {
+chip_test_suite("tests") {
   output_name = "libMessagingLayerTests"
 
   test_sources = [
@@ -73,12 +66,10 @@ chip_test_suite_using_nltest("tests") {
     "${chip_root}/src/lib/core",
     "${chip_root}/src/lib/support",
     "${chip_root}/src/lib/support:test_utils",
-    "${chip_root}/src/lib/support:testing_nlunit",
     "${chip_root}/src/messaging",
     "${chip_root}/src/protocols",
     "${chip_root}/src/transport",
     "${chip_root}/src/transport/raw/tests:helpers",
-    "${nlunit_test_root}:nlunit-test",
   ]
 
   if (chip_enable_icd_server) {

diff --git a/src/messaging/tests/MessagingContext.h b/src/messaging/tests/MessagingContext.h
@@ -32,8 +32,6 @@
 #include <transport/tests/LoopbackTransportManager.h>
 #include <transport/tests/UDPTransportManager.h>
 
-#include <nlunit-test.h>
-
 #include <vector>
 
 namespace chip {
@@ -98,6 +96,8 @@ class MessagingContext : public PlatformMemoryUser
         mInitialized(false), mAliceAddress(Transport::PeerAddress::UDP(GetAddress(), CHIP_PORT + 1)),
         mBobAddress(Transport::PeerAddress::UDP(GetAddress(), CHIP_PORT))
     {}
+    // TODO Replace VerifyOrDie with Pigweed assert after transition app/tests to Pigweed.
+    // TODO Currently src/app/icd/server/tests is using MessagingConetext as dependency.
     ~MessagingContext() { VerifyOrDie(mInitialized == false); }
 
     // Whether Alice and Bob are initialized, must be called before Init