Skip to content

Commit

Permalink
Remove the incorrect GroupKeySetIDs field from the KeySetReadAllIndic…
Browse files Browse the repository at this point in the history
…es 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 project-chip#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.
  • Loading branch information
bzbarsky-apple committed Jun 6, 2023
1 parent 4f6923f commit 1326922
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 7 deletions.
7 changes: 4 additions & 3 deletions src/app/tests/suites/TestGroupKeyManagementCluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ limitations under the License.

<command source="client" code="0x04" name="KeySetReadAllIndices" response="KeySetReadAllIndicesResponse" isFabricScoped="true" optional="false" cli="zcl GroupKeyManagement KeySetReadAllIndices">
<description>Return the list of Group Key Sets associated with the accessing fabric</description>
<arg name="GroupKeySetIDs" type="INT16U" array="true"/>
<access op="invoke" privilege="administer"/>
</command>

Expand Down
37 changes: 37 additions & 0 deletions src/darwin/Framework/CHIP/MTRBackwardsCompatShims.h
Original file line number Diff line number Diff line change
@@ -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 <Foundation/Foundation.h>

#import <Matter/MTRCommandPayloadsObjc.h>

/**
* 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
1 change: 1 addition & 0 deletions src/darwin/Framework/CHIP/Matter.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#import <Foundation/Foundation.h>

#import <Matter/MTRAsyncCallbackWorkQueue.h>
#import <Matter/MTRBackwardsCompatShims.h>
#import <Matter/MTRBaseClusters.h>
#import <Matter/MTRBaseDevice.h>
#import <Matter/MTRCSRInfo.h>
Expand Down
4 changes: 4 additions & 0 deletions src/darwin/Framework/CHIP/templates/MTRBaseClusters-src.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
13 changes: 12 additions & 1 deletion src/darwin/Framework/CHIP/templates/MTRBaseClusters.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions src/darwin/Framework/CHIP/templates/MTRClusters-src.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -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<NSDictionary<NSString *, id> *> *)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)
Expand Down
13 changes: 12 additions & 1 deletion src/darwin/Framework/CHIP/templates/MTRClusters.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -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<NSDictionary<NSString *, id> *> * _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<NSDictionary<NSString *, id> *> *)expectedValues expectedValueInterval:(NSNumber *)expectedValueIntervalMs completion:({{>command_completion_type command=.}})completion {{availability cluster command=command minimalRelease="First major API revamp"}};
- (void){{asLowerCamelCase name}}WithExpectedValues:(NSArray<NSDictionary<NSString *, id> *> *)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}}
Expand Down Expand Up @@ -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<NSDictionary<NSString *, id> *> * _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<NSDictionary<NSString *, id> *> *)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)
Expand Down
2 changes: 1 addition & 1 deletion src/darwin/Framework/CHIP/templates/availability.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7046,7 +7046,7 @@
PumpFeature:
LocalOperation: Local

- release: "TBD"
- release: "Fall 2023"
versions: "future"
introduced:
clusters:
Expand Down
4 changes: 4 additions & 0 deletions src/darwin/Framework/Matter.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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 */; };
Expand Down Expand Up @@ -461,6 +462,7 @@
51E51FBC282AD37A00FC978D /* MTRDeviceControllerStartupParams.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRDeviceControllerStartupParams.h; sourceTree = "<group>"; };
51E51FBD282AD37A00FC978D /* MTRDeviceControllerStartupParams_Internal.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRDeviceControllerStartupParams_Internal.h; sourceTree = "<group>"; };
51E51FBE282AD37A00FC978D /* MTRDeviceControllerStartupParams.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MTRDeviceControllerStartupParams.mm; sourceTree = "<group>"; };
51EF279E2A2A3EB100E33F75 /* MTRBackwardsCompatShims.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRBackwardsCompatShims.h; sourceTree = "<group>"; };
5A60370727EA1FF60020DB79 /* MTRClusterStateCacheContainer+XPC.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "MTRClusterStateCacheContainer+XPC.h"; sourceTree = "<group>"; };
5A6FEC8B27B5609C00F25F42 /* MTRDeviceOverXPC.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MTRDeviceOverXPC.h; sourceTree = "<group>"; };
5A6FEC8D27B5624E00F25F42 /* MTRDeviceControllerOverXPC.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MTRDeviceControllerOverXPC.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -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 */,
Expand Down

0 comments on commit 1326922

Please sign in to comment.