From 1326922db71461b39357b5db8b125aa7413b3f33 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 2 Jun 2023 10:27:15 -0400 Subject: [PATCH] Remove the incorrect GroupKeySetIDs field from the KeySetReadAllIndices command. This field does not exist in the spec. This is safe to do in terms of compat because the generated command parsing code only parses the fields that are actually present and does not error out if mandatory fields are missing; instead if uses type-default values for them. So commands updated clients send will still be parse-able by servers that had this field in their XML. Fixes part of https://github.com/project-chip/connectedhomeip/issues/25642 For the Darwin codegen, I just special-cased this command in the templates for now, but if we end up with more commands going from having required fields to not having them we can figure out a more automated solution. --- .../suites/TestGroupKeyManagementCluster.yaml | 7 ++-- .../chip/group-key-mgmt-cluster.xml | 1 - .../Framework/CHIP/MTRBackwardsCompatShims.h | 37 +++++++++++++++++++ src/darwin/Framework/CHIP/Matter.h | 1 + .../CHIP/templates/MTRBaseClusters-src.zapt | 4 ++ .../CHIP/templates/MTRBaseClusters.zapt | 13 ++++++- .../CHIP/templates/MTRClusters-src.zapt | 4 ++ .../Framework/CHIP/templates/MTRClusters.zapt | 13 ++++++- .../CHIP/templates/availability.yaml | 2 +- .../Matter.xcodeproj/project.pbxproj | 4 ++ 10 files changed, 79 insertions(+), 7 deletions(-) create mode 100644 src/darwin/Framework/CHIP/MTRBackwardsCompatShims.h diff --git a/src/app/tests/suites/TestGroupKeyManagementCluster.yaml b/src/app/tests/suites/TestGroupKeyManagementCluster.yaml index 4aedb61d7e28ee..bc1091050d828a 100644 --- a/src/app/tests/suites/TestGroupKeyManagementCluster.yaml +++ b/src/app/tests/suites/TestGroupKeyManagementCluster.yaml @@ -190,13 +190,14 @@ tests: EpochStartTime2: 1110002, } - - label: "KeySet Read All Indicers" - disabled: true + - label: "KeySet Read All Indices" command: "KeySetReadAllIndices" response: values: - name: "GroupKeySetIDs" - value: [0x01a1, 0x01a2] + constraints: + # Note: There's always an IPK keyset with index 0 + contains: [0x01a1, 0x01a2, 0] - label: "Write Group Keys (invalid)" command: "writeAttribute" diff --git a/src/app/zap-templates/zcl/data-model/chip/group-key-mgmt-cluster.xml b/src/app/zap-templates/zcl/data-model/chip/group-key-mgmt-cluster.xml index 3b4961ea816505..9c4fe41cd058d9 100644 --- a/src/app/zap-templates/zcl/data-model/chip/group-key-mgmt-cluster.xml +++ b/src/app/zap-templates/zcl/data-model/chip/group-key-mgmt-cluster.xml @@ -90,7 +90,6 @@ limitations under the License. Return the list of Group Key Sets associated with the accessing fabric - diff --git a/src/darwin/Framework/CHIP/MTRBackwardsCompatShims.h b/src/darwin/Framework/CHIP/MTRBackwardsCompatShims.h new file mode 100644 index 00000000000000..fa7ddd4f8f2088 --- /dev/null +++ b/src/darwin/Framework/CHIP/MTRBackwardsCompatShims.h @@ -0,0 +1,37 @@ +/** + * Copyright (c) 2023 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#import + +#import + +/** + * This file defines manual backwards-compat shims of various sorts to handle + * API changes that happened. + */ + +NS_ASSUME_NONNULL_BEGIN + +@interface MTRGroupKeyManagementClusterKeySetReadAllIndicesParams () +/** + * This command used to incorrectly have a groupKeySetIDs field. + */ +@property (nonatomic, copy) NSArray * groupKeySetIDs API_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1)) + MTR_NEWLY_DEPRECATED("This field has been removed"); + +@end + +NS_ASSUME_NONNULL_END diff --git a/src/darwin/Framework/CHIP/Matter.h b/src/darwin/Framework/CHIP/Matter.h index 094494dd20a1bc..a9da6744c08285 100644 --- a/src/darwin/Framework/CHIP/Matter.h +++ b/src/darwin/Framework/CHIP/Matter.h @@ -18,6 +18,7 @@ #import #import +#import #import #import #import diff --git a/src/darwin/Framework/CHIP/templates/MTRBaseClusters-src.zapt b/src/darwin/Framework/CHIP/templates/MTRBaseClusters-src.zapt index 6ff3a05793ee17..fda7cee26be83a 100644 --- a/src/darwin/Framework/CHIP/templates/MTRBaseClusters-src.zapt +++ b/src/darwin/Framework/CHIP/templates/MTRBaseClusters-src.zapt @@ -271,11 +271,15 @@ reportHandler:(void (^)({{asObjectiveCClass type parent.name}} * _Nullable value ]; } {{#unless hasArguments}} +{{! KeySetReadAllIndices grew this params-less API later _after_ it had already been shipped, so it needs to be special-cased here }} +{{#unless (and (isStrEqual cluster "GroupKeyManagement") + (isStrEqual command "KeySetReadAllIndices"))}} - (void){{asLowerCamelCase command}}WithCompletionHandler:({{>command_completion_type command=. compatRemapNames=true}})completionHandler { [self {{asLowerCamelCase command}}WithParams:nil completionHandler:completionHandler]; } {{/unless}} +{{/unless}} {{/if}} {{/inline}} {{> commandImpl cluster=(compatClusterNameRemapping parent.name) diff --git a/src/darwin/Framework/CHIP/templates/MTRBaseClusters.zapt b/src/darwin/Framework/CHIP/templates/MTRBaseClusters.zapt index 576f811105c94f..8432e20679bc3f 100644 --- a/src/darwin/Framework/CHIP/templates/MTRBaseClusters.zapt +++ b/src/darwin/Framework/CHIP/templates/MTRBaseClusters.zapt @@ -35,7 +35,14 @@ NS_ASSUME_NONNULL_BEGIN */ - (void){{asLowerCamelCase name}}WithParams:(MTR{{cluster}}Cluster{{command}}Params * {{#unless commandHasRequiredField }}_Nullable{{/unless}})params completion:({{>command_completion_type command=.}})completion {{availability cluster command=command minimalRelease="First major API revamp"}}; {{#unless hasArguments}} -- (void){{asLowerCamelCase name}}WithCompletion:({{>command_completion_type command=.}})completion {{availability cluster command=command minimalRelease="First major API revamp"}}; +- (void){{asLowerCamelCase name}}WithCompletion:({{>command_completion_type command=.}})completion +{{! KeySetReadAllIndices grew this params-less API later _after_ it had already been shipped, so it needs to be special-cased here }} +{{#if (and (isStrEqual command "KeySetReadAllIndices") + (isStrEqual cluster "GroupKeyManagement"))}} +{{availability cluster command=command minimalRelease="Fall 2023"}}; +{{else}} +{{availability cluster command=command minimalRelease="First major API revamp"}}; +{{/if}} {{/unless}} {{/if}} {{/inline}} @@ -197,9 +204,13 @@ typedef NS_OPTIONS({{asUnderlyingZclType name}}, {{objCEnumName clusterName bitm - (void){{asLowerCamelCase command}}WithParams:(MTR{{cluster}}Cluster{{command}}Params * {{#unless commandHasRequiredField }}_Nullable{{/unless}})params completionHandler:({{>command_completion_type command=. compatRemapNames=true}})completionHandler {{availability cluster command=command deprecatedRelease="First major API revamp" deprecationMessage=(concat "Please use " (asLowerCamelCase name) "WithParams:completion:")}}; {{#unless hasArguments}} +{{! KeySetReadAllIndices grew this params-less API later _after_ it had already been shipped, so it needs to be special-cased here }} +{{#unless (and (isStrEqual cluster "GroupKeyManagement") + (isStrEqual command "KeySetReadAllIndices"))}} - (void){{asLowerCamelCase command}}WithCompletionHandler:({{>command_completion_type command=. compatRemapNames=true}})completionHandler {{availability cluster command=command deprecatedRelease="First major API revamp" deprecationMessage=(concat "Please use " (asLowerCamelCase name) "WithCompletion:")}}; {{/unless}} +{{/unless}} {{/if}} {{/inline}} {{> commandDecl cluster=(compatClusterNameRemapping parent.name) diff --git a/src/darwin/Framework/CHIP/templates/MTRClusters-src.zapt b/src/darwin/Framework/CHIP/templates/MTRClusters-src.zapt index 11ba013f232f8b..169be4edc3b1c0 100644 --- a/src/darwin/Framework/CHIP/templates/MTRClusters-src.zapt +++ b/src/darwin/Framework/CHIP/templates/MTRClusters-src.zapt @@ -261,11 +261,15 @@ MTRCommandIDTypeCluster{{cluster}}Command{{command}}ID ]; } {{#unless hasArguments}} +{{! KeySetReadAllIndices grew this params-less API later _after_ it had already been shipped, so it needs to be special-cased here }} +{{#unless (and (isStrEqual cluster "GroupKeyManagement") + (isStrEqual command "KeySetReadAllIndices"))}} - (void){{asLowerCamelCase command}}WithExpectedValues:(NSArray *> *)expectedValues expectedValueInterval:(NSNumber *)expectedValueIntervalMs completionHandler:({{>command_completion_type command=. compatRemapNames=true}})completionHandler { [self {{asLowerCamelCase command}}WithParams:nil expectedValues:expectedValues expectedValueInterval:expectedValueIntervalMs completionHandler:completionHandler]; } {{/unless}} +{{/unless}} {{/if}} {{/inline}} {{> commandImpl cluster=(compatClusterNameRemapping parent.name) diff --git a/src/darwin/Framework/CHIP/templates/MTRClusters.zapt b/src/darwin/Framework/CHIP/templates/MTRClusters.zapt index ef09d89f9e165f..3285e3cba52cea 100644 --- a/src/darwin/Framework/CHIP/templates/MTRClusters.zapt +++ b/src/darwin/Framework/CHIP/templates/MTRClusters.zapt @@ -30,7 +30,14 @@ NS_ASSUME_NONNULL_BEGIN {{#if (isSupported cluster command=command)}} - (void){{asLowerCamelCase name}}WithParams:(MTR{{cluster}}Cluster{{command}}Params * {{#unless commandHasRequiredField}}_Nullable{{/unless}})params expectedValues:(NSArray *> * _Nullable)expectedDataValueDictionaries expectedValueInterval:(NSNumber * _Nullable)expectedValueIntervalMs completion:({{>command_completion_type command=.}})completion {{availability cluster command=command minimalRelease="First major API revamp"}}; {{#unless hasArguments}} -- (void){{asLowerCamelCase name}}WithExpectedValues:(NSArray *> *)expectedValues expectedValueInterval:(NSNumber *)expectedValueIntervalMs completion:({{>command_completion_type command=.}})completion {{availability cluster command=command minimalRelease="First major API revamp"}}; +- (void){{asLowerCamelCase name}}WithExpectedValues:(NSArray *> *)expectedValues expectedValueInterval:(NSNumber *)expectedValueIntervalMs completion:({{>command_completion_type command=.}})completion +{{! KeySetReadAllIndices grew this params-less API later _after_ it had already been shipped, so it needs to be special-cased here }} +{{#if (and (isStrEqual command "KeySetReadAllIndices") + (isStrEqual cluster "GroupKeyManagement"))}} +{{availability cluster command=command minimalRelease="Fall 2023"}}; +{{else}} +{{availability cluster command=command minimalRelease="First major API revamp"}}; +{{/if}} {{/unless}} {{/if}} {{/inline}} @@ -91,8 +98,12 @@ NS_ASSUME_NONNULL_BEGIN (isSupported cluster command=command))}} - (void){{asLowerCamelCase command}}WithParams:(MTR{{cluster}}Cluster{{command}}Params * {{#unless commandHasRequiredField}}_Nullable{{/unless}})params expectedValues:(NSArray *> * _Nullable)expectedDataValueDictionaries expectedValueInterval:(NSNumber * _Nullable)expectedValueIntervalMs completionHandler:({{>command_completion_type command=. compatRemapNames=true}})completionHandler {{availability cluster command=command deprecatedRelease="First major API revamp" deprecationMessage=(concat "Please use " (asLowerCamelCase name) "WithParams:expectedValues:expectedValueInterval:completion:")}}; {{#unless hasArguments}} +{{! KeySetReadAllIndices grew this params-less API later _after_ it had already been shipped, so it needs to be special-cased here }} +{{#unless (and (isStrEqual cluster "GroupKeyManagement") + (isStrEqual command "KeySetReadAllIndices"))}} - (void){{asLowerCamelCase command}}WithExpectedValues:(NSArray *> *)expectedValues expectedValueInterval:(NSNumber *)expectedValueIntervalMs completionHandler:({{>command_completion_type command=. compatRemapNames=true}})completionHandler {{availability cluster command=command deprecatedRelease="First major API revamp" deprecationMessage=(concat "Please use " (asLowerCamelCase name) "WithExpectedValues:expectedValueInterval:completion:")}}; {{/unless}} +{{/unless}} {{/if}} {{/inline}} {{> commandDecl cluster=(compatClusterNameRemapping parent.name) diff --git a/src/darwin/Framework/CHIP/templates/availability.yaml b/src/darwin/Framework/CHIP/templates/availability.yaml index 1d98350e945875..7775c92ef0552d 100644 --- a/src/darwin/Framework/CHIP/templates/availability.yaml +++ b/src/darwin/Framework/CHIP/templates/availability.yaml @@ -7046,7 +7046,7 @@ PumpFeature: LocalOperation: Local -- release: "TBD" +- release: "Fall 2023" versions: "future" introduced: clusters: diff --git a/src/darwin/Framework/Matter.xcodeproj/project.pbxproj b/src/darwin/Framework/Matter.xcodeproj/project.pbxproj index d93ddc7cc9b841..dd96371ef40a0d 100644 --- a/src/darwin/Framework/Matter.xcodeproj/project.pbxproj +++ b/src/darwin/Framework/Matter.xcodeproj/project.pbxproj @@ -170,6 +170,7 @@ 51E51FBF282AD37A00FC978D /* MTRDeviceControllerStartupParams.h in Headers */ = {isa = PBXBuildFile; fileRef = 51E51FBC282AD37A00FC978D /* MTRDeviceControllerStartupParams.h */; settings = {ATTRIBUTES = (Public, ); }; }; 51E51FC0282AD37A00FC978D /* MTRDeviceControllerStartupParams_Internal.h in Headers */ = {isa = PBXBuildFile; fileRef = 51E51FBD282AD37A00FC978D /* MTRDeviceControllerStartupParams_Internal.h */; }; 51E51FC1282AD37A00FC978D /* MTRDeviceControllerStartupParams.mm in Sources */ = {isa = PBXBuildFile; fileRef = 51E51FBE282AD37A00FC978D /* MTRDeviceControllerStartupParams.mm */; }; + 51EF279F2A2A3EB100E33F75 /* MTRBackwardsCompatShims.h in Headers */ = {isa = PBXBuildFile; fileRef = 51EF279E2A2A3EB100E33F75 /* MTRBackwardsCompatShims.h */; settings = {ATTRIBUTES = (Public, ); }; }; 5A60370827EA1FF60020DB79 /* MTRClusterStateCacheContainer+XPC.h in Headers */ = {isa = PBXBuildFile; fileRef = 5A60370727EA1FF60020DB79 /* MTRClusterStateCacheContainer+XPC.h */; }; 5A6FEC9027B563D900F25F42 /* MTRDeviceControllerOverXPC.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5A6FEC8F27B563D900F25F42 /* MTRDeviceControllerOverXPC.mm */; }; 5A6FEC9227B5669C00F25F42 /* MTRDeviceControllerOverXPC.h in Headers */ = {isa = PBXBuildFile; fileRef = 5A6FEC8D27B5624E00F25F42 /* MTRDeviceControllerOverXPC.h */; }; @@ -461,6 +462,7 @@ 51E51FBC282AD37A00FC978D /* MTRDeviceControllerStartupParams.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRDeviceControllerStartupParams.h; sourceTree = ""; }; 51E51FBD282AD37A00FC978D /* MTRDeviceControllerStartupParams_Internal.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRDeviceControllerStartupParams_Internal.h; sourceTree = ""; }; 51E51FBE282AD37A00FC978D /* MTRDeviceControllerStartupParams.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MTRDeviceControllerStartupParams.mm; sourceTree = ""; }; + 51EF279E2A2A3EB100E33F75 /* MTRBackwardsCompatShims.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRBackwardsCompatShims.h; sourceTree = ""; }; 5A60370727EA1FF60020DB79 /* MTRClusterStateCacheContainer+XPC.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "MTRClusterStateCacheContainer+XPC.h"; sourceTree = ""; }; 5A6FEC8B27B5609C00F25F42 /* MTRDeviceOverXPC.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MTRDeviceOverXPC.h; sourceTree = ""; }; 5A6FEC8D27B5624E00F25F42 /* MTRDeviceControllerOverXPC.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MTRDeviceControllerOverXPC.h; sourceTree = ""; }; @@ -937,6 +939,7 @@ 27A53C1627FBC6920053F131 /* MTRAttestationTrustStoreBridge.mm */, 513DDB852761F69300DAA01A /* MTRAttributeTLVValueDecoder_Internal.h */, 75B765C02A1D71BC0014719B /* MTRAttributeSpecifiedCheck.h */, + 51EF279E2A2A3EB100E33F75 /* MTRBackwardsCompatShims.h */, 51E4D120291D0EB400C8C535 /* MTRBaseClusterUtils.h */, 2C222ADE255C811800E446B9 /* MTRBaseDevice_Internal.h */, 2C222ACE255C620600E446B9 /* MTRBaseDevice.h */, @@ -1137,6 +1140,7 @@ isa = PBXHeadersBuildPhase; buildActionMask = 2147483647; files = ( + 51EF279F2A2A3EB100E33F75 /* MTRBackwardsCompatShims.h in Headers */, 5173A47729C0E2ED00F67F48 /* MTRFabricInfo.h in Headers */, 517BF3F0282B62B800A8B7DB /* MTRCertificates.h in Headers */, 51E51FBF282AD37A00FC978D /* MTRDeviceControllerStartupParams.h in Headers */,