diff --git a/.github/workflows/minimal-build.yaml b/.github/workflows/minimal-build.yaml index 9ba9565b44c25b..ade1bd56ea36ff 100644 --- a/.github/workflows/minimal-build.yaml +++ b/.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 diff --git a/config/recommended.gni b/config/recommended.gni new file mode 100644 index 00000000000000..a30045ced7814e --- /dev/null +++ b/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 +} diff --git a/examples/shell/shell_common/BUILD.gn b/examples/shell/shell_common/BUILD.gn index e4aa2973f8cc04..645c8ddb510220 100644 --- a/examples/shell/shell_common/BUILD.gn +++ b/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") { diff --git a/src/app/common_flags.gni b/src/app/common_flags.gni index 953afc23a20650..d3e7ce34bf0338 100644 --- a/src/app/common_flags.gni +++ b/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" diff --git a/src/lib/core/core.gni b/src/lib/core/core.gni index f2189198e36131..eea73a5c56f7b3 100644 --- a/src/lib/core/core.gni +++ b/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 diff --git a/src/python_testing/TC_BRBINFO_4_1.py b/src/python_testing/TC_BRBINFO_4_1.py index df922748999dd0..8b064a775f358b 100644 --- a/src/python_testing/TC_BRBINFO_4_1.py +++ b/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 diff --git a/src/python_testing/TC_MCORE_FS_1_2.py b/src/python_testing/TC_MCORE_FS_1_2.py index 816abe5d876b00..b18dcc5b42ef64 100644 --- a/src/python_testing/TC_MCORE_FS_1_2.py +++ b/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") diff --git a/src/python_testing/TC_MCORE_FS_1_5.py b/src/python_testing/TC_MCORE_FS_1_5.py index df80072d5126e3..22654c994bf41f 100755 --- a/src/python_testing/TC_MCORE_FS_1_5.py +++ b/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) diff --git a/src/python_testing/TC_SEAR_1_2.py b/src/python_testing/TC_SEAR_1_2.py index 2c253efa860038..1bed2c8d12cac8 100644 --- a/src/python_testing/TC_SEAR_1_2.py +++ b/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? diff --git a/src/tracing/tracing_args.gni b/src/tracing/tracing_args.gni index d6ddb1fd2e99b8..085844e4a53605 100644 --- a/src/tracing/tracing_args.gni +++ b/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: