diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 1ac9f82f88374d..9024b0e3a207ee 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -537,6 +537,7 @@ jobs: scripts/run_in_python_env.sh out/venv 'python3 src/python_testing/TestIdChecks.py' scripts/run_in_python_env.sh out/venv 'python3 src/python_testing/TestMatterTestingSupport.py' scripts/run_in_python_env.sh out/venv 'python3 src/python_testing/TestSpecParsingDeviceType.py' + scripts/run_in_python_env.sh out/venv 'python3 src/python_testing/TestSpecParsingSelection.py' scripts/run_in_python_env.sh out/venv 'python3 src/python_testing/TestSpecParsingSupport.py' - name: Run Tests diff --git a/examples/chef/common/chef-concentration-measurement.cpp b/examples/chef/common/chef-concentration-measurement.cpp index a23e66561d4cce..f0db3395cf4f9c 100644 --- a/examples/chef/common/chef-concentration-measurement.cpp +++ b/examples/chef/common/chef-concentration-measurement.cpp @@ -306,7 +306,7 @@ void emberAfPm1ConcentrationMeasurementClusterInitCallback(EndpointId endpoint) void emberAfPm10ConcentrationMeasurementClusterInitCallback(EndpointId endpoint) { gPm10ConcentrationMeasurementInstance[EndpointId(endpoint)] = new Instance( - EndpointId(endpoint), Pm10ConcentrationMeasurement::Id, MeasurementMediumEnum::kAir, MeasurementUnitEnum::kPpm); + EndpointId(endpoint), Pm10ConcentrationMeasurement::Id, MeasurementMediumEnum::kAir, MeasurementUnitEnum::kUgm3); gPm10ConcentrationMeasurementInstance[EndpointId(endpoint)]->Init(); gPm10ConcentrationMeasurementInstance[EndpointId(endpoint)]->SetMeasuredValue(MakeNullable(50.0f)); gPm10ConcentrationMeasurementInstance[EndpointId(endpoint)]->SetMinMeasuredValue(MakeNullable(1.0f)); diff --git a/examples/common/pigweed/rpc_services/AccessInterceptor.h b/examples/common/pigweed/rpc_services/AccessInterceptor.h new file mode 100644 index 00000000000000..c63adcef8d9e3e --- /dev/null +++ b/examples/common/pigweed/rpc_services/AccessInterceptor.h @@ -0,0 +1,55 @@ +/* + * + * Copyright (c) 2025 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. + */ + +#pragma once + +#include "pw_status/status.h" +#include +#include +#include + +namespace chip { +namespace rpc { + +/** + * Callback class that clusters can implement in order to interpose custom + * interception logic. + */ +class PigweedDebugAccessInterceptor +{ +public: + PigweedDebugAccessInterceptor() = default; + virtual ~PigweedDebugAccessInterceptor() = default; + + /** + * Callback for writing attributes. + * + * The implementation can do one of three things: + * + * Returns: + * - `std::nullopt` if the `path` was not handled by this Interceptor. + * Interceptor MUST NOT have attepted to decode `decoder`. + * - A `::pw::Status` value that is considered the FINAL result of the + * write (i.e. write handled) either with success or failure. + */ + virtual std::optional<::pw::Status> Write(const chip::app::ConcreteDataAttributePath & path, + chip::app::AttributeValueDecoder & decoder) = 0; +}; + +} // namespace rpc +} // namespace chip diff --git a/examples/common/pigweed/rpc_services/AccessInterceptorRegistry.h b/examples/common/pigweed/rpc_services/AccessInterceptorRegistry.h new file mode 100644 index 00000000000000..bc0a6aeb0972e9 --- /dev/null +++ b/examples/common/pigweed/rpc_services/AccessInterceptorRegistry.h @@ -0,0 +1,73 @@ +/* + * + * Copyright (c) 2025 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. + */ + +#pragma once + +#include "pw_status/status.h" +#include +#include + +namespace chip { +namespace rpc { + +/** @brief Custom debug request interceptors. + * + * This class is specifically meant for registering custom Accessors that + * allow mini-handlers to process PigweedRPC read/writes separately from the cluster + * code. It is meant to be used by samples using this PigweedRPC services to allow the RPC + * interface to be used in more ways than simply for example, simulating writes from a Matter + * client at the IM level. Handlers registered here by applications should be attempted before any + * standard processing. + */ +class PigweedDebugAccessInterceptorRegistry +{ +public: + /** + * Registers an attribute access inteceptor within this registry. Use `Unregister` to + * unregister an interceptor that was previously registered. + * @param attrOverride - the interceptor to be registered. + */ + void Register(PigweedDebugAccessInterceptor * attrOverride) { mAccessors.insert(attrOverride); } + + void Unregister(PigweedDebugAccessInterceptor * attrOverride) + { + if (mAccessors.find(attrOverride) == mAccessors.end()) + { + ChipLogError(Support, "Attempt to unregister accessor that is not registered."); + return; + } + mAccessors.erase(attrOverride); + } + + const std::set & GetAllAccessors() const { return mAccessors; } + + /** + * Returns the singleton instance of the attribute accessor registory. + */ + static PigweedDebugAccessInterceptorRegistry & Instance() + { + static PigweedDebugAccessInterceptorRegistry instance; + return instance; + } + +private: + std::set mAccessors; +}; + +} // namespace rpc +} // namespace chip diff --git a/examples/common/pigweed/rpc_services/Attributes.h b/examples/common/pigweed/rpc_services/Attributes.h index eff2af5d66db31..9ac206f71267df 100644 --- a/examples/common/pigweed/rpc_services/Attributes.h +++ b/examples/common/pigweed/rpc_services/Attributes.h @@ -33,14 +33,42 @@ #include #include #include +#include #include #include #include +#include +#include #include +#include namespace chip { namespace rpc { +std::optional<::pw::Status> TryWriteViaAccessor(const chip::app::ConcreteDataAttributePath & path, + chip::app::AttributeValueDecoder & decoder) +{ + std::set accessors = PigweedDebugAccessInterceptorRegistry::Instance().GetAllAccessors(); + + for (PigweedDebugAccessInterceptor * accessor : accessors) + { + std::optional<::pw::Status> result = accessor->Write(path, decoder); + if (result.has_value()) // Write was either a success or failure. + { + return result; + } + else if (decoder.TriedDecode()) + { + ChipLogError(Support, "Interceptor tried decode but did not return status."); + return ::pw::Status::FailedPrecondition(); + } + } + + VerifyOrReturnError(!decoder.TriedDecode(), ::pw::Status::FailedPrecondition()); + + return std::nullopt; +} + // Implementation class for chip.rpc.Attributes. class Attributes : public pw_rpc::nanopb::Attributes::Service { @@ -207,6 +235,14 @@ class Attributes : public pw_rpc::nanopb::Attributes::Service } app::AttributeValueDecoder decoder(tlvReader.value(), subjectDescriptor); + + std::optional<::pw::Status> interceptResult = TryWriteViaAccessor(write_request.path, decoder); + if (interceptResult.has_value()) + { + return *interceptResult; + } + ChipLogProgress(Support, "No custom PigweedRPC Attribute Accessor registration found, using fake write access."); + app::DataModel::ActionReturnStatus result = provider->WriteAttribute(write_request, decoder); if (!result.IsSuccess()) diff --git a/examples/lighting-app/telink/README.md b/examples/lighting-app/telink/README.md index ba41630d9ff37f..d940d55aafe794 100644 --- a/examples/lighting-app/telink/README.md +++ b/examples/lighting-app/telink/README.md @@ -74,6 +74,33 @@ To get output from device, connect UART to following pins: Baud rate: 115200 bits/s +### Using USB COM Port Instead of UART + +Alternatively, the USB COM port can be used instead of UART for console output. + +1. Build the project with the following parameter: + + ```bash + $ west build -b -- -DTLNK_USB_DONGLE=y + ``` + +2. Connect the USB cable to your device. A new serial device should appear in + your system (e.g., `/dev/ttyACM0` on Linux or a COM port on Windows). +3. Use your preferred terminal application (like `minicom`, `screen`, or + `PuTTY`) to connect to the newly detected serial device. +4. In your source code, ensure the following header is included and the USB + device stack is initialized: + + ```c + #ifdef CONFIG_USB_DEVICE_STACK + #include + #endif /* CONFIG_USB_DEVICE_STACK */ + + #ifdef CONFIG_USB_DEVICE_STACK + usb_enable(NULL); + #endif /* CONFIG_USB_DEVICE_STACK */ + ``` + ### Buttons The following buttons are available on **tlsr9518adk80d** board: diff --git a/integrations/docker/images/base/chip-build/Dockerfile b/integrations/docker/images/base/chip-build/Dockerfile index 2a64ab7049d2be..5c154f6e82b74a 100644 --- a/integrations/docker/images/base/chip-build/Dockerfile +++ b/integrations/docker/images/base/chip-build/Dockerfile @@ -105,6 +105,7 @@ RUN set -x \ unzip \ wget \ zlib1g-dev \ + zstd \ && rm -rf /var/lib/apt/lists/ \ && git lfs install \ && : # last line diff --git a/integrations/docker/images/base/chip-build/version b/integrations/docker/images/base/chip-build/version index b0158daf675e10..0ff86b56e79125 100644 --- a/integrations/docker/images/base/chip-build/version +++ b/integrations/docker/images/base/chip-build/version @@ -1 +1 @@ -109 : [Tizen] Fix race when storing cert credentials +111 : [Android] Update android sdk and java version diff --git a/integrations/docker/images/stage-2/chip-build-java/Dockerfile b/integrations/docker/images/stage-2/chip-build-java/Dockerfile index a8f328153c2dff..c5b54e4c439f8e 100644 --- a/integrations/docker/images/stage-2/chip-build-java/Dockerfile +++ b/integrations/docker/images/stage-2/chip-build-java/Dockerfile @@ -6,7 +6,7 @@ LABEL org.opencontainers.image.source https://github.com/project-chip/connectedh RUN set -x \ && apt-get update \ && DEBIAN_FRONTEND=noninteractive apt-get install -fy \ - openjdk-17-jdk \ + openjdk-11-jdk \ && rm -rf /var/lib/apt/lists/ \ && : # last line @@ -20,4 +20,4 @@ RUN set -x \ && : # last line ENV PATH $PATH:/usr/lib/kotlinc/bin -ENV JAVA_HOME=/usr/lib/jvm/java-17-openjdk-amd64 +ENV JAVA_HOME=/usr/lib/jvm/java-11-openjdk-amd64 diff --git a/integrations/docker/images/stage-3/chip-build-android/Dockerfile b/integrations/docker/images/stage-3/chip-build-android/Dockerfile index 3f3d8d40f1c396..eb2360ccb937e0 100644 --- a/integrations/docker/images/stage-3/chip-build-android/Dockerfile +++ b/integrations/docker/images/stage-3/chip-build-android/Dockerfile @@ -16,26 +16,26 @@ RUN set -x \ # Download and install android SDK RUN set -x \ - && wget -O /tmp/android-26.zip https://dl.google.com/android/repository/platform-26_r02.zip \ + && wget -O /tmp/android-30.zip https://dl.google.com/android/repository/platform-30_r03.zip \ && mkdir -p /opt/android/sdk/platforms \ && cd /opt/android/sdk/platforms \ - && unzip /tmp/android-26.zip \ - && mv android-8.0.0 android-26 \ - && rm -f /tmp/android-26.zip \ + && unzip /tmp/android-30.zip \ + && mv android-11 android-30 \ + && rm -f /tmp/android-30.zip \ && chmod -R a+rX /opt/android/sdk \ - && test -d /opt/android/sdk/platforms/android-26 \ + && test -d /opt/android/sdk/platforms/android-30 \ && : # last line # Download and install android command line tool (for installing `sdkmanager`) -# We need create latest folder inide cmdline-tools, since latest android commandline tool looks for this latest folder +# We need create 10.0 folder inide cmdline-tools, since latest android commandline tool looks for this latest folder # when running sdkmanager --licenses RUN set -x \ - && wget -O /tmp/cmdline-tools.zip https://dl.google.com/android/repository/commandlinetools-linux-11076708_latest.zip \ + && wget -O /tmp/cmdline-tools.zip https://dl.google.com/android/repository/commandlinetools-linux-9862592_latest.zip \ && cd /opt/android/sdk \ && mkdir -p temp \ && unzip /tmp/cmdline-tools.zip -d temp \ - && mkdir -p cmdline-tools/latest \ - && cp -rf temp/cmdline-tools/* cmdline-tools/latest \ + && mkdir -p cmdline-tools/10.0 \ + && cp -rf temp/cmdline-tools/* cmdline-tools/10.0 \ && rm -rf temp \ && test -d /opt/android/sdk/cmdline-tools \ && : # last line diff --git a/scripts/py_matter_idl/matter_idl/generators/java/ChipClusters_java.jinja b/scripts/py_matter_idl/matter_idl/generators/java/ChipClusters_java.jinja index dd2627221c08fa..954bd7de57dcbf 100644 --- a/scripts/py_matter_idl/matter_idl/generators/java/ChipClusters_java.jinja +++ b/scripts/py_matter_idl/matter_idl/generators/java/ChipClusters_java.jinja @@ -105,7 +105,6 @@ import chip.devicecontroller.model.NodeState; import chip.devicecontroller.model.Status; import javax.annotation.Nullable; -import java.lang.ref.Cleaner; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -170,23 +169,10 @@ public class ChipClusters { private Optional timeoutMillis = Optional.empty(); - private final Cleaner.Cleanable cleanable; - public BaseChipCluster(long devicePtr, int endpointId, long clusterId) { this.devicePtr = devicePtr; this.endpointId = endpointId; this.clusterId = clusterId; - - this.cleanable = - Cleaner.create() - .register( - this, - () -> { - if (chipClusterPtr != 0) { - deleteCluster(chipClusterPtr); - chipClusterPtr = 0; - } - }); } /** @@ -255,6 +241,15 @@ public class ChipClusters { @Deprecated public void deleteCluster(long chipClusterPtr) {} + @SuppressWarnings("deprecation") + protected void finalize() throws Throwable { + super.finalize(); + + if (chipClusterPtr != 0) { + deleteCluster(chipClusterPtr); + chipClusterPtr = 0; + } + } } abstract static class ReportCallbackImpl implements ReportCallback, SubscriptionEstablishedCallback, ResubscriptionAttemptCallback { diff --git a/scripts/py_matter_idl/matter_idl/tests/outputs/several_clusters/java/ChipClusters.java b/scripts/py_matter_idl/matter_idl/tests/outputs/several_clusters/java/ChipClusters.java index 997631bb47d546..037870ee206c5a 100644 --- a/scripts/py_matter_idl/matter_idl/tests/outputs/several_clusters/java/ChipClusters.java +++ b/scripts/py_matter_idl/matter_idl/tests/outputs/several_clusters/java/ChipClusters.java @@ -28,7 +28,6 @@ import chip.devicecontroller.model.Status; import javax.annotation.Nullable; -import java.lang.ref.Cleaner; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -93,23 +92,10 @@ public static abstract class BaseChipCluster { private Optional timeoutMillis = Optional.empty(); - private final Cleaner.Cleanable cleanable; - public BaseChipCluster(long devicePtr, int endpointId, long clusterId) { this.devicePtr = devicePtr; this.endpointId = endpointId; this.clusterId = clusterId; - - this.cleanable = - Cleaner.create() - .register( - this, - () -> { - if (chipClusterPtr != 0) { - deleteCluster(chipClusterPtr); - chipClusterPtr = 0; - } - }); } /** @@ -178,6 +164,15 @@ protected void invoke( @Deprecated public void deleteCluster(long chipClusterPtr) {} + @SuppressWarnings("deprecation") + protected void finalize() throws Throwable { + super.finalize(); + + if (chipClusterPtr != 0) { + deleteCluster(chipClusterPtr); + chipClusterPtr = 0; + } + } } abstract static class ReportCallbackImpl implements ReportCallback, SubscriptionEstablishedCallback, ResubscriptionAttemptCallback { diff --git a/src/controller/java/generated/java/chip/devicecontroller/ChipClusters.java b/src/controller/java/generated/java/chip/devicecontroller/ChipClusters.java index b41b847b1eab51..da6e6e5c7d63da 100644 --- a/src/controller/java/generated/java/chip/devicecontroller/ChipClusters.java +++ b/src/controller/java/generated/java/chip/devicecontroller/ChipClusters.java @@ -28,7 +28,6 @@ import chip.devicecontroller.model.Status; import javax.annotation.Nullable; -import java.lang.ref.Cleaner; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -93,23 +92,10 @@ public static abstract class BaseChipCluster { private Optional timeoutMillis = Optional.empty(); - private final Cleaner.Cleanable cleanable; - public BaseChipCluster(long devicePtr, int endpointId, long clusterId) { this.devicePtr = devicePtr; this.endpointId = endpointId; this.clusterId = clusterId; - - this.cleanable = - Cleaner.create() - .register( - this, - () -> { - if (chipClusterPtr != 0) { - deleteCluster(chipClusterPtr); - chipClusterPtr = 0; - } - }); } /** @@ -178,6 +164,15 @@ protected void invoke( @Deprecated public void deleteCluster(long chipClusterPtr) {} + @SuppressWarnings("deprecation") + protected void finalize() throws Throwable { + super.finalize(); + + if (chipClusterPtr != 0) { + deleteCluster(chipClusterPtr); + chipClusterPtr = 0; + } + } } abstract static class ReportCallbackImpl implements ReportCallback, SubscriptionEstablishedCallback, ResubscriptionAttemptCallback { diff --git a/src/controller/java/src/chip/devicecontroller/BatchInvokeCallbackJni.java b/src/controller/java/src/chip/devicecontroller/BatchInvokeCallbackJni.java index f873ce6addc442..6982f58429a353 100644 --- a/src/controller/java/src/chip/devicecontroller/BatchInvokeCallbackJni.java +++ b/src/controller/java/src/chip/devicecontroller/BatchInvokeCallbackJni.java @@ -19,31 +19,17 @@ import chip.devicecontroller.model.InvokeResponseData; import chip.devicecontroller.model.NoInvokeResponseData; -import java.lang.ref.Cleaner; import java.util.Optional; import javax.annotation.Nullable; /** JNI wrapper callback class for {@link InvokeCallback}. */ -public final class BatchInvokeCallbackJni { - private final BatchInvokeCallbackJni wrappedBatchInvokeCallback; +public final class ExtendableInvokeCallbackJni { + private final ExtendableInvokeCallback wrappedExtendableInvokeCallback; private long callbackHandle; - private final Cleaner.Cleanable cleanable; - - public BatchInvokeCallbackJni(BatchInvokeCallback wrappedBatchInvokeCallback) { - this.wrappedBatchInvokeCallback = wrappedBatchInvokeCallback; + public ExtendableInvokeCallbackJni(ExtendableInvokeCallback wrappedExtendableInvokeCallback) { + this.wrappedExtendableInvokeCallback = wrappedExtendableInvokeCallback; this.callbackHandle = newCallback(); - - this.cleanable = - Cleaner.create() - .register( - this, - () -> { - if (chipClusterPtr != 0) { - deleteCluster(chipClusterPtr); - chipClusterPtr = 0; - } - }); } long getCallbackHandle() { @@ -55,7 +41,7 @@ long getCallbackHandle() { private native void deleteCallback(long callbackHandle); private void onError(Exception e) { - wrappedBatchInvokeCallback.onError(e); + wrappedExtendableInvokeCallback.onError(e); } private void onResponse( @@ -88,10 +74,21 @@ private void onResponse( } private void onNoResponse(int commandRef) { - wrappedBatchInvokeCallback.onNoResponse(NoInvokeResponseData.newInstance(commandRef)); + wrappedExtendableInvokeCallback.onNoResponse(NoInvokeResponseData.newInstance(commandRef)); } private void onDone() { - wrappedBatchInvokeCallback.onDone(); + wrappedExtendableInvokeCallback.onDone(); + } + + // TODO(#8578): Replace finalizer with PhantomReference. + @SuppressWarnings("deprecation") + protected void finalize() throws Throwable { + super.finalize(); + + if (callbackHandle != 0) { + deleteCallback(callbackHandle); + callbackHandle = 0; + } } } diff --git a/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java b/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java index 893307be2116bf..c3249489749110 100644 --- a/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java +++ b/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java @@ -26,7 +26,6 @@ import chip.devicecontroller.model.ChipEventPath; import chip.devicecontroller.model.DataVersionFilter; import chip.devicecontroller.model.InvokeElement; -import java.lang.ref.Cleaner; import java.time.ZoneOffset; import java.util.ArrayList; import java.util.Calendar; @@ -45,8 +44,6 @@ public class ChipDeviceController { private ScanNetworksListener scanNetworksListener; private NOCChainIssuer nocChainIssuer; - private final Cleaner.Cleanable cleanable; - /** * To load class and jni, we need to new AndroidChipPlatform after jni load but before new * ChipDeviceController @@ -70,17 +67,6 @@ public ChipDeviceController(ControllerParams params) { throw new NullPointerException("params cannot be null"); } deviceControllerPtr = newDeviceController(params); - - this.cleanable = - Cleaner.create() - .register( - this, - () -> { - if (deviceControllerPtr != 0) { - deleteDeviceController(deviceControllerPtr); - deviceControllerPtr = 0; - } - }); } public void setCompletionListener(CompletionListener listener) { @@ -1787,6 +1773,16 @@ private native void updateCommissioningICDRegistrationInfo( System.loadLibrary("CHIPController"); } + @SuppressWarnings("deprecation") + protected void finalize() throws Throwable { + super.finalize(); + + if (deviceControllerPtr != 0) { + deleteDeviceController(deviceControllerPtr); + deviceControllerPtr = 0; + } + } + /** Interface to implement custom operational credentials issuer (NOC chain generation). */ public interface NOCChainIssuer { /** diff --git a/src/controller/java/src/chip/devicecontroller/ExtendableInvokeCallbackJni.java b/src/controller/java/src/chip/devicecontroller/ExtendableInvokeCallbackJni.java index 6c12940fb6a1d5..6982f58429a353 100644 --- a/src/controller/java/src/chip/devicecontroller/ExtendableInvokeCallbackJni.java +++ b/src/controller/java/src/chip/devicecontroller/ExtendableInvokeCallbackJni.java @@ -19,7 +19,6 @@ import chip.devicecontroller.model.InvokeResponseData; import chip.devicecontroller.model.NoInvokeResponseData; -import java.lang.ref.Cleaner; import java.util.Optional; import javax.annotation.Nullable; @@ -28,22 +27,9 @@ public final class ExtendableInvokeCallbackJni { private final ExtendableInvokeCallback wrappedExtendableInvokeCallback; private long callbackHandle; - private final Cleaner.Cleanable cleanable; - public ExtendableInvokeCallbackJni(ExtendableInvokeCallback wrappedExtendableInvokeCallback) { this.wrappedExtendableInvokeCallback = wrappedExtendableInvokeCallback; this.callbackHandle = newCallback(); - - this.cleanable = - Cleaner.create() - .register( - this, - () -> { - if (callbackHandle != 0) { - deleteCallback(callbackHandle); - callbackHandle = 0; - } - }); } long getCallbackHandle() { @@ -94,4 +80,15 @@ private void onNoResponse(int commandRef) { private void onDone() { wrappedExtendableInvokeCallback.onDone(); } + + // TODO(#8578): Replace finalizer with PhantomReference. + @SuppressWarnings("deprecation") + protected void finalize() throws Throwable { + super.finalize(); + + if (callbackHandle != 0) { + deleteCallback(callbackHandle); + callbackHandle = 0; + } + } } diff --git a/src/controller/java/src/chip/devicecontroller/GetConnectedDeviceCallbackJni.java b/src/controller/java/src/chip/devicecontroller/GetConnectedDeviceCallbackJni.java index 84338f82340cd6..b45b04a778b053 100644 --- a/src/controller/java/src/chip/devicecontroller/GetConnectedDeviceCallbackJni.java +++ b/src/controller/java/src/chip/devicecontroller/GetConnectedDeviceCallbackJni.java @@ -17,29 +17,14 @@ */ package chip.devicecontroller; -import java.lang.ref.Cleaner; - /** JNI wrapper callback class for getting a connected device. */ public class GetConnectedDeviceCallbackJni { private final GetConnectedDeviceCallback wrappedCallback; private long callbackHandle; - private final Cleaner.Cleanable cleanable; - public GetConnectedDeviceCallbackJni(GetConnectedDeviceCallback wrappedCallback) { this.wrappedCallback = wrappedCallback; this.callbackHandle = newCallback(wrappedCallback); - - this.cleanable = - Cleaner.create() - .register( - this, - () -> { - if (callbackHandle != 0) { - deleteCallback(callbackHandle); - callbackHandle = 0; - } - }); } long getCallbackHandle() { @@ -50,6 +35,17 @@ long getCallbackHandle() { private native void deleteCallback(long callbackHandle); + // TODO(#8578): Replace finalizer with PhantomReference. + @SuppressWarnings("deprecation") + protected void finalize() throws Throwable { + super.finalize(); + + if (callbackHandle != 0) { + deleteCallback(callbackHandle); + callbackHandle = 0; + } + } + /** Callbacks for getting a device connected with PASE or CASE, depending on the context. */ public interface GetConnectedDeviceCallback { void onDeviceConnected(long devicePointer); diff --git a/src/controller/java/src/chip/devicecontroller/InvokeCallbackJni.java b/src/controller/java/src/chip/devicecontroller/InvokeCallbackJni.java index 69881a778481e7..ceb35eccefd9b0 100644 --- a/src/controller/java/src/chip/devicecontroller/InvokeCallbackJni.java +++ b/src/controller/java/src/chip/devicecontroller/InvokeCallbackJni.java @@ -18,29 +18,15 @@ package chip.devicecontroller; import chip.devicecontroller.model.InvokeElement; -import java.lang.ref.Cleaner; /** JNI wrapper callback class for {@link InvokeCallback}. */ public final class InvokeCallbackJni { private final InvokeCallback wrappedInvokeCallback; private long callbackHandle; - private final Cleaner.Cleanable cleanable; - public InvokeCallbackJni(InvokeCallback wrappedInvokeCallback) { this.wrappedInvokeCallback = wrappedInvokeCallback; this.callbackHandle = newCallback(); - - this.cleanable = - Cleaner.create() - .register( - this, - () -> { - if (callbackHandle != 0) { - deleteCallback(callbackHandle); - callbackHandle = 0; - } - }); } long getCallbackHandle() { @@ -69,4 +55,15 @@ private void onResponse( private void onDone() { wrappedInvokeCallback.onDone(); } + + // TODO(#8578): Replace finalizer with PhantomReference. + @SuppressWarnings("deprecation") + protected void finalize() throws Throwable { + super.finalize(); + + if (callbackHandle != 0) { + deleteCallback(callbackHandle); + callbackHandle = 0; + } + } } diff --git a/src/controller/java/src/chip/devicecontroller/ReportCallbackJni.java b/src/controller/java/src/chip/devicecontroller/ReportCallbackJni.java index 9ff86d1e9f602b..4f2529fa8d58b0 100644 --- a/src/controller/java/src/chip/devicecontroller/ReportCallbackJni.java +++ b/src/controller/java/src/chip/devicecontroller/ReportCallbackJni.java @@ -20,7 +20,6 @@ import chip.devicecontroller.model.ChipAttributePath; import chip.devicecontroller.model.ChipEventPath; import chip.devicecontroller.model.NodeState; -import java.lang.ref.Cleaner; import javax.annotation.Nullable; /** JNI wrapper callback class for {@link ReportCallback}. */ @@ -31,8 +30,6 @@ public class ReportCallbackJni { private long callbackHandle; @Nullable private NodeState nodeState; - private final Cleaner.Cleanable cleanable; - public ReportCallbackJni( @Nullable SubscriptionEstablishedCallback subscriptionEstablishedCallback, ReportCallback reportCallback, @@ -42,17 +39,6 @@ public ReportCallbackJni( this.wrappedResubscriptionAttemptCallback = resubscriptionAttemptCallback; this.callbackHandle = newCallback(subscriptionEstablishedCallback, resubscriptionAttemptCallback); - - this.cleanable = - Cleaner.create() - .register( - this, - () -> { - if (callbackHandle != 0) { - deleteCallback(callbackHandle); - callbackHandle = 0; - } - }); } long getCallbackHandle() { @@ -102,4 +88,15 @@ private void onError( private void onDone() { wrappedReportCallback.onDone(); } + + // TODO(#8578): Replace finalizer with PhantomReference. + @SuppressWarnings("deprecation") + protected void finalize() throws Throwable { + super.finalize(); + + if (callbackHandle != 0) { + deleteCallback(callbackHandle); + callbackHandle = 0; + } + } } diff --git a/src/controller/java/src/chip/devicecontroller/WriteAttributesCallbackJni.java b/src/controller/java/src/chip/devicecontroller/WriteAttributesCallbackJni.java index 84a74c4d9eec0d..7b95b30759222f 100644 --- a/src/controller/java/src/chip/devicecontroller/WriteAttributesCallbackJni.java +++ b/src/controller/java/src/chip/devicecontroller/WriteAttributesCallbackJni.java @@ -19,7 +19,6 @@ import chip.devicecontroller.model.ChipAttributePath; import chip.devicecontroller.model.Status; -import java.lang.ref.Cleaner; import javax.annotation.Nullable; /** JNI wrapper callback class for {@link WriteAttributesCallback}. */ @@ -27,22 +26,9 @@ public final class WriteAttributesCallbackJni { private final WriteAttributesCallback wrappedWriteAttributesCallback; private long callbackHandle; - private final Cleaner.Cleanable cleanable; - public WriteAttributesCallbackJni(WriteAttributesCallback wrappedWriteAttributesCallback) { this.wrappedWriteAttributesCallback = wrappedWriteAttributesCallback; this.callbackHandle = newCallback(); - - this.cleanable = - Cleaner.create() - .register( - this, - () -> { - if (callbackHandle != 0) { - deleteCallback(callbackHandle); - callbackHandle = 0; - } - }); } long getCallbackHandle() { @@ -75,4 +61,15 @@ private void onResponse( private void onDone() { wrappedWriteAttributesCallback.onDone(); } + + // TODO(#8578): Replace finalizer with PhantomReference. + @SuppressWarnings("deprecation") + protected void finalize() throws Throwable { + super.finalize(); + + if (callbackHandle != 0) { + deleteCallback(callbackHandle); + callbackHandle = 0; + } + } } diff --git a/src/darwin/Framework/CHIP/MTRDeviceDataValidation.h b/src/darwin/Framework/CHIP/MTRDeviceDataValidation.h index 5d448d8aa4fcba..72ee6f4679341a 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceDataValidation.h +++ b/src/darwin/Framework/CHIP/MTRDeviceDataValidation.h @@ -39,4 +39,9 @@ MTR_EXTERN MTR_TESTABLE BOOL MTREventReportIsWellFormed(NSArray * response); +// Returns whether the provided invoke reponses actually has the right sorts of +// objects in the right places. This differes from +// MTRInvokeResponseIsWellFormed in not enforcing that there is only one response. +MTR_EXTERN MTR_TESTABLE BOOL MTRInvokeResponsesAreWellFormed(NSArray * response); + NS_ASSUME_NONNULL_END diff --git a/src/darwin/Framework/CHIP/MTRDeviceDataValidation.mm b/src/darwin/Framework/CHIP/MTRDeviceDataValidation.mm index 838f33d093e933..ba71a807170b6e 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceDataValidation.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceDataValidation.mm @@ -165,51 +165,68 @@ BOOL MTREventReportIsWellFormed(NSArray * repo BOOL MTRInvokeResponseIsWellFormed(NSArray * response) { - if (!MTR_SAFE_CAST(response, NSArray)) { - MTR_LOG_ERROR("Invoke response is not an array: %@", response); + if (!MTRInvokeResponsesAreWellFormed(response)) { return NO; } - // Input is an array with a single value that must have MTRCommandPathKey. + // Input is an array with a single value. if (response.count != 1) { MTR_LOG_ERROR("Invoke response is not an array with exactly one entry: %@", response); return NO; } - MTRDeviceResponseValueDictionary responseValue = response[0]; + return YES; +} - if (!MTR_SAFE_CAST(responseValue, NSDictionary) || !MTR_SAFE_CAST(responseValue[MTRCommandPathKey], MTRCommandPath)) { - MTR_LOG_ERROR("Invoke response is not an array with the right things in it: %@", response); +BOOL MTRInvokeResponsesAreWellFormed(NSArray * response) +{ + if (!MTR_SAFE_CAST(response, NSArray)) { + MTR_LOG_ERROR("Invoke response is not an array: %@", response); return NO; } - MTRDeviceDataValueDictionary _Nullable data = responseValue[MTRDataKey]; - NSError * _Nullable error = responseValue[MTRErrorKey]; + for (MTRDeviceResponseValueDictionary responseValue in response) { + // Each entry must be a dictionary that has MTRCommandPathKey. - if (data != nil && error != nil) { - MTR_LOG_ERROR("Invoke response claims to have both data and error: %@", responseValue); - return NO; - } + if (!MTR_SAFE_CAST(responseValue, NSDictionary) || !MTR_SAFE_CAST(responseValue[MTRCommandPathKey], MTRCommandPath)) { + MTR_LOG_ERROR("Invoke response has an invalid array entry: %@", responseValue); + return NO; + } - if (error != nil) { - return MTR_SAFE_CAST(error, NSError) != nil; - } + MTRDeviceDataValueDictionary _Nullable data = responseValue[MTRDataKey]; + NSError * _Nullable error = responseValue[MTRErrorKey]; - if (data == nil) { - // This is valid: indicates a success status response. - return YES; - } + if (data != nil && error != nil) { + MTR_LOG_ERROR("Invoke response claims to have both data and error: %@", responseValue); + return NO; + } - if (!MTRDataValueDictionaryIsWellFormed(data)) { - MTR_LOG_ERROR("Invoke response claims to have data that is not a data-value: %@", data); - return NO; - } + if (error != nil) { + if (!MTR_SAFE_CAST(error, NSError)) { + MTR_LOG_ERROR("Invoke response %@ has %@ instead of an NSError", responseValue, error); + return NO; + } - // Now we know data is a dictionary (in fact a data-value). The only thing - // we promise about it is that it has type MTRStructureValueType. - if (![MTRStructureValueType isEqual:data[MTRTypeKey]]) { - MTR_LOG_ERROR("Invoke response data is not of structure type: %@", data); - return NO; + // Valid error response. + continue; + } + + if (data == nil) { + // This is valid: indicates a success status response. + continue; + } + + if (!MTRDataValueDictionaryIsWellFormed(data)) { + MTR_LOG_ERROR("Invoke response claims to have data that is not a data-value: %@", data); + return NO; + } + + // Now we know data is a dictionary (in fact a data-value). The only thing + // we promise about it is that it has type MTRStructureValueType. + if (![MTRStructureValueType isEqual:data[MTRTypeKey]]) { + MTR_LOG_ERROR("Invoke response data is not of structure type: %@", data); + return NO; + } } return YES; diff --git a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm index 2a9ef219ee0956..4e29b0e5e3d3a3 100644 --- a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm +++ b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm @@ -331,6 +331,9 @@ @interface MTRDevice_Concrete () @property (nonatomic) NSDate * estimatedStartTimeFromGeneralDiagnosticsUpTime; +@property (nonatomic) NSDate * lastDeviceBecameActiveCallbackTime; +@property (nonatomic) BOOL throttlingDeviceBecameActiveCallbacks; + /** * If currentReadClient is non-null, that means that we successfully * called SendAutoResubscribeRequest on the ReadClient and have not yet gotten @@ -470,6 +473,7 @@ - (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceControlle _persistedClusters = [NSMutableSet set]; _highestObservedEventNumber = nil; _matterCPPObjectsHolder = [[MTRDeviceMatterCPPObjectsHolder alloc] init]; + _throttlingDeviceBecameActiveCallbacks = NO; // If there is a data store, make sure we have an observer to monitor system clock changes, so // NSDate-based write coalescing could be reset and not get into a bad state. @@ -864,6 +868,7 @@ - (void)_setDSTOffsets:(NSArray // subscription intervals are in seconds #define MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MIN (10 * 60) // 10 minutes (for now) #define MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MAX (60 * 60) // 60 minutes +#define MTR_DEVICE_MIN_SECONDS_BETWEEN_DEVICE_BECAME_ACTIVE_CALLBACKS (1 * 60) // 1 minute (for now) - (BOOL)_subscriptionsAllowed { @@ -1634,12 +1639,29 @@ - (void)_handleUnsolicitedMessageFromPublisher [self _changeState:MTRDeviceStateReachable]; - [self _callDelegatesWithBlock:^(id delegate) { - if ([delegate respondsToSelector:@selector(deviceBecameActive:)]) { - [delegate deviceBecameActive:self]; + // Given the framework requests a minimum subscription keep alive time of devices, this callback is not expected to happen more often than that + BOOL shouldCallDelegate = NO; + if (self.lastDeviceBecameActiveCallbackTime) { + NSTimeInterval intervalSinceLastCallback = -[self.lastDeviceBecameActiveCallbackTime timeIntervalSinceNow]; + if (intervalSinceLastCallback > MTR_DEVICE_MIN_SECONDS_BETWEEN_DEVICE_BECAME_ACTIVE_CALLBACKS) { + shouldCallDelegate = YES; } - }]; - [self _notifyDelegateOfPrivateInternalPropertiesChanges]; + } else { + shouldCallDelegate = YES; + } + + if (shouldCallDelegate) { + [self _callDelegatesWithBlock:^(id delegate) { + if ([delegate respondsToSelector:@selector(deviceBecameActive:)]) { + [delegate deviceBecameActive:self]; + } + }]; + self.lastDeviceBecameActiveCallbackTime = [NSDate now]; + self.throttlingDeviceBecameActiveCallbacks = NO; + } else if (!self.throttlingDeviceBecameActiveCallbacks) { + MTR_LOG("%@ throttling deviceBecameActive callbacks because report came in too soon after %@", self, self.lastDeviceBecameActiveCallbackTime); + self.throttlingDeviceBecameActiveCallbacks = YES; + } // in case this is called during exponential back off of subscription // reestablishment, this starts the attempt right away diff --git a/src/darwin/Framework/CHIP/MTRDevice_XPC.mm b/src/darwin/Framework/CHIP/MTRDevice_XPC.mm index 31cf08287657d7..fed4238f6b0554 100644 --- a/src/darwin/Framework/CHIP/MTRDevice_XPC.mm +++ b/src/darwin/Framework/CHIP/MTRDevice_XPC.mm @@ -488,7 +488,7 @@ - (void)invokeCommands:(NSArray *> *)c return; } - if (responses != nil && !MTRInvokeResponseIsWellFormed(responses)) { + if (responses != nil && !MTRInvokeResponsesAreWellFormed(responses)) { MTR_LOG_ERROR("%@ got invoke responses for %@ that has invalid data: %@", self, commands, responses); completion(nil, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INVALID_ARGUMENT]); return; diff --git a/src/darwin/Framework/CHIP/MTROTAImageTransferHandler.mm b/src/darwin/Framework/CHIP/MTROTAImageTransferHandler.mm index efa84ef9168a8e..6a6dce94df3d46 100644 --- a/src/darwin/Framework/CHIP/MTROTAImageTransferHandler.mm +++ b/src/darwin/Framework/CHIP/MTROTAImageTransferHandler.mm @@ -310,7 +310,7 @@ - (instancetype)initWithMTROTAImageTransferHandler:(MTROTAImageTransferHandler * TransferSession::OutputEventType eventType = event.EventType; - ChipLogError(BDX, "OutputEvent type: %s", event.ToString(eventType)); + ChipLogDetail(BDX, "OutputEvent type: %s", event.ToString(eventType)); CHIP_ERROR err = CHIP_NO_ERROR; switch (event.EventType) { diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m index 71f6aa29fb35d2..7184e434a4eff9 100644 --- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m +++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m @@ -5752,6 +5752,7 @@ - (void)test045_MTRDeviceInvokeGroups completion:^(NSArray *> * _Nullable values, NSError * _Nullable error) { XCTAssertNil(error); XCTAssertNotNil(values); + XCTAssertTrue(MTRInvokeResponsesAreWellFormed(values)); // Successful invoke is represented as a value with the relevant // command path and neither data nor error. @@ -5781,6 +5782,7 @@ - (void)test045_MTRDeviceInvokeGroups completion:^(NSArray *> * _Nullable values, NSError * _Nullable error) { XCTAssertNil(error); XCTAssertNotNil(values); + XCTAssertTrue(MTRInvokeResponsesAreWellFormed(values)); // We should not have anything for groups after the first one XCTAssertEqual(values.count, 2); @@ -5814,6 +5816,7 @@ - (void)test045_MTRDeviceInvokeGroups completion:^(NSArray *> * _Nullable values, NSError * _Nullable error) { XCTAssertNil(error); XCTAssertNotNil(values); + XCTAssertTrue(MTRInvokeResponsesAreWellFormed(values)); // We should not have anything for groups after the first one __auto_type * expectedValues = @[ @@ -5858,6 +5861,7 @@ - (void)test045_MTRDeviceInvokeGroups completion:^(NSArray *> * _Nullable values, NSError * _Nullable error) { XCTAssertNil(error); XCTAssertNotNil(values); + XCTAssertTrue(MTRInvokeResponsesAreWellFormed(values)); XCTAssertEqual(values.count, 3); @@ -5902,6 +5906,7 @@ - (void)test045_MTRDeviceInvokeGroups completion:^(NSArray *> * _Nullable values, NSError * _Nullable error) { XCTAssertNil(error); XCTAssertNotNil(values); + XCTAssertTrue(MTRInvokeResponsesAreWellFormed(values)); XCTAssertEqual(values.count, 3); @@ -5946,6 +5951,7 @@ - (void)test045_MTRDeviceInvokeGroups completion:^(NSArray *> * _Nullable values, NSError * _Nullable error) { XCTAssertNil(error); XCTAssertNotNil(values); + XCTAssertTrue(MTRInvokeResponsesAreWellFormed(values)); XCTAssertEqual(values.count, 2); @@ -5988,6 +5994,7 @@ - (void)test045_MTRDeviceInvokeGroups completion:^(NSArray *> * _Nullable values, NSError * _Nullable error) { XCTAssertNil(error); XCTAssertNotNil(values); + XCTAssertTrue(MTRInvokeResponsesAreWellFormed(values)); XCTAssertEqual(values.count, 2); diff --git a/src/platform/Tizen/ThreadStackManagerImpl.cpp b/src/platform/Tizen/ThreadStackManagerImpl.cpp index 0ae0f8f84ad7c8..f69974521c55b6 100644 --- a/src/platform/Tizen/ThreadStackManagerImpl.cpp +++ b/src/platform/Tizen/ThreadStackManagerImpl.cpp @@ -521,12 +521,15 @@ CHIP_ERROR ThreadStackManagerImpl::_GetThreadVersion(uint16_t & version) { VerifyOrReturnError(mIsInitialized, CHIP_ERROR_UNINITIALIZED); +#if defined(TIZEN_NETWORK_THREAD_VERSION) && TIZEN_NETWORK_THREAD_VERSION >= 0x000900 int threadErr = thread_get_version(mThreadInstance, &version); VerifyOrReturnError(threadErr == THREAD_ERROR_NONE, TizenToChipError(threadErr), ChipLogError(DeviceLayer, "FAIL: Get thread version: %s", get_error_message(threadErr))); - ChipLogProgress(DeviceLayer, "Thread version [%u]", version); return CHIP_NO_ERROR; +#else + return CHIP_ERROR_NOT_IMPLEMENTED; +#endif } CHIP_ERROR ThreadStackManagerImpl::_GetPollPeriod(uint32_t & buf) diff --git a/src/protocols/bdx/AsyncTransferFacilitator.cpp b/src/protocols/bdx/AsyncTransferFacilitator.cpp index eb3d996b1e53b8..94cb3f1f3c2b55 100644 --- a/src/protocols/bdx/AsyncTransferFacilitator.cpp +++ b/src/protocols/bdx/AsyncTransferFacilitator.cpp @@ -178,9 +178,6 @@ CHIP_ERROR AsyncResponder::Init(System::Layer * layer, Messaging::ExchangeContex void AsyncResponder::NotifyEventHandled(const TransferSession::OutputEventType eventType, CHIP_ERROR status) { - ChipLogDetail(BDX, "NotifyEventHandled : Event %s Error %" CHIP_ERROR_FORMAT, - TransferSession::OutputEvent::TypeToString(eventType), status.Format()); - // If this is the end of the transfer (whether a clean end, or some sort of error condition), ensure // that we destroy ourselves after unwinding the processing loop in the ProcessOutputEvents API. // We can ignore the status for these output events because none of them are supposed to result in @@ -194,6 +191,8 @@ void AsyncResponder::NotifyEventHandled(const TransferSession::OutputEventType e eventType == TransferSession::OutputEventType::kTransferTimeout || eventType == TransferSession::OutputEventType::kStatusReceived) { + ChipLogProgress(BDX, "NotifyEventHandled : Event %s Error %" CHIP_ERROR_FORMAT, + TransferSession::OutputEvent::TypeToString(eventType), status.Format()); mDestroySelfAfterProcessingEvents = true; } else if (status != CHIP_NO_ERROR) diff --git a/src/protocols/bdx/BdxTransferSession.cpp b/src/protocols/bdx/BdxTransferSession.cpp index fdf9bb2c848e54..c2715ef1b01f76 100644 --- a/src/protocols/bdx/BdxTransferSession.cpp +++ b/src/protocols/bdx/BdxTransferSession.cpp @@ -291,11 +291,6 @@ CHIP_ERROR TransferSession::PrepareBlockQuery() ReturnErrorOnFailure(WriteToPacketBuffer(queryMsg, mPendingMsgHandle)); -#if CHIP_AUTOMATION_LOGGING - ChipLogAutomation("Sending BDX Message"); - queryMsg.LogMessage(msgType); -#endif // CHIP_AUTOMATION_LOGGING - mAwaitingResponse = true; mLastQueryNum = mNextQueryNum++; @@ -319,11 +314,6 @@ CHIP_ERROR TransferSession::PrepareBlockQueryWithSkip(const uint64_t & bytesToSk ReturnErrorOnFailure(WriteToPacketBuffer(queryMsg, mPendingMsgHandle)); -#if CHIP_AUTOMATION_LOGGING - ChipLogAutomation("Sending BDX Message"); - queryMsg.LogMessage(msgType); -#endif // CHIP_AUTOMATION_LOGGING - mAwaitingResponse = true; mLastQueryNum = mNextQueryNum++; @@ -351,13 +341,12 @@ CHIP_ERROR TransferSession::PrepareBlock(const BlockData & inData) const MessageType msgType = inData.IsEof ? MessageType::BlockEOF : MessageType::Block; -#if CHIP_AUTOMATION_LOGGING - ChipLogAutomation("Sending BDX Message"); - blockMsg.LogMessage(msgType); -#endif // CHIP_AUTOMATION_LOGGING - if (msgType == MessageType::BlockEOF) { +#if CHIP_AUTOMATION_LOGGING + ChipLogAutomation("Sending BDX Message"); + blockMsg.LogMessage(msgType); +#endif // CHIP_AUTOMATION_LOGGING mState = TransferState::kAwaitingEOFAck; } @@ -382,11 +371,6 @@ CHIP_ERROR TransferSession::PrepareBlockAck() ReturnErrorOnFailure(WriteToPacketBuffer(ackMsg, mPendingMsgHandle)); -#if CHIP_AUTOMATION_LOGGING - ChipLogAutomation("Sending BDX Message"); - ackMsg.LogMessage(msgType); -#endif // CHIP_AUTOMATION_LOGGING - if (mState == TransferState::kTransferInProgress) { if (mControlMode == TransferControlFlags::kSenderDrive) @@ -399,6 +383,10 @@ CHIP_ERROR TransferSession::PrepareBlockAck() } else if (mState == TransferState::kReceivedEOF) { +#if CHIP_AUTOMATION_LOGGING + ChipLogAutomation("Sending BDX Message"); + ackMsg.LogMessage(msgType); +#endif // CHIP_AUTOMATION_LOGGING mState = TransferState::kTransferDone; mAwaitingResponse = false; } @@ -475,20 +463,25 @@ CHIP_ERROR TransferSession::HandleBdxMessage(const PayloadHeader & header, Syste const MessageType msgType = static_cast(header.GetMessageType()); -#if CHIP_AUTOMATION_LOGGING - ChipLogAutomation("Handling received BDX Message"); -#endif // CHIP_AUTOMATION_LOGGING - switch (msgType) { case MessageType::SendInit: case MessageType::ReceiveInit: +#if CHIP_AUTOMATION_LOGGING + ChipLogAutomation("Handling received BDX Message"); +#endif // CHIP_AUTOMATION_LOGGING HandleTransferInit(msgType, std::move(msg)); break; case MessageType::SendAccept: +#if CHIP_AUTOMATION_LOGGING + ChipLogAutomation("Handling received BDX Message"); +#endif // CHIP_AUTOMATION_LOGGING HandleSendAccept(std::move(msg)); break; case MessageType::ReceiveAccept: +#if CHIP_AUTOMATION_LOGGING + ChipLogAutomation("Handling received BDX Message"); +#endif // CHIP_AUTOMATION_LOGGING HandleReceiveAccept(std::move(msg)); break; case MessageType::BlockQuery: @@ -672,10 +665,6 @@ void TransferSession::HandleBlockQuery(System::PacketBufferHandle msgData) mAwaitingResponse = false; mLastQueryNum = query.BlockCounter; - -#if CHIP_AUTOMATION_LOGGING - query.LogMessage(MessageType::BlockQuery); -#endif // CHIP_AUTOMATION_LOGGING } void TransferSession::HandleBlockQueryWithSkip(System::PacketBufferHandle msgData) @@ -695,10 +684,6 @@ void TransferSession::HandleBlockQueryWithSkip(System::PacketBufferHandle msgDat mAwaitingResponse = false; mLastQueryNum = query.BlockCounter; mBytesToSkip.BytesToSkip = query.BytesToSkip; - -#if CHIP_AUTOMATION_LOGGING - query.LogMessage(MessageType::BlockQueryWithSkip); -#endif // CHIP_AUTOMATION_LOGGING } void TransferSession::HandleBlock(System::PacketBufferHandle msgData) @@ -733,10 +718,6 @@ void TransferSession::HandleBlock(System::PacketBufferHandle msgData) mLastBlockNum = blockMsg.BlockCounter; mAwaitingResponse = false; - -#if CHIP_AUTOMATION_LOGGING - blockMsg.LogMessage(MessageType::Block); -#endif // CHIP_AUTOMATION_LOGGING } void TransferSession::HandleBlockEOF(System::PacketBufferHandle msgData) @@ -787,10 +768,6 @@ void TransferSession::HandleBlockAck(System::PacketBufferHandle msgData) // In Receiver Drive, the Receiver can send a BlockAck to indicate receipt of the message and reset the timeout. // In this case, the Sender should wait to receive a BlockQuery next. mAwaitingResponse = (mControlMode == TransferControlFlags::kReceiverDrive); - -#if CHIP_AUTOMATION_LOGGING - ackMsg.LogMessage(MessageType::BlockAck); -#endif // CHIP_AUTOMATION_LOGGING } void TransferSession::HandleBlockAckEOF(System::PacketBufferHandle msgData) diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp index c608c5b1ca4be9..c314ce404800e1 100644 --- a/src/protocols/secure_channel/CASESession.cpp +++ b/src/protocols/secure_channel/CASESession.cpp @@ -1054,7 +1054,7 @@ CASESession::NextStep CASESession::HandleSigma1(System::PacketBufferHandle && ms ReturnErrorVariantOnFailure(NextStep, ParseSigma1(tlvReader, parsedSigma1)); - ChipLogDetail(SecureChannel, "Peer assigned session key ID %d", parsedSigma1.initiatorSessionId); + ChipLogDetail(SecureChannel, "Peer (Initiator) assigned session ID %d", parsedSigma1.initiatorSessionId); SetPeerSessionId(parsedSigma1.initiatorSessionId); // Set the Session parameters provided in the Sigma1 message @@ -1222,23 +1222,23 @@ CHIP_ERROR CASESession::PrepareSigma2(EncodeSigma2Inputs & outSigma2Data) ReturnErrorOnFailure(DeriveSigmaKey(saltSpan, ByteSpan(kKDFSR2Info), sr2k)); // Construct Sigma2 TBS Data - size_t msgR2SignedLen = EstimateStructOverhead(kMaxCHIPCertLength, // responderNoc - kMaxCHIPCertLength, // responderICAC - kP256_PublicKey_Length, // responderEphPubKey - kP256_PublicKey_Length // InitiatorEphPubKey - ); - P256ECDSASignature tbsData2Signature; { + size_t msgR2SignedLen = EstimateStructOverhead(kMaxCHIPCertLength, // responderNoc + kMaxCHIPCertLength, // responderICAC + kP256_PublicKey_Length, // responderEphPubKey + kP256_PublicKey_Length // InitiatorEphPubKey + ); + chip::Platform::ScopedMemoryBuffer msgR2Signed; VerifyOrReturnError(msgR2Signed.Alloc(msgR2SignedLen), CHIP_ERROR_NO_MEMORY); + MutableByteSpan msgR2SignedSpan{ msgR2Signed.Get(), msgR2SignedLen }; ReturnErrorOnFailure(ConstructTBSData(nocCert, icaCert, ByteSpan(mEphemeralKey->Pubkey(), mEphemeralKey->Pubkey().Length()), - ByteSpan(mRemotePubKey, mRemotePubKey.Length()), msgR2Signed.Get(), msgR2SignedLen)); + ByteSpan(mRemotePubKey, mRemotePubKey.Length()), msgR2SignedSpan)); // Generate a Signature - ReturnErrorOnFailure( - mFabricsTable->SignWithOpKeypair(mFabricIndex, ByteSpan{ msgR2Signed.Get(), msgR2SignedLen }, tbsData2Signature)); + ReturnErrorOnFailure(mFabricsTable->SignWithOpKeypair(mFabricIndex, msgR2SignedSpan, tbsData2Signature)); } // Construct Sigma2 TBE Data size_t msgR2SignedEncLen = EstimateStructOverhead(nocCert.size(), // responderNoc @@ -1381,7 +1381,8 @@ CHIP_ERROR CASESession::HandleSigma2Resume(System::PacketBufferHandle && msg) GetRemoteSessionParameters()); } - ChipLogDetail(SecureChannel, "Peer assigned session ID %d", parsedSigma2Resume.responderSessionId); + ChipLogDetail(SecureChannel, "Peer " ChipLogFormatScopedNodeId " assigned session ID %d", ChipLogValueScopedNodeId(GetPeer()), + parsedSigma2Resume.responderSessionId); SetPeerSessionId(parsedSigma2Resume.responderSessionId); if (mSessionResumptionStorage != nullptr) @@ -1517,12 +1518,15 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg) nullptr, 0, parsedSigma2.msgR2MIC.data(), parsedSigma2.msgR2MIC.size(), sr2k.KeyHandle(), kTBEData2_Nonce, kTBEDataNonceLength, parsedSigma2.msgR2EncryptedPayload.data())); + parsedSigma2.msgR2Decrypted = std::move(parsedSigma2.msgR2Encrypted); + size_t msgR2DecryptedLength = parsedSigma2.msgR2EncryptedPayload.size(); + ContiguousBufferTLVReader decryptedDataTlvReader; - decryptedDataTlvReader.Init(parsedSigma2.msgR2EncryptedPayload.data(), parsedSigma2.msgR2EncryptedPayload.size()); + decryptedDataTlvReader.Init(parsedSigma2.msgR2Decrypted.Get(), msgR2DecryptedLength); ParsedSigma2TBEData parsedSigma2TBEData; ReturnErrorOnFailure(ParseSigma2TBEData(decryptedDataTlvReader, parsedSigma2TBEData)); - // Validate responder identity located in msgR2Encrypted + // Validate responder identity located in msgR2Decrypted // Constructing responder identity P256PublicKey responderPublicKey; { @@ -1540,7 +1544,7 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg) VerifyOrReturnError(mPeerNodeId == responderNodeId, CHIP_ERROR_INVALID_CASE_PARAMETER); } - // Construct msgR2Signed and validate the signature in msgR2Encrypted. + // Construct msgR2Signed and validate the signature in msgR2Decrypted. size_t msgR2SignedLen = EstimateStructOverhead(parsedSigma2TBEData.responderNOC.size(), // resonderNOC parsedSigma2TBEData.responderICAC.size(), // responderICAC kP256_PublicKey_Length, // responderEphPubKey @@ -1549,16 +1553,18 @@ CHIP_ERROR CASESession::HandleSigma2(System::PacketBufferHandle && msg) chip::Platform::ScopedMemoryBuffer msgR2Signed; VerifyOrReturnError(msgR2Signed.Alloc(msgR2SignedLen), CHIP_ERROR_NO_MEMORY); + MutableByteSpan msgR2SignedSpan{ msgR2Signed.Get(), msgR2SignedLen }; - ReturnErrorOnFailure(ConstructTBSData( - parsedSigma2TBEData.responderNOC, parsedSigma2TBEData.responderICAC, ByteSpan(mRemotePubKey, mRemotePubKey.Length()), - ByteSpan(mEphemeralKey->Pubkey(), mEphemeralKey->Pubkey().Length()), msgR2Signed.Get(), msgR2SignedLen)); + ReturnErrorOnFailure(ConstructTBSData(parsedSigma2TBEData.responderNOC, parsedSigma2TBEData.responderICAC, + ByteSpan(mRemotePubKey, mRemotePubKey.Length()), + ByteSpan(mEphemeralKey->Pubkey(), mEphemeralKey->Pubkey().Length()), msgR2SignedSpan)); // Validate signature - ReturnErrorOnFailure( - responderPublicKey.ECDSA_validate_msg_signature(msgR2Signed.Get(), msgR2SignedLen, parsedSigma2TBEData.tbsData2Signature)); + ReturnErrorOnFailure(responderPublicKey.ECDSA_validate_msg_signature(msgR2SignedSpan.data(), msgR2SignedSpan.size(), + parsedSigma2TBEData.tbsData2Signature)); - ChipLogDetail(SecureChannel, "Peer assigned session ID %d", parsedSigma2.responderSessionId); + ChipLogDetail(SecureChannel, "Peer " ChipLogFormatScopedNodeId " assigned session ID %d", ChipLogValueScopedNodeId(GetPeer()), + parsedSigma2.responderSessionId); SetPeerSessionId(parsedSigma2.responderSessionId); std::copy(parsedSigma2TBEData.resumptionId.begin(), parsedSigma2TBEData.resumptionId.end(), mNewResumptionId.begin()); @@ -1728,14 +1734,18 @@ CHIP_ERROR CASESession::SendSigma3a() ReturnErrorOnFailure(mFabricsTable->FetchNOCCert(mFabricIndex, data.nocCert)); // Prepare Sigma3 TBS Data Blob - data.msg_r3_signed_len = - EstimateStructOverhead(data.icaCert.size(), data.nocCert.size(), kP256_PublicKey_Length, kP256_PublicKey_Length); + size_t msgR3SignedLen = EstimateStructOverhead(data.nocCert.size(), // initiatorNOC + data.icaCert.size(), // initiatorICAC + kP256_PublicKey_Length, // initiatorEphPubKey + kP256_PublicKey_Length // responderEphPubKey + ); - VerifyOrReturnError(data.msg_R3_Signed.Alloc(data.msg_r3_signed_len), CHIP_ERROR_NO_MEMORY); + VerifyOrReturnError(data.msgR3Signed.Alloc(msgR3SignedLen), CHIP_ERROR_NO_MEMORY); + data.msgR3SignedSpan = MutableByteSpan{ data.msgR3Signed.Get(), msgR3SignedLen }; - ReturnErrorOnFailure( - ConstructTBSData(data.nocCert, data.icaCert, ByteSpan(mEphemeralKey->Pubkey(), mEphemeralKey->Pubkey().Length()), - ByteSpan(mRemotePubKey, mRemotePubKey.Length()), data.msg_R3_Signed.Get(), data.msg_r3_signed_len)); + ReturnErrorOnFailure(ConstructTBSData(data.nocCert, data.icaCert, + ByteSpan(mEphemeralKey->Pubkey(), mEphemeralKey->Pubkey().Length()), + ByteSpan(mRemotePubKey, mRemotePubKey.Length()), data.msgR3SignedSpan)); if (data.keystore != nullptr) { @@ -1759,14 +1769,12 @@ CHIP_ERROR CASESession::SendSigma3b(SendSigma3Data & data, bool & cancel) if (data.keystore != nullptr) { // Recommended case: delegate to operational keystore - ReturnErrorOnFailure(data.keystore->SignWithOpKeypair( - data.fabricIndex, ByteSpan{ data.msg_R3_Signed.Get(), data.msg_r3_signed_len }, data.tbsData3Signature)); + ReturnErrorOnFailure(data.keystore->SignWithOpKeypair(data.fabricIndex, data.msgR3SignedSpan, data.tbsData3Signature)); } else { // Legacy case: delegate to fabric table fabric info - ReturnErrorOnFailure(data.fabricTable->SignWithOpKeypair( - data.fabricIndex, ByteSpan{ data.msg_R3_Signed.Get(), data.msg_r3_signed_len }, data.tbsData3Signature)); + ReturnErrorOnFailure(data.fabricTable->SignWithOpKeypair(data.fabricIndex, data.msgR3SignedSpan, data.tbsData3Signature)); } // Prepare Sigma3 TBE Data Blob @@ -1950,17 +1958,18 @@ CHIP_ERROR CASESession::HandleSigma3a(System::PacketBufferHandle && msg) SuccessOrExit(err = ParseSigma3TBEData(decryptedDataTlvReader, data)); // Step 3 - Construct Sigma3 TBS Data - data.msgR3SignedLen = TLV::EstimateStructOverhead(data.initiatorNOC.size(), // initiatorNOC - data.initiatorICAC.size(), // initiatorICAC - kP256_PublicKey_Length, // initiatorEphPubKey - kP256_PublicKey_Length // responderEphPubKey + size_t msgR3SignedLen = TLV::EstimateStructOverhead(data.initiatorNOC.size(), // initiatorNOC + data.initiatorICAC.size(), // initiatorICAC + kP256_PublicKey_Length, // initiatorEphPubKey + kP256_PublicKey_Length // responderEphPubKey ); - VerifyOrExit(data.msgR3Signed.Alloc(data.msgR3SignedLen), err = CHIP_ERROR_NO_MEMORY); + VerifyOrExit(data.msgR3Signed.Alloc(msgR3SignedLen), err = CHIP_ERROR_NO_MEMORY); + data.msgR3SignedSpan = MutableByteSpan{ data.msgR3Signed.Get(), msgR3SignedLen }; SuccessOrExit(err = ConstructTBSData(data.initiatorNOC, data.initiatorICAC, ByteSpan(mRemotePubKey, mRemotePubKey.Length()), ByteSpan(mEphemeralKey->Pubkey(), mEphemeralKey->Pubkey().Length()), - data.msgR3Signed.Get(), data.msgR3SignedLen)); + data.msgR3SignedSpan)); // Prepare for Step 4/5 { @@ -1977,9 +1986,9 @@ CHIP_ERROR CASESession::HandleSigma3a(System::PacketBufferHandle && msg) // initiatorNOC and initiatorICAC are spans into msgR3Encrypted // which is going away, so to save memory, redirect them to their - // copies in msg_R3_signed, which is staying around + // copies in msgR3Signed, which is staying around TLV::ContiguousBufferTLVReader signedDataTlvReader; - signedDataTlvReader.Init(data.msgR3Signed.Get(), data.msgR3SignedLen); + signedDataTlvReader.Init(data.msgR3SignedSpan); SuccessOrExit(err = signedDataTlvReader.Next(containerType, AnonymousTag())); SuccessOrExit(err = signedDataTlvReader.EnterContainer(containerType)); @@ -2089,14 +2098,9 @@ CHIP_ERROR CASESession::HandleSigma3b(HandleSigma3Data & data, bool & cancel) unused, initiatorFabricId, data.initiatorNodeId, initiatorPublicKey)); VerifyOrReturnError(data.fabricId == initiatorFabricId, CHIP_ERROR_INVALID_CASE_PARAMETER); - // TODO - Validate message signature prior to validating the received operational credentials. - // The op cert check requires traversal of cert chain, that is a more expensive operation. - // If message signature check fails, the cert chain check will be unnecessary, but with the - // current flow of code, a malicious node can trigger a DoS style attack on the device. - // The same change should be made in Sigma2 processing. // Step 7 - Validate Signature - ReturnErrorOnFailure( - initiatorPublicKey.ECDSA_validate_msg_signature(data.msgR3Signed.Get(), data.msgR3SignedLen, data.tbsData3Signature)); + ReturnErrorOnFailure(initiatorPublicKey.ECDSA_validate_msg_signature(data.msgR3SignedSpan.data(), data.msgR3SignedSpan.size(), + data.tbsData3Signature)); return CHIP_NO_ERROR; } @@ -2241,12 +2245,12 @@ CHIP_ERROR CASESession::ValidateSigmaResumeMIC(const ByteSpan & resumeMIC, const } CHIP_ERROR CASESession::ConstructTBSData(const ByteSpan & senderNOC, const ByteSpan & senderICAC, const ByteSpan & senderPubKey, - const ByteSpan & receiverPubKey, uint8_t * tbsData, size_t & tbsDataLen) + const ByteSpan & receiverPubKey, MutableByteSpan & outTbsData) { TLVWriter tlvWriter; TLVType outerContainerType = kTLVType_NotSpecified; - tlvWriter.Init(tbsData, tbsDataLen); + tlvWriter.Init(outTbsData); ReturnErrorOnFailure(tlvWriter.StartContainer(AnonymousTag(), kTLVType_Structure, outerContainerType)); ReturnErrorOnFailure(tlvWriter.Put(AsTlvContextTag(TBSDataTags::kSenderNOC), senderNOC)); if (!senderICAC.empty()) @@ -2257,7 +2261,7 @@ CHIP_ERROR CASESession::ConstructTBSData(const ByteSpan & senderNOC, const ByteS ReturnErrorOnFailure(tlvWriter.Put(AsTlvContextTag(TBSDataTags::kReceiverPubKey), receiverPubKey)); ReturnErrorOnFailure(tlvWriter.EndContainer(outerContainerType)); ReturnErrorOnFailure(tlvWriter.Finalize()); - tbsDataLen = static_cast(tlvWriter.GetLengthWritten()); + outTbsData.reduce_size(static_cast(tlvWriter.GetLengthWritten())); return CHIP_NO_ERROR; } diff --git a/src/protocols/secure_channel/CASESession.h b/src/protocols/secure_channel/CASESession.h index d232e42d22c26e..ccbcc6dd1c142c 100644 --- a/src/protocols/secure_channel/CASESession.h +++ b/src/protocols/secure_channel/CASESession.h @@ -249,6 +249,7 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler, ByteSpan responderEphPubKey; Platform::ScopedMemoryBufferWithSize msgR2Encrypted; + Platform::ScopedMemoryBufferWithSize msgR2Decrypted; // Below ByteSpans are Backed by: msgR2Encrypted buffer // Lifetime: Valid as long as msgR2Encrypted is not released MutableByteSpan msgR2EncryptedPayload; @@ -260,8 +261,8 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler, struct ParsedSigma2TBEData { - // Below ByteSpans are Backed by: msgR2Encrypted Buffer, member of ParsedSigma2 struct - // Lifetime: Valid for the lifetime of the instance of ParsedSigma2 that contains the msgR2Encrypted Buffer. + // Below ByteSpans are Backed by: msgR2Decrypted Buffer, member of ParsedSigma2 struct + // Lifetime: Valid for the lifetime of the instance of ParsedSigma2 that contains the msgR2Decrypted Buffer. ByteSpan responderNOC; ByteSpan responderICAC; ByteSpan resumptionId; @@ -297,8 +298,8 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler, const FabricTable * fabricTable; const Crypto::OperationalKeystore * keystore; - chip::Platform::ScopedMemoryBuffer msg_R3_Signed; - size_t msg_r3_signed_len; + chip::Platform::ScopedMemoryBuffer msgR3Signed; + MutableByteSpan msgR3SignedSpan; chip::Platform::ScopedMemoryBuffer msg_R3_Encrypted; size_t msg_r3_encrypted_len; @@ -315,7 +316,7 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler, struct HandleSigma3Data { chip::Platform::ScopedMemoryBuffer msgR3Signed; - size_t msgR3SignedLen; + MutableByteSpan msgR3SignedSpan; // Below ByteSpans are Backed by: msgR3Encrypted Buffer, local to the HandleSigma3a() method, // The Spans are later modified to point to the msgR3Signed member of this struct. @@ -382,9 +383,9 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler, * Parse a decrypted TBEData2Encrypted message. This function will return success only if the message passes schema checks. * * @param tlvReader a reference to the TLVReader that points to the decrypted TBEData2Encrypted buffer (i.e. - * msgR2Encrypted member of ParsedSigma2 struct) + * msgR2Decrypted member of ParsedSigma2 struct) * @param outParsedSigma2TBEData a reference to ParsedSigma2TBEData. All members of parsedMessage will stay valid as long - * as the msgR2Encrypted member of ParsedSigma2 is valid + * as the msgR2Decrypted member of ParsedSigma2 is valid * * @note Calls to this function must always be made with a newly created and fresh ParsedSigma2TBEData parameter. **/ @@ -513,7 +514,7 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler, CHIP_ERROR ConstructSaltSigma2(const ByteSpan & rand, const Crypto::P256PublicKey & pubkey, const ByteSpan & ipk, MutableByteSpan & salt); CHIP_ERROR ConstructTBSData(const ByteSpan & senderNOC, const ByteSpan & senderICAC, const ByteSpan & senderPubKey, - const ByteSpan & receiverPubKey, uint8_t * tbsData, size_t & tbsDataLen); + const ByteSpan & receiverPubKey, MutableByteSpan & outTbsData); CHIP_ERROR ConstructSaltSigma3(const ByteSpan & ipk, MutableByteSpan & salt); CHIP_ERROR ConstructSigmaResumeKey(const ByteSpan & initiatorRandom, const ByteSpan & resumptionID, const ByteSpan & skInfo, diff --git a/src/python_testing/TC_AccessChecker.py b/src/python_testing/TC_AccessChecker.py index 5f3b3d69ec2146..03595d7ac1dfe5 100644 --- a/src/python_testing/TC_AccessChecker.py +++ b/src/python_testing/TC_AccessChecker.py @@ -28,7 +28,7 @@ from chip.testing.global_attribute_ids import GlobalAttributeIds from chip.testing.matter_testing import (AttributePathLocation, ClusterPathLocation, MatterBaseTest, TestStep, async_test_body, default_matter_test_main) -from chip.testing.spec_parsing import XmlCluster, build_xml_clusters +from chip.testing.spec_parsing import XmlCluster from chip.tlv import uint @@ -72,7 +72,8 @@ async def setup_class(self): self.user_params["use_pase_only"] = False super().setup_class() await self.setup_class_helper() - self.xml_clusters, self.problems = build_xml_clusters() + self.build_spec_xmls() + acl_attr = Clusters.AccessControl.Attributes.Acl self.default_acl = await self.read_single_attribute_check_success(cluster=Clusters.AccessControl, attribute=acl_attr) self._record_errors() diff --git a/src/python_testing/TC_DeviceConformance.py b/src/python_testing/TC_DeviceConformance.py index de6714abb30a1e..d2d22ecee79aca 100644 --- a/src/python_testing/TC_DeviceConformance.py +++ b/src/python_testing/TC_DeviceConformance.py @@ -47,16 +47,14 @@ device_type_id_type, is_valid_device_type_id) from chip.testing.matter_testing import (AttributePathLocation, ClusterPathLocation, CommandPathLocation, DeviceTypePathLocation, MatterBaseTest, ProblemNotice, ProblemSeverity, async_test_body, default_matter_test_main) -from chip.testing.spec_parsing import CommandType, build_xml_clusters, build_xml_device_types +from chip.testing.spec_parsing import CommandType from chip.tlv import uint class DeviceConformanceTests(BasicCompositionTests): async def setup_class_helper(self): await super().setup_class_helper() - self.xml_clusters, self.problems = build_xml_clusters() - self.xml_device_types, problems = build_xml_device_types() - self.problems.extend(problems) + self.build_spec_xmls() def _get_device_type_id(self, device_type_name: str) -> int: id = [id for id, dt in self.xml_device_types.items() if dt.name.lower() == device_type_name.lower()] diff --git a/src/python_testing/TC_pics_checker.py b/src/python_testing/TC_pics_checker.py index 07e8613cc1cc4d..510429dff0e36c 100644 --- a/src/python_testing/TC_pics_checker.py +++ b/src/python_testing/TC_pics_checker.py @@ -20,9 +20,9 @@ from chip.testing.basic_composition import BasicCompositionTests from chip.testing.global_attribute_ids import GlobalAttributeIds from chip.testing.matter_testing import (AttributePathLocation, ClusterPathLocation, CommandPathLocation, FeaturePathLocation, - MatterBaseTest, ProblemLocation, TestStep, async_test_body, default_matter_test_main) + MatterBaseTest, TestStep, UnknownProblemLocation, async_test_body, + default_matter_test_main) from chip.testing.pics import accepted_cmd_pics_str, attribute_pics_str, feature_pics_str, generated_cmd_pics_str -from chip.testing.spec_parsing import build_xml_clusters from mobly import asserts @@ -31,10 +31,7 @@ class TC_PICS_Checker(MatterBaseTest, BasicCompositionTests): async def setup_class(self): super().setup_class() await self.setup_class_helper(False) - # build_xml_cluster returns a list of issues found when paring the XML - # Problems in the XML shouldn't cause test failure, but we want them recorded - # so they are added to the list of problems that get output when the test set completes. - self.xml_clusters, self.problems = build_xml_clusters() + self.build_spec_xmls() def _check_and_record_errors(self, location, required, pics): if required and not self.check_pics(pics): @@ -178,7 +175,7 @@ def test_TC_IDM_10_4(self): self.step(7) if self.is_pics_sdk_ci_only: - self.record_error("PICS check", location=ProblemLocation(), + self.record_error("PICS check", location=UnknownProblemLocation(), problem="PICS PICS_SDK_CI_ONLY found in PICS list. This PICS is disallowed for certification.") self.success = False diff --git a/src/python_testing/TestConformanceTest.py b/src/python_testing/TestConformanceTest.py index c7b73495b67596..4c446ad73b83dd 100644 --- a/src/python_testing/TestConformanceTest.py +++ b/src/python_testing/TestConformanceTest.py @@ -22,7 +22,7 @@ from chip.testing.conformance import ConformanceDecision from chip.testing.global_attribute_ids import GlobalAttributeIds from chip.testing.matter_testing import MatterBaseTest, async_test_body, default_matter_test_main -from chip.testing.spec_parsing import build_xml_clusters, build_xml_device_types +from chip.testing.spec_parsing import PrebuiltDataModelDirectory, build_xml_clusters, build_xml_device_types from mobly import asserts from TC_DeviceConformance import DeviceConformanceTests @@ -118,8 +118,10 @@ def is_mandatory(conformance): class TestConformanceSupport(MatterBaseTest, DeviceConformanceTests): def setup_class(self): - self.xml_clusters, self.problems = build_xml_clusters() - self.xml_device_types, problems = build_xml_device_types() + # Latest fully qualified version + # TODO: It might be good to find a way to run this against each directory. + self.xml_clusters, self.problems = build_xml_clusters(PrebuiltDataModelDirectory.k1_4) + self.xml_device_types, problems = build_xml_device_types(PrebuiltDataModelDirectory.k1_4) self.problems.extend(problems) @async_test_body diff --git a/src/python_testing/TestSpecParsingDeviceType.py b/src/python_testing/TestSpecParsingDeviceType.py index 66a41b0fc5785b..baf7ac4d9c42e6 100644 --- a/src/python_testing/TestSpecParsingDeviceType.py +++ b/src/python_testing/TestSpecParsingDeviceType.py @@ -35,8 +35,9 @@ def test_spec_device_parsing(self): print(str(d)) def setup_class(self): - self.xml_clusters, self.xml_cluster_problems = build_xml_clusters() - self.xml_device_types, self.xml_device_types_problems = build_xml_device_types() + # Latest fully qualified release + self.xml_clusters, self.xml_cluster_problems = build_xml_clusters(PrebuiltDataModelDirectory.k1_4) + self.xml_device_types, self.xml_device_types_problems = build_xml_device_types(PrebuiltDataModelDirectory.k1_4) self.device_type_id = 0xBBEF self.revision = 2 diff --git a/src/python_testing/TestSpecParsingSelection.py b/src/python_testing/TestSpecParsingSelection.py new file mode 100644 index 00000000000000..51e76a8f59ab94 --- /dev/null +++ b/src/python_testing/TestSpecParsingSelection.py @@ -0,0 +1,149 @@ +# +# Copyright (c) 2025 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. +# +import chip.clusters as Clusters +from chip.testing.conformance import ConformanceDecision, ConformanceException +from chip.testing.global_attribute_ids import is_standard_attribute_id +from chip.testing.matter_testing import MatterBaseTest, default_matter_test_main +from chip.testing.spec_parsing import PrebuiltDataModelDirectory, build_xml_clusters, dm_from_spec_version +from chip.tlv import uint +from mobly import asserts, signals +from TC_DeviceConformance import DeviceConformanceTests + + +class TestSpecParsingSelection(MatterBaseTest, DeviceConformanceTests): + def setup_class(self): + # Overriding the DeviceConformanceTest setup_class so we don't go out to a real device + pass + + def test_dm_from_spec_version(self): + asserts.assert_equal(dm_from_spec_version(0x01030000), PrebuiltDataModelDirectory.k1_3, + "Incorrect directory selected for 1.3 with patch 0") + asserts.assert_equal(dm_from_spec_version(0x01030100), PrebuiltDataModelDirectory.k1_3, + "Incorrect directory selected for 1.3 with patch 1") + asserts.assert_equal(dm_from_spec_version(0x01040100), PrebuiltDataModelDirectory.k1_4_1, + "Incorrect directory selected for 1.4.1") + asserts.assert_equal(dm_from_spec_version(0x01040100), PrebuiltDataModelDirectory.k1_4_1, + "Incorrect directory selected for 1.4.1") + asserts.assert_equal(dm_from_spec_version(0x01050000), PrebuiltDataModelDirectory.kMaster, + "Incorrect directory selected for 1.5") + + # We don't have data model files for 1.2, so these should error + with asserts.assert_raises(ConformanceException, "Expected assertion was not raised for spec version 1.2"): + dm_from_spec_version(0x01020000) + + # Any dot release besides 0 and 1 for 1.4 should error + with asserts.assert_raises(ConformanceException, "Data model incorrectly identified for 1.4.2"): + dm_from_spec_version(0x01040200) + + with asserts.assert_raises(ConformanceException, "Data model incorrectly identified for 1.4.FF"): + dm_from_spec_version(0x0104FF00) + + # Any dot release besides 0 for 1.5 should error + with asserts.assert_raises(ConformanceException, "Data model incorrectly identified for 1.5.1"): + dm_from_spec_version(0x01050100) + with asserts.assert_raises(ConformanceException, "Data model incorrectly identified for 1.5.FF"): + dm_from_spec_version(0x0105FF00) + + # Any value with stuff in reserved should error + with asserts.assert_raises(ConformanceException, "Error not returned for specification revision with non-zero reserved values"): + dm_from_spec_version(0x01030001) + with asserts.assert_raises(ConformanceException, "Error not returned for specification revision with non-zero reserved values"): + dm_from_spec_version(0x01040001) + with asserts.assert_raises(ConformanceException, "Error not returned for specification revision with non-zero reserved values"): + dm_from_spec_version(0x01040101) + with asserts.assert_raises(ConformanceException, "Error not returned for specification revision with non-zero reserved values"): + dm_from_spec_version(0x01050001) + + def _create_device(self, spec_version: uint, tc_enabled: bool): + # Build at 1.4.1 so we can have TC info + xml_clusters, _ = build_xml_clusters(PrebuiltDataModelDirectory.k1_4_1) + + gc_feature_map = Clusters.GeneralCommissioning.Bitmaps.Feature.kTermsAndConditions if tc_enabled else 0 + + def create_cluster_globals(cluster, feature_map): + spec_attributes = xml_clusters[cluster.id].attributes + spec_accepted_commands = xml_clusters[cluster.id].accepted_commands + spec_generated_commands = xml_clusters[cluster.id].generated_commands + # Build just the lists - basic composition checks the wildcard against the lists, conformance just uses lists + attributes = [id for id, a in spec_attributes.items() if a.conformance( + feature_map, [], []).decision == ConformanceDecision.MANDATORY] + accepted_commands = [id for id, c in spec_accepted_commands.items() if c.conformance( + feature_map, [], []).decision == ConformanceDecision.MANDATORY] + generated_commands = [id for id, c in spec_generated_commands.items() if c.conformance( + feature_map, [], []).decision == ConformanceDecision.MANDATORY] + attr = cluster.Attributes + + resp = {} + non_global_attrs = [a for a in attributes if is_standard_attribute_id(a)] + for attribute_id in non_global_attrs: + # We don't use the values in these tests, set them all to 0. The types are wrong, but it shouldn't matter + resp[Clusters.ClusterObjects.ALL_ATTRIBUTES[cluster.id][attribute_id]] = 0 + + resp[attr.AttributeList] = attributes + resp[attr.AcceptedCommandList] = accepted_commands + resp[attr.GeneratedCommandList] = generated_commands + resp[attr.FeatureMap] = feature_map + resp[attr.ClusterRevision] = xml_clusters[cluster.id].revision + + return resp + + def get_tlv(resp): + # This only works because there are no structs in here. + # structs need special handling. Beware. + return {k.attribute_id: v for k, v in resp.items()} + + gc_resp = create_cluster_globals(Clusters.GeneralCommissioning, gc_feature_map) + bi_resp = create_cluster_globals(Clusters.BasicInformation, 0) + bi_resp[Clusters.BasicInformation.Attributes.SpecificationVersion] = spec_version + + self.endpoints = {0: {Clusters.GeneralCommissioning: gc_resp, Clusters.BasicInformation: bi_resp}} + self.endpoints_tlv = {0: {Clusters.GeneralCommissioning.id: get_tlv( + gc_resp), Clusters.BasicInformation.id: get_tlv(bi_resp)}} + + def _run_conformance_against_device(self, spec_version: uint, tc_enabled: bool, expect_success_conformance: bool, expect_success_revisions: bool): + self._create_device(spec_version, tc_enabled) + # build the spec XMLs for the stated version + self.build_spec_xmls() + success, problems = self.check_conformance(ignore_in_progress=False, is_ci=False, allow_provisional=False) + problem_strs = [str(p) for p in problems] + problem_str = "\n".join(problem_strs) + asserts.assert_equal(success, expect_success_conformance, + f"Improper conformance result for spec version {spec_version:08X}, TC: {tc_enabled} problems: {problem_str}") + + success, problems = self.check_revisions(ignore_in_progress=False) + asserts.assert_equal(success, expect_success_revisions, + f"Improper revision result for spec version {spec_version:08X}, TC: {tc_enabled} problems: {problems}") + + def test_conformance(self): + + # 1.4 is OK if TC is off + self._run_conformance_against_device(0x01040000, False, expect_success_conformance=True, expect_success_revisions=True) + # 1.4.1 is OK if TC is off + self._run_conformance_against_device(0x01040100, False, expect_success_conformance=True, expect_success_revisions=True) + # 1.4.1 is OK if TC is on + self._run_conformance_against_device(0x01040100, True, expect_success_conformance=True, expect_success_revisions=True) + # 1.4 is NOT OK if TC is on + self._run_conformance_against_device(0x01040000, True, expect_success_conformance=False, expect_success_revisions=True) + + # Check that we get a test failure on a bad spec revision + self._create_device(0xFFFFFFFF, False) + with asserts.assert_raises(signals.TestFailure, "Exception not properly raised for bad spec type"): + self.build_spec_xmls() + + +if __name__ == "__main__": + default_matter_test_main() diff --git a/src/python_testing/TestSpecParsingSupport.py b/src/python_testing/TestSpecParsingSupport.py index 35c1f88d121268..ce461f89682046 100644 --- a/src/python_testing/TestSpecParsingSupport.py +++ b/src/python_testing/TestSpecParsingSupport.py @@ -253,7 +253,8 @@ def get_access_enum_from_string(access_str: str) -> Clusters.AccessControl.Enums class TestSpecParsingSupport(MatterBaseTest): def setup_class(self): super().setup_class() - self.spec_xml_clusters, self.spec_problems = build_xml_clusters() + # Latest fully certified build + self.spec_xml_clusters, self.spec_problems = build_xml_clusters(PrebuiltDataModelDirectory.k1_4) self.all_spec_clusters = set([(id, c.name, c.pics) for id, c in self.spec_xml_clusters.items()]) def test_build_xml_override(self): diff --git a/src/python_testing/matter_testing_infrastructure/chip/testing/basic_composition.py b/src/python_testing/matter_testing_infrastructure/chip/testing/basic_composition.py index debf902e76a414..089cec929435a3 100644 --- a/src/python_testing/matter_testing_infrastructure/chip/testing/basic_composition.py +++ b/src/python_testing/matter_testing_infrastructure/chip/testing/basic_composition.py @@ -31,6 +31,8 @@ import chip.clusters.ClusterObjects import chip.tlv from chip.clusters.Attribute import ValueDecodeFailure +from chip.testing.conformance import ConformanceException +from chip.testing.spec_parsing import PrebuiltDataModelDirectory, build_xml_clusters, build_xml_device_types, dm_from_spec_version from mobly import asserts @@ -210,3 +212,23 @@ def fail_current_test(self, msg: Optional[str] = None): asserts.fail(msg=self.problems[-1].problem) else: asserts.fail(msg) + + def _get_dm(self) -> PrebuiltDataModelDirectory: + try: + spec_version = self.endpoints[0][Clusters.BasicInformation][Clusters.BasicInformation.Attributes.SpecificationVersion] + except KeyError: + asserts.fail( + "Specification Version not found on device - ensure device bas a basic information cluster on EP0 supporting Specification Version") + try: + return dm_from_spec_version(spec_version) + except ConformanceException as e: + asserts.fail(f"Unable to identify specification version: {e}") + + def build_spec_xmls(self): + dm = self._get_dm() + logging.info("----------------------------------------------------------------------------------") + logging.info(f"-- Running tests against Specification version {dm.dirname}") + logging.info("----------------------------------------------------------------------------------") + self.xml_clusters, self.problems = build_xml_clusters(dm) + self.xml_device_types, problems = build_xml_device_types(dm) + self.problems.extend(problems) diff --git a/src/python_testing/matter_testing_infrastructure/chip/testing/matter_testing.py b/src/python_testing/matter_testing_infrastructure/chip/testing/matter_testing.py index 49249cc7dabb0f..bac18a7d408414 100644 --- a/src/python_testing/matter_testing_infrastructure/chip/testing/matter_testing.py +++ b/src/python_testing/matter_testing_infrastructure/chip/testing/matter_testing.py @@ -803,7 +803,12 @@ def __str__(self): return msg -ProblemLocation = typing.Union[ClusterPathLocation, DeviceTypePathLocation] +class UnknownProblemLocation: + def __str__(self): + return '\n Unknown Locations - see message for more details' + + +ProblemLocation = typing.Union[ClusterPathLocation, DeviceTypePathLocation, UnknownProblemLocation] # ProblemSeverity is not using StrEnum, but rather Enum, since StrEnum only # appeared in 3.11. To make it JSON serializable easily, multiple inheritance diff --git a/src/python_testing/matter_testing_infrastructure/chip/testing/runner.py b/src/python_testing/matter_testing_infrastructure/chip/testing/runner.py index ea060074662b5c..8a773fe647da56 100644 --- a/src/python_testing/matter_testing_infrastructure/chip/testing/runner.py +++ b/src/python_testing/matter_testing_infrastructure/chip/testing/runner.py @@ -75,6 +75,7 @@ def run_test_with_mock_read(self, read_cache: Attribute.AsyncReadTransaction.Rea self.default_controller.Read = AsyncMock(return_value=read_cache) # This doesn't need to do anything since we are overriding the read anyway self.default_controller.FindOrEstablishPASESession = AsyncMock(return_value=None) + self.default_controller.GetConnectedDevice = AsyncMock(return_value=None) with asyncio.Runner() as runner: return run_tests_no_exit(self.test_class, self.config, runner.get_loop(), hooks, self.default_controller, self.stack) diff --git a/src/python_testing/matter_testing_infrastructure/chip/testing/spec_parsing.py b/src/python_testing/matter_testing_infrastructure/chip/testing/spec_parsing.py index e18df47855caef..a514c232344302 100644 --- a/src/python_testing/matter_testing_infrastructure/chip/testing/spec_parsing.py +++ b/src/python_testing/matter_testing_infrastructure/chip/testing/spec_parsing.py @@ -582,7 +582,7 @@ def get_data_model_directory(data_model_directory: Union[PrebuiltDataModelDirect return path.joinpath(data_model_level.dirname) -def build_xml_clusters(data_model_directory: Union[PrebuiltDataModelDirectory, Traversable] = PrebuiltDataModelDirectory.k1_4_1) -> typing.Tuple[dict[uint, XmlCluster], list[ProblemNotice]]: +def build_xml_clusters(data_model_directory: Union[PrebuiltDataModelDirectory, Traversable]) -> typing.Tuple[dict[uint, XmlCluster], list[ProblemNotice]]: """ Build XML clusters from the specified data model directory. This function supports both pre-built locations and full paths. @@ -851,7 +851,7 @@ def parse_single_device_type(root: ElementTree.Element) -> tuple[dict[int, XmlDe return device_types, problems -def build_xml_device_types(data_model_directory: typing.Union[PrebuiltDataModelDirectory, Traversable] = PrebuiltDataModelDirectory.k1_4_1) -> tuple[dict[int, XmlDeviceType], list[ProblemNotice]]: +def build_xml_device_types(data_model_directory: typing.Union[PrebuiltDataModelDirectory, Traversable]) -> tuple[dict[int, XmlDeviceType], list[ProblemNotice]]: top = get_data_model_directory(data_model_directory, DataModelLevel.kDeviceType) device_types: dict[int, XmlDeviceType] = {} problems: list[ProblemNotice] = [] @@ -881,3 +881,26 @@ def build_xml_device_types(data_model_directory: typing.Union[PrebuiltDataModelD device_types.pop(-1) return device_types, problems + + +def dm_from_spec_version(specification_version: uint) -> PrebuiltDataModelDirectory: + ''' Returns the data model directory for a given specification revision. + + input: specification revision attribute data from the basic information cluster + output: PrebuiltDataModelDirectory + raises: ConformanceException if the given specification_version does not conform to a known data model + ''' + # Specification version attribute is 2 bytes major, 2 bytes minor, 2 bytes dot 2 bytes reserved. + # However, 1.3 allowed the dot to be any value + if specification_version < 0x01040000: + specification_version &= 0xFFFF00FF + + version_to_dm = {0x01030000: PrebuiltDataModelDirectory.k1_3, + 0x01040000: PrebuiltDataModelDirectory.k1_4, + 0x01040100: PrebuiltDataModelDirectory.k1_4_1, + 0x01050000: PrebuiltDataModelDirectory.kMaster} + + if specification_version not in version_to_dm.keys(): + raise ConformanceException(f"Unknown specification_version {specification_version:08X}") + + return version_to_dm[specification_version] diff --git a/src/python_testing/test_metadata.yaml b/src/python_testing/test_metadata.yaml index 850a6ee756615d..5bd91ecf542152 100644 --- a/src/python_testing/test_metadata.yaml +++ b/src/python_testing/test_metadata.yaml @@ -55,6 +55,8 @@ not_automated: reason: Unit test - does not run against an app - name: TestConformanceTest.py reason: Unit test - does not run against an app + - name: TestSpecParsingSelection.py + reason: Unit test - does not run against an app - name: TestMatterTestingSupport.py reason: Unit test - does not run against an app - name: TestSpecParsingSupport.py diff --git a/src/python_testing/test_testing/example_pics_xml_basic_info.xml b/src/python_testing/test_testing/example_pics_xml_basic_info.xml index 3d488c3ae90ace..b1c3c5347bd648 100644 --- a/src/python_testing/test_testing/example_pics_xml_basic_info.xml +++ b/src/python_testing/test_testing/example_pics_xml_basic_info.xml @@ -199,6 +199,27 @@ Draft O false + + BINFO.S.A0015 + Does the DUT(server) support the SpecificationVersion attribute? + 9.2.1. Attributes - index.html[pdf] + M + true + + + BINFO.S.A0016 + Does the DUT(server) support the MaxPathsPerInvoke attribute? + 9.2.1. Attributes - index.html[pdf] + M + false + + + BINFO.S.A0017 + Does the DUT(server) support the DeviceLocation attribute? + 9.2.1. Attributes - index.html[pdf] + O + false + diff --git a/src/python_testing/test_testing/test_IDM_10_4.py b/src/python_testing/test_testing/test_IDM_10_4.py index f7e93e9f47ea33..a03be21c342fa6 100644 --- a/src/python_testing/test_testing/test_IDM_10_4.py +++ b/src/python_testing/test_testing/test_IDM_10_4.py @@ -49,6 +49,7 @@ def create_read(include_reachable: bool = False, include_max_paths: bool = False bi.ProductLabel: 'myProduct', bi.SerialNumber: 'ABCD1234', bi.LocalConfigDisabled: False, + bi.SpecificationVersion: 0x01040000, bi.UniqueID: 'Hashy-McHashface'} if include_reachable: attrs_bi[bi.Reachable] = True