Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an Invoke implementation in CodegenDataModel #34220

Merged
merged 3 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions src/app/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ static_library("interaction-model") {

public_deps = [
":app_config",
":command-handler",
":command-handler-impl",
":constants",
":paths",
":subscription-info-provider",
Expand Down Expand Up @@ -333,16 +333,32 @@ source_set("status-response") {
]
}

source_set("command-handler") {
source_set("command-handler-interface") {
sources = [
"CommandHandler.cpp",
"CommandHandler.h",
"CommandHandlerExchangeInterface.h",
]

public_deps = [
":paths",
"${chip_root}/src/access:types",
"${chip_root}/src/app/data-model",
"${chip_root}/src/lib/core",
"${chip_root}/src/lib/support",
"${chip_root}/src/messaging",
"${chip_root}/src/protocols/interaction_model",
]
}

source_set("command-handler-impl") {
sources = [
"CommandHandlerImpl.cpp",
"CommandHandlerImpl.h",
]

public_deps = [
":command-handler-interface",
":paths",
":required-privileges",
":status-response",
Expand Down
16 changes: 13 additions & 3 deletions src/app/codegen-data-model/CodegenDataModel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#include <app-common/zap-generated/attribute-type.h>
#include <app/RequiredPrivilege.h>
#include <app/util/IMClusterCommandHandler.h>
#include <app/util/attribute-storage.h>
#include <app/util/endpoint-config-api.h>
#include <lib/core/DataModelTypes.h>
Expand Down Expand Up @@ -232,10 +233,19 @@ bool CodegenDataModel::EmberCommandListIterator::Exists(const CommandId * list,
}

CHIP_ERROR CodegenDataModel::Invoke(const InteractionModel::InvokeRequest & request, TLV::TLVReader & input_arguments,
InteractionModel::InvokeReply & reply)
CommandHandler * handler)
{
// TODO: this needs an implementation
return CHIP_ERROR_NOT_IMPLEMENTED;
// TODO: CommandHandlerInterface support is currently
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
// residing in InteractionModelEngine itself. We may want to separate this out
// into its own registry, similar to attributes, so that IM is decoupled from actual storage of things.
//
// Open issue at https://github.com/project-chip/connectedhomeip/issues/34258

// Ember dispatching automatically uses `handler` to set an appropriate result or status
// This never fails (as handler error is encoded as needed).
DispatchSingleClusterCommand(request.path, input_arguments, handler);

return CHIP_NO_ERROR;
}

EndpointId CodegenDataModel::FirstEndpoint()
Expand Down
2 changes: 1 addition & 1 deletion src/app/codegen-data-model/CodegenDataModel.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class CodegenDataModel : public chip::app::InteractionModel::DataModel
CHIP_ERROR ReadAttribute(const InteractionModel::ReadAttributeRequest & request, AttributeValueEncoder & encoder) override;
CHIP_ERROR WriteAttribute(const InteractionModel::WriteAttributeRequest & request, AttributeValueDecoder & decoder) override;
CHIP_ERROR Invoke(const InteractionModel::InvokeRequest & request, chip::TLV::TLVReader & input_arguments,
InteractionModel::InvokeReply & reply) override;
CommandHandler * handler) override;

/// attribute tree iteration
EndpointId FirstEndpoint() override;
Expand Down
2 changes: 2 additions & 0 deletions src/app/codegen-data-model/tests/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ source_set("ember_extra_files") {
"${chip_root}/src/app/util/ember-io-storage.cpp",
"AttributeReportIBEncodeDecode.cpp",
"AttributeReportIBEncodeDecode.h",
"EmberInvokeOverride.cpp",
"EmberInvokeOverride.h",
"EmberReadWriteOverride.cpp",
"EmberReadWriteOverride.h",
"InteractionModelTemporaryOverrides.cpp",
Expand Down
55 changes: 55 additions & 0 deletions src/app/codegen-data-model/tests/EmberInvokeOverride.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright (c) 2024 Project CHIP Authors
* All rights reserved.
*
* 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.
*/
#include "EmberInvokeOverride.h"

#include <app/util/IMClusterCommandHandler.h>

namespace {

chip::app::ConcreteCommandPath gLastDispatchPath;
uint32_t gDispatchCount = 0;

} // namespace

namespace chip {
namespace Test {

app::ConcreteCommandPath GetLastDispatchPath()
{
return gLastDispatchPath;
}

uint32_t DispatchCount()
{
return gDispatchCount;
}

} // namespace Test
} // namespace chip

namespace chip {
namespace app {

void DispatchSingleClusterCommand(const ConcreteCommandPath & aRequestCommandPath, chip::TLV::TLVReader & aReader,
CommandHandler * apCommandObj)
{
gLastDispatchPath = aRequestCommandPath;
gDispatchCount++;
}

} // namespace app
} // namespace chip
31 changes: 31 additions & 0 deletions src/app/codegen-data-model/tests/EmberInvokeOverride.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright (c) 2024 Project CHIP Authors
* All rights reserved.
*
* 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.
*/
#pragma once

#include <app/ConcreteCommandPath.h>

namespace chip {
namespace Test {

/// what was the last path on which DispatchSingleClusterCommand was called
app::ConcreteCommandPath GetLastDispatchPath();

/// How many times was DispatchSingleClusterCommand called
uint32_t DispatchCount();

} // namespace Test
} // namespace chip
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,6 @@ CHIP_ERROR ReadSingleClusterData(const Access::SubjectDescriptor & aSubjectDescr
return CHIP_ERROR_NOT_IMPLEMENTED;
}

void DispatchSingleClusterCommand(const ConcreteCommandPath & aRequestCommandPath, chip::TLV::TLVReader & aReader,
CommandHandler * apCommandObj)
{
// TODO: total hardcoded noop
}

} // namespace app
} // namespace chip

Expand Down
46 changes: 46 additions & 0 deletions src/app/codegen-data-model/tests/TestCodegenModelViaMocks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include "app/ConcreteCommandPath.h"
#include <app/codegen-data-model/CodegenDataModel.h>

#include <app/codegen-data-model/tests/AttributeReportIBEncodeDecode.h>
#include <app/codegen-data-model/tests/EmberInvokeOverride.h>
#include <app/codegen-data-model/tests/EmberReadWriteOverride.h>

#include <access/AccessControl.h>
Expand Down Expand Up @@ -69,6 +71,9 @@ constexpr NodeId kTestNodeId = 0xFFFF'1234'ABCD'4321;
constexpr AttributeId kAttributeIdReadOnly = 0x3001;
constexpr AttributeId kAttributeIdTimedWrite = 0x3002;

constexpr CommandId kMockCommandId1 = 0x1234;
constexpr CommandId kMockCommandId2 = 0x1122;

constexpr EndpointId kEndpointIdThatIsMissing = kMockEndpointMin - 1;

constexpr AttributeId kReadOnlyAttributeId = 0x5001;
Expand Down Expand Up @@ -2430,6 +2435,47 @@ TEST(TestCodegenModelViaMocks, EmberWriteAttributeAccessInterfaceTest)
TestEmberScalarNullWrite<int64_t, ZCL_INT64S_ATTRIBUTE_TYPE>();
}

