diff --git a/docs/platforms/esp32/config_options.md b/docs/platforms/esp32/config_options.md index 6dc94b814b8335..8055efb885c142 100644 --- a/docs/platforms/esp32/config_options.md +++ b/docs/platforms/esp32/config_options.md @@ -15,14 +15,12 @@ CONFIG_LWIP_IPV4=n ### Executable component not in "main" component The ESP-IDF framework allows renaming the main component, which can be useful if -you want to place the app_main() function in a different component. +you want to place the `app_main()` function in a different component. For required changes in the executable component, please refer to the [esp-idf documentation](https://docs.espressif.com/projects/esp-idf/en/stable/esp32/api-guides/build-system.html#renaming-main-component). -If you're building applications that support Matter and want to place app_main() -in a component other than main, use the following command: - -``` -idf.py -DEXECUTABLE_COMPONENT_NAME="your_component" build -``` +You need to list the required components in `idf_component_register()`. If this +module contains Matter related code, you may need to include +`chip, app_update, spi_flash, and nvs_flash` as `PRIV_REQUIRES`, along with any +other necessary dependencies. diff --git a/examples/chip-tool/commands/clusters/ModelCommand.h b/examples/chip-tool/commands/clusters/ModelCommand.h index f9ae83aed79f0a..d3e45101b5b830 100644 --- a/examples/chip-tool/commands/clusters/ModelCommand.h +++ b/examples/chip-tool/commands/clusters/ModelCommand.h @@ -28,8 +28,10 @@ class ModelCommand : public CHIPCommand { public: - ModelCommand(const char * commandName, CredentialIssuerCommands * credsIssuerConfig, bool supportsMultipleEndpoints = false) : - CHIPCommand(commandName, credsIssuerConfig), mOnDeviceConnectedCallback(OnDeviceConnectedFn, this), + ModelCommand(const char * commandName, CredentialIssuerCommands * credsIssuerConfig, bool supportsMultipleEndpoints = false, + const char * helpText = nullptr) : + CHIPCommand(commandName, credsIssuerConfig, helpText), + mOnDeviceConnectedCallback(OnDeviceConnectedFn, this), mOnDeviceConnectionFailureCallback(OnDeviceConnectionFailureFn, this), mSupportsMultipleEndpoints(supportsMultipleEndpoints) {} diff --git a/examples/chip-tool/commands/clusters/ReportCommand.h b/examples/chip-tool/commands/clusters/ReportCommand.h index e2c0f1db785900..f9d1d1e9ba33ac 100644 --- a/examples/chip-tool/commands/clusters/ReportCommand.h +++ b/examples/chip-tool/commands/clusters/ReportCommand.h @@ -26,8 +26,9 @@ class ReportCommand : public InteractionModelReports, public ModelCommand, public chip::app::ReadClient::Callback { public: - ReportCommand(const char * commandName, CredentialIssuerCommands * credsIssuerConfig) : - InteractionModelReports(this), ModelCommand(commandName, credsIssuerConfig, /* supportsMultipleEndpoints = */ true) + ReportCommand(const char * commandName, CredentialIssuerCommands * credsIssuerConfig, const char * helpText = nullptr) : + InteractionModelReports(this), + ModelCommand(commandName, credsIssuerConfig, /* supportsMultipleEndpoints = */ true, helpText) {} /////////// ReadClient Callback Interface ///////// @@ -133,8 +134,8 @@ class ReportCommand : public InteractionModelReports, public ModelCommand, publi class ReadCommand : public ReportCommand { protected: - ReadCommand(const char * commandName, CredentialIssuerCommands * credsIssuerConfig) : - ReportCommand(commandName, credsIssuerConfig) + ReadCommand(const char * commandName, CredentialIssuerCommands * credsIssuerConfig, const char * helpText = nullptr) : + ReportCommand(commandName, credsIssuerConfig, helpText) {} void OnDone(chip::app::ReadClient * aReadClient) override @@ -147,8 +148,8 @@ class ReadCommand : public ReportCommand class SubscribeCommand : public ReportCommand { protected: - SubscribeCommand(const char * commandName, CredentialIssuerCommands * credsIssuerConfig) : - ReportCommand(commandName, credsIssuerConfig) + SubscribeCommand(const char * commandName, CredentialIssuerCommands * credsIssuerConfig, const char * helpText = nullptr) : + ReportCommand(commandName, credsIssuerConfig, helpText) {} void OnSubscriptionEstablished(chip::SubscriptionId subscriptionId) override @@ -187,7 +188,8 @@ class SubscribeCommand : public ReportCommand class ReadAttribute : public ReadCommand { public: - ReadAttribute(CredentialIssuerCommands * credsIssuerConfig) : ReadCommand("read-by-id", credsIssuerConfig) + ReadAttribute(CredentialIssuerCommands * credsIssuerConfig) : + ReadCommand("read-by-id", credsIssuerConfig, "Read attributes for the given attribute path (which may include wildcards).") { AddArgument("cluster-ids", 0, UINT32_MAX, &mClusterIds, "Comma-separated list of cluster ids to read from (e.g. \"6\" or \"8,0x201\").\n Allowed to be 0xFFFFFFFF to " @@ -198,7 +200,8 @@ class ReadAttribute : public ReadCommand } ReadAttribute(chip::ClusterId clusterId, CredentialIssuerCommands * credsIssuerConfig) : - ReadCommand("read-by-id", credsIssuerConfig), mClusterIds(1, clusterId) + ReadCommand("read-by-id", credsIssuerConfig, "Read attributes from this cluster; allows wildcard endpoint and attribute."), + mClusterIds(1, clusterId) { AddAttributeIdArgument(); AddCommonArguments(); @@ -245,7 +248,9 @@ class ReadAttribute : public ReadCommand class SubscribeAttribute : public SubscribeCommand { public: - SubscribeAttribute(CredentialIssuerCommands * credsIssuerConfig) : SubscribeCommand("subscribe-by-id", credsIssuerConfig) + SubscribeAttribute(CredentialIssuerCommands * credsIssuerConfig) : + SubscribeCommand("subscribe-by-id", credsIssuerConfig, + "Subscribe to attributes for the given attribute path (which may include wildcards).") { AddArgument("cluster-ids", 0, UINT32_MAX, &mClusterIds, "Comma-separated list of cluster ids to subscribe to (e.g. \"6\" or \"8,0x201\").\n Allowed to be 0xFFFFFFFF " @@ -256,7 +261,9 @@ class SubscribeAttribute : public SubscribeCommand } SubscribeAttribute(chip::ClusterId clusterId, CredentialIssuerCommands * credsIssuerConfig) : - SubscribeCommand("subscribe-by-id", credsIssuerConfig), mClusterIds(1, clusterId) + SubscribeCommand("subscribe-by-id", credsIssuerConfig, + "Subscribe to attributes from this cluster; allows wildcard endpoint and attribute."), + mClusterIds(1, clusterId) { AddAttributeIdArgument(); AddCommonArguments(); @@ -312,7 +319,8 @@ class SubscribeAttribute : public SubscribeCommand class ReadEvent : public ReadCommand { public: - ReadEvent(CredentialIssuerCommands * credsIssuerConfig) : ReadCommand("read-event-by-id", credsIssuerConfig) + ReadEvent(CredentialIssuerCommands * credsIssuerConfig) : + ReadCommand("read-event-by-id", credsIssuerConfig, "Read events for the given event path (which may include wildcards).") { AddArgument("cluster-id", 0, UINT32_MAX, &mClusterIds); AddArgument("event-id", 0, UINT32_MAX, &mEventIds); @@ -322,7 +330,8 @@ class ReadEvent : public ReadCommand } ReadEvent(chip::ClusterId clusterId, CredentialIssuerCommands * credsIssuerConfig) : - ReadCommand("read-event-by-id", credsIssuerConfig), mClusterIds(1, clusterId) + ReadCommand("read-event-by-id", credsIssuerConfig, "Read events from this cluster; allows wildcard endpoint and event."), + mClusterIds(1, clusterId) { AddArgument("event-id", 0, UINT32_MAX, &mEventIds); AddArgument("fabric-filtered", 0, 1, &mFabricFiltered); @@ -356,7 +365,9 @@ class ReadEvent : public ReadCommand class SubscribeEvent : public SubscribeCommand { public: - SubscribeEvent(CredentialIssuerCommands * credsIssuerConfig) : SubscribeCommand("subscribe-event-by-id", credsIssuerConfig) + SubscribeEvent(CredentialIssuerCommands * credsIssuerConfig) : + SubscribeCommand("subscribe-event-by-id", credsIssuerConfig, + "Subscribe to events for the given event path (which may include wildcards).") { AddArgument("cluster-id", 0, UINT32_MAX, &mClusterIds); AddArgument("event-id", 0, UINT32_MAX, &mEventIds); @@ -365,7 +376,9 @@ class SubscribeEvent : public SubscribeCommand } SubscribeEvent(chip::ClusterId clusterId, CredentialIssuerCommands * credsIssuerConfig) : - SubscribeCommand("subscribe-event-by-id", credsIssuerConfig), mClusterIds(1, clusterId) + SubscribeCommand("subscribe-event-by-id", credsIssuerConfig, + "Subscribe to events from this cluster; allows wildcard endpoint and event."), + mClusterIds(1, clusterId) { AddArgument("event-id", 0, UINT32_MAX, &mEventIds); AddCommonArguments(); @@ -447,7 +460,8 @@ class ReadNone : public ReadCommand class ReadAll : public ReadCommand { public: - ReadAll(CredentialIssuerCommands * credsIssuerConfig) : ReadCommand("read-all", credsIssuerConfig) + ReadAll(CredentialIssuerCommands * credsIssuerConfig) : + ReadCommand("read-all", credsIssuerConfig, "Read attributes and events for the given paths (which may include wildcards).") { AddArgument("cluster-ids", 0, UINT32_MAX, &mClusterIds, "Comma-separated list of cluster ids to read from (e.g. \"6\" or \"8,0x201\").\n Allowed to be 0xFFFFFFFF to " @@ -513,7 +527,9 @@ class SubscribeNone : public SubscribeCommand class SubscribeAll : public SubscribeCommand { public: - SubscribeAll(CredentialIssuerCommands * credsIssuerConfig) : SubscribeCommand("subscribe-all", credsIssuerConfig) + SubscribeAll(CredentialIssuerCommands * credsIssuerConfig) : + SubscribeCommand("subscribe-all", credsIssuerConfig, + "Subscribe to attributes and events for the given paths (which may include wildcards).") { AddArgument("cluster-ids", 0, UINT32_MAX, &mClusterIds, "Comma-separated list of cluster ids to read from (e.g. \"6\" or \"8,0x201\").\n Allowed to be 0xFFFFFFFF to " diff --git a/integrations/docker/images/base/chip-build/version b/integrations/docker/images/base/chip-build/version index aded04f44ab3e9..2a9e09092c18c5 100644 --- a/integrations/docker/images/base/chip-build/version +++ b/integrations/docker/images/base/chip-build/version @@ -1 +1 @@ -105 : Upgrade android docker with new kotlin/gradle +106 : Upgrade android docker with java 17 and adjust the location for android cmdline tool diff --git a/integrations/docker/images/stage-2/chip-build-java/Dockerfile b/integrations/docker/images/stage-2/chip-build-java/Dockerfile index bda9cb3e9f3c0a..1619a9d12b8e93 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-11-jdk \ + openjdk-17-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_PATH=/usr/lib/jvm/java-11-openjdk-amd64 +ENV JAVA_PATH=/usr/lib/jvm/java-17-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 ea60b3db45074e..3f3d8d40f1c396 100644 --- a/integrations/docker/images/stage-3/chip-build-android/Dockerfile +++ b/integrations/docker/images/stage-3/chip-build-android/Dockerfile @@ -27,11 +27,16 @@ RUN set -x \ && : # 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 +# when running sdkmanager --licenses RUN set -x \ && wget -O /tmp/cmdline-tools.zip https://dl.google.com/android/repository/commandlinetools-linux-11076708_latest.zip \ && cd /opt/android/sdk \ - && unzip /tmp/cmdline-tools.zip \ - && rm -f /tmp/cmdline-tools.zip \ + && mkdir -p temp \ + && unzip /tmp/cmdline-tools.zip -d temp \ + && mkdir -p cmdline-tools/latest \ + && cp -rf temp/cmdline-tools/* cmdline-tools/latest \ + && rm -rf temp \ && test -d /opt/android/sdk/cmdline-tools \ && : # last line diff --git a/src/app/data-model-provider/Provider.h b/src/app/data-model-provider/Provider.h index 13d5a4a4f55e7c..530911b484fcc0 100644 --- a/src/app/data-model-provider/Provider.h +++ b/src/app/data-model-provider/Provider.h @@ -85,6 +85,16 @@ class Provider : public ProviderMetadataTree /// This includes cases where command handling and value return will be done asynchronously. /// - returning a value other than Success implies an error reply (error and data are mutually exclusive) /// + /// Preconditions: + /// - `request.path` MUST be valid: Invoke` is only guaranteed to function correctly for + /// VALID paths (i.e. use `ProviderMetadataTree::AcceptedCommands` to check). This is + /// because we assume ACL or flags (like timed invoke) have to happen before invoking + /// this command. + /// - TODO: as interfaces are updated, we may want to make the above requirement more + /// relaxed, as it seems desirable for users of this interface to have guaranteed + /// behavior (like error on invalid paths) where as today this seems unclear as some + /// command intercepts do not validate if the path is valid per endpoints. + /// /// Return value expectations: /// - if a response has been placed into `handler` then std::nullopt MUST be returned. In particular /// note that CHIP_NO_ERROR is NOT the same as std::nullopt: diff --git a/src/controller/java/AndroidLogDownloadFromNode.cpp b/src/controller/java/AndroidLogDownloadFromNode.cpp index 785567476fc125..f439cd64d881fe 100644 --- a/src/controller/java/AndroidLogDownloadFromNode.cpp +++ b/src/controller/java/AndroidLogDownloadFromNode.cpp @@ -152,7 +152,7 @@ void AndroidLogDownloadFromNode::OnResponseRetrieveLogs(void * context, using namespace chip::app::Clusters::DiagnosticLogs; if (data.status == StatusEnum::kSuccess) { - ChipLogProgress(Controller, "Success. Will receive log from BDX protocol.") + ChipLogProgress(Controller, "Success. Will receive log from BDX protocol."); } else if (data.status == StatusEnum::kExhausted) { @@ -210,8 +210,8 @@ void AndroidLogDownloadFromNode::FinishLogDownloadFromNode(void * context, CHIP_ JniLocalReferenceScope scope(env); jobject jCallback = self->mJavaCallback.ObjectRef(); - jint jFabricIndex = self->mController->GetFabricIndex(); - jlong jremoteNodeId = self->mRemoteNodeId; + jint jFabricIndex = static_cast(self->mController->GetFabricIndex()); + jlong jremoteNodeId = static_cast(self->mRemoteNodeId); VerifyOrExit(env != nullptr, ChipLogError(Controller, "Could not get JNIEnv for current thread")); @@ -274,7 +274,8 @@ void AndroidLogDownloadFromNode::OnTransferCallback(FabricIndex fabricIndex, Nod if (ret != JNI_TRUE) { - ChipLogError(Controller, "Transfer will be rejected.") * errInfoOnFailure = CHIP_ERROR_INTERNAL; + ChipLogError(Controller, "Transfer will be rejected."); + *errInfoOnFailure = CHIP_ERROR_INTERNAL; } } diff --git a/src/controller/java/CHIPP256KeypairBridge.cpp b/src/controller/java/CHIPP256KeypairBridge.cpp index a73eb417203c10..a42c5bc50e87a0 100644 --- a/src/controller/java/CHIPP256KeypairBridge.cpp +++ b/src/controller/java/CHIPP256KeypairBridge.cpp @@ -108,14 +108,14 @@ CHIP_ERROR CHIPP256KeypairBridge::ECDSA_sign_msg(const uint8_t * msg, size_t msg return CHIP_ERROR_INCORRECT_STATE; } - VerifyOrReturnError(CanCastTo(msg_length), CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(CanCastTo(msg_length), CHIP_ERROR_INVALID_ARGUMENT); CHIP_ERROR err = CHIP_NO_ERROR; jbyteArray jniMsg; jobject signedResult = nullptr; JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread(); VerifyOrReturnError(env != nullptr, err = CHIP_JNI_ERROR_NO_ENV); - err = JniReferences::GetInstance().N2J_ByteArray(env, msg, static_cast(msg_length), jniMsg); + err = JniReferences::GetInstance().N2J_ByteArray(env, msg, static_cast(msg_length), jniMsg); VerifyOrReturnError(err == CHIP_NO_ERROR, err); VerifyOrReturnError(jniMsg != nullptr, err); VerifyOrReturnError(mDelegate.HasValidObjectRef(), CHIP_ERROR_INCORRECT_STATE); diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.h b/src/darwin/Framework/CHIP/MTRDeviceController.h index 8177eda25cd5ba..41c97e805be95d 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.h +++ b/src/darwin/Framework/CHIP/MTRDeviceController.h @@ -309,6 +309,13 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1)) */ - (void)resume MTR_AVAILABLE(ios(18.2), macos(15.2), watchos(11.2), tvos(18.2)); +/** + * Returns the list of node IDs for which this controller has stored + * information. Returns empty list if the controller does not have any + * information stored. + */ +- (NSArray *)nodesWithStoredData MTR_AVAILABLE(ios(18.4), macos(15.4), watchos(11.4), tvos(18.4)); + /** * Shut down the controller. Calls to shutdown after the first one are NO-OPs. * This must be called, either directly or via shutting down the diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index 7bdbf2302740e9..93b02c9a693ea7 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -508,6 +508,12 @@ + (void)forceLocalhostAdvertisingOnly } #endif // DEBUG +- (NSArray *)nodesWithStoredData +{ + MTR_ABSTRACT_METHOD(); + return @[]; +} + #pragma mark - MTRDeviceControllerDelegate management // Note these are implemented in the base class so that XPC subclass can use it as well diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h b/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h index f4f38855420345..3feaa5a3e480af 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h @@ -105,6 +105,12 @@ typedef void (^MTRDeviceControllerDataStoreClusterDataHandler)(NSDictionary *)nodesWithStoredData; + @end NS_ASSUME_NONNULL_END diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.mm index 7ca30225b84f88..8a0057ca1ed9f2 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.mm @@ -19,6 +19,7 @@ // Importing MTRBaseDevice.h for the MTRAttributePath class. Needs to change when https://github.com/project-chip/connectedhomeip/issues/31247 is fixed. #import "MTRBaseDevice.h" #import "MTRLogging_Internal.h" +#import "MTRUnfairLock.h" #include #include @@ -111,6 +112,11 @@ @implementation MTRDeviceControllerDataStore { __weak MTRDeviceController * _controller; // Array of nodes with resumption info, oldest-stored first. NSMutableArray * _nodesWithResumptionInfo; + // Array of nodes with attribute info. + NSMutableArray * _nodesWithAttributeInfo; + // Lock protecting access to the _nodesWithAttributeInfo and + // _nodesWithResumptionInfo arrays. + os_unfair_lock _nodeArrayLock; } - (nullable instancetype)initWithController:(MTRDeviceController *)controller @@ -124,8 +130,10 @@ - (nullable instancetype)initWithController:(MTRDeviceController *)controller _controller = controller; _storageDelegate = storageDelegate; _storageDelegateQueue = storageDelegateQueue; + _nodeArrayLock = OS_UNFAIR_LOCK_INIT; __block id resumptionNodeList; + __block NSArray * nodesWithAttributeInfo; dispatch_sync(_storageDelegateQueue, ^{ @autoreleasepool { // NOTE: controller, not our weak ref, since we know it's still @@ -134,6 +142,8 @@ - (nullable instancetype)initWithController:(MTRDeviceController *)controller valueForKey:sResumptionNodeListKey securityLevel:MTRStorageSecurityLevelSecure sharingType:MTRStorageSharingTypeNotShared]; + + nodesWithAttributeInfo = [self _fetchNodeIndex]; } }); if (resumptionNodeList != nil) { @@ -152,6 +162,12 @@ - (nullable instancetype)initWithController:(MTRDeviceController *)controller _nodesWithResumptionInfo = [[NSMutableArray alloc] init]; } + if (nodesWithAttributeInfo != nil) { + _nodesWithAttributeInfo = [nodesWithAttributeInfo mutableCopy]; + } else { + _nodesWithAttributeInfo = [[NSMutableArray alloc] init]; + } + return self; } @@ -196,6 +212,8 @@ - (void)storeResumptionInfo:(MTRCASESessionResumptionInfo *)resumptionInfo removeValueForKey:ResumptionByResumptionIDKey(oldInfo.resumptionID) securityLevel:MTRStorageSecurityLevelSecure sharingType:MTRStorageSharingTypeNotShared]; + + std::lock_guard lock(self->_nodeArrayLock); [_nodesWithResumptionInfo removeObject:resumptionInfo.nodeID]; } @@ -211,9 +229,14 @@ - (void)storeResumptionInfo:(MTRCASESessionResumptionInfo *)resumptionInfo sharingType:MTRStorageSharingTypeNotShared]; // Update our resumption info node list. - [_nodesWithResumptionInfo addObject:resumptionInfo.nodeID]; + NSArray * valueToStore; + { + std::lock_guard lock(self->_nodeArrayLock); + [_nodesWithResumptionInfo addObject:resumptionInfo.nodeID]; + valueToStore = [_nodesWithResumptionInfo copy]; + } [_storageDelegate controller:controller - storeValue:[_nodesWithResumptionInfo copy] + storeValue:valueToStore forKey:sResumptionNodeListKey securityLevel:MTRStorageSecurityLevelSecure sharingType:MTRStorageSharingTypeNotShared]; @@ -226,7 +249,9 @@ - (void)clearAllResumptionInfo VerifyOrReturn(controller != nil); // No way to call delegate without controller. // Can we do less dispatch? We would need to have a version of - // _findResumptionInfoWithKey that assumes we are already on the right queue. + // _findResumptionInfoWithKey that assumes we are already on the right + // queue. + std::lock_guard lock(_nodeArrayLock); for (NSNumber * nodeID in _nodesWithResumptionInfo) { [self _clearResumptionInfoForNodeID:nodeID controller:controller]; } @@ -240,6 +265,8 @@ - (void)clearResumptionInfoForNodeID:(NSNumber *)nodeID VerifyOrReturn(controller != nil); // No way to call delegate without controller. [self _clearResumptionInfoForNodeID:nodeID controller:controller]; + + std::lock_guard lock(_nodeArrayLock); [_nodesWithResumptionInfo removeObject:nodeID]; } @@ -588,6 +615,20 @@ - (void)unitTestPruneEmptyStoredClusterDataBranches [self _pruneEmptyStoredClusterDataBranches]; }); } + +- (void)unitTestRereadNodeIndex +{ + dispatch_sync(_storageDelegateQueue, ^{ + auto * newIndex = [self _fetchNodeIndex]; + + std::lock_guard lock(self->_nodeArrayLock); + if (newIndex != nil) { + self->_nodesWithAttributeInfo = [newIndex mutableCopy]; + } else { + self->_nodesWithAttributeInfo = [[NSMutableArray alloc] init]; + } + }); +} #endif - (void)_pruneEmptyStoredClusterDataBranches @@ -596,9 +637,8 @@ - (void)_pruneEmptyStoredClusterDataBranches NSUInteger storeFailures = 0; - // Fetch node index - NSArray * nodeIndex = [self _fetchNodeIndex]; - NSMutableArray * nodeIndexCopy = [nodeIndex mutableCopy]; + std::lock_guard lock(self->_nodeArrayLock); + NSArray * nodeIndex = [_nodesWithAttributeInfo copy]; for (NSNumber * nodeID in nodeIndex) { // Fetch endpoint index @@ -638,7 +678,7 @@ - (void)_pruneEmptyStoredClusterDataBranches if (endpointIndexCopy.count) { success = [self _storeEndpointIndex:endpointIndexCopy forNodeID:nodeID]; } else { - [nodeIndexCopy removeObject:nodeID]; + [_nodesWithAttributeInfo removeObject:nodeID]; success = [self _deleteEndpointIndexForNodeID:nodeID]; } if (!success) { @@ -648,16 +688,16 @@ - (void)_pruneEmptyStoredClusterDataBranches } } - if (nodeIndex.count != nodeIndexCopy.count) { + if (nodeIndex.count != _nodesWithAttributeInfo.count) { BOOL success; - if (nodeIndexCopy.count) { - success = [self _storeNodeIndex:nodeIndexCopy]; + if (_nodesWithAttributeInfo.count) { + success = [self _storeNodeIndex:_nodesWithAttributeInfo]; } else { success = [self _deleteNodeIndex]; } if (!success) { storeFailures++; - MTR_LOG_ERROR("Store failed in _pruneEmptyStoredClusterDataBranches for nodeIndex (%lu)", static_cast(nodeIndexCopy.count)); + MTR_LOG_ERROR("Store failed in _pruneEmptyStoredClusterDataBranches for nodeIndex (%lu)", static_cast(_nodesWithAttributeInfo.count)); } } @@ -713,18 +753,19 @@ - (void)clearStoredClusterDataForNodeID:(NSNumber *)nodeID { dispatch_async(_storageDelegateQueue, ^{ [self _clearStoredClusterDataForNodeID:nodeID]; - NSArray * nodeIndex = [self _fetchNodeIndex]; - NSMutableArray * nodeIndexCopy = [nodeIndex mutableCopy]; - [nodeIndexCopy removeObject:nodeID]; - if (nodeIndex.count != nodeIndexCopy.count) { + + std::lock_guard lock(self->_nodeArrayLock); + auto oldCount = self->_nodesWithAttributeInfo.count; + [self->_nodesWithAttributeInfo removeObject:nodeID]; + if (self->_nodesWithAttributeInfo.count != oldCount) { BOOL success; - if (nodeIndexCopy.count) { - success = [self _storeNodeIndex:nodeIndexCopy]; + if (self->_nodesWithAttributeInfo.count) { + success = [self _storeNodeIndex:self->_nodesWithAttributeInfo]; } else { success = [self _deleteNodeIndex]; } if (!success) { - MTR_LOG_ERROR("Store failed in clearStoredAttributesForNodeID for nodeIndex (%lu)", static_cast(nodeIndexCopy.count)); + MTR_LOG_ERROR("Store failed in clearStoredAttributesForNodeID for nodeIndex (%lu)", static_cast(self->_nodesWithAttributeInfo.count)); } } }); @@ -804,12 +845,12 @@ - (void)clearAllStoredClusterData { dispatch_async(_storageDelegateQueue, ^{ // Fetch node index - NSArray * nodeIndex = [self _fetchNodeIndex]; - - for (NSNumber * nodeID in nodeIndex) { + std::lock_guard lock(self->_nodeArrayLock); + for (NSNumber * nodeID in self->_nodesWithAttributeInfo) { [self _clearStoredClusterDataForNodeID:nodeID]; } + [self->_nodesWithAttributeInfo removeAllObjects]; BOOL success = [self _deleteNodeIndex]; if (!success) { MTR_LOG_ERROR("Delete failed for nodeIndex"); @@ -826,14 +867,13 @@ - (void)clearAllStoredClusterData __block NSMutableDictionary * clusterDataToReturn = nil; dispatch_sync(_storageDelegateQueue, ^{ - // Fetch node index - NSArray * nodeIndex = [self _fetchNodeIndex]; + std::lock_guard lock(self->_nodeArrayLock); #if ATTRIBUTE_CACHE_VERBOSE_LOGGING - MTR_LOG("Fetch got %lu values for nodeIndex", static_cast(nodeIndex.count)); + MTR_LOG("Fetch got %lu values for nodeIndex", static_cast(self->_nodesWithAttributeInfo.count)); #endif - if (![nodeIndex containsObject:nodeID]) { + if (![self->_nodesWithAttributeInfo containsObject:nodeID]) { // Sanity check and delete if nodeID exists in index NSArray * endpointIndex = [self _fetchEndpointIndexForNodeID:nodeID]; if (endpointIndex) { @@ -1086,14 +1126,13 @@ - (void)storeClusterData:(NSDictionary } // Check if node index needs updating / creation - NSArray * nodeIndex = [self _fetchNodeIndex]; NSArray * nodeIndexToStore = nil; - if (!nodeIndex) { - // Ensure node index exists - MTR_LOG("No entry found for for nodeIndex - creating for node 0x%016llX", nodeID.unsignedLongLongValue); - nodeIndexToStore = [NSArray arrayWithObject:nodeID]; - } else if (![nodeIndex containsObject:nodeID]) { - nodeIndexToStore = [nodeIndex arrayByAddingObject:nodeID]; + { + std::lock_guard lock(self->_nodeArrayLock); + if (![self->_nodesWithAttributeInfo containsObject:nodeID]) { + [self->_nodesWithAttributeInfo addObject:nodeID]; + nodeIndexToStore = [self->_nodesWithAttributeInfo copy]; + } } if (nodeIndexToStore) { @@ -1206,6 +1245,27 @@ - (void)synchronouslyPerformBlock:(void (^_Nullable)(void))block }); } +- (NSArray *)nodesWithStoredData +{ + // We have three types of stored data: + // + // 1) Attribute data + // 2) Session resumption data + // 3) Device data. + // + // Items 1 and 2 come with node indices. Item 3 does not, but in practice + // we should have device data if and only if we have attribute data for that + // node ID, barring odd error conditions. + // + // TODO: Consider changing how we store device data so we can easily recover + // the relevant set of node IDs. + NSMutableSet * nodeSet = [NSMutableSet set]; + std::lock_guard lock(_nodeArrayLock); + [nodeSet addObjectsFromArray:_nodesWithResumptionInfo]; + [nodeSet addObjectsFromArray:_nodesWithAttributeInfo]; + return [nodeSet allObjects]; +} + @end @implementation MTRCASESessionResumptionInfo diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm index 4c427e7dd9fbcb..47e3f75884354c 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm @@ -1720,6 +1720,16 @@ - (nullable NSNumber *)neededReadPrivilegeForClusterID:(NSNumber *)clusterID att return nil; } +- (NSArray *)nodesWithStoredData +{ + if (!self.controllerDataStore) { + // We have nothing stored, if we have no way to store. + return @[]; + } + + return [self.controllerDataStore nodesWithStoredData]; +} + @end /** diff --git a/src/darwin/Framework/CHIP/MTRDeviceController_XPC.mm b/src/darwin/Framework/CHIP/MTRDeviceController_XPC.mm index 72c7151355021c..3be424b955b749 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController_XPC.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController_XPC.mm @@ -64,6 +64,11 @@ @implementation MTRDeviceController_XPC : (NSNumber *) nodeID, deleteNodeID : (NSNumber *) nodeID) +MTR_DEVICECONTROLLER_SIMPLE_REMOTE_XPC_GETTER(nodesWithStoredData, + NSArray *, + @[], // Default return value + getNodesWithStoredDataWithReply) + - (void)_updateRegistrationInfo { NSMutableDictionary * registrationInfo = [NSMutableDictionary dictionary]; diff --git a/src/darwin/Framework/CHIP/XPC Protocol/MTRXPCServerProtocol.h b/src/darwin/Framework/CHIP/XPC Protocol/MTRXPCServerProtocol.h index 1c57480565dacb..3e80bc4e67770f 100644 --- a/src/darwin/Framework/CHIP/XPC Protocol/MTRXPCServerProtocol.h +++ b/src/darwin/Framework/CHIP/XPC Protocol/MTRXPCServerProtocol.h @@ -77,6 +77,7 @@ MTR_AVAILABLE(ios(18.3), macos(15.3), watchos(11.3), tvos(18.3)) - (oneway void)deviceController:(NSUUID *)controller unregisterNodeID:(NSNumber *)nodeID; - (oneway void)deviceController:(NSUUID *)controller updateControllerConfiguration:(NSDictionary *)controllerState MTR_AVAILABLE(ios(18.3), macos(15.3), watchos(11.3), tvos(18.3)); +- (oneway void)deviceController:(NSUUID *)controller getNodesWithStoredDataWithReply:(void (^)(NSArray *))reply MTR_AVAILABLE(ios(18.4), macos(15.4), watchos(11.4), tvos(18.4)); @end MTR_AVAILABLE(ios(18.3), macos(15.3), watchos(11.3), tvos(18.3)) diff --git a/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m b/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m index 2abfda4fee4b8b..779b42abdfc0be 100644 --- a/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m +++ b/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m @@ -1521,6 +1521,8 @@ - (void)test008_TestDataStoreDirect dispatch_sync(_storageQueue, ^{ [storageDelegate controller:controller storeValues:testBulkValues securityLevel:MTRStorageSecurityLevelSecure sharingType:MTRStorageSharingTypeNotShared]; }); + // Since we messed with the node index, tell the data store to re-sync it's cache. + [controller.controllerDataStore unitTestRereadNodeIndex]; // Verify that the store resulted in the correct values NSDictionary * dataStoreClusterData = [controller.controllerDataStore getStoredClusterDataForNodeID:@(3001)]; XCTAssertEqualObjects(dataStoreClusterData, bulkTestClusterDataDictionary); @@ -1772,6 +1774,10 @@ - (void)test010_TestDataStoreMTRDeviceWithBulkReadWrite XCTAssertNotNil([dataStore findResumptionInfoByNodeID:deviceID]); XCTAssertNotNil([dataStore getStoredDeviceDataForNodeID:deviceID]); XCTAssertNotNil([dataStore getStoredClusterDataForNodeID:deviceID]); + __auto_type * nodesWithStoredData = [controller nodesWithStoredData]; + XCTAssertTrue([nodesWithStoredData containsObject:deviceID]); + XCTAssertEqualObjects(nodesWithStoredData, [dataStore nodesWithStoredData]); + XCTAssertEqualObjects(nodesWithStoredData, deviceAttributeCounts.allKeys); [controller forgetDeviceWithNodeID:deviceID]; deviceAttributeCounts = [controller unitTestGetDeviceAttributeCounts]; @@ -1779,6 +1785,9 @@ - (void)test010_TestDataStoreMTRDeviceWithBulkReadWrite XCTAssertNil([dataStore findResumptionInfoByNodeID:deviceID]); XCTAssertNil([dataStore getStoredDeviceDataForNodeID:deviceID]); XCTAssertNil([dataStore getStoredClusterDataForNodeID:deviceID]); + nodesWithStoredData = [controller nodesWithStoredData]; + XCTAssertFalse([nodesWithStoredData containsObject:deviceID]); + XCTAssertEqualObjects(nodesWithStoredData, [dataStore nodesWithStoredData]); [controller shutdown]; XCTAssertFalse([controller isRunning]); diff --git a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h index e2a9ae76df964e..3b6c3da0ea65ac 100644 --- a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h +++ b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h @@ -36,6 +36,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)clearStoredClusterDataForNodeID:(NSNumber *)nodeID; - (void)clearAllStoredClusterData; - (void)unitTestPruneEmptyStoredClusterDataBranches; +- (void)unitTestRereadNodeIndex; - (NSString *)_endpointIndexKeyForNodeID:(NSNumber *)nodeID; - (NSString *)_clusterIndexKeyForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID; - (NSString *)_clusterDataKeyForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID; @@ -43,6 +44,7 @@ NS_ASSUME_NONNULL_BEGIN - (nullable NSArray *)_fetchClusterIndexForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID; - (nullable MTRDeviceClusterData *)_fetchClusterDataForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID; - (nullable NSDictionary *)getStoredDeviceDataForNodeID:(NSNumber *)nodeID; +- (NSArray *)nodesWithStoredData; @end // Declare internal methods for testing diff --git a/src/data-model-providers/codegen/CodegenDataModelProvider.cpp b/src/data-model-providers/codegen/CodegenDataModelProvider.cpp index a3889fb1e25ebb..74fc5b769189a5 100644 --- a/src/data-model-providers/codegen/CodegenDataModelProvider.cpp +++ b/src/data-model-providers/codegen/CodegenDataModelProvider.cpp @@ -14,6 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +#include "data-model-providers/codegen/EmberMetadata.h" #include #include @@ -324,6 +325,12 @@ const EmberAfCluster * CodegenDataModelProvider::FindServerCluster(const Concret CHIP_ERROR CodegenDataModelProvider::AcceptedCommands(const ConcreteClusterPath & path, DataModel::ListBuilder & builder) { + // Some CommandHandlerInterface instances are registered of ALL endpoints, so make sure first that + // the cluster actually exists on this endpoint before asking the CommandHandlerInterface whether it + // supports the command. + const EmberAfCluster * serverCluster = FindServerCluster(path); + VerifyOrReturnError(serverCluster != nullptr, CHIP_ERROR_NOT_FOUND); + CommandHandlerInterface * interface = CommandHandlerInterfaceRegistry::Instance().GetCommandHandler(path.mEndpointId, path.mClusterId); if (interface != nullptr) @@ -377,8 +384,6 @@ CHIP_ERROR CodegenDataModelProvider::AcceptedCommands(const ConcreteClusterPath VerifyOrReturnError(err == CHIP_ERROR_NOT_IMPLEMENTED, err); } - const EmberAfCluster * serverCluster = FindServerCluster(path); - VerifyOrReturnError(serverCluster != nullptr, CHIP_ERROR_NOT_FOUND); VerifyOrReturnError(serverCluster->acceptedCommandList != nullptr, CHIP_NO_ERROR); const chip::CommandId * endOfList = serverCluster->acceptedCommandList; @@ -404,6 +409,12 @@ CHIP_ERROR CodegenDataModelProvider::AcceptedCommands(const ConcreteClusterPath CHIP_ERROR CodegenDataModelProvider::GeneratedCommands(const ConcreteClusterPath & path, DataModel::ListBuilder & builder) { + // Some CommandHandlerInterface instances are registered of ALL endpoints, so make sure first that + // the cluster actually exists on this endpoint before asking the CommandHandlerInterface whether it + // supports the command. + const EmberAfCluster * serverCluster = FindServerCluster(path); + VerifyOrReturnError(serverCluster != nullptr, CHIP_ERROR_NOT_FOUND); + CommandHandlerInterface * interface = CommandHandlerInterfaceRegistry::Instance().GetCommandHandler(path.mEndpointId, path.mClusterId); if (interface != nullptr) @@ -454,8 +465,6 @@ CHIP_ERROR CodegenDataModelProvider::GeneratedCommands(const ConcreteClusterPath VerifyOrReturnError(err == CHIP_ERROR_NOT_IMPLEMENTED, err); } - const EmberAfCluster * serverCluster = FindServerCluster(path); - VerifyOrReturnError(serverCluster != nullptr, CHIP_ERROR_NOT_FOUND); VerifyOrReturnError(serverCluster->generatedCommandList != nullptr, CHIP_NO_ERROR); const chip::CommandId * endOfList = serverCluster->generatedCommandList; diff --git a/src/data-model-providers/codegen/EmberMetadata.cpp b/src/data-model-providers/codegen/EmberMetadata.cpp index 57a9b48e561d41..3410cda64c081d 100644 --- a/src/data-model-providers/codegen/EmberMetadata.cpp +++ b/src/data-model-providers/codegen/EmberMetadata.cpp @@ -36,16 +36,11 @@ FindAttributeMetadata(const ConcreteAttributePath & aPath) if (metadata == nullptr) { - const EmberAfEndpointType * type = emberAfFindEndpointType(aPath.mEndpointId); - if (type == nullptr) - { - return Status::UnsupportedEndpoint; - } + Status status = ValidateClusterPath(aPath); - const EmberAfCluster * cluster = emberAfFindClusterInType(type, aPath.mClusterId, CLUSTER_MASK_SERVER); - if (cluster == nullptr) + if (status != Status::Success) { - return Status::UnsupportedCluster; + return status; } // Since we know the attribute is unsupported and the endpoint/cluster are @@ -56,6 +51,24 @@ FindAttributeMetadata(const ConcreteAttributePath & aPath) return metadata; } +Status ValidateClusterPath(const ConcreteClusterPath & path) +{ + + const EmberAfEndpointType * type = emberAfFindEndpointType(path.mEndpointId); + if (type == nullptr) + { + return Status::UnsupportedEndpoint; + } + + const EmberAfCluster * cluster = emberAfFindClusterInType(type, path.mClusterId, CLUSTER_MASK_SERVER); + if (cluster == nullptr) + { + return Status::UnsupportedCluster; + } + + return Status::Success; +} + } // namespace Ember } // namespace app } // namespace chip diff --git a/src/data-model-providers/codegen/EmberMetadata.h b/src/data-model-providers/codegen/EmberMetadata.h index 509cc1f82f14a7..14c8894290ac03 100644 --- a/src/data-model-providers/codegen/EmberMetadata.h +++ b/src/data-model-providers/codegen/EmberMetadata.h @@ -16,6 +16,7 @@ */ #pragma once +#include "app/ConcreteClusterPath.h" #include #include #include @@ -40,6 +41,14 @@ std::variant FindAttributeMetadata(const ConcreteAttributePath & aPath); +/// Returns the status for a given cluster existing in the ember metadata. +/// +/// Return code will be one of: +/// - Status::UnsupportedEndpoint if the path endpoint does not exist +/// - Status::UnsupportedCluster if the cluster does not exist on the given endpoint +/// - Status::Success if the cluster exists in the ember metadata. +Protocols::InteractionModel::Status ValidateClusterPath(const ConcreteClusterPath & path); + } // namespace Ember } // namespace app } // namespace chip diff --git a/src/data-model-providers/codegen/tests/TestCodegenModelViaMocks.cpp b/src/data-model-providers/codegen/tests/TestCodegenModelViaMocks.cpp index 407d561dc22c8d..4d54a194b1e0c5 100644 --- a/src/data-model-providers/codegen/tests/TestCodegenModelViaMocks.cpp +++ b/src/data-model-providers/codegen/tests/TestCodegenModelViaMocks.cpp @@ -218,6 +218,44 @@ class MockAccessControl : public Access::AccessControl::Delegate, public Access: bool IsDeviceTypeOnEndpoint(DeviceTypeId deviceType, EndpointId endpoint) override { return true; } }; +class MockCommandHandler : public CommandHandler +{ +public: + CHIP_ERROR FallibleAddStatus(const ConcreteCommandPath & aRequestCommandPath, + const Protocols::InteractionModel::ClusterStatusCode & aStatus, + const char * context = nullptr) override + { + // MOCK: do not do anything here + return CHIP_NO_ERROR; + } + + void AddStatus(const ConcreteCommandPath & aRequestCommandPath, const Protocols::InteractionModel::ClusterStatusCode & aStatus, + const char * context = nullptr) override + { + // MOCK: do not do anything here + } + + FabricIndex GetAccessingFabricIndex() const override { return 1; } + + CHIP_ERROR AddResponseData(const ConcreteCommandPath & aRequestCommandPath, CommandId aResponseCommandId, + const DataModel::EncodableToTLV & aEncodable) override + { + return CHIP_NO_ERROR; + } + + void AddResponse(const ConcreteCommandPath & aRequestCommandPath, CommandId aResponseCommandId, + const DataModel::EncodableToTLV & aEncodable) override + {} + + bool IsTimedInvoke() const override { return false; } + + void FlushAcksRightAwayOnSlowCommand() override {} + + Access::SubjectDescriptor GetSubjectDescriptor() const override { return kAdminSubjectDescriptor; } + + Messaging::ExchangeContext * GetExchangeContext() const override { return nullptr; } +}; + /// Overrides Enumerate*Commands in the CommandHandlerInterface to allow /// testing of behaviors when command enumeration is done in the interace. class CustomListCommandHandler : public CommandHandlerInterface @@ -229,7 +267,20 @@ class CustomListCommandHandler : public CommandHandlerInterface } ~CustomListCommandHandler() { CommandHandlerInterfaceRegistry::Instance().UnregisterCommandHandler(this); } - void InvokeCommand(HandlerContext & handlerContext) override { handlerContext.SetCommandNotHandled(); } + void InvokeCommand(HandlerContext & handlerContext) override + { + if (mHandleCommand) + { + handlerContext.mCommandHandler.AddStatus(handlerContext.mRequestPath, Protocols::InteractionModel::Status::Success); + handlerContext.SetCommandHandled(); + } + else + { + handlerContext.SetCommandNotHandled(); + } + } + + void SetHandleCommands(bool handle) { mHandleCommand = handle; } CHIP_ERROR EnumerateAcceptedCommands(const ConcreteClusterPath & cluster, CommandIdCallback callback, void * context) override { @@ -268,6 +319,7 @@ class CustomListCommandHandler : public CommandHandlerInterface private: bool mOverrideAccepted = false; bool mOverrideGenerated = false; + bool mHandleCommand = false; std::vector mAccepted; std::vector mGenerated; @@ -1156,6 +1208,37 @@ TEST_F(TestCodegenModelViaMocks, IterateOverGeneratedCommands) ASSERT_TRUE(cmds.data_equal(Span(expectedCommands3))); } +TEST_F(TestCodegenModelViaMocks, AcceptedGeneratedCommandsOnInvalidEndpoints) +{ + UseMockNodeConfig config(gTestNodeConfig); + CodegenDataModelProviderWithContext model; + + // register a CHI on ALL endpoints + CustomListCommandHandler handler(chip::NullOptional, MockClusterId(1)); + handler.SetHandleCommands(true); + + DataModel::ListBuilder generatedBuilder; + DataModel::ListBuilder acceptedBuilder; + + // valid endpoint will result in valid data (even though list is empty) + ASSERT_EQ(model.GeneratedCommands(ConcreteClusterPath(kMockEndpoint1, MockClusterId(1)), generatedBuilder), CHIP_NO_ERROR); + ASSERT_TRUE(generatedBuilder.IsEmpty()); + ASSERT_EQ(model.AcceptedCommands(ConcreteClusterPath(kMockEndpoint1, MockClusterId(1)), acceptedBuilder), CHIP_NO_ERROR); + ASSERT_TRUE(acceptedBuilder.IsEmpty()); + + // Invalid endpoint fails - we will get no commands there (even though CHI is registered) + ASSERT_EQ(model.GeneratedCommands(ConcreteClusterPath(kEndpointIdThatIsMissing, MockClusterId(1)), generatedBuilder), + CHIP_ERROR_NOT_FOUND); + ASSERT_EQ(model.AcceptedCommands(ConcreteClusterPath(kEndpointIdThatIsMissing, MockClusterId(1)), acceptedBuilder), + CHIP_ERROR_NOT_FOUND); + + // same for invalid cluster ID + ASSERT_EQ(model.GeneratedCommands(ConcreteClusterPath(kMockEndpoint1, MockClusterId(0x1123)), generatedBuilder), + CHIP_ERROR_NOT_FOUND); + ASSERT_EQ(model.AcceptedCommands(ConcreteClusterPath(kMockEndpoint1, MockClusterId(0x1123)), acceptedBuilder), + CHIP_ERROR_NOT_FOUND); +} + TEST_F(TestCodegenModelViaMocks, CommandHandlerInterfaceCommandHandling) { diff --git a/src/python_testing/TestInvokeReturnCodes.py b/src/python_testing/TestInvokeReturnCodes.py new file mode 100644 index 00000000000000..cd01f8882338c4 --- /dev/null +++ b/src/python_testing/TestInvokeReturnCodes.py @@ -0,0 +1,81 @@ +# +# 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. +# + +# === BEGIN CI TEST ARGUMENTS === +# test-runner-runs: +# run1: +# app: ${ALL_CLUSTERS_APP} +# app-args: > +# --discriminator 1234 +# --KVS kvs1 +# --trace-to json:${TRACE_APP}.json +# script-args: > +# --storage-path admin_storage.json +# --commissioning-method on-network +# --discriminator 1234 +# --passcode 20202021 +# --trace-to json:${TRACE_TEST_JSON}.json +# --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto +# factory-reset: true +# quiet: true +# === END CI TEST ARGUMENTS === + +import chip.clusters as Clusters +from chip.interaction_model import InteractionModelError, Status +from chip.testing.matter_testing import MatterBaseTest, async_test_body, default_matter_test_main +from mobly import asserts + + +class TestInvokeReturnCodes(MatterBaseTest): + """ + Validates that the invoke action correctly refuses commands + on invalid endpoints. + """ + + @async_test_body + async def test_invalid_endpoint_command(self): + self.print_step(0, "Commissioning - already done") + + self.print_step(1, "Find an invalid endpoint id") + root_parts = await self.read_single_attribute_check_success( + cluster=Clusters.Descriptor, + attribute=Clusters.Descriptor.Attributes.PartsList, + endpoint=0, + ) + endpoints = set(root_parts) + invalid_endpoint_id = 1 + while invalid_endpoint_id in endpoints: + invalid_endpoint_id += 1 + + self.print_step( + 2, + "Attempt to invoke SoftwareDiagnostics::ResetWatermarks on an invalid endpoint", + ) + try: + await self.send_single_cmd( + cmd=Clusters.SoftwareDiagnostics.Commands.ResetWatermarks(), + endpoint=invalid_endpoint_id, + ) + asserts.fail("Unexpected command success on an invalid endpoint") + except InteractionModelError as e: + asserts.assert_equal( + e.status, Status.UnsupportedEndpoint, "Unexpected error returned" + ) + + +if __name__ == "__main__": + default_matter_test_main()