From 8f9e8f18a5f5a1ba1e27119f2ae5b947f36c2285 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Thu, 29 Aug 2024 16:21:30 +0200 Subject: [PATCH 01/12] [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 02/12] 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 \ From 3e529e1bb72dbe96ed9363f5a8a4687631ad6847 Mon Sep 17 00:00:00 2001 From: Raul Marquez <130402456+raul-marquez-csa@users.noreply.github.com> Date: Thu, 29 Aug 2024 09:01:45 -0700 Subject: [PATCH 03/12] Removes verification item (#35049) --- src/app/tests/suites/certification/Test_TC_DEMM_1_2.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/src/app/tests/suites/certification/Test_TC_DEMM_1_2.yaml b/src/app/tests/suites/certification/Test_TC_DEMM_1_2.yaml index 55aa1400ec1ac5..900428998445ea 100644 --- a/src/app/tests/suites/certification/Test_TC_DEMM_1_2.yaml +++ b/src/app/tests/suites/certification/Test_TC_DEMM_1_2.yaml @@ -37,7 +37,6 @@ tests: at least one entry the values of the Value fields that are not larger than 16 bits - for each Value field: Is the mode tag value a defined common tag value (Auto(0x0000), Quick(0x0001), Quiet(0x0002), LowNoise(0x0003), LowEnergy(0x0004), Vacation(0x0005), Min(0x0006), Max(0x0007), Night(0x0008), Day(0x0009)) or a defined cluster-derived tag value (No Optimization, Device Optimization, Local Optimization, Grid Optimization) or in the MfgTags (0x8000 to 0xBFFF) range - for at least one Value field: Is the mode tag value a defined common tag value (Auto(0x0000), Quick(0x0001), Quiet(0x0002), LowNoise(0x0003), LowEnergy(0x0004), Vacation(0x0005), Min(0x0006), Max(0x0007), Night(0x0008), Day(0x0009)) or a derived cluster value (RapidCool, RapidFreeze) - - if the Value field is in the MfgTags (0x8000 to 0xBFFF) range, the TagName field is a string with a length between 1 and 64 - Verify that at least one ModeOptionsStruct entry includes either the RapidCool semantic tag or the RapidFreeze semantic tag in the SemanticTags field - Save the Mode field values as supported_modes_dut on the TH (Chip-tool) and below is the sample log provided for the raspi platform: From 9a0add3be9c25a082ea26a36b807c8bfc00916ab Mon Sep 17 00:00:00 2001 From: Nivi Sarkar <55898241+nivi-apple@users.noreply.github.com> Date: Thu, 29 Aug 2024 09:03:49 -0700 Subject: [PATCH 04/12] Make fixes to match the test scripts to the test plan (#35265) * Make fixes to match the test scripts to the test plan - Set the PICS code for SetpointHoldExpiryTimestamp to 0 - Remove the test cases for GetRelayLog and GetRelayLogResponse - Update PICS code for Preset related commands * Restyled by prettier-yaml --------- Co-authored-by: Restyled.io --- src/app/tests/suites/certification/PICS.yaml | 7 ++++ .../certification/Test_TC_TSTAT_1_1.yaml | 33 ++++--------------- .../tests/suites/certification/ci-pics-values | 1 + 3 files changed, 14 insertions(+), 27 deletions(-) diff --git a/src/app/tests/suites/certification/PICS.yaml b/src/app/tests/suites/certification/PICS.yaml index be995be90b30b7..89638f4e631747 100644 --- a/src/app/tests/suites/certification/PICS.yaml +++ b/src/app/tests/suites/certification/PICS.yaml @@ -6494,6 +6494,13 @@ PICS: - label: "Does the device implement the Schedules attribute?" id: TSTAT.S.A0051 + - label: "Does the device implement the Schedules attribute?" + id: TSTAT.S.A0051 + + - label: + "Does the device implement the SetpointHoldExpiryTimestamp attribute?" + id: TSTAT.S.A0052 + # # server / commandsReceived # diff --git a/src/app/tests/suites/certification/Test_TC_TSTAT_1_1.yaml b/src/app/tests/suites/certification/Test_TC_TSTAT_1_1.yaml index dc16d0d8a6c4bb..ef8f688bf5e89c 100644 --- a/src/app/tests/suites/certification/Test_TC_TSTAT_1_1.yaml +++ b/src/app/tests/suites/certification/Test_TC_TSTAT_1_1.yaml @@ -606,17 +606,7 @@ tests: contains: [1, 2, 3] - label: - "Step 6c: TH reads the optional (GetRelayStatusLog) command in - AcceptedCommandList" - PICS: TSTAT.S.C04.Rsp - command: "readAttribute" - attribute: "AcceptedCommandList" - response: - constraints: - type: list - contains: [4] - - label: - "Step 6d: TH reads Feature dependent(TSTAT.S.F08(PRES)) commands in + "Step 6c: TH reads Feature dependent(TSTAT.S.F08(PRES)) commands in AcceptedCommandList" PICS: TSTAT.S.F08 command: "readAttribute" @@ -627,18 +617,7 @@ tests: contains: [6, 254] - label: - "Step 7a: TH reads Feature dependent(TSTAT.S.F08(PRES)) commands in - the GeneratedCommandList attribute." - PICS: TSTAT.S.Cfe.Rsp - command: "readAttribute" - attribute: "GeneratedCommandList" - response: - value: [0xFD] # AtomicResponse - constraints: - type: list - - - label: - "Step 7b: TH reads Feature dependent(TSTAT.S.F03(SCH)) commands in + "Step 7a: TH reads Feature dependent(TSTAT.S.F03(SCH)) commands in GeneratedCommandList" PICS: TSTAT.S.F03 command: "readAttribute" @@ -649,12 +628,12 @@ tests: contains: [0] - label: - "Step 7c: TH reads optional command (GetRelayStatusLogResponse) in - GeneratedCommandList" - PICS: TSTAT.S.C04.Rsp + "Step 7b: TH reads Feature dependent(TSTAT.S.F08(PRES)) commands in + the GeneratedCommandList attribute." + PICS: TSTAT.S.F08 & TSTAT.S.Cfe.Rsp command: "readAttribute" attribute: "GeneratedCommandList" response: + value: [0xFD] # AtomicResponse constraints: type: list - contains: [1] diff --git a/src/app/tests/suites/certification/ci-pics-values b/src/app/tests/suites/certification/ci-pics-values index 373f8e3c9aea9c..24abab5e9aacb3 100644 --- a/src/app/tests/suites/certification/ci-pics-values +++ b/src/app/tests/suites/certification/ci-pics-values @@ -1989,6 +1989,7 @@ TSTAT.S.A004a=1 TSTAT.S.A004e=1 TSTAT.S.A0050=1 TSTAT.S.A0051=0 +TSTAT.S.A0052=0 TSTAT.S.M.MinSetpointDeadBandWritable=1 TSTAT.S.M.HVACSystemTypeConfigurationWritable=0 From 68371622f60003b9264aa2e61149462a3cd24698 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Thu, 29 Aug 2024 09:31:17 -0700 Subject: [PATCH 05/12] Fix crash when try to read UniqueId from a legacy device (#35268) --- .../fabric-admin/device_manager/DeviceSynchronization.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/examples/fabric-admin/device_manager/DeviceSynchronization.cpp b/examples/fabric-admin/device_manager/DeviceSynchronization.cpp index 69ca9cdb722b23..3239f89046fb6e 100644 --- a/examples/fabric-admin/device_manager/DeviceSynchronization.cpp +++ b/examples/fabric-admin/device_manager/DeviceSynchronization.cpp @@ -72,6 +72,13 @@ void DeviceSynchronizer::OnAttributeData(const ConcreteDataAttributePath & path, VerifyOrDie(path.mEndpointId == kRootEndpointId); VerifyOrDie(path.mClusterId == Clusters::BasicInformation::Id); + CHIP_ERROR error = status.ToChipError(); + if (CHIP_NO_ERROR != error) + { + ChipLogError(NotSpecified, "Response Failure: %" CHIP_ERROR_FORMAT, error.Format()); + return; + } + switch (path.mAttributeId) { case Clusters::BasicInformation::Attributes::UniqueID::Id: From 95ee15751c0796979b6047e241f6a67fa8474e8d Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho <116586593+rquidute@users.noreply.github.com> Date: Thu, 29 Aug 2024 14:04:57 -0300 Subject: [PATCH 06/12] Add apps to chip cert bins image (#35207) * Add Apps to chip_cert_bins image * Minor fixes --- .../docker/images/chip-cert-bins/Dockerfile | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/integrations/docker/images/chip-cert-bins/Dockerfile b/integrations/docker/images/chip-cert-bins/Dockerfile index 99b4f38a0caa2b..c386359c718ed5 100644 --- a/integrations/docker/images/chip-cert-bins/Dockerfile +++ b/integrations/docker/images/chip-cert-bins/Dockerfile @@ -193,6 +193,9 @@ RUN case ${TARGETPLATFORM} in \ --target linux-x64-energy-management-ipv6only \ --target linux-x64-microwave-oven-ipv6only \ --target linux-x64-rvc-ipv6only \ + --target linux-x64-fabric-bridge-rpc-ipv6only \ + --target linux-x64-fabric-admin-rpc-ipv6only \ + --target linux-x64-network-manager-ipv6only \ build \ && mv out/linux-x64-chip-tool-ipv6only-platform-mdns/chip-tool out/chip-tool \ && mv out/linux-x64-shell-ipv6only-platform-mdns/chip-shell out/chip-shell \ @@ -213,6 +216,9 @@ RUN case ${TARGETPLATFORM} in \ && mv out/linux-x64-energy-management-ipv6only/chip-energy-management-app out/chip-energy-management-app \ && mv out/linux-x64-microwave-oven-ipv6only/chip-microwave-oven-app out/chip-microwave-oven-app \ && mv out/linux-x64-rvc-ipv6only/chip-rvc-app out/chip-rvc-app \ + && mv out/linux-x64-fabric-bridge-rpc-ipv6only/fabric-bridge-app out/fabric-bridge-app \ + && mv out/linux-x64-fabric-admin-rpc-ipv6only/fabric-admin out/fabric-admin \ + && mv out/linux-x64-network-manager-ipv6only/matter-network-manager-app out/matter-network-manager-app \ ;; \ "linux/arm64")\ set -x \ @@ -237,6 +243,9 @@ RUN case ${TARGETPLATFORM} in \ --target linux-arm64-energy-management-ipv6only \ --target linux-arm64-microwave-oven-ipv6only \ --target linux-arm64-rvc-ipv6only \ + --target linux-arm64-fabric-bridge-rpc-ipv6only \ + --target linux-arm64-fabric-admin-rpc-ipv6only \ + --target linux-arm64-network-manager-ipv6only \ build \ && mv out/linux-arm64-chip-tool-ipv6only-platform-mdns/chip-tool out/chip-tool \ && mv out/linux-arm64-shell-ipv6only-platform-mdns/chip-shell out/chip-shell \ @@ -257,6 +266,9 @@ RUN case ${TARGETPLATFORM} in \ && mv out/linux-arm64-energy-management-ipv6only/chip-energy-management-app out/chip-energy-management-app \ && mv out/linux-arm64-microwave-oven-ipv6only/chip-microwave-oven-app out/chip-microwave-oven-app \ && mv out/linux-arm64-rvc-ipv6only/chip-rvc-app out/chip-rvc-app \ + && mv out/linux-arm64-fabric-bridge-rpc-ipv6only/fabric-bridge-app out/fabric-bridge-app \ + && mv out/linux-arm64-fabric-admin-rpc-ipv6only/fabric-admin out/fabric-admin \ + && mv out/linux-arm64-network-manager-ipv6only/matter-network-manager-app out/matter-network-manager-app \ ;; \ *) ;; \ esac @@ -290,6 +302,9 @@ COPY --from=chip-build-cert-bins /root/connectedhomeip/out/lit-icd-app lit-icd-a COPY --from=chip-build-cert-bins /root/connectedhomeip/out/chip-energy-management-app chip-energy-management-app COPY --from=chip-build-cert-bins /root/connectedhomeip/out/chip-microwave-oven-app chip-microwave-oven-app COPY --from=chip-build-cert-bins /root/connectedhomeip/out/chip-rvc-app chip-rvc-app +COPY --from=chip-build-cert-bins /root/connectedhomeip/out/fabric-bridge-app fabric-bridge-app +COPY --from=chip-build-cert-bins /root/connectedhomeip/out/fabric-admin fabric-admin +COPY --from=chip-build-cert-bins /root/connectedhomeip/out/matter-network-manager-app matter-network-manager-app # Stage 3.1: Setup the Matter Python environment COPY --from=chip-build-cert-bins /root/connectedhomeip/out/python_lib python_lib @@ -304,6 +319,6 @@ COPY --from=chip-build-cert-bins /root/connectedhomeip/src/python_testing/requir RUN pip install --break-system-packages -r /tmp/requirements.txt && rm /tmp/requirements.txt # PIP requires MASON package compilation, which seems to require a JDK -RUN set -x && DEBIAN_FRONTEND=noninteractive apt-get install -fy openjdk-8-jdk +RUN set -x && DEBIAN_FRONTEND=noninteractive apt-get update; apt-get install -fy openjdk-8-jdk RUN pip install --break-system-packages --no-cache-dir python_lib/controller/python/chip*.whl From 9c598998609cc756d2c79fe7975721b76f92b07b Mon Sep 17 00:00:00 2001 From: shripad621git <79364691+shripad621git@users.noreply.github.com> Date: Thu, 29 Aug 2024 22:44:06 +0530 Subject: [PATCH 07/12] Fixed the crash due to packet buffers running out of space in CommandHandlerImpl. (#35279) * Fixed the crash due to packet buffers running out of space in CommandHandlerImpl * Update src/app/CommandHandlerImpl.h Co-authored-by: Terence Hampson --------- Co-authored-by: Terence Hampson --- src/app/CommandHandlerImpl.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/app/CommandHandlerImpl.h b/src/app/CommandHandlerImpl.h index 55e356bcceb35a..ddd499658e15d9 100644 --- a/src/app/CommandHandlerImpl.h +++ b/src/app/CommandHandlerImpl.h @@ -294,7 +294,12 @@ class CommandHandlerImpl : public CommandHandler { return CHIP_NO_ERROR; } - ReturnErrorOnFailure(RollbackResponse()); + // The error value of RollbackResponse is not important if it fails, we prioritize + // conveying the error generated by addResponseFunction to the caller. + if (RollbackResponse() != CHIP_NO_ERROR) + { + return err; + } // If we failed to add a command due to lack of space in the // packet, we will make another attempt to add the response using // an additional InvokeResponseMessage. From 9c778ceeae57914b1bde8669e9369d2ef6288ef2 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Thu, 29 Aug 2024 20:04:40 +0200 Subject: [PATCH 08/12] Fix Fabric Sync test script issues discovered on TH (#35283) --- src/python_testing/TC_CCTRL_2_2.py | 9 ++++++--- src/python_testing/TC_CCTRL_2_3.py | 9 ++++++--- src/python_testing/TC_MCORE_FS_1_1.py | 9 ++++++--- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/python_testing/TC_CCTRL_2_2.py b/src/python_testing/TC_CCTRL_2_2.py index 4d11692d14ba24..8080f9db324ff3 100644 --- a/src/python_testing/TC_CCTRL_2_2.py +++ b/src/python_testing/TC_CCTRL_2_2.py @@ -53,11 +53,14 @@ async def setup_class(self): self.port = 5543 discriminator = random.randint(0, 4095) passcode = 20202021 - app_args = f'--secured-device-port {self.port} --discriminator {discriminator} --passcode {passcode} --KVS {self.kvs}' - cmd = f'{app} {app_args}' + cmd = [app] + cmd.extend(['--secured-device-port', str(5543)]) + cmd.extend(['--discriminator', str(discriminator)]) + cmd.extend(['--passcode', str(passcode)]) + cmd.extend(['--KVS', self.kvs]) # TODO: Determine if we want these logs cooked or pushed to somewhere else logging.info("Starting TH_SERVER") - self.app_process = subprocess.Popen(cmd, bufsize=0, shell=True) + self.app_process = subprocess.Popen(cmd) logging.info("TH_SERVER started") time.sleep(3) diff --git a/src/python_testing/TC_CCTRL_2_3.py b/src/python_testing/TC_CCTRL_2_3.py index c733235f635ff8..9baa0856412d5f 100644 --- a/src/python_testing/TC_CCTRL_2_3.py +++ b/src/python_testing/TC_CCTRL_2_3.py @@ -52,11 +52,14 @@ async def setup_class(self): self.port = 5543 discriminator = random.randint(0, 4095) passcode = 20202021 - app_args = f'--secured-device-port {self.port} --discriminator {discriminator} --passcode {passcode} --KVS {self.kvs}' - cmd = f'{app} {app_args}' + cmd = [app] + cmd.extend(['--secured-device-port', str(5543)]) + cmd.extend(['--discriminator', str(discriminator)]) + cmd.extend(['--passcode', str(passcode)]) + cmd.extend(['--KVS', self.kvs]) # TODO: Determine if we want these logs cooked or pushed to somewhere else logging.info("Starting TH_SERVER") - self.app_process = subprocess.Popen(cmd, bufsize=0, shell=True) + self.app_process = subprocess.Popen(cmd) logging.info("TH_SERVER started") time.sleep(3) diff --git a/src/python_testing/TC_MCORE_FS_1_1.py b/src/python_testing/TC_MCORE_FS_1_1.py index f64b718e96f2e8..27978af3dee234 100755 --- a/src/python_testing/TC_MCORE_FS_1_1.py +++ b/src/python_testing/TC_MCORE_FS_1_1.py @@ -45,11 +45,14 @@ async def setup_class(self): self.port = 5543 discriminator = random.randint(0, 4095) passcode = 20202021 - app_args = f'--secured-device-port {self.port} --discriminator {discriminator} --passcode {passcode} --KVS {self.kvs}' - cmd = f'{app} {app_args}' + cmd = [app] + cmd.extend(['--secured-device-port', str(5543)]) + cmd.extend(['--discriminator', str(discriminator)]) + cmd.extend(['--passcode', str(passcode)]) + cmd.extend(['--KVS', self.kvs]) # TODO: Determine if we want these logs cooked or pushed to somewhere else logging.info("Starting application to acts mock a server portion of TH_FSA") - self.app_process = subprocess.Popen(cmd, bufsize=0, shell=True) + self.app_process = subprocess.Popen(cmd) logging.info("Started application to acts mock a server portion of TH_FSA") time.sleep(3) From 401a2a0eb00f3365dd096e685548b04be07b475b Mon Sep 17 00:00:00 2001 From: Thomas Lea <35579828+tleacmcsa@users.noreply.github.com> Date: Thu, 29 Aug 2024 13:06:01 -0500 Subject: [PATCH 09/12] M-ACL: switch from UnsupportedAccess to AccessRestricted (#35263) * Implement new AccessRestricted error code * [NXP][Zephyr] Provide AP band in connection request parameters (#35181) Signed-off-by: Axel Le Bourhis * added missed ERROR_CODES.md update * fixed build issue * restyled * fix return type in CodegenDataModelProvider_Write.cpp * fix review comments --------- Signed-off-by: Axel Le Bourhis Co-authored-by: Axel Le Bourhis <45206070+axelnxp@users.noreply.github.com> --- docs/ERROR_CODES.md | 2 ++ src/access/AccessControl.cpp | 5 ----- src/access/AccessRestrictionProvider.cpp | 20 ------------------- .../tests/TestAccessRestrictionProvider.cpp | 2 +- src/app/CommandHandlerImpl.cpp | 5 +++-- src/app/EventManagement.cpp | 4 +++- .../CodegenDataModelProvider_Read.cpp | 8 +++++--- .../CodegenDataModelProvider_Write.cpp | 4 ++-- src/app/reporting/Engine.cpp | 11 ++++++---- .../util/ember-compatibility-functions.cpp | 12 +++++++---- .../chip/devicecontroller/model/Status.java | 1 + .../src/matter/controller/model/Status.kt | 1 + .../python/chip/interaction_model/__init__.py | 1 + src/lib/core/CHIPError.cpp | 3 +++ src/lib/core/CHIPError.h | 9 ++++++++- src/lib/core/tests/TestCHIPErrorStr.cpp | 1 + .../interaction_model/StatusCodeList.h | 1 + src/python_testing/TC_ACL_2_11.py | 16 +++++++-------- 18 files changed, 55 insertions(+), 51 deletions(-) diff --git a/docs/ERROR_CODES.md b/docs/ERROR_CODES.md index 5799c7a3f1b885..ff549e6df6b6dc 100644 --- a/docs/ERROR_CODES.md +++ b/docs/ERROR_CODES.md @@ -118,6 +118,7 @@ This file was **AUTOMATICALLY** generated by | 165 | 0xA5 | `CHIP_ERROR_ACCESS_DENIED` | | 166 | 0xA6 | `CHIP_ERROR_UNKNOWN_RESOURCE_ID` | | 167 | 0xA7 | `CHIP_ERROR_VERSION_MISMATCH` | +| 168 | 0xA8 | `CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL` | | 171 | 0xAB | `CHIP_ERROR_EVENT_ID_FOUND` | | 172 | 0xAC | `CHIP_ERROR_INTERNAL` | | 173 | 0xAD | `CHIP_ERROR_OPEN_FAILED` | @@ -252,6 +253,7 @@ This file was **AUTOMATICALLY** generated by | 1426 | 0x592 | `DATA_VERSION_MISMATCH` | | 1428 | 0x594 | `TIMEOUT` | | 1436 | 0x59C | `BUSY` | +| 1437 | 0x59D | `ACCESS_RESTRICTED` | | 1475 | 0x5C3 | `UNSUPPORTED_CLUSTER` | | 1477 | 0x5C5 | `NO_UPSTREAM_SUBSCRIPTION` | | 1478 | 0x5C6 | `NEEDS_TIMED_INTERACTION` | diff --git a/src/access/AccessControl.cpp b/src/access/AccessControl.cpp index 8302fb0b122265..c9da05e51038cc 100644 --- a/src/access/AccessControl.cpp +++ b/src/access/AccessControl.cpp @@ -538,12 +538,7 @@ CHIP_ERROR AccessControl::CheckARL(const SubjectDescriptor & subjectDescriptor, if (result != CHIP_NO_ERROR) { ChipLogProgress(DataManagement, "AccessControl: %s", -#if 0 - // TODO(#35177): new error code coming when access check plumbing are fixed in callers (result == CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL) ? "denied (restricted)" : "denied (restriction error)"); -#else - (result == CHIP_ERROR_ACCESS_DENIED) ? "denied (restricted)" : "denied (restriction error)"); -#endif return result; } diff --git a/src/access/AccessRestrictionProvider.cpp b/src/access/AccessRestrictionProvider.cpp index 23e8082353abc8..e1a818fde0fc41 100644 --- a/src/access/AccessRestrictionProvider.cpp +++ b/src/access/AccessRestrictionProvider.cpp @@ -197,45 +197,25 @@ CHIP_ERROR AccessRestrictionProvider::DoCheck(const std::vector & entries if (requestPath.requestType == RequestType::kAttributeReadRequest || requestPath.requestType == RequestType::kAttributeWriteRequest) { -#if 0 - // TODO(#35177): use new ARL error code when access checks are fixed return CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL; -#else - return CHIP_ERROR_ACCESS_DENIED; -#endif } break; case Type::kAttributeWriteForbidden: if (requestPath.requestType == RequestType::kAttributeWriteRequest) { -#if 0 - // TODO(#35177): use new ARL error code when access checks are fixed return CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL; -#else - return CHIP_ERROR_ACCESS_DENIED; -#endif } break; case Type::kCommandForbidden: if (requestPath.requestType == RequestType::kCommandInvokeRequest) { -#if 0 - // TODO(#35177): use new ARL error code when access checks are fixed return CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL; -#else - return CHIP_ERROR_ACCESS_DENIED; -#endif } break; case Type::kEventForbidden: if (requestPath.requestType == RequestType::kEventReadRequest) { -#if 0 - // TODO(#35177): use new ARL error code when access checks are fixed return CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL; -#else - return CHIP_ERROR_ACCESS_DENIED; -#endif } break; } diff --git a/src/access/tests/TestAccessRestrictionProvider.cpp b/src/access/tests/TestAccessRestrictionProvider.cpp index ddf58ae2488f4d..723d023e48b2ff 100644 --- a/src/access/tests/TestAccessRestrictionProvider.cpp +++ b/src/access/tests/TestAccessRestrictionProvider.cpp @@ -174,7 +174,7 @@ void RunChecks(const CheckData * checkData, size_t count) { for (size_t i = 0; i < count; i++) { - CHIP_ERROR expectedResult = checkData[i].allow ? CHIP_NO_ERROR : CHIP_ERROR_ACCESS_DENIED; + CHIP_ERROR expectedResult = checkData[i].allow ? CHIP_NO_ERROR : CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL; EXPECT_EQ(accessControl.Check(checkData[i].subjectDescriptor, checkData[i].requestPath, checkData[i].privilege), expectedResult); } diff --git a/src/app/CommandHandlerImpl.cpp b/src/app/CommandHandlerImpl.cpp index 1945e7e5e69dc3..366199a3b9f1d4 100644 --- a/src/app/CommandHandlerImpl.cpp +++ b/src/app/CommandHandlerImpl.cpp @@ -410,12 +410,13 @@ Status CommandHandlerImpl::ProcessCommandDataIB(CommandDataIB::Parser & aCommand err = Access::GetAccessControl().Check(subjectDescriptor, requestPath, requestPrivilege); if (err != CHIP_NO_ERROR) { - if (err != CHIP_ERROR_ACCESS_DENIED) + if ((err != CHIP_ERROR_ACCESS_DENIED) && (err != CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL)) { return FallibleAddStatus(concretePath, Status::Failure) != CHIP_NO_ERROR ? Status::Failure : Status::Success; } // TODO: when wildcard invokes are supported, handle them to discard rather than fail with status - return FallibleAddStatus(concretePath, Status::UnsupportedAccess) != CHIP_NO_ERROR ? Status::Failure : Status::Success; + Status status = err == CHIP_ERROR_ACCESS_DENIED ? Status::UnsupportedAccess : Status::AccessRestricted; + return FallibleAddStatus(concretePath, status) != CHIP_NO_ERROR ? Status::Failure : Status::Success; } } diff --git a/src/app/EventManagement.cpp b/src/app/EventManagement.cpp index 2419d564d0bc7a..7c0df844872b47 100644 --- a/src/app/EventManagement.cpp +++ b/src/app/EventManagement.cpp @@ -563,7 +563,9 @@ CHIP_ERROR EventManagement::CheckEventContext(EventLoadOutContext * eventLoadOut Access::GetAccessControl().Check(eventLoadOutContext->mSubjectDescriptor, requestPath, requestPrivilege); if (accessControlError != CHIP_NO_ERROR) { - ReturnErrorCodeIf(accessControlError != CHIP_ERROR_ACCESS_DENIED, accessControlError); + ReturnErrorCodeIf((accessControlError != CHIP_ERROR_ACCESS_DENIED) && + (accessControlError != CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL), + accessControlError); ret = CHIP_ERROR_UNEXPECTED_EVENT; } diff --git a/src/app/codegen-data-model-provider/CodegenDataModelProvider_Read.cpp b/src/app/codegen-data-model-provider/CodegenDataModelProvider_Read.cpp index de17d1059be127..82baba4835c8b5 100644 --- a/src/app/codegen-data-model-provider/CodegenDataModelProvider_Read.cpp +++ b/src/app/codegen-data-model-provider/CodegenDataModelProvider_Read.cpp @@ -281,15 +281,17 @@ DataModel::ActionReturnStatus CodegenDataModelProvider::ReadAttribute(const Data RequiredPrivilege::ForReadAttribute(request.path)); if (err != CHIP_NO_ERROR) { - ReturnErrorCodeIf(err != CHIP_ERROR_ACCESS_DENIED, err); + ReturnErrorCodeIf((err != CHIP_ERROR_ACCESS_DENIED) && (err != CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL), err); // Implementation of 8.4.3.2 of the spec for path expansion if (request.path.mExpanded) { return CHIP_NO_ERROR; } - // access denied has a specific code for IM - return CHIP_IM_GLOBAL_STATUS(UnsupportedAccess); + + // access denied and access restricted have specific codes for IM + return err == CHIP_ERROR_ACCESS_DENIED ? CHIP_IM_GLOBAL_STATUS(UnsupportedAccess) + : CHIP_IM_GLOBAL_STATUS(AccessRestricted); } } diff --git a/src/app/codegen-data-model-provider/CodegenDataModelProvider_Write.cpp b/src/app/codegen-data-model-provider/CodegenDataModelProvider_Write.cpp index 7420427f84909b..d5f50454b18085 100644 --- a/src/app/codegen-data-model-provider/CodegenDataModelProvider_Write.cpp +++ b/src/app/codegen-data-model-provider/CodegenDataModelProvider_Write.cpp @@ -287,10 +287,10 @@ DataModel::ActionReturnStatus CodegenDataModelProvider::WriteAttribute(const Dat if (err != CHIP_NO_ERROR) { - ReturnErrorCodeIf(err != CHIP_ERROR_ACCESS_DENIED, err); + ReturnErrorCodeIf((err != CHIP_ERROR_ACCESS_DENIED) && (err != CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL), err); // TODO: when wildcard/group writes are supported, handle them to discard rather than fail with status - return Status::UnsupportedAccess; + return err == CHIP_ERROR_ACCESS_DENIED ? Status::UnsupportedAccess : Status::AccessRestricted; } } diff --git a/src/app/reporting/Engine.cpp b/src/app/reporting/Engine.cpp index 99d038a8c35471..2dfbec1fdf5d6d 100644 --- a/src/app/reporting/Engine.cpp +++ b/src/app/reporting/Engine.cpp @@ -346,23 +346,26 @@ CHIP_ERROR Engine::CheckAccessDeniedEventPaths(TLV::TLVWriter & aWriter, bool & Access::Privilege requestPrivilege = RequiredPrivilege::ForReadEvent(path); err = Access::GetAccessControl().Check(apReadHandler->GetSubjectDescriptor(), requestPath, requestPrivilege); - if (err != CHIP_ERROR_ACCESS_DENIED) + if ((err != CHIP_ERROR_ACCESS_DENIED) && (err != CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL)) { ReturnErrorOnFailure(err); } else { TLV::TLVWriter checkpoint = aWriter; - err = EventReportIB::ConstructEventStatusIB(aWriter, path, StatusIB(Status::UnsupportedAccess)); + err = EventReportIB::ConstructEventStatusIB(aWriter, path, + err == CHIP_ERROR_ACCESS_DENIED ? StatusIB(Status::UnsupportedAccess) + : StatusIB(Status::AccessRestricted)); + if (err != CHIP_NO_ERROR) { aWriter = checkpoint; break; } aHasEncodedData = true; - ChipLogDetail(InteractionModel, "Access to event (%u, " ChipLogFormatMEI ", " ChipLogFormatMEI ") denied by ACL", + ChipLogDetail(InteractionModel, "Access to event (%u, " ChipLogFormatMEI ", " ChipLogFormatMEI ") denied by %s", current->mValue.mEndpointId, ChipLogValueMEI(current->mValue.mClusterId), - ChipLogValueMEI(current->mValue.mEventId)); + ChipLogValueMEI(current->mValue.mEventId), err == CHIP_ERROR_ACCESS_DENIED ? "ACL" : "ARL"); } current = current->mpNext; } diff --git a/src/app/util/ember-compatibility-functions.cpp b/src/app/util/ember-compatibility-functions.cpp index ad0542a9ff0614..7dadac8695fd63 100644 --- a/src/app/util/ember-compatibility-functions.cpp +++ b/src/app/util/ember-compatibility-functions.cpp @@ -302,12 +302,13 @@ CHIP_ERROR ReadSingleClusterData(const SubjectDescriptor & aSubjectDescriptor, b CHIP_ERROR err = Access::GetAccessControl().Check(aSubjectDescriptor, requestPath, requestPrivilege); if (err != CHIP_NO_ERROR) { - ReturnErrorCodeIf(err != CHIP_ERROR_ACCESS_DENIED, err); + ReturnErrorCodeIf((err != CHIP_ERROR_ACCESS_DENIED) && (err != CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL), err); if (aPath.mExpanded) { return CHIP_NO_ERROR; } - return CHIP_IM_GLOBAL_STATUS(UnsupportedAccess); + return err == CHIP_ERROR_ACCESS_DENIED ? CHIP_IM_GLOBAL_STATUS(UnsupportedAccess) + : CHIP_IM_GLOBAL_STATUS(AccessRestricted); } } @@ -701,9 +702,12 @@ CHIP_ERROR WriteSingleClusterData(const SubjectDescriptor & aSubjectDescriptor, } if (err != CHIP_NO_ERROR) { - ReturnErrorCodeIf(err != CHIP_ERROR_ACCESS_DENIED, err); + ReturnErrorCodeIf((err != CHIP_ERROR_ACCESS_DENIED) && (err != CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL), err); // TODO: when wildcard/group writes are supported, handle them to discard rather than fail with status - return apWriteHandler->AddStatus(aPath, Protocols::InteractionModel::Status::UnsupportedAccess); + return apWriteHandler->AddStatus(aPath, + err == CHIP_ERROR_ACCESS_DENIED + ? Protocols::InteractionModel::Status::UnsupportedAccess + : Protocols::InteractionModel::Status::AccessRestricted); } apWriteHandler->CacheACLCheckResult({ aPath, requestPrivilege }); } diff --git a/src/controller/java/src/chip/devicecontroller/model/Status.java b/src/controller/java/src/chip/devicecontroller/model/Status.java index 28abd36465689b..7a7eaa27ad5348 100644 --- a/src/controller/java/src/chip/devicecontroller/model/Status.java +++ b/src/controller/java/src/chip/devicecontroller/model/Status.java @@ -57,6 +57,7 @@ public enum Code { Reserved99(0x99), Reserved9a(0x9a), Busy(0x9c), + AccessRestricted(0x9d), Deprecatedc0(0xc0), Deprecatedc1(0xc1), Deprecatedc2(0xc2), diff --git a/src/controller/java/src/matter/controller/model/Status.kt b/src/controller/java/src/matter/controller/model/Status.kt index fa12f0b1cb25a5..84d06f550e9fac 100644 --- a/src/controller/java/src/matter/controller/model/Status.kt +++ b/src/controller/java/src/matter/controller/model/Status.kt @@ -57,6 +57,7 @@ data class Status(val status: Int, val clusterStatus: Int?) { RESERVED99(0X99), RESERVED9A(0X9A), BUSY(0X9C), + ACCESS_RESTRICTED(0x9D), DEPRECATEDC0(0XC0), DEPRECATEDC1(0XC1), DEPRECATEDC2(0XC2), diff --git a/src/controller/python/chip/interaction_model/__init__.py b/src/controller/python/chip/interaction_model/__init__.py index ec100846b085a5..39ade4e2f7f467 100644 --- a/src/controller/python/chip/interaction_model/__init__.py +++ b/src/controller/python/chip/interaction_model/__init__.py @@ -73,6 +73,7 @@ class Status(enum.IntEnum): Reserved99 = 0x99 Reserved9a = 0x9a Busy = 0x9c + AccessRestricted = 0x9d Deprecatedc0 = 0xc0 Deprecatedc1 = 0xc1 Deprecatedc2 = 0xc2 diff --git a/src/lib/core/CHIPError.cpp b/src/lib/core/CHIPError.cpp index 2b2461ca79d09c..31959718b94380 100644 --- a/src/lib/core/CHIPError.cpp +++ b/src/lib/core/CHIPError.cpp @@ -362,6 +362,9 @@ bool FormatCHIPError(char * buf, uint16_t bufSize, CHIP_ERROR err) case CHIP_ERROR_VERSION_MISMATCH.AsInteger(): desc = "Version mismatch"; break; + case CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL.AsInteger(): + desc = "The CHIP message's access is restricted by ARL"; + break; case CHIP_EVENT_ID_FOUND.AsInteger(): desc = "Event ID matching criteria was found"; break; diff --git a/src/lib/core/CHIPError.h b/src/lib/core/CHIPError.h index 84b0de1a47f3ba..0a002d89c088f5 100644 --- a/src/lib/core/CHIPError.h +++ b/src/lib/core/CHIPError.h @@ -1484,7 +1484,14 @@ using CHIP_ERROR = ::chip::ChipError; */ #define CHIP_ERROR_VERSION_MISMATCH CHIP_CORE_ERROR(0xa7) -// AVAILABLE: 0xa8 +/** + * @def CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL + * + * @brief + * The CHIP message is not granted access for further processing due to Access Restriction List. + */ +#define CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL CHIP_CORE_ERROR(0xa8) + // AVAILABLE: 0xa9 // AVAILABLE: 0xaa diff --git a/src/lib/core/tests/TestCHIPErrorStr.cpp b/src/lib/core/tests/TestCHIPErrorStr.cpp index d466eb281fe0fb..01130aca4261d7 100644 --- a/src/lib/core/tests/TestCHIPErrorStr.cpp +++ b/src/lib/core/tests/TestCHIPErrorStr.cpp @@ -140,6 +140,7 @@ static const CHIP_ERROR kTestElements[] = CHIP_ERROR_ACCESS_DENIED, CHIP_ERROR_UNKNOWN_RESOURCE_ID, CHIP_ERROR_VERSION_MISMATCH, + CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL, CHIP_EVENT_ID_FOUND, CHIP_ERROR_INTERNAL, CHIP_ERROR_OPEN_FAILED, diff --git a/src/protocols/interaction_model/StatusCodeList.h b/src/protocols/interaction_model/StatusCodeList.h index 5538478d76ce52..da059ac4596180 100644 --- a/src/protocols/interaction_model/StatusCodeList.h +++ b/src/protocols/interaction_model/StatusCodeList.h @@ -61,6 +61,7 @@ CHIP_IM_STATUS_CODE(Reserved98 , Reserved98 , 0x98) CHIP_IM_STATUS_CODE(Reserved99 , Reserved99 , 0x99) CHIP_IM_STATUS_CODE(Reserved9a , Reserved9a , 0x9a) CHIP_IM_STATUS_CODE(Busy , BUSY , 0x9c) +CHIP_IM_STATUS_CODE(AccessRestricted , ACCESS_RESTRICTED , 0x9d) CHIP_IM_STATUS_CODE(Deprecatedc0 , Deprecatedc0 , 0xc0) CHIP_IM_STATUS_CODE(Deprecatedc1 , Deprecatedc1 , 0xc1) CHIP_IM_STATUS_CODE(Deprecatedc2 , Deprecatedc2 , 0xc2) diff --git a/src/python_testing/TC_ACL_2_11.py b/src/python_testing/TC_ACL_2_11.py index a979b1ca3f5b1a..2ac145d7645d17 100644 --- a/src/python_testing/TC_ACL_2_11.py +++ b/src/python_testing/TC_ACL_2_11.py @@ -73,9 +73,9 @@ def steps_TC_ACL_2_11(self) -> list[TestStep]: TestStep(2, "TH1 reads DUT Endpoint 0 AccessControl cluster CommissioningARL attribute"), TestStep(3, "TH1 reads DUT Endpoint 0 AccessControl cluster ARL attribute"), TestStep(4, "For each entry in ARL, iterate over each restriction and attempt access the restriction's ID on the Endpoint and Cluster in the ARL entry.", - "If the restriction is Type AttributeAccessForbidden, read the restriction's attribute ID and verify the response is UNSUPPORTED_ACCESS." - "If the restriction is Type AttributeWriteForbidden, write restriction's the attribute ID and verify the response is UNSUPPORTED_ACCESS." - "If the restriction is Type CommandForbidden, invoke the restriction's command ID and verify the response is UNSUPPORTED_ACCESS."), + "If the restriction is Type AttributeAccessForbidden, read the restriction's attribute ID and verify the response is ACCESS_RESTRICTED." + "If the restriction is Type AttributeWriteForbidden, write restriction's the attribute ID and verify the response is ACCESS_RESTRICTED." + "If the restriction is Type CommandForbidden, invoke the restriction's command ID and verify the response is ACCESS_RESTRICTED."), TestStep(5, "TH1 sends DUT Endpoint 0 AccessControl cluster command ReviewFabricRestrictions"), TestStep(6, "Wait for up to 1 hour. Follow instructions provided by device maker to remove all access restrictions", "AccessRestrictionReviewUpdate event is received"), @@ -119,15 +119,15 @@ async def test_TC_ACL_2_11(self): command = ALL_ACCEPTED_COMMANDS[C1][ID1] if restriction_type == AccessControl.Enums.AccessRestrictionTypeEnum.kAttributeAccessForbidden: - await self.read_single_attribute_expect_error(cluster=cluster, attribute=attribute, error=Status.UnsupportedAccess, endpoint=E1) + await self.read_single_attribute_expect_error(cluster=cluster, attribute=attribute, error=Status.AccessRestricted, endpoint=E1) elif restriction_type == AccessControl.Enums.AccessRestrictionTypeEnum.kAttributeWriteForbidden: status = await self.write_single_attribute(attribute_value=attribute, endpoint_id=E1) - asserts.assert_equal(status, Status.UnsupportedAccess, - f"Failed to verify UNSUPPORTED_ACCESS when writing to Attribute {ID1} Cluster {C1} Endpoint {E1}") + asserts.assert_equal(status, Status.AccessRestricted, + f"Failed to verify ACCESS_RESTRICTED when writing to Attribute {ID1} Cluster {C1} Endpoint {E1}") elif restriction_type == AccessControl.Enums.AccessRestrictionTypeEnum.kCommandForbidden: result = await self.send_single_cmd(cmd=command, endpoint=E1) - asserts.assert_equal(result.status, Status.UnsupportedAccess, - f"Failed to verify UNSUPPORTED_ACCESS when sending command {ID1} to Cluster {C1} Endpoint {E1}") + asserts.assert_equal(result.status, Status.AccessRestricted, + f"Failed to verify ACCESS_RESTRICTED when sending command {ID1} to Cluster {C1} Endpoint {E1}") # Belongs to step 6, but needs to be subscribed before executing step 5: begin arru_queue = queue.Queue() From 71d416487db8a912c03d40c8f98b8f2e332a2b00 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Thu, 29 Aug 2024 20:28:38 +0200 Subject: [PATCH 10/12] Update ECOINFO cluster to reflect latest spec text (#35285) --------- Co-authored-by: Restyled.io --- .../ecosystem-information-server.cpp | 5 --- .../chip/ecosystem-information-cluster.xml | 4 +- src/python_testing/TC_ECOINFO_2_1.py | 39 ++++++++++++++----- 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/app/clusters/ecosystem-information-server/ecosystem-information-server.cpp b/src/app/clusters/ecosystem-information-server/ecosystem-information-server.cpp index 4af2adcb533ca3..4880c1058dc7f9 100644 --- a/src/app/clusters/ecosystem-information-server/ecosystem-information-server.cpp +++ b/src/app/clusters/ecosystem-information-server/ecosystem-information-server.cpp @@ -34,9 +34,6 @@ constexpr size_t kUniqueLocationIdMaxSize = 64; constexpr size_t kUniqueLocationIdsListMaxSize = 64; constexpr size_t kLocationDescriptorNameMaxSize = 128; -constexpr size_t kDeviceDirectoryMaxSize = 256; -constexpr size_t kLocationDirectoryMaxSize = 64; - class AttrAccess : public AttributeAccessInterface { public: @@ -264,7 +261,6 @@ CHIP_ERROR EcosystemInformationServer::AddDeviceInfo(EndpointId aEndpoint, std:: VerifyOrReturnError((aEndpoint != kRootEndpointId && aEndpoint != kInvalidEndpointId), CHIP_ERROR_INVALID_ARGUMENT); auto & deviceInfo = mDevicesMap[aEndpoint]; - VerifyOrReturnError((deviceInfo.mDeviceDirectory.size() < kDeviceDirectoryMaxSize), CHIP_ERROR_NO_MEMORY); deviceInfo.mDeviceDirectory.push_back(std::move(aDevice)); return CHIP_NO_ERROR; } @@ -282,7 +278,6 @@ CHIP_ERROR EcosystemInformationServer::AddLocationInfo(EndpointId aEndpoint, con EcosystemLocationKey key = { .mUniqueLocationId = aLocationId, .mFabricIndex = aFabricIndex }; VerifyOrReturnError((deviceInfo.mLocationDirectory.find(key) == deviceInfo.mLocationDirectory.end()), CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrReturnError((deviceInfo.mLocationDirectory.size() < kLocationDirectoryMaxSize), CHIP_ERROR_NO_MEMORY); deviceInfo.mLocationDirectory[key] = std::move(aLocation); return CHIP_NO_ERROR; } diff --git a/src/app/zap-templates/zcl/data-model/chip/ecosystem-information-cluster.xml b/src/app/zap-templates/zcl/data-model/chip/ecosystem-information-cluster.xml index 7abf54ccb7942a..cf84a7e229aed6 100644 --- a/src/app/zap-templates/zcl/data-model/chip/ecosystem-information-cluster.xml +++ b/src/app/zap-templates/zcl/data-model/chip/ecosystem-information-cluster.xml @@ -44,11 +44,11 @@ limitations under the License. true - + DeviceDirectory - + LocationDirectory diff --git a/src/python_testing/TC_ECOINFO_2_1.py b/src/python_testing/TC_ECOINFO_2_1.py index 5b952647ab4113..202d1073779cb3 100644 --- a/src/python_testing/TC_ECOINFO_2_1.py +++ b/src/python_testing/TC_ECOINFO_2_1.py @@ -25,11 +25,21 @@ class TC_ECOINFO_2_1(MatterBaseTest): - def _validate_device_directory(self, device_directory): - num_of_devices = len(device_directory) - asserts.assert_less_equal(num_of_devices, 256, "Too many device entries") + def _validate_device_directory(self, current_fabric_index, device_directory): for device in device_directory: - # TODO do fabric index check first + if current_fabric_index != device.fabricIndex: + # Fabric sensitve field still exist in python, just that they have default values + asserts.assert_equal(device.deviceName, None, "Unexpected value in deviceName") + asserts.assert_equal(device.deviceNameLastEdit, None, "Unexpected value in deviceNameLastEdit") + asserts.assert_equal(device.bridgedEndpoint, 0, "Unexpected value in bridgedEndpoint") + asserts.assert_equal(device.originalEndpoint, 0, "Unexpected value in originalEndpoint") + asserts.assert_true(type_matches(device.deviceTypes, list), "DeviceTypes should be a list") + asserts.assert_equal(len(device.deviceTypes), 0, "DeviceTypes list should be empty") + asserts.assert_true(type_matches(device.uniqueLocationIDs, list), "UniqueLocationIds should be a list") + asserts.assert_equal(len(device.uniqueLocationIDs), 0, "uniqueLocationIDs list should be empty") + asserts.assert_equal(device.uniqueLocationIDsLastEdit, 0, "Unexpected value in uniqueLocationIDsLastEdit") + continue + if device.deviceName is not None: asserts.assert_true(type_matches(device.deviceName, str), "DeviceName should be a string") asserts.assert_less_equal(len(device.deviceName), 64, "DeviceName should be <= 64") @@ -72,10 +82,20 @@ def _validate_device_directory(self, device_directory): if num_of_unique_location_ids: asserts.assert_greater(device.uniqueLocationIDsLastEdit, 0, "UniqueLocationIdsLastEdit must be non-zero") - def _validate_location_directory(self, location_directory): - num_of_locations = len(location_directory) - asserts.assert_less_equal(num_of_locations, 64, "Too many location entries") + def _validate_location_directory(self, current_fabric_index, location_directory): for location in location_directory: + if current_fabric_index != location.fabricIndex: + # Fabric sensitve field still exist in python, just that they have default values + asserts.assert_equal(location.uniqueLocationID, "", "Unexpected value in uniqueLocationID") + asserts.assert_equal(location.locationDescriptor.locationName, "", + "Unexpected value in locationDescriptor.locationName") + asserts.assert_equal(location.locationDescriptor.floorNumber, NullValue, + "Unexpected value in locationDescriptor.floorNumber") + asserts.assert_equal(location.locationDescriptor.areaType, NullValue, + "Unexpected value in locationDescriptor.areaType") + asserts.assert_equal(location.locationDescriptorLastEdit, 0, "Unexpected value in locationDescriptorLastEdit") + continue + asserts.assert_true(type_matches(location.uniqueLocationID, str), "UniqueLocationId should be a string") location_id_string_length = len(location.uniqueLocationID) asserts.assert_greater_equal(location_id_string_length, 1, @@ -120,6 +140,7 @@ async def test_TC_ECOINFO_2_1(self): self.wait_for_user_input( "Paused test to allow for manufacturer to satisfy precondition where one or more bridged devices of a supported type is connected to DUT") + current_fabric_index = await self.read_single_attribute_check_success(cluster=Clusters.OperationalCredentials, attribute=Clusters.OperationalCredentials.Attributes.CurrentFabricIndex) self.step(1) endpoint_wild_card_read = await dev_ctrl.ReadAttribute(dut_node_id, [(Clusters.EcosystemInformation.Attributes.ClusterRevision)]) list_of_endpoints = list(endpoint_wild_card_read.keys()) @@ -134,7 +155,7 @@ async def test_TC_ECOINFO_2_1(self): attribute=Clusters.EcosystemInformation.Attributes.DeviceDirectory, fabricFiltered=False) - self._validate_device_directory(device_directory) + self._validate_device_directory(current_fabric_index, device_directory) if idx == 0: self.step(3) @@ -145,7 +166,7 @@ async def test_TC_ECOINFO_2_1(self): attribute=Clusters.EcosystemInformation.Attributes.LocationDirectory, fabricFiltered=False) - self._validate_location_directory(location_directory) + self._validate_location_directory(current_fabric_index, location_directory) if idx == 0: self.step(4) From a68b7fccd2610ed3236cedff6358dc919e559694 Mon Sep 17 00:00:00 2001 From: Hasty Granbery Date: Thu, 29 Aug 2024 11:49:11 -0700 Subject: [PATCH 11/12] [HVAC] Clear the active preset when the relevant setpoint attributes are set manually (#35223) * Add support for Presets attributes and commands to the Thermostat cluster Clean up the Thermostat cluster and remove the TemperatureSetpointHoldPolicy attribute and SetTemperatureSetpointHoldPolicy command * Restyled by whitespace * Restyled by clang-format * Restyled by gn. * Fix build error for Linux configure build of all-clusters-app * Fix Darwin CI issues Editorial fixes * Restyled by clang-format * More fixes * Restyled by clang-format * BUILD.gn fixes for CI * Apply suggestions from code review Co-authored-by: Boris Zbarsky * Address review comments. * Restyled by clang-format * Regenerate Thermostat XML from spec * Move atomic enum to global-enums.xml, actually # Conflicts: # src/app/zap-templates/zcl/data-model/chip/global-structs.xml * Regenerate XML and convert thermostat-server to atomic writes * Pull in ACCapacityFormat typo un-fix * Update Test_TC_TSTAT_1_1 to know about AtomicResponse command. * Restyled patch * Fix weird merge with upstream * Fix emberAfIsTypeSigned not understanding temperature type * Merge fixes from atomic write branch * Relocate thermostat-manager sample code to all-clusters-common * Fix g++ build error on linux * Fix C formatter for long int, cast whole expression * Sync cast fix with master * Add thermostat-common dependency to thermostat app under linux * Remove MatterPostAttributeChangeCallback from thermostat-manager, as it conflicts with other implementations * Convert Atomic enums and structs to global * Restyled patch * Apply suggestions from code review Co-authored-by: Boris Zbarsky * Regen with alchemy 0.6.1 * Updates based on comments * Add TC_MCORE_FS_1_3.py test implementation (#34650) * Fix most TC-SWTCH-2.4 remaining issues (#34677) - Move 2.4 in a better place in the file - Add test steps properly - Allow default button press position override Issue #34656 Testing done: - Test still passes on DUT with automation * Initial test script for Fabric Sync TC_MCORE_FS_1_2 (#34675) * Initial test script for Fabric Sync TC_MCORE_FS_1_2 * Apply suggestions from code review Co-authored-by: C Freeman * Address Review Comments * Address review comments * Fix default timeout after other timeouts changed * Restyled by autopep8 * Fix linter error --------- Co-authored-by: C Freeman Co-authored-by: Restyled.io * Test automation for FabricSync ICD BridgedDeviceBasicInfoCluster (#34628) * WIP Bridged ICD, commissioning to both fabrics * wip testing sending KeepActive * wip most steps implemented * using SIGSTOP and SIGCONT to control ICD server pausing * Update src/python_testing/TC_BRBINFO_4_1.py Co-authored-by: Terence Hampson * comments addressed * more comments addressed * lint pass * Update src/python_testing/TC_BRBINFO_4_1.py Co-authored-by: C Freeman * comments addressed, incl TH_SERVER configurable * added setupQRCode and setupManualCode as options for DUT commissioning * Restyled by autopep8 * Restyled by isort * Update src/python_testing/TC_BRBINFO_4_1.py Co-authored-by: Terence Hampson * Update src/python_testing/TC_BRBINFO_4_1.py Co-authored-by: Terence Hampson * Update src/python_testing/TC_BRBINFO_4_1.py Co-authored-by: Terence Hampson * comments addressed * Restyled by autopep8 --------- Co-authored-by: Terence Hampson Co-authored-by: C Freeman Co-authored-by: Restyled.io * ServiceArea test scripts (#34548) * initial commit * fix bugs * fix issues reported by the linter * fix bug in checking for unique areaDesc * add TC 1.5 * Update src/python_testing/TC_SEAR_1_2.py Co-authored-by: William * Update src/python_testing/TC_SEAR_1_2.py Co-authored-by: William * address code review comments * fix issue introduced by the previous commit * address code review feedback * Update src/python_testing/TC_SEAR_1_2.py Co-authored-by: Kiel Oleson * address code review feedback * remove PICS checked by the TC_SEAR_1.6 * more code review updates * Restyled by autopep8 --------- Co-authored-by: William Co-authored-by: Kiel Oleson Co-authored-by: Restyled.io * Remove manual tests for Thermostat presets (#34679) * Dump details about leaked ExchangeContexts before aborting (#34617) * Dump details about leaked ExchangeContexts before aborting This is implemented via a VerifyOrDieWithObject() variant of the existing VerifyOrDie() macro that calls a DumpToLog() method on the provided object if it exists (otherwise this is simply a no-op). If CHIP_CONFIG_VERBOSE_VERIFY_OR_DIE is not enabled, VerifyOrDieWithObject() simply behaves like a plain VerifyOrDie(). DumpToLog() implementations can use ChipLogFormatRtti to log type information about an object (usually a delegate); if RTTI is disabled this simply outputs whether the object was null or not. * Address review comments * Make gcc happy and improve documentation * Remove unused include * Fix compile error without CHIP_CONFIG_VERBOSE_VERIFY_OR_DIE * Avoid unused parameter warning * [TI] CC13x4_26x4 build fixes (#34682) * lwip pbuf, map file, and hex creation when OTA is disabled * added cc13x4 family define around the non OTA hex creation * whitespace fix * reversed custom factoy data flash with cc13x4 check * more whitespace fixes * [ICD] Add missing polling function to NoWifi connectivity manager (#34684) * Add missing polling function to NoWifi connectivity manager * Update GenericConnectivityManagerImpl_NoWiFi.h Co-authored-by: Boris Zbarsky --------- Co-authored-by: Boris Zbarsky * [OPSTATE] Add Q test script for CountdownTime (#34632) * Add Q test * Added test to test set * Remove unused var * Restyled by autopep8 * Restyled by isort * Fix name * Use pics over other method * Removed unused stuff * Added pipe commands * Fix reset * Get example to report appropriate changes. * WiP * Added some comments * Changes to make things work * Removed dev msgs * Missed some * Removed dev msgs * Straggler * Restyled by clang-format * Restyled by autopep8 * Restyled by isort * Commented unused var * Update examples/all-clusters-app/linux/AllClustersCommandDelegate.cpp * Fix bug --------- Co-authored-by: Restyled.io * YAML update to BRBINFO, ProductId (#34513) * Bridged Device Information Cluster, Attribute ProductID test reflects marking as O, not X * Update src/app/tests/suites/certification/Test_TC_BRBINFO_2_1.yaml Co-authored-by: Terence Hampson * corrected pics * corrected pics * WIP Bridged ICD, commissioning to both fabrics * wip testing sending KeepActive * update to bridged-device-basic-information.xml and zap generated files * removed unrelated file --------- Co-authored-by: Terence Hampson Co-authored-by: Andrei Litvin * Fix simplified Linux tv-casting-app gn build error. (#34692) * adding parallel execution to restyle-diff (#34663) * adding parallel execution to restyle-diff * using xargs to call restyle-paths * fixing Copyright year * restyle the restyler * Add some bits to exercise global structs/enums to Unit Testing cluster. (#34540) * Adds things to the Unit Testing cluster XML. * This requires those things to be enabled in all-clusters-app, all-clusters-minimal-app, and one of the chef contact sensors to pass CI. * That requires an implementation in test-cluster-server * At which point might as well add a YAML test to exercise it all. * [Silabs] Port platform specific Multi-Chip OTA work (#34440) * Pull request #1836: Cherry multi ota Merge in WMN_TOOLS/matter from cherry-multi-ota to silabs_slc_1.3 Squashed commit of the following: commit 4320bb46571658bc44fb82345348265def394991 Author: Michael Rupp Date: Fri May 10 14:26:07 2024 -0400 remove some unwanted diffs in provision files commit be160931dc600de7e7ead378b70d6a43c3945e46 Author: Michael Rupp Date: Fri May 10 14:24:25 2024 -0400 revert changes to generator.project.mak commit 14b6605887166e6d5284a61feb2bf407d850bdcf Author: Michael Rupp Date: Fri May 10 13:06:12 2024 -0400 revert NVM key changes and script changes ... and 8 more commits * Restyled by whitespace * Restyled by clang-format * Restyled by gn * Restyled by autopep8 * remove unused libs caught by linter * update doctree with new readmes * rerun CI, cirque failing for unknown reasons * fix include guards in provision examples * Restyled by clang-format --------- Co-authored-by: Restyled.io * Add python tests for Thermostat presets feature (#34693) * Add python tests for Thermostat presets feature * Restyled by autopep8 * Restyled by isort * Update the PICS code for presets attribute --------- Co-authored-by: Restyled.io * removing unneccessary git fetch (#34698) * Restyle patch * Regen to fix ordering of global structs * Apply suggestions from code review Co-authored-by: Boris Zbarsky * Return correct AtomicResponse when committing or rolling back * Patch tests for atomic write of presets * Fix tests to work with the new setup. Specific changes: * Enable SetActivePresetRequest command in all-clusters-app. * Fix assignment of a PresetStructWithOwnedMembers to another PresetStructWithOwnedMembers to actually work correctly. * Move constraint checks that happen on write from commit to write. * Fix sending of atomic responses to not have use-stack-after-return. * Fix PICS for the tests involved. * Fix PICS values for atomic requests * Remove PresetsSchedulesEditable and QueuedPreset from various places * Restyled patch * Restyled patch, again * Remove PICS value for PresetsSchedulesEditable * clang-tidy fixes * clang-tidy fixes * Clear associated atomic writes when fabric is removed * Add tests for fabric removal and lockout of clients outside of atomic write * Python linter * Restyled patch * Clear timer when fabric is removed * Check for open atomic write before resetting * Revert auto delegate declaration on lines where there's no collision * Allow Thermostat delegate to provide timeout for atomic requests * Relocate thermostat example code to thermostat-common * Remove thermostat-manager code, replace with thermostat delegate * Sync atomic write error order with spec * Restyle patch * Drop memset of atomic write sessions * Add PreCommit stage to allow rollback of multiple attributes when only one fails * Separate OnTimerExpired method, vs ResetWrite * Method documentation * Apply suggestions from code review Co-authored-by: Nivi Sarkar <55898241+nivi-apple@users.noreply.github.com> * Remove unused InWrite check * Drop imcode alias * Switch AtomicWriteState to enum class * DRY up atomic write manager * Apply suggestions from code review Co-authored-by: Nivi Sarkar <55898241+nivi-apple@users.noreply.github.com> * Drop duplicate doc comments * Rename GetAtomicWriteScopedNodeId to GetAtomicWriteOriginatorScopedNodeId * Updates based on comments * Add MatterReportingAttributeChangeCallback calls for updated attributes * Relocate thermostat example code to thermostat-common, and remove thermostat-manager * Merge atomic write code back into thermostat-server * Apply suggestions from code review Co-authored-by: Boris Zbarsky * Fix build after suggestions * Actually track attribute IDs associated with atomic write * Only commit presets if all attribute precommits were successful * Fix scope on err * Add documentation to methods * Remove duplicate preset check. * Move various functions into anonymous namespaces, or Thermostat namespace * Drop impossible non-atomic attribute status after rollback * Allow null BuiltIn field when saving Presets * Namespace workaround for compilers on other platforms * Fix bad merge * Fix readability issue * Force built-in to false on new presets * [HVAC] Clear ActivePresetHandle attribute when changing relevant setpoint attributes * Apply suggestions from code review Co-authored-by: Boris Zbarsky --------- Co-authored-by: Nivedita Sarkar Co-authored-by: Restyled.io Co-authored-by: Nivi Sarkar <55898241+nivi-apple@users.noreply.github.com> Co-authored-by: Boris Zbarsky Co-authored-by: Terence Hampson Co-authored-by: Tennessee Carmel-Veilleux Co-authored-by: Chris Letnick Co-authored-by: C Freeman Co-authored-by: Douglas Rocha Ferraz Co-authored-by: Petru Lauric <81822411+plauric@users.noreply.github.com> Co-authored-by: William Co-authored-by: Kiel Oleson Co-authored-by: Karsten Sperling <113487422+ksperling-apple@users.noreply.github.com> Co-authored-by: Anu Biradar <104591549+abiradarti@users.noreply.github.com> Co-authored-by: mkardous-silabs <84793247+mkardous-silabs@users.noreply.github.com> Co-authored-by: Rob Bultman Co-authored-by: Andrei Litvin Co-authored-by: Shao Ling Tan <161761051+shaoltan-amazon@users.noreply.github.com> Co-authored-by: Amine Alami <43780877+Alami-Amine@users.noreply.github.com> Co-authored-by: Michael Rupp <95718139+mykrupp@users.noreply.github.com> --- .../app-templates/endpoint_config.h | 3 +- .../thermostat-server/thermostat-server.cpp | 51 +++++++++++++++++++ .../thermostat-server/thermostat-server.h | 1 + src/app/common/templates/config-data.yaml | 1 + src/python_testing/TC_TSTAT_4_2.py | 12 +++++ 5 files changed, 67 insertions(+), 1 deletion(-) 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 a28eb56c6eb299..6ea480e199be51 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 @@ -2217,6 +2217,7 @@ }; \ const EmberAfGenericClusterFunction chipFuncArrayThermostatServer[] = { \ (EmberAfGenericClusterFunction) emberAfThermostatClusterServerInitCallback, \ + (EmberAfGenericClusterFunction) MatterThermostatClusterServerAttributeChangedCallback, \ (EmberAfGenericClusterFunction) MatterThermostatClusterServerShutdownCallback, \ (EmberAfGenericClusterFunction) MatterThermostatClusterServerPreAttributeChangedCallback, \ }; \ @@ -3756,7 +3757,7 @@ .attributes = ZAP_ATTRIBUTE_INDEX(616), \ .attributeCount = 26, \ .clusterSize = 72, \ - .mask = ZAP_CLUSTER_MASK(SERVER) | ZAP_CLUSTER_MASK(INIT_FUNCTION) | ZAP_CLUSTER_MASK(SHUTDOWN_FUNCTION) | ZAP_CLUSTER_MASK(PRE_ATTRIBUTE_CHANGED_FUNCTION), \ + .mask = ZAP_CLUSTER_MASK(SERVER) | ZAP_CLUSTER_MASK(INIT_FUNCTION) | ZAP_CLUSTER_MASK(ATTRIBUTE_CHANGED_FUNCTION) | ZAP_CLUSTER_MASK(SHUTDOWN_FUNCTION) | ZAP_CLUSTER_MASK(PRE_ATTRIBUTE_CHANGED_FUNCTION), \ .functions = chipFuncArrayThermostatServer, \ .acceptedCommandList = ZAP_GENERATED_COMMANDS_INDEX( 241 ), \ .generatedCommandList = ZAP_GENERATED_COMMANDS_INDEX( 246 ), \ diff --git a/src/app/clusters/thermostat-server/thermostat-server.cpp b/src/app/clusters/thermostat-server/thermostat-server.cpp index fe8ccf8aab3cf0..9e6e0f074d5e52 100644 --- a/src/app/clusters/thermostat-server/thermostat-server.cpp +++ b/src/app/clusters/thermostat-server/thermostat-server.cpp @@ -468,6 +468,52 @@ void ThermostatAttrAccess::OnFabricRemoved(const FabricTable & fabricTable, Fabr } } +void MatterThermostatClusterServerAttributeChangedCallback(const ConcreteAttributePath & attributePath) +{ + uint32_t flags; + if (FeatureMap::Get(attributePath.mEndpointId, &flags) != Status::Success) + { + ChipLogError(Zcl, "MatterThermostatClusterServerAttributeChangedCallback: could not get feature flags"); + return; + } + + auto featureMap = BitMask(flags); + if (!featureMap.Has(Feature::kPresets)) + { + // This server does not support presets, so nothing to do + return; + } + + bool occupied = true; + if (featureMap.Has(Feature::kOccupancy)) + { + BitMask occupancy; + if (Occupancy::Get(attributePath.mEndpointId, &occupancy) == Status::Success) + { + occupied = occupancy.Has(OccupancyBitmap::kOccupied); + } + } + + bool clearActivePreset = false; + switch (attributePath.mAttributeId) + { + case OccupiedHeatingSetpoint::Id: + case OccupiedCoolingSetpoint::Id: + clearActivePreset = occupied; + break; + case UnoccupiedHeatingSetpoint::Id: + case UnoccupiedCoolingSetpoint::Id: + clearActivePreset = !occupied; + break; + } + if (!clearActivePreset) + { + return; + } + ChipLogProgress(Zcl, "Setting active preset to null"); + gThermostatAttrAccess.SetActivePreset(attributePath.mEndpointId, std::nullopt); +} + } // namespace Thermostat } // namespace Clusters } // namespace app @@ -762,6 +808,11 @@ MatterThermostatClusterServerPreAttributeChangedCallback(const app::ConcreteAttr } } +void MatterThermostatClusterServerAttributeChangedCallback(const ConcreteAttributePath & attributePath) +{ + Thermostat::MatterThermostatClusterServerAttributeChangedCallback(attributePath); +} + bool emberAfThermostatClusterClearWeeklyScheduleCallback(app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath, const Commands::ClearWeeklySchedule::DecodableType & commandData) diff --git a/src/app/clusters/thermostat-server/thermostat-server.h b/src/app/clusters/thermostat-server/thermostat-server.h index cc941cfa766d92..2d365ffd8288d1 100644 --- a/src/app/clusters/thermostat-server/thermostat-server.h +++ b/src/app/clusters/thermostat-server/thermostat-server.h @@ -207,6 +207,7 @@ class ThermostatAttrAccess : public chip::app::AttributeAccessInterface, public friend void TimerExpiredCallback(System::Layer * systemLayer, void * callbackContext); friend void MatterThermostatClusterServerShutdownCallback(EndpointId endpoint); + friend void MatterThermostatClusterServerAttributeChangedCallback(const chip::app::ConcreteAttributePath & attributePath); friend bool emberAfThermostatClusterSetActivePresetRequestCallback( CommandHandler * commandObj, const ConcreteCommandPath & commandPath, diff --git a/src/app/common/templates/config-data.yaml b/src/app/common/templates/config-data.yaml index 660ca0b3aa1df4..61e7c45582268a 100644 --- a/src/app/common/templates/config-data.yaml +++ b/src/app/common/templates/config-data.yaml @@ -72,6 +72,7 @@ ClustersWithAttributeChangedFunctions: - Pump Configuration and Control - Window Covering - Fan Control + - Thermostat ClustersWithShutdownFunctions: - Barrier Control diff --git a/src/python_testing/TC_TSTAT_4_2.py b/src/python_testing/TC_TSTAT_4_2.py index 641c827b388882..56cf356b6b8098 100644 --- a/src/python_testing/TC_TSTAT_4_2.py +++ b/src/python_testing/TC_TSTAT_4_2.py @@ -291,6 +291,11 @@ async def test_TC_TSTAT_4_2(self): logger.info(f"Rx'd Presets: {presets}") asserts.assert_equal(presets, new_presets_with_handle, "Presets were not updated which is not expected") + # Send the SetActivePresetRequest command + await self.send_set_active_preset_handle_request_command(value=b'\x03') + + activePresetHandle = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.ActivePresetHandle) + self.step("5") 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")): @@ -327,6 +332,13 @@ async def test_TC_TSTAT_4_2(self): # Send the AtomicRequest commit command and expect InvalidInState for presets. await self.send_atomic_request_commit_command(expected_overall_status=Status.Failure, expected_preset_status=Status.InvalidInState) + # Write the occupied cooling setpoint to a different value + await self.write_single_attribute(attribute_value=cluster.Attributes.OccupiedCoolingSetpoint(2300), endpoint_id=endpoint) + + activePresetHandle = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.ActivePresetHandle) + logger.info(f"Rx'd ActivePresetHandle: {activePresetHandle}") + asserts.assert_equal(activePresetHandle, NullValue, "Active preset handle was not cleared as expected") + self.step("7") 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")): From e772b6975633c14273ecdb686f67a28b97eb019c Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Thu, 29 Aug 2024 12:40:36 -0700 Subject: [PATCH 12/12] Update TC_MCORE_FS_1_3 to align with the Test Spec (#35270) * Update TC_MCORE_FS_1_3 to align with the test spec * Instead of th_server_app_path use th_server_no_uid_app_path --- src/python_testing/TC_MCORE_FS_1_3.py | 60 +++++++++------------------ 1 file changed, 19 insertions(+), 41 deletions(-) diff --git a/src/python_testing/TC_MCORE_FS_1_3.py b/src/python_testing/TC_MCORE_FS_1_3.py index e707b500a6fba8..4245bc139ceb38 100644 --- a/src/python_testing/TC_MCORE_FS_1_3.py +++ b/src/python_testing/TC_MCORE_FS_1_3.py @@ -15,7 +15,7 @@ # limitations under the License. # -# This test requires a TH_SERVER application that returns UnsupportedAttribute when reading UniqueID from BasicInformation Cluster. Please specify with --string-arg th_server_app_path: +# This test requires a TH_SERVER application that returns UnsupportedAttribute when reading UniqueID from BasicInformation Cluster. Please specify with --string-arg th_server_no_uid_app_path: import logging import os @@ -36,14 +36,10 @@ class TC_MCORE_FS_1_3(MatterBaseTest): @async_test_body async def setup_class(self): super().setup_class() - self.device_for_th_eco_nodeid = 1111 - self.device_for_th_eco_kvs = None - self.device_for_th_eco_port = 5543 - self.app_process_for_th_eco = None - - self.device_for_dut_eco_nodeid = 1112 - self.device_for_dut_eco_kvs = None - self.device_for_dut_eco_port = 5544 + + self.th_server_nodeid = 1111 + self.th_server_kvs = None + self.th_server_port = 5543 self.app_process_for_dut_eco = None # Create a second controller on a new fabric to communicate to the server @@ -57,20 +53,14 @@ def teardown_class(self): logging.warning("Stopping app with SIGTERM") self.app_process_for_dut_eco.send_signal(signal.SIGTERM.value) self.app_process_for_dut_eco.wait() - if self.app_process_for_th_eco is not None: - logging.warning("Stopping app with SIGTERM") - self.app_process_for_th_eco.send_signal(signal.SIGTERM.value) - self.app_process_for_th_eco.wait() - os.remove(self.device_for_dut_eco_kvs) - if self.device_for_th_eco_kvs is not None: - os.remove(self.device_for_th_eco_kvs) + os.remove(self.th_server_kvs) super().teardown_class() async def create_device_and_commission_to_th_fabric(self, kvs, port, node_id_for_th, device_info): - app = self.user_params.get("th_server_app_path", None) + app = self.user_params.get("th_server_no_uid_app_path", None) if not app: - asserts.fail('This test requires a TH_SERVER app. Specify app path with --string-arg th_server_app_path:') + asserts.fail('This test requires a TH_SERVER app. Specify app path with --string-arg th_server_no_uid_app_path:') if not os.path.exists(app): asserts.fail(f'The path {app} does not exist') @@ -95,33 +85,33 @@ async def create_device_and_commission_to_th_fabric(self, kvs, port, node_id_for logging.info("Commissioning device for DUT ecosystem onto TH for managing") def steps_TC_MCORE_FS_1_3(self) -> list[TestStep]: - steps = [TestStep(1, "DUT_FSA commissions TH_SED_DUT to DUT_FSAs fabric and generates a UniqueID", is_commissioning=True), - TestStep(2, "TH_FSA commissions TH_SED_TH onto TH_FSAs fabric and generates a UniqueID."), - TestStep(3, "Follow manufacturer provided instructions to enable DUT_FSA to synchronize TH_SED_TH onto DUT_FSAs fabric."), - TestStep(4, "DUT_FSA synchronizes TH_SED_TH onto DUT_FSAs fabric and copies the UniqueID presented by TH_FSAs Bridged Device Basic Information Cluster.")] + steps = [TestStep(1, "TH commissions TH_SERVER to TH’s fabric.", is_commissioning=True), + TestStep(2, "DUT_FSA commissions TH_SERVER to DUT_FSA’s fabric and generates a UniqueID.")] return steps @async_test_body async def test_TC_MCORE_FS_1_3(self): self.is_ci = self.check_pics('PICS_SDK_CI_ONLY') self.print_step(0, "Commissioning DUT to TH, already done") + self.step(1) - # These steps are not explicitly in step 1, but they help identify the dynamically added endpoint in step 1. root_node_endpoint = 0 root_part_list = await self.read_single_attribute_check_success(cluster=Clusters.Descriptor, attribute=Clusters.Descriptor.Attributes.PartsList, endpoint=root_node_endpoint) set_of_endpoints_before_adding_device = set(root_part_list) + logging.info(f"Set of endpoints before adding the device: {set_of_endpoints_before_adding_device}") kvs = f'kvs_{str(uuid.uuid4())}' - device_info = "for DUT ecosystem" - await self.create_device_and_commission_to_th_fabric(kvs, self.device_for_dut_eco_port, self.device_for_dut_eco_nodeid, device_info) + device_info = "for TH ecosystem" + await self.create_device_and_commission_to_th_fabric(kvs, self.th_server_port, self.th_server_nodeid, device_info) - self.device_for_dut_eco_kvs = kvs - read_result = await self.TH_server_controller.ReadAttribute(self.device_for_dut_eco_nodeid, [(root_node_endpoint, Clusters.BasicInformation.Attributes.UniqueID)]) + self.th_server_kvs = kvs + read_result = await self.TH_server_controller.ReadAttribute(self.th_server_nodeid, [(root_node_endpoint, Clusters.BasicInformation.Attributes.UniqueID)]) result = read_result[root_node_endpoint][Clusters.BasicInformation][Clusters.BasicInformation.Attributes.UniqueID] asserts.assert_true(type_matches(result, Clusters.Attribute.ValueDecodeFailure), "We were expecting a value decode failure") asserts.assert_equal(result.Reason.status, Status.UnsupportedAttribute, "Incorrect error returned from reading UniqueID") - params = await self.openCommissioningWindow(dev_ctrl=self.TH_server_controller, node_id=self.device_for_dut_eco_nodeid) + self.step(2) + params = await self.openCommissioningWindow(dev_ctrl=self.TH_server_controller, node_id=self.th_server_nodeid) self.wait_for_user_input( prompt_msg=f"Using the DUT vendor's provided interface, commission the device using the following parameters:\n" @@ -134,6 +124,7 @@ async def test_TC_MCORE_FS_1_3(self): root_part_list = await self.read_single_attribute_check_success(cluster=Clusters.Descriptor, attribute=Clusters.Descriptor.Attributes.PartsList, endpoint=root_node_endpoint) set_of_endpoints_after_adding_device = set(root_part_list) + logging.info(f"Set of endpoints after adding the device: {set_of_endpoints_after_adding_device}") asserts.assert_true(set_of_endpoints_after_adding_device.issuperset( set_of_endpoints_before_adding_device), "Expected only new endpoints to be added") @@ -145,19 +136,6 @@ async def test_TC_MCORE_FS_1_3(self): asserts.assert_true(type_matches(th_sed_dut_unique_id, str), "UniqueID should be a string") asserts.assert_true(th_sed_dut_unique_id, "UniqueID should not be an empty string") - self.step(2) - kvs = f'kvs_{str(uuid.uuid4())}' - device_info = "for TH_FSA ecosystem" - await self.create_device_and_commission_to_th_fabric(kvs, self.device_for_th_eco_port, self.device_for_th_eco_nodeid, device_info) - self.device_for_th_eco_kvs = kvs - # TODO(https://github.com/CHIP-Specifications/chip-test-plans/issues/4375) During setup we need to create the TH_FSA device - # where we would commission device created in create_device_and_commission_to_th_fabric to be commissioned into TH_FSA. - - # TODO(https://github.com/CHIP-Specifications/chip-test-plans/issues/4375) Because we cannot create a TH_FSA and there is - # no way to mock it the following 2 test steps are skipped for now. - self.skip_step(3) - self.skip_step(4) - if __name__ == "__main__": default_matter_test_main()