TEST(TestCodegenModelViaMocks, EmberInvokeTest)
{
// Ember invoke is fully code-generated - there is a single function for Dispatch
// that will do a `switch` on the path elements and invoke a corresponding `emberAf*`
// callback.
//
// The only thing that can be validated is that this `DispatchSingleClusterCommand`
// is actually invoked.

UseMockNodeConfig config(gTestNodeConfig);
chip::app::CodegenDataModel model;

{
const ConcreteCommandPath kCommandPath(kMockEndpoint1, MockClusterId(1), kMockCommandId1);
const InvokeRequest kInvokeRequest{ .path = kCommandPath };
chip::TLV::TLVReader tlvReader;

const uint32_t kDispatchCountPre = chip::Test::DispatchCount();

// Using a handler set to nullpotr as it is not used by the impl
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
ASSERT_EQ(model.Invoke(kInvokeRequest, tlvReader, /* handler = */ nullptr), CHIP_NO_ERROR);

EXPECT_EQ(chip::Test::DispatchCount(), kDispatchCountPre + 1); // single dispatch
EXPECT_EQ(chip::Test::GetLastDispatchPath(), kCommandPath); // for the right path
}

{
const ConcreteCommandPath kCommandPath(kMockEndpoint1, MockClusterId(1), kMockCommandId2);
const InvokeRequest kInvokeRequest{ .path = kCommandPath };
chip::TLV::TLVReader tlvReader;

const uint32_t kDispatchCountPre = chip::Test::DispatchCount();

// Using a handler set to nullpotr as it is not used by the impl
ASSERT_EQ(model.Invoke(kInvokeRequest, tlvReader, /* handler = */ nullptr), CHIP_NO_ERROR);

EXPECT_EQ(chip::Test::DispatchCount(), kDispatchCountPre + 1); // single dispatch
EXPECT_EQ(chip::Test::GetLastDispatchPath(), kCommandPath); // for the right path
}
}

