From 97c2839ea35233f15064634a9a33569af8581885 Mon Sep 17 00:00:00 2001 From: Jerry Johns Date: Tue, 9 Nov 2021 21:50:37 -0800 Subject: [PATCH] Dynamic Command Dispatch (Server-side) (#11339) --- config/standalone/CHIPProjectConfig.h | 4 + examples/bridge-app/esp32/main/main.cpp | 12 +- examples/bridge-app/linux/main.cpp | 28 +- src/app/AttributeAccessInterface.h | 2 +- src/app/CommandHandler.cpp | 6 +- src/app/CommandHandler.h | 15 +- src/app/CommandHandlerInterface.h | 181 ++++++++++++ src/app/InteractionModelEngine.cpp | 133 ++++++++- src/app/InteractionModelEngine.h | 18 +- src/app/tests/TestCommandInteraction.cpp | 7 +- src/app/util/attribute-storage.cpp | 9 +- src/app/util/attribute-storage.h | 4 +- src/controller/tests/BUILD.gn | 5 + .../tests/TestServerCommandDispatch.cpp | 261 ++++++++++++++++++ 14 files changed, 652 insertions(+), 33 deletions(-) create mode 100644 src/app/CommandHandlerInterface.h create mode 100644 src/controller/tests/TestServerCommandDispatch.cpp diff --git a/config/standalone/CHIPProjectConfig.h b/config/standalone/CHIPProjectConfig.h index 35343fc1561630..b6a6cf69ea079b 100644 --- a/config/standalone/CHIPProjectConfig.h +++ b/config/standalone/CHIPProjectConfig.h @@ -69,4 +69,8 @@ #define CHIP_CONFIG_DATA_MANAGEMENT_CLIENT_EXPERIMENTAL 1 +#ifndef DYNAMIC_ENDPOINT_COUNT +#define DYNAMIC_ENDPOINT_COUNT 4 +#endif + #endif /* CHIPPROJECTCONFIG_H */ diff --git a/examples/bridge-app/esp32/main/main.cpp b/examples/bridge-app/esp32/main/main.cpp index 80d17acd86d325..c83caa07ebcb5b 100644 --- a/examples/bridge-app/esp32/main/main.cpp +++ b/examples/bridge-app/esp32/main/main.cpp @@ -74,27 +74,27 @@ static Device gLight4("Light 4", "Den"); // Declare On/Off cluster attributes DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(onOffAttrs) -DECLARE_DYNAMIC_ATTRIBUTE(ZCL_ON_OFF_ATTRIBUTE_ID, BOOLEAN, 1, 0) /* on/off */ -DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); +DECLARE_DYNAMIC_ATTRIBUTE(ZCL_ON_OFF_ATTRIBUTE_ID, BOOLEAN, 1, 0), /* on/off */ + DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); // Declare Descriptor cluster attributes DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(descriptorAttrs) DECLARE_DYNAMIC_ATTRIBUTE(ZCL_DEVICE_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0), /* device list */ DECLARE_DYNAMIC_ATTRIBUTE(ZCL_SERVER_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0), /* server list */ DECLARE_DYNAMIC_ATTRIBUTE(ZCL_CLIENT_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0), /* client list */ - DECLARE_DYNAMIC_ATTRIBUTE(ZCL_PARTS_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0) /* parts list */ + DECLARE_DYNAMIC_ATTRIBUTE(ZCL_PARTS_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0), /* parts list */ DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); // Declare Bridged Device Basic information cluster attributes DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(bridgedDeviceBasicAttrs) DECLARE_DYNAMIC_ATTRIBUTE(ZCL_USER_LABEL_ATTRIBUTE_ID, CHAR_STRING, kUserLabelSize, 0), /* UserLabel */ - DECLARE_DYNAMIC_ATTRIBUTE(ZCL_REACHABLE_ATTRIBUTE_ID, BOOLEAN, 1, 0) /* Reachable */ + DECLARE_DYNAMIC_ATTRIBUTE(ZCL_REACHABLE_ATTRIBUTE_ID, BOOLEAN, 1, 0), /* Reachable */ DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); // Declare Fixed Label cluster attributes DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(fixedLabelAttrs) -DECLARE_DYNAMIC_ATTRIBUTE(ZCL_LABEL_LIST_ATTRIBUTE_ID, ARRAY, kFixedLabelAttributeArraySize, 0) /* label list */ -DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); +DECLARE_DYNAMIC_ATTRIBUTE(ZCL_LABEL_LIST_ATTRIBUTE_ID, ARRAY, kFixedLabelAttributeArraySize, 0), /* label list */ + DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); // Declare Cluster List for Bridged Light endpoint DECLARE_DYNAMIC_CLUSTER_LIST_BEGIN(bridgedLightClusters) diff --git a/examples/bridge-app/linux/main.cpp b/examples/bridge-app/linux/main.cpp index 530e255599cc49..2339e88e2ba376 100644 --- a/examples/bridge-app/linux/main.cpp +++ b/examples/bridge-app/linux/main.cpp @@ -94,27 +94,27 @@ static Device * gDevices[DYNAMIC_ENDPOINT_COUNT]; // Declare On/Off cluster attributes DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(onOffAttrs) -DECLARE_DYNAMIC_ATTRIBUTE(ZCL_ON_OFF_ATTRIBUTE_ID, BOOLEAN, 1, 0) /* on/off */ -DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); +DECLARE_DYNAMIC_ATTRIBUTE(ZCL_ON_OFF_ATTRIBUTE_ID, BOOLEAN, 1, 0), /* on/off */ + DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); // Declare Descriptor cluster attributes DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(descriptorAttrs) DECLARE_DYNAMIC_ATTRIBUTE(ZCL_DEVICE_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0), /* device list */ DECLARE_DYNAMIC_ATTRIBUTE(ZCL_SERVER_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0), /* server list */ DECLARE_DYNAMIC_ATTRIBUTE(ZCL_CLIENT_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0), /* client list */ - DECLARE_DYNAMIC_ATTRIBUTE(ZCL_PARTS_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0) /* parts list */ + DECLARE_DYNAMIC_ATTRIBUTE(ZCL_PARTS_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0), /* parts list */ DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); // Declare Bridged Device Basic information cluster attributes DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(bridgedDeviceBasicAttrs) DECLARE_DYNAMIC_ATTRIBUTE(ZCL_USER_LABEL_ATTRIBUTE_ID, CHAR_STRING, kUserLabelSize, 0), /* UserLabel */ - DECLARE_DYNAMIC_ATTRIBUTE(ZCL_REACHABLE_ATTRIBUTE_ID, BOOLEAN, 1, 0) /* Reachable */ + DECLARE_DYNAMIC_ATTRIBUTE(ZCL_REACHABLE_ATTRIBUTE_ID, BOOLEAN, 1, 0), /* Reachable */ DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); // Declare Fixed Label cluster attributes DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(fixedLabelAttrs) -DECLARE_DYNAMIC_ATTRIBUTE(ZCL_LABEL_LIST_ATTRIBUTE_ID, ARRAY, kFixedLabelAttributeArraySize, 0) /* label list */ -DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); +DECLARE_DYNAMIC_ATTRIBUTE(ZCL_LABEL_LIST_ATTRIBUTE_ID, ARRAY, kFixedLabelAttributeArraySize, 0), /* label list */ + DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); // Declare Cluster List for Bridged Light endpoint DECLARE_DYNAMIC_CLUSTER_LIST_BEGIN(bridgedLightClusters) @@ -135,10 +135,10 @@ DECLARE_DYNAMIC_ENDPOINT(bridgedLightEndpoint, bridgedLightClusters); // Declare Switch cluster attributes DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(switchAttrs) -DECLARE_DYNAMIC_ATTRIBUTE(ZCL_NUMBER_OF_POSITIONS_ATTRIBUTE_ID, INT8U, 1, 0), /* NumberOfPositions */ - DECLARE_DYNAMIC_ATTRIBUTE(ZCL_CURRENT_POSITION_ATTRIBUTE_ID, INT8U, 1, 0), /* CurrentPosition */ - DECLARE_DYNAMIC_ATTRIBUTE(ZCL_MULTI_PRESS_MAX_ATTRIBUTE_ID, INT8U, 1, 0), /* MultiPressMax */ - DECLARE_DYNAMIC_ATTRIBUTE(ZCL_FEATURE_MAP_SERVER_ATTRIBUTE_ID, BITMAP32, 4, 0) /* FeatureMap */ +DECLARE_DYNAMIC_ATTRIBUTE(ZCL_NUMBER_OF_POSITIONS_ATTRIBUTE_ID, INT8U, 1, 0), /* NumberOfPositions */ + DECLARE_DYNAMIC_ATTRIBUTE(ZCL_CURRENT_POSITION_ATTRIBUTE_ID, INT8U, 1, 0), /* CurrentPosition */ + DECLARE_DYNAMIC_ATTRIBUTE(ZCL_MULTI_PRESS_MAX_ATTRIBUTE_ID, INT8U, 1, 0), /* MultiPressMax */ + DECLARE_DYNAMIC_ATTRIBUTE(ZCL_FEATURE_MAP_SERVER_ATTRIBUTE_ID, BITMAP32, 4, 0), /* FeatureMap */ DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); // Declare Descriptor cluster attributes @@ -146,19 +146,19 @@ DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(switchDescriptorAttrs) DECLARE_DYNAMIC_ATTRIBUTE(ZCL_DEVICE_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0), /* device list */ DECLARE_DYNAMIC_ATTRIBUTE(ZCL_SERVER_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0), /* server list */ DECLARE_DYNAMIC_ATTRIBUTE(ZCL_CLIENT_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0), /* client list */ - DECLARE_DYNAMIC_ATTRIBUTE(ZCL_PARTS_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0) /* parts list */ + DECLARE_DYNAMIC_ATTRIBUTE(ZCL_PARTS_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0), /* parts list */ DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); // Declare Bridged Device Basic information cluster attributes DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(switchBridgedDeviceBasicAttrs) DECLARE_DYNAMIC_ATTRIBUTE(ZCL_USER_LABEL_ATTRIBUTE_ID, CHAR_STRING, kUserLabelSize, 0), /* UserLabel */ - DECLARE_DYNAMIC_ATTRIBUTE(ZCL_REACHABLE_ATTRIBUTE_ID, BOOLEAN, 1, 0) /* Reachable */ + DECLARE_DYNAMIC_ATTRIBUTE(ZCL_REACHABLE_ATTRIBUTE_ID, BOOLEAN, 1, 0), /* Reachable */ DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); // Declare Fixed Label cluster attributes DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(switchFixedLabelAttrs) -DECLARE_DYNAMIC_ATTRIBUTE(ZCL_LABEL_LIST_ATTRIBUTE_ID, ARRAY, kFixedLabelAttributeArraySize, 0) /* label list */ -DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); +DECLARE_DYNAMIC_ATTRIBUTE(ZCL_LABEL_LIST_ATTRIBUTE_ID, ARRAY, kFixedLabelAttributeArraySize, 0), /* label list */ + DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); // Declare Cluster List for Bridged Switch endpoint DECLARE_DYNAMIC_CLUSTER_LIST_BEGIN(bridgedSwitchClusters) diff --git a/src/app/AttributeAccessInterface.h b/src/app/AttributeAccessInterface.h index 15627a40fead13..8c9a020e18106a 100644 --- a/src/app/AttributeAccessInterface.h +++ b/src/app/AttributeAccessInterface.h @@ -149,7 +149,7 @@ class AttributeAccessInterface * specific endpoint. This is used to clean up overrides registered for an * endpoint that becomes disabled. */ - bool MatchesExactly(EndpointId aEndpointId) const { return mEndpointId.HasValue() && mEndpointId.Value() == aEndpointId; } + bool MatchesEndpoint(EndpointId aEndpointId) const { return mEndpointId.HasValue() && mEndpointId.Value() == aEndpointId; } /** * Check whether another AttributeAccessInterface wants to handle the same set of diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp index c2ada186d9e53d..9e01074e1ed2f3 100644 --- a/src/app/CommandHandler.cpp +++ b/src/app/CommandHandler.cpp @@ -129,7 +129,7 @@ void CommandHandler::Close() if (mpCallback) { - mpCallback->OnDone(this); + mpCallback->OnDone(*this); } } @@ -171,7 +171,7 @@ CHIP_ERROR CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommand err = commandPath.GetEndpointId(&endpointId); SuccessOrExit(err); - VerifyOrExit(ServerClusterCommandExists(ConcreteCommandPath(endpointId, clusterId, commandId)), + VerifyOrExit(mpCallback->CommandExists(ConcreteCommandPath(endpointId, clusterId, commandId)), err = CHIP_ERROR_INVALID_PROFILE_ID); err = aCommandElement.GetData(&commandDataReader); if (CHIP_END_OF_TLV == err) @@ -189,7 +189,7 @@ CHIP_ERROR CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommand endpointId, ChipLogValueMEI(clusterId), ChipLogValueMEI(commandId)); const ConcreteCommandPath concretePath(endpointId, clusterId, commandId); SuccessOrExit(MatterPreCommandReceivedCallback(concretePath)); - DispatchSingleClusterCommand(concretePath, commandDataReader, this); + mpCallback->DispatchCommand(*this, ConcreteCommandPath(endpointId, clusterId, commandId), commandDataReader); MatterPostCommandReceivedCallback(concretePath); } diff --git a/src/app/CommandHandler.h b/src/app/CommandHandler.h index 3bcd67df051f96..5ea3810c2e17a4 100644 --- a/src/app/CommandHandler.h +++ b/src/app/CommandHandler.h @@ -56,7 +56,19 @@ class CommandHandler : public Command * Method that signals to a registered callback that this object * has completed doing useful work and is now safe for release/destruction. */ - virtual void OnDone(CommandHandler * apCommandObj) = 0; + virtual void OnDone(CommandHandler & apCommandObj) = 0; + + /* + * Upon processing of a CommandDataIB, this method is invoked to dispatch the command + * to the right server-side handler provided by the application. + */ + virtual void DispatchCommand(CommandHandler & apCommandObj, const ConcreteCommandPath & aCommandPath, + TLV::TLVReader & apPayload) = 0; + + /* + * Check to see if a command implementation exists for a specific concrete command path. + */ + virtual bool CommandExists(const ConcreteCommandPath & aCommandPath) = 0; }; /* @@ -137,5 +149,6 @@ class CommandHandler : public Command bool mSuppressResponse = false; bool mTimedRequest = false; }; + } // namespace app } // namespace chip diff --git a/src/app/CommandHandlerInterface.h b/src/app/CommandHandlerInterface.h new file mode 100644 index 00000000000000..c16ee865a8df78 --- /dev/null +++ b/src/app/CommandHandlerInterface.h @@ -0,0 +1,181 @@ +/* + * + * Copyright (c) 2021 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 +#include +#include +#include // So we can encode lists + +namespace chip { +namespace app { + +/* + * This interface permits applications to register a server-side command handler + * at run-time for a given cluster. The handler can either be configured to handle all endpoints + * for the given cluster or only handle a specific endpoint. + * + * If a command is not handled through this interface, it will default to invoking the generated DispatchSingleClusterCommand + * instead. + * + */ +class CommandHandlerInterface +{ +public: + struct HandlerContext + { + public: + HandlerContext(CommandHandler & commandHandler, const ConcreteCommandPath & requestPath, TLV::TLVReader & aReader) : + mCommandHandler(commandHandler), mRequestPath(requestPath), mPayload(aReader) + {} + + void SetCommandHandled() { mCommandHandled = true; } + void SetCommandNotHandled() { mCommandHandled = false; } + + /* + * Returns a TLVReader positioned at the TLV struct that contains the payload of the command. + * + * If the reader is requested from the context, then we can assume there is an intention + * to access the payload of this command and consequently, to handle this command. + * + * If this is not true, the application should call SetCommandNotHandled(). + * + */ + TLV::TLVReader & GetReader() + { + SetCommandHandled(); + return mPayload; + } + + CommandHandler & mCommandHandler; + const ConcreteCommandPath & mRequestPath; + TLV::TLVReader & mPayload; + bool mCommandHandled = false; + }; + + /** + * aEndpointId can be Missing to indicate that this object is meant to be + * used with all endpoints. + */ + CommandHandlerInterface(Optional aEndpointId, ClusterId aClusterId) : + mEndpointId(aEndpointId), mClusterId(aClusterId) + {} + + virtual ~CommandHandlerInterface() {} + + /** + * Callback that must be implemented to handle an invoke request. + * + * The callee is required to handle *all* errors that may occur during the handling of this command, + * including errors like those encountered during decode and encode of the payloads as + * well as application-specific errors. As part of handling the error, the callee is required + * to handle generation of an appropriate status response. + * + * The only exception to this rule is if the HandleCommand helper method is used below - it will + * help handle some of these cases (see below). + * + * @param [in] handlerContext Context that encapsulates the current invoke request. + * Handlers are responsible for correctly calling SetCommandHandled() + * on the context if they did handle the command. + * + * This is not necessary if the HandleCommand() method below is invoked. + */ + virtual void InvokeCommand(HandlerContext & handlerContext) = 0; + + /** + * Mechanism for keeping track of a chain of CommandHandlerInterface. + */ + void SetNext(CommandHandlerInterface * aNext) { mNext = aNext; } + CommandHandlerInterface * GetNext() const { return mNext; } + + /** + * Check whether a this CommandHandlerInterface is relevant for a + * particular endpoint+cluster. An CommandHandlerInterface will be used + * for an invoke from a particular cluster only when this function returns + * true. + */ + bool Matches(EndpointId aEndpointId, ClusterId aClusterId) const + { + return (!mEndpointId.HasValue() || mEndpointId.Value() == aEndpointId) && mClusterId == aClusterId; + } + + /** + * Check whether an CommandHandlerInterface is relevant for a particular + * specific endpoint. This is used to clean up overrides registered for an + * endpoint that becomes disabled. + */ + bool MatchesEndpoint(EndpointId aEndpointId) const { return mEndpointId.HasValue() && mEndpointId.Value() == aEndpointId; } + + /** + * Check whether another CommandHandlerInterface wants to handle the same set of + * commands as we do. + */ + bool Matches(const CommandHandlerInterface & aOther) const + { + return mClusterId == aOther.mClusterId && + (!mEndpointId.HasValue() || !aOther.mEndpointId.HasValue() || mEndpointId.Value() == aOther.mEndpointId.Value()); + } + +protected: + /* + * Helper function to automatically de-serialize the data payload into a cluster object + * of type RequestT if the Cluster ID and Command ID in the context match. Upon successful + * de-serialization, the provided function is invoked and passed in a reference to the cluster object. + * + * Any errors encountered in this function prior to calling func result in the automatic generation of a status response. + * If `func` is called, the responsibility for doing so shifts to the callee to handle any further errors that are encountered. + * + * The provided function is expected to have the following signature: + * void Func(HandlerContext &handlerContext, const RequestT &requestPayload); + */ + template + void HandleCommand(HandlerContext & handlerContext, FuncT func) + { + if (!handlerContext.mCommandHandled && (handlerContext.mRequestPath.mClusterId == RequestT::GetClusterId()) && + (handlerContext.mRequestPath.mCommandId == RequestT::GetCommandId())) + { + RequestT requestPayload; + + // + // If the command matches what the caller is looking for, let's mark this as being handled + // even if errors happen after this. This ensures that we don't execute any fall-back strategies + // to handle this command since at this point, the caller is taking responsibility for handling + // the command in its entirety, warts and all. + // + handlerContext.SetCommandHandled(); + + if (DataModel::Decode(handlerContext.mPayload, requestPayload) != CHIP_NO_ERROR) + { + handlerContext.mCommandHandler.AddStatus(handlerContext.mRequestPath, + Protocols::InteractionModel::Status::InvalidCommand); + return; + } + + func(handlerContext, requestPayload); + } + } + +private: + Optional mEndpointId; + ClusterId mClusterId; + CommandHandlerInterface * mNext = nullptr; +}; + +} // namespace app +} // namespace chip diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp index 1186ca94cdf4db..b48978ee13d0e2 100644 --- a/src/app/InteractionModelEngine.cpp +++ b/src/app/InteractionModelEngine.cpp @@ -61,6 +61,21 @@ CHIP_ERROR InteractionModelEngine::Init(Messaging::ExchangeManager * apExchangeM void InteractionModelEngine::Shutdown() { + CommandHandlerInterface * handlerIter = mCommandHandlerList; + + // + // Walk our list of command handlers and de-register them, before finally + // nulling out the list entirely. + // + while (handlerIter) + { + CommandHandlerInterface * next = handlerIter->GetNext(); + handlerIter->SetNext(nullptr); + handlerIter = next; + } + + mCommandHandlerList = nullptr; + // // Since modifying the pool during iteration is generally frowned upon, // I've chosen to just destroy the object but not necessarily de-allocate it. @@ -277,9 +292,9 @@ CHIP_ERROR InteractionModelEngine::OnUnknownMsgType(Messaging::ExchangeContext * return err; } -void InteractionModelEngine::OnDone(CommandHandler * apCommandObj) +void InteractionModelEngine::OnDone(CommandHandler & apCommandObj) { - mCommandHandlerObjs.ReleaseObject(apCommandObj); + mCommandHandlerObjs.ReleaseObject(&apCommandObj); } CHIP_ERROR InteractionModelEngine::OnInvokeCommandRequest(Messaging::ExchangeContext * apExchangeContext, @@ -545,5 +560,119 @@ bool InteractionModelEngine::IsOverlappedAttributePath(ClusterInfo & aAttributeP return false; } +void InteractionModelEngine::DispatchCommand(CommandHandler & apCommandObj, const ConcreteCommandPath & aCommandPath, + TLV::TLVReader & apPayload) +{ + CommandHandlerInterface * handler = FindCommandHandler(aCommandPath.mEndpointId, aCommandPath.mClusterId); + + if (handler) + { + CommandHandlerInterface::HandlerContext context(apCommandObj, aCommandPath, apPayload); + handler->InvokeCommand(context); + + // + // If the command was handled, don't proceed any further and return successfully. + // + if (context.mCommandHandled) + { + return; + } + } + + DispatchSingleClusterCommand(aCommandPath, apPayload, &apCommandObj); +} + +bool InteractionModelEngine::CommandExists(const ConcreteCommandPath & aCommandPath) +{ + return ServerClusterCommandExists(aCommandPath); +} + +CHIP_ERROR InteractionModelEngine::RegisterCommandHandler(CommandHandlerInterface * handler) +{ + VerifyOrReturnError(handler != nullptr, CHIP_ERROR_INVALID_ARGUMENT); + + for (auto * cur = mCommandHandlerList; cur; cur = cur->GetNext()) + { + if (cur->Matches(*handler)) + { + ChipLogError(InteractionModel, "Duplicate command handler registration failed"); + return CHIP_ERROR_INCORRECT_STATE; + } + } + + handler->SetNext(mCommandHandlerList); + mCommandHandlerList = handler; + + return CHIP_NO_ERROR; +} + +void InteractionModelEngine::UnregisterCommandHandlers(EndpointId endpointId) +{ + CommandHandlerInterface * prev = nullptr; + + for (auto * cur = mCommandHandlerList; cur; cur = cur->GetNext()) + { + if (cur->MatchesEndpoint(endpointId)) + { + if (prev == nullptr) + { + mCommandHandlerList = cur->GetNext(); + } + else + { + prev->SetNext(cur->GetNext()); + } + + cur->SetNext(nullptr); + } + else + { + prev = cur; + } + } +} + +CHIP_ERROR InteractionModelEngine::UnregisterCommandHandler(CommandHandlerInterface * handler) +{ + VerifyOrReturnError(handler != nullptr, CHIP_ERROR_INVALID_ARGUMENT); + CommandHandlerInterface * prev = nullptr; + + for (auto * cur = mCommandHandlerList; cur; cur = cur->GetNext()) + { + if (cur->Matches(*handler)) + { + if (prev == nullptr) + { + mCommandHandlerList = cur->GetNext(); + } + else + { + prev->SetNext(cur->GetNext()); + } + + cur->SetNext(nullptr); + + return CHIP_NO_ERROR; + } + + prev = cur; + } + + return CHIP_ERROR_KEY_NOT_FOUND; +} + +CommandHandlerInterface * InteractionModelEngine::FindCommandHandler(EndpointId endpointId, ClusterId clusterId) +{ + for (auto * cur = mCommandHandlerList; cur; cur = cur->GetNext()) + { + if (cur->Matches(endpointId, clusterId)) + { + return cur; + } + } + + return nullptr; +} + } // namespace app } // namespace chip diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h index 8c6143474f7bba..d5fce192f6e56a 100644 --- a/src/app/InteractionModelEngine.h +++ b/src/app/InteractionModelEngine.h @@ -40,6 +40,7 @@ #include #include +#include #include #include #include @@ -187,19 +188,24 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate, public Comman bool MergeOverlappedAttributePath(ClusterInfo * apAttributePathList, ClusterInfo & aAttributePath); bool IsOverlappedAttributePath(ClusterInfo & aAttributePath); + CHIP_ERROR RegisterCommandHandler(CommandHandlerInterface * handler); + CHIP_ERROR UnregisterCommandHandler(CommandHandlerInterface * handler); + CommandHandlerInterface * FindCommandHandler(EndpointId endpointId, ClusterId clusterId); + void UnregisterCommandHandlers(EndpointId endpointId); + private: friend class reporting::Engine; friend class TestCommandInteraction; - void OnDone(CommandHandler * apCommandObj); + void OnDone(CommandHandler & apCommandObj) override; CHIP_ERROR OnUnknownMsgType(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload); CHIP_ERROR OnInvokeCommandRequest(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload); CHIP_ERROR OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader, - System::PacketBufferHandle && aPayload); - void OnResponseTimeout(Messaging::ExchangeContext * ec); + System::PacketBufferHandle && aPayload) override; + void OnResponseTimeout(Messaging::ExchangeContext * ec) override; /** * Called when Interaction Model receives a Read Request message. Errors processing @@ -222,9 +228,15 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate, public Comman CHIP_ERROR OnUnsolicitedReportData(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload); + void DispatchCommand(CommandHandler & apCommandObj, const ConcreteCommandPath & aCommandPath, + TLV::TLVReader & apPayload) override; + bool CommandExists(const ConcreteCommandPath & aCommandPath) override; + Messaging::ExchangeManager * mpExchangeMgr = nullptr; InteractionModelDelegate * mpDelegate = nullptr; + CommandHandlerInterface * mCommandHandlerList = nullptr; + // TODO(#8006): investgate if we can disable some IM functions on some compact accessories. // TODO(#8006): investgate if we can provide more flexible object management on devices with more resources. BitMapObjectPool mCommandHandlerObjs; diff --git a/src/app/tests/TestCommandInteraction.cpp b/src/app/tests/TestCommandInteraction.cpp index b248171eb30427..5883e649bd7fa8 100644 --- a/src/app/tests/TestCommandInteraction.cpp +++ b/src/app/tests/TestCommandInteraction.cpp @@ -140,7 +140,12 @@ class MockCommandSenderCallback : public CommandSender::Callback class MockCommandHandlerCallback : public CommandHandler::Callback { public: - void OnDone(chip::app::CommandHandler * apCommandHandler) final { onFinalCalledTimes++; } + void OnDone(CommandHandler & apCommandHandler) final { onFinalCalledTimes++; } + void DispatchCommand(CommandHandler & apCommandObj, const ConcreteCommandPath & aCommandPath, TLV::TLVReader & apPayload) final + { + DispatchSingleClusterCommand(aCommandPath, apPayload, &apCommandObj); + } + bool CommandExists(const ConcreteCommandPath & aCommandPath) { return ServerClusterCommandExists(aCommandPath); } int onFinalCalledTimes = 0; } mockCommandHandlerDelegate; diff --git a/src/app/util/attribute-storage.cpp b/src/app/util/attribute-storage.cpp index d600c92748d997..fedddb9ea5a572 100644 --- a/src/app/util/attribute-storage.cpp +++ b/src/app/util/attribute-storage.cpp @@ -40,6 +40,7 @@ ******************************************************************************/ #include "app/util/common.h" +#include #include #include #include @@ -970,6 +971,10 @@ bool emberAfEndpointEnableDisable(EndpointId endpoint, bool enable) (cluster->mask & CLUSTER_MASK_CLIENT ? EMBER_AF_CLIENT_CLUSTER_TICK : EMBER_AF_SERVER_CLUSTER_TICK)); } + // Clear out any command handler overrides registered for this + // endpoint. + chip::app::InteractionModelEngine::GetInstance()->UnregisterCommandHandlers(endpoint); + // Clear out any attribute access overrides registered for this // endpoint. app::AttributeAccessInterface * prev = nullptr; @@ -977,7 +982,7 @@ bool emberAfEndpointEnableDisable(EndpointId endpoint, bool enable) while (cur) { app::AttributeAccessInterface * next = cur->GetNext(); - if (cur->MatchesExactly(endpoint)) + if (cur->MatchesEndpoint(endpoint)) { // Remove it from the list if (prev) @@ -989,6 +994,8 @@ bool emberAfEndpointEnableDisable(EndpointId endpoint, bool enable) gAttributeAccessOverrides = next; } + cur->SetNext(nullptr); + // Do not change prev in this case. } else diff --git a/src/app/util/attribute-storage.h b/src/app/util/attribute-storage.h index 8a4b9075cf43d0..641412b92f1833 100644 --- a/src/app/util/attribute-storage.h +++ b/src/app/util/attribute-storage.h @@ -91,7 +91,9 @@ #define DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(attrListName) EmberAfAttributeMetadata attrListName[] = { #define DECLARE_DYNAMIC_ATTRIBUTE_LIST_END() \ - , { 0xFFFD, ZAP_TYPE(INT16U), 2, ZAP_ATTRIBUTE_MASK(EXTERNAL_STORAGE), ZAP_EMPTY_DEFAULT() } /* cluster revision */ \ + { \ + 0xFFFD, ZAP_TYPE(INT16U), 2, ZAP_ATTRIBUTE_MASK(EXTERNAL_STORAGE), ZAP_EMPTY_DEFAULT() \ + } /* cluster revision */ \ } #define DECLARE_DYNAMIC_ATTRIBUTE(attId, attType, attSizeBytes, attrMask) \ diff --git a/src/controller/tests/BUILD.gn b/src/controller/tests/BUILD.gn index 2aec5d280f4191..f944124b504fcd 100644 --- a/src/controller/tests/BUILD.gn +++ b/src/controller/tests/BUILD.gn @@ -23,10 +23,15 @@ chip_test_suite("tests") { test_sources = [ "TestCommissionableNodeController.cpp" ] + if (chip_device_platform != "mbed" && chip_device_platform != "efr32") { + test_sources += [ "TestServerCommandDispatch.cpp" ] + } + cflags = [ "-Wconversion" ] public_deps = [ "${chip_root}/src/app/common:cluster-objects", + "${chip_root}/src/app/tests:helpers", "${chip_root}/src/controller", "${chip_root}/src/controller/tests/data_model:interaction-tests", "${chip_root}/src/messaging/tests:helpers", diff --git a/src/controller/tests/TestServerCommandDispatch.cpp b/src/controller/tests/TestServerCommandDispatch.cpp new file mode 100644 index 00000000000000..ae30c6fb73dbce --- /dev/null +++ b/src/controller/tests/TestServerCommandDispatch.cpp @@ -0,0 +1,261 @@ +/* + * + * Copyright (c) 2021 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. + */ + +/** + * @file + * This file implements unit tests for CHIP Interaction Model Command Interaction + * + */ + +#include "app-common/zap-generated/ids/Attributes.h" +#include "app-common/zap-generated/ids/Clusters.h" +#include "protocols/interaction_model/Constants.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using TestContext = chip::Test::AppContext; + +using namespace chip; +using namespace chip::app::Clusters; + +namespace { +constexpr EndpointId kTestEndpointId = 1; + +enum ResponseDirective +{ + kSendDataResponse, + kSendSuccessStatusCode, + kSendError, + kSendSuccessStatusCodeWithClusterStatus, + kSendErrorWithClusterStatus, +}; + +ResponseDirective responseDirective; + +class TestClusterCommandHandler : public chip::app::CommandHandlerInterface +{ +public: + TestClusterCommandHandler() : chip::app::CommandHandlerInterface(Optional::Missing(), TestCluster::Id) + { + chip::app::InteractionModelEngine::GetInstance()->RegisterCommandHandler(this); + } + + ~TestClusterCommandHandler() { chip::app::InteractionModelEngine::GetInstance()->UnregisterCommandHandler(this); } + +private: + void InvokeCommand(chip::app::CommandHandlerInterface::HandlerContext & handlerContext) final; +}; + +void TestClusterCommandHandler::InvokeCommand(chip::app::CommandHandlerInterface::HandlerContext & handlerContext) +{ + HandleCommand( + handlerContext, [](chip::app::CommandHandlerInterface::HandlerContext & ctx, const auto & requestPayload) { + if (responseDirective == kSendDataResponse) + { + TestCluster::Commands::TestStructArrayArgumentResponse::Type dataResponse; + TestCluster::Structs::NestedStructList::Type nestedStructList[4]; + + uint8_t i = 0; + for (auto & item : nestedStructList) + { + item.a = i; + item.b = false; + item.c.a = i; + item.c.b = true; + i++; + } + + dataResponse.arg1 = nestedStructList; + dataResponse.arg6 = true; + + ctx.mCommandHandler.AddResponseData(ctx.mRequestPath, dataResponse); + } + + return CHIP_NO_ERROR; + }); +} + +} // namespace + +namespace { + +class TestCommandInteraction +{ +public: + TestCommandInteraction() {} + static void TestNoHandler(nlTestSuite * apSuite, void * apContext); + static void TestDataResponse(nlTestSuite * apSuite, void * apContext); + +private: +}; + +void TestCommandInteraction::TestNoHandler(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + TestCluster::Commands::TestSimpleArgumentRequest::Type request; + auto sessionHandle = ctx.GetSessionBobToAlice(); + + request.arg1 = true; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onSuccessCb = [apSuite](const app::ConcreteCommandPath & commandPath, const app::StatusIB & aStatus, + const auto & dataResponse) { + // + // We shouldn't be arriving here, since we don't have a command handler installed. + // + NL_TEST_ASSERT(apSuite, false); + }; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onFailureCb = [apSuite](const app::StatusIB & aStatus, CHIP_ERROR aError) { + NL_TEST_ASSERT(apSuite, aStatus.mStatus == Protocols::InteractionModel::Status::InvalidCommand); + }; + + responseDirective = kSendDataResponse; + + ctx.EnableAsyncDispatch(); + + chip::Controller::InvokeCommandRequest( + &ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, request, onSuccessCb, onFailureCb); + + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); +} + +static const int kDescriptorAttributeArraySize = 254; + +// Declare Descriptor cluster attributes +DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(descriptorAttrs) +DECLARE_DYNAMIC_ATTRIBUTE(chip::app::Clusters::Descriptor::Attributes::DeviceList::Id, ARRAY, kDescriptorAttributeArraySize, + 0), /* device list */ + DECLARE_DYNAMIC_ATTRIBUTE(chip::app::Clusters::Descriptor::Attributes::ServerList::Id, ARRAY, kDescriptorAttributeArraySize, + 0), /* server list */ + DECLARE_DYNAMIC_ATTRIBUTE(chip::app::Clusters::Descriptor::Attributes::ClientList::Id, ARRAY, kDescriptorAttributeArraySize, + 0), /* client list */ + DECLARE_DYNAMIC_ATTRIBUTE(chip::app::Clusters::Descriptor::Attributes::PartsList::Id, ARRAY, kDescriptorAttributeArraySize, + 0), /* parts list */ + DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); + +DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(testClusterAttrs) +DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(); + +DECLARE_DYNAMIC_CLUSTER_LIST_BEGIN(testEndpointClusters) +DECLARE_DYNAMIC_CLUSTER(chip::app::Clusters::TestCluster::Id, testClusterAttrs), + DECLARE_DYNAMIC_CLUSTER(chip::app::Clusters::Descriptor::Id, descriptorAttrs), DECLARE_DYNAMIC_CLUSTER_LIST_END; + +DECLARE_DYNAMIC_ENDPOINT(testEndpoint, testEndpointClusters); + +void TestCommandInteraction::TestDataResponse(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + TestCluster::Commands::TestSimpleArgumentRequest::Type request; + auto sessionHandle = ctx.GetSessionBobToAlice(); + TestClusterCommandHandler commandHandler; + + bool onSuccessWasCalled = false; + bool onFailureWasCalled = false; + + request.arg1 = true; + + // + // Register descriptors for this endpoint since they are needed + // at command validation time to ensure the command actually exists on that endpoint. + // + emberAfSetDynamicEndpoint(0, kTestEndpointId, &testEndpoint, 0, 0); + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onSuccessCb = [apSuite, &onSuccessWasCalled](const app::ConcreteCommandPath & commandPath, const app::StatusIB & aStatus, + const auto & dataResponse) { + uint8_t i = 0; + auto iter = dataResponse.arg1.begin(); + while (iter.Next()) + { + auto & item = iter.GetValue(); + + NL_TEST_ASSERT(apSuite, item.a == i); + NL_TEST_ASSERT(apSuite, item.b == false); + NL_TEST_ASSERT(apSuite, item.c.a == i); + NL_TEST_ASSERT(apSuite, item.c.b == true); + i++; + } + + NL_TEST_ASSERT(apSuite, iter.GetStatus() == CHIP_NO_ERROR); + NL_TEST_ASSERT(apSuite, dataResponse.arg6 == true); + + onSuccessWasCalled = true; + }; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onFailureCb = [&onFailureWasCalled](const app::StatusIB & aStatus, CHIP_ERROR aError) { onFailureWasCalled = true; }; + + responseDirective = kSendDataResponse; + + chip::Controller::InvokeCommandRequest( + &ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, request, onSuccessCb, onFailureCb); + + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(apSuite, onSuccessWasCalled && !onFailureWasCalled); + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); +} + +// clang-format off +const nlTest sTests[] = +{ + NL_TEST_DEF("TestNoHandler", TestCommandInteraction::TestNoHandler), + NL_TEST_DEF("TestDataResponse", TestCommandInteraction::TestDataResponse), + NL_TEST_SENTINEL() +}; + +// clang-format on + +// clang-format off +nlTestSuite sSuite = +{ + "TestCommands", + &sTests[0], + TestContext::InitializeAsync, + TestContext::Finalize +}; +// clang-format on + +} // namespace + +int TestCommandInteractionTest() +{ + TestContext gContext; + nlTestRunner(&sSuite, &gContext); + return (nlTestRunnerStats(&sSuite)); +} + +CHIP_REGISTER_TEST_SUITE(TestCommandInteractionTest)