Merge branch 'master' into bugfix/fixing_code_bugs

.github/workflows/minimal-build.yaml

@@ -17,7 +17,7 @@ name: Minimal Build (Linux / configure)
 on:
     push:
         branches-ignore:
-            - 'dependabot/**'
+            - "dependabot/**"
     pull_request:
     merge_group:
 
@@ -42,11 +42,11 @@ jobs:
             - name: Checkout submodules # but don't bootstrap!
               uses: ./.github/actions/checkout-submodules
               with:
-                platform: linux
+                  platform: linux
 
             - name: Configure and build All Clusters App
               run: |
-                  CC=gcc CXX=g++ scripts/configure --project=examples/all-clusters-app/linux && ./ninja-build
+                  CC=gcc CXX=g++ scripts/configure --project=examples/all-clusters-app/linux --enable-recommended=no && ./ninja-build
 
     minimal-network-manager:
         name: Linux / configure build of network-manager-app
@@ -64,8 +64,8 @@ jobs:
             - name: Checkout submodules # but don't bootstrap!
               uses: ./.github/actions/checkout-submodules
               with:
-                platform: linux
+                  platform: linux
 
             - name: Configure and build Network Manager App
               run: |
-                  CC=gcc CXX=g++ scripts/configure --project=examples/network-manager-app/linux && ./ninja-build
+                  CC=gcc CXX=g++ scripts/configure --project=examples/network-manager-app/linux --enable-recommended=no && ./ninja-build

config/recommended.gni

@@ -0,0 +1,31 @@
+# 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.
+
+declare_args() {
+  # Note for SDK developers:  As additional features with their own settings
+  # are added to the SDK, consider using the `matter_enable_recommended`
+  # meta-setting instead of a default value of 'true', especially where a
+  # different default is used based on platform (current_os): Often various
+  # debugging features have previously been defaulted to on for Linux and/or
+  # Mac but off for embedded platforms (on the assumption that Linux / Mac
+  # don't have resource constraints?); build settings of that nature should
+  # instead reference this meta-setting. E.g.
+  #   enable_flux_capacitor =  matter_enable_recommended && current_os == "linux"
+
+  # Enable recommended settings by default. This is a meta-setting
+  # that is enabled by default, and acts as a default for various
+  # other settings. Setting it to false produces a more conservative /
+  # minimal set of defaults.
+  matter_enable_recommended = true
+}

examples/shell/shell_common/BUILD.gn

@@ -13,13 +13,12 @@
 # limitations under the License.
 
 import("//build_overrides/chip.gni")