TEST(TestCodegenModelViaMocks, EmberWriteAttributeAccessInterfaceReturningError)
{
UseMockNodeConfig config(gTestNodeConfig);
Expand Down
2 changes: 1 addition & 1 deletion src/app/data-model-interface/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ source_set("data-model-interface") {
"DataModel.h",
"DataModelChangeListener.h",
"EventsGenerator.h",
"InvokeResponder.h",
"MetadataTypes.cpp",
"MetadataTypes.h",
"OperationTypes.h",
Expand All @@ -29,6 +28,7 @@ source_set("data-model-interface") {
public_deps = [
"${chip_root}/src/access:types",
"${chip_root}/src/app:attribute-access",
"${chip_root}/src/app:command-handler-interface",
"${chip_root}/src/app:events",
"${chip_root}/src/app:paths",
"${chip_root}/src/app/MessageDef",
Expand Down
22 changes: 3 additions & 19 deletions src/app/data-model-interface/DataModel.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@

#include <app/AttributeValueDecoder.h>
#include <app/AttributeValueEncoder.h>
#include <app/CommandHandler.h>

#include <app/data-model-interface/Context.h>
#include <app/data-model-interface/InvokeResponder.h>
#include <app/data-model-interface/MetadataTypes.h>
#include <app/data-model-interface/OperationTypes.h>

Expand Down Expand Up @@ -99,25 +99,9 @@ class DataModel : public DataModelMetadataTree
/// - `NeedsTimedInteraction` for writes that are not timed however are required to be so
virtual CHIP_ERROR WriteAttribute(const WriteAttributeRequest & request, AttributeValueDecoder & decoder) = 0;

/// `reply` is used to send back the reply.
/// - calling Reply() or ReplyAsync() will let the application control the reply
/// - returning a CHIP_NO_ERROR without reply/reply_async implies a Status::Success reply without data
/// `handler` is used to send back the reply.
/// - returning a value other than CHIP_NO_ERROR implies an error reply (error and data are mutually exclusive)
///
/// See InvokeReply/AutoCompleteInvokeResponder for details on how to send back replies and expected
/// error handling. If you need to know weather a response was successfully sent, use the underlying
/// `reply` object instead of returning an error code from Invoke.
///
/// Return codes
/// CHIP_IM_GLOBAL_STATUS(code):
/// - error codes that are translatable to specific IM codes
/// - in particular, the following codes are interesting/expected
/// - `UnsupportedEndpoint` for invalid endpoint
/// - `UnsupportedCluster` for no such cluster on the endpoint
/// - `UnsupportedCommand` for no such command in the cluster
/// - `UnsupportedAccess` for permission errors (ACL or fabric scoped with invalid fabric)
/// - `NeedsTimedInteraction` if the invoke requires timed interaction support
virtual CHIP_ERROR Invoke(const InvokeRequest & request, chip::TLV::TLVReader & input_arguments, InvokeReply & reply) = 0;
virtual CHIP_ERROR Invoke(const InvokeRequest & request, chip::TLV::TLVReader & input_arguments, CommandHandler * handler) = 0;

private:
InteractionModelContext mContext = { nullptr };
Expand Down
Loading
Loading