From 506d973496e92cd4e57faa39d6a40708954098ad Mon Sep 17 00:00:00 2001 From: Tennessee Carmel-Veilleux Date: Wed, 28 Aug 2024 17:27:48 -0400 Subject: [PATCH 01/13] Testing fixes for TC_SWTCH from TE2 (#34984) * Testing fixes for TC_SWTCH from TE2 - all-clusters-app was not generating button position changes in some cases. This was not detected in some situations since the test cases don't always test for this. - Prompts are missing endpoint ID which makes it hard when running per-endpoint tests to know where it applies. - Some partials could fail on decode errors, causing test errors instead of fails. This PR: - Adds correct generation of positions on press/release. - Adds a way to claim endpoint tested in user prompts - Fixes failing on decode errors in partials Testing done: - TC_SWTCH still passes - Manually validated button position in multi-press test/simulation (update to TC_SWTCH needs test plan changes). Issue is in all-clusters-app for CI only. See https://github.com/CHIP-Specifications/chip-test-plans/issues/4493 * Restyled by autopep8 * Update prompt support --------- Co-authored-by: Restyled.io --- .../linux/ButtonEventsSimulator.cpp | 2 ++ .../matter_yamltests/hooks.py | 4 ++- src/python_testing/TC_SWTCH.py | 12 ++++++--- src/python_testing/matter_testing_support.py | 25 ++++++++++++++++--- 4 files changed, 34 insertions(+), 9 deletions(-) diff --git a/examples/all-clusters-app/linux/ButtonEventsSimulator.cpp b/examples/all-clusters-app/linux/ButtonEventsSimulator.cpp index 53a08672fdbc07..0122eba4336340 100644 --- a/examples/all-clusters-app/linux/ButtonEventsSimulator.cpp +++ b/examples/all-clusters-app/linux/ButtonEventsSimulator.cpp @@ -235,6 +235,7 @@ void ButtonEventsSimulator::Next() break; } case ButtonEventsSimulator::State::kEmitStartOfMultiPress: { + SetButtonPosition(mEndpointId, mPressedButtonId); EmitInitialPress(mEndpointId, mPressedButtonId); if (mFeatureMap & static_cast(Clusters::Switch::Feature::kActionSwitch)) { @@ -268,6 +269,7 @@ void ButtonEventsSimulator::Next() { EmitShortRelease(mEndpointId, mPressedButtonId); } + SetButtonPosition(mEndpointId, mIdleButtonId); StartTimer(mMultiPressReleasedTimeMillis); break; } diff --git a/scripts/py_matter_yamltests/matter_yamltests/hooks.py b/scripts/py_matter_yamltests/matter_yamltests/hooks.py index 78905826f55757..ca739b8ea27633 100644 --- a/scripts/py_matter_yamltests/matter_yamltests/hooks.py +++ b/scripts/py_matter_yamltests/matter_yamltests/hooks.py @@ -221,7 +221,9 @@ async def step_manual(self): def show_prompt(self, msg: str, placeholder: Optional[str] = None, - default_value: Optional[str] = None) -> None: + default_value: Optional[str] = None, + endpoint_id: Optional[int] = None, + ) -> None: """ This method is called when the step needs to ask the user to perform some action or provide some value. """ diff --git a/src/python_testing/TC_SWTCH.py b/src/python_testing/TC_SWTCH.py index d6c4694560b16d..8f9a694db7c496 100644 --- a/src/python_testing/TC_SWTCH.py +++ b/src/python_testing/TC_SWTCH.py @@ -44,6 +44,8 @@ logger = logging.getLogger(__name__) +SIMULATED_LONG_PRESS_LENGTH_SECONDS = 2.0 + def bump_substep(step: str) -> str: """Given a string like "5a", bump it to "5b", or "6c" to "6d" """ @@ -94,7 +96,8 @@ def _send_multi_press_named_pipe_command(self, endpoint_id: int, number_of_press def _send_long_press_named_pipe_command(self, endpoint_id: int, pressed_position: int, feature_map: int): command_dict = {"Name": "SimulateLongPress", "EndpointId": endpoint_id, - "ButtonId": pressed_position, "LongPressDelayMillis": 5000, "LongPressDurationMillis": 5500, "FeatureMap": feature_map} + "ButtonId": pressed_position, "LongPressDelayMillis": int(1000 * (SIMULATED_LONG_PRESS_LENGTH_SECONDS - 0.5)), + "LongPressDurationMillis": int(1000 * SIMULATED_LONG_PRESS_LENGTH_SECONDS), "FeatureMap": feature_map} self._send_named_pipe_command(command_dict) def _send_latching_switch_named_pipe_command(self, endpoint_id: int, new_position: int): @@ -168,7 +171,9 @@ def _ask_for_release(self): prompt_msg="Release the button." ) else: - time.sleep(self.keep_pressed_delay/1000) + # This will await for the events to be generated properly. Note that there is a bit of a + # race here for the button simulator, but this race is extremely unlikely to be lost. + time.sleep(SIMULATED_LONG_PRESS_LENGTH_SECONDS + 0.5) def _await_sequence_of_events(self, event_queue: queue.Queue, endpoint_id: int, sequence: list[ClusterObjects.ClusterEvent], timeout_sec: float): start_time = time.time() @@ -373,7 +378,6 @@ async def test_TC_SWTCH_2_3(self): self.step(5) # We're using a long press here with a very long duration (in computer-land). This will let us check the intermediate values. # This is 1s larger than the subscription ceiling - self.keep_pressed_delay = 6000 self.pressed_position = 1 self._ask_for_keep_pressed(endpoint_id, self.pressed_position, feature_map) event_listener.wait_for_event_report(cluster.Events.InitialPress) @@ -595,7 +599,7 @@ def steps_TC_SWTCH_2_5(self): @staticmethod def should_run_SWTCH_2_5(wildcard, endpoint): msm = has_feature(Clusters.Switch, Clusters.Switch.Bitmaps.Feature.kMomentarySwitchMultiPress) - asf = has_feature(Clusters.Switch, 0x20) + asf = has_feature(Clusters.Switch, Clusters.Switch.Bitmaps.Feature.kActionSwitch) return msm(wildcard, endpoint) and not asf(wildcard, endpoint) @per_endpoint_test(should_run_SWTCH_2_5) diff --git a/src/python_testing/matter_testing_support.py b/src/python_testing/matter_testing_support.py index 4127ead64c3083..20c33a0d36b785 100644 --- a/src/python_testing/matter_testing_support.py +++ b/src/python_testing/matter_testing_support.py @@ -1380,11 +1380,21 @@ def wait_for_user_input(self, Returns: str: User input or none if input is closed. """ + + # TODO(#31928): Remove any assumptions of test params for endpoint ID. + + # Get the endpoint user param instead of `--endpoint-id` result, if available, temporarily. + endpoint_id = self.user_params.get("endpoint", None) + if endpoint_id is None or not isinstance(endpoint_id, int): + endpoint_id = self.matter_test_config.endpoint + if self.runner_hook: + # TODO(#31928): Add endpoint support to hooks. self.runner_hook.show_prompt(msg=prompt_msg, placeholder=prompt_msg_placeholder, default_value=default_value) - logging.info("========= USER PROMPT =========") + + logging.info(f"========= USER PROMPT for Endpoint {endpoint_id} =========") logging.info(f">>> {prompt_msg.rstrip()} (press enter to confirm)") try: return input() @@ -1881,6 +1891,9 @@ def _has_attribute(wildcard, endpoint, attribute: ClusterObjects.ClusterAttribut cluster = get_cluster_from_attribute(attribute) try: attr_list = wildcard.attributes[endpoint][cluster][cluster.Attributes.AttributeList] + if not isinstance(attr_list, list): + asserts.fail( + f"Failed to read mandatory AttributeList attribute value for cluster {cluster} on endpoint {endpoint}: {attr_list}.") return attribute.attribute_id in attr_list except KeyError: return False @@ -1910,9 +1923,13 @@ def has_attribute(attribute: ClusterObjects.ClusterAttributeDescriptor) -> Endpo return partial(_has_attribute, attribute=attribute) -def _has_feature(wildcard, endpoint, cluster: ClusterObjects.ClusterObjectDescriptor, feature: IntFlag) -> bool: +def _has_feature(wildcard, endpoint: int, cluster: ClusterObjects.ClusterObjectDescriptor, feature: IntFlag) -> bool: try: feature_map = wildcard.attributes[endpoint][cluster][cluster.Attributes.FeatureMap] + if not isinstance(feature_map, int): + asserts.fail( + f"Failed to read mandatory FeatureMap attribute value for cluster {cluster} on endpoint {endpoint}: {feature_map}.") + return (feature & feature_map) != 0 except KeyError: return False @@ -1926,8 +1943,8 @@ def has_feature(cluster: ClusterObjects.ClusterObjectDescriptor, feature: IntFla EP0: cluster A, B, C EP1: cluster D with feature F0 - EP2, cluster D with feature F0 - EP3, cluster D without feature F0 + EP2: cluster D with feature F0 + EP3: cluster D without feature F0 And the following test specification: @per_endpoint_test(has_feature(Clusters.D.Bitmaps.Feature.F0)) From 44c84ac42b3db818fbaf2ab996731390a7f2ead1 Mon Sep 17 00:00:00 2001 From: Nivi Sarkar <55898241+nivi-apple@users.noreply.github.com> Date: Wed, 28 Aug 2024 14:38:05 -0700 Subject: [PATCH 02/13] =?UTF-8?q?Add=20test=20cases=20for=20testing=20addi?= =?UTF-8?q?tional=20Presets=20write=20and=20commit=20constr=E2=80=A6=20(#3?= =?UTF-8?q?5141)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add test cases for testing additional Presets write and commit constraints - Add a test for adding a preset with a preset scenario not present in PresetTypes - Add a test for testing addition of presets such that the total number of presets added is greater than the total number of presets supported * Add rollback after test step 18 * Modify the number of presets supported test case to read the number of presets supported and build a preset list whose size exceeds that to test * Modify the number of presets supported test case to read the number of presets supported and build a preset list whose size exceeds that to test * Update thermostat-delegate-impl.h * Address review comments * Add support to check for numberOfPresets supported for each preset type and build the presets list with multiple presets of each type * Restyled by autopep8 * Fix log line formatting * Update src/python_testing/TC_TSTAT_4_2.py Co-authored-by: Boris Zbarsky * Fix test step 17 to find a preset scenario in PresetScenarioEnum that is not present in PresetTypes to run the test - Fix test step 18 to build a presets list that exceeds the number of presets supported correctly * Restyled by autopep8 * Fix lint errors * Add a while loop to add more presets until max is reached --------- Co-authored-by: Restyled.io Co-authored-by: Boris Zbarsky --- .../include/thermostat-delegate-impl.h | 4 + .../src/thermostat-delegate-impl.cpp | 5 +- src/python_testing/TC_TSTAT_4_2.py | 85 ++++++++++++++++++- 3 files changed, 92 insertions(+), 2 deletions(-) diff --git a/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h b/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h index b57ee2492122e7..7726fc337866bb 100644 --- a/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h +++ b/examples/thermostat/thermostat-common/include/thermostat-delegate-impl.h @@ -39,6 +39,10 @@ static constexpr uint8_t kMaxNumberOfPresetTypes = 6; // We will support only one preset of each preset type. static constexpr uint8_t kMaxNumberOfPresetsOfEachType = 1; +// For testing the use case where number of presets added exceeds the number of presets supported, we will have the value of +// kMaxNumberOfPresetsSupported < kMaxNumberOfPresetTypes * kMaxNumberOfPresetsOfEachType +static constexpr uint8_t kMaxNumberOfPresetsSupported = kMaxNumberOfPresetTypes * kMaxNumberOfPresetsOfEachType - 1; + class ThermostatDelegate : public Delegate { public: diff --git a/examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp b/examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp index 7991e48323a62f..da58c840049275 100644 --- a/examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp +++ b/examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp @@ -31,7 +31,7 @@ ThermostatDelegate ThermostatDelegate::sInstance; ThermostatDelegate::ThermostatDelegate() { - mNumberOfPresets = kMaxNumberOfPresetTypes * kMaxNumberOfPresetsOfEachType; + mNumberOfPresets = kMaxNumberOfPresetsSupported; mNextFreeIndexInPresetsList = 0; mNextFreeIndexInPendingPresetsList = 0; @@ -87,6 +87,9 @@ CHIP_ERROR ThermostatDelegate::GetPresetTypeAtIndex(size_t index, PresetTypeStru { .presetScenario = PresetScenarioEnum::kVacation, .numberOfPresets = kMaxNumberOfPresetsOfEachType, .presetTypeFeatures = to_underlying(PresetTypeFeaturesBitmap::kSupportsNames) }, + { .presetScenario = PresetScenarioEnum::kUserDefined, + .numberOfPresets = kMaxNumberOfPresetsOfEachType, + .presetTypeFeatures = to_underlying(PresetTypeFeaturesBitmap::kSupportsNames) }, }; if (index < ArraySize(presetTypes)) { diff --git a/src/python_testing/TC_TSTAT_4_2.py b/src/python_testing/TC_TSTAT_4_2.py index b133b1be2c8f22..641c827b388882 100644 --- a/src/python_testing/TC_TSTAT_4_2.py +++ b/src/python_testing/TC_TSTAT_4_2.py @@ -210,6 +210,10 @@ def steps_TC_TSTAT_4_2(self) -> list[TestStep]: "Verify that the write request is rejected"), TestStep("16", "TH starts an atomic write, and before it's complete, TH2 removes TH's fabric; TH2 then opens an atomic write", "Verify that the atomic request is successful"), + TestStep("17", "TH writes to the Presets attribute with a preset that has a presetScenario not present in PresetTypes attribute", + "Verify that the write request returned CONSTRAINT_ERROR (0x87)."), + TestStep("18", "TH writes to the Presets attribute such that the total number of presets is greater than the number of presets supported", + "Verify that the write request returned RESOURCE_EXHAUSTED (0x89)."), ] return steps @@ -469,7 +473,86 @@ async def test_TC_TSTAT_4_2(self): # Roll back await self.send_atomic_request_rollback_command() - # TODO: Add tests for the total number of Presets exceeds the NumberOfPresets supported. Also Add tests for adding presets with preset scenario not present in PresetTypes. + self.step("17") + if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): + + # Read the PresetTypes to get the preset scenarios supported by the Thermostat. + presetTypes = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.PresetTypes) + + scenarioNotPresent = None + + # Find a preset scenario not present in PresetTypes to run this test. + for scenario in cluster.Enums.PresetScenarioEnum: + foundMatchingScenario = False + for presetType in presetTypes: + if presetType.presetScenario == scenario: + foundMatchingScenario = True + break + if not foundMatchingScenario: + scenarioNotPresent = scenario + break + + if scenarioNotPresent is None: + logger.info( + "Couldn't run test step 17 since all preset types in PresetScenarioEnum are supported by this Thermostat") + else: + test_presets = new_presets_with_handle.copy() + test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=scenarioNotPresent, + name="Preset", coolingSetpoint=2500, heatingSetpoint=1700, builtIn=False)) + + # Send the AtomicRequest begin command + await self.send_atomic_request_begin_command() + + await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ConstraintError) + + # Clear state for next test. + await self.send_atomic_request_rollback_command() + + self.step("18") + if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.Cfe.Rsp")): + + # Read the numberOfPresets supported. + numberOfPresetsSupported = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.NumberOfPresets) + + # Read the PresetTypes to get the preset scenarios to build the Presets list. + presetTypes = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.PresetTypes) + + # Read the Presets to copy the existing presets into our testPresets list below. + presets = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.Presets) + + # Calculate the length of the Presets list that could be created using the preset scenarios in PresetTypes and numberOfPresets supported for each scenario. + totalExpectedPresetsLength = 0 + for presetType in presetTypes: + totalExpectedPresetsLength += presetType.numberOfPresets + + if totalExpectedPresetsLength > numberOfPresetsSupported: + testPresets = [] + for presetType in presetTypes: + scenario = presetType.presetScenario + + # For each supported scenario, copy all the existing presets that match it, then add more presets + # until we hit the cap on the number of presets for that scenario. + presetsAddedForScenario = 0 + for preset in presets: + if scenario == preset.presetScenario: + testPresets.append(preset) + presetsAddedForScenario = presetsAddedForScenario + 1 + + while presetsAddedForScenario < presetType.numberOfPresets: + testPresets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=scenario, + name="Preset", coolingSetpoint=2500, heatingSetpoint=1700, builtIn=False)) + presetsAddedForScenario = presetsAddedForScenario + 1 + + # Send the AtomicRequest begin command + await self.send_atomic_request_begin_command() + + await self.write_presets(endpoint=endpoint, presets=testPresets, expected_status=Status.ResourceExhausted) + + # Clear state for next test. + await self.send_atomic_request_rollback_command() + else: + logger.info( + "Couldn't run test step 18 since there are not enough preset types to build a Presets list that exceeds the number of presets supported") if __name__ == "__main__": From 89f740d8f7f9364ed6fbeeb6fb301dbce43500f0 Mon Sep 17 00:00:00 2001 From: Thomas Lea <35579828+tleacmcsa@users.noreply.github.com> Date: Wed, 28 Aug 2024 16:51:47 -0500 Subject: [PATCH 03/13] Allow TestAccessControl to run with ARL (#35262) * Allow TestAccessControl to run with ARL Since RequestType is required, set each test data entry to some value that will pass AccessRestrictionProvider checks (since the focus is on AccessControl for these tests). * Copy the test data's request path and optionally add RequestType --- src/access/tests/TestAccessControl.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/access/tests/TestAccessControl.cpp b/src/access/tests/TestAccessControl.cpp index 400ea04b76e535..00b95baf39164b 100644 --- a/src/access/tests/TestAccessControl.cpp +++ b/src/access/tests/TestAccessControl.cpp @@ -1752,7 +1752,11 @@ TEST_F(TestAccessControl, TestCheck) for (const auto & checkData : checkData1) { CHIP_ERROR expectedResult = checkData.allow ? CHIP_NO_ERROR : CHIP_ERROR_ACCESS_DENIED; - EXPECT_EQ(accessControl.Check(checkData.subjectDescriptor, checkData.requestPath, checkData.privilege), expectedResult); + auto requestPath = checkData.requestPath; +#if CHIP_CONFIG_USE_ACCESS_RESTRICTIONS + requestPath.requestType = Access::RequestType::kAttributeReadRequest; +#endif + EXPECT_EQ(accessControl.Check(checkData.subjectDescriptor, requestPath, checkData.privilege), expectedResult); } } From 7d24ea30ad1554c55e0aebdf34d9068ce51bb32b Mon Sep 17 00:00:00 2001 From: Karsten Sperling <113487422+ksperling-apple@users.noreply.github.com> Date: Thu, 29 Aug 2024 10:24:02 +1200 Subject: [PATCH 04/13] Make zap_downloadl.py create a usable zap.app on Mac (#35242) Use the unzip utility on Mac for unzipping instead of zipfile. In addition to not supporting file modes (which the script already works around) the zipfile module also doesn't support symlinks. The embedded frameworks inside zap.app rely on symlinks for the application to work. --- scripts/tools/zap/zap_download.py | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/scripts/tools/zap/zap_download.py b/scripts/tools/zap/zap_download.py index 72dea27048a0c6..9d1a553feac0e8 100755 --- a/scripts/tools/zap/zap_download.py +++ b/scripts/tools/zap/zap_download.py @@ -23,6 +23,7 @@ import shutil import subprocess import sys +import tempfile import zipfile from typing import Optional @@ -123,13 +124,22 @@ def _SetupReleaseZap(install_directory: str, zap_version: str): logging.info("Fetching: %s", url) r = requests.get(url, stream=True) - z = zipfile.ZipFile(io.BytesIO(r.content)) - - logging.info("Data downloaded, extracting ...") - # extractall() does not preserve permissions (https://github.com/python/cpython/issues/59999) - for entry in z.filelist: - path = z.extract(entry, install_directory) - os.chmod(path, (entry.external_attr >> 16) & 0o777) + if zap_platform == 'mac': + # zipfile does not support symlinks (https://github.com/python/cpython/issues/82102), + # making a zap.app extracted with it unusable due to embedded frameworks. + with tempfile.NamedTemporaryFile(suffix='.zip') as tf: + for chunk in r.iter_content(chunk_size=4096): + tf.write(chunk) + tf.flush() + os.makedirs(install_directory, exist_ok=True) + _ExecuteProcess(['/usr/bin/unzip', '-oq', tf.name], install_directory) + else: + z = zipfile.ZipFile(io.BytesIO(r.content)) + logging.info("Data downloaded, extracting ...") + # extractall() does not preserve permissions (https://github.com/python/cpython/issues/59999) + for entry in z.filelist: + path = z.extract(entry, install_directory) + os.chmod(path, (entry.external_attr >> 16) & 0o777) logging.info("Done extracting.") From 9c343164114e7cec3770612d9efbb28b9b66ea58 Mon Sep 17 00:00:00 2001 From: marchemi <56955785+marchemi@users.noreply.github.com> Date: Thu, 29 Aug 2024 01:04:02 +0200 Subject: [PATCH 05/13] TBRM Tests scripts consistency with te2 fixes (#35153) * Add files via upload Add yaml test script for TBRM * Update TEST_TC_TBRM_2.2.yaml * Update TEST_TC_TBRM_2.3.yaml * Update TEST_TC_TBRM_2.4.yaml * Test script consitancy wit test plan after TE2 * Test script consitancy wit test plan after TE2 * Update src/app/tests/suites/certification/Test_TC_TBRM_2_3.yaml Co-authored-by: Karsten Sperling <113487422+ksperling-apple@users.noreply.github.com> * Update src/app/tests/suites/certification/Test_TC_TBRM_2_2.yaml Co-authored-by: Karsten Sperling <113487422+ksperling-apple@users.noreply.github.com> * Restyled by whitespace * Restyled by prettier-yaml * Test_TC_TBRM_2_4. synchronisation with TC-THNETDIR-2.3 according test Plan * Restyled by whitespace * Test tweaks to get CI to pass - Use pairing payload matching the other parameters - Check response of ArmFailSafe commands - Fix bad merge in commissioner_commands.py * Restyled by prettier-yaml --------- Co-authored-by: StephaneGUELEC <94045077+StephaneGUELEC@users.noreply.github.com> Co-authored-by: Karsten Sperling <113487422+ksperling-apple@users.noreply.github.com> Co-authored-by: Restyled.io Co-authored-by: Karsten Sperling --- .../certification/Test_TC_TBRM_2_2.yaml | 22 ++++- .../certification/Test_TC_TBRM_2_3.yaml | 29 +++++-- .../certification/Test_TC_TBRM_2_4.yaml | 86 +++++++++++++++++++ .../Test_TC_TBRM_3_1_Simulated.yaml | 44 ++++++++++ 4 files changed, 170 insertions(+), 11 deletions(-) create mode 100644 src/app/tests/suites/certification/Test_TC_TBRM_2_4.yaml create mode 100644 src/app/tests/suites/certification/Test_TC_TBRM_3_1_Simulated.yaml diff --git a/src/app/tests/suites/certification/Test_TC_TBRM_2_2.yaml b/src/app/tests/suites/certification/Test_TC_TBRM_2_2.yaml index 852e18a73adc8a..a6b54026729c8f 100644 --- a/src/app/tests/suites/certification/Test_TC_TBRM_2_2.yaml +++ b/src/app/tests/suites/certification/Test_TC_TBRM_2_2.yaml @@ -41,19 +41,21 @@ tests: values: - name: nodeId value: nodeId - + # Step 1 - label: "TH reads the ActiveDatasetTimestamp attribute from the DUT" command: readAttribute attribute: ActiveDatasetTimestamp response: value: null + # Step 2 - label: "TH reads the PendingDatasetTimestamp attribute from the DUT" command: readAttribute attribute: PendingDatasetTimestamp response: value: null + # Step 3 - label: "TH sends a valid ActiveDatasetRequest command to the DUT without having armed the fail-safe" @@ -65,6 +67,7 @@ tests: response: error: FAILSAFE_REQUIRED + # Step 4 - label: "TH sends ArmFailSafe command to the DUT" cluster: General Commissioning command: ArmFailSafe @@ -75,7 +78,12 @@ tests: value: 60 - name: Breadcrumb value: 1 + response: + values: + - name: ErrorCode + value: CommissioningErrorEnum.OK + # Step 5 - label: "TH sends an invalid ActiveDatasetRequest command to the DUT" command: SetActiveDatasetRequest arguments: @@ -85,6 +93,7 @@ tests: response: error: INVALID_COMMAND + # Step 6 - label: "TH sends a valid ActiveDatasetRequest command to the DUT" command: SetActiveDatasetRequest arguments: @@ -92,12 +101,20 @@ tests: - name: ActiveDataset value: PIXIT.TBRM.THREAD_ACTIVE_DATASET + # Step 7 + - label: "TH sends CommissioningComplete command to the DUT" + cluster: General Commissioning + command: CommissioningComplete + endpoint: 0 + + # Step 8 - label: "TH reads the InterfaceEnabled attribute from the DUT" command: readAttribute attribute: InterfaceEnabled response: value: true + # Step 9 - label: "TH reads the ActiveDatasetTimestamp attribute from the DUT" command: readAttribute attribute: ActiveDatasetTimestamp @@ -106,7 +123,8 @@ tests: constraints: type: int64u - - label: "TH sends a valid GetActiveDatasetRequest command to the DUT" + # Step 10 + - label: "TH send a GetActiveDatasetRequest command to the DUT" command: GetActiveDatasetRequest response: values: diff --git a/src/app/tests/suites/certification/Test_TC_TBRM_2_3.yaml b/src/app/tests/suites/certification/Test_TC_TBRM_2_3.yaml index 7a55191f471f0b..a0ecd11693ef8d 100644 --- a/src/app/tests/suites/certification/Test_TC_TBRM_2_3.yaml +++ b/src/app/tests/suites/certification/Test_TC_TBRM_2_3.yaml @@ -49,7 +49,7 @@ tests: constraints: type: int64u - - label: "If the ActiveDatasetTimestamp attribute not null, go to step 4" + - label: "If the ActiveDatasetTimestamp attribute not null, go to step 5" cluster: EqualityCommands command: UnsignedNumberEquals arguments: @@ -75,6 +75,10 @@ tests: value: 60 - name: Breadcrumb value: 1 + response: + values: + - name: ErrorCode + value: CommissioningErrorEnum.OK # Step 3 - label: "TH sends a valid ActiveDatasetRequest command to the DUT" @@ -86,6 +90,13 @@ tests: value: PIXIT.TBRM.THREAD_ACTIVE_DATASET # Step 4 + - label: "TH sends CommissioningComplete command to the DUT" + runIf: noActiveDataset + cluster: General Commissioning + command: CommissioningComplete + endpoint: 0 + + # Step 5 - label: "TH reads the PendingDatasetTimestamp attribute from the DUT" command: readAttribute attribute: PendingDatasetTimestamp @@ -94,7 +105,7 @@ tests: constraints: type: int64u - # Step 5 + # Step 6 - label: "TH sends a SetPendingDatasetRequest command to the DUT" command: SetPendingDatasetRequest arguments: @@ -102,7 +113,7 @@ tests: - name: PendingDataset value: PIXIT.TBRM.THREAD_PENDING_DATASET - # Step 6 + # Step 7 - label: "TH sends a GetPendingDatasetRequest command to the DUT" command: GetPendingDatasetRequest response: @@ -112,7 +123,7 @@ tests: type: octet_string # TODO: This should be PIXIT.TBRM.THREAD_PENDING_DATASET but ignoring the Delay Timer element if present - # Step 7 + # Step 8 - label: "TH reads the PendingDatasetTimestamp attribute from the DUT" command: readAttribute attribute: PendingDatasetTimestamp @@ -121,7 +132,7 @@ tests: type: int64u notValue: initialPendingTimestamp - # Step 8 + # Step 9 - label: "TH subscribes to the ActiveDatasetTimestamp attribute from the DUT" command: subscribeAttribute @@ -141,15 +152,15 @@ tests: constraints: hasValue: true - # Step 9 + # Step 10 - label: "TH reads the PendingDatasetTimestamp attribute from the DUT" command: readAttribute attribute: PendingDatasetTimestamp response: value: null - # Step 10 - - label: "TH sends a valid GetActiveDatasetRequest command to the DUT" + # Step 11 + - label: "TH sends a GetActiveDatasetRequest command to the DUT" command: GetActiveDatasetRequest response: values: @@ -158,7 +169,7 @@ tests: constraints: type: octet_string - # Step 11 + # Step 12 - label: "TH reads the InterfaceEnabled attribute from the DUT" command: readAttribute attribute: InterfaceEnabled diff --git a/src/app/tests/suites/certification/Test_TC_TBRM_2_4.yaml b/src/app/tests/suites/certification/Test_TC_TBRM_2_4.yaml new file mode 100644 index 00000000000000..d1483c1c5a930a --- /dev/null +++ b/src/app/tests/suites/certification/Test_TC_TBRM_2_4.yaml @@ -0,0 +1,86 @@ +# 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. + +name: + "[TC-TBRM-2.4] Verify that getting Active or Pending Dataset in the PASE + session results in unsupported access" + +PICS: + - TBRM.S + +config: + nodeId: 0x12344321 + + payload: "MT:-24J0AFN00KA0648G00" + discriminator: 3840 + PakeVerifier: + type: octet_string + defaultValue: "hex:b96170aae803346884724fe9a3b287c30330c2a660375d17bb205a8cf1aecb350457f8ab79ee253ab6a8e46bb09e543ae422736de501e3db37d441fe344920d09548e4c18240630c4ff4913c53513839b7c07fcc0627a1b8573a149fcd1fa466cf" + +tests: + - label: "Wait for the commissioned device to be retrieved" + cluster: DelayCommands + command: WaitForCommissionee + arguments: + values: + - name: nodeId + value: nodeId + + - label: "Open Commissioning Window" + endpoint: 0 + cluster: Administrator Commissioning + command: OpenCommissioningWindow + timedInteractionTimeoutMs: 2000 + arguments: + values: + - name: CommissioningTimeout + value: 180 + - name: PAKEPasscodeVerifier + value: PakeVerifier + - name: Discriminator + value: discriminator + - name: Iterations + value: 1000 + - name: Salt + value: "SPAKE2P Key Salt" + + - label: "TH2 establishes a PASE session with the DUT" + identity: beta + cluster: CommissionerCommands + endpoint: 0 + command: EstablishPASESession + arguments: + values: + - name: nodeId + value: nodeId + - name: payload + value: payload + + - label: + "TH2 send GetActiveDatasetRequest command to the DUT in PASE session" + identity: beta + cluster: Thread Border Router Management + endpoint: 1 + command: GetActiveDatasetRequest + response: + error: UNSUPPORTED_ACCESS + + - label: + "TH2 send GetPendingDatasetRequest command to the DUT in PASE session" + identity: beta + cluster: Thread Border Router Management + endpoint: 1 + command: GetPendingDatasetRequest + response: + error: UNSUPPORTED_ACCESS diff --git a/src/app/tests/suites/certification/Test_TC_TBRM_3_1_Simulated.yaml b/src/app/tests/suites/certification/Test_TC_TBRM_3_1_Simulated.yaml new file mode 100644 index 00000000000000..095711a0fcee95 --- /dev/null +++ b/src/app/tests/suites/certification/Test_TC_TBRM_3_1_Simulated.yaml @@ -0,0 +1,44 @@ +# 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. + +name: "[TC-TBRM-3.1] Functionality with client as DUT" + +PICS: + - TBRM.C + +config: + nodeId: 0x12344321 + cluster: Thread Border Router Management + endpoint: 1 + +tests: + # Step 1 + - label: "DUT send SetActiveDatasetRequest to TH" + PICS: TBRM.C.C03.Tx + wait: "SetActiveDatasetRequest" + + # Step 2 + - label: "DUT send SetPendingDatasetRequest to TH" + PICS: TBRM.C.C04.Tx + wait: "SetPendingDatasetRequest" + + # Step 3 + - label: "DUT send GetActiveDatasetRequest to TH" + PICS: TBRM.C.C00.Tx + wait: "GetActiveDatasetRequest" + + # Step 4 + - label: "DUT send GetPendingDatasetRequest to TH" + PICS: TBRM.C.C01.Tx + wait: "GetPendingDatasetRequest" From 63da5409095845fe8d6125ace84859365c9f612d Mon Sep 17 00:00:00 2001 From: Jae Son <12516308+jaehs6sam@users.noreply.github.com> Date: Wed, 28 Aug 2024 19:09:09 -0500 Subject: [PATCH 06/13] TC OCC 3.1 and 3.2 CI Implementation (#34836) * Update tests.yaml adding occ test case 3.1 and 3.2 * Update TC_OCC_3_1.py added CI on step 3 and 4 * Update TC_OCC_3_2.py added ci implementation on step 3a and 3c * Update AllClustersCommandDelegate.cpp Added occupancy sensor ci named path implementation * Update AllClustersCommandDelegate.h added occ ci named path implemetation * Update AllClustersCommandDelegate.cpp change to be restyled * Update TC_OCC_3_1.py restyled * Update TC_OCC_3_2.py restyled * Update TC_OCC_3_1.py restyled * Update TC_OCC_3_2.py restyled * Update TC_OCC_3_1.py sleep module fix * Update TC_OCC_3_2.py sleep module fix * Update TC_OCC_3_2.py restyled * Update TC_OCC_3_1.py minor fixes * Update TC_OCC_3_2.py Added one more write to consider proper callback sequence * Update TC_OCC_3_2.py restyled * Update TC_OCC_3_2.py * Update tests.yaml * Update TC_OCC_3_2.py Fixed a bug on occupancy attribute testing * Update TC_OCC_3_1.py * Update TC_OCC_3_1.py * Update TC_OCC_3_2.py * Update TC_OCC_3_1.py * Update TC_OCC_3_2.py * Update AllClustersCommandDelegate.cpp Removed 1 == endpointId at line 798 Replaced nullptr with reinterpret_cast(static_cast(endpointId)) at line 804 Replaced (1, cleaValue) in line 817 with (static_cast(reinterpret_cast(appState)), clearValue) * Update TC_OCC_3_1.py Modified HoldTime implementation on testing and added Occupancy attribute subscription and event testing in 3.1. * Update TC_OCC_3_1.py * Update TC_OCC_3_1.py * Update AllClustersCommandDelegate.cpp * Update TC_OCC_3_1.py fixed python errors * Update TC_OCC_3_1.py * Update TC_OCC_3_2.py Undated with holdtime testing with holdtime Min/Max * Update TC_OCC_3_2.py * Update TC_OCC_3_2.py * Update TC_OCC_3_2.py * Update .github/workflows/tests.yaml Co-authored-by: Tennessee Carmel-Veilleux * Update .github/workflows/tests.yaml Co-authored-by: Tennessee Carmel-Veilleux * Fix TC-OCC-3.1/3.2 tests - Implement event handling - Properly implement event checks - Correctly handle HoldTime -> Legacy timing dependencies - Simplify flows - Add missing steps * Update AllClustersCommandDelegate.cpp restyled * Update occupancy-sensor-server.cpp restyled * Update TC_OCC_3_1.py restyled * Update TC_OCC_3_2.py restyled * Update matter_testing_support.py restyled * Update TC_OCC_3_2.py * Fix a lint --------- Co-authored-by: Tennessee Carmel-Veilleux --- .../linux/AllClustersCommandDelegate.cpp | 99 +++++ .../linux/AllClustersCommandDelegate.h | 6 + .../occupancy-sensor-server.cpp | 22 +- .../occupancy-sensor-server.h | 2 +- src/python_testing/TC_OCC_3_1.py | 332 +++++++++------ src/python_testing/TC_OCC_3_2.py | 402 +++++++++--------- src/python_testing/matter_testing_support.py | 102 ++++- 7 files changed, 604 insertions(+), 361 deletions(-) diff --git a/examples/all-clusters-app/linux/AllClustersCommandDelegate.cpp b/examples/all-clusters-app/linux/AllClustersCommandDelegate.cpp index 6c1467fc62a145..64465384054574 100644 --- a/examples/all-clusters-app/linux/AllClustersCommandDelegate.cpp +++ b/examples/all-clusters-app/linux/AllClustersCommandDelegate.cpp @@ -19,7 +19,9 @@ #include "AllClustersCommandDelegate.h" #include +#include #include +#include #include #include #include @@ -246,6 +248,24 @@ void HandleSimulateLatchPosition(Json::Value & jsonValue) } } +void EmitOccupancyChangedEvent(EndpointId endpointId, uint8_t occupancyValue) +{ + Clusters::OccupancySensing::Events::OccupancyChanged::Type event{}; + event.occupancy = static_cast>(occupancyValue); + EventNumber eventNumber = 0; + + CHIP_ERROR err = LogEvent(event, endpointId, eventNumber); + if (err != CHIP_NO_ERROR) + { + ChipLogError(NotSpecified, "Failed to log OccupancyChanged event: %" CHIP_ERROR_FORMAT, err.Format()); + } + else + { + ChipLogProgress(NotSpecified, "Logged OccupancyChanged(occupancy=%u) on Endpoint %u", static_cast(occupancyValue), + static_cast(endpointId)); + } +} + } // namespace AllClustersAppCommandHandler * AllClustersAppCommandHandler::FromJSON(const char * json) @@ -407,6 +427,20 @@ void AllClustersAppCommandHandler::HandleCommand(intptr_t context) { HandleSimulateLatchPosition(self->mJsonValue); } + else if (name == "SetOccupancy") + { + uint8_t occupancy = static_cast(self->mJsonValue["Occupancy"].asUInt()); + EndpointId endpointId = static_cast(self->mJsonValue["EndpointId"].asUInt()); + + if (1 == occupancy || 0 == occupancy) + { + self->HandleSetOccupancyChange(endpointId, occupancy); + } + else + { + ChipLogError(NotSpecified, "Invalid Occupancy state to set."); + } + } else { ChipLogError(NotSpecified, "Unhandled command '%s': this hould never happen", name.c_str()); @@ -769,6 +803,71 @@ void AllClustersAppCommandHandler::OnAirQualityChange(uint32_t aNewValue) } } +void AllClustersAppCommandHandler::HandleSetOccupancyChange(EndpointId endpointId, uint8_t newOccupancyValue) +{ + BitMask currentOccupancy; + Protocols::InteractionModel::Status status = OccupancySensing::Attributes::Occupancy::Get(endpointId, ¤tOccupancy); + + if (static_cast>(newOccupancyValue) == currentOccupancy) + { + ChipLogDetail(NotSpecified, "Skipping setting occupancy changed due to same value."); + return; + } + + status = OccupancySensing::Attributes::Occupancy::Set(endpointId, newOccupancyValue); + ChipLogDetail(NotSpecified, "Set Occupancy attribute to %u", newOccupancyValue); + + if (status != Protocols::InteractionModel::Status::Success) + { + ChipLogDetail(NotSpecified, "Invalid value/endpoint to set."); + return; + } + + EmitOccupancyChangedEvent(endpointId, newOccupancyValue); + + if (1 == newOccupancyValue) + { + uint16_t * holdTime = chip::app::Clusters::OccupancySensing::GetHoldTimeForEndpoint(endpointId); + if (holdTime != nullptr) + { + CHIP_ERROR err = chip::DeviceLayer::SystemLayer().StartTimer( + chip::System::Clock::Seconds16(*holdTime), AllClustersAppCommandHandler::OccupancyPresentTimerHandler, + reinterpret_cast(static_cast(endpointId))); + ChipLogDetail(NotSpecified, "Start HoldTime timer"); + if (CHIP_NO_ERROR != err) + { + ChipLogError(NotSpecified, "Failed to start HoldTime timer."); + } + } + } +} + +void AllClustersAppCommandHandler::OccupancyPresentTimerHandler(System::Layer * systemLayer, void * appState) +{ + EndpointId endpointId = static_cast(reinterpret_cast(appState)); + chip::BitMask currentOccupancy; + + Protocols::InteractionModel::Status status = OccupancySensing::Attributes::Occupancy::Get(endpointId, ¤tOccupancy); + VerifyOrDie(status == Protocols::InteractionModel::Status::Success); + + uint8_t clearValue = 0; + if (!currentOccupancy.Has(Clusters::OccupancySensing::OccupancyBitmap::kOccupied)) + { + return; + } + + status = OccupancySensing::Attributes::Occupancy::Set(endpointId, clearValue); + if (status != Protocols::InteractionModel::Status::Success) + { + ChipLogDetail(NotSpecified, "Failed to set occupancy state."); + } + else + { + ChipLogDetail(NotSpecified, "Set Occupancy attribute to clear"); + EmitOccupancyChangedEvent(endpointId, clearValue); + } +} + void AllClustersCommandDelegate::OnEventCommandReceived(const char * json) { auto handler = AllClustersAppCommandHandler::FromJSON(json); diff --git a/examples/all-clusters-app/linux/AllClustersCommandDelegate.h b/examples/all-clusters-app/linux/AllClustersCommandDelegate.h index f1b873fc0d69c4..b0d1f2e99750ca 100644 --- a/examples/all-clusters-app/linux/AllClustersCommandDelegate.h +++ b/examples/all-clusters-app/linux/AllClustersCommandDelegate.h @@ -115,6 +115,12 @@ class AllClustersAppCommandHandler * Should be called when it is necessary to change the operational state as a manual operation. */ void OnOvenOperationalStateChange(std::string device, std::string operation, Json::Value param); + + /** + * Should be called when it is necessary to change the Occupancy attribute. + */ + void HandleSetOccupancyChange(chip::EndpointId endpointId, uint8_t occupancyValue); + static void OccupancyPresentTimerHandler(chip::System::Layer * systemLayer, void * appState); }; class AllClustersCommandDelegate : public NamedPipeCommandDelegate diff --git a/src/app/clusters/occupancy-sensor-server/occupancy-sensor-server.cpp b/src/app/clusters/occupancy-sensor-server/occupancy-sensor-server.cpp index c60fdc5255c023..cd7ba5b8f659c2 100644 --- a/src/app/clusters/occupancy-sensor-server/occupancy-sensor-server.cpp +++ b/src/app/clusters/occupancy-sensor-server/occupancy-sensor-server.cpp @@ -94,8 +94,10 @@ CHIP_ERROR Instance::Write(const ConcreteDataAttributePath & aPath, AttributeVal switch (aPath.mAttributeId) { - case Attributes::HoldTime::Id: { - + case Attributes::HoldTime::Id: + case Attributes::PIROccupiedToUnoccupiedDelay::Id: + case Attributes::UltrasonicOccupiedToUnoccupiedDelay::Id: + case Attributes::PhysicalContactOccupiedToUnoccupiedDelay::Id: { uint16_t newHoldTime; ReturnErrorOnFailure(aDecoder.Decode(newHoldTime)); @@ -173,16 +175,26 @@ uint16_t * GetHoldTimeForEndpoint(EndpointId endpoint) return &sHoldTime[index]; } -CHIP_ERROR SetHoldTime(EndpointId endpointId, const uint16_t & holdTime) +CHIP_ERROR SetHoldTime(EndpointId endpointId, uint16_t newHoldTime) { VerifyOrReturnError(kInvalidEndpointId != endpointId, CHIP_ERROR_INVALID_ARGUMENT); uint16_t * holdTimeForEndpoint = GetHoldTimeForEndpoint(endpointId); VerifyOrReturnError(holdTimeForEndpoint != nullptr, CHIP_ERROR_INVALID_ARGUMENT); - *holdTimeForEndpoint = holdTime; + uint16_t previousHoldTime = *holdTimeForEndpoint; + *holdTimeForEndpoint = newHoldTime; + + if (previousHoldTime != newHoldTime) + { + MatterReportingAttributeChangeCallback(endpointId, OccupancySensing::Id, Attributes::HoldTime::Id); + } - MatterReportingAttributeChangeCallback(endpointId, OccupancySensing::Id, Attributes::HoldTime::Id); + // Blindly try to write RAM-backed legacy attributes (will fail silently if absent) + // to keep them in sync. + (void) Attributes::PIROccupiedToUnoccupiedDelay::Set(endpointId, newHoldTime); + (void) Attributes::UltrasonicOccupiedToUnoccupiedDelay::Set(endpointId, newHoldTime); + (void) Attributes::PhysicalContactOccupiedToUnoccupiedDelay::Set(endpointId, newHoldTime); return CHIP_NO_ERROR; } diff --git a/src/app/clusters/occupancy-sensor-server/occupancy-sensor-server.h b/src/app/clusters/occupancy-sensor-server/occupancy-sensor-server.h index c4c7181b626f3a..512fd288fb73cd 100644 --- a/src/app/clusters/occupancy-sensor-server/occupancy-sensor-server.h +++ b/src/app/clusters/occupancy-sensor-server/occupancy-sensor-server.h @@ -54,7 +54,7 @@ class Instance : public AttributeAccessInterface CHIP_ERROR SetHoldTimeLimits(EndpointId endpointId, const Structs::HoldTimeLimitsStruct::Type & holdTimeLimits); -CHIP_ERROR SetHoldTime(EndpointId endpointId, const uint16_t & holdTime); +CHIP_ERROR SetHoldTime(EndpointId endpointId, uint16_t newHoldTime); Structs::HoldTimeLimitsStruct::Type * GetHoldTimeLimitsForEndpoint(EndpointId endpoint); diff --git a/src/python_testing/TC_OCC_3_1.py b/src/python_testing/TC_OCC_3_1.py index 246e3a13f2a211..cfec9a86532c18 100644 --- a/src/python_testing/TC_OCC_3_1.py +++ b/src/python_testing/TC_OCC_3_1.py @@ -1,129 +1,203 @@ -# -# Copyright (c) 2024 Project CHIP Authors -# All rights reserved. -# -# 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. -# - -# There are CI issues to be followed up for the test cases below that implements manually controlling sensor device for -# the occupancy state ON/OFF change. -# [TC-OCC-3.1] test procedure step 4 -# [TC-OCC-3.2] test precedure step 3c - -import logging -import time -from typing import Any, Optional - -import chip.clusters as Clusters -from chip.interaction_model import Status -from matter_testing_support import MatterBaseTest, TestStep, async_test_body, default_matter_test_main -from mobly import asserts - - -class TC_OCC_3_1(MatterBaseTest): - async def read_occ_attribute_expect_success(self, attribute): - cluster = Clusters.Objects.OccupancySensing - endpoint = self.matter_test_config.endpoint - return await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=attribute) - - async def write_hold_time(self, hold_time: Optional[Any]) -> Status: - dev_ctrl = self.default_controller - node_id = self.dut_node_id - endpoint = self.matter_test_config.endpoint - - cluster = Clusters.OccupancySensing - write_result = await dev_ctrl.WriteAttribute(node_id, [(endpoint, cluster.Attributes.HoldTime(hold_time))]) - return write_result[0].Status - - def desc_TC_OCC_3_1(self) -> str: - return "[TC-OCC-3.1] Primary functionality with server as DUT" - - def steps_TC_OCC_3_1(self) -> list[TestStep]: - steps = [ - TestStep(1, "Commission DUT to TH and obtain DUT attribute list.", is_commissioning=True), - TestStep(2, "Change DUT HoldTime attribute value to 10 seconds."), - TestStep(3, "Do not trigger DUT occupancy sensing for the period of HoldTime. TH reads Occupancy attribute from DUT."), - TestStep(4, "Trigger DUT occupancy sensing to change the occupancy state and start a timer."), - TestStep(5, "After 10 seconds, TH reads Occupancy attribute from DUT.") - ] - return steps - - def pics_TC_OCC_3_1(self) -> list[str]: - pics = [ - "OCC.S", - ] - return pics - - @async_test_body - async def test_TC_OCC_3_1(self): - hold_time = 10 # 10 seconds for occupancy state hold time - - self.step(1) # Commissioning already done - - self.step(2) - - cluster = Clusters.OccupancySensing - attributes = cluster.Attributes - attribute_list = await self.read_occ_attribute_expect_success(attribute=attributes.AttributeList) - - has_hold_time = attributes.HoldTime.attribute_id in attribute_list - - if has_hold_time: - # write 10 as a HoldTime attribute - await self.write_single_attribute(cluster.Attributes.HoldTime(hold_time)) - else: - logging.info("No HoldTime attribute supports. Will test only occupancy attribute triggering functionality") - - self.step(3) - # check if Occupancy attribute is 0 - occupancy_dut = await self.read_occ_attribute_expect_success(attribute=attributes.Occupancy) - - # if occupancy is on, then wait until the sensor occupancy state is 0. - if occupancy_dut == 1: - # Don't trigger occupancy sensor to render occupancy attribute to 0 - if has_hold_time: - time.sleep(hold_time + 2.0) # add some extra 2 seconds to ensure hold time has passed. - else: # a user wait until a sensor specific time to change occupancy attribute to 0. This is the case where the sensor doesn't support HoldTime. - self.wait_for_user_input( - prompt_msg="Type any letter and press ENTER after the sensor occupancy is detection ready state (occupancy attribute = 0)") - - # check sensor occupancy state is 0 for the next test step - occupancy_dut = await self.read_occ_attribute_expect_success(attribute=attributes.Occupancy) - asserts.assert_equal(occupancy_dut, 0, "Occupancy attribute is still 1.") - - self.step(4) - # Trigger occupancy sensor to change Occupancy attribute value to 1 => TESTER ACTION on DUT - self.wait_for_user_input(prompt_msg="Type any letter and press ENTER after a sensor occupancy is triggered.") - - # And then check if Occupancy attribute has changed. - occupancy_dut = await self.read_occ_attribute_expect_success(attribute=attributes.Occupancy) - asserts.assert_equal(occupancy_dut, 1, "Occupancy state is not changed to 1") - - self.step(5) - # check if Occupancy attribute is back to 0 after HoldTime attribute period - # Tester should not be triggering the sensor for this test step. - if has_hold_time: - - # Start a timer based on HoldTime - time.sleep(hold_time + 2.0) # add some extra 2 seconds to ensure hold time has passed. - - occupancy_dut = await self.read_occ_attribute_expect_success(attribute=attributes.Occupancy) - asserts.assert_equal(occupancy_dut, 0, "Occupancy state is not 0 after HoldTime period") - - else: - logging.info("HoldTime attribute not supported. Skip this return to 0 timing test procedure.") - self.skip_step(5) - - -if __name__ == "__main__": - default_matter_test_main() +# +# Copyright (c) 2024 Project CHIP Authors +# All rights reserved. +# +# 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. +# +# === BEGIN CI TEST ARGUMENTS === +# test-runner-runs: run1 +# test-runner-run/run1/app: ${ALL_CLUSTERS_APP} +# test-runner-run/run1/factoryreset: True +# test-runner-run/run1/quiet: True +# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json +# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto --endpoint 1 --bool-arg simulate_occupancy:true +# === END CI TEST ARGUMENTS === +# There are CI issues to be followed up for the test cases below that implements manually controlling sensor device for +# the occupancy state ON/OFF change. +# [TC-OCC-3.1] test procedure step 3, 4 +# [TC-OCC-3.2] test precedure step 3a, 3c + +import logging +import time +from typing import Any, Optional + +import chip.clusters as Clusters +from chip.interaction_model import Status +from matter_testing_support import (ClusterAttributeChangeAccumulator, EventChangeCallback, MatterBaseTest, TestStep, + async_test_body, await_sequence_of_reports, default_matter_test_main) +from mobly import asserts + + +class TC_OCC_3_1(MatterBaseTest): + def setup_test(self): + super().setup_test() + self.is_ci = self.matter_test_config.global_test_params.get('simulate_occupancy', False) + + async def read_occ_attribute_expect_success(self, attribute): + cluster = Clusters.Objects.OccupancySensing + endpoint = self.matter_test_config.endpoint + return await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=attribute) + + async def write_hold_time(self, hold_time: Optional[Any]) -> Status: + dev_ctrl = self.default_controller + node_id = self.dut_node_id + endpoint = self.matter_test_config.endpoint + + cluster = Clusters.OccupancySensing + write_result = await dev_ctrl.WriteAttribute(node_id, [(endpoint, cluster.Attributes.HoldTime(hold_time))]) + return write_result[0].Status + + def desc_TC_OCC_3_1(self) -> str: + return "[TC-OCC-3.1] Primary functionality with server as DUT" + + def steps_TC_OCC_3_1(self) -> list[TestStep]: + steps = [ + TestStep(1, "Commission DUT to TH.", is_commissioning=True), + TestStep(2, "If HoldTime is supported, TH writes HoldTime attribute to 10 sec on DUT."), + TestStep(3, "Prompt operator to await until DUT occupancy changes to unoccupied state."), + TestStep(4, "TH subscribes to Occupancy sensor attributes and events."), + TestStep("5a", "Prompt operator to trigger occupancy change."), + TestStep("5b", "TH reads Occupancy attribute from DUT. Verify occupancy changed to occupied and Occupancy attribute was reported as occupied."), + TestStep("5c", "If supported, verify OccupancyChangedEvent was reported as occupied."), + TestStep(6, "If HoldTime is supported, wait for HoldTime, otherwise prompt operator to wait until no longer occupied."), + TestStep("7a", "TH reads Occupancy attribute from DUT. Verify occupancy changed to unoccupied and Occupancy attribute was reported as unoccupied."), + TestStep("7b", "If supported, verify OccupancyChangedEvent was reported as unoccupied."), + ] + return steps + + def pics_TC_OCC_3_1(self) -> list[str]: + pics = [ + "OCC.S", + ] + return pics + + # Sends and out-of-band command to the all-clusters-app + def write_to_app_pipe(self, command): + # CI app pipe id creation + self.app_pipe = "/tmp/chip_all_clusters_fifo_" + if self.is_ci: + app_pid = self.matter_test_config.app_pid + if app_pid == 0: + asserts.fail("The --app-pid flag must be set when using named pipe") + self.app_pipe = self.app_pipe + str(app_pid) + + with open(self.app_pipe, "w") as app_pipe: + app_pipe.write(command + "\n") + # Delay for pipe command to be processed (otherwise tests are flaky) + time.sleep(0.001) + + @async_test_body + async def test_TC_OCC_3_1(self): + hold_time = 10 if not self.is_ci else 1.0 # 10 seconds for occupancy state hold time + + self.step(1) # Commissioning already done + + self.step(2) + + cluster = Clusters.OccupancySensing + attributes = cluster.Attributes + attribute_list = await self.read_occ_attribute_expect_success(attribute=attributes.AttributeList) + + has_hold_time = attributes.HoldTime.attribute_id in attribute_list + occupancy_event_supported = self.check_pics("OCC.M.OccupancyChange") or self.is_ci + + if has_hold_time: + # write HoldTimeLimits HoldtimeMin to be 10 sec. + await self.write_single_attribute(cluster.Attributes.HoldTime(hold_time)) + holdtime_dut = await self.read_occ_attribute_expect_success(attribute=attributes.HoldTime) + asserts.assert_equal(holdtime_dut, hold_time, "Hold time read-back does not match hold time written") + else: + logging.info("No HoldTime attribute supports. Will test only occupancy attribute triggering functionality only.") + + self.step(3) + + if self.is_ci: + # CI call to trigger unoccupied. + self.write_to_app_pipe('{"Name":"SetOccupancy", "EndpointId": 1, "Occupancy": 0}') + else: + self.wait_for_user_input( + prompt_msg="Type any letter and press ENTER after the sensor occupancy is unoccupied state (occupancy attribute = 0)") + + # check sensor occupancy state is 0 for the next test step + occupancy_dut = await self.read_occ_attribute_expect_success(attribute=attributes.Occupancy) + asserts.assert_equal(occupancy_dut, 0, "Occupancy attribute is not unoccupied.") + + self.step(4) + # Setup Occupancy attribute subscription here + endpoint_id = self.matter_test_config.endpoint + node_id = self.dut_node_id + dev_ctrl = self.default_controller + attrib_listener = ClusterAttributeChangeAccumulator(cluster) + await attrib_listener.start(dev_ctrl, node_id, endpoint=endpoint_id, min_interval_sec=0, max_interval_sec=30) + + if occupancy_event_supported: + event_listener = EventChangeCallback(cluster) + await event_listener.start(dev_ctrl, node_id, endpoint=endpoint_id, min_interval_sec=0, max_interval_sec=30) + + self.step("5a") + # CI call to trigger on + if self.is_ci: + self.write_to_app_pipe('{"Name":"SetOccupancy", "EndpointId": 1, "Occupancy": 1}') + else: + # Trigger occupancy sensor to change Occupancy attribute value to 1 => TESTER ACTION on DUT + self.wait_for_user_input(prompt_msg="Type any letter and press ENTER after a sensor occupancy is triggered.") + + # And then check if Occupancy attribute has changed. + self.step("5b") + occupancy_dut = await self.read_occ_attribute_expect_success(attribute=attributes.Occupancy) + asserts.assert_equal(occupancy_dut, 1, "Occupancy state is not changed to 1") + + # subscription verification + post_prompt_settle_delay_seconds = 1.0 if self.is_ci else 10.0 + await_sequence_of_reports(report_queue=attrib_listener.attribute_queue, endpoint_id=endpoint_id, attribute=cluster.Attributes.Occupancy, sequence=[ + 1], timeout_sec=post_prompt_settle_delay_seconds) + + if occupancy_event_supported: + self.step("5c") + event = event_listener.wait_for_event_report( + cluster.Events.OccupancyChanged, timeout_sec=post_prompt_settle_delay_seconds) + asserts.assert_equal(event.occupancy, 1, "Unexpected occupancy on OccupancyChanged") + else: + self.skip_step("5c") + + self.step(6) + if self.is_ci: + # CI call to trigger unoccupied. + self.write_to_app_pipe('{"Name":"SetOccupancy", "EndpointId": 1, "Occupancy": 0}') + + if has_hold_time: + time.sleep(hold_time + 2.0) # add some extra 2 seconds to ensure hold time has passed. + else: + self.wait_for_user_input( + prompt_msg="Type any letter and press ENTER after the sensor occupancy is back to unoccupied state (occupancy attribute = 0)") + + # Check if Occupancy attribute is back to 0 after HoldTime attribute period + # Tester should not be triggering the sensor for this test step. + self.step("7a") + occupancy_dut = await self.read_occ_attribute_expect_success(attribute=attributes.Occupancy) + asserts.assert_equal(occupancy_dut, 0, "Occupancy state is not back to 0 after HoldTime period") + + await_sequence_of_reports(report_queue=attrib_listener.attribute_queue, endpoint_id=endpoint_id, attribute=cluster.Attributes.Occupancy, sequence=[ + 0], timeout_sec=post_prompt_settle_delay_seconds) + + if occupancy_event_supported: + self.step("7b") + event = event_listener.wait_for_event_report( + cluster.Events.OccupancyChanged, timeout_sec=post_prompt_settle_delay_seconds) + asserts.assert_equal(event.occupancy, 0, "Unexpected occupancy on OccupancyChanged") + else: + self.skip_step("7b") + + +if __name__ == "__main__": + default_matter_test_main() diff --git a/src/python_testing/TC_OCC_3_2.py b/src/python_testing/TC_OCC_3_2.py index 64a588b6eb36e8..c4049104feba68 100644 --- a/src/python_testing/TC_OCC_3_2.py +++ b/src/python_testing/TC_OCC_3_2.py @@ -1,204 +1,198 @@ -# -# Copyright (c) 2024 Project CHIP (Matter) Authors -# All rights reserved. -# -# 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. -# - -# TODO: There are CI issues to be followed up for the test cases below that implements manually controlling sensor device for -# the occupancy state ON/OFF change. -# [TC-OCC-3.1] test procedure step 4 -# [TC-OCC-3.2] test precedure step 3a, 3c - -import logging - -import chip.clusters as Clusters -from matter_testing_support import (ClusterAttributeChangeAccumulator, MatterBaseTest, TestStep, async_test_body, - await_sequence_of_reports, default_matter_test_main) -from mobly import asserts - - -class TC_OCC_3_2(MatterBaseTest): - async def read_occ_attribute_expect_success(self, attribute): - cluster = Clusters.Objects.OccupancySensing - endpoint_id = self.matter_test_config.endpoint - return await self.read_single_attribute_check_success(endpoint=endpoint_id, cluster=cluster, attribute=attribute) - - def desc_TC_OCC_3_2(self) -> str: - return "[TC-OCC-3.2] Subscription Report Verification with server as DUT" - - def steps_TC_OCC_3_2(self) -> list[TestStep]: - steps = [ - TestStep(1, "Commission DUT to TH if not already done", is_commissioning=True), - TestStep(2, "TH establishes a wildcard subscription to all attributes on Occupancy Sensing Cluster on the endpoint under test. Subscription min interval = 0 and max interval = 30 seconds."), - TestStep("3a", "Do not trigger DUT for occupancy state change."), - TestStep("3b", "TH reads DUT Occupancy attribute and saves the initial value as initial"), - TestStep("3c", "Trigger DUT to change the occupancy state."), - TestStep("3d", "TH awaits a ReportDataMessage containing an attribute report for DUT Occupancy attribute."), - TestStep("4a", "Check if DUT supports HoldTime attribute, If not supported, then stop and skip the rest of test cases."), - TestStep("4b", "TH reads DUT HoldTime attribute and saves the initial value as initial"), - TestStep("4c", "TH writes a different value to DUT HoldTime attribute."), - TestStep("4d", "TH awaits a ReportDataMessage containing an attribute report for DUT HoldTime attribute."), - TestStep("5a", "Check if DUT supports DUT feature flag PIR or (!PIR & !US & !PHY) and has the PIROccupiedToUnoccupiedDelay attribute, If not supported, then skip 5b, 5c, 5d."), - TestStep("5b", "TH reads DUT PIROccupiedToUnoccupiedDelay attribute and saves the initial value as initial"), - TestStep("5c", "TH writes a different value to DUT PIROccupiedToUnoccupiedDelay attribute."), - TestStep("5d", "TH awaits a ReportDataMessage containing an attribute report for DUT PIROccupiedToUnoccupiedDelay attribute."), - TestStep("6a", "Check if DUT supports DUT feature flag US and has the UltrasonicOccupiedToUnoccupiedDelay attribute. If not supported, then skip 6b, 6c, 6d."), - TestStep("6b", "TH reads DUT UltrasonicOccupiedToUnoccupiedDelay attribute and saves the initial value as initial"), - TestStep("6c", "TH writes a different value to DUT UltrasonicOccupiedToUnoccupiedDelay attribute."), - TestStep("6d", "TH awaits a ReportDataMessage containing an attribute report for DUT UltrasonicOccupiedToUnoccupiedDelay attribute."), - TestStep("7a", "Check if DUT supports DUT feature flag PHY and has the PhysicalContactOccupiedToUnoccupiedDelay attribute. If not supported, skip 7b, 7c, 7d."), - TestStep("7b", "TH reads DUT PhysicalContactOccupiedToUnoccupiedDelay attribute and saves the initial value as initial"), - TestStep("7c", "TH writes a different value to DUT PhysicalContactOccupiedToUnoccupiedDelay attribute."), - TestStep("7d", "TH awaits a ReportDataMessage containing an attribute report for DUT PhysicalContactOccupiedToUnoccupiedDelay attribute.") - ] - return steps - - def pics_TC_OCC_3_2(self) -> list[str]: - pics = [ - "OCC.S", - ] - return pics - - @async_test_body - async def test_TC_OCC_3_2(self): - endpoint_id = self.matter_test_config.endpoint - node_id = self.dut_node_id - dev_ctrl = self.default_controller - - post_prompt_settle_delay_seconds = 10.0 - cluster = Clusters.Objects.OccupancySensing - attributes = cluster.Attributes - - self.step(1) # Commissioning already done - - self.step(2) - feature_map = await self.read_occ_attribute_expect_success(attribute=attributes.FeatureMap) - has_feature_pir = (feature_map & cluster.Bitmaps.Feature.kPassiveInfrared) != 0 - has_feature_ultrasonic = (feature_map & cluster.Bitmaps.Feature.kUltrasonic) != 0 - has_feature_contact = (feature_map & cluster.Bitmaps.Feature.kPhysicalContact) != 0 - - logging.info( - f"Feature map: 0x{feature_map:x}. PIR: {has_feature_pir}, US:{has_feature_ultrasonic}, PHY:{has_feature_contact}") - - attribute_list = await self.read_occ_attribute_expect_success(attribute=attributes.AttributeList) - has_pir_timing_attrib = attributes.PIROccupiedToUnoccupiedDelay.attribute_id in attribute_list - has_ultrasonic_timing_attrib = attributes.UltrasonicOccupiedToUnoccupiedDelay.attribute_id in attribute_list - has_contact_timing_attrib = attributes.PhysicalContactOccupiedToUnoccupiedDelay.attribute_id in attribute_list - logging.info(f"Attribute list: {attribute_list}") - logging.info(f"--> Has PIROccupiedToUnoccupiedDelay: {has_pir_timing_attrib}") - logging.info(f"--> Has UltrasonicOccupiedToUnoccupiedDelay: {has_ultrasonic_timing_attrib}") - logging.info(f"--> Has PhysicalContactOccupiedToUnoccupiedDelay: {has_contact_timing_attrib}") - - # min interval = 0, and max interval = 30 seconds - attrib_listener = ClusterAttributeChangeAccumulator(Clusters.Objects.OccupancySensing) - await attrib_listener.start(dev_ctrl, node_id, endpoint=endpoint_id, min_interval_sec=0, max_interval_sec=30) - - # TODO - Will add Namepiped to assimilate the manual sensor untrigger here - self.step("3a") - self.wait_for_user_input(prompt_msg="Type any letter and press ENTER after DUT goes back to unoccupied state.") - - self.step("3b") - if attributes.Occupancy.attribute_id in attribute_list: - initial_dut = await self.read_occ_attribute_expect_success(attribute=attributes.Occupancy) - asserts.assert_equal(initial_dut, 0, "Occupancy attribute is still detected state") - - # TODO - Will add Namepiped to assimilate the manual sensor trigger here - self.step("3c") - self.wait_for_user_input( - prompt_msg="Type any letter and press ENTER after the sensor occupancy is triggered and its occupancy state changed.") - - self.step("3d") - await_sequence_of_reports(report_queue=attrib_listener.attribute_queue, endpoint_id=endpoint_id, attribute=cluster.Attributes.Occupancy, sequence=[ - 1], timeout_sec=post_prompt_settle_delay_seconds) - - self.step("4a") - if attributes.HoldTime.attribute_id not in attribute_list: - logging.info("No HoldTime attribute supports. Terminate this test case") - self.skip_all_remaining_steps("4b") - - self.step("4b") - initial_dut = await self.read_occ_attribute_expect_success(attribute=attributes.HoldTime) - - self.step("4c") - # write a different a HoldTime attribute value - diff_val = 12 - await self.write_single_attribute(attributes.HoldTime(diff_val)) - - self.step("4d") - await_sequence_of_reports(report_queue=attrib_listener.attribute_queue, endpoint_id=endpoint_id, attribute=cluster.Attributes.HoldTime, sequence=[ - diff_val], timeout_sec=post_prompt_settle_delay_seconds) - - self.step("5a") - - has_no_legacy_features = ((not has_feature_pir) and (not has_feature_ultrasonic) and (not has_feature_contact)) - - if has_pir_timing_attrib and (has_feature_pir or has_no_legacy_features): - self.step("5b") - initial_dut = await self.read_occ_attribute_expect_success(attribute=attributes.PIROccupiedToUnoccupiedDelay) - - self.step("5c") - # write the new attribute value - diff_val = 11 - await self.write_single_attribute(attributes.PIROccupiedToUnoccupiedDelay(diff_val)) - - self.step("5d") - await_sequence_of_reports(report_queue=attrib_listener.attribute_queue, endpoint_id=endpoint_id, attribute=cluster.Attributes.PIROccupiedToUnoccupiedDelay, sequence=[ - diff_val], timeout_sec=post_prompt_settle_delay_seconds) - else: - logging.info("No PIR timing attribute support. Skipping steps 5b, 5c, 5d") - self.skip_step("5b") - self.skip_step("5c") - self.skip_step("5d") - - self.step("6a") - if not has_feature_ultrasonic or not has_ultrasonic_timing_attrib: - logging.info("No Ultrasonic timing attribute supports. Skipping steps 6b, 6c, 6d") - self.skip_step("6b") - self.skip_step("6c") - self.skip_step("6d") - else: - self.step("6b") - initial_dut = await self.read_occ_attribute_expect_success(attribute=attributes.UltrasonicOccupiedToUnoccupiedDelay) - - self.step("6c") - # write the new attribute value - diff_val = 14 - await self.write_single_attribute(attributes.UltrasonicOccupiedToUnoccupiedDelay(diff_val)) - - self.step("6d") - await_sequence_of_reports(report_queue=attrib_listener.attribute_queue, endpoint_id=endpoint_id, attribute=cluster.Attributes.UltrasonicOccupiedToUnoccupiedDelay, sequence=[ - diff_val], timeout_sec=post_prompt_settle_delay_seconds) - - self.step("7a") - if not has_feature_contact or not has_contact_timing_attrib: - logging.info("No Physical contact timing attribute supports. Skipping steps 7b, 7c, 7d") - self.skip_step("7b") - self.skip_step("7c") - self.skip_step("7d") - else: - self.step("7b") - initial_dut = await self.read_occ_attribute_expect_success(attribute=attributes.PhysicalContactOccupiedToUnoccupiedDelay) - - self.step("7c") - # write the new attribute value - diff_val = 9 - await self.write_single_attribute(attributes.PhysicalContactOccupiedToUnoccupiedDelay(diff_val)) - - self.step("7d") - await_sequence_of_reports(report_queue=attrib_listener.attribute_queue, endpoint_id=endpoint_id, attribute=cluster.Attributes.PhysicalContactOccupiedToUnoccupiedDelay, sequence=[ - diff_val], timeout_sec=post_prompt_settle_delay_seconds) - - -if __name__ == "__main__": - default_matter_test_main() +# +# Copyright (c) 2024 Project CHIP (Matter) Authors +# All rights reserved. +# +# 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. +# +# See https://github.com/project-chip/connectedhomeip/blob/master/docs/testing/python.md#defining-the-ci-test-arguments +# for details about the block below. +# +# === BEGIN CI TEST ARGUMENTS === +# test-runner-runs: run1 +# test-runner-run/run1/app: ${ALL_CLUSTERS_APP} +# test-runner-run/run1/factoryreset: True +# test-runner-run/run1/quiet: True +# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json +# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto --endpoint 1 --bool-arg simulate_occupancy:true +# === END CI TEST ARGUMENTS === + +# There are CI integration for the test cases below that implements manually controlling sensor device for +# the occupancy state ON/OFF change. +# [TC-OCC-3.1] test procedure step 3, 4 +# [TC-OCC-3.2] test precedure step 3a, 3c + +import logging +import time + +import chip.clusters as Clusters +from matter_testing_support import (AttributeValue, ClusterAttributeChangeAccumulator, MatterBaseTest, TestStep, async_test_body, + await_sequence_of_reports, default_matter_test_main) +from mobly import asserts + + +class TC_OCC_3_2(MatterBaseTest): + def setup_test(self): + super().setup_test() + self.is_ci = self.matter_test_config.global_test_params.get('simulate_occupancy', False) + + async def read_occ_attribute_expect_success(self, attribute): + cluster = Clusters.Objects.OccupancySensing + endpoint_id = self.matter_test_config.endpoint + return await self.read_single_attribute_check_success(endpoint=endpoint_id, cluster=cluster, attribute=attribute) + + def desc_TC_OCC_3_2(self) -> str: + return "[TC-OCC-3.2] Subscription Report Verification with server as DUT" + + def steps_TC_OCC_3_2(self) -> list[TestStep]: + steps = [ + TestStep(1, "Commission DUT to TH if not already done", is_commissioning=True), + TestStep(2, "TH establishes a wildcard subscription to all attributes on Occupancy Sensing Cluster on the endpoint under test. Subscription min interval = 0 and max interval = 30 seconds."), + TestStep("3a", "Prepare DUT to be unoccupied state."), + TestStep("3b", "TH reads DUT Occupancy attribute."), + TestStep("3c", "Trigger DUT to change the occupancy state."), + TestStep("3d", "TH awaits a ReportDataMessage containing an attribute report for DUT Occupancy attribute."), + TestStep("4a", "Check if DUT supports HoldTime attribute, If not supported, then stop and skip the rest of test cases."), + TestStep("4b", "TH writes HoldTimeMin to HoldTime attribute."), + TestStep("4c", "TH clears its report history and writes HoldTimeMax to HoldTime attribute."), + TestStep("4d", "TH awaits a ReportDataMessage containing an attribute report for DUT HoldTime attribute and all legacy attributes supported."), + ] + return steps + + def pics_TC_OCC_3_2(self) -> list[str]: + pics = [ + "OCC.S", + ] + return pics + + # Sends and out-of-band command to the all-clusters-app + def write_to_app_pipe(self, command): + # CI app pipe id creation + self.app_pipe = "/tmp/chip_all_clusters_fifo_" + if self.is_ci: + app_pid = self.matter_test_config.app_pid + if app_pid == 0: + asserts.fail("The --app-pid flag must be set when using named pipe") + self.app_pipe = self.app_pipe + str(app_pid) + + with open(self.app_pipe, "w") as app_pipe: + app_pipe.write(command + "\n") + # Delay for pipe command to be processed (otherwise tests are flaky) + time.sleep(0.001) + + @async_test_body + async def test_TC_OCC_3_2(self): + endpoint_id = self.matter_test_config.endpoint + node_id = self.dut_node_id + dev_ctrl = self.default_controller + + post_prompt_settle_delay_seconds = 10.0 + cluster = Clusters.Objects.OccupancySensing + attributes = cluster.Attributes + + self.step(1) # Commissioning already done + + self.step(2) + feature_map = await self.read_occ_attribute_expect_success(attribute=attributes.FeatureMap) + has_feature_pir = (feature_map & cluster.Bitmaps.Feature.kPassiveInfrared) != 0 + has_feature_ultrasonic = (feature_map & cluster.Bitmaps.Feature.kUltrasonic) != 0 + has_feature_contact = (feature_map & cluster.Bitmaps.Feature.kPhysicalContact) != 0 + + logging.info( + f"Feature map: 0x{feature_map:x}. PIR: {has_feature_pir}, US:{has_feature_ultrasonic}, PHY:{has_feature_contact}") + + attribute_list = await self.read_occ_attribute_expect_success(attribute=attributes.AttributeList) + has_pir_timing_attrib = attributes.PIROccupiedToUnoccupiedDelay.attribute_id in attribute_list + has_ultrasonic_timing_attrib = attributes.UltrasonicOccupiedToUnoccupiedDelay.attribute_id in attribute_list + has_contact_timing_attrib = attributes.PhysicalContactOccupiedToUnoccupiedDelay.attribute_id in attribute_list + logging.info(f"Attribute list: {attribute_list}") + logging.info(f"--> Has PIROccupiedToUnoccupiedDelay: {has_pir_timing_attrib}") + logging.info(f"--> Has UltrasonicOccupiedToUnoccupiedDelay: {has_ultrasonic_timing_attrib}") + logging.info(f"--> Has PhysicalContactOccupiedToUnoccupiedDelay: {has_contact_timing_attrib}") + + # min interval = 0, and max interval = 30 seconds + attrib_listener = ClusterAttributeChangeAccumulator(Clusters.Objects.OccupancySensing) + await attrib_listener.start(dev_ctrl, node_id, endpoint=endpoint_id, min_interval_sec=0, max_interval_sec=30) + + # add Namepiped to assimilate the manual sensor untrigger here + self.step("3a") + # CI call to trigger off + if self.is_ci: + self.write_to_app_pipe('{"Name":"SetOccupancy", "EndpointId": 1, "Occupancy": 0}') + else: + self.wait_for_user_input(prompt_msg="Type any letter and press ENTER after DUT goes back to unoccupied state.") + + self.step("3b") + initial_dut = await self.read_occ_attribute_expect_success(attribute=attributes.Occupancy) + asserts.assert_equal(initial_dut, 0, "Occupancy attribute is still detected state") + + # add Namepiped to assimilate the manual sensor trigger here + self.step("3c") + attrib_listener.reset() + + # CI call to trigger on + if self.is_ci: + self.write_to_app_pipe('{"Name":"SetOccupancy", "EndpointId": 1, "Occupancy": 1}') + else: + self.wait_for_user_input( + prompt_msg="Type any letter and press ENTER after the sensor occupancy is triggered and its occupancy state changed.") + + self.step("3d") + await_sequence_of_reports(report_queue=attrib_listener.attribute_queue, endpoint_id=endpoint_id, attribute=cluster.Attributes.Occupancy, sequence=[ + 1], timeout_sec=post_prompt_settle_delay_seconds) + + self.step("4a") + if attributes.HoldTime.attribute_id not in attribute_list: + logging.info("No HoldTime attribute supports. Terminate this test case") + self.skip_all_remaining_steps("4b") + + self.step("4b") + hold_time_limits_dut = await self.read_occ_attribute_expect_success(attribute=attributes.HoldTimeLimits) + hold_time_min = hold_time_limits_dut.holdTimeMin + hold_time_max = hold_time_limits_dut.holdTimeMax + await self.write_single_attribute(attributes.HoldTime(hold_time_min)) + hold_time_dut = await self.read_occ_attribute_expect_success(attribute=attributes.HoldTime) + asserts.assert_equal(hold_time_dut, hold_time_min, "HoldTime did not match written HoldTimeMin") + + # HoldTime may already have been HoldTimeMin, or not. Make sure we look only at subsequent reports. + attrib_listener.reset() + + self.step("4c") + await self.write_single_attribute(attributes.HoldTime(hold_time_max)) + hold_time_dut = await self.read_occ_attribute_expect_success(attribute=attributes.HoldTime) + asserts.assert_equal(hold_time_dut, hold_time_max, "HoldTime did not match written HoldTimeMax") + + self.step("4d") + has_no_legacy_features = ((not has_feature_pir) and (not has_feature_ultrasonic) and (not has_feature_contact)) + + expect_legacy_pir_timing = has_pir_timing_attrib and (has_feature_pir or has_no_legacy_features) + expect_legacy_us_timing = has_ultrasonic_timing_attrib and has_feature_ultrasonic + expect_legacy_phy_timing = has_contact_timing_attrib and has_feature_contact + + # Build list of expectations based on attributes present. + all_expected_final_values = [AttributeValue(endpoint_id, attribute=cluster.Attributes.HoldTime, value=hold_time_max)] + if expect_legacy_pir_timing: + all_expected_final_values.append(AttributeValue( + endpoint_id, attribute=cluster.Attributes.PIROccupiedToUnoccupiedDelay, value=hold_time_max)) + if expect_legacy_us_timing: + all_expected_final_values.append(AttributeValue( + endpoint_id, attribute=cluster.Attributes.UltrasonicOccupiedToUnoccupiedDelay, value=hold_time_max)) + if expect_legacy_phy_timing: + all_expected_final_values.append(AttributeValue( + endpoint_id, attribute=cluster.Attributes.PhysicalContactOccupiedToUnoccupiedDelay, value=hold_time_max)) + + # Wait for the reports to come. + attrib_listener.await_all_final_values_reported(all_expected_final_values, timeout_sec=post_prompt_settle_delay_seconds) + + +if __name__ == "__main__": + default_matter_test_main() diff --git a/src/python_testing/matter_testing_support.py b/src/python_testing/matter_testing_support.py index 20c33a0d36b785..bd1b32e671ad79 100644 --- a/src/python_testing/matter_testing_support.py +++ b/src/python_testing/matter_testing_support.py @@ -28,6 +28,7 @@ import re import sys import textwrap +import threading import time import typing import uuid @@ -37,7 +38,7 @@ from datetime import datetime, timedelta, timezone from enum import Enum, IntFlag from functools import partial -from typing import Any, List, Optional, Tuple +from typing import Any, Iterable, List, Optional, Tuple from chip.tlv import float32, uint @@ -263,6 +264,7 @@ def __call__(self, res: EventReadResult, transaction: SubscriptionTransaction): def wait_for_event_report(self, expected_event: ClusterObjects.ClusterEvent, timeout_sec: float = 10.0) -> Any: """This function allows a test script to block waiting for the specific event to be the next event to arrive within a timeout (specified in seconds). It returns the event data so that the values can be checked.""" + logging.info(f"Waiting for {expected_event} for {timeout_sec:.1f} seconds") try: res = self._q.get(block=True, timeout=timeout_sec) except queue.Empty: @@ -270,6 +272,7 @@ def wait_for_event_report(self, expected_event: ClusterObjects.ClusterEvent, tim asserts.assert_equal(res.Header.ClusterId, expected_event.cluster_id, "Expected cluster ID not found in event report") asserts.assert_equal(res.Header.EventId, expected_event.event_id, "Expected event ID not found in event report") + logging.info(f"Successfully waited for {expected_event}") return res.Data def wait_for_event_expect_no_report(self, timeout_sec: float = 10.0): @@ -343,6 +346,14 @@ def clear_queue(report_queue: queue.Queue): break +@dataclass +class AttributeValue: + endpoint_id: int + attribute: ClusterObjects.ClusterAttributeDescriptor + value: Any + timestamp_utc: Optional[datetime] = None + + def await_sequence_of_reports(report_queue: queue.Queue, endpoint_id: int, attribute: TypedAttributePath, sequence: list[Any], timeout_sec: float): """Given a queue.Queue hooked-up to an attribute change accumulator, await a given expected sequence of attribute reports. @@ -370,6 +381,7 @@ def await_sequence_of_reports(report_queue: queue.Queue, endpoint_id: int, attri while time_remaining > 0: expected_value = sequence[sequence_idx] logging.info(f"Expecting value {expected_value} for attribute {attribute} on endpoint {endpoint_id}") + logging.info(f"Waiting for {timeout_sec:.1f} seconds for all reports.") try: item: AttributeValue = report_queue.get(block=True, timeout=time_remaining) @@ -382,7 +394,7 @@ def await_sequence_of_reports(report_queue: queue.Queue, endpoint_id: int, attri sequence_idx += 1 else: asserts.assert_equal(item.value, expected_value, - msg="Did not get expected attribute value in correct sequence.") + msg=f"Did not get expected attribute value in correct sequence. Sequence so far: {actual_values}") # We are done waiting when we have accumulated all results. if sequence_idx == len(sequence): @@ -398,29 +410,25 @@ def await_sequence_of_reports(report_queue: queue.Queue, endpoint_id: int, attri asserts.fail(f"Did not get full sequence {sequence} in {timeout_sec:.1f} seconds. Got {actual_values} before time-out.") -@dataclass -class AttributeValue: - endpoint_id: int - attribute: ClusterObjects.ClusterAttributeDescriptor - value: Any - timestamp_utc: datetime - - class ClusterAttributeChangeAccumulator: def __init__(self, expected_cluster: ClusterObjects.Cluster): self._expected_cluster = expected_cluster self._subscription = None + self._lock = threading.Lock() + self._q = queue.Queue() self.reset() def reset(self): - self._attribute_report_counts = {} - attrs = [cls for name, cls in inspect.getmembers(self._expected_cluster.Attributes) if inspect.isclass( - cls) and issubclass(cls, ClusterObjects.ClusterAttributeDescriptor)] - self._attribute_reports = {} - for a in attrs: - self._attribute_report_counts[a] = 0 - self._attribute_reports[a] = [] - self._q = queue.Queue() + with self._lock: + self._attribute_report_counts = {} + attrs = [cls for name, cls in inspect.getmembers(self._expected_cluster.Attributes) if inspect.isclass( + cls) and issubclass(cls, ClusterObjects.ClusterAttributeDescriptor)] + self._attribute_reports = {} + for a in attrs: + self._attribute_report_counts[a] = 0 + self._attribute_reports[a] = [] + + self.flush_reports() async def start(self, dev_ctrl, node_id: int, endpoint: int, fabric_filtered: bool = False, min_interval_sec: int = 0, max_interval_sec: int = 5) -> Any: """This starts a subscription for attributes on the specified node_id and endpoint. The cluster is specified when the class instance is created.""" @@ -443,8 +451,56 @@ def __call__(self, path: TypedAttributePath, transaction: SubscriptionTransactio value=data, timestamp_utc=datetime.now(timezone.utc)) logging.info(f"Got subscription report for {path.AttributeType}: {data}") self._q.put(value) - self._attribute_report_counts[path.AttributeType] += 1 - self._attribute_reports[path.AttributeType].append(value) + with self._lock: + self._attribute_report_counts[path.AttributeType] += 1 + self._attribute_reports[path.AttributeType].append(value) + + def await_all_final_values_reported(self, expected_final_values: Iterable[AttributeValue], timeout_sec: float = 1.0): + """Expect that every `expected_final_value` report is the last value reported for the given attribute, ignoring timestamps. + + Waits for at least `timeout_sec` seconds. + + This is a form of barrier for a set of attribute changes that should all happen together for an action. + """ + start_time = time.time() + elapsed = 0.0 + time_remaining = timeout_sec + + last_report_matches: dict[int, bool] = {idx: False for idx, _ in enumerate(expected_final_values)} + + for element in expected_final_values: + logging.info( + f"--> Expecting report for value {element.value} for attribute {element.attribute} on endpoint {element.endpoint_id}") + logging.info(f"Waiting for {timeout_sec:.1f} seconds for all reports.") + + while time_remaining > 0: + # Snapshot copy at the beginning of the loop. This is thread-safe based on the design. + all_reports = self._attribute_reports + + # Recompute all last-value matches + for expected_idx, expected_element in enumerate(expected_final_values): + last_value = None + for report in all_reports.get(expected_element.attribute, []): + if report.endpoint_id == expected_element.endpoint_id: + last_value = report.value + + last_report_matches[expected_idx] = (last_value is not None and last_value == expected_element.value) + + # Determine if all were met + if all(last_report_matches.values()): + logging.info("Found all expected reports were true.") + return + + elapsed = time.time() - start_time + time_remaining = timeout_sec - elapsed + time.sleep(0.1) + + # If we reach here, there was no early return and we failed to find all the values. + logging.error("Reached time-out without finding all expected report values.") + logging.info("Values found:") + for expected_idx, expected_element in enumerate(expected_final_values): + logging.info(f" -> {expected_element} found: {last_report_matches.get(expected_idx)}") + asserts.fail("Did not find all expected last report values before time-out") @property def attribute_queue(self) -> queue.Queue: @@ -452,11 +508,13 @@ def attribute_queue(self) -> queue.Queue: @property def attribute_report_counts(self) -> dict[ClusterObjects.ClusterAttributeDescriptor, int]: - return self._attribute_report_counts + with self._lock: + return self._attribute_report_counts @property def attribute_reports(self) -> dict[ClusterObjects.ClusterAttributeDescriptor, AttributeValue]: - return self._attribute_reports + with self._lock: + return self._attribute_reports.copy() def get_last_report(self) -> Optional[Any]: """Flush entire queue, returning last (newest) report only.""" From c14a370535c4a3f30277761cc4ce277f84ba8949 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Thu, 29 Aug 2024 02:27:28 +0200 Subject: [PATCH 07/13] Allow to specify run delay between app and test script (#35216) * Allow to specify run delay between app and test script * Restyled by prettier-markdown * Explain the use case of the delay --------- Co-authored-by: Restyled.io --- docs/testing/python.md | 10 ++++++++++ scripts/tests/run_python_test.py | 13 ++++++++++--- .../metadata_parser/metadata.py | 5 +++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/docs/testing/python.md b/docs/testing/python.md index 354490c896c445..3b5c8ffc74abf7 100644 --- a/docs/testing/python.md +++ b/docs/testing/python.md @@ -635,9 +635,19 @@ markers must be present. - `test-runner-run//script-args`: Specifies the arguments to be passed to the test script. + - Example: `--storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto` +- `test-runner-run//script-start-delay`: Specifies the number + of seconds to wait before starting the test script. This parameter can be + used to allow the application to initialize itself properly before the test + script will try to commission it (e.g. in case if the application needs to + be commissioned to some other controller first). By default, the delay is 0 + seconds. + + - Example: `10` + This structured format ensures that all necessary configurations are clearly defined and easily understood, allowing for consistent and reliable test execution. diff --git a/scripts/tests/run_python_test.py b/scripts/tests/run_python_test.py index 61ec274bfc1c64..eb8f0633c0a91a 100755 --- a/scripts/tests/run_python_test.py +++ b/scripts/tests/run_python_test.py @@ -87,11 +87,14 @@ def DumpProgramOutputToQueue(thread_list: typing.List[threading.Thread], tag: st 'mobile-device-test.py'), help='Test script to use.') @click.option("--script-args", type=str, default='', help='Script arguments, can use placeholders like {SCRIPT_BASE_NAME}.') +@click.option("--script-start-delay", type=int, default=0, + help='Delay in seconds before starting the script.') @click.option("--script-gdb", is_flag=True, help='Run script through gdb') @click.option("--quiet", is_flag=True, help="Do not print output from passing tests. Use this flag in CI to keep github log sizes manageable.") @click.option("--load-from-env", default=None, help="YAML file that contains values for environment variables.") -def main(app: str, factoryreset: bool, factoryreset_app_only: bool, app_args: str, script: str, script_args: str, script_gdb: bool, quiet: bool, load_from_env): +def main(app: str, factoryreset: bool, factoryreset_app_only: bool, app_args: str, + script: str, script_args: str, script_start_delay: int, script_gdb: bool, quiet: bool, load_from_env): if load_from_env: reader = MetadataReader(load_from_env) runs = reader.parse_script(script) @@ -103,6 +106,7 @@ def main(app: str, factoryreset: bool, factoryreset_app_only: bool, app_args: st app=app, app_args=app_args, script_args=script_args, + script_start_delay=script_start_delay, factoryreset=factoryreset, factoryreset_app_only=factoryreset_app_only, script_gdb=script_gdb, @@ -117,10 +121,11 @@ def main(app: str, factoryreset: bool, factoryreset_app_only: bool, app_args: st for run in runs: print(f"Executing {run.py_script_path.split('/')[-1]} {run.run}") main_impl(run.app, run.factoryreset, run.factoryreset_app_only, run.app_args, - run.py_script_path, run.script_args, run.script_gdb, run.quiet) + run.py_script_path, run.script_args, run.script_start_delay, run.script_gdb, run.quiet) -def main_impl(app: str, factoryreset: bool, factoryreset_app_only: bool, app_args: str, script: str, script_args: str, script_gdb: bool, quiet: bool): +def main_impl(app: str, factoryreset: bool, factoryreset_app_only: bool, app_args: str, + script: str, script_args: str, script_start_delay: int, script_gdb: bool, quiet: bool): app_args = app_args.replace('{SCRIPT_BASE_NAME}', os.path.splitext(os.path.basename(script))[0]) script_args = script_args.replace('{SCRIPT_BASE_NAME}', os.path.splitext(os.path.basename(script))[0]) @@ -175,6 +180,8 @@ def main_impl(app: str, factoryreset: bool, factoryreset_app_only: bool, app_arg DumpProgramOutputToQueue( log_cooking_threads, Fore.GREEN + "APP " + Style.RESET_ALL, app_process, stream_output, log_queue) + time.sleep(script_start_delay) + script_command = [script, "--paa-trust-store-path", os.path.join(DEFAULT_CHIP_ROOT, MATTER_DEVELOPMENT_PAA_ROOT_CERTS), '--log-format', '%(message)s', "--app-pid", str(app_pid)] + shlex.split(script_args) diff --git a/src/python_testing/matter_testing_infrastructure/metadata_parser/metadata.py b/src/python_testing/matter_testing_infrastructure/metadata_parser/metadata.py index 050f20f9920d6b..f16d3e9803e825 100644 --- a/src/python_testing/matter_testing_infrastructure/metadata_parser/metadata.py +++ b/src/python_testing/matter_testing_infrastructure/metadata_parser/metadata.py @@ -33,6 +33,7 @@ class Metadata: app: str app_args: str script_args: str + script_start_delay: int = 0 factoryreset: bool = False factoryreset_app_only: bool = False script_gdb: bool = False @@ -60,6 +61,9 @@ def copy_from_dict(self, attr_dict: Dict[str, Any]) -> None: if "script-args" in attr_dict: self.script_args = attr_dict["script-args"] + if "script-start-delay" in attr_dict: + self.script_start_delay = int(attr_dict["script-start-delay"]) + if "py_script_path" in attr_dict: self.py_script_path = attr_dict["py_script_path"] @@ -187,6 +191,7 @@ def parse_script(self, py_script_path: str) -> List[Metadata]: app=attr.get("app", ""), app_args=attr.get("app_args", ""), script_args=attr.get("script_args", ""), + script_start_delay=int(attr.get("script_start_delay", 0)), factoryreset=bool(attr.get("factoryreset", False)), factoryreset_app_only=bool(attr.get("factoryreset_app_only", False)), script_gdb=bool(attr.get("script_gdb", False)), From d2569264f9f8204765312fe3d91f39dd7b13b4cd Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Thu, 29 Aug 2024 08:55:55 +0200 Subject: [PATCH 08/13] [Fabric-Sync] Allow RPC ports customization in example apps (#35194) * [Fabric-Sync] Allow RPC ports customization in example apps * Restyled by clang-format * Rename SetRpcClientPort to SetRpcRemoteServerPort --------- Co-authored-by: Restyled.io --- .../interactive/InteractiveCommands.cpp | 6 +- .../interactive/InteractiveCommands.h | 16 +++++- examples/fabric-admin/main.cpp | 9 --- examples/fabric-admin/rpc/RpcClient.cpp | 8 ++- examples/fabric-admin/rpc/RpcClient.h | 16 +++--- examples/fabric-admin/rpc/RpcServer.h | 2 - .../fabric-bridge-app/linux/RpcClient.cpp | 8 ++- .../linux/include/RpcClient.h | 12 ++-- .../linux/include/RpcServer.h | 2 - examples/fabric-bridge-app/linux/main.cpp | 55 +++++++++++++++++-- 10 files changed, 98 insertions(+), 36 deletions(-) diff --git a/examples/fabric-admin/commands/interactive/InteractiveCommands.cpp b/examples/fabric-admin/commands/interactive/InteractiveCommands.cpp index 22b634145ab93f..aabcb09f2314b2 100644 --- a/examples/fabric-admin/commands/interactive/InteractiveCommands.cpp +++ b/examples/fabric-admin/commands/interactive/InteractiveCommands.cpp @@ -31,6 +31,7 @@ #if defined(PW_RPC_ENABLED) #include +#include #endif using namespace chip; @@ -116,7 +117,7 @@ void ENFORCE_FORMAT(3, 0) LoggingCallback(const char * module, uint8_t category, #if defined(PW_RPC_ENABLED) void AttemptRpcClientConnect(System::Layer * systemLayer, void * appState) { - if (InitRpcClient(kFabricBridgeServerPort) == CHIP_NO_ERROR) + if (StartRpcClient() == CHIP_NO_ERROR) { ChipLogProgress(NotSpecified, "Connected to Fabric-Bridge"); } @@ -196,6 +197,9 @@ CHIP_ERROR InteractiveStartCommand::RunCommand() } #if defined(PW_RPC_ENABLED) + SetRpcRemoteServerPort(mFabricBridgeServerPort.Value()); + InitRpcServer(mLocalServerPort.Value()); + ChipLogProgress(NotSpecified, "PW_RPC initialized."); DeviceLayer::PlatformMgr().ScheduleWork(ExecuteDeferredConnect, 0); #endif diff --git a/examples/fabric-admin/commands/interactive/InteractiveCommands.h b/examples/fabric-admin/commands/interactive/InteractiveCommands.h index e3d28080657e26..648984d1ce94a5 100644 --- a/examples/fabric-admin/commands/interactive/InteractiveCommands.h +++ b/examples/fabric-admin/commands/interactive/InteractiveCommands.h @@ -24,6 +24,9 @@ #include +constexpr uint16_t kFabricBridgeServerPort = 33002; +constexpr uint16_t kFabricLocalServerPort = 33001; + class Commands; class InteractiveCommand : public CHIPCommand @@ -55,7 +58,13 @@ class InteractiveStartCommand : public InteractiveCommand InteractiveStartCommand(Commands * commandsHandler, CredentialIssuerCommands * credsIssuerConfig) : InteractiveCommand("start", commandsHandler, "Start an interactive shell that can then run other commands.", credsIssuerConfig) - {} + { +#if defined(PW_RPC_ENABLED) + AddArgument("fabric-bridge-server-port", 0, UINT16_MAX, &mFabricBridgeServerPort, + "The fabric-bridge RPC port number to connect to."); + AddArgument("local-server-port", 0, UINT16_MAX, &mLocalServerPort, "The port number for local RPC server."); +#endif + } /////////// CHIPCommand Interface ///////// CHIP_ERROR RunCommand() override; @@ -63,6 +72,11 @@ class InteractiveStartCommand : public InteractiveCommand private: char * GetCommand(char * command); std::string GetHistoryFilePath() const; + +#if defined(PW_RPC_ENABLED) + chip::Optional mFabricBridgeServerPort{ kFabricBridgeServerPort }; + chip::Optional mLocalServerPort{ kFabricLocalServerPort }; +#endif }; void PushCommand(const std::string & command); diff --git a/examples/fabric-admin/main.cpp b/examples/fabric-admin/main.cpp index c7f9089c120c8d..5768abf496fefc 100644 --- a/examples/fabric-admin/main.cpp +++ b/examples/fabric-admin/main.cpp @@ -28,19 +28,10 @@ #include #include -#if defined(PW_RPC_ENABLED) -#include -#endif - using namespace chip; void ApplicationInit() { -#if defined(PW_RPC_ENABLED) - InitRpcServer(kFabricAdminServerPort); - ChipLogProgress(NotSpecified, "PW_RPC initialized."); -#endif - DeviceMgr().Init(); } diff --git a/examples/fabric-admin/rpc/RpcClient.cpp b/examples/fabric-admin/rpc/RpcClient.cpp index 29fc2b1ea9d73c..df7a475071c4ba 100644 --- a/examples/fabric-admin/rpc/RpcClient.cpp +++ b/examples/fabric-admin/rpc/RpcClient.cpp @@ -117,9 +117,13 @@ void RpcCompletedWithEmptyResponse(const pw_protobuf_Empty & response, pw::Statu } // namespace -CHIP_ERROR InitRpcClient(uint16_t rpcServerPort) +void SetRpcRemoteServerPort(uint16_t port) +{ + rpc::client::SetRpcServerPort(port); +} + +CHIP_ERROR StartRpcClient() { - rpc::client::SetRpcServerPort(rpcServerPort); return rpc::client::StartPacketProcessing(); } diff --git a/examples/fabric-admin/rpc/RpcClient.h b/examples/fabric-admin/rpc/RpcClient.h index 6dd2b5b20bbd90..41d37cf85191b8 100644 --- a/examples/fabric-admin/rpc/RpcClient.h +++ b/examples/fabric-admin/rpc/RpcClient.h @@ -22,17 +22,19 @@ #include "fabric_bridge_service/fabric_bridge_service.rpc.pb.h" -constexpr uint16_t kFabricBridgeServerPort = 33002; - /** - * @brief Initializes the RPC client with the specified server port. + * @brief Sets the RPC server port to which the RPC client will connect. * - * This function sets the RPC server port and starts packet processing for the RPC client. + * @param port The port number. + */ +void SetRpcRemoteServerPort(uint16_t port); + +/** + * @brief Starts packet processing for the RPC client. * - * @param rpcServerPort The port number on which the RPC server is running. - * @return CHIP_NO_ERROR on successful initialization, or an appropriate CHIP_ERROR on failure. + * @return CHIP_NO_ERROR on successful start, or an appropriate CHIP_ERROR on failure. */ -CHIP_ERROR InitRpcClient(uint16_t rpcServerPort); +CHIP_ERROR StartRpcClient(); /** * @brief Adds a synchronized device to the RPC client. diff --git a/examples/fabric-admin/rpc/RpcServer.h b/examples/fabric-admin/rpc/RpcServer.h index bc03bc0ac4abd3..d283c0db5a0248 100644 --- a/examples/fabric-admin/rpc/RpcServer.h +++ b/examples/fabric-admin/rpc/RpcServer.h @@ -18,6 +18,4 @@ #pragma once -constexpr uint16_t kFabricAdminServerPort = 33001; - void InitRpcServer(uint16_t rpcServerPort); diff --git a/examples/fabric-bridge-app/linux/RpcClient.cpp b/examples/fabric-bridge-app/linux/RpcClient.cpp index 633b652b74180b..3ef88e8ed7fec5 100644 --- a/examples/fabric-bridge-app/linux/RpcClient.cpp +++ b/examples/fabric-bridge-app/linux/RpcClient.cpp @@ -107,9 +107,13 @@ void RpcCompletedWithEmptyResponse(const pw_protobuf_Empty & response, pw::Statu } // namespace -CHIP_ERROR InitRpcClient(uint16_t rpcServerPort) +void SetRpcRemoteServerPort(uint16_t port) +{ + rpc::client::SetRpcServerPort(port); +} + +CHIP_ERROR StartRpcClient() { - rpc::client::SetRpcServerPort(rpcServerPort); return rpc::client::StartPacketProcessing(); } diff --git a/examples/fabric-bridge-app/linux/include/RpcClient.h b/examples/fabric-bridge-app/linux/include/RpcClient.h index f183b2ed38aa2b..2455bc56c8400f 100644 --- a/examples/fabric-bridge-app/linux/include/RpcClient.h +++ b/examples/fabric-bridge-app/linux/include/RpcClient.h @@ -21,17 +21,21 @@ #include #include -constexpr uint16_t kFabricAdminServerPort = 33001; +/** + * Sets the RPC server port to which the RPC client will connect. + * + * @param port The port number. + */ +void SetRpcRemoteServerPort(uint16_t port); /** - * Initializes the RPC client by setting the server port and starting packet processing. + * Starts packet processing for the RPC client. * - * @param rpcServerPort The port number of the RPC server. * @return CHIP_ERROR An error code indicating the success or failure of the initialization process. * - CHIP_NO_ERROR: Initialization was successful. * - Other error codes indicating specific failure reasons. */ -CHIP_ERROR InitRpcClient(uint16_t rpcServerPort); +CHIP_ERROR StartRpcClient(); /** * Opens a commissioning window for a specified node using setup PIN (passcode). diff --git a/examples/fabric-bridge-app/linux/include/RpcServer.h b/examples/fabric-bridge-app/linux/include/RpcServer.h index f86858b19bdfe3..d283c0db5a0248 100644 --- a/examples/fabric-bridge-app/linux/include/RpcServer.h +++ b/examples/fabric-bridge-app/linux/include/RpcServer.h @@ -18,6 +18,4 @@ #pragma once -constexpr uint16_t kFabricBridgeServerPort = 33002; - void InitRpcServer(uint16_t rpcServerPort); diff --git a/examples/fabric-bridge-app/linux/main.cpp b/examples/fabric-bridge-app/linux/main.cpp index 08c70782de16ab..d08673ad12a7a6 100644 --- a/examples/fabric-bridge-app/linux/main.cpp +++ b/examples/fabric-bridge-app/linux/main.cpp @@ -16,6 +16,10 @@ * limitations under the License. */ +#include +#include +#include + #include #include "BridgedAdministratorCommissioning.h" @@ -28,15 +32,13 @@ #include #include #include +#include #if defined(PW_RPC_FABRIC_BRIDGE_SERVICE) && PW_RPC_FABRIC_BRIDGE_SERVICE #include "RpcClient.h" #include "RpcServer.h" #endif -#include -#include - // This is declared here and not in a header because zap/embr assumes all clusters // are defined in a static endpoint in the .zap file. From there, the codegen will // automatically use PluginApplicationCallbacksHeader.jinja to declare and call @@ -59,8 +61,48 @@ constexpr uint16_t kPollIntervalMs = 100; constexpr uint16_t kRetryIntervalS = 3; #endif +uint16_t gFabricAdminServerPort = 33001; +uint16_t gLocalServerPort = 33002; + BridgedDeviceBasicInformationImpl gBridgedDeviceBasicInformationAttributes; +constexpr uint16_t kOptionFabricAdminServerPortNumber = 0xFF01; +constexpr uint16_t kOptionLocalServerPortNumber = 0xFF02; + +ArgParser::OptionDef sProgramCustomOptionDefs[] = { + { "fabric-admin-server-port", ArgParser::kArgumentRequired, kOptionFabricAdminServerPortNumber }, + { "local-server-port", ArgParser::kArgumentRequired, kOptionLocalServerPortNumber }, + {}, +}; + +const char sProgramCustomOptionHelp[] = " --fabric-admin-server-port \n" + " The fabric-admin RPC port number to connect to (default: 33001).\n" + " --local-server-port \n" + " The port number for local RPC server (default: 33002).\n" + "\n"; + +bool HandleCustomOption(const char * aProgram, ArgParser::OptionSet * aOptions, int aIdentifier, const char * aName, + const char * aValue) +{ + switch (aIdentifier) + { + case kOptionFabricAdminServerPortNumber: + gFabricAdminServerPort = atoi(aValue); + break; + case kOptionLocalServerPortNumber: + gLocalServerPort = atoi(aValue); + break; + default: + ArgParser::PrintArgError("%s: INTERNAL ERROR: Unhandled option: %s\n", aProgram, aName); + return false; + } + + return true; +} + +ArgParser::OptionSet sProgramCustomOptions = { HandleCustomOption, sProgramCustomOptionDefs, "GENERAL OPTIONS", + sProgramCustomOptionHelp }; + bool KeyboardHit() { int bytesWaiting; @@ -105,7 +147,7 @@ void BridgePollingThread() #if defined(PW_RPC_FABRIC_BRIDGE_SERVICE) && PW_RPC_FABRIC_BRIDGE_SERVICE void AttemptRpcClientConnect(System::Layer * systemLayer, void * appState) { - if (InitRpcClient(kFabricAdminServerPort) == CHIP_NO_ERROR) + if (StartRpcClient() == CHIP_NO_ERROR) { ChipLogProgress(NotSpecified, "Connected to Fabric-Admin"); } @@ -258,7 +300,8 @@ void ApplicationInit() AttributeAccessInterfaceRegistry::Instance().Register(&gBridgedDeviceBasicInformationAttributes); #if defined(PW_RPC_FABRIC_BRIDGE_SERVICE) && PW_RPC_FABRIC_BRIDGE_SERVICE - InitRpcServer(kFabricBridgeServerPort); + SetRpcRemoteServerPort(gFabricAdminServerPort); + InitRpcServer(gLocalServerPort); AttemptRpcClientConnect(&DeviceLayer::SystemLayer(), nullptr); #endif @@ -285,7 +328,7 @@ void ApplicationShutdown() int main(int argc, char * argv[]) { - if (ChipLinuxAppInit(argc, argv) != 0) + if (ChipLinuxAppInit(argc, argv, &sProgramCustomOptions) != 0) { return -1; } From 203f03a6169f7a980445ac1c0a9dd2c905d86c05 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Thu, 29 Aug 2024 01:25:52 -0700 Subject: [PATCH 09/13] Update CommissionerControlCluster xml definition to align with spec (#35272) * Update CommissionerControlCluster xml defination to align with spec * Clean up commissioner-control-server --- .../fabric-bridge-app.matter | 2 - .../linux/CommissionerControl.cpp | 3 +- .../linux/include/CommissionerControl.h | 3 +- .../commissioner-control-server.cpp | 20 ++++----- .../commissioner-control-server.h | 42 +------------------ .../chip/commissioner-control-cluster.xml | 5 +-- .../data_model/controller-clusters.matter | 2 - .../chip/devicecontroller/ChipClusters.java | 14 ++----- .../devicecontroller/ClusterIDMapping.java | 2 +- .../devicecontroller/ClusterInfoMapping.java | 12 ------ .../clusters/CommissionerControlCluster.kt | 8 ---- .../python/chip/clusters/CHIPClusters.py | 2 - .../python/chip/clusters/Objects.py | 4 -- .../zap-generated/MTRCommandPayloadsObjc.h | 4 -- .../zap-generated/MTRCommandPayloadsObjc.mm | 20 +-------- .../zap-generated/cluster-objects.cpp | 10 ----- .../zap-generated/cluster-objects.h | 6 --- .../zap-generated/cluster/Commands.h | 2 - .../zap-generated/cluster/Commands.h | 20 --------- 19 files changed, 17 insertions(+), 164 deletions(-) diff --git a/examples/fabric-bridge-app/fabric-bridge-common/fabric-bridge-app.matter b/examples/fabric-bridge-app/fabric-bridge-common/fabric-bridge-app.matter index 3fb7436adbfe5a..22f5ae6ccda3be 100644 --- a/examples/fabric-bridge-app/fabric-bridge-common/fabric-bridge-app.matter +++ b/examples/fabric-bridge-app/fabric-bridge-common/fabric-bridge-app.matter @@ -1730,8 +1730,6 @@ provisional cluster CommissionerControl = 1873 { request struct CommissionNodeRequest { int64u requestId = 0; int16u responseTimeoutSeconds = 1; - optional octet_string ipAddress = 2; - optional int16u port = 3; } response struct ReverseOpenCommissioningWindow = 2 { diff --git a/examples/fabric-bridge-app/linux/CommissionerControl.cpp b/examples/fabric-bridge-app/linux/CommissionerControl.cpp index 0466ab8b711b69..1e67dc0f30c1b4 100644 --- a/examples/fabric-bridge-app/linux/CommissionerControl.cpp +++ b/examples/fabric-bridge-app/linux/CommissionerControl.cpp @@ -166,8 +166,7 @@ CHIP_ERROR CommissionerControlDelegate::GetCommissioningWindowParams(Commissioni return CHIP_NO_ERROR; } -CHIP_ERROR CommissionerControlDelegate::HandleCommissionNode(const CommissioningWindowParams & params, - const Optional & ipAddress, const Optional & port) +CHIP_ERROR CommissionerControlDelegate::HandleCommissionNode(const CommissioningWindowParams & params) { CHIP_ERROR err = CHIP_NO_ERROR; diff --git a/examples/fabric-bridge-app/linux/include/CommissionerControl.h b/examples/fabric-bridge-app/linux/include/CommissionerControl.h index 486668ffa212f7..6e1d9c69d54bf9 100644 --- a/examples/fabric-bridge-app/linux/include/CommissionerControl.h +++ b/examples/fabric-bridge-app/linux/include/CommissionerControl.h @@ -32,8 +32,7 @@ class CommissionerControlDelegate : public Delegate CHIP_ERROR HandleCommissioningApprovalRequest(const CommissioningApprovalRequest & request) override; CHIP_ERROR ValidateCommissionNodeCommand(NodeId clientNodeId, uint64_t requestId) override; CHIP_ERROR GetCommissioningWindowParams(CommissioningWindowParams & outParams) override; - CHIP_ERROR HandleCommissionNode(const CommissioningWindowParams & params, const Optional & ipAddress, - const Optional & port) override; + CHIP_ERROR HandleCommissionNode(const CommissioningWindowParams & params) override; ~CommissionerControlDelegate() = default; diff --git a/src/app/clusters/commissioner-control-server/commissioner-control-server.cpp b/src/app/clusters/commissioner-control-server/commissioner-control-server.cpp index 4df397326894ed..80d65b2c4d16fc 100644 --- a/src/app/clusters/commissioner-control-server/commissioner-control-server.cpp +++ b/src/app/clusters/commissioner-control-server/commissioner-control-server.cpp @@ -64,14 +64,14 @@ void AddReverseOpenCommissioningWindowResponse(CommandHandler * commandObj, cons void RunDeferredCommissionNode(intptr_t commandArg) { - auto * info = reinterpret_cast(commandArg); + auto * params = reinterpret_cast(commandArg); Clusters::CommissionerControl::Delegate * delegate = Clusters::CommissionerControl::CommissionerControlServer::Instance().GetDelegate(); if (delegate != nullptr) { - CHIP_ERROR err = delegate->HandleCommissionNode(info->params, info->ipAddress.GetIPAddress(), info->port); + CHIP_ERROR err = delegate->HandleCommissionNode(*params); if (err != CHIP_NO_ERROR) { ChipLogError(Zcl, "HandleCommissionNode error: %" CHIP_ERROR_FORMAT, err.Format()); @@ -82,7 +82,7 @@ void RunDeferredCommissionNode(intptr_t commandArg) ChipLogError(Zcl, "No delegate available for HandleCommissionNode"); } - delete info; + delete params; } } // namespace @@ -234,31 +234,27 @@ bool emberAfCommissionerControlClusterCommissionNodeCallback( auto requestId = commandData.requestId; - auto commissionNodeInfo = std::make_unique(); + auto commissioningWindowParams = std::make_unique(); Clusters::CommissionerControl::Delegate * delegate = Clusters::CommissionerControl::CommissionerControlServer::Instance().GetDelegate(); VerifyOrExit(delegate != nullptr, err = CHIP_ERROR_INCORRECT_STATE); - // Set IP address and port in the CommissionNodeInfo struct - commissionNodeInfo->port = commandData.port; - err = commissionNodeInfo->ipAddress.SetIPAddress(commandData.ipAddress); - SuccessOrExit(err); - // Validate the commission node command. err = delegate->ValidateCommissionNodeCommand(sourceNodeId, requestId); SuccessOrExit(err); // Populate the parameters for the commissioning window - err = delegate->GetCommissioningWindowParams(commissionNodeInfo->params); + err = delegate->GetCommissioningWindowParams(*commissioningWindowParams); SuccessOrExit(err); // Add the response for the commissioning window. - AddReverseOpenCommissioningWindowResponse(commandObj, commandPath, commissionNodeInfo->params); + AddReverseOpenCommissioningWindowResponse(commandObj, commandPath, *commissioningWindowParams); // Schedule the deferred reverse commission node task - DeviceLayer::PlatformMgr().ScheduleWork(RunDeferredCommissionNode, reinterpret_cast(commissionNodeInfo.release())); + DeviceLayer::PlatformMgr().ScheduleWork(RunDeferredCommissionNode, + reinterpret_cast(commissioningWindowParams.release())); exit: if (err != CHIP_NO_ERROR) diff --git a/src/app/clusters/commissioner-control-server/commissioner-control-server.h b/src/app/clusters/commissioner-control-server/commissioner-control-server.h index 60a4e81a93c3af..e63769de1d0a3e 100644 --- a/src/app/clusters/commissioner-control-server/commissioner-control-server.h +++ b/src/app/clusters/commissioner-control-server/commissioner-control-server.h @@ -46,43 +46,6 @@ struct CommissioningWindowParams ByteSpan salt; }; -class ProtectedIPAddress -{ -public: - const Optional GetIPAddress() { return ipAddress; } - - CHIP_ERROR SetIPAddress(const Optional & address) - { - if (!address.HasValue()) - { - ipAddress.ClearValue(); - return CHIP_NO_ERROR; - } - - const ByteSpan & addressSpan = address.Value(); - size_t addressLength = addressSpan.size(); - if (addressLength != 4 && addressLength != 16) - { - return CHIP_ERROR_INVALID_ARGUMENT; - } - - memcpy(ipAddressBuffer, addressSpan.data(), addressLength); - ipAddress.SetValue(ByteSpan(ipAddressBuffer, addressLength)); - return CHIP_NO_ERROR; - } - -private: - Optional ipAddress; - uint8_t ipAddressBuffer[kIpAddressBufferSize]; -}; - -struct CommissionNodeInfo -{ - CommissioningWindowParams params; - ProtectedIPAddress ipAddress; - Optional port; -}; - class Delegate { public: @@ -130,12 +93,9 @@ class Delegate * Commission a node specified by the previously approved request. * * @param params The parameters for the commissioning window. - * @param ipAddress Optional IP address for the commissioning window. - * @param port Optional port for the commissioning window. * @return CHIP_ERROR indicating the success or failure of the operation. */ - virtual CHIP_ERROR HandleCommissionNode(const CommissioningWindowParams & params, const Optional & ipAddress, - const Optional & port) = 0; + virtual CHIP_ERROR HandleCommissionNode(const CommissioningWindowParams & params) = 0; virtual ~Delegate() = default; }; diff --git a/src/app/zap-templates/zcl/data-model/chip/commissioner-control-cluster.xml b/src/app/zap-templates/zcl/data-model/chip/commissioner-control-cluster.xml index 7228b8f47a20ab..af9a734d6e7176 100644 --- a/src/app/zap-templates/zcl/data-model/chip/commissioner-control-cluster.xml +++ b/src/app/zap-templates/zcl/data-model/chip/commissioner-control-cluster.xml @@ -49,10 +49,7 @@ limitations under the License. This command is sent by a client to request that the server begins commissioning a previously approved request. - - - - + diff --git a/src/controller/data_model/controller-clusters.matter b/src/controller/data_model/controller-clusters.matter index e21a82213b661a..9b73af37ddb32e 100644 --- a/src/controller/data_model/controller-clusters.matter +++ b/src/controller/data_model/controller-clusters.matter @@ -9450,8 +9450,6 @@ provisional cluster CommissionerControl = 1873 { request struct CommissionNodeRequest { int64u requestId = 0; int16u responseTimeoutSeconds = 1; - optional octet_string ipAddress = 2; - optional int16u port = 3; } response struct ReverseOpenCommissioningWindow = 2 { diff --git a/src/controller/java/generated/java/chip/devicecontroller/ChipClusters.java b/src/controller/java/generated/java/chip/devicecontroller/ChipClusters.java index 8dd9332beb6885..fd792a14bb2e4f 100644 --- a/src/controller/java/generated/java/chip/devicecontroller/ChipClusters.java +++ b/src/controller/java/generated/java/chip/devicecontroller/ChipClusters.java @@ -61087,11 +61087,11 @@ public void onResponse(StructType invokeStructValue) { }}, commandId, commandArgs, timedInvokeTimeoutMs); } - public void commissionNode(ReverseOpenCommissioningWindowCallback callback, Long requestId, Integer responseTimeoutSeconds, Optional ipAddress, Optional port) { - commissionNode(callback, requestId, responseTimeoutSeconds, ipAddress, port, 0); + public void commissionNode(ReverseOpenCommissioningWindowCallback callback, Long requestId, Integer responseTimeoutSeconds) { + commissionNode(callback, requestId, responseTimeoutSeconds, 0); } - public void commissionNode(ReverseOpenCommissioningWindowCallback callback, Long requestId, Integer responseTimeoutSeconds, Optional ipAddress, Optional port, int timedInvokeTimeoutMs) { + public void commissionNode(ReverseOpenCommissioningWindowCallback callback, Long requestId, Integer responseTimeoutSeconds, int timedInvokeTimeoutMs) { final long commandId = 1L; ArrayList elements = new ArrayList<>(); @@ -61103,14 +61103,6 @@ public void commissionNode(ReverseOpenCommissioningWindowCallback callback, Long BaseTLVType responseTimeoutSecondstlvValue = new UIntType(responseTimeoutSeconds); elements.add(new StructElement(responseTimeoutSecondsFieldID, responseTimeoutSecondstlvValue)); - final long ipAddressFieldID = 2L; - BaseTLVType ipAddresstlvValue = ipAddress.map((nonOptionalipAddress) -> new ByteArrayType(nonOptionalipAddress)).orElse(new EmptyType()); - elements.add(new StructElement(ipAddressFieldID, ipAddresstlvValue)); - - final long portFieldID = 3L; - BaseTLVType porttlvValue = port.map((nonOptionalport) -> new UIntType(nonOptionalport)).orElse(new EmptyType()); - elements.add(new StructElement(portFieldID, porttlvValue)); - StructType commandArgs = new StructType(elements); invoke(new InvokeCallbackImpl(callback) { @Override diff --git a/src/controller/java/generated/java/chip/devicecontroller/ClusterIDMapping.java b/src/controller/java/generated/java/chip/devicecontroller/ClusterIDMapping.java index 829cf623e75ba0..b48f4029b62237 100644 --- a/src/controller/java/generated/java/chip/devicecontroller/ClusterIDMapping.java +++ b/src/controller/java/generated/java/chip/devicecontroller/ClusterIDMapping.java @@ -17424,7 +17424,7 @@ public static RequestCommissioningApprovalCommandField value(int id) throws NoSu } throw new NoSuchFieldError(); } - }public enum CommissionNodeCommandField {RequestId(0),ResponseTimeoutSeconds(1),IpAddress(2),Port(3),; + }public enum CommissionNodeCommandField {RequestId(0),ResponseTimeoutSeconds(1),; private final int id; CommissionNodeCommandField(int id) { this.id = id; diff --git a/src/controller/java/generated/java/chip/devicecontroller/ClusterInfoMapping.java b/src/controller/java/generated/java/chip/devicecontroller/ClusterInfoMapping.java index fe948f9429f516..55659d8463581d 100644 --- a/src/controller/java/generated/java/chip/devicecontroller/ClusterInfoMapping.java +++ b/src/controller/java/generated/java/chip/devicecontroller/ClusterInfoMapping.java @@ -29046,12 +29046,6 @@ public Map> getCommandMap() { CommandParameterInfo commissionerControlcommissionNoderesponseTimeoutSecondsCommandParameterInfo = new CommandParameterInfo("responseTimeoutSeconds", Integer.class, Integer.class); commissionerControlcommissionNodeCommandParams.put("responseTimeoutSeconds",commissionerControlcommissionNoderesponseTimeoutSecondsCommandParameterInfo); - - CommandParameterInfo commissionerControlcommissionNodeipAddressCommandParameterInfo = new CommandParameterInfo("ipAddress", Optional.class, byte[].class); - commissionerControlcommissionNodeCommandParams.put("ipAddress",commissionerControlcommissionNodeipAddressCommandParameterInfo); - - CommandParameterInfo commissionerControlcommissionNodeportCommandParameterInfo = new CommandParameterInfo("port", Optional.class, Integer.class); - commissionerControlcommissionNodeCommandParams.put("port",commissionerControlcommissionNodeportCommandParameterInfo); InteractionInfo commissionerControlcommissionNodeInteractionInfo = new InteractionInfo( (cluster, callback, commandArguments) -> { ((ChipClusters.CommissionerControlCluster) cluster) @@ -29062,12 +29056,6 @@ public Map> getCommandMap() { , (Integer) commandArguments.get("responseTimeoutSeconds") - , (Optional) - commandArguments.get("ipAddress") - - , (Optional) - commandArguments.get("port") - ); }, () -> new DelegatedCommissionerControlClusterReverseOpenCommissioningWindowCallback(), diff --git a/src/controller/java/generated/java/matter/controller/cluster/clusters/CommissionerControlCluster.kt b/src/controller/java/generated/java/matter/controller/cluster/clusters/CommissionerControlCluster.kt index 2daf2b464e948f..3e9d07f3f04354 100644 --- a/src/controller/java/generated/java/matter/controller/cluster/clusters/CommissionerControlCluster.kt +++ b/src/controller/java/generated/java/matter/controller/cluster/clusters/CommissionerControlCluster.kt @@ -130,8 +130,6 @@ class CommissionerControlCluster( suspend fun commissionNode( requestId: ULong, responseTimeoutSeconds: UShort, - ipAddress: ByteArray?, - port: UShort?, timedInvokeTimeout: Duration? = null, ): ReverseOpenCommissioningWindow { val commandId: UInt = 1u @@ -144,12 +142,6 @@ class CommissionerControlCluster( val TAG_RESPONSE_TIMEOUT_SECONDS_REQ: Int = 1 tlvWriter.put(ContextSpecificTag(TAG_RESPONSE_TIMEOUT_SECONDS_REQ), responseTimeoutSeconds) - - val TAG_IP_ADDRESS_REQ: Int = 2 - ipAddress?.let { tlvWriter.put(ContextSpecificTag(TAG_IP_ADDRESS_REQ), ipAddress) } - - val TAG_PORT_REQ: Int = 3 - port?.let { tlvWriter.put(ContextSpecificTag(TAG_PORT_REQ), port) } tlvWriter.endStructure() val request: InvokeRequest = diff --git a/src/controller/python/chip/clusters/CHIPClusters.py b/src/controller/python/chip/clusters/CHIPClusters.py index 925ce90cfcf3a8..356d3b43569ea2 100644 --- a/src/controller/python/chip/clusters/CHIPClusters.py +++ b/src/controller/python/chip/clusters/CHIPClusters.py @@ -13389,8 +13389,6 @@ class ChipClusters: "args": { "requestId": "int", "responseTimeoutSeconds": "int", - "ipAddress": "bytes", - "port": "int", }, }, }, diff --git a/src/controller/python/chip/clusters/Objects.py b/src/controller/python/chip/clusters/Objects.py index 82f257f764052c..8c9cbe7db50b87 100644 --- a/src/controller/python/chip/clusters/Objects.py +++ b/src/controller/python/chip/clusters/Objects.py @@ -47477,14 +47477,10 @@ def descriptor(cls) -> ClusterObjectDescriptor: Fields=[ ClusterObjectFieldDescriptor(Label="requestId", Tag=0, Type=uint), ClusterObjectFieldDescriptor(Label="responseTimeoutSeconds", Tag=1, Type=uint), - ClusterObjectFieldDescriptor(Label="ipAddress", Tag=2, Type=typing.Optional[bytes]), - ClusterObjectFieldDescriptor(Label="port", Tag=3, Type=typing.Optional[uint]), ]) requestId: 'uint' = 0 responseTimeoutSeconds: 'uint' = 0 - ipAddress: 'typing.Optional[bytes]' = None - port: 'typing.Optional[uint]' = None @dataclass class ReverseOpenCommissioningWindow(ClusterCommand): diff --git a/src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.h b/src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.h index 95b3675d94e0ae..044b0116ff4778 100644 --- a/src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.h +++ b/src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.h @@ -10933,10 +10933,6 @@ MTR_PROVISIONALLY_AVAILABLE @property (nonatomic, copy) NSNumber * _Nonnull requestId MTR_PROVISIONALLY_AVAILABLE; @property (nonatomic, copy) NSNumber * _Nonnull responseTimeoutSeconds MTR_PROVISIONALLY_AVAILABLE; - -@property (nonatomic, copy) NSData * _Nullable ipAddress MTR_PROVISIONALLY_AVAILABLE; - -@property (nonatomic, copy) NSNumber * _Nullable port MTR_PROVISIONALLY_AVAILABLE; /** * Controls whether the command is a timed command (using Timed Invoke). * diff --git a/src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.mm b/src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.mm index 00dc34a9a37ee6..d24ce54fc84028 100644 --- a/src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.mm +++ b/src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.mm @@ -31583,10 +31583,6 @@ - (instancetype)init _requestId = @(0); _responseTimeoutSeconds = @(0); - - _ipAddress = nil; - - _port = nil; _timedInvokeTimeoutMs = nil; _serverSideProcessingTimeout = nil; } @@ -31599,8 +31595,6 @@ - (id)copyWithZone:(NSZone * _Nullable)zone; other.requestId = self.requestId; other.responseTimeoutSeconds = self.responseTimeoutSeconds; - other.ipAddress = self.ipAddress; - other.port = self.port; other.timedInvokeTimeoutMs = self.timedInvokeTimeoutMs; other.serverSideProcessingTimeout = self.serverSideProcessingTimeout; @@ -31609,7 +31603,7 @@ - (id)copyWithZone:(NSZone * _Nullable)zone; - (NSString *)description { - NSString * descriptionString = [NSString stringWithFormat:@"<%@: requestId:%@; responseTimeoutSeconds:%@; ipAddress:%@; port:%@; >", NSStringFromClass([self class]), _requestId, _responseTimeoutSeconds, [_ipAddress base64EncodedStringWithOptions:0], _port]; + NSString * descriptionString = [NSString stringWithFormat:@"<%@: requestId:%@; responseTimeoutSeconds:%@; >", NSStringFromClass([self class]), _requestId, _responseTimeoutSeconds]; return descriptionString; } @@ -31627,18 +31621,6 @@ - (CHIP_ERROR)_encodeToTLVReader:(chip::System::PacketBufferTLVReader &)reader { encodableStruct.responseTimeoutSeconds = self.responseTimeoutSeconds.unsignedShortValue; } - { - if (self.ipAddress != nil) { - auto & definedValue_0 = encodableStruct.ipAddress.Emplace(); - definedValue_0 = AsByteSpan(self.ipAddress); - } - } - { - if (self.port != nil) { - auto & definedValue_0 = encodableStruct.port.Emplace(); - definedValue_0 = self.port.unsignedShortValue; - } - } auto buffer = chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSizeWithoutReserve, 0); if (buffer.IsNull()) { diff --git a/zzz_generated/app-common/app-common/zap-generated/cluster-objects.cpp b/zzz_generated/app-common/app-common/zap-generated/cluster-objects.cpp index c3947539864e2c..4be7782087f3d6 100644 --- a/zzz_generated/app-common/app-common/zap-generated/cluster-objects.cpp +++ b/zzz_generated/app-common/app-common/zap-generated/cluster-objects.cpp @@ -28973,8 +28973,6 @@ CHIP_ERROR Type::Encode(TLV::TLVWriter & aWriter, TLV::Tag aTag) const DataModel::WrappedStructEncoder encoder{ aWriter, aTag }; encoder.Encode(to_underlying(Fields::kRequestId), requestId); encoder.Encode(to_underlying(Fields::kResponseTimeoutSeconds), responseTimeoutSeconds); - encoder.Encode(to_underlying(Fields::kIpAddress), ipAddress); - encoder.Encode(to_underlying(Fields::kPort), port); return encoder.Finalize(); } @@ -29000,14 +28998,6 @@ CHIP_ERROR DecodableType::Decode(TLV::TLVReader & reader) { err = DataModel::Decode(reader, responseTimeoutSeconds); } - else if (__context_tag == to_underlying(Fields::kIpAddress)) - { - err = DataModel::Decode(reader, ipAddress); - } - else if (__context_tag == to_underlying(Fields::kPort)) - { - err = DataModel::Decode(reader, port); - } else { } diff --git a/zzz_generated/app-common/app-common/zap-generated/cluster-objects.h b/zzz_generated/app-common/app-common/zap-generated/cluster-objects.h index 3309b8ee41878b..78022f3ac6ce11 100644 --- a/zzz_generated/app-common/app-common/zap-generated/cluster-objects.h +++ b/zzz_generated/app-common/app-common/zap-generated/cluster-objects.h @@ -42016,8 +42016,6 @@ enum class Fields : uint8_t { kRequestId = 0, kResponseTimeoutSeconds = 1, - kIpAddress = 2, - kPort = 3, }; struct Type @@ -42029,8 +42027,6 @@ struct Type uint64_t requestId = static_cast(0); uint16_t responseTimeoutSeconds = static_cast(0); - Optional ipAddress; - Optional port; CHIP_ERROR Encode(TLV::TLVWriter & aWriter, TLV::Tag aTag) const; @@ -42047,8 +42043,6 @@ struct DecodableType uint64_t requestId = static_cast(0); uint16_t responseTimeoutSeconds = static_cast(0); - Optional ipAddress; - Optional port; CHIP_ERROR Decode(TLV::TLVReader & reader); }; }; // namespace CommissionNode diff --git a/zzz_generated/chip-tool/zap-generated/cluster/Commands.h b/zzz_generated/chip-tool/zap-generated/cluster/Commands.h index e145de431ed5b1..f50b5721bce5d5 100644 --- a/zzz_generated/chip-tool/zap-generated/cluster/Commands.h +++ b/zzz_generated/chip-tool/zap-generated/cluster/Commands.h @@ -13886,8 +13886,6 @@ class CommissionerControlCommissionNode : public ClusterCommand { AddArgument("RequestId", 0, UINT64_MAX, &mRequest.requestId); AddArgument("ResponseTimeoutSeconds", 0, UINT16_MAX, &mRequest.responseTimeoutSeconds); - AddArgument("IpAddress", &mRequest.ipAddress); - AddArgument("Port", 0, UINT16_MAX, &mRequest.port); ClusterCommand::AddArguments(); } diff --git a/zzz_generated/darwin-framework-tool/zap-generated/cluster/Commands.h b/zzz_generated/darwin-framework-tool/zap-generated/cluster/Commands.h index e8aa8848588f3c..a77ea1f918b90b 100644 --- a/zzz_generated/darwin-framework-tool/zap-generated/cluster/Commands.h +++ b/zzz_generated/darwin-framework-tool/zap-generated/cluster/Commands.h @@ -164752,12 +164752,6 @@ class CommissionerControlCommissionNode : public ClusterCommand { #endif // MTR_ENABLE_PROVISIONAL #if MTR_ENABLE_PROVISIONAL AddArgument("ResponseTimeoutSeconds", 0, UINT16_MAX, &mRequest.responseTimeoutSeconds); -#endif // MTR_ENABLE_PROVISIONAL -#if MTR_ENABLE_PROVISIONAL - AddArgument("IpAddress", &mRequest.ipAddress); -#endif // MTR_ENABLE_PROVISIONAL -#if MTR_ENABLE_PROVISIONAL - AddArgument("Port", 0, UINT16_MAX, &mRequest.port); #endif // MTR_ENABLE_PROVISIONAL ClusterCommand::AddArguments(); } @@ -164778,20 +164772,6 @@ class CommissionerControlCommissionNode : public ClusterCommand { #endif // MTR_ENABLE_PROVISIONAL #if MTR_ENABLE_PROVISIONAL params.responseTimeoutSeconds = [NSNumber numberWithUnsignedShort:mRequest.responseTimeoutSeconds]; -#endif // MTR_ENABLE_PROVISIONAL -#if MTR_ENABLE_PROVISIONAL - if (mRequest.ipAddress.HasValue()) { - params.ipAddress = [NSData dataWithBytes:mRequest.ipAddress.Value().data() length:mRequest.ipAddress.Value().size()]; - } else { - params.ipAddress = nil; - } -#endif // MTR_ENABLE_PROVISIONAL -#if MTR_ENABLE_PROVISIONAL - if (mRequest.port.HasValue()) { - params.port = [NSNumber numberWithUnsignedShort:mRequest.port.Value()]; - } else { - params.port = nil; - } #endif // MTR_ENABLE_PROVISIONAL uint16_t repeatCount = mRepeatCount.ValueOr(1); uint16_t __block responsesNeeded = repeatCount; From 9bbf5b9f1de62c23bed4e8974140e3a5ddf79777 Mon Sep 17 00:00:00 2001 From: Axel Le Bourhis <45206070+axelnxp@users.noreply.github.com> Date: Thu, 29 Aug 2024 12:27:50 +0200 Subject: [PATCH 10/13] [NXP][Zephyr] Integrate wifi improvements and updates from nrfconnect (#35273) * [NXP][Zephyr] Integrate wifi improvements and updates from nrfconnect * WiFiNetworkDiagnostics events generation * Make Wi-Fi manager use Wi-Fi interface only * Rework joining/leaving mcast group * mDNS refresh after IPv6 change * Scan for a specific SSID * Fix tx/rx unicast counters * Fix missing network band in scan response * Various other fixes/improvements Signed-off-by: Axel Le Bourhis * Restyled by clang-format --------- Signed-off-by: Axel Le Bourhis Co-authored-by: Restyled.io --- .../wifi/ConnectivityManagerImplWiFi.cpp | 16 +- src/platform/Zephyr/wifi/WiFiManager.cpp | 351 +++++++++++------- src/platform/Zephyr/wifi/WiFiManager.h | 47 ++- src/platform/Zephyr/wifi/ZephyrWifiDriver.cpp | 57 ++- src/platform/Zephyr/wifi/ZephyrWifiDriver.h | 15 +- .../nxp/zephyr/ConnectivityManagerImpl.cpp | 82 +++- 6 files changed, 395 insertions(+), 173 deletions(-) diff --git a/src/platform/Zephyr/wifi/ConnectivityManagerImplWiFi.cpp b/src/platform/Zephyr/wifi/ConnectivityManagerImplWiFi.cpp index 5bef79bb22fdc5..9b605c31560142 100644 --- a/src/platform/Zephyr/wifi/ConnectivityManagerImplWiFi.cpp +++ b/src/platform/Zephyr/wifi/ConnectivityManagerImplWiFi.cpp @@ -42,7 +42,7 @@ ConnectivityManager::WiFiStationMode ConnectivityManagerImplWiFi::_GetWiFiStatio { if (mStationMode != ConnectivityManager::WiFiStationMode::kWiFiStationMode_ApplicationControlled) { - mStationMode = (WiFiManager::StationStatus::DISABLED == WiFiManager().Instance().GetStationStatus()) + mStationMode = (WiFiManager::StationStatus::DISABLED == WiFiManager::Instance().GetStationStatus()) ? ConnectivityManager::WiFiStationMode::kWiFiStationMode_Disabled : ConnectivityManager::WiFiStationMode::kWiFiStationMode_Enabled; } @@ -60,7 +60,7 @@ CHIP_ERROR ConnectivityManagerImplWiFi::_SetWiFiStationMode(ConnectivityManager: bool ConnectivityManagerImplWiFi::_IsWiFiStationEnabled(void) { - return (WiFiManager::StationStatus::DISABLED <= WiFiManager().Instance().GetStationStatus()); + return (WiFiManager::StationStatus::DISABLED <= WiFiManager::Instance().GetStationStatus()); } bool ConnectivityManagerImplWiFi::_IsWiFiStationApplicationControlled(void) @@ -70,7 +70,7 @@ bool ConnectivityManagerImplWiFi::_IsWiFiStationApplicationControlled(void) bool ConnectivityManagerImplWiFi::_IsWiFiStationConnected(void) { - return (WiFiManager::StationStatus::CONNECTED == WiFiManager().Instance().GetStationStatus()); + return (WiFiManager::StationStatus::CONNECTED == WiFiManager::Instance().GetStationStatus()); } System::Clock::Timeout ConnectivityManagerImplWiFi::_GetWiFiStationReconnectInterval(void) @@ -88,14 +88,14 @@ bool ConnectivityManagerImplWiFi::_IsWiFiStationProvisioned(void) { // from Matter perspective `provisioned` means that the supplicant has been provided // with SSID and password (doesn't matter if valid or not) - return (WiFiManager::StationStatus::CONNECTING <= WiFiManager().Instance().GetStationStatus()); + return (WiFiManager::StationStatus::CONNECTING <= WiFiManager::Instance().GetStationStatus()); } void ConnectivityManagerImplWiFi::_ClearWiFiStationProvision(void) { if (_IsWiFiStationProvisioned()) { - if (CHIP_NO_ERROR != WiFiManager().Instance().ClearStationProvisioningData()) + if (CHIP_NO_ERROR != WiFiManager::Instance().ClearStationProvisioningData()) { ChipLogError(DeviceLayer, "Cannot clear WiFi station provisioning data"); } @@ -104,9 +104,9 @@ void ConnectivityManagerImplWiFi::_ClearWiFiStationProvision(void) bool ConnectivityManagerImplWiFi::_CanStartWiFiScan() { - return (WiFiManager::StationStatus::DISABLED != WiFiManager().Instance().GetStationStatus() && - WiFiManager::StationStatus::SCANNING != WiFiManager().Instance().GetStationStatus() && - WiFiManager::StationStatus::CONNECTING != WiFiManager().Instance().GetStationStatus()); + return (WiFiManager::StationStatus::DISABLED != WiFiManager::Instance().GetStationStatus() && + WiFiManager::StationStatus::SCANNING != WiFiManager::Instance().GetStationStatus() && + WiFiManager::StationStatus::CONNECTING != WiFiManager::Instance().GetStationStatus()); } void ConnectivityManagerImplWiFi::_OnWiFiStationProvisionChange() diff --git a/src/platform/Zephyr/wifi/WiFiManager.cpp b/src/platform/Zephyr/wifi/WiFiManager.cpp index 01d0d937b19b3a..6dbc4930314fb1 100644 --- a/src/platform/Zephyr/wifi/WiFiManager.cpp +++ b/src/platform/Zephyr/wifi/WiFiManager.cpp @@ -23,10 +23,9 @@ #include "WiFiManager.h" #include -#include -#include #include #include +#include #include #include @@ -50,6 +49,21 @@ namespace DeviceLayer { namespace { +app::Clusters::NetworkCommissioning::WiFiBandEnum ConvertBandEnum(uint8_t band) +{ + switch (band) + { + case WIFI_FREQ_BAND_2_4_GHZ: + return app::Clusters::NetworkCommissioning::WiFiBandEnum::k2g4; + case WIFI_FREQ_BAND_5_GHZ: + return app::Clusters::NetworkCommissioning::WiFiBandEnum::k5g; + case WIFI_FREQ_BAND_6_GHZ: + return app::Clusters::NetworkCommissioning::WiFiBandEnum::k6g; + default: + return app::Clusters::NetworkCommissioning::WiFiBandEnum::kUnknownEnumValue; + } +} + NetworkCommissioning::WiFiScanResponse ToScanResponse(const wifi_scan_result * result) { NetworkCommissioning::WiFiScanResponse response = {}; @@ -62,9 +76,10 @@ NetworkCommissioning::WiFiScanResponse ToScanResponse(const wifi_scan_result * r // TODO: Distinguish WPA versions response.security.Set(result->security == WIFI_SECURITY_TYPE_PSK ? NetworkCommissioning::WiFiSecurity::kWpaPersonal : NetworkCommissioning::WiFiSecurity::kUnencrypted); - response.channel = result->channel; - response.rssi = result->rssi; - response.ssidLen = result->ssid_length; + response.channel = result->channel; + response.rssi = result->rssi; + response.ssidLen = result->ssid_length; + response.wiFiBand = ConvertBandEnum(result->band); memcpy(response.ssid, result->ssid, result->ssid_length); // TODO: MAC/BSSID is not filled by the Wi-Fi driver memcpy(response.bssid, result->mac, result->mac_length); @@ -138,57 +153,43 @@ const Map { WIFI_STATE_GROUP_HANDSHAKE, WiFiManager::StationStatus::PROVISIONING }, { WIFI_STATE_COMPLETED, WiFiManager::StationStatus::FULLY_PROVISIONED } }); -const Map - WiFiManager::sEventHandlerMap({ { NET_EVENT_WIFI_SCAN_RESULT, WiFiManager::ScanResultHandler }, - { NET_EVENT_WIFI_SCAN_DONE, WiFiManager::ScanDoneHandler }, - { NET_EVENT_WIFI_CONNECT_RESULT, WiFiManager::ConnectHandler }, - { NET_EVENT_WIFI_DISCONNECT_RESULT, WiFiManager::DisconnectHandler }, - { NET_EVENT_WIFI_DISCONNECT_COMPLETE, WiFiManager::DisconnectHandler } }); +const Map WiFiManager::sEventHandlerMap({ + { NET_EVENT_WIFI_SCAN_RESULT, WiFiManager::ScanResultHandler }, + { NET_EVENT_WIFI_SCAN_DONE, WiFiManager::ScanDoneHandler }, + { NET_EVENT_WIFI_CONNECT_RESULT, WiFiManager::ConnectHandler }, + { NET_EVENT_WIFI_DISCONNECT_RESULT, WiFiManager::DisconnectHandler }, + { NET_EVENT_WIFI_DISCONNECT_COMPLETE, WiFiManager::DisconnectHandler }, +}); void WiFiManager::WifiMgmtEventHandler(net_mgmt_event_callback * cb, uint32_t mgmtEvent, net_if * iface) { - if (0 == strcmp(iface->if_dev->dev->name, InetUtils::GetInterface()->if_dev->dev->name)) + if (iface == Instance().mNetIf) { Platform::UniquePtr eventData(new uint8_t[cb->info_length]); VerifyOrReturn(eventData); memcpy(eventData.get(), cb->info, cb->info_length); - sEventHandlerMap[mgmtEvent](std::move(eventData)); + sEventHandlerMap[mgmtEvent](std::move(eventData), cb->info_length); } } -CHIP_ERROR WiFiManager::Init() +void WiFiManager::IPv6MgmtEventHandler(net_mgmt_event_callback * cb, uint32_t mgmtEvent, net_if * iface) { - // TODO: consider moving these to ConnectivityManagerImpl to be prepared for handling multiple interfaces on a single device. - Inet::UDPEndPointImplSockets::SetMulticastGroupHandler([](Inet::InterfaceId interfaceId, const Inet::IPAddress & address, - Inet::UDPEndPointImplSockets::MulticastOperation operation) { - const in6_addr addr = InetUtils::ToZephyrAddr(address); - net_if * iface = InetUtils::GetInterface(interfaceId); - VerifyOrReturnError(iface != nullptr, INET_ERROR_UNKNOWN_INTERFACE); - - if (operation == Inet::UDPEndPointImplSockets::MulticastOperation::kJoin) - { - net_if_mcast_addr * maddr = net_if_ipv6_maddr_add(iface, &addr); - - if (maddr && !net_if_ipv6_maddr_is_joined(maddr) && !net_ipv6_is_addr_mcast_link_all_nodes(&addr)) - { - net_if_ipv6_maddr_join(iface, maddr); - } - } - else if (operation == Inet::UDPEndPointImplSockets::MulticastOperation::kLeave) - { - VerifyOrReturnError(net_ipv6_is_addr_mcast_link_all_nodes(&addr) || net_if_ipv6_maddr_rm(iface, &addr), - CHIP_ERROR_INVALID_ADDRESS); - } - else - { - return CHIP_ERROR_INCORRECT_STATE; - } + if (((mgmtEvent == NET_EVENT_IPV6_ADDR_ADD) || (mgmtEvent == NET_EVENT_IPV6_ADDR_DEL)) && cb->info) + { + IPv6AddressChangeHandler(cb->info); + } +} - return CHIP_NO_ERROR; - }); +CHIP_ERROR WiFiManager::Init() +{ + mNetIf = InetUtils::GetWiFiInterface(); + VerifyOrReturnError(mNetIf != nullptr, INET_ERROR_UNKNOWN_INTERFACE); net_mgmt_init_event_callback(&mWiFiMgmtClbk, WifiMgmtEventHandler, kWifiManagementEvents); + net_mgmt_init_event_callback(&mIPv6MgmtClbk, IPv6MgmtEventHandler, kIPv6ManagementEvents); + net_mgmt_add_event_callback(&mWiFiMgmtClbk); + net_mgmt_add_event_callback(&mIPv6MgmtClbk); ChipLogDetail(DeviceLayer, "WiFiManager has been initialized"); @@ -197,9 +198,6 @@ CHIP_ERROR WiFiManager::Init() CHIP_ERROR WiFiManager::Scan(const ByteSpan & ssid, ScanResultCallback resultCallback, ScanDoneCallback doneCallback, bool internalScan) { - net_if * iface = InetUtils::GetInterface(); - VerifyOrReturnError(nullptr != iface, CHIP_ERROR_INTERNAL); - mInternalScan = internalScan; mScanResultCallback = resultCallback; mScanDoneCallback = doneCallback; @@ -207,17 +205,22 @@ CHIP_ERROR WiFiManager::Scan(const ByteSpan & ssid, ScanResultCallback resultCal mWiFiState = WIFI_STATE_SCANNING; mSsidFound = false; - /* If the ssid is not null, it means the scan must target a specific SSID, and only include this one in the scan - * result. To do so, we save the requested ssid and we will filter the scan results accordingly in the scan done - * handler. */ - if ((ssid.size() > 0) && (!mInternalScan)) + wifi_scan_params * scanParams{ nullptr }; + size_t scanParamsSize{ 0 }; + + if (!ssid.empty()) { - mNetworkToScan.Erase(); - memcpy(mNetworkToScan.ssid, ssid.data(), ssid.size()); - mNetworkToScan.ssidLen = ssid.size(); + /* We must assume that the ssid is handled as a NULL-terminated string. + Note that the mScanSsidBuffer is initialized with zeros. */ + VerifyOrReturnError(ssid.size() < sizeof(mScanSsidBuffer), CHIP_ERROR_INVALID_ARGUMENT); + memcpy(mScanSsidBuffer, ssid.data(), ssid.size()); + mScanSsidBuffer[ssid.size()] = 0; // indicate the end of ssid string + mScanParams.ssids[0] = mScanSsidBuffer; + mScanParams.ssids[1] = nullptr; // indicate the end of ssids list + scanParams = &mScanParams; + scanParamsSize = sizeof(*scanParams); } - - if (0 != net_mgmt(NET_REQUEST_WIFI_SCAN, iface, NULL, 0)) + if (0 != net_mgmt(NET_REQUEST_WIFI_SCAN, mNetIf, scanParams, scanParamsSize)) { ChipLogError(DeviceLayer, "Scan request failed"); return CHIP_ERROR_INTERNAL; @@ -239,9 +242,7 @@ CHIP_ERROR WiFiManager::Connect(const ByteSpan & ssid, const ByteSpan & credenti { ChipLogDetail(DeviceLayer, "Connecting to WiFi network: %.*s", ssid.size(), ssid.data()); - mHandling.mOnConnectionSuccess = handling.mOnConnectionSuccess; - mHandling.mOnConnectionFailed = handling.mOnConnectionFailed; - mHandling.mConnectionTimeout = handling.mConnectionTimeout; + mHandling = handling; mWiFiState = WIFI_STATE_ASSOCIATING; @@ -258,11 +259,8 @@ CHIP_ERROR WiFiManager::Connect(const ByteSpan & ssid, const ByteSpan & credenti CHIP_ERROR WiFiManager::Disconnect() { - net_if * iface = InetUtils::GetInterface(); - VerifyOrReturnError(nullptr != iface, CHIP_ERROR_INTERNAL); - mApplicationDisconnectRequested = true; - int status = net_mgmt(NET_REQUEST_WIFI_DISCONNECT, iface, NULL, 0); + int status = net_mgmt(NET_REQUEST_WIFI_DISCONNECT, mNetIf, NULL, 0); if (status) { @@ -287,11 +285,9 @@ CHIP_ERROR WiFiManager::Disconnect() CHIP_ERROR WiFiManager::GetWiFiInfo(WiFiInfo & info) const { - net_if * iface = InetUtils::GetInterface(); - VerifyOrReturnError(nullptr != iface, CHIP_ERROR_INTERNAL); - struct wifi_iface_status status = { 0 }; + wifi_iface_status status = { 0 }; - if (net_mgmt(NET_REQUEST_WIFI_IFACE_STATUS, iface, &status, sizeof(struct wifi_iface_status))) + if (net_mgmt(NET_REQUEST_WIFI_IFACE_STATUS, mNetIf, &status, sizeof(wifi_iface_status))) { ChipLogError(DeviceLayer, "Status request failed"); return CHIP_ERROR_INTERNAL; @@ -316,22 +312,31 @@ CHIP_ERROR WiFiManager::GetWiFiInfo(WiFiInfo & info) const CHIP_ERROR WiFiManager::GetNetworkStatistics(NetworkStatistics & stats) const { net_stats_wifi data{}; - net_mgmt(NET_REQUEST_STATS_GET_WIFI, InetUtils::GetInterface(), &data, sizeof(data)); + net_mgmt(NET_REQUEST_STATS_GET_WIFI, mNetIf, &data, sizeof(data)); stats.mPacketMulticastRxCount = data.multicast.rx; stats.mPacketMulticastTxCount = data.multicast.tx; - stats.mPacketUnicastRxCount = data.pkts.rx - data.multicast.rx - data.broadcast.rx; - stats.mPacketUnicastTxCount = data.pkts.tx - data.multicast.tx - data.broadcast.tx; - stats.mBeaconsSuccessCount = data.sta_mgmt.beacons_rx; - stats.mBeaconsLostCount = data.sta_mgmt.beacons_miss; +#if CONFIG_CHIP_NXP_PLATFORM + /* Zephyr 3.6 doesn't support the unicast stat in net_stats_wifi struct */ + stats.mPacketUnicastRxCount = data.pkts.rx - data.multicast.rx - data.broadcast.rx; + stats.mPacketUnicastRxCount = data.pkts.tx - data.multicast.tx - data.broadcast.tx; +#else + stats.mPacketUnicastRxCount = data.unicast.rx; + stats.mPacketUnicastTxCount = data.unicast.tx; +#endif + stats.mBeaconsSuccessCount = data.sta_mgmt.beacons_rx; + stats.mBeaconsLostCount = data.sta_mgmt.beacons_miss; return CHIP_NO_ERROR; } -void WiFiManager::ScanResultHandler(Platform::UniquePtr data) +void WiFiManager::ScanResultHandler(Platform::UniquePtr data, size_t length) { + // Validate that input data size matches the expected one. + VerifyOrReturn(length == sizeof(wifi_scan_result)); + // Contrary to other handlers, offload accumulating of the scan results from the CHIP thread to the caller's thread - const struct wifi_scan_result * scanResult = reinterpret_cast(data.get()); + const wifi_scan_result * scanResult = reinterpret_cast(data.get()); if (Instance().mInternalScan && Instance().mWantedNetwork.GetSsidSpan().data_equal(ByteSpan(scanResult->ssid, scanResult->ssid_length))) @@ -369,45 +374,33 @@ void WiFiManager::ScanResultHandler(Platform::UniquePtr data) if (Instance().mScanResultCallback && !Instance().mInternalScan) { - /* Here we need to check if the scan is targeting a specific network and filter the scan result accordingly, - * to make sure only the targeted SSID is reported */ - if (Instance().mNetworkToScan.GetSsidSpan().size() == 0) - { - Instance().mScanResultCallback(ToScanResponse(scanResult)); - } - else if (Instance().mNetworkToScan.GetSsidSpan().data_equal(ByteSpan(scanResult->ssid, scanResult->ssid_length))) - { - Instance().mScanResultCallback(ToScanResponse(scanResult)); - } + Instance().mScanResultCallback(ToScanResponse(scanResult)); } } -void WiFiManager::ScanDoneHandler(Platform::UniquePtr data) +void WiFiManager::ScanDoneHandler(Platform::UniquePtr data, size_t length) { + // Validate that input data size matches the expected one. + VerifyOrReturn(length == sizeof(wifi_status)); + CHIP_ERROR err = SystemLayer().ScheduleLambda([capturedData = data.get()] { Platform::UniquePtr safePtr(capturedData); - uint8_t * rawData = safePtr.get(); - const wifi_status * status = reinterpret_cast(rawData); - WiFiRequestStatus requestStatus = static_cast(status->status); - - /* Reset the specific network to scan */ - if (Instance().mNetworkToScan.GetSsidSpan().size() > 0) - { - Instance().mNetworkToScan.Erase(); - } + uint8_t * rawData = safePtr.get(); + const wifi_status * status = reinterpret_cast(rawData); + ScanDoneStatus scanDoneStatus = status->status; - if (requestStatus == WiFiRequestStatus::FAILURE) + if (scanDoneStatus) { - ChipLogError(DeviceLayer, "Wi-Fi scan finalization failure (%d)", status->status); + ChipLogError(DeviceLayer, "Wi-Fi scan finalization failure (%d)", scanDoneStatus); } else { - ChipLogProgress(DeviceLayer, "Wi-Fi scan done (%d)", status->status); + ChipLogProgress(DeviceLayer, "Wi-Fi scan done"); } if (Instance().mScanDoneCallback && !Instance().mInternalScan) { - Instance().mScanDoneCallback(requestStatus); + Instance().mScanDoneCallback(scanDoneStatus); // restore the connection state from before the scan request was issued Instance().mWiFiState = Instance().mCachedWiFiState; return; @@ -416,6 +409,7 @@ void WiFiManager::ScanDoneHandler(Platform::UniquePtr data) // Internal scan is supposed to be followed by a connection request if the SSID has been found if (Instance().mInternalScan) { + if (!Instance().mSsidFound) { ChipLogProgress(DeviceLayer, "No requested SSID found"); @@ -430,12 +424,13 @@ void WiFiManager::ScanDoneHandler(Platform::UniquePtr data) net_if * iface = InetUtils::GetInterface(); VerifyOrReturn(nullptr != iface, CHIP_ERROR_INTERNAL); - if (net_mgmt(NET_REQUEST_WIFI_CONNECT, iface, &(Instance().mWiFiParams.mParams), sizeof(wifi_connect_req_params))) + if (net_mgmt(NET_REQUEST_WIFI_CONNECT, Instance().mNetIf, &(Instance().mWiFiParams.mParams), + sizeof(wifi_connect_req_params))) { ChipLogError(DeviceLayer, "Connection request failed"); - if (Instance().mHandling.mOnConnectionFailed) + if (Instance().mHandling.mOnConnectionDone) { - Instance().mHandling.mOnConnectionFailed(); + Instance().mHandling.mOnConnectionDone(WIFI_STATUS_CONN_FAIL); } Instance().mWiFiState = WIFI_STATE_DISCONNECTED; return; @@ -455,38 +450,65 @@ void WiFiManager::ScanDoneHandler(Platform::UniquePtr data) void WiFiManager::SendRouterSolicitation(System::Layer * layer, void * param) { - net_if * iface = InetUtils::GetInterface(); - if (iface && iface->if_dev->link_addr.type == NET_LINK_ETHERNET) + net_if_start_rs(Instance().mNetIf); + Instance().mRouterSolicitationCounter++; + if (Instance().mRouterSolicitationCounter < kRouterSolicitationMaxCount) { - net_if_start_rs(iface); - Instance().mRouterSolicitationCounter++; - if (Instance().mRouterSolicitationCounter < kRouterSolicitationMaxCount) - { - DeviceLayer::SystemLayer().StartTimer(System::Clock::Milliseconds32(kRouterSolicitationIntervalMs), - SendRouterSolicitation, nullptr); - } - else - { - Instance().mRouterSolicitationCounter = 0; - } + DeviceLayer::SystemLayer().StartTimer(System::Clock::Milliseconds32(kRouterSolicitationIntervalMs), SendRouterSolicitation, + nullptr); + } + else + { + Instance().mRouterSolicitationCounter = 0; } } -void WiFiManager::ConnectHandler(Platform::UniquePtr data) +void WiFiManager::ConnectHandler(Platform::UniquePtr data, size_t length) { + using app::Clusters::WiFiNetworkDiagnostics::AssociationFailureCauseEnum; + + // Validate that input data size matches the expected one. + VerifyOrReturn(length == sizeof(wifi_status)); + CHIP_ERROR err = SystemLayer().ScheduleLambda([capturedData = data.get()] { Platform::UniquePtr safePtr(capturedData); - uint8_t * rawData = safePtr.get(); - const wifi_status * status = reinterpret_cast(rawData); - WiFiRequestStatus requestStatus = static_cast(status->status); + uint8_t * rawData = safePtr.get(); + const wifi_status * status = reinterpret_cast(rawData); + wifi_conn_status connStatus = status->conn_status; - if (requestStatus == WiFiRequestStatus::FAILURE || requestStatus == WiFiRequestStatus::TERMINATED) + if (connStatus) { ChipLogProgress(DeviceLayer, "Connection to WiFi network failed or was terminated by another request"); Instance().mWiFiState = WIFI_STATE_DISCONNECTED; - if (Instance().mHandling.mOnConnectionFailed) + if (Instance().mHandling.mOnConnectionDone) + { + Instance().mHandling.mOnConnectionDone(connStatus); + } + + WiFiDiagnosticsDelegate * delegate = GetDiagnosticDataProvider().GetWiFiDiagnosticsDelegate(); + if (delegate) { - Instance().mHandling.mOnConnectionFailed(); + uint16_t reason = Instance().GetLastDisconnectReason(); + uint8_t associationFailureCause; + + switch (connStatus) + { + case WIFI_STATUS_CONN_WRONG_PASSWORD: + associationFailureCause = to_underlying(AssociationFailureCauseEnum::kAuthenticationFailed); + break; + case WIFI_STATUS_CONN_FAIL: + case WIFI_STATUS_CONN_TIMEOUT: + associationFailureCause = to_underlying(AssociationFailureCauseEnum::kAssociationFailed); + break; + case WIFI_STATUS_CONN_AP_NOT_FOUND: + associationFailureCause = to_underlying(AssociationFailureCauseEnum::kSsidNotFound); + break; + default: + associationFailureCause = to_underlying(AssociationFailureCauseEnum::kUnknown); + break; + } + + delegate->OnAssociationFailureDetected(associationFailureCause, reason); } } else // The connection has been established successfully. @@ -498,9 +520,9 @@ void WiFiManager::ConnectHandler(Platform::UniquePtr data) ChipLogProgress(DeviceLayer, "Connected to WiFi network"); Instance().mWiFiState = WIFI_STATE_COMPLETED; - if (Instance().mHandling.mOnConnectionSuccess) + if (Instance().mHandling.mOnConnectionDone) { - Instance().mHandling.mOnConnectionSuccess(); + Instance().mHandling.mOnConnectionDone(connStatus); } Instance().PostConnectivityStatusChange(kConnectivity_Established); @@ -513,6 +535,13 @@ void WiFiManager::ConnectHandler(Platform::UniquePtr data) { ChipLogError(DeviceLayer, "Cannot post event [error: %s]", ErrorStr(error)); } + + WiFiDiagnosticsDelegate * delegate = GetDiagnosticDataProvider().GetWiFiDiagnosticsDelegate(); + if (delegate) + { + delegate->OnConnectionStatusChanged( + to_underlying(app::Clusters::WiFiNetworkDiagnostics::ConnectionStatusEnum::kConnected)); + } } // cleanup the provisioning data as it is configured per each connect request Instance().ClearStationProvisioningData(); @@ -525,13 +554,74 @@ void WiFiManager::ConnectHandler(Platform::UniquePtr data) } } -void WiFiManager::DisconnectHandler(Platform::UniquePtr) +void WiFiManager::DisconnectHandler(Platform::UniquePtr data, size_t length) { - SystemLayer().ScheduleLambda([] { + // Validate that input data size matches the expected one. + VerifyOrReturn(length == sizeof(wifi_status)); + + CHIP_ERROR err = SystemLayer().ScheduleLambda([capturedData = data.get()] { + Platform::UniquePtr safePtr(capturedData); + uint8_t * rawData = safePtr.get(); + const wifi_status * status = reinterpret_cast(rawData); + uint16_t reason; + + switch (status->disconn_reason) + { + case WIFI_REASON_DISCONN_UNSPECIFIED: + reason = WLAN_REASON_UNSPECIFIED; + break; + case WIFI_REASON_DISCONN_USER_REQUEST: + reason = WLAN_REASON_DEAUTH_LEAVING; + break; + case WIFI_REASON_DISCONN_AP_LEAVING: + reason = WLAN_REASON_DEAUTH_LEAVING; + break; + case WIFI_REASON_DISCONN_INACTIVITY: + reason = WLAN_REASON_DISASSOC_DUE_TO_INACTIVITY; + break; + default: + reason = WLAN_REASON_UNSPECIFIED; + break; + } + Instance().SetLastDisconnectReason(reason); + ChipLogProgress(DeviceLayer, "WiFi station disconnected"); Instance().mWiFiState = WIFI_STATE_DISCONNECTED; Instance().PostConnectivityStatusChange(kConnectivity_Lost); + + WiFiDiagnosticsDelegate * delegate = GetDiagnosticDataProvider().GetWiFiDiagnosticsDelegate(); + if (delegate) + { + delegate->OnConnectionStatusChanged( + to_underlying(app::Clusters::WiFiNetworkDiagnostics::ConnectionStatusEnum::kNotConnected)); + delegate->OnDisconnectionDetected(reason); + } }); + + if (CHIP_NO_ERROR == err) + { + // the ownership has been transferred to the worker thread - release the buffer + data.release(); + } +} + +void WiFiManager::IPv6AddressChangeHandler(const void * data) +{ + const in6_addr * addr = reinterpret_cast(data); + + // Filter out link-local addresses that are not routable outside of a local network. + if (!net_ipv6_is_ll_addr(addr)) + { + // This is needed to send mDNS queries containing updated IPv6 addresses. + ChipDeviceEvent event; + event.Type = DeviceEventType::kDnssdRestartNeeded; + + CHIP_ERROR error = PlatformMgr().PostEvent(&event); + if (error != CHIP_NO_ERROR) + { + ChipLogError(DeviceLayer, "Cannot post event: %" CHIP_ERROR_FORMAT, error.Format()); + } + } } WiFiManager::StationStatus WiFiManager::GetStationStatus() const @@ -597,11 +687,10 @@ System::Clock::Milliseconds32 WiFiManager::CalculateNextRecoveryTime() CHIP_ERROR WiFiManager::SetLowPowerMode(bool onoff) { - net_if * iface = InetUtils::GetInterface(); - VerifyOrReturnError(nullptr != iface, CHIP_ERROR_INTERNAL); + VerifyOrReturnError(nullptr != mNetIf, CHIP_ERROR_INTERNAL); wifi_ps_config currentConfig{}; - if (net_mgmt(NET_REQUEST_WIFI_PS_CONFIG, iface, ¤tConfig, sizeof(currentConfig))) + if (net_mgmt(NET_REQUEST_WIFI_PS_CONFIG, mNetIf, ¤tConfig, sizeof(currentConfig))) { ChipLogError(DeviceLayer, "Get current low power mode config request failed"); return CHIP_ERROR_INTERNAL; @@ -611,7 +700,7 @@ CHIP_ERROR WiFiManager::SetLowPowerMode(bool onoff) (currentConfig.ps_params.enabled == WIFI_PS_DISABLED && onoff == true)) { wifi_ps_params params{ .enabled = onoff ? WIFI_PS_ENABLED : WIFI_PS_DISABLED }; - if (net_mgmt(NET_REQUEST_WIFI_PS, iface, ¶ms, sizeof(params))) + if (net_mgmt(NET_REQUEST_WIFI_PS, mNetIf, ¶ms, sizeof(params))) { ChipLogError(DeviceLayer, "Set low power mode request failed"); return CHIP_ERROR_INTERNAL; @@ -624,5 +713,15 @@ CHIP_ERROR WiFiManager::SetLowPowerMode(bool onoff) return CHIP_NO_ERROR; } +void WiFiManager::SetLastDisconnectReason(uint16_t reason) +{ + mLastDisconnectedReason = reason; +} + +uint16_t WiFiManager::GetLastDisconnectReason() +{ + return mLastDisconnectedReason; +} + } // namespace DeviceLayer } // namespace chip diff --git a/src/platform/Zephyr/wifi/WiFiManager.h b/src/platform/Zephyr/wifi/WiFiManager.h index d48536fb3d0da3..99bf777547a800 100644 --- a/src/platform/Zephyr/wifi/WiFiManager.h +++ b/src/platform/Zephyr/wifi/WiFiManager.h @@ -88,16 +88,16 @@ class Map class WiFiManager { public: - enum WiFiRequestStatus : int - { - SUCCESS = 0, - FAILURE = 1, - TERMINATED = 2 - }; + /* No copy, nor move. */ + WiFiManager(const WiFiManager &) = delete; + WiFiManager & operator=(const WiFiManager &) = delete; + WiFiManager(WiFiManager &&) = delete; + WiFiManager & operator=(WiFiManager &&) = delete; + using ScanDoneStatus = decltype(wifi_status::status); using ScanResultCallback = void (*)(const NetworkCommissioning::WiFiScanResponse &); - using ScanDoneCallback = void (*)(WiFiRequestStatus); - using ConnectionCallback = void (*)(); + using ScanDoneCallback = void (*)(const ScanDoneStatus &); + using ConnectionCallback = void (*)(const wifi_conn_status &); enum class StationStatus : uint8_t { @@ -120,8 +120,7 @@ class WiFiManager struct ConnectionHandling { - ConnectionCallback mOnConnectionSuccess{}; - ConnectionCallback mOnConnectionFailed{}; + ConnectionCallback mOnConnectionDone{}; System::Clock::Seconds32 mConnectionTimeout{}; }; @@ -182,12 +181,18 @@ class WiFiManager CHIP_ERROR ClearStationProvisioningData(); CHIP_ERROR Disconnect(); CHIP_ERROR GetWiFiInfo(WiFiInfo & info) const; + const WiFiNetwork & GetWantedNetwork() const { return mWantedNetwork; } CHIP_ERROR GetNetworkStatistics(NetworkStatistics & stats) const; void AbortConnectionRecovery(); CHIP_ERROR SetLowPowerMode(bool onoff); + void SetLastDisconnectReason(uint16_t reason); + uint16_t GetLastDisconnectReason(); private: - using NetEventHandler = void (*)(Platform::UniquePtr); + using NetEventHandler = void (*)(Platform::UniquePtr, size_t); + + WiFiManager() = default; + ~WiFiManager() = default; struct ConnectionParams { @@ -198,14 +203,18 @@ class WiFiManager constexpr static uint32_t kWifiManagementEvents = NET_EVENT_WIFI_SCAN_RESULT | NET_EVENT_WIFI_SCAN_DONE | NET_EVENT_WIFI_CONNECT_RESULT | NET_EVENT_WIFI_DISCONNECT_RESULT | NET_EVENT_WIFI_IFACE_STATUS; + constexpr static uint32_t kIPv6ManagementEvents = NET_EVENT_IPV6_ADDR_ADD | NET_EVENT_IPV6_ADDR_DEL; + // Event handling static void WifiMgmtEventHandler(net_mgmt_event_callback * cb, uint32_t mgmtEvent, net_if * iface); - static void ScanResultHandler(Platform::UniquePtr data); - static void ScanDoneHandler(Platform::UniquePtr data); - static void ConnectHandler(Platform::UniquePtr data); - static void DisconnectHandler(Platform::UniquePtr data); + static void IPv6MgmtEventHandler(net_mgmt_event_callback * cb, uint32_t mgmtEvent, net_if * iface); + static void ScanResultHandler(Platform::UniquePtr data, size_t length); + static void ScanDoneHandler(Platform::UniquePtr data, size_t length); + static void ConnectHandler(Platform::UniquePtr data, size_t length); + static void DisconnectHandler(Platform::UniquePtr data, size_t length); static void PostConnectivityStatusChange(ConnectivityChange changeType); static void SendRouterSolicitation(System::Layer * layer, void * param); + static void IPv6AddressChangeHandler(const void * data); // Connection Recovery feature // This feature allows re-scanning and re-connecting the connection to the known network after @@ -220,21 +229,25 @@ class WiFiManager void ResetRecoveryTime(); System::Clock::Milliseconds32 CalculateNextRecoveryTime(); + net_if * mNetIf{ nullptr }; ConnectionParams mWiFiParams{}; - ConnectionHandling mHandling; + ConnectionHandling mHandling{}; + wifi_scan_params mScanParams{}; + char mScanSsidBuffer[DeviceLayer::Internal::kMaxWiFiSSIDLength + 1] = { 0 }; wifi_iface_state mWiFiState; wifi_iface_state mCachedWiFiState; net_mgmt_event_callback mWiFiMgmtClbk{}; + net_mgmt_event_callback mIPv6MgmtClbk{}; ScanResultCallback mScanResultCallback{ nullptr }; ScanDoneCallback mScanDoneCallback{ nullptr }; WiFiNetwork mWantedNetwork{}; - WiFiNetwork mNetworkToScan{}; bool mInternalScan{ false }; uint8_t mRouterSolicitationCounter = 0; bool mSsidFound{ false }; uint32_t mConnectionRecoveryCounter{ 0 }; uint32_t mConnectionRecoveryTimeMs{ kConnectionRecoveryMinIntervalMs }; bool mApplicationDisconnectRequested{ false }; + uint16_t mLastDisconnectedReason = WLAN_REASON_UNSPECIFIED; static const Map sStatusMap; static const Map sEventHandlerMap; diff --git a/src/platform/Zephyr/wifi/ZephyrWifiDriver.cpp b/src/platform/Zephyr/wifi/ZephyrWifiDriver.cpp index 53b7fa684a2213..7404a0272297be 100644 --- a/src/platform/Zephyr/wifi/ZephyrWifiDriver.cpp +++ b/src/platform/Zephyr/wifi/ZephyrWifiDriver.cpp @@ -17,10 +17,13 @@ #include "ZephyrWifiDriver.h" +#include + #include #include #include +#include #include using namespace ::chip; @@ -103,8 +106,9 @@ CHIP_ERROR ZephyrWifiDriver::Init(NetworkStatusChangeCallback * networkStatusCha if (mStagingNetwork.IsConfigured()) { - WiFiManager::ConnectionHandling handling{ [] { Instance().OnNetworkStatusChanged(Status::kSuccess); }, - [] { Instance().OnNetworkStatusChanged(Status::kUnknownError); }, + WiFiManager::ConnectionHandling handling{ [](const wifi_conn_status & connStatus) { + Instance().OnNetworkConnStatusChanged(connStatus); + }, System::Clock::Seconds32{ kWiFiConnectNetworkTimeoutSeconds } }; ReturnErrorOnFailure( WiFiManager::Instance().Connect(mStagingNetwork.GetSsidSpan(), mStagingNetwork.GetPassSpan(), handling)); @@ -113,8 +117,11 @@ CHIP_ERROR ZephyrWifiDriver::Init(NetworkStatusChangeCallback * networkStatusCha return CHIP_NO_ERROR; } -void ZephyrWifiDriver::OnNetworkStatusChanged(Status status) +void ZephyrWifiDriver::OnNetworkConnStatusChanged(const wifi_conn_status & connStatus) { + // TODO: check if we can report more accurate errors + Status status = connStatus ? Status::kUnknownError : Status::kSuccess; + if (status == Status::kSuccess) { ConnectivityMgr().SetWiFiStationMode(ConnectivityManager::kWiFiStationMode_Enabled); @@ -122,7 +129,23 @@ void ZephyrWifiDriver::OnNetworkStatusChanged(Status status) if (mpNetworkStatusChangeCallback) { - mpNetworkStatusChangeCallback->OnNetworkingStatusChange(status, NullOptional, NullOptional); + const uint8_t * ssid{}; + size_t ssidLen{}; + WiFiManager::WiFiInfo wifiInfo; + + if (CHIP_NO_ERROR == WiFiManager::Instance().GetWiFiInfo(wifiInfo)) + { + ssid = wifiInfo.mSsid; + ssidLen = wifiInfo.mSsidLen; + } + else + { + ssid = WiFiManager::Instance().GetWantedNetwork().ssid; + ssidLen = WiFiManager::Instance().GetWantedNetwork().ssidLen; + } + mpNetworkStatusChangeCallback->OnNetworkingStatusChange(status, MakeOptional(ByteSpan(ssid, ssidLen)), + connStatus ? MakeOptional(static_cast(connStatus)) + : NullOptional); } if (mpConnectCallback) @@ -158,12 +181,15 @@ CHIP_ERROR ZephyrWifiDriver::RevertConfiguration() // we are already connected to this network, so return prematurely return CHIP_NO_ERROR; } + + WiFiManager::Instance().Disconnect(); } if (mStagingNetwork.IsConfigured()) { - WiFiManager::ConnectionHandling handling{ [] { Instance().OnNetworkStatusChanged(Status::kSuccess); }, - [] { Instance().OnNetworkStatusChanged(Status::kUnknownError); }, + WiFiManager::ConnectionHandling handling{ [](const wifi_conn_status & connStatus) { + Instance().OnNetworkConnStatusChanged(connStatus); + }, System::Clock::Seconds32{ kWiFiConnectNetworkTimeoutSeconds } }; ReturnErrorOnFailure( WiFiManager::Instance().Connect(mStagingNetwork.GetSsidSpan(), mStagingNetwork.GetPassSpan(), handling)); @@ -217,8 +243,9 @@ void ZephyrWifiDriver::ConnectNetwork(ByteSpan networkId, ConnectCallback * call { Status status = Status::kSuccess; WiFiManager::StationStatus stationStatus; - WiFiManager::ConnectionHandling handling{ [] { Instance().OnNetworkStatusChanged(Status::kSuccess); }, - [] { Instance().OnNetworkStatusChanged(Status::kUnknownError); }, + WiFiManager::ConnectionHandling handling{ [](const wifi_conn_status & connStatus) { + Instance().OnNetworkConnStatusChanged(connStatus); + }, System::Clock::Seconds32{ kWiFiConnectNetworkTimeoutSeconds } }; VerifyOrExit(mpConnectCallback == nullptr, status = Status::kUnknownError); @@ -262,11 +289,10 @@ void ZephyrWifiDriver::LoadFromStorage() mStagingNetwork = network; } -void ZephyrWifiDriver::OnScanWiFiNetworkDone(WiFiManager::WiFiRequestStatus status) +void ZephyrWifiDriver::OnScanWiFiNetworkDone(const WiFiManager::ScanDoneStatus & status) { VerifyOrReturn(mScanCallback != nullptr); - mScanCallback->OnFinished(status == WiFiManager::WiFiRequestStatus::SUCCESS ? Status::kSuccess : Status::kUnknownError, - CharSpan(), &mScanResponseIterator); + mScanCallback->OnFinished(status ? Status::kUnknownError : Status::kSuccess, CharSpan(), &mScanResponseIterator); mScanCallback = nullptr; } @@ -280,7 +306,7 @@ void ZephyrWifiDriver::ScanNetworks(ByteSpan ssid, WiFiDriver::ScanCallback * ca mScanCallback = callback; CHIP_ERROR error = WiFiManager::Instance().Scan( ssid, [](const WiFiScanResponse & response) { Instance().OnScanWiFiNetworkResult(response); }, - [](WiFiManager::WiFiRequestStatus status) { Instance().OnScanWiFiNetworkDone(status); }); + [](const WiFiManager::ScanDoneStatus & status) { Instance().OnScanWiFiNetworkDone(status); }); if (error != CHIP_NO_ERROR) { @@ -289,6 +315,13 @@ void ZephyrWifiDriver::ScanNetworks(ByteSpan ssid, WiFiDriver::ScanCallback * ca } } +uint32_t ZephyrWifiDriver::GetSupportedWiFiBandsMask() const +{ + uint32_t bands = static_cast(1UL << chip::to_underlying(WiFiBandEnum::k2g4)); + bands |= static_cast(1UL << chip::to_underlying(WiFiBandEnum::k5g)); + return bands; +} + } // namespace NetworkCommissioning } // namespace DeviceLayer } // namespace chip diff --git a/src/platform/Zephyr/wifi/ZephyrWifiDriver.h b/src/platform/Zephyr/wifi/ZephyrWifiDriver.h index 9096aa408b69bd..414311a07ec4bf 100644 --- a/src/platform/Zephyr/wifi/ZephyrWifiDriver.h +++ b/src/platform/Zephyr/wifi/ZephyrWifiDriver.h @@ -25,9 +25,9 @@ namespace chip { namespace DeviceLayer { namespace NetworkCommissioning { -constexpr uint8_t kMaxWiFiNetworks = 1; -constexpr uint8_t kWiFiScanNetworksTimeOutSeconds = 10; -constexpr uint8_t kWiFiConnectNetworkTimeoutSeconds = 35; +inline constexpr uint8_t kMaxWiFiNetworks = 1; +inline constexpr uint8_t kWiFiScanNetworksTimeOutSeconds = 10; +inline constexpr uint8_t kWiFiConnectNetworkTimeoutSeconds = 35; class ZephyrWifiScanResponseIterator : public Iterator { @@ -48,8 +48,8 @@ class ZephyrWifiDriver final : public WiFiDriver public: // Define non-volatile storage keys for SSID and password. // The naming convention is aligned with DefaultStorageKeyAllocator class. - static constexpr const char * kSsidKey = "g/wi/s"; - static constexpr const char * kPassKey = "g/wi/p"; + static constexpr char kSsidKey[] = "g/wi/s"; + static constexpr char kPassKey[] = "g/wi/p"; class WiFiNetworkIterator final : public NetworkIterator { @@ -86,6 +86,7 @@ class ZephyrWifiDriver final : public WiFiDriver Status AddOrUpdateNetwork(ByteSpan ssid, ByteSpan credentials, MutableCharSpan & outDebugText, uint8_t & outNetworkIndex) override; void ScanNetworks(ByteSpan ssid, ScanCallback * callback) override; + uint32_t GetSupportedWiFiBandsMask() const override; static ZephyrWifiDriver & Instance() { @@ -93,9 +94,9 @@ class ZephyrWifiDriver final : public WiFiDriver return sInstance; } - void OnNetworkStatusChanged(Status status); + void OnNetworkConnStatusChanged(const wifi_conn_status & connStatus); void OnScanWiFiNetworkResult(const WiFiScanResponse & result); - void OnScanWiFiNetworkDone(WiFiManager::WiFiRequestStatus status); + void OnScanWiFiNetworkDone(const WiFiManager::ScanDoneStatus & status); private: void LoadFromStorage(); diff --git a/src/platform/nxp/zephyr/ConnectivityManagerImpl.cpp b/src/platform/nxp/zephyr/ConnectivityManagerImpl.cpp index f927df2ac17480..4d183558c18953 100644 --- a/src/platform/nxp/zephyr/ConnectivityManagerImpl.cpp +++ b/src/platform/nxp/zephyr/ConnectivityManagerImpl.cpp @@ -17,11 +17,16 @@ #include +#include +#include +#include #include +#include #include -#include -#include +#ifndef CONFIG_ARCH_POSIX +#include +#endif #include @@ -34,15 +39,64 @@ #endif #if CHIP_DEVICE_CONFIG_ENABLE_THREAD +#include #include #endif -using namespace ::chip; +using namespace ::chip::Inet; using namespace ::chip::DeviceLayer::Internal; namespace chip { namespace DeviceLayer { +namespace { +CHIP_ERROR JoinLeaveMulticastGroup(net_if * iface, const Inet::IPAddress & address, + UDPEndPointImplSockets::MulticastOperation operation) +{ +#if CHIP_DEVICE_CONFIG_ENABLE_THREAD + if (net_if_l2(iface) == &NET_L2_GET_NAME(OPENTHREAD)) + { + const otIp6Address otAddress = ToOpenThreadIP6Address(address); + const auto handler = operation == UDPEndPointImplSockets::MulticastOperation::kJoin ? otIp6SubscribeMulticastAddress + : otIp6UnsubscribeMulticastAddress; + otError error; + + ThreadStackMgr().LockThreadStack(); + error = handler(openthread_get_default_instance(), &otAddress); + ThreadStackMgr().UnlockThreadStack(); + + return MapOpenThreadError(error); + } +#endif + +#if CHIP_DEVICE_CONFIG_ENABLE_WIFI + // The following code should also be valid for other interface types, such as Ethernet, + // but they are not officially supported, so for now enable it for Wi-Fi only. + const in6_addr in6Addr = InetUtils::ToZephyrAddr(address); + + if (operation == UDPEndPointImplSockets::MulticastOperation::kJoin) + { + net_if_mcast_addr * maddr = net_if_ipv6_maddr_add(iface, &in6Addr); + + if (maddr && !net_if_ipv6_maddr_is_joined(maddr)) + { + net_if_ipv6_maddr_join(iface, maddr); + } + } + else if (operation == UDPEndPointImplSockets::MulticastOperation::kLeave) + { + VerifyOrReturnError(net_if_ipv6_maddr_rm(iface, &in6Addr), CHIP_ERROR_INVALID_ADDRESS); + } + else + { + return CHIP_ERROR_INCORRECT_STATE; + } +#endif + + return CHIP_NO_ERROR; +} +} // namespace + ConnectivityManagerImpl ConnectivityManagerImpl::sInstance; CHIP_ERROR ConnectivityManagerImpl::_Init() @@ -53,6 +107,28 @@ CHIP_ERROR ConnectivityManagerImpl::_Init() #if CHIP_DEVICE_CONFIG_ENABLE_WIFI ReturnErrorOnFailure(InitWiFi()); #endif + +#if CHIP_DEVICE_CONFIG_ENABLE_THREAD || CHIP_DEVICE_CONFIG_ENABLE_WIFI + UDPEndPointImplSockets::SetMulticastGroupHandler( + [](InterfaceId interfaceId, const IPAddress & address, UDPEndPointImplSockets::MulticastOperation operation) { + if (interfaceId.IsPresent()) + { + net_if * iface = InetUtils::GetInterface(interfaceId); + VerifyOrReturnError(iface != nullptr, INET_ERROR_UNKNOWN_INTERFACE); + + return JoinLeaveMulticastGroup(iface, address, operation); + } + + // If the interface is not specified, join or leave the multicast group on all interfaces. + for (int i = 1; net_if * iface = net_if_get_by_index(i); i++) + { + ReturnErrorOnFailure(JoinLeaveMulticastGroup(iface, address, operation)); + } + + return CHIP_NO_ERROR; + }); +#endif // CHIP_DEVICE_CONFIG_ENABLE_THREAD || CHIP_DEVICE_CONFIG_ENABLE_WIFI + return CHIP_NO_ERROR; } From 6c6b11f723692855446744c8bbc36ec1c5404512 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 29 Aug 2024 08:42:18 -0400 Subject: [PATCH 11/13] Add DirectModeChange feature to RVC Run Mode and RVC Clean Mode cluster XML and all-clusters-app implementation (#35258) * Add DirectModeChange feature to RVC Run Mode and RVC Clean Mode cluster XML. * Run update_cluster_revisions.py to update to the new RVC Run Mode cluster revision. * Run update_cluster_revisions.py to update to the new RVC Clean Mode cluster revision. * Regenerate generated code. --- .../all-clusters-app.matter | 12 +++--- .../all-clusters-common/all-clusters-app.zap | 4 +- .../all-clusters-common/src/rvc-modes.cpp | 39 +++++++++++-------- .../chef/common/chef-rvc-mode-delegate.cpp | 8 ++-- ...tnode_colortemperaturelight_hbUnzYVeyn.zap | 14 +++---- ...ode_roboticvacuumcleaner_1807ff0c49.matter | 12 +++--- ...otnode_roboticvacuumcleaner_1807ff0c49.zap | 4 +- examples/rvc-app/rvc-common/rvc-app.matter | 12 +++--- examples/rvc-app/rvc-common/rvc-app.zap | 4 +- .../zap/tests/inputs/all-clusters-app.zap | 4 +- .../app-templates/endpoint_config.h | 4 +- .../chip/rvc-clean-mode-cluster.xml | 19 +++++---- .../data-model/chip/rvc-run-mode-cluster.xml | 20 ++++++---- .../data_model/controller-clusters.matter | 8 ++-- .../data_model/controller-clusters.zap | 4 +- .../python/chip/clusters/Objects.py | 4 +- .../CHIP/zap-generated/MTRBaseClusters.h | 4 +- .../app-common/zap-generated/cluster-enums.h | 4 +- 18 files changed, 99 insertions(+), 81 deletions(-) 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 index 6926da18d78a61..1f8bc5463d8a9d 100644 --- a/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter +++ b/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter @@ -3235,7 +3235,7 @@ cluster LaundryWasherControls = 83 { /** Attributes and commands for selecting a mode from a list of supported options. */ cluster RvcRunMode = 84 { - revision 2; + revision 3; enum ModeTag : enum16 { kIdle = 16384; @@ -3255,7 +3255,7 @@ cluster RvcRunMode = 84 { } bitmap Feature : bitmap32 { - kNoFeatures = 0x0; + kDirectModeChange = 0x10000; } struct ModeTagStruct { @@ -3294,7 +3294,7 @@ cluster RvcRunMode = 84 { /** Attributes and commands for selecting a mode from a list of supported options. */ cluster RvcCleanMode = 85 { - revision 2; + revision 3; enum ModeTag : enum16 { kDeepClean = 16384; @@ -3307,7 +3307,7 @@ cluster RvcCleanMode = 85 { } bitmap Feature : bitmap32 { - kNoFeatures = 0x0; + kDirectModeChange = 0x10000; } struct ModeTagStruct { @@ -8489,7 +8489,7 @@ endpoint 1 { callback attribute eventList; callback attribute attributeList; callback attribute featureMap; - ram attribute clusterRevision default = 2; + ram attribute clusterRevision default = 3; handle command ChangeToMode; handle command ChangeToModeResponse; @@ -8503,7 +8503,7 @@ endpoint 1 { callback attribute eventList; callback attribute attributeList; callback attribute featureMap; - ram attribute clusterRevision default = 2; + ram attribute clusterRevision default = 3; handle command ChangeToMode; handle command ChangeToModeResponse; diff --git a/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap b/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap index 51ea8b7188a96b..00f525cf1710ae 100644 --- a/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap +++ b/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap @@ -9648,7 +9648,7 @@ "storageOption": "RAM", "singleton": 0, "bounded": 0, - "defaultValue": "2", + "defaultValue": "3", "reportable": 1, "minInterval": 1, "maxInterval": 65534, @@ -9804,7 +9804,7 @@ "storageOption": "RAM", "singleton": 0, "bounded": 0, - "defaultValue": "2", + "defaultValue": "3", "reportable": 1, "minInterval": 1, "maxInterval": 65534, diff --git a/examples/all-clusters-app/all-clusters-common/src/rvc-modes.cpp b/examples/all-clusters-app/all-clusters-common/src/rvc-modes.cpp index 627709a8c30921..5571834c375646 100644 --- a/examples/all-clusters-app/all-clusters-common/src/rvc-modes.cpp +++ b/examples/all-clusters-app/all-clusters-common/src/rvc-modes.cpp @@ -16,6 +16,7 @@ * limitations under the License. */ #include +#include #include #include @@ -41,12 +42,15 @@ void RvcRunModeDelegate::HandleChangeToMode(uint8_t NewMode, ModeBase::Commands: { uint8_t currentMode = mInstance->GetCurrentMode(); - // Our business logic states that we can only switch into a running mode from the idle state. - if (NewMode != RvcRunMode::ModeIdle && currentMode != RvcRunMode::ModeIdle) + if (!gRvcRunModeInstance->HasFeature(static_cast(RvcRunMode::Feature::kDirectModeChange))) { - response.status = to_underlying(ModeBase::StatusCode::kInvalidInMode); - response.statusText.SetValue(chip::CharSpan::fromCharString("Change to a running mode is only allowed from idle")); - return; + // Our business logic states that we can only switch into a running mode from the idle state. + if (NewMode != RvcRunMode::ModeIdle && currentMode != RvcRunMode::ModeIdle) + { + response.status = to_underlying(ModeBase::StatusCode::kInvalidInMode); + response.statusText.SetValue(chip::CharSpan::fromCharString("Change to a running mode is only allowed from idle")); + return; + } } auto rvcOpStateInstance = RvcOperationalState::GetRvcOperationalStateInstance(); @@ -123,8 +127,8 @@ void emberAfRvcRunModeClusterInitCallback(chip::EndpointId endpointId) VerifyOrDie(endpointId == 1); // this cluster is only enabled for endpoint 1. VerifyOrDie(gRvcRunModeDelegate == nullptr && gRvcRunModeInstance == nullptr); gRvcRunModeDelegate = new RvcRunMode::RvcRunModeDelegate; - gRvcRunModeInstance = - new ModeBase::Instance(gRvcRunModeDelegate, 0x1, RvcRunMode::Id, chip::to_underlying(RvcRunMode::Feature::kNoFeatures)); + gRvcRunModeInstance = new ModeBase::Instance(gRvcRunModeDelegate, 0x1, RvcRunMode::Id, + chip::to_underlying(RvcRunMode::Feature::kDirectModeChange)); gRvcRunModeInstance->Init(); } @@ -139,14 +143,17 @@ CHIP_ERROR RvcCleanModeDelegate::Init() void RvcCleanModeDelegate::HandleChangeToMode(uint8_t NewMode, ModeBase::Commands::ChangeToModeResponse::Type & response) { - uint8_t rvcRunCurrentMode = gRvcRunModeInstance->GetCurrentMode(); - - if (rvcRunCurrentMode != RvcRunMode::ModeIdle) + if (!gRvcCleanModeInstance->HasFeature(static_cast(RvcCleanMode::Feature::kDirectModeChange))) { - response.status = to_underlying(ModeBase::StatusCode::kInvalidInMode); - response.statusText.SetValue( - chip::CharSpan::fromCharString("Cannot change the cleaning mode when the device is not in idle")); - return; + uint8_t rvcRunCurrentMode = gRvcRunModeInstance->GetCurrentMode(); + + if (rvcRunCurrentMode != RvcRunMode::ModeIdle) + { + response.status = to_underlying(ModeBase::StatusCode::kInvalidInMode); + response.statusText.SetValue( + chip::CharSpan::fromCharString("Cannot change the cleaning mode when the device is not in idle")); + return; + } } response.status = to_underlying(ModeBase::StatusCode::kSuccess); @@ -213,7 +220,7 @@ void emberAfRvcCleanModeClusterInitCallback(chip::EndpointId endpointId) VerifyOrDie(endpointId == 1); // this cluster is only enabled for endpoint 1. VerifyOrDie(gRvcCleanModeDelegate == nullptr && gRvcCleanModeInstance == nullptr); gRvcCleanModeDelegate = new RvcCleanMode::RvcCleanModeDelegate; - gRvcCleanModeInstance = - new ModeBase::Instance(gRvcCleanModeDelegate, 0x1, RvcCleanMode::Id, chip::to_underlying(RvcRunMode::Feature::kNoFeatures)); + gRvcCleanModeInstance = new ModeBase::Instance(gRvcCleanModeDelegate, 0x1, RvcCleanMode::Id, + chip::to_underlying(RvcCleanMode::Feature::kDirectModeChange)); gRvcCleanModeInstance->Init(); } diff --git a/examples/chef/common/chef-rvc-mode-delegate.cpp b/examples/chef/common/chef-rvc-mode-delegate.cpp index 32b4cad0236899..9caf2ab94fd835 100644 --- a/examples/chef/common/chef-rvc-mode-delegate.cpp +++ b/examples/chef/common/chef-rvc-mode-delegate.cpp @@ -154,8 +154,8 @@ void emberAfRvcRunModeClusterInitCallback(chip::EndpointId endpointId) VerifyOrDie(!gRvcRunModeDelegate && !gRvcRunModeInstance); gRvcRunModeDelegate = std::make_unique(); - gRvcRunModeInstance = std::make_unique(gRvcRunModeDelegate.get(), endpointId, RvcRunMode::Id, - chip::to_underlying(RvcRunMode::Feature::kNoFeatures)); + gRvcRunModeInstance = + std::make_unique(gRvcRunModeDelegate.get(), endpointId, RvcRunMode::Id, 0 /* No feature bits */); gRvcRunModeInstance->Init(); } @@ -290,8 +290,8 @@ void emberAfRvcCleanModeClusterInitCallback(chip::EndpointId endpointId) VerifyOrDie(!gRvcCleanModeDelegate && !gRvcCleanModeInstance); gRvcCleanModeDelegate = std::make_unique(); - gRvcCleanModeInstance = std::make_unique(gRvcCleanModeDelegate.get(), endpointId, RvcCleanMode::Id, - chip::to_underlying(RvcCleanMode::Feature::kNoFeatures)); + gRvcCleanModeInstance = + std::make_unique(gRvcCleanModeDelegate.get(), endpointId, RvcCleanMode::Id, 0 /* No feature bits */); gRvcCleanModeInstance->Init(); } #endif // MATTER_DM_PLUGIN_RVC_CLEAN_MODE_SERVER diff --git a/examples/chef/devices/rootnode_colortemperaturelight_hbUnzYVeyn.zap b/examples/chef/devices/rootnode_colortemperaturelight_hbUnzYVeyn.zap index c38b543d1385e9..9121f5bb32a4f0 100644 --- a/examples/chef/devices/rootnode_colortemperaturelight_hbUnzYVeyn.zap +++ b/examples/chef/devices/rootnode_colortemperaturelight_hbUnzYVeyn.zap @@ -17,13 +17,6 @@ } ], "package": [ - { - "pathRelativity": "relativeToZap", - "path": "../../../src/app/zap-templates/app-templates.json", - "type": "gen-templates-json", - "category": "matter", - "version": "chip-v1" - }, { "pathRelativity": "relativeToZap", "path": "../../../src/app/zap-templates/zcl/zcl.json", @@ -31,6 +24,13 @@ "category": "matter", "version": 1, "description": "Matter SDK ZCL data" + }, + { + "pathRelativity": "relativeToZap", + "path": "../../../src/app/zap-templates/app-templates.json", + "type": "gen-templates-json", + "category": "matter", + "version": "chip-v1" } ], "endpointTypes": [ diff --git a/examples/chef/devices/rootnode_roboticvacuumcleaner_1807ff0c49.matter b/examples/chef/devices/rootnode_roboticvacuumcleaner_1807ff0c49.matter index d53a594c514919..ae1989b2777122 100644 --- a/examples/chef/devices/rootnode_roboticvacuumcleaner_1807ff0c49.matter +++ b/examples/chef/devices/rootnode_roboticvacuumcleaner_1807ff0c49.matter @@ -1584,7 +1584,7 @@ cluster GroupKeyManagement = 63 { /** Attributes and commands for selecting a mode from a list of supported options. */ cluster RvcRunMode = 84 { - revision 2; + revision 3; enum ModeTag : enum16 { kIdle = 16384; @@ -1604,7 +1604,7 @@ cluster RvcRunMode = 84 { } bitmap Feature : bitmap32 { - kNoFeatures = 0x0; + kDirectModeChange = 0x10000; } struct ModeTagStruct { @@ -1643,7 +1643,7 @@ cluster RvcRunMode = 84 { /** Attributes and commands for selecting a mode from a list of supported options. */ cluster RvcCleanMode = 85 { - revision 2; + revision 3; enum ModeTag : enum16 { kDeepClean = 16384; @@ -1656,7 +1656,7 @@ cluster RvcCleanMode = 85 { } bitmap Feature : bitmap32 { - kNoFeatures = 0x0; + kDirectModeChange = 0x10000; } struct ModeTagStruct { @@ -2010,7 +2010,7 @@ endpoint 1 { callback attribute eventList; callback attribute attributeList; callback attribute featureMap; - ram attribute clusterRevision default = 2; + ram attribute clusterRevision default = 3; handle command ChangeToMode; handle command ChangeToModeResponse; @@ -2024,7 +2024,7 @@ endpoint 1 { callback attribute eventList; callback attribute attributeList; callback attribute featureMap; - ram attribute clusterRevision default = 2; + ram attribute clusterRevision default = 3; handle command ChangeToMode; handle command ChangeToModeResponse; diff --git a/examples/chef/devices/rootnode_roboticvacuumcleaner_1807ff0c49.zap b/examples/chef/devices/rootnode_roboticvacuumcleaner_1807ff0c49.zap index 81144c4b3b437f..0463c7d0fb801c 100644 --- a/examples/chef/devices/rootnode_roboticvacuumcleaner_1807ff0c49.zap +++ b/examples/chef/devices/rootnode_roboticvacuumcleaner_1807ff0c49.zap @@ -2852,7 +2852,7 @@ "storageOption": "RAM", "singleton": 0, "bounded": 0, - "defaultValue": "2", + "defaultValue": "3", "reportable": 1, "minInterval": 1, "maxInterval": 65534, @@ -3008,7 +3008,7 @@ "storageOption": "RAM", "singleton": 0, "bounded": 0, - "defaultValue": "2", + "defaultValue": "3", "reportable": 1, "minInterval": 1, "maxInterval": 65534, diff --git a/examples/rvc-app/rvc-common/rvc-app.matter b/examples/rvc-app/rvc-common/rvc-app.matter index ee4d751919be22..4c0ebdf1acc682 100644 --- a/examples/rvc-app/rvc-common/rvc-app.matter +++ b/examples/rvc-app/rvc-common/rvc-app.matter @@ -1248,7 +1248,7 @@ cluster GroupKeyManagement = 63 { /** Attributes and commands for selecting a mode from a list of supported options. */ cluster RvcRunMode = 84 { - revision 2; + revision 3; enum ModeTag : enum16 { kIdle = 16384; @@ -1268,7 +1268,7 @@ cluster RvcRunMode = 84 { } bitmap Feature : bitmap32 { - kNoFeatures = 0x0; + kDirectModeChange = 0x10000; } struct ModeTagStruct { @@ -1307,7 +1307,7 @@ cluster RvcRunMode = 84 { /** Attributes and commands for selecting a mode from a list of supported options. */ cluster RvcCleanMode = 85 { - revision 2; + revision 3; enum ModeTag : enum16 { kDeepClean = 16384; @@ -1320,7 +1320,7 @@ cluster RvcCleanMode = 85 { } bitmap Feature : bitmap32 { - kNoFeatures = 0x0; + kDirectModeChange = 0x10000; } struct ModeTagStruct { @@ -1735,7 +1735,7 @@ endpoint 1 { callback attribute eventList; callback attribute attributeList; callback attribute featureMap; - ram attribute clusterRevision default = 2; + ram attribute clusterRevision default = 3; handle command ChangeToMode; handle command ChangeToModeResponse; @@ -1749,7 +1749,7 @@ endpoint 1 { callback attribute eventList; callback attribute attributeList; callback attribute featureMap; - ram attribute clusterRevision default = 2; + ram attribute clusterRevision default = 3; handle command ChangeToMode; handle command ChangeToModeResponse; diff --git a/examples/rvc-app/rvc-common/rvc-app.zap b/examples/rvc-app/rvc-common/rvc-app.zap index 89fe82c64f5c21..770686677a8630 100644 --- a/examples/rvc-app/rvc-common/rvc-app.zap +++ b/examples/rvc-app/rvc-common/rvc-app.zap @@ -2478,7 +2478,7 @@ "storageOption": "RAM", "singleton": 0, "bounded": 0, - "defaultValue": "2", + "defaultValue": "3", "reportable": 1, "minInterval": 1, "maxInterval": 65534, @@ -2634,7 +2634,7 @@ "storageOption": "RAM", "singleton": 0, "bounded": 0, - "defaultValue": "2", + "defaultValue": "3", "reportable": 1, "minInterval": 1, "maxInterval": 65534, diff --git a/scripts/tools/zap/tests/inputs/all-clusters-app.zap b/scripts/tools/zap/tests/inputs/all-clusters-app.zap index 763b4f66c42140..0ec92a5c61294d 100644 --- a/scripts/tools/zap/tests/inputs/all-clusters-app.zap +++ b/scripts/tools/zap/tests/inputs/all-clusters-app.zap @@ -9648,7 +9648,7 @@ "storageOption": "RAM", "singleton": 0, "bounded": 0, - "defaultValue": "2", + "defaultValue": "3", "reportable": 1, "minInterval": 1, "maxInterval": 65534, @@ -9804,7 +9804,7 @@ "storageOption": "RAM", "singleton": 0, "bounded": 0, - "defaultValue": "2", + "defaultValue": "3", "reportable": 1, "minInterval": 1, "maxInterval": 65534, diff --git a/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h b/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h index 9eff91f3832e75..a28eb56c6eb299 100644 --- a/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h +++ b/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h @@ -977,13 +977,13 @@ { ZAP_EMPTY_DEFAULT(), 0x00000000, 0, ZAP_TYPE(ARRAY), ZAP_ATTRIBUTE_MASK(EXTERNAL_STORAGE) }, /* SupportedModes */ \ { ZAP_EMPTY_DEFAULT(), 0x00000001, 1, ZAP_TYPE(INT8U), ZAP_ATTRIBUTE_MASK(EXTERNAL_STORAGE) }, /* CurrentMode */ \ { ZAP_EMPTY_DEFAULT(), 0x0000FFFC, 4, ZAP_TYPE(BITMAP32), ZAP_ATTRIBUTE_MASK(EXTERNAL_STORAGE) }, /* FeatureMap */ \ - { ZAP_SIMPLE_DEFAULT(2), 0x0000FFFD, 2, ZAP_TYPE(INT16U), 0 }, /* ClusterRevision */ \ + { ZAP_SIMPLE_DEFAULT(3), 0x0000FFFD, 2, ZAP_TYPE(INT16U), 0 }, /* ClusterRevision */ \ \ /* Endpoint: 1, Cluster: RVC Clean Mode (server) */ \ { ZAP_EMPTY_DEFAULT(), 0x00000000, 0, ZAP_TYPE(ARRAY), ZAP_ATTRIBUTE_MASK(EXTERNAL_STORAGE) }, /* SupportedModes */ \ { ZAP_EMPTY_DEFAULT(), 0x00000001, 1, ZAP_TYPE(INT8U), ZAP_ATTRIBUTE_MASK(EXTERNAL_STORAGE) }, /* CurrentMode */ \ { ZAP_EMPTY_DEFAULT(), 0x0000FFFC, 4, ZAP_TYPE(BITMAP32), ZAP_ATTRIBUTE_MASK(EXTERNAL_STORAGE) }, /* FeatureMap */ \ - { ZAP_SIMPLE_DEFAULT(2), 0x0000FFFD, 2, ZAP_TYPE(INT16U), 0 }, /* ClusterRevision */ \ + { ZAP_SIMPLE_DEFAULT(3), 0x0000FFFD, 2, ZAP_TYPE(INT16U), 0 }, /* ClusterRevision */ \ \ /* Endpoint: 1, Cluster: Temperature Control (server) */ \ { ZAP_SIMPLE_DEFAULT(0), 0x00000004, 1, ZAP_TYPE(INT8U), 0 }, /* SelectedTemperatureLevel */ \ diff --git a/src/app/zap-templates/zcl/data-model/chip/rvc-clean-mode-cluster.xml b/src/app/zap-templates/zcl/data-model/chip/rvc-clean-mode-cluster.xml index c528759d2a8cb7..afd87fd18f3d54 100644 --- a/src/app/zap-templates/zcl/data-model/chip/rvc-clean-mode-cluster.xml +++ b/src/app/zap-templates/zcl/data-model/chip/rvc-clean-mode-cluster.xml @@ -37,8 +37,19 @@ limitations under the License. true true Attributes and commands for selecting a mode from a list of supported options. - + + + + + + + + + SupportedModes CurrentMode @@ -62,10 +73,4 @@ limitations under the License. - - - - - - diff --git a/src/app/zap-templates/zcl/data-model/chip/rvc-run-mode-cluster.xml b/src/app/zap-templates/zcl/data-model/chip/rvc-run-mode-cluster.xml index f13f86ac2dc97e..df27b2c15e0b83 100644 --- a/src/app/zap-templates/zcl/data-model/chip/rvc-run-mode-cluster.xml +++ b/src/app/zap-templates/zcl/data-model/chip/rvc-run-mode-cluster.xml @@ -44,7 +44,19 @@ limitations under the License. true true Attributes and commands for selecting a mode from a list of supported options. - + + + + + + + + + + SupportedModes CurrentMode @@ -67,10 +79,4 @@ limitations under the License. - - - - - - diff --git a/src/controller/data_model/controller-clusters.matter b/src/controller/data_model/controller-clusters.matter index 9b73af37ddb32e..42086a2caa93c5 100644 --- a/src/controller/data_model/controller-clusters.matter +++ b/src/controller/data_model/controller-clusters.matter @@ -3453,7 +3453,7 @@ cluster LaundryWasherControls = 83 { /** Attributes and commands for selecting a mode from a list of supported options. */ cluster RvcRunMode = 84 { - revision 2; + revision 3; enum ModeTag : enum16 { kIdle = 16384; @@ -3473,7 +3473,7 @@ cluster RvcRunMode = 84 { } bitmap Feature : bitmap32 { - kNoFeatures = 0x0; + kDirectModeChange = 0x10000; } struct ModeTagStruct { @@ -3512,7 +3512,7 @@ cluster RvcRunMode = 84 { /** Attributes and commands for selecting a mode from a list of supported options. */ cluster RvcCleanMode = 85 { - revision 2; + revision 3; enum ModeTag : enum16 { kDeepClean = 16384; @@ -3525,7 +3525,7 @@ cluster RvcCleanMode = 85 { } bitmap Feature : bitmap32 { - kNoFeatures = 0x0; + kDirectModeChange = 0x10000; } struct ModeTagStruct { diff --git a/src/controller/data_model/controller-clusters.zap b/src/controller/data_model/controller-clusters.zap index 0c4c6a7a1a842e..32b6ea18fc8fb8 100644 --- a/src/controller/data_model/controller-clusters.zap +++ b/src/controller/data_model/controller-clusters.zap @@ -2332,7 +2332,7 @@ "storageOption": "RAM", "singleton": 0, "bounded": 0, - "defaultValue": "2", + "defaultValue": "3", "reportable": 1, "minInterval": 1, "maxInterval": 65534, @@ -2392,7 +2392,7 @@ "storageOption": "RAM", "singleton": 0, "bounded": 0, - "defaultValue": "2", + "defaultValue": "3", "reportable": 1, "minInterval": 1, "maxInterval": 65534, diff --git a/src/controller/python/chip/clusters/Objects.py b/src/controller/python/chip/clusters/Objects.py index 8c9cbe7db50b87..a5988bbf54e71a 100644 --- a/src/controller/python/chip/clusters/Objects.py +++ b/src/controller/python/chip/clusters/Objects.py @@ -17551,7 +17551,7 @@ class StatusCode(MatterIntEnum): class Bitmaps: class Feature(IntFlag): - kNoFeatures = 0x0 + kDirectModeChange = 0x10000 class Structs: @dataclass @@ -17795,7 +17795,7 @@ class StatusCode(MatterIntEnum): class Bitmaps: class Feature(IntFlag): - kNoFeatures = 0x0 + kDirectModeChange = 0x10000 class Structs: @dataclass diff --git a/src/darwin/Framework/CHIP/zap-generated/MTRBaseClusters.h b/src/darwin/Framework/CHIP/zap-generated/MTRBaseClusters.h index cf8380030b57cb..bedf8161036583 100644 --- a/src/darwin/Framework/CHIP/zap-generated/MTRBaseClusters.h +++ b/src/darwin/Framework/CHIP/zap-generated/MTRBaseClusters.h @@ -18583,7 +18583,7 @@ typedef NS_ENUM(uint8_t, MTRRVCRunModeStatusCode) { } MTR_AVAILABLE(ios(17.4), macos(14.4), watchos(10.4), tvos(17.4)); typedef NS_OPTIONS(uint32_t, MTRRVCRunModeFeature) { - MTRRVCRunModeFeatureNoFeatures MTR_PROVISIONALLY_AVAILABLE = 0x0, + MTRRVCRunModeFeatureDirectModeChange MTR_PROVISIONALLY_AVAILABLE = 0x10000, } MTR_AVAILABLE(ios(17.4), macos(14.4), watchos(10.4), tvos(17.4)); typedef NS_ENUM(uint16_t, MTRRVCCleanModeModeTag) { @@ -18597,7 +18597,7 @@ typedef NS_ENUM(uint8_t, MTRRVCCleanModeStatusCode) { } MTR_AVAILABLE(ios(17.4), macos(14.4), watchos(10.4), tvos(17.4)); typedef NS_OPTIONS(uint32_t, MTRRVCCleanModeFeature) { - MTRRVCCleanModeFeatureNoFeatures MTR_PROVISIONALLY_AVAILABLE = 0x0, + MTRRVCCleanModeFeatureDirectModeChange MTR_PROVISIONALLY_AVAILABLE = 0x10000, } MTR_AVAILABLE(ios(17.4), macos(14.4), watchos(10.4), tvos(17.4)); typedef NS_OPTIONS(uint32_t, MTRTemperatureControlFeature) { diff --git a/zzz_generated/app-common/app-common/zap-generated/cluster-enums.h b/zzz_generated/app-common/app-common/zap-generated/cluster-enums.h index 998a3c931b42b1..b1a50bcd44a722 100644 --- a/zzz_generated/app-common/app-common/zap-generated/cluster-enums.h +++ b/zzz_generated/app-common/app-common/zap-generated/cluster-enums.h @@ -2155,7 +2155,7 @@ enum class StatusCode : uint8_t // Bitmap for Feature enum class Feature : uint32_t { - kNoFeatures = 0x0, + kDirectModeChange = 0x10000, }; } // namespace RvcRunMode @@ -2188,7 +2188,7 @@ enum class StatusCode : uint8_t // Bitmap for Feature enum class Feature : uint32_t { - kNoFeatures = 0x0, + kDirectModeChange = 0x10000, }; } // namespace RvcCleanMode From 8f9e8f18a5f5a1ba1e27119f2ae5b947f36c2285 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Thu, 29 Aug 2024 16:21:30 +0200 Subject: [PATCH 12/13] [Fabric-Sync] Fix for PINCode and port customization in fabric-bridge (#35248) Fixes #35030 --- docs/guides/fabric_synchronization_guide.md | 2 +- .../commands/fabric-sync/FabricSyncCommand.cpp | 11 ++++++++++- .../commands/fabric-sync/FabricSyncCommand.h | 14 +++++++++++--- .../fabric-admin/device_manager/DeviceManager.cpp | 13 +++++-------- .../fabric-admin/device_manager/DeviceManager.h | 10 +++++++++- 5 files changed, 36 insertions(+), 14 deletions(-) diff --git a/docs/guides/fabric_synchronization_guide.md b/docs/guides/fabric_synchronization_guide.md index 22d616211fbd01..e5d47dda8f9563 100644 --- a/docs/guides/fabric_synchronization_guide.md +++ b/docs/guides/fabric_synchronization_guide.md @@ -107,7 +107,7 @@ fabricsync add-local-bridge 1 Pair the Ecosystem 2 bridge to Ecosystem 1 with node ID 2: ``` -fabricsync add-bridge 2 +fabricsync add-bridge 2 ``` This command will initiate the reverse commissioning process. After a few diff --git a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp index 6a7b5be05bb3bd..3c389c468f9485 100644 --- a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp +++ b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp @@ -104,7 +104,7 @@ CHIP_ERROR FabricSyncAddBridgeCommand::RunCommand(NodeId remoteId) pairingCommand->RegisterCommissioningDelegate(this); mBridgeNodeId = remoteId; - DeviceMgr().PairRemoteFabricBridge(remoteId, reinterpret_cast(mRemoteAddr.data())); + DeviceMgr().PairRemoteFabricBridge(remoteId, mSetupPINCode, reinterpret_cast(mRemoteAddr.data()), mRemotePort); return CHIP_NO_ERROR; } @@ -207,6 +207,15 @@ CHIP_ERROR FabricSyncAddLocalBridgeCommand::RunCommand(NodeId deviceId) pairingCommand->RegisterCommissioningDelegate(this); mLocalBridgeNodeId = deviceId; + if (mSetupPINCode.HasValue()) + { + DeviceMgr().SetLocalBridgeSetupPinCode(mSetupPINCode.Value()); + } + if (mLocalPort.HasValue()) + { + DeviceMgr().SetLocalBridgePort(mLocalPort.Value()); + } + DeviceMgr().PairLocalFabricBridge(deviceId); return CHIP_NO_ERROR; diff --git a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h index 1bbd22d293f39d..669edd75d653e6 100644 --- a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h +++ b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h @@ -31,8 +31,10 @@ class FabricSyncAddBridgeCommand : public CHIPCommand, public CommissioningDeleg public: FabricSyncAddBridgeCommand(CredentialIssuerCommands * credIssuerCommands) : CHIPCommand("add-bridge", credIssuerCommands) { - AddArgument("nodeid", 0, UINT64_MAX, &mNodeId); - AddArgument("device-remote-ip", &mRemoteAddr); + AddArgument("node-id", 0, UINT64_MAX, &mNodeId); + AddArgument("setup-pin-code", 0, 0x7FFFFFF, &mSetupPINCode, "Setup PIN code for the remote bridge device."); + AddArgument("device-remote-ip", &mRemoteAddr, "The IP address of the remote bridge device."); + AddArgument("device-remote-port", 0, UINT16_MAX, &mRemotePort, "The secured device port of the remote bridge device."); } void OnCommissioningComplete(chip::NodeId deviceId, CHIP_ERROR err) override; @@ -45,7 +47,9 @@ class FabricSyncAddBridgeCommand : public CHIPCommand, public CommissioningDeleg private: chip::NodeId mNodeId; chip::NodeId mBridgeNodeId; + uint32_t mSetupPINCode; chip::ByteSpan mRemoteAddr; + uint16_t mRemotePort; CHIP_ERROR RunCommand(NodeId remoteId); }; @@ -73,7 +77,9 @@ class FabricSyncAddLocalBridgeCommand : public CHIPCommand, public Commissioning FabricSyncAddLocalBridgeCommand(CredentialIssuerCommands * credIssuerCommands) : CHIPCommand("add-local-bridge", credIssuerCommands) { - AddArgument("nodeid", 0, UINT64_MAX, &mNodeId); + AddArgument("node-id", 0, UINT64_MAX, &mNodeId); + AddArgument("setup-pin-code", 0, 0x7FFFFFF, &mSetupPINCode, "Setup PIN code for the local bridge device."); + AddArgument("local-port", 0, UINT16_MAX, &mLocalPort, "The secured device port of the local bridge device."); } void OnCommissioningComplete(NodeId deviceId, CHIP_ERROR err) override; @@ -85,6 +91,8 @@ class FabricSyncAddLocalBridgeCommand : public CHIPCommand, public Commissioning private: chip::NodeId mNodeId; + chip::Optional mSetupPINCode; + chip::Optional mLocalPort; chip::NodeId mLocalBridgeNodeId; CHIP_ERROR RunCommand(chip::NodeId deviceId); diff --git a/examples/fabric-admin/device_manager/DeviceManager.cpp b/examples/fabric-admin/device_manager/DeviceManager.cpp index ad8def646a0594..54c4bd8d45debf 100644 --- a/examples/fabric-admin/device_manager/DeviceManager.cpp +++ b/examples/fabric-admin/device_manager/DeviceManager.cpp @@ -30,10 +30,6 @@ using namespace chip::app::Clusters; namespace { -// Constants -constexpr uint32_t kSetupPinCode = 20202021; -constexpr uint16_t kRemoteBridgePort = 5540; -constexpr uint16_t kLocalBridgePort = 5540; constexpr uint16_t kWindowTimeout = 300; constexpr uint16_t kIteration = 1000; constexpr uint16_t kSubscribeMinInterval = 0; @@ -137,7 +133,7 @@ void DeviceManager::OpenRemoteDeviceCommissioningWindow(EndpointId remoteEndpoin // that is part of a different fabric, accessed through a fabric bridge. StringBuilder commandBuilder; - // Use random discriminator to have less chance of collission. + // Use random discriminator to have less chance of collision. uint16_t discriminator = Crypto::GetRandU16() % (kMaxDiscriminatorLength + 1); // Include the upper limit kMaxDiscriminatorLength @@ -148,12 +144,13 @@ void DeviceManager::OpenRemoteDeviceCommissioningWindow(EndpointId remoteEndpoin PushCommand(commandBuilder.c_str()); } -void DeviceManager::PairRemoteFabricBridge(NodeId nodeId, const char * deviceRemoteIp) +void DeviceManager::PairRemoteFabricBridge(chip::NodeId nodeId, uint32_t setupPINCode, const char * deviceRemoteIp, + uint16_t deviceRemotePort) { StringBuilder commandBuilder; commandBuilder.Add("pairing already-discovered "); - commandBuilder.AddFormat("%lu %d %s %d", nodeId, kSetupPinCode, deviceRemoteIp, kRemoteBridgePort); + commandBuilder.AddFormat("%lu %d %s %d", nodeId, setupPINCode, deviceRemoteIp, deviceRemotePort); PushCommand(commandBuilder.c_str()); } @@ -173,7 +170,7 @@ void DeviceManager::PairLocalFabricBridge(NodeId nodeId) StringBuilder commandBuilder; commandBuilder.Add("pairing already-discovered "); - commandBuilder.AddFormat("%lu %d ::1 %d", nodeId, kSetupPinCode, kLocalBridgePort); + commandBuilder.AddFormat("%lu %d ::1 %d", nodeId, mLocalBridgeSetupPinCode, mLocalBridgePort); PushCommand(commandBuilder.c_str()); } diff --git a/examples/fabric-admin/device_manager/DeviceManager.h b/examples/fabric-admin/device_manager/DeviceManager.h index fa94f3d09d18cf..22a3004aa1642b 100644 --- a/examples/fabric-admin/device_manager/DeviceManager.h +++ b/examples/fabric-admin/device_manager/DeviceManager.h @@ -24,6 +24,8 @@ #include +constexpr uint32_t kDefaultSetupPinCode = 20202021; +constexpr uint16_t kDefaultLocalBridgePort = 5540; constexpr uint16_t kResponseTimeoutSeconds = 30; class Device @@ -61,6 +63,8 @@ class DeviceManager : public PairingDelegate void SetRemoteBridgeNodeId(chip::NodeId nodeId) { mRemoteBridgeNodeId = nodeId; } + void SetLocalBridgePort(uint16_t port) { mLocalBridgePort = port; } + void SetLocalBridgeSetupPinCode(uint32_t pinCode) { mLocalBridgeSetupPinCode = pinCode; } void SetLocalBridgeNodeId(chip::NodeId nodeId) { mLocalBridgeNodeId = nodeId; } bool IsAutoSyncEnabled() const { return mAutoSyncEnabled; } @@ -125,9 +129,11 @@ class DeviceManager : public PairingDelegate * @param nodeId The user-defined ID for the node being commissioned. It doesn’t need to be the same ID, * as for the first fabric. + * @param setupPINCode The setup PIN code used to authenticate the pairing process. * @param deviceRemoteIp The IP address of the remote device that is being paired as part of the fabric bridge. + * @param deviceRemotePort The secured device port of the remote device that is being paired as part of the fabric bridge. */ - void PairRemoteFabricBridge(chip::NodeId nodeId, const char * deviceRemoteIp); + void PairRemoteFabricBridge(chip::NodeId nodeId, uint32_t setupPINCode, const char * deviceRemoteIp, uint16_t deviceRemotePort); /** * @brief Pair a remote Matter device to the current fabric. @@ -190,6 +196,8 @@ class DeviceManager : public PairingDelegate // This represents the bridge on the other ecosystem. chip::NodeId mRemoteBridgeNodeId = chip::kUndefinedNodeId; + uint16_t mLocalBridgePort = kDefaultLocalBridgePort; + uint32_t mLocalBridgeSetupPinCode = kDefaultSetupPinCode; // The Node ID of the local bridge used for Fabric-Sync // This represents the bridge within its own ecosystem. chip::NodeId mLocalBridgeNodeId = chip::kUndefinedNodeId; From cfdbba92318717c09efe52a88c8ba60b995f5be9 Mon Sep 17 00:00:00 2001 From: Amine Alami <43780877+Alami-Amine@users.noreply.github.com> Date: Thu, 29 Aug 2024 17:16:41 +0200 Subject: [PATCH 13/13] Docker Image build fix: adding abseil as system lib (#35277) * image build fix: adding abseil to docker image * adding reference to issue --- integrations/docker/images/base/chip-build/Dockerfile | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/integrations/docker/images/base/chip-build/Dockerfile b/integrations/docker/images/base/chip-build/Dockerfile index db209a82f20b87..37edf89730ecd9 100644 --- a/integrations/docker/images/base/chip-build/Dockerfile +++ b/integrations/docker/images/base/chip-build/Dockerfile @@ -123,6 +123,14 @@ RUN set -x \ ruff \ && : # last line +#TODO Issue #35280: this is only added as a workaround to bloaty build failures, remove it once bloaty fixes issue +# Clone and install abseil-cpp +RUN git clone https://github.com/abseil/abseil-cpp.git /tmp/abseil-cpp \ + && cd /tmp/abseil-cpp \ + && cmake -B build -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/usr/local \ + && cmake --build build --target install \ + && rm -rf /tmp/abseil-cpp + # Install bloat comparison tools RUN set -x \ && git clone https://github.com/google/bloaty.git \