Skip to content

Commit 3700a5a

Browse files
authored
Post-merge comment fixes for project-chip#37207 (project-chip#37358)
Post-merge review highlighted a few more updates. Performed them here.
1 parent e30cd1c commit 3700a5a

File tree

2 files changed

+14
-11
lines changed

2 files changed

+14
-11
lines changed

src/app/data-model-provider/Provider.h

+9-6
Original file line numberDiff line numberDiff line change
@@ -86,14 +86,17 @@ class Provider : public ProviderMetadataTree
8686
/// - returning a value other than Success implies an error reply (error and data are mutually exclusive)
8787
///
8888
/// Preconditions:
89-
/// - `request.path` MUST be valid: Invoke` is only guaranteed to function correctly for
90-
/// VALID paths (i.e. use `ProviderMetadataTree::AcceptedCommands` to check). This is
91-
/// because we assume ACL or flags (like timed invoke) have to happen before invoking
92-
/// this command.
89+
/// - `request.path` MUST refer to a command that actually exists. This is because in practice
90+
/// callers must do ACL and flag checks (e.g. for timed invoke) before calling this function.
91+
///
92+
/// Callers that do not care about those checks should use `ProviderMetadataTree::AcceptedCommands`
93+
/// to check for command existence.
94+
///
9395
/// - TODO: as interfaces are updated, we may want to make the above requirement more
9496
/// relaxed, as it seems desirable for users of this interface to have guaranteed
95-
/// behavior (like error on invalid paths) where as today this seems unclear as some
96-
/// command intercepts do not validate if the path is valid per endpoints.
97+
/// behavior (like error on invalid paths) whereas today this seems unclear as some
98+
/// command intercepts do not validate that the command is in fact accepted on the
99+
/// endpoint provided.
97100
///
98101
/// Return value expectations:
99102
/// - if a response has been placed into `handler` then std::nullopt MUST be returned. In particular

src/data-model-providers/codegen/CodegenDataModelProvider.cpp

+5-5
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
* See the License for the specific language governing permissions and
1515
* limitations under the License.
1616
*/
17-
#include "data-model-providers/codegen/EmberMetadata.h"
1817
#include <data-model-providers/codegen/CodegenDataModelProvider.h>
1918

2019
#include <access/AccessControl.h>
@@ -37,6 +36,7 @@
3736
#include <app/util/endpoint-config-api.h>
3837
#include <app/util/persistence/AttributePersistenceProvider.h>
3938
#include <app/util/persistence/DefaultAttributePersistenceProvider.h>
39+
#include <data-model-providers/codegen/EmberMetadata.h>
4040
#include <lib/core/CHIPError.h>
4141
#include <lib/core/DataModelTypes.h>
4242
#include <lib/support/CodeUtils.h>
@@ -326,8 +326,8 @@ CHIP_ERROR CodegenDataModelProvider::AcceptedCommands(const ConcreteClusterPath
326326
DataModel::ListBuilder<DataModel::AcceptedCommandEntry> & builder)
327327
{
328328
// Some CommandHandlerInterface instances are registered of ALL endpoints, so make sure first that
329-
// the cluster actually exists on this endpoint before asking the CommandHandlerInterface whether it
330-
// supports the command.
329+
// the cluster actually exists on this endpoint before asking the CommandHandlerInterface what commands
330+
// it claims to support.
331331
const EmberAfCluster * serverCluster = FindServerCluster(path);
332332
VerifyOrReturnError(serverCluster != nullptr, CHIP_ERROR_NOT_FOUND);
333333

@@ -410,8 +410,8 @@ CHIP_ERROR CodegenDataModelProvider::GeneratedCommands(const ConcreteClusterPath
410410
DataModel::ListBuilder<CommandId> & builder)
411411
{
412412
// Some CommandHandlerInterface instances are registered of ALL endpoints, so make sure first that
413-
// the cluster actually exists on this endpoint before asking the CommandHandlerInterface whether it
414-
// supports the command.
413+
// the cluster actually exists on this endpoint before asking the CommandHandlerInterface what commands
414+
// it claims to support.
415415
const EmberAfCluster * serverCluster = FindServerCluster(path);
416416
VerifyOrReturnError(serverCluster != nullptr, CHIP_ERROR_NOT_FOUND);
417417

0 commit comments

Comments
 (0)