Skip to content

Commit

Permalink
Post-merge comment fixes for project-chip#37207 (project-chip#37358)
Browse files Browse the repository at this point in the history
Post-merge review highlighted a few more updates. Performed them here.
  • Loading branch information
andy31415 authored Feb 4, 2025
1 parent e30cd1c commit 3700a5a
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 11 deletions.
15 changes: 9 additions & 6 deletions src/app/data-model-provider/Provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,17 @@ class Provider : public ProviderMetadataTree
/// - 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.
/// - `request.path` MUST refer to a command that actually exists. This is because in practice
/// callers must do ACL and flag checks (e.g. for timed invoke) before calling this function.
///
/// Callers that do not care about those checks should use `ProviderMetadataTree::AcceptedCommands`
/// to check for command existence.
///
/// - 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.
/// behavior (like error on invalid paths) whereas today this seems unclear as some
/// command intercepts do not validate that the command is in fact accepted on the
/// endpoint provided.
///
/// Return value expectations:
/// - if a response has been placed into `handler` then std::nullopt MUST be returned. In particular
Expand Down
10 changes: 5 additions & 5 deletions src/data-model-providers/codegen/CodegenDataModelProvider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include "data-model-providers/codegen/EmberMetadata.h"
#include <data-model-providers/codegen/CodegenDataModelProvider.h>

#include <access/AccessControl.h>
Expand All @@ -37,6 +36,7 @@
#include <app/util/endpoint-config-api.h>
#include <app/util/persistence/AttributePersistenceProvider.h>
#include <app/util/persistence/DefaultAttributePersistenceProvider.h>
#include <data-model-providers/codegen/EmberMetadata.h>
#include <lib/core/CHIPError.h>
#include <lib/core/DataModelTypes.h>
#include <lib/support/CodeUtils.h>
Expand Down Expand Up @@ -326,8 +326,8 @@ CHIP_ERROR CodegenDataModelProvider::AcceptedCommands(const ConcreteClusterPath
DataModel::ListBuilder<DataModel::AcceptedCommandEntry> & 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.
// the cluster actually exists on this endpoint before asking the CommandHandlerInterface what commands
// it claims to support.
const EmberAfCluster * serverCluster = FindServerCluster(path);
VerifyOrReturnError(serverCluster != nullptr, CHIP_ERROR_NOT_FOUND);

Expand Down Expand Up @@ -410,8 +410,8 @@ CHIP_ERROR CodegenDataModelProvider::GeneratedCommands(const ConcreteClusterPath
DataModel::ListBuilder<CommandId> & 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.
// the cluster actually exists on this endpoint before asking the CommandHandlerInterface what commands
// it claims to support.
const EmberAfCluster * serverCluster = FindServerCluster(path);
VerifyOrReturnError(serverCluster != nullptr, CHIP_ERROR_NOT_FOUND);

Expand Down

0 comments on commit 3700a5a

Please sign in to comment.