-import("//build_overrides/openthread.gni")
-
+import("${chip_root}/config/recommended.gni")
 import("${chip_root}/src/platform/device.gni")
 
 declare_args() {
-  # Enable server command only on linux for now.
-  chip_shell_cmd_server = current_os == "linux" || current_os == "mac"
+  chip_shell_cmd_server = matter_enable_recommended &&
+                          (current_os == "linux" || current_os == "mac")
 }
 
 config("shell_common_config") {

src/app/common_flags.gni

@@ -12,6 +12,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import("//build_overrides/chip.gni")
+import("${chip_root}/config/recommended.gni")
+
 declare_args() {
   # Temporary flag for interaction model and echo protocols, set it to true to enable
   chip_app_use_echo = false
@@ -26,7 +29,7 @@ declare_args() {
   #   - disabled: does not use data model interface at all
   #   - check:    runs BOTH datamodel and non-data-model (if possible) functionality and compares results
   #   - enabled:  runs only the data model interface (does not use the legacy code)
-  if (current_os == "linux") {
+  if (matter_enable_recommended && current_os == "linux") {
     chip_use_data_model_interface = "check"
   } else {
     chip_use_data_model_interface = "disabled"

src/lib/core/core.gni

@@ -12,6 +12,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import("//build_overrides/chip.gni")
+import("${chip_root}/config/recommended.gni")
+
 declare_args() {
   # Enable logging. Shorthand for chip_error_logging, etc.
   chip_logging = true
@@ -28,13 +31,16 @@ declare_args() {
   chip_progress_logging = chip_logging
 
   # Enable detail logging.
-  chip_detail_logging = chip_logging
+  chip_detail_logging = matter_enable_recommended && chip_logging
 
   # Enable automation logging.
-  chip_automation_logging = chip_logging
+  chip_automation_logging = matter_enable_recommended && chip_logging
+}
 
+declare_args() {
   # Configure the maximum size for logged messages
-  if (current_os == "linux" || current_os == "mac" || current_os == "ios") {
+  if ((matter_enable_recommended || chip_detail_logging) &&
+      (current_os == "linux" || current_os == "mac" || current_os == "ios")) {
     # Allow base64 encoding of 1 MTU (4 * (1280 / 3) + padding
     chip_log_message_max_size = 1708
   } else {
@@ -88,7 +94,8 @@ declare_args() {
   chip_config_memory_debug_dmalloc = false
 
   # When enabled trace messages using tansport trace hook.
-  chip_enable_transport_trace = current_os == "linux" || current_os == "mac"
+  chip_enable_transport_trace = matter_enable_recommended &&
+                                (current_os == "linux" || current_os == "mac")
 
   # When this is enabled trace messages will be sent to pw_trace.
   chip_enable_transport_pw_trace = false

src/python_testing/TC_BRBINFO_4_1.py

@@ -123,6 +123,7 @@ async def setup_class(self):
         self.set_of_dut_endpoints_before_adding_device = set(root_part_list)
 
         super().setup_class()
+        self._active_change_event_subscription = None
         self.app_process = None
         self.app_process_paused = False
         app = self.user_params.get("th_icd_server_app_path", None)
@@ -156,6 +157,10 @@ async def setup_class(self):
                                                          params.commissioningParameters.setupManualCode, params.commissioningParameters.setupQRCode)
 
     def teardown_class(self):
+        if self._active_change_event_subscription is not None:
+            self._active_change_event_subscription.Shutdown()
+            self._active_change_event_subscription = None
+
         # In case the th_icd_server_app_path does not exist, then we failed the test
         # and there is nothing to remove
         if self.app_process is not None:
@@ -239,8 +244,8 @@ async def test_TC_BRBINFO_4_1(self):
         self.q = queue.Queue()
         urgent = 1
         cb = SimpleEventCallback("ActiveChanged", event.cluster_id, event.event_id, self.q)
-        subscription = await self.default_controller.ReadEvent(nodeid=self.dut_node_id, events=[(dynamic_endpoint_id, event, urgent)], reportInterval=[1, 3])
-        subscription.SetEventUpdateCallback(callback=cb)
+        self._active_change_event_subscription = await self.default_controller.ReadEvent(nodeid=self.dut_node_id, events=[(dynamic_endpoint_id, event, urgent)], reportInterval=[1, 3])
+        self._active_change_event_subscription.SetEventUpdateCallback(callback=cb)
 
         self.step("3")
         stay_active_duration_ms = 1000

src/python_testing/TC_MCORE_FS_1_2.py

@@ -63,10 +63,15 @@ class TC_MCORE_FS_1_2(MatterBaseTest):
     @async_test_body
     async def setup_class(self):
         super().setup_class()
+        self._partslist_subscription = None
         self._app_th_server_process = None
         self._th_server_kvs = None
 
     def teardown_class(self):
+        if self._partslist_subscription is not None:
+            self._partslist_subscription.Shutdown()
+            self._partslist_subscription = None
+
         if self._app_th_server_process is not None:
             logging.warning("Stopping app with SIGTERM")
             self._app_th_server_process.send_signal(signal.SIGTERM.value)
@@ -142,7 +147,7 @@ async def test_TC_MCORE_FS_1_2(self):
         subscription_contents = [
             (root_endpoint, Clusters.Descriptor.Attributes.PartsList)
         ]
-        sub = await self.default_controller.ReadAttribute(
+        self._partslist_subscription = await self.default_controller.ReadAttribute(
             nodeid=self.dut_node_id,
             attributes=subscription_contents,
             reportInterval=(min_report_interval_sec, max_report_interval_sec),
@@ -152,8 +157,8 @@ async def test_TC_MCORE_FS_1_2(self):
         parts_list_queue = queue.Queue()
         attribute_handler = AttributeChangeAccumulator(
             name=self.default_controller.name, expected_attribute=Clusters.Descriptor.Attributes.PartsList, output=parts_list_queue)
-        sub.SetAttributeUpdateCallback(attribute_handler)
-        cached_attributes = sub.GetAttributes()
+        self._partslist_subscription.SetAttributeUpdateCallback(attribute_handler)
+        cached_attributes = self._partslist_subscription.GetAttributes()
         step_1_dut_parts_list = cached_attributes[root_endpoint][Clusters.Descriptor][Clusters.Descriptor.Attributes.PartsList]
 
         asserts.assert_true(type_matches(step_1_dut_parts_list, list), "PartsList is expected to be a list")

src/python_testing/TC_MCORE_FS_1_5.py

@@ -61,10 +61,20 @@ class TC_MCORE_FS_1_5(MatterBaseTest):
     @async_test_body
     async def setup_class(self):
         super().setup_class()
+        self._partslist_subscription = None
+        self._cadmin_subscription = None
         self._app_th_server_process = None
         self._th_server_kvs = None
 
     def teardown_class(self):
+        if self._partslist_subscription is not None:
+            self._partslist_subscription.Shutdown()
+            self._partslist_subscription = None
+
+        if self._cadmin_subscription is not None:
+            self._cadmin_subscription.Shutdown()
+            self._cadmin_subscription = None
+
         if self._app_th_server_process is not None:
             logging.warning("Stopping app with SIGTERM")
             self._app_th_server_process.send_signal(signal.SIGTERM.value)
@@ -142,7 +152,7 @@ async def test_TC_MCORE_FS_1_5(self):
         parts_list_subscription_contents = [
             (root_endpoint, Clusters.Descriptor.Attributes.PartsList)
         ]
-        parts_list_sub = await self.default_controller.ReadAttribute(
+        self._partslist_subscription = await self.default_controller.ReadAttribute(
             nodeid=self.dut_node_id,
             attributes=parts_list_subscription_contents,
             reportInterval=(min_report_interval_sec, max_report_interval_sec),
@@ -152,8 +162,8 @@ async def test_TC_MCORE_FS_1_5(self):
         parts_list_queue = queue.Queue()
         parts_list_attribute_handler = AttributeChangeAccumulator(
             name=self.default_controller.name, expected_attribute=Clusters.Descriptor.Attributes.PartsList, output=parts_list_queue)
-        parts_list_sub.SetAttributeUpdateCallback(parts_list_attribute_handler)
-        parts_list_cached_attributes = parts_list_sub.GetAttributes()
+        self._partslist_subscription.SetAttributeUpdateCallback(parts_list_attribute_handler)
+        parts_list_cached_attributes = self._partslist_subscription.GetAttributes()
         step_1_dut_parts_list = parts_list_cached_attributes[root_endpoint][Clusters.Descriptor][Clusters.Descriptor.Attributes.PartsList]
 
         asserts.assert_true(type_matches(step_1_dut_parts_list, list), "PartsList is expected to be a list")
@@ -219,7 +229,7 @@ async def test_TC_MCORE_FS_1_5(self):
         cadmin_subscription_contents = [
             (newly_added_endpoint, Clusters.AdministratorCommissioning)
         ]
-        cadmin_sub = await self.default_controller.ReadAttribute(
+        self._cadmin_subscription = await self.default_controller.ReadAttribute(
             nodeid=self.dut_node_id,
             attributes=cadmin_subscription_contents,
             reportInterval=(min_report_interval_sec, max_report_interval_sec),
@@ -230,7 +240,7 @@ async def test_TC_MCORE_FS_1_5(self):
         # This AttributeChangeAccumulator is really just to let us know when new subscription came in
         cadmin_attribute_handler = AttributeChangeAccumulator(
             name=self.default_controller.name, expected_attribute=Clusters.AdministratorCommissioning.Attributes.WindowStatus, output=cadmin_queue)
-        cadmin_sub.SetAttributeUpdateCallback(cadmin_attribute_handler)
+        self._cadmin_subscription.SetAttributeUpdateCallback(cadmin_attribute_handler)
         time.sleep(1)
 
         self.step(7)

src/python_testing/TC_SEAR_1_2.py

@@ -180,12 +180,12 @@ async def read_and_validate_progress(self, step):
                 progareaid_list.append(p.areaID)
                 asserts.assert_true(p.areaID in self.areaid_list,
                                     f"Progress entry has invalid AreaID value ({p.areaID})")
-                asserts.assert_true(p.status in (Clusters.ServiceArea.OperationalStatusEnum.kPending,
-                                                 Clusters.ServiceArea.OperationalStatusEnum.kOperating,
-                                                 Clusters.ServiceArea.OperationalStatusEnum.kSkipped,
-                                                 Clusters.ServiceArea.OperationalStatusEnum.kCompleted),
+                asserts.assert_true(p.status in (Clusters.ServiceArea.Enums.OperationalStatusEnum.kPending,
+                                                 Clusters.ServiceArea.Enums.OperationalStatusEnum.kOperating,
+                                                 Clusters.ServiceArea.Enums.OperationalStatusEnum.kSkipped,
+                                                 Clusters.ServiceArea.Enums.OperationalStatusEnum.kCompleted),
                                     f"Progress entry has invalid Status value ({p.status})")
-                if p.status not in (Clusters.ServiceArea.OperationalStatusEnum.kSkipped, Clusters.ServiceArea.OperationalStatusEnum.kCompleted):
+                if p.status not in (Clusters.ServiceArea.Enums.OperationalStatusEnum.kSkipped, Clusters.ServiceArea.Enums.OperationalStatusEnum.kCompleted):
                     asserts.assert_true(p.totalOperationalTime is NullValue,
                                         f"Progress entry should have a null TotalOperationalTime value (Status is {p.status})")
                 # TODO how to check that InitialTimeEstimate is either null or uint32?

src/tracing/tracing_args.gni

@@ -11,9 +11,8 @@
 # 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_root}/config/compiler/compiler.gni")
+import("${chip_root}/config/recommended.gni")
 import("${chip_root}/src/platform/device.gni")
 
 declare_args() {
@@ -24,7 +23,8 @@ declare_args() {
   # Additionally, if tracing is enabled, the main() function has to add
   # backends explicitly
   matter_enable_tracing_support =
-      current_os == "android" || chip_device_platform == "darwin"
+      matter_enable_recommended &&
+      (current_os == "android" || chip_device_platform == "darwin")
 
   # Defines the trace backend. Current matter tracing splits the logic
   # into two parts: