diff --git a/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp b/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp index 4d42fc90282104..aa8e2e20d9cc8d 100644 --- a/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp +++ b/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp @@ -17,11 +17,16 @@ #include #include +#include #include +#include +#include #include +#include #include #include #include +#include #include #include @@ -31,6 +36,113 @@ namespace chip { namespace app { namespace { +/// Handles going through callback-based enumeration of generated/accepted commands +/// for CommandHandlerInterface based items. +/// +/// Offers the ability to focus on some operation for finding a given +/// command id: +/// - FindFirst will return the first found element +/// - FindExact finds the element with the given id +/// - FindNext finds the element following the given id +class EnumeratorCommandFinder +{ +public: + using HandlerCallbackFunction = CHIP_ERROR (CommandHandlerInterface::*)(const ConcreteClusterPath &, + CommandHandlerInterface::CommandIdCallback, void *); + + enum class Operation + { + kFindFirst, // Find the first value in the list + kFindExact, // Find the given value + kFindNext // Find the value AFTER this value + }; + + EnumeratorCommandFinder(HandlerCallbackFunction callback) : + mCallback(callback), mOperation(Operation::kFindFirst), mTarget(kInvalidCommandId) + {} + + /// Find the given command ID that matches the given operation/path. + /// + /// If operation is kFindFirst, then path commandID is ignored. Otherwise it is used as a key to + /// kFindExact or kFindNext. + /// + /// Returns: + /// - std::nullopt if no command found using the command handler interface + /// - kInvalidCommandId if the find failed (but command handler interface does provide a list) + /// - valid id if command handler interface usage succeeds + std::optional FindCommandId(Operation operation, const ConcreteCommandPath & path); + + /// Uses FindCommandId to find the given command and loads the command entry data + std::optional FindCommandEntry(Operation operation, const ConcreteCommandPath & path); + +private: + HandlerCallbackFunction mCallback; + Operation mOperation; + CommandId mTarget; + std::optional mFound = std::nullopt; + + Loop HandlerCallback(CommandId id) + { + switch (mOperation) + { + case Operation::kFindFirst: + mFound = id; + return Loop::Break; + case Operation::kFindExact: + if (mTarget == id) + { + mFound = id; // found it + return Loop::Break; + } + break; + case Operation::kFindNext: + if (mTarget == id) + { + // Once we found the ID, get the first + mOperation = Operation::kFindFirst; + } + break; + } + return Loop::Continue; // keep searching + } + + static Loop HandlerCallbackFn(CommandId id, void * context) + { + auto self = static_cast(context); + return self->HandlerCallback(id); + } +}; + +std::optional EnumeratorCommandFinder::FindCommandId(Operation operation, const ConcreteCommandPath & path) +{ + mOperation = operation; + mTarget = path.mCommandId; + + CommandHandlerInterface * interface = + CommandHandlerInterfaceRegistry::Instance().GetCommandHandler(path.mEndpointId, path.mClusterId); + + if (interface == nullptr) + { + return std::nullopt; // no data: no interface + } + + CHIP_ERROR err = (interface->*mCallback)(path, HandlerCallbackFn, this); + if (err == CHIP_ERROR_NOT_IMPLEMENTED) + { + return std::nullopt; // no data provided by the interface + } + + if (err != CHIP_NO_ERROR) + { + // Report the error here since we lose actual error. This generally should NOT be possible as CommandHandlerInterface + // usually returns unimplemented or should just work for our use case (our callback never fails) + ChipLogError(DataManagement, "Enumerate error: %" CHIP_ERROR_FORMAT, err.Format()); + return kInvalidCommandId; + } + + return mFound.value_or(kInvalidCommandId); +} + /// Load the cluster information into the specified destination std::variant LoadClusterInfo(const ConcreteClusterPath & path, const EmberAfCluster & cluster) { @@ -160,6 +272,20 @@ DataModel::CommandEntry CommandEntryFrom(const ConcreteClusterPath & clusterPath return entry; } +std::optional EnumeratorCommandFinder::FindCommandEntry(Operation operation, + const ConcreteCommandPath & path) +{ + + std::optional id = FindCommandId(operation, path); + + if (!id.has_value()) + { + return std::nullopt; + } + + return (*id == kInvalidCommandId) ? DataModel::CommandEntry::kInvalid : CommandEntryFrom(path, *id); +} + const ConcreteCommandPath kInvalidCommandPath(kInvalidEndpointId, kInvalidClusterId, kInvalidCommandId); } // namespace @@ -492,6 +618,15 @@ std::optional CodegenDataModelProvider::GetAttributeIn DataModel::CommandEntry CodegenDataModelProvider::FirstAcceptedCommand(const ConcreteClusterPath & path) { + auto handlerInterfaceValue = EnumeratorCommandFinder(&CommandHandlerInterface::EnumerateAcceptedCommands) + .FindCommandEntry(EnumeratorCommandFinder::Operation::kFindFirst, + ConcreteCommandPath(path.mEndpointId, path.mClusterId, kInvalidCommandId)); + + if (handlerInterfaceValue.has_value()) + { + return *handlerInterfaceValue; + } + const EmberAfCluster * cluster = FindServerCluster(path); VerifyOrReturnValue(cluster != nullptr, DataModel::CommandEntry::kInvalid); @@ -504,6 +639,16 @@ DataModel::CommandEntry CodegenDataModelProvider::FirstAcceptedCommand(const Con DataModel::CommandEntry CodegenDataModelProvider::NextAcceptedCommand(const ConcreteCommandPath & before) { + // TODO: `Next` redirecting to a callback is slow O(n^2). + // see https://github.com/project-chip/connectedhomeip/issues/35790 + auto handlerInterfaceValue = EnumeratorCommandFinder(&CommandHandlerInterface::EnumerateAcceptedCommands) + .FindCommandEntry(EnumeratorCommandFinder::Operation::kFindNext, before); + + if (handlerInterfaceValue.has_value()) + { + return *handlerInterfaceValue; + } + const EmberAfCluster * cluster = FindServerCluster(before); VerifyOrReturnValue(cluster != nullptr, DataModel::CommandEntry::kInvalid); @@ -516,6 +661,14 @@ DataModel::CommandEntry CodegenDataModelProvider::NextAcceptedCommand(const Conc std::optional CodegenDataModelProvider::GetAcceptedCommandInfo(const ConcreteCommandPath & path) { + auto handlerInterfaceValue = EnumeratorCommandFinder(&CommandHandlerInterface::EnumerateAcceptedCommands) + .FindCommandEntry(EnumeratorCommandFinder::Operation::kFindExact, path); + + if (handlerInterfaceValue.has_value()) + { + return handlerInterfaceValue->IsValid() ? std::make_optional(handlerInterfaceValue->info) : std::nullopt; + } + const EmberAfCluster * cluster = FindServerCluster(path); VerifyOrReturnValue(cluster != nullptr, std::nullopt); @@ -526,17 +679,37 @@ std::optional CodegenDataModelProvider::GetAcceptedComma ConcreteCommandPath CodegenDataModelProvider::FirstGeneratedCommand(const ConcreteClusterPath & path) { - const EmberAfCluster * cluster = FindServerCluster(path); + std::optional commandId = + EnumeratorCommandFinder(&CommandHandlerInterface::EnumerateGeneratedCommands) + .FindCommandId(EnumeratorCommandFinder::Operation::kFindFirst, + ConcreteCommandPath(path.mEndpointId, path.mClusterId, kInvalidCommandId)); + if (commandId.has_value()) + { + return *commandId == kInvalidCommandId ? kInvalidCommandPath + : ConcreteCommandPath(path.mEndpointId, path.mClusterId, *commandId); + } + const EmberAfCluster * cluster = FindServerCluster(path); VerifyOrReturnValue(cluster != nullptr, kInvalidCommandPath); - std::optional commandId = mGeneratedCommandsIterator.First(cluster->generatedCommandList); + commandId = mGeneratedCommandsIterator.First(cluster->generatedCommandList); VerifyOrReturnValue(commandId.has_value(), kInvalidCommandPath); return ConcreteCommandPath(path.mEndpointId, path.mClusterId, *commandId); } ConcreteCommandPath CodegenDataModelProvider::NextGeneratedCommand(const ConcreteCommandPath & before) { + // TODO: `Next` redirecting to a callback is slow O(n^2). + // see https://github.com/project-chip/connectedhomeip/issues/35790 + auto nextId = EnumeratorCommandFinder(&CommandHandlerInterface::EnumerateGeneratedCommands) + .FindCommandId(EnumeratorCommandFinder::Operation::kFindNext, before); + + if (nextId.has_value()) + { + return (*nextId == kInvalidCommandId) ? kInvalidCommandPath + : ConcreteCommandPath(before.mEndpointId, before.mClusterId, *nextId); + } + const EmberAfCluster * cluster = FindServerCluster(before); VerifyOrReturnValue(cluster != nullptr, kInvalidCommandPath); diff --git a/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp index 077376bcb44311..5314684fc6f9a4 100644 --- a/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp @@ -15,8 +15,6 @@ * limitations under the License. */ -#include - #include #include @@ -30,11 +28,14 @@ #include #include #include +#include +#include #include #include #include #include #include +#include #include #include #include @@ -62,6 +63,8 @@ #include #include +#include + using namespace chip; using namespace chip::Test; using namespace chip::app; @@ -183,6 +186,61 @@ class MockAccessControl : public Access::AccessControl::Delegate, public Access: bool IsDeviceTypeOnEndpoint(DeviceTypeId deviceType, EndpointId endpoint) override { return true; } }; +/// Overrides Enumerate*Commands in the CommandHandlerInterface to allow +/// testing of behaviors when command enumeration is done in the interace. +class CustomListCommandHandler : public CommandHandlerInterface +{ +public: + CustomListCommandHandler(Optional endpointId, ClusterId clusterId) : CommandHandlerInterface(endpointId, clusterId) + { + CommandHandlerInterfaceRegistry::Instance().RegisterCommandHandler(this); + } + ~CustomListCommandHandler() { CommandHandlerInterfaceRegistry::Instance().UnregisterCommandHandler(this); } + + void InvokeCommand(HandlerContext & handlerContext) override { handlerContext.SetCommandNotHandled(); } + + CHIP_ERROR EnumerateAcceptedCommands(const ConcreteClusterPath & cluster, CommandIdCallback callback, void * context) override + { + VerifyOrReturnError(mOverrideAccepted, CHIP_ERROR_NOT_IMPLEMENTED); + + for (auto id : mAccepted) + { + if (callback(id, context) != Loop::Continue) + { + break; + } + } + return CHIP_NO_ERROR; + } + + CHIP_ERROR EnumerateGeneratedCommands(const ConcreteClusterPath & cluster, CommandIdCallback callback, void * context) override + { + VerifyOrReturnError(mOverrideGenerated, CHIP_ERROR_NOT_IMPLEMENTED); + + for (auto id : mGenerated) + { + if (callback(id, context) != Loop::Continue) + { + break; + } + } + return CHIP_NO_ERROR; + } + + void SetOverrideAccepted(bool o) { mOverrideAccepted = o; } + void SetOverrideGenerated(bool o) { mOverrideGenerated = o; } + + std::vector & AcceptedVec() { return mAccepted; } + std::vector & GeneratedVec() { return mGenerated; } + +private: + bool mOverrideAccepted = false; + bool mOverrideGenerated = false; + + std::vector mAccepted; + std::vector mGenerated; +}; + class ScopedMockAccessControl { public: @@ -1193,6 +1251,60 @@ TEST(TestCodegenModelViaMocks, IterateOverGeneratedCommands) } } +TEST(TestCodegenModelViaMocks, CommandHandlerInterfaceAcceptedCommands) +{ + + UseMockNodeConfig config(gTestNodeConfig); + CodegenDataModelProviderWithContext model; + + // Command handler interface is capable to override accepted and generated commands. + // Validate that these work + CustomListCommandHandler handler(MakeOptional(kMockEndpoint1), MockClusterId(1)); + + // At this point, without overrides, there should be no accepted/generated commands + EXPECT_FALSE(model.FirstAcceptedCommand(ConcreteClusterPath(kMockEndpoint1, MockClusterId(1))).IsValid()); + EXPECT_FALSE(model.FirstGeneratedCommand(ConcreteClusterPath(kMockEndpoint1, MockClusterId(1))).HasValidIds()); + EXPECT_FALSE(model.GetAcceptedCommandInfo(ConcreteCommandPath(kMockEndpoint1, MockClusterId(1), 1234)).has_value()); + + handler.SetOverrideAccepted(true); + handler.SetOverrideGenerated(true); + + // with overrides, the list is still empty ... + EXPECT_FALSE(model.FirstAcceptedCommand(ConcreteClusterPath(kMockEndpoint1, MockClusterId(1))).IsValid()); + EXPECT_FALSE(model.FirstGeneratedCommand(ConcreteClusterPath(kMockEndpoint1, MockClusterId(1))).HasValidIds()); + EXPECT_FALSE(model.GetAcceptedCommandInfo(ConcreteCommandPath(kMockEndpoint1, MockClusterId(1), 1234)).has_value()); + + // set some overrides + handler.AcceptedVec().push_back(1234); + handler.AcceptedVec().push_back(999); + + handler.GeneratedVec().push_back(33); + + DataModel::CommandEntry entry; + + entry = model.FirstAcceptedCommand(ConcreteClusterPath(kMockEndpoint1, MockClusterId(1))); + EXPECT_TRUE(entry.IsValid()); + EXPECT_EQ(entry.path, ConcreteCommandPath(kMockEndpoint1, MockClusterId(1), 1234)); + + entry = model.NextAcceptedCommand(entry.path); + EXPECT_TRUE(entry.IsValid()); + EXPECT_EQ(entry.path, ConcreteCommandPath(kMockEndpoint1, MockClusterId(1), 999)); + + entry = model.NextAcceptedCommand(entry.path); + EXPECT_FALSE(entry.IsValid()); + + ConcreteCommandPath path = model.FirstGeneratedCommand(ConcreteClusterPath(kMockEndpoint1, MockClusterId(1))); + EXPECT_TRUE(path.HasValidIds()); + EXPECT_EQ(path, ConcreteCommandPath(kMockEndpoint1, MockClusterId(1), 33)); + path = model.NextGeneratedCommand(path); + EXPECT_FALSE(path.HasValidIds()); + + // Command finding should work + EXPECT_TRUE(model.GetAcceptedCommandInfo(ConcreteCommandPath(kMockEndpoint1, MockClusterId(1), 1234)).has_value()); + EXPECT_FALSE(model.GetAcceptedCommandInfo(ConcreteCommandPath(kMockEndpoint1, MockClusterId(1), 88)).has_value()); + EXPECT_FALSE(model.GetAcceptedCommandInfo(ConcreteCommandPath(kMockEndpoint1, MockClusterId(1), 33)).has_value()); +} + TEST(TestCodegenModelViaMocks, EmberAttributeReadAclDeny) { UseMockNodeConfig config(gTestNodeConfig);