Skip to content

Commit

Permalink
Restore Darwin APIs that went missing. (#31163)
Browse files Browse the repository at this point in the history
We were inconsistent about our checks for the "allowed to not have params" case:
some places used "command has no fields" as the check, and some used "command
has only optional fields".

This caused some APIs (for invoking commands with no params) to disappear when
commands that used to have no fields got optional fields added to them.

The fix:

1. Fixes the checks to consistently be "command has only optional fields" so the
   no-params APIs continue to exist for the commands that had optional fields
   added to them.
2. Fixes availability annotations for pre-existing commands with only optional
   fields so that we will not claim the no-params API is available before it was
   actually available, by basically listing all such commands in
   config-data.yaml.
3. Avoids generating deprecated overloads for the thins listed in item 2.
4. Moves the Darwin-only bits from the config-data.yaml in src/app into the
   Darwin version we are adding, and starts using that config file.
  • Loading branch information
bzbarsky-apple authored Dec 22, 2023
1 parent 52f66e1 commit ab58186
Show file tree
Hide file tree
Showing 11 changed files with 318 additions and 16 deletions.
7 changes: 0 additions & 7 deletions src/app/common/templates/config-data.yaml
Original file line number Diff line number Diff line change
@@ -1,10 +1,3 @@
DarwinForceWritable:
# Work-around for not allowing changes from writable to read-only
# happened in https://github.com/project-chip/connectedhomeip/pull/30134
- ApplicationLauncher::CurrentApp
- ContentLauncher::SupportedStreamingProtocols
- FanControl::FanModeSequence

EnumsNotUsedAsTypeInXML:
# List of enums that are not used as a type in XML. By adding an enum
# to this list you prevent incorrectly assuming from code that you are
Expand Down
8 changes: 6 additions & 2 deletions src/darwin/Framework/CHIP/templates/MTRBaseClusters-src.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ MTR{{compatClusterNameRemapping parent.name}}Cluster{{compatCommandNameRemapping
MTR{{cluster}}Cluster{{command}}Params
{{/unless}}
{{/inline}}
{{#unless hasArguments}}
{{#unless commandHasRequiredField}}
- (void){{asLowerCamelCase name}}WithCompletion:({{>command_completion_type command=.}})completion
{
[self {{asLowerCamelCase name}}WithParams:nil completion:completion];
Expand Down Expand Up @@ -246,7 +246,10 @@ reportHandler:(void (^)({{asObjectiveCClass type parent.name}} * _Nullable value
{{/if}}
];
}
{{#unless hasArguments}}
{{#unless commandHasRequiredField}}
{{#unless (isInConfigList
(concat (asUpperCamelCase cluster preserveAcronyms=true) "::" (asUpperCamelCase command preserveAcronyms=true))
"LegacyCommandsWithOnlyOptionalArguments")}}
{{! 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"))}}
Expand All @@ -256,6 +259,7 @@ reportHandler:(void (^)({{asObjectiveCClass type parent.name}} * _Nullable value
}
{{/unless}}
{{/unless}}
{{/unless}}
{{/if}}
{{/inline}}
{{> commandImpl cluster=(compatClusterNameRemapping parent.name)
Expand Down
15 changes: 13 additions & 2 deletions src/darwin/Framework/CHIP/templates/MTRBaseClusters.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,21 @@ NS_ASSUME_NONNULL_BEGIN
* {{description}}
*/
- (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}}
{{#unless commandHasRequiredField}}
- (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}}
{{#if (isInConfigList
(concat (asUpperCamelCase cluster preserveAcronyms=true) "::" (asUpperCamelCase command preserveAcronyms=true))
"LegacyCommandsWithOnlyOptionalArguments")}}
{{availability cluster command=command minimalRelease="Future"}}; {{! TODO: Use the right thing here when we know what it's called }}
{{else}}
{{availability cluster command=command minimalRelease="First major API revamp"}};
{{/if}}
{{/if}}
{{/unless}}
{{/if}}
{{/inline}}
Expand Down Expand Up @@ -217,14 +223,19 @@ typedef NS_OPTIONS({{asUnderlyingZclType name}}, {{objCEnumName clusterName bitm
(isSupported cluster command=command))}}
- (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}}
{{#unless commandHasRequiredField}}
{{! No need for these backwards-compat APIs for commands that never shipped them. }}
{{#unless (isInConfigList
(concat (asUpperCamelCase cluster preserveAcronyms=true) "::" (asUpperCamelCase command preserveAcronyms=true))
"LegacyCommandsWithOnlyOptionalArguments")}}
{{! 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}}
{{/unless}}
{{/if}}
{{/inline}}
{{> commandDecl cluster=(compatClusterNameRemapping parent.name)
Expand Down
8 changes: 6 additions & 2 deletions src/darwin/Framework/CHIP/templates/MTRClusters-src.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ MTR{{compatClusterNameRemapping parent.name}}Cluster{{compatCommandNameRemapping
MTR{{cluster}}Cluster{{command}}Params
{{/unless}}
{{/inline}}
{{#unless hasArguments}}
{{#unless commandHasRequiredField}}
- (void){{asLowerCamelCase name}}WithExpectedValues:(NSArray<NSDictionary<NSString *, id> *> *)expectedValues expectedValueInterval:(NSNumber *)expectedValueIntervalMs completion:({{>command_completion_type command=.}})completion
{
[self {{asLowerCamelCase name}}WithParams:nil expectedValues:expectedValues expectedValueInterval:expectedValueIntervalMs completion:completion];
Expand Down Expand Up @@ -171,7 +171,10 @@ MTR{{cluster}}Cluster{{command}}Params
{{/if}}
];
}
{{#unless hasArguments}}
{{#unless commandHasRequiredField}}
{{#unless (isInConfigList
(concat (asUpperCamelCase cluster preserveAcronyms=true) "::" (asUpperCamelCase command preserveAcronyms=true))
"LegacyCommandsWithOnlyOptionalArguments")}}
{{! 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"))}}
Expand All @@ -181,6 +184,7 @@ MTR{{cluster}}Cluster{{command}}Params
}
{{/unless}}
{{/unless}}
{{/unless}}
{{/if}}
{{/inline}}
{{> commandImpl cluster=(compatClusterNameRemapping parent.name)
Expand Down
15 changes: 13 additions & 2 deletions src/darwin/Framework/CHIP/templates/MTRClusters.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,21 @@ NS_ASSUME_NONNULL_BEGIN
{{#*inline "commandDecl"}}
{{#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}}
{{#unless commandHasRequiredField}}
- (void){{asLowerCamelCase name}}WithExpectedValues:(NSArray<NSDictionary<NSString *, id> *> * _Nullable)expectedValues expectedValueInterval:(NSNumber * _Nullable)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}}
{{#if (isInConfigList
(concat (asUpperCamelCase cluster preserveAcronyms=true) "::" (asUpperCamelCase command preserveAcronyms=true))
"LegacyCommandsWithOnlyOptionalArguments")}}
{{availability cluster command=command minimalRelease="Future"}}; {{! TODO: Use the right thing here when we know what it's called }}
{{else}}
{{availability cluster command=command minimalRelease="First major API revamp"}};
{{/if}}
{{/if}}
{{/unless}}
{{/if}}
{{/inline}}
Expand Down Expand Up @@ -113,13 +119,18 @@ NS_ASSUME_NONNULL_BEGIN
{{#if (and (wasIntroducedBeforeRelease "First major API revamp" cluster command=command)
(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}}
{{#unless commandHasRequiredField}}
{{! No need for these backwards-compat APIs for commands that never shipped them. }}
{{#unless (isInConfigList
(concat (asUpperCamelCase cluster preserveAcronyms=true) "::" (asUpperCamelCase command preserveAcronyms=true))
"LegacyCommandsWithOnlyOptionalArguments")}}
{{! 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> *> * _Nullable)expectedValues 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) "WithExpectedValues:expectedValueInterval:completion:")}};
{{/unless}}
{{/unless}}
{{/unless}}
{{/if}}
{{/inline}}
{{> commandDecl cluster=(compatClusterNameRemapping parent.name)
Expand Down
20 changes: 20 additions & 0 deletions src/darwin/Framework/CHIP/templates/config-data.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
DarwinForceWritable:
# Work-around for not allowing changes from writable to read-only
# happened in https://github.com/project-chip/connectedhomeip/pull/30134
- ApplicationLauncher::CurrentApp
- ContentLauncher::SupportedStreamingProtocols
- FanControl::FanModeSequence

# A list of commands that used to have only optional arguments before we started
# generating the no-params-needed variants of methods for that case. These
# declarations need to have availability based on when we started generating
# those variants, not the command's own availability.
LegacyCommandsWithOnlyOptionalArguments:
- NetworkCommissioning::ScanNetworks
- DoorLock::LockDoor
- DoorLock::UnlockDoor
- ApplicationLauncher::LaunchApp
- ApplicationLauncher::StopApp
- ApplicationLauncher::HideApp
- UnitTesting::TestNullableOptionalRequest
- UnitTesting::TestSimpleOptionalArgumentRequest
2 changes: 1 addition & 1 deletion src/darwin/Framework/CHIP/templates/templates.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
],
"resources": {
"availability-data": "availability.yaml",
"config-data": "../../../../app/common/templates/config-data.yaml"
"config-data": "config-data.yaml"
},
"override": "../../../../../src/app/zap-templates/common/override.js",
"partials": [
Expand Down
Loading

0 comments on commit ab58186

Please sign in to comment.