From 1cac8ce91f415edd77d69469171ac3212151f3e7 Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Fri, 13 Sep 2024 11:34:59 +0530 Subject: [PATCH 01/25] Regenerate zap for actions cluster --- .../all-clusters-app.matter | 17 ++- .../all-clusters-common/all-clusters-app.zap | 104 +++++++++++++++++- src/app/common/templates/config-data.yaml | 1 + .../app-common/zap-generated/callback.h | 72 ------------ 4 files changed, 117 insertions(+), 77 deletions(-) diff --git a/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter b/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter index 34c5d04bbc92a8..56a820e2169040 100644 --- a/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter +++ b/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter @@ -8167,9 +8167,22 @@ endpoint 1 { server cluster Actions { callback attribute actionList; callback attribute endpointLists; - callback attribute setupURL; + ram attribute setupURL default = "https://example.com"; ram attribute featureMap default = 0; - callback attribute clusterRevision default = 1; + ram attribute clusterRevision default = 1; + + handle command InstantAction; + handle command InstantActionWithTransition; + handle command StartAction; + handle command StartActionWithDuration; + handle command StopAction; + handle command PauseAction; + handle command PauseActionWithDuration; + handle command ResumeAction; + handle command EnableAction; + handle command EnableActionWithDuration; + handle command DisableAction; + handle command DisableActionWithDuration; } server cluster PowerSource { diff --git a/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap b/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap index 4949c94a7b641a..e46e30e1e87f53 100644 --- a/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap +++ b/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap @@ -7061,6 +7061,104 @@ "define": "ACTIONS_CLUSTER", "side": "server", "enabled": 1, + "commands": [ + { + "name": "InstantAction", + "code": 0, + "mfgCode": null, + "source": "client", + "isIncoming": 1, + "isEnabled": 1 + }, + { + "name": "InstantActionWithTransition", + "code": 1, + "mfgCode": null, + "source": "client", + "isIncoming": 1, + "isEnabled": 1 + }, + { + "name": "StartAction", + "code": 2, + "mfgCode": null, + "source": "client", + "isIncoming": 1, + "isEnabled": 1 + }, + { + "name": "StartActionWithDuration", + "code": 3, + "mfgCode": null, + "source": "client", + "isIncoming": 1, + "isEnabled": 1 + }, + { + "name": "StopAction", + "code": 4, + "mfgCode": null, + "source": "client", + "isIncoming": 1, + "isEnabled": 1 + }, + { + "name": "PauseAction", + "code": 5, + "mfgCode": null, + "source": "client", + "isIncoming": 1, + "isEnabled": 1 + }, + { + "name": "PauseActionWithDuration", + "code": 6, + "mfgCode": null, + "source": "client", + "isIncoming": 1, + "isEnabled": 1 + }, + { + "name": "ResumeAction", + "code": 7, + "mfgCode": null, + "source": "client", + "isIncoming": 1, + "isEnabled": 1 + }, + { + "name": "EnableAction", + "code": 8, + "mfgCode": null, + "source": "client", + "isIncoming": 1, + "isEnabled": 1 + }, + { + "name": "EnableActionWithDuration", + "code": 9, + "mfgCode": null, + "source": "client", + "isIncoming": 1, + "isEnabled": 1 + }, + { + "name": "DisableAction", + "code": 10, + "mfgCode": null, + "source": "client", + "isIncoming": 1, + "isEnabled": 1 + }, + { + "name": "DisableActionWithDuration", + "code": 11, + "mfgCode": null, + "source": "client", + "isIncoming": 1, + "isEnabled": 1 + } + ], "attributes": [ { "name": "ActionList", @@ -7101,10 +7199,10 @@ "side": "server", "type": "long_char_string", "included": 1, - "storageOption": "External", + "storageOption": "RAM", "singleton": 0, "bounded": 0, - "defaultValue": "", + "defaultValue": "https://example.com", "reportable": 1, "minInterval": 0, "maxInterval": 65344, @@ -7133,7 +7231,7 @@ "side": "server", "type": "int16u", "included": 1, - "storageOption": "External", + "storageOption": "RAM", "singleton": 0, "bounded": 0, "defaultValue": "1", diff --git a/src/app/common/templates/config-data.yaml b/src/app/common/templates/config-data.yaml index 7dcf2b6b68ec6f..53e4362e9a6192 100644 --- a/src/app/common/templates/config-data.yaml +++ b/src/app/common/templates/config-data.yaml @@ -54,6 +54,7 @@ CommandHandlerInterfaceOnlyClusters: - Software Diagnostics - Wi-Fi Network Diagnostics - Administrator Commissioning + - Actions # We need a more configurable way of deciding which clusters have which init functions.... # See https://github.com/project-chip/connectedhomeip/issues/4369 diff --git a/zzz_generated/app-common/app-common/zap-generated/callback.h b/zzz_generated/app-common/app-common/zap-generated/callback.h index c8c3b01f738881..819cda45d5ac75 100644 --- a/zzz_generated/app-common/app-common/zap-generated/callback.h +++ b/zzz_generated/app-common/app-common/zap-generated/callback.h @@ -5894,78 +5894,6 @@ bool emberAfLevelControlClusterMoveToClosestFrequencyCallback( bool emberAfAccessControlClusterReviewFabricRestrictionsCallback( chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, const chip::app::Clusters::AccessControl::Commands::ReviewFabricRestrictions::DecodableType & commandData); -/** - * @brief Actions Cluster InstantAction Command callback (from client) - */ -bool emberAfActionsClusterInstantActionCallback( - chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, - const chip::app::Clusters::Actions::Commands::InstantAction::DecodableType & commandData); -/** - * @brief Actions Cluster InstantActionWithTransition Command callback (from client) - */ -bool emberAfActionsClusterInstantActionWithTransitionCallback( - chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, - const chip::app::Clusters::Actions::Commands::InstantActionWithTransition::DecodableType & commandData); -/** - * @brief Actions Cluster StartAction Command callback (from client) - */ -bool emberAfActionsClusterStartActionCallback( - chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, - const chip::app::Clusters::Actions::Commands::StartAction::DecodableType & commandData); -/** - * @brief Actions Cluster StartActionWithDuration Command callback (from client) - */ -bool emberAfActionsClusterStartActionWithDurationCallback( - chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, - const chip::app::Clusters::Actions::Commands::StartActionWithDuration::DecodableType & commandData); -/** - * @brief Actions Cluster StopAction Command callback (from client) - */ -bool emberAfActionsClusterStopActionCallback(chip::app::CommandHandler * commandObj, - const chip::app::ConcreteCommandPath & commandPath, - const chip::app::Clusters::Actions::Commands::StopAction::DecodableType & commandData); -/** - * @brief Actions Cluster PauseAction Command callback (from client) - */ -bool emberAfActionsClusterPauseActionCallback( - chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, - const chip::app::Clusters::Actions::Commands::PauseAction::DecodableType & commandData); -/** - * @brief Actions Cluster PauseActionWithDuration Command callback (from client) - */ -bool emberAfActionsClusterPauseActionWithDurationCallback( - chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, - const chip::app::Clusters::Actions::Commands::PauseActionWithDuration::DecodableType & commandData); -/** - * @brief Actions Cluster ResumeAction Command callback (from client) - */ -bool emberAfActionsClusterResumeActionCallback( - chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, - const chip::app::Clusters::Actions::Commands::ResumeAction::DecodableType & commandData); -/** - * @brief Actions Cluster EnableAction Command callback (from client) - */ -bool emberAfActionsClusterEnableActionCallback( - chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, - const chip::app::Clusters::Actions::Commands::EnableAction::DecodableType & commandData); -/** - * @brief Actions Cluster EnableActionWithDuration Command callback (from client) - */ -bool emberAfActionsClusterEnableActionWithDurationCallback( - chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, - const chip::app::Clusters::Actions::Commands::EnableActionWithDuration::DecodableType & commandData); -/** - * @brief Actions Cluster DisableAction Command callback (from client) - */ -bool emberAfActionsClusterDisableActionCallback( - chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, - const chip::app::Clusters::Actions::Commands::DisableAction::DecodableType & commandData); -/** - * @brief Actions Cluster DisableActionWithDuration Command callback (from client) - */ -bool emberAfActionsClusterDisableActionWithDurationCallback( - chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, - const chip::app::Clusters::Actions::Commands::DisableActionWithDuration::DecodableType & commandData); /** * @brief Basic Information Cluster MfgSpecificPing Command callback (from client) */ From ae306d3c86a4c4c491adeecf29e4e4ee14164545 Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Fri, 13 Sep 2024 11:36:02 +0530 Subject: [PATCH 02/25] Add generic implementation for action cluster --- .../include/bridged-actions-stub.h | 110 ++++++ .../src/bridged-actions-stub.cpp | 141 +++++--- .../actions-server/actions-server.cpp | 330 ++++++++++++++++++ .../clusters/actions-server/actions-server.h | 108 ++++++ 4 files changed, 635 insertions(+), 54 deletions(-) create mode 100644 examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h create mode 100644 src/app/clusters/actions-server/actions-server.cpp create mode 100644 src/app/clusters/actions-server/actions-server.h diff --git a/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h b/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h new file mode 100644 index 00000000000000..2c38869f26802f --- /dev/null +++ b/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h @@ -0,0 +1,110 @@ +/* + * + * Copyright (c) 2024 Project CHIP Authors + * + * 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 +#include +#include +#include +#include +#include + +using namespace chip; +using namespace chip::app; +using namespace chip::app::Clusters; +using namespace chip::app::Clusters::Actions; +using namespace chip::app::Clusters::Actions::Attributes; + +namespace chip { +namespace app { +namespace Clusters { +namespace Actions { +const uint16_t kActionListSize = 3; +const uint16_t kEndpointListSize = 3; +class ActionsDelegateImpl : public Delegate +{ +private: + using ActionListStructType = Structs::ActionStruct::Type; + using EndpointListStructType = Structs::EndpointListStruct::Type; + + const ActionListStructType kActionList[kActionListSize] = { + ActionListStructType{ .actionID = 0, + .name = CharSpan::fromCharString("TurnOnLight"), + .type = ActionTypeEnum::kScene, + .endpointListID = 0, + .supportedCommands = 0, + .state = ActionStateEnum::kInactive }, + + ActionListStructType{ .actionID = 1, + .name = CharSpan::fromCharString("TurnOffLight"), + .type = ActionTypeEnum::kScene, + .endpointListID = 1, + .supportedCommands = 0, + .state = ActionStateEnum::kInactive }, + + ActionListStructType{ .actionID = 2, + .name = CharSpan::fromCharString("ToggleLight"), + .type = ActionTypeEnum::kScene, + .endpointListID = 2, + .supportedCommands = 0, + .state = ActionStateEnum::kInactive }, + }; + + // Dummy endpoint list. + const EndpointId firstEpList[1] = { 0 }; + const EndpointId secondEpList[1] = { 0 }; + const EndpointId thirdEpList[1] = { 0 }; + + const EndpointListStructType kEndpointList[kEndpointListSize] = { + EndpointListStructType{ .endpointListID = 0, + .name = CharSpan::fromCharString("On"), + .type = EndpointListTypeEnum::kOther, + .endpoints = DataModel::List(firstEpList) }, + + EndpointListStructType{ .endpointListID = 1, + .name = CharSpan::fromCharString("Off"), + .type = EndpointListTypeEnum::kOther, + .endpoints = DataModel::List(secondEpList) }, + + EndpointListStructType{ .endpointListID = 2, + .name = CharSpan::fromCharString("Toggle"), + .type = EndpointListTypeEnum::kOther, + .endpoints = DataModel::List(thirdEpList) }, + }; + CHIP_ERROR ReadActionAtIndex(uint16_t index, ActionListStructType & action) override; + CHIP_ERROR ReadEndpointListAtIndex(uint16_t index, EndpointListStructType & epList) override; + CHIP_ERROR FindActionIdInActionList(uint16_t actionId) override; + + Status HandleInstantAction(uint16_t actionId, Optional invokeId) override; + Status HandleInstantActionWithTransition(uint16_t actionId, uint16_t transitionTime, Optional invokeId) override; + Status HandleStartAction(uint16_t actionId, Optional invokeId) override; + Status HandleStartActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) override; + Status HandleStopAction(uint16_t actionId, Optional invokeId) override; + Status HandlePauseAction(uint16_t actionId, Optional invokeId) override; + Status HandlePauseActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) override; + Status HandleResumeAction(uint16_t actionId, Optional invokeId) override; + Status HandleEnableAction(uint16_t actionId, Optional invokeId) override; + Status HandleEnableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) override; + Status HandleDisableAction(uint16_t actionId, Optional invokeId) override; + Status HandleDisableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) override; +}; +} // namespace Actions +} // namespace Clusters +} // namespace app +} // namespace chip diff --git a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp index 90d1b9ebd115b0..ab2670f3aa2c8f 100644 --- a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp +++ b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp @@ -1,6 +1,6 @@ /* * - * Copyright (c) 2021 Project CHIP Authors + * Copyright (c) 2024 Project CHIP Authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,88 +15,121 @@ * limitations under the License. */ -#include -#include -#include -#include -#include -#include -#include -#include +#include using namespace chip; using namespace chip::app; using namespace chip::app::Clusters; +using namespace chip::app::Clusters::Actions; using namespace chip::app::Clusters::Actions::Attributes; -namespace { +CHIP_ERROR ActionsDelegateImpl::ReadActionAtIndex(uint16_t index, ActionListStructType & action) +{ + if (index >= ArraySize(kActionList)) + { + return CHIP_ERROR_PROVIDER_LIST_EXHAUSTED; + } + action.actionID = kActionList[index].actionID; + action.name = kActionList[index].name; + action.type = kActionList[index].type; + action.endpointListID = kActionList[index].endpointListID; + action.supportedCommands = kActionList[index].supportedCommands; + action.state = kActionList[index].state; + return CHIP_NO_ERROR; +} -class ActionsAttrAccess : public AttributeAccessInterface +CHIP_ERROR ActionsDelegateImpl::ReadEndpointListAtIndex(uint16_t index, EndpointListStructType & epList) { -public: - // Register for the Actions cluster on all endpoints. - ActionsAttrAccess() : AttributeAccessInterface(Optional::Missing(), Actions::Id) {} + if (index >= ArraySize(kEndpointList)) + { + return CHIP_ERROR_PROVIDER_LIST_EXHAUSTED; + } + epList.endpointListID = kEndpointList[index].endpointListID; + epList.name = kEndpointList[index].name; + epList.type = kEndpointList[index].type; + epList.endpoints = kEndpointList[index].endpoints; + return CHIP_NO_ERROR; +} - CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; +CHIP_ERROR ActionsDelegateImpl::FindActionIdInActionList(uint16_t actionId) +{ + for (uint16_t i = 0; i < kActionListSize; i++) + { + if (kActionList[i].actionID == actionId) + return CHIP_NO_ERROR; + } + return CHIP_ERROR_NOT_FOUND; +} -private: - static constexpr uint16_t ClusterRevision = 1; +Status ActionsDelegateImpl::HandleInstantAction(uint16_t actionId, Optional invokeId) +{ + // Not implemented + return Status::NotFound; +} - CHIP_ERROR ReadActionListAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder); - CHIP_ERROR ReadEndpointListAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder); - CHIP_ERROR ReadSetupUrlAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder); - CHIP_ERROR ReadClusterRevision(EndpointId endpoint, AttributeValueEncoder & aEncoder); -}; +Status ActionsDelegateImpl::HandleInstantActionWithTransition(uint16_t actionId, uint16_t transitionTime, + Optional invokeId) +{ + // Not implemented + return Status::NotFound; +} -constexpr uint16_t ActionsAttrAccess::ClusterRevision; +Status ActionsDelegateImpl::HandleStartAction(uint16_t actionId, Optional invokeId) +{ + // Not implemented + return Status::NotFound; +} + +Status ActionsDelegateImpl::HandleStartActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) +{ + // Not implemented + return Status::NotFound; +} -CHIP_ERROR ActionsAttrAccess::ReadActionListAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder) +Status ActionsDelegateImpl::HandleStopAction(uint16_t actionId, Optional invokeId) { - // Just return an empty list - return aEncoder.EncodeEmptyList(); + // Not implemented + return Status::NotFound; } -CHIP_ERROR ActionsAttrAccess::ReadEndpointListAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder) +Status ActionsDelegateImpl::HandlePauseAction(uint16_t actionId, Optional invokeId) { - // Just return an empty list - return aEncoder.EncodeEmptyList(); + // Not implemented + return Status::NotFound; } -CHIP_ERROR ActionsAttrAccess::ReadSetupUrlAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder) +Status ActionsDelegateImpl::HandlePauseActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) { - static const char SetupUrl[] = "https://example.com"; - return aEncoder.Encode(chip::Span(SetupUrl, strlen(SetupUrl))); + // Not implemented + return Status::NotFound; } -CHIP_ERROR ActionsAttrAccess::ReadClusterRevision(EndpointId endpoint, AttributeValueEncoder & aEncoder) +Status ActionsDelegateImpl::HandleResumeAction(uint16_t actionId, Optional invokeId) { - return aEncoder.Encode(ClusterRevision); + // Not implemented + return Status::NotFound; } -ActionsAttrAccess gAttrAccess; +Status ActionsDelegateImpl::HandleEnableAction(uint16_t actionId, Optional invokeId) +{ + // Not implemented + return Status::NotFound; +} -CHIP_ERROR ActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) +Status ActionsDelegateImpl::HandleEnableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) { - VerifyOrDie(aPath.mClusterId == Actions::Id); + // Not implemented + return Status::NotFound; +} - switch (aPath.mAttributeId) - { - case ActionList::Id: - return ReadActionListAttribute(aPath.mEndpointId, aEncoder); - case EndpointLists::Id: - return ReadEndpointListAttribute(aPath.mEndpointId, aEncoder); - case SetupURL::Id: - return ReadSetupUrlAttribute(aPath.mEndpointId, aEncoder); - case ClusterRevision::Id: - return ReadClusterRevision(aPath.mEndpointId, aEncoder); - default: - break; - } - return CHIP_NO_ERROR; +Status ActionsDelegateImpl::HandleDisableAction(uint16_t actionId, Optional invokeId) +{ + // Not implemented + return Status::NotFound; } -} // anonymous namespace -void MatterActionsPluginServerInitCallback() +Status ActionsDelegateImpl::HandleDisableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) { - AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); + // Not implemented + return Status::NotFound; } diff --git a/src/app/clusters/actions-server/actions-server.cpp b/src/app/clusters/actions-server/actions-server.cpp new file mode 100644 index 00000000000000..3e25d19c3a099d --- /dev/null +++ b/src/app/clusters/actions-server/actions-server.cpp @@ -0,0 +1,330 @@ +/* + * + * Copyright (c) 2024 Project CHIP Authors + * + * 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 "actions-server.h" +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace chip; +using namespace chip::app; +using namespace chip::app::Clusters; +using namespace chip::app::Clusters::Actions; +using namespace chip::app::Clusters::Actions::Attributes; + +Instance Instance::instance; +Instance * Instance::GetInstance() +{ + return &instance; +} + +CHIP_ERROR Instance::ReadActionListAttribute(const AttributeValueEncoder::ListEncodeHelper & encoder) +{ + if (GetInstance()->mDelegate == nullptr) + { + ChipLogError(Zcl, "Actions delegate is null!!!"); + return CHIP_ERROR_INCORRECT_STATE; + } + for (uint16_t i = 0; true; i++) + { + Actions::Structs::ActionStruct::Type action; + + CHIP_ERROR err = GetInstance()->mDelegate->ReadActionAtIndex(i, action); + if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED) + { + return CHIP_NO_ERROR; + } + + ReturnErrorOnFailure(encoder.Encode(action)); + } + return CHIP_NO_ERROR; +} + +CHIP_ERROR Instance::ReadEndpointListAttribute(const AttributeValueEncoder::ListEncodeHelper & encoder) +{ + if (GetInstance()->mDelegate == nullptr) + { + ChipLogError(Zcl, "Actions delegate is null!!!"); + return CHIP_ERROR_INCORRECT_STATE; + } + for (uint16_t i = 0; true; i++) + { + Actions::Structs::EndpointListStruct::Type epList; + + CHIP_ERROR err = GetInstance()->mDelegate->ReadEndpointListAtIndex(i, epList); + if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED) + { + return CHIP_NO_ERROR; + } + + ReturnErrorOnFailure(encoder.Encode(epList)); + } + return CHIP_NO_ERROR; +} + +void Instance::SetDefaultDelegate(Delegate * aDelegate) +{ + if (aDelegate == nullptr) + { + ChipLogError(Zcl, "Cannot set empty delegate!!!"); + return; + } + mDelegate = aDelegate; +} + +CHIP_ERROR Instance::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) +{ + VerifyOrDie(aPath.mClusterId == Actions::Id); + + switch (aPath.mAttributeId) + { + case ActionList::Id: { + Instance * d = this; + CHIP_ERROR err = + aEncoder.EncodeList([d](const auto & encoder) -> CHIP_ERROR { return d->ReadActionListAttribute(encoder); }); + return err; + } + case EndpointLists::Id: { + Instance * d = this; + CHIP_ERROR err = + aEncoder.EncodeList([d](const auto & encoder) -> CHIP_ERROR { return d->ReadEndpointListAttribute(encoder); }); + return err; + } + default: + break; + } + return CHIP_NO_ERROR; +} + +CHIP_ERROR Instance::FindActionIdInActionList(uint16_t actionId) +{ + if (GetInstance()->mDelegate == nullptr) + { + ChipLogError(Zcl, "Actions delegate is null!!!"); + return CHIP_ERROR_INCORRECT_STATE; + } + return GetInstance()->mDelegate->FindActionIdInActionList(actionId); +} + +template +void Instance::HandleCommand(HandlerContext & handlerContext, FuncT func) +{ + if (!handlerContext.mCommandHandled && (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; + } + + uint16_t actionId = requestPayload.actionID; + CHIP_ERROR err = GetInstance()->FindActionIdInActionList(actionId); + if (err != CHIP_NO_ERROR) + { + handlerContext.mCommandHandler.AddStatus(handlerContext.mRequestPath, Protocols::InteractionModel::Status::NotFound); + return; + } + + func(handlerContext, requestPayload); + } +} + +void Instance::HandleInstantAction(HandlerContext & ctx, const Commands::InstantAction::DecodableType & commandData) +{ + uint16_t actionId = commandData.actionID; + Optional invokeId = commandData.invokeID; + Status status = GetInstance()->mDelegate->HandleInstantAction(actionId, invokeId); + ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); +} + +void Instance::HandleInstantActionWithTransition(HandlerContext & ctx, + const Commands::InstantActionWithTransition::DecodableType & commandData) +{ + uint16_t actionId = commandData.actionID; + Optional invokeId = commandData.invokeID; + uint16_t transitionTime = commandData.transitionTime; + Status status = GetInstance()->mDelegate->HandleInstantActionWithTransition(actionId, transitionTime, invokeId); + ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); +} + +void Instance::HandleStartAction(HandlerContext & ctx, const Commands::StartAction::DecodableType & commandData) +{ + uint16_t actionId = commandData.actionID; + Optional invokeId = commandData.invokeID; + Status status = GetInstance()->mDelegate->HandleStartAction(actionId, invokeId); + ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); +} + +void Instance::HandleStartActionWithDuration(HandlerContext & ctx, + const Commands::StartActionWithDuration::DecodableType & commandData) +{ + uint16_t actionId = commandData.actionID; + Optional invokeId = commandData.invokeID; + uint32_t duration = commandData.duration; + Status status = GetInstance()->mDelegate->HandleStartActionWithDuration(actionId, duration, invokeId); + ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); +} + +void Instance::HandleStopAction(HandlerContext & ctx, const Commands::StopAction::DecodableType & commandData) +{ + uint16_t actionId = commandData.actionID; + Optional invokeId = commandData.invokeID; + Status status = GetInstance()->mDelegate->HandleStopAction(actionId, invokeId); + ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); +} + +void Instance::HandlePauseAction(HandlerContext & ctx, const Commands::PauseAction::DecodableType & commandData) +{ + uint16_t actionId = commandData.actionID; + Optional invokeId = commandData.invokeID; + Status status = GetInstance()->mDelegate->HandlePauseAction(actionId, invokeId); + ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); +} + +void Instance::HandlePauseActionWithDuration(HandlerContext & ctx, + const Commands::PauseActionWithDuration::DecodableType & commandData) +{ + uint16_t actionId = commandData.actionID; + Optional invokeId = commandData.invokeID; + uint32_t duration = commandData.duration; + Status status = GetInstance()->mDelegate->HandlePauseActionWithDuration(actionId, duration, invokeId); + ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); +} + +void Instance::HandleResumeAction(HandlerContext & ctx, const Commands::ResumeAction::DecodableType & commandData) +{ + uint16_t actionId = commandData.actionID; + Optional invokeId = commandData.invokeID; + Status status = GetInstance()->mDelegate->HandleResumeAction(actionId, invokeId); + ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); +} + +void Instance::HandleEnableAction(HandlerContext & ctx, const Commands::EnableAction::DecodableType & commandData) +{ + uint16_t actionId = commandData.actionID; + Optional invokeId = commandData.invokeID; + Status status = GetInstance()->mDelegate->HandleEnableAction(actionId, invokeId); + ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); +} + +void Instance::HandleEnableActionWithDuration(HandlerContext & ctx, + const Commands::EnableActionWithDuration::DecodableType & commandData) +{ + uint16_t actionId = commandData.actionID; + Optional invokeId = commandData.invokeID; + uint32_t duration = commandData.duration; + Status status = GetInstance()->mDelegate->HandleEnableActionWithDuration(actionId, duration, invokeId); + ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); +} + +void Instance::HandleDisableAction(HandlerContext & ctx, const Commands::DisableAction::DecodableType & commandData) +{ + uint16_t actionId = commandData.actionID; + Optional invokeId = commandData.invokeID; + Status status = GetInstance()->mDelegate->HandleDisableAction(actionId, invokeId); + ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); +} + +void Instance::HandleDisableActionWithDuration(HandlerContext & ctx, + const Commands::DisableActionWithDuration::DecodableType & commandData) +{ + uint16_t actionId = commandData.actionID; + Optional invokeId = commandData.invokeID; + uint32_t duration = commandData.duration; + Status status = GetInstance()->mDelegate->HandleDisableActionWithDuration(actionId, duration, invokeId); + ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); +} + +void Instance::InvokeCommand(HandlerContext & handlerContext) +{ + switch (handlerContext.mRequestPath.mCommandId) + { + case Actions::Commands::InstantAction::Id: + HandleCommand( + handlerContext, [this](HandlerContext & ctx, const auto & commandData) { HandleInstantAction(ctx, commandData); }); + return; + case Actions::Commands::InstantActionWithTransition::Id: + HandleCommand( + handlerContext, + [this](HandlerContext & ctx, const auto & commandData) { HandleInstantActionWithTransition(ctx, commandData); }); + return; + case Actions::Commands::StartAction::Id: + HandleCommand( + handlerContext, [this](HandlerContext & ctx, const auto & commandData) { HandleStartAction(ctx, commandData); }); + return; + case Actions::Commands::StartActionWithDuration::Id: + HandleCommand( + handlerContext, + [this](HandlerContext & ctx, const auto & commandData) { HandleStartActionWithDuration(ctx, commandData); }); + return; + case Actions::Commands::StopAction::Id: + HandleCommand( + handlerContext, [this](HandlerContext & ctx, const auto & commandData) { HandleStopAction(ctx, commandData); }); + return; + case Actions::Commands::PauseAction::Id: + HandleCommand( + handlerContext, [this](HandlerContext & ctx, const auto & commandData) { HandlePauseAction(ctx, commandData); }); + return; + case Actions::Commands::PauseActionWithDuration::Id: + HandleCommand( + handlerContext, + [this](HandlerContext & ctx, const auto & commandData) { HandlePauseActionWithDuration(ctx, commandData); }); + return; + case Actions::Commands::ResumeAction::Id: + HandleCommand( + handlerContext, [this](HandlerContext & ctx, const auto & commandData) { HandleResumeAction(ctx, commandData); }); + return; + case Actions::Commands::EnableAction::Id: + HandleCommand( + handlerContext, [this](HandlerContext & ctx, const auto & commandData) { HandleEnableAction(ctx, commandData); }); + return; + case Actions::Commands::EnableActionWithDuration::Id: + HandleCommand( + handlerContext, + [this](HandlerContext & ctx, const auto & commandData) { HandleEnableActionWithDuration(ctx, commandData); }); + return; + case Actions::Commands::DisableAction::Id: + HandleCommand( + handlerContext, [this](HandlerContext & ctx, const auto & commandData) { HandleDisableAction(ctx, commandData); }); + return; + case Actions::Commands::DisableActionWithDuration::Id: + HandleCommand( + handlerContext, + [this](HandlerContext & ctx, const auto & commandData) { HandleDisableActionWithDuration(ctx, commandData); }); + return; + } +} +void MatterActionsPluginServerInitCallback() +{ + AttributeAccessInterfaceRegistry::Instance().Register(Instance::GetInstance()); + CommandHandlerInterfaceRegistry::Instance().RegisterCommandHandler(Instance::GetInstance()); +} diff --git a/src/app/clusters/actions-server/actions-server.h b/src/app/clusters/actions-server/actions-server.h new file mode 100644 index 00000000000000..7c7a4f6d80cbe9 --- /dev/null +++ b/src/app/clusters/actions-server/actions-server.h @@ -0,0 +1,108 @@ +/* + * + * Copyright (c) 2024 Project CHIP Authors + * + * 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 +#include + +using namespace chip; +using namespace chip::app; +using namespace chip::app::Clusters; +using namespace chip::Protocols::InteractionModel; + +namespace chip { +namespace app { +namespace Clusters { +namespace Actions { + +class Delegate; + +class Instance : public AttributeAccessInterface, public CommandHandlerInterface +{ +public: + // Register for the Actions cluster on all endpoints. + Instance() : + AttributeAccessInterface(Optional::Missing(), Actions::Id), + CommandHandlerInterface(Optional::Missing(), Actions::Id) + {} + + static Instance * GetInstance(); + void SetDefaultDelegate(Delegate * aDelegate); + +private: + Delegate * mDelegate; + static Instance instance; + + CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; + + CHIP_ERROR ReadActionListAttribute(const AttributeValueEncoder::ListEncodeHelper & encoder); + CHIP_ERROR ReadEndpointListAttribute(const AttributeValueEncoder::ListEncodeHelper & encoder); + CHIP_ERROR FindActionIdInActionList(uint16_t actionId); + + // CommandHandlerInterface + template + void HandleCommand(HandlerContext & handlerContext, FuncT func); + + void InvokeCommand(HandlerContext & handlerContext) override; + + void HandleInstantAction(HandlerContext & ctx, const Commands::InstantAction::DecodableType & commandData); + void HandleInstantActionWithTransition(HandlerContext & ctx, + const Commands::InstantActionWithTransition::DecodableType & commandData); + void HandleStartAction(HandlerContext & ctx, const Commands::StartAction::DecodableType & commandData); + void HandleStartActionWithDuration(HandlerContext & ctx, const Commands::StartActionWithDuration::DecodableType & commandData); + void HandleStopAction(HandlerContext & ctx, const Commands::StopAction::DecodableType & commandData); + void HandlePauseAction(HandlerContext & ctx, const Commands::PauseAction::DecodableType & commandData); + void HandlePauseActionWithDuration(HandlerContext & ctx, const Commands::PauseActionWithDuration::DecodableType & commandData); + void HandleResumeAction(HandlerContext & ctx, const Commands::ResumeAction::DecodableType & commandData); + void HandleEnableAction(HandlerContext & ctx, const Commands::EnableAction::DecodableType & commandData); + void HandleEnableActionWithDuration(HandlerContext & ctx, + const Commands::EnableActionWithDuration::DecodableType & commandData); + void HandleDisableAction(HandlerContext & ctx, const Commands::DisableAction::DecodableType & commandData); + void HandleDisableActionWithDuration(HandlerContext & ctx, + const Commands::DisableActionWithDuration::DecodableType & commandData); +}; + +class Delegate +{ +public: + virtual ~Delegate() = default; + + virtual CHIP_ERROR ReadActionAtIndex(uint16_t index, Structs::ActionStruct::Type & action); + virtual CHIP_ERROR ReadEndpointListAtIndex(uint16_t index, Structs::EndpointListStruct::Type & epList); + virtual CHIP_ERROR FindActionIdInActionList(uint16_t actionId); + + virtual Status HandleInstantAction(uint16_t actionId, Optional invokeId); + virtual Status HandleInstantActionWithTransition(uint16_t actionId, uint16_t transitionTime, Optional invokeId); + virtual Status HandleStartAction(uint16_t actionId, Optional invokeId); + virtual Status HandleStartActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId); + virtual Status HandleStopAction(uint16_t actionId, Optional invokeId); + virtual Status HandlePauseAction(uint16_t actionId, Optional invokeId); + virtual Status HandlePauseActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId); + virtual Status HandleResumeAction(uint16_t actionId, Optional invokeId); + virtual Status HandleEnableAction(uint16_t actionId, Optional invokeId); + virtual Status HandleEnableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId); + virtual Status HandleDisableAction(uint16_t actionId, Optional invokeId); + virtual Status HandleDisableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId); +}; + +} // namespace Actions +} // namespace Clusters +} // namespace app +} // namespace chip From 5240da97eb0ad00cef83d21c895fe558aebe5215 Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Fri, 13 Sep 2024 11:36:42 +0530 Subject: [PATCH 03/25] Example changes --- examples/all-clusters-app/esp32/main/main.cpp | 4 +++- examples/bridge-app/esp32/main/main.cpp | 8 -------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/examples/all-clusters-app/esp32/main/main.cpp b/examples/all-clusters-app/esp32/main/main.cpp index 7c96990865556d..485dc7cf30a641 100644 --- a/examples/all-clusters-app/esp32/main/main.cpp +++ b/examples/all-clusters-app/esp32/main/main.cpp @@ -35,6 +35,7 @@ #include "shell_extension/launch.h" #include #include +#include #include #include #include @@ -96,6 +97,7 @@ AppCallbacks sCallbacks; app::Clusters::TemperatureControl::AppSupportedTemperatureLevelsDelegate sAppSupportedTemperatureLevelsDelegate; app::Clusters::ModeSelect::StaticSupportedModesManager sStaticSupportedModesManager; +app::Clusters::Actions::ActionsDelegateImpl sActionsDelegateImpl; constexpr EndpointId kNetworkCommissioningEndpointSecondary = 0xFFFE; @@ -126,9 +128,9 @@ static void InitServer(intptr_t context) app::Clusters::TemperatureControl::SetInstance(&sAppSupportedTemperatureLevelsDelegate); app::Clusters::ModeSelect::setSupportedModesManager(&sStaticSupportedModesManager); + app::Clusters::Actions::Instance::GetInstance()->SetDefaultDelegate(&sActionsDelegateImpl); } -// #include #include #include diff --git a/examples/bridge-app/esp32/main/main.cpp b/examples/bridge-app/esp32/main/main.cpp index ae56a1d8b53d39..4efb9e4acba57c 100644 --- a/examples/bridge-app/esp32/main/main.cpp +++ b/examples/bridge-app/esp32/main/main.cpp @@ -349,14 +349,6 @@ void HandleDeviceStatusChanged(Device * dev, Device::Changed_t itemChangedMask) } } -bool emberAfActionsClusterInstantActionCallback(app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath, - const Actions::Commands::InstantAction::DecodableType & commandData) -{ - // No actions are implemented, just return status NotFound. - commandObj->AddStatus(commandPath, Protocols::InteractionModel::Status::NotFound); - return true; -} - const EmberAfDeviceType gRootDeviceTypes[] = { { DEVICE_TYPE_ROOT_NODE, DEVICE_VERSION_DEFAULT } }; const EmberAfDeviceType gAggregateNodeDeviceTypes[] = { { DEVICE_TYPE_BRIDGE, DEVICE_VERSION_DEFAULT } }; From 5a4459ce0a862dcd62ef720ec57148a3ad8ecc73 Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Tue, 17 Sep 2024 11:38:52 +0530 Subject: [PATCH 04/25] Add documentation and fix pipeline --- .../clusters/actions-server/actions-server.h | 136 ++++++++++++++++++ src/app/zap_cluster_list.json | 3 +- 2 files changed, 138 insertions(+), 1 deletion(-) diff --git a/src/app/clusters/actions-server/actions-server.h b/src/app/clusters/actions-server/actions-server.h index 7c7a4f6d80cbe9..657bd872f69d61 100644 --- a/src/app/clusters/actions-server/actions-server.h +++ b/src/app/clusters/actions-server/actions-server.h @@ -84,21 +84,157 @@ class Delegate public: virtual ~Delegate() = default; + /** + * Get the action at the Nth index from list of actions. + * @param index The index of the action to be returned. It is assumed that actions are indexable from 0 and with no gaps. + * @param action A reference to the action struct which copies existing and initialised buffer at index. + * @return Returns a CHIP_NO_ERROR if there was no error and the action was returned successfully. + * CHIP_ERROR_PROVIDER_LIST_EXHAUSTED if the index in beyond the list of available actions. + */ virtual CHIP_ERROR ReadActionAtIndex(uint16_t index, Structs::ActionStruct::Type & action); + + /** + * Get the EndpointList at the Nth index from list of endpointList. + * @param index The index of the endpointList to be returned. It is assumed that actions are indexable from 0 and with no gaps. + * @param epList A reference to the endpointList struct which copies existing and initialised buffer at index. + * @return Returns a CHIP_NO_ERROR if there was no error and the epList was returned successfully. + * CHIP_ERROR_PROVIDER_LIST_EXHAUSTED if the index in beyond the list of available endpointList. + */ virtual CHIP_ERROR ReadEndpointListAtIndex(uint16_t index, Structs::EndpointListStruct::Type & epList); + + /** + * Find the action with matching actionId in the list of action. + * @param actionId The action to be find in the list of action. + * @return Returns a CHIP_NO_ERROR if matching action is found. + * CHIP_ERROR_NOT_FOUND if the matching action does not found in the list of action. + */ virtual CHIP_ERROR FindActionIdInActionList(uint16_t actionId); + /** + * On reciept of each and every command, + * if the InvokeID data field is provided by the client when invoking a command, the server SHALL generate a StateChanged event + * when the action changes to a new state or an ActionFailed event when execution of the action fails. + * + * @return If the command refers to an action which currently is not in a state where the command applies, a response SHALL be + * generated with the StatusCode INVALID_COMMAND. + */ + + /** + * When an InstantAction command is recieved, an action (state change) on the involved endpoints shall trigger, + * in a "fire and forget" manner. Afterwards, the action’s state SHALL be Inactive. + * + * @param actionId The id of an action on which an action shall takes place. + * @return Returns a Success if an action took place successfully otherwise, suitable error. + */ virtual Status HandleInstantAction(uint16_t actionId, Optional invokeId); + + /** + * When an InstantActionWithTransition command is recieved, an action (state change) on the involved endpoints shall trigger, + * with a specified time to transition from the current state to the new state. During the transition, the action’s state SHALL + * be Active. Afterwards, the action’s state SHALL be Inactive. + * + * @param actionId The id of an action on which an action shall takes place. + * @param transitionTime The time for transition from the current state to the new state. + * @return Returns a Success if an action took place successfully otherwise, suitable error. + */ virtual Status HandleInstantActionWithTransition(uint16_t actionId, uint16_t transitionTime, Optional invokeId); + + /** + * When a StartAction command is recieved, the commencement of an action on the involved endpoints shall trigger. Afterwards, + * the action’s state SHALL be Inactive. + * + * @param actionId The id of an action on which an action shall takes place. + * @return Returns a Success if an action took place successfully otherwise, suitable error. + */ virtual Status HandleStartAction(uint16_t actionId, Optional invokeId); + + /** + * When a StartActionWithDuration command is recieved, the commencement of an action on the involved endpoints shall trigger, + * and SHALL change the action’s state to Active. After the specified Duration, the action will stop, and the action’s state + * SHALL change to Inactive. + * + * @param actionId The id of an action on which an action shall takes place. + * @param duration The time for which an action shall be in start state. + * @return Returns a Success if an action took place successfully otherwise, suitable error. + */ virtual Status HandleStartActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId); + + /** + * When a StopAction command is recieved, the ongoing action on the involved endpoints shall stop. Afterwards, the action’s + * state SHALL be Inactive. + * + * @param actionId The id of an action on which an action shall takes place. + * @return Returns a Success if an action took place successfully otherwise, suitable error. + */ virtual Status HandleStopAction(uint16_t actionId, Optional invokeId); + + /** + * When a PauseAction command is recieved, the ongoing action on the involved endpoints shall pause and SHALL change the + * action’s state to Paused. + * + * @param actionId The id of an action on which an action shall takes place. + * @return Returns a Success if an action took place successfully otherwise, suitable error. + */ virtual Status HandlePauseAction(uint16_t actionId, Optional invokeId); + + /** + * When a PauseActionWithDuration command is recieved, pauses an ongoing action, and SHALL change the action’s state to Paused. + * After the specified Duration, the ongoing action will be automatically resumed. which SHALL change the action’s state to + * Active. + * + * @param actionId The id of an action on which an action shall takes place. + * @param duration The time for which an action shall be in pause state. + * @return Returns a Success if an action took place successfully otherwise, suitable error. + */ virtual Status HandlePauseActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId); + + /** + * When a ResumeAction command is recieved, the previously paused action shall resume and SHALL change the action’s state to + * Active. + * + * @param actionId The id of an action on which an action shall takes place. + * @return Returns a Success if an action took place successfully otherwise, suitable error. + */ virtual Status HandleResumeAction(uint16_t actionId, Optional invokeId); + + /** + * When an EnableAction command is recieved, it enables a certain action or automation. Afterwards, the action’s state SHALL be + * Active. + * + * @param actionId The id of an action on which an action shall takes place. + * @return Returns a Success if an action took place successfully otherwise, suitable error. + */ virtual Status HandleEnableAction(uint16_t actionId, Optional invokeId); + + /** + * When an EnableActionWithDuration command is recieved, it enables a certain action or automation, and SHALL change the + * action’s state to be Active. After the specified Duration, the action or automation will stop, and the action’s state SHALL + * change to Disabled. + * + * @param actionId The id of an action on which an action shall takes place. + * @param duration The time for which an action shall be in active state. + * @return Returns a Success if an action took place successfully otherwise, suitable error. + */ virtual Status HandleEnableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId); + + /** + * When a DisableAction command is recieved, it disables a certain action or automation, and SHALL change the action’s state to + * Inactive. + * + * @param actionId The id of an action on which an action shall takes place. + * @return Returns a Success if an action took place successfully otherwise, suitable error. + */ virtual Status HandleDisableAction(uint16_t actionId, Optional invokeId); + + /** + * When a DisableActionWithDuration command is recieved, it disables a certain action or automation, and SHALL change the + * action’s state to Disabled. After the specified Duration, the action or automation will re-start, and the action’s state + * SHALL change to either Inactive or Active, depending on the actions. + * + * @param actionId The id of an action on which an action shall takes place. + * @param duration The time for which an action shall be in disable state. + * @return Returns a Success if an action took place successfully otherwise, suitable error. + */ virtual Status HandleDisableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId); }; diff --git a/src/app/zap_cluster_list.json b/src/app/zap_cluster_list.json index 0cbdba0d6e2c6e..bf9ad0e418d3fe 100644 --- a/src/app/zap_cluster_list.json +++ b/src/app/zap_cluster_list.json @@ -330,6 +330,7 @@ "WINDOW_COVERING_CLUSTER": ["window-covering-server"], "WATER_HEATER_MANAGEMENT_CLUSTER": ["water-heater-management-server"], "WATER_HEATER_MODE_CLUSTER": ["mode-base-server"], - "ZONE_MANAGEMENT_CLUSTER": [] + "ZONE_MANAGEMENT_CLUSTER": [], + "ACTIONS_CLUSTER": ["actions-server"] } } From e501d3a3043c9765e45f47949899d599a23b607f Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Tue, 17 Sep 2024 17:02:29 +0530 Subject: [PATCH 05/25] Some example changes --- .../include/bridged-actions-stub.h | 30 ++++++++----------- .../src/bridged-actions-stub.cpp | 1 + .../asr/src/bridged-actions-stub.cpp | 8 ++--- .../bridge-app/linux/bridged-actions-stub.cpp | 8 ++--- .../bridge-app/telink/src/DeviceCallbacks.cpp | 8 ++--- .../actions-server/actions-server.cpp | 1 + .../clusters/actions-server/actions-server.h | 29 ++++++++---------- 7 files changed, 38 insertions(+), 47 deletions(-) diff --git a/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h b/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h index 2c38869f26802f..1366f404b98672 100644 --- a/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h +++ b/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h @@ -25,12 +25,6 @@ #include #include -using namespace chip; -using namespace chip::app; -using namespace chip::app::Clusters; -using namespace chip::app::Clusters::Actions; -using namespace chip::app::Clusters::Actions::Attributes; - namespace chip { namespace app { namespace Clusters { @@ -91,18 +85,18 @@ class ActionsDelegateImpl : public Delegate CHIP_ERROR ReadEndpointListAtIndex(uint16_t index, EndpointListStructType & epList) override; CHIP_ERROR FindActionIdInActionList(uint16_t actionId) override; - Status HandleInstantAction(uint16_t actionId, Optional invokeId) override; - Status HandleInstantActionWithTransition(uint16_t actionId, uint16_t transitionTime, Optional invokeId) override; - Status HandleStartAction(uint16_t actionId, Optional invokeId) override; - Status HandleStartActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) override; - Status HandleStopAction(uint16_t actionId, Optional invokeId) override; - Status HandlePauseAction(uint16_t actionId, Optional invokeId) override; - Status HandlePauseActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) override; - Status HandleResumeAction(uint16_t actionId, Optional invokeId) override; - Status HandleEnableAction(uint16_t actionId, Optional invokeId) override; - Status HandleEnableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) override; - Status HandleDisableAction(uint16_t actionId, Optional invokeId) override; - Status HandleDisableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) override; + Protocols::InteractionModel::Status HandleInstantAction(uint16_t actionId, Optional invokeId) override; + Protocols::InteractionModel::Status HandleInstantActionWithTransition(uint16_t actionId, uint16_t transitionTime, Optional invokeId) override; + Protocols::InteractionModel::Status HandleStartAction(uint16_t actionId, Optional invokeId) override; + Protocols::InteractionModel::Status HandleStartActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) override; + Protocols::InteractionModel::Status HandleStopAction(uint16_t actionId, Optional invokeId) override; + Protocols::InteractionModel::Status HandlePauseAction(uint16_t actionId, Optional invokeId) override; + Protocols::InteractionModel::Status HandlePauseActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) override; + Protocols::InteractionModel::Status HandleResumeAction(uint16_t actionId, Optional invokeId) override; + Protocols::InteractionModel::Status HandleEnableAction(uint16_t actionId, Optional invokeId) override; + Protocols::InteractionModel::Status HandleEnableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) override; + Protocols::InteractionModel::Status HandleDisableAction(uint16_t actionId, Optional invokeId) override; + Protocols::InteractionModel::Status HandleDisableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) override; }; } // namespace Actions } // namespace Clusters diff --git a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp index ab2670f3aa2c8f..bd9a588f6047e4 100644 --- a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp +++ b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp @@ -22,6 +22,7 @@ using namespace chip::app; using namespace chip::app::Clusters; using namespace chip::app::Clusters::Actions; using namespace chip::app::Clusters::Actions::Attributes; +using namespace chip::Protocols::InteractionModel; CHIP_ERROR ActionsDelegateImpl::ReadActionAtIndex(uint16_t index, ActionListStructType & action) { diff --git a/examples/bridge-app/asr/src/bridged-actions-stub.cpp b/examples/bridge-app/asr/src/bridged-actions-stub.cpp index 73b7e8dd9877a0..d6289a733f2259 100755 --- a/examples/bridge-app/asr/src/bridged-actions-stub.cpp +++ b/examples/bridge-app/asr/src/bridged-actions-stub.cpp @@ -95,7 +95,7 @@ CHIP_ERROR ActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, Attr } } // anonymous namespace -void MatterActionsPluginServerInitCallback(void) -{ - AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); -} +//void MatterActionsPluginServerInitCallback(void) +//{ +// AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); +//} diff --git a/examples/bridge-app/linux/bridged-actions-stub.cpp b/examples/bridge-app/linux/bridged-actions-stub.cpp index 580f4f2239bd1a..cca8d38d809c94 100644 --- a/examples/bridge-app/linux/bridged-actions-stub.cpp +++ b/examples/bridge-app/linux/bridged-actions-stub.cpp @@ -131,7 +131,7 @@ CHIP_ERROR ActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, Attr } } // anonymous namespace -void MatterActionsPluginServerInitCallback() -{ - AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); -} +//void MatterActionsPluginServerInitCallback() +//{ +// AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); +//} diff --git a/examples/bridge-app/telink/src/DeviceCallbacks.cpp b/examples/bridge-app/telink/src/DeviceCallbacks.cpp index 9e3273e7107472..65218c68295bba 100644 --- a/examples/bridge-app/telink/src/DeviceCallbacks.cpp +++ b/examples/bridge-app/telink/src/DeviceCallbacks.cpp @@ -99,7 +99,7 @@ CHIP_ERROR ActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, Attr } } // anonymous namespace -void MatterActionsPluginServerInitCallback(void) -{ - AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); -} +//void MatterActionsPluginServerInitCallback(void) +//{ +// AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); +//} diff --git a/src/app/clusters/actions-server/actions-server.cpp b/src/app/clusters/actions-server/actions-server.cpp index 3e25d19c3a099d..023cbcf9c1a0cd 100644 --- a/src/app/clusters/actions-server/actions-server.cpp +++ b/src/app/clusters/actions-server/actions-server.cpp @@ -30,6 +30,7 @@ using namespace chip::app; using namespace chip::app::Clusters; using namespace chip::app::Clusters::Actions; using namespace chip::app::Clusters::Actions::Attributes; +using namespace chip::Protocols::InteractionModel; Instance Instance::instance; Instance * Instance::GetInstance() diff --git a/src/app/clusters/actions-server/actions-server.h b/src/app/clusters/actions-server/actions-server.h index 657bd872f69d61..e4d65a803e8a8e 100644 --- a/src/app/clusters/actions-server/actions-server.h +++ b/src/app/clusters/actions-server/actions-server.h @@ -22,11 +22,6 @@ #include #include -using namespace chip; -using namespace chip::app; -using namespace chip::app::Clusters; -using namespace chip::Protocols::InteractionModel; - namespace chip { namespace app { namespace Clusters { @@ -126,7 +121,7 @@ class Delegate * @param actionId The id of an action on which an action shall takes place. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Status HandleInstantAction(uint16_t actionId, Optional invokeId); + virtual Protocols::InteractionModel::Status HandleInstantAction(uint16_t actionId, Optional invokeId); /** * When an InstantActionWithTransition command is recieved, an action (state change) on the involved endpoints shall trigger, @@ -137,7 +132,7 @@ class Delegate * @param transitionTime The time for transition from the current state to the new state. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Status HandleInstantActionWithTransition(uint16_t actionId, uint16_t transitionTime, Optional invokeId); + virtual Protocols::InteractionModel::Status HandleInstantActionWithTransition(uint16_t actionId, uint16_t transitionTime, Optional invokeId); /** * When a StartAction command is recieved, the commencement of an action on the involved endpoints shall trigger. Afterwards, @@ -146,7 +141,7 @@ class Delegate * @param actionId The id of an action on which an action shall takes place. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Status HandleStartAction(uint16_t actionId, Optional invokeId); + virtual Protocols::InteractionModel::Status HandleStartAction(uint16_t actionId, Optional invokeId); /** * When a StartActionWithDuration command is recieved, the commencement of an action on the involved endpoints shall trigger, @@ -157,7 +152,7 @@ class Delegate * @param duration The time for which an action shall be in start state. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Status HandleStartActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId); + virtual Protocols::InteractionModel::Status HandleStartActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId); /** * When a StopAction command is recieved, the ongoing action on the involved endpoints shall stop. Afterwards, the action’s @@ -166,7 +161,7 @@ class Delegate * @param actionId The id of an action on which an action shall takes place. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Status HandleStopAction(uint16_t actionId, Optional invokeId); + virtual Protocols::InteractionModel::Status HandleStopAction(uint16_t actionId, Optional invokeId); /** * When a PauseAction command is recieved, the ongoing action on the involved endpoints shall pause and SHALL change the @@ -175,7 +170,7 @@ class Delegate * @param actionId The id of an action on which an action shall takes place. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Status HandlePauseAction(uint16_t actionId, Optional invokeId); + virtual Protocols::InteractionModel::Status HandlePauseAction(uint16_t actionId, Optional invokeId); /** * When a PauseActionWithDuration command is recieved, pauses an ongoing action, and SHALL change the action’s state to Paused. @@ -186,7 +181,7 @@ class Delegate * @param duration The time for which an action shall be in pause state. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Status HandlePauseActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId); + virtual Protocols::InteractionModel::Status HandlePauseActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId); /** * When a ResumeAction command is recieved, the previously paused action shall resume and SHALL change the action’s state to @@ -195,7 +190,7 @@ class Delegate * @param actionId The id of an action on which an action shall takes place. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Status HandleResumeAction(uint16_t actionId, Optional invokeId); + virtual Protocols::InteractionModel::Status HandleResumeAction(uint16_t actionId, Optional invokeId); /** * When an EnableAction command is recieved, it enables a certain action or automation. Afterwards, the action’s state SHALL be @@ -204,7 +199,7 @@ class Delegate * @param actionId The id of an action on which an action shall takes place. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Status HandleEnableAction(uint16_t actionId, Optional invokeId); + virtual Protocols::InteractionModel::Status HandleEnableAction(uint16_t actionId, Optional invokeId); /** * When an EnableActionWithDuration command is recieved, it enables a certain action or automation, and SHALL change the @@ -215,7 +210,7 @@ class Delegate * @param duration The time for which an action shall be in active state. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Status HandleEnableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId); + virtual Protocols::InteractionModel::Status HandleEnableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId); /** * When a DisableAction command is recieved, it disables a certain action or automation, and SHALL change the action’s state to @@ -224,7 +219,7 @@ class Delegate * @param actionId The id of an action on which an action shall takes place. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Status HandleDisableAction(uint16_t actionId, Optional invokeId); + virtual Protocols::InteractionModel::Status HandleDisableAction(uint16_t actionId, Optional invokeId); /** * When a DisableActionWithDuration command is recieved, it disables a certain action or automation, and SHALL change the @@ -235,7 +230,7 @@ class Delegate * @param duration The time for which an action shall be in disable state. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Status HandleDisableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId); + virtual Protocols::InteractionModel::Status HandleDisableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId); }; } // namespace Actions From 8c065fd3b6121a7ea5026b9c0a8b19221e79964a Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Tue, 24 Sep 2024 17:00:52 +0530 Subject: [PATCH 06/25] Add comment about scope of name --- src/app/clusters/actions-server/actions-server.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/app/clusters/actions-server/actions-server.h b/src/app/clusters/actions-server/actions-server.h index e4d65a803e8a8e..03bdb38754e08f 100644 --- a/src/app/clusters/actions-server/actions-server.h +++ b/src/app/clusters/actions-server/actions-server.h @@ -82,7 +82,10 @@ class Delegate /** * Get the action at the Nth index from list of actions. * @param index The index of the action to be returned. It is assumed that actions are indexable from 0 and with no gaps. - * @param action A reference to the action struct which copies existing and initialised buffer at index. + * @param action A reference to the action struct which copies existing initialised buffer at index. + * Name field (CharSpan) of ActionStruct contails the name of an action. + * The underlying data must remain allocated throughout the lifetime of the device, + * as the API does not make a copy. * @return Returns a CHIP_NO_ERROR if there was no error and the action was returned successfully. * CHIP_ERROR_PROVIDER_LIST_EXHAUSTED if the index in beyond the list of available actions. */ @@ -91,7 +94,10 @@ class Delegate /** * Get the EndpointList at the Nth index from list of endpointList. * @param index The index of the endpointList to be returned. It is assumed that actions are indexable from 0 and with no gaps. - * @param epList A reference to the endpointList struct which copies existing and initialised buffer at index. + * @param epList A reference to the endpointList struct which copies existing initialised buffer at index. + * Name field (CharSpan) of EndpointList contails the name of an endpointList. + * The underlying data must remain allocated throughout the lifetime of the device, + * as the API does not make a copy. * @return Returns a CHIP_NO_ERROR if there was no error and the epList was returned successfully. * CHIP_ERROR_PROVIDER_LIST_EXHAUSTED if the index in beyond the list of available endpointList. */ From 9f5ecff0e921c84f96cb585dbdecd01a9433e23f Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Tue, 24 Sep 2024 11:31:23 +0000 Subject: [PATCH 07/25] Restyled by clang-format --- .../include/bridged-actions-stub.h | 15 ++++++++++----- .../bridge-app/asr/src/bridged-actions-stub.cpp | 6 +++--- .../bridge-app/linux/bridged-actions-stub.cpp | 6 +++--- .../bridge-app/telink/src/DeviceCallbacks.cpp | 6 +++--- .../linux/src/bridged-actions-stub.cpp | 8 ++++---- src/app/clusters/actions-server/actions-server.h | 15 ++++++++++----- 6 files changed, 33 insertions(+), 23 deletions(-) mode change 100755 => 100644 examples/bridge-app/asr/src/bridged-actions-stub.cpp diff --git a/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h b/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h index 1366f404b98672..6c1a5c65fd8f20 100644 --- a/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h +++ b/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h @@ -86,17 +86,22 @@ class ActionsDelegateImpl : public Delegate CHIP_ERROR FindActionIdInActionList(uint16_t actionId) override; Protocols::InteractionModel::Status HandleInstantAction(uint16_t actionId, Optional invokeId) override; - Protocols::InteractionModel::Status HandleInstantActionWithTransition(uint16_t actionId, uint16_t transitionTime, Optional invokeId) override; + Protocols::InteractionModel::Status HandleInstantActionWithTransition(uint16_t actionId, uint16_t transitionTime, + Optional invokeId) override; Protocols::InteractionModel::Status HandleStartAction(uint16_t actionId, Optional invokeId) override; - Protocols::InteractionModel::Status HandleStartActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) override; + Protocols::InteractionModel::Status HandleStartActionWithDuration(uint16_t actionId, uint32_t duration, + Optional invokeId) override; Protocols::InteractionModel::Status HandleStopAction(uint16_t actionId, Optional invokeId) override; Protocols::InteractionModel::Status HandlePauseAction(uint16_t actionId, Optional invokeId) override; - Protocols::InteractionModel::Status HandlePauseActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) override; + Protocols::InteractionModel::Status HandlePauseActionWithDuration(uint16_t actionId, uint32_t duration, + Optional invokeId) override; Protocols::InteractionModel::Status HandleResumeAction(uint16_t actionId, Optional invokeId) override; Protocols::InteractionModel::Status HandleEnableAction(uint16_t actionId, Optional invokeId) override; - Protocols::InteractionModel::Status HandleEnableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) override; + Protocols::InteractionModel::Status HandleEnableActionWithDuration(uint16_t actionId, uint32_t duration, + Optional invokeId) override; Protocols::InteractionModel::Status HandleDisableAction(uint16_t actionId, Optional invokeId) override; - Protocols::InteractionModel::Status HandleDisableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) override; + Protocols::InteractionModel::Status HandleDisableActionWithDuration(uint16_t actionId, uint32_t duration, + Optional invokeId) override; }; } // namespace Actions } // namespace Clusters diff --git a/examples/bridge-app/asr/src/bridged-actions-stub.cpp b/examples/bridge-app/asr/src/bridged-actions-stub.cpp old mode 100755 new mode 100644 index d6289a733f2259..84dca65f832107 --- a/examples/bridge-app/asr/src/bridged-actions-stub.cpp +++ b/examples/bridge-app/asr/src/bridged-actions-stub.cpp @@ -95,7 +95,7 @@ CHIP_ERROR ActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, Attr } } // anonymous namespace -//void MatterActionsPluginServerInitCallback(void) +// void MatterActionsPluginServerInitCallback(void) //{ -// AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); -//} +// AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); +// } diff --git a/examples/bridge-app/linux/bridged-actions-stub.cpp b/examples/bridge-app/linux/bridged-actions-stub.cpp index cca8d38d809c94..0eff1e7b6d05bc 100644 --- a/examples/bridge-app/linux/bridged-actions-stub.cpp +++ b/examples/bridge-app/linux/bridged-actions-stub.cpp @@ -131,7 +131,7 @@ CHIP_ERROR ActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, Attr } } // anonymous namespace -//void MatterActionsPluginServerInitCallback() +// void MatterActionsPluginServerInitCallback() //{ -// AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); -//} +// AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); +// } diff --git a/examples/bridge-app/telink/src/DeviceCallbacks.cpp b/examples/bridge-app/telink/src/DeviceCallbacks.cpp index 65218c68295bba..8511d7047dfb0a 100644 --- a/examples/bridge-app/telink/src/DeviceCallbacks.cpp +++ b/examples/bridge-app/telink/src/DeviceCallbacks.cpp @@ -99,7 +99,7 @@ CHIP_ERROR ActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, Attr } } // anonymous namespace -//void MatterActionsPluginServerInitCallback(void) +// void MatterActionsPluginServerInitCallback(void) //{ -// AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); -//} +// AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); +// } diff --git a/examples/placeholder/linux/src/bridged-actions-stub.cpp b/examples/placeholder/linux/src/bridged-actions-stub.cpp index d7abf17cd9106e..ad934cc881077c 100644 --- a/examples/placeholder/linux/src/bridged-actions-stub.cpp +++ b/examples/placeholder/linux/src/bridged-actions-stub.cpp @@ -96,7 +96,7 @@ CHIP_ERROR ActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, Attr } } // anonymous namespace -void MatterActionsPluginServerInitCallback(void) -{ - chip::app::AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); -} +// void MatterActionsPluginServerInitCallback(void) +//{ +// chip::app::AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); +// } diff --git a/src/app/clusters/actions-server/actions-server.h b/src/app/clusters/actions-server/actions-server.h index 03bdb38754e08f..f702069cfa68b7 100644 --- a/src/app/clusters/actions-server/actions-server.h +++ b/src/app/clusters/actions-server/actions-server.h @@ -138,7 +138,8 @@ class Delegate * @param transitionTime The time for transition from the current state to the new state. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Protocols::InteractionModel::Status HandleInstantActionWithTransition(uint16_t actionId, uint16_t transitionTime, Optional invokeId); + virtual Protocols::InteractionModel::Status HandleInstantActionWithTransition(uint16_t actionId, uint16_t transitionTime, + Optional invokeId); /** * When a StartAction command is recieved, the commencement of an action on the involved endpoints shall trigger. Afterwards, @@ -158,7 +159,8 @@ class Delegate * @param duration The time for which an action shall be in start state. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Protocols::InteractionModel::Status HandleStartActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId); + virtual Protocols::InteractionModel::Status HandleStartActionWithDuration(uint16_t actionId, uint32_t duration, + Optional invokeId); /** * When a StopAction command is recieved, the ongoing action on the involved endpoints shall stop. Afterwards, the action’s @@ -187,7 +189,8 @@ class Delegate * @param duration The time for which an action shall be in pause state. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Protocols::InteractionModel::Status HandlePauseActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId); + virtual Protocols::InteractionModel::Status HandlePauseActionWithDuration(uint16_t actionId, uint32_t duration, + Optional invokeId); /** * When a ResumeAction command is recieved, the previously paused action shall resume and SHALL change the action’s state to @@ -216,7 +219,8 @@ class Delegate * @param duration The time for which an action shall be in active state. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Protocols::InteractionModel::Status HandleEnableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId); + virtual Protocols::InteractionModel::Status HandleEnableActionWithDuration(uint16_t actionId, uint32_t duration, + Optional invokeId); /** * When a DisableAction command is recieved, it disables a certain action or automation, and SHALL change the action’s state to @@ -236,7 +240,8 @@ class Delegate * @param duration The time for which an action shall be in disable state. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Protocols::InteractionModel::Status HandleDisableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId); + virtual Protocols::InteractionModel::Status HandleDisableActionWithDuration(uint16_t actionId, uint32_t duration, + Optional invokeId); }; } // namespace Actions From 24aa6f8353f0bd99c85c57cf4a3b49deaf1594e2 Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Fri, 4 Oct 2024 13:09:42 +0530 Subject: [PATCH 08/25] Add generic struct to copy the ActionStruct and EndpointListStruct along with name buffer and added events --- .../include/bridged-actions-stub.h | 55 +++------ .../src/bridged-actions-stub.cpp | 16 +-- .../actions-server/actions-server.cpp | 34 +++++- .../clusters/actions-server/actions-server.h | 115 ++++++++++++++++-- 4 files changed, 156 insertions(+), 64 deletions(-) diff --git a/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h b/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h index 6c1a5c65fd8f20..91c9999d7cf90d 100644 --- a/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h +++ b/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h @@ -34,30 +34,10 @@ const uint16_t kEndpointListSize = 3; class ActionsDelegateImpl : public Delegate { private: - using ActionListStructType = Structs::ActionStruct::Type; - using EndpointListStructType = Structs::EndpointListStruct::Type; - - const ActionListStructType kActionList[kActionListSize] = { - ActionListStructType{ .actionID = 0, - .name = CharSpan::fromCharString("TurnOnLight"), - .type = ActionTypeEnum::kScene, - .endpointListID = 0, - .supportedCommands = 0, - .state = ActionStateEnum::kInactive }, - - ActionListStructType{ .actionID = 1, - .name = CharSpan::fromCharString("TurnOffLight"), - .type = ActionTypeEnum::kScene, - .endpointListID = 1, - .supportedCommands = 0, - .state = ActionStateEnum::kInactive }, - - ActionListStructType{ .actionID = 2, - .name = CharSpan::fromCharString("ToggleLight"), - .type = ActionTypeEnum::kScene, - .endpointListID = 2, - .supportedCommands = 0, - .state = ActionStateEnum::kInactive }, + const GenericActionStruct kActionList[kActionListSize] = { + GenericActionStruct(0, CharSpan::fromCharString("TurnOnLight"), ActionTypeEnum::kScene, 0, 0, ActionStateEnum::kInactive), + GenericActionStruct(1, CharSpan::fromCharString("TurnOffLight"), ActionTypeEnum::kScene, 1, 0, ActionStateEnum::kInactive), + GenericActionStruct(2, CharSpan::fromCharString("ToggleLight"), ActionTypeEnum::kScene, 2, 0, ActionStateEnum::kInactive), }; // Dummy endpoint list. @@ -65,24 +45,17 @@ class ActionsDelegateImpl : public Delegate const EndpointId secondEpList[1] = { 0 }; const EndpointId thirdEpList[1] = { 0 }; - const EndpointListStructType kEndpointList[kEndpointListSize] = { - EndpointListStructType{ .endpointListID = 0, - .name = CharSpan::fromCharString("On"), - .type = EndpointListTypeEnum::kOther, - .endpoints = DataModel::List(firstEpList) }, - - EndpointListStructType{ .endpointListID = 1, - .name = CharSpan::fromCharString("Off"), - .type = EndpointListTypeEnum::kOther, - .endpoints = DataModel::List(secondEpList) }, - - EndpointListStructType{ .endpointListID = 2, - .name = CharSpan::fromCharString("Toggle"), - .type = EndpointListTypeEnum::kOther, - .endpoints = DataModel::List(thirdEpList) }, + const GenericEndpointList kEndpointList[kEndpointListSize] = { + GenericEndpointList(0, CharSpan::fromCharString("On"), EndpointListTypeEnum::kOther, + DataModel::List(firstEpList)), + GenericEndpointList(1, CharSpan::fromCharString("Off"), EndpointListTypeEnum::kOther, + DataModel::List(secondEpList)), + GenericEndpointList(2, CharSpan::fromCharString("Toggle"), EndpointListTypeEnum::kOther, + DataModel::List(thirdEpList)), }; - CHIP_ERROR ReadActionAtIndex(uint16_t index, ActionListStructType & action) override; - CHIP_ERROR ReadEndpointListAtIndex(uint16_t index, EndpointListStructType & epList) override; + + CHIP_ERROR ReadActionAtIndex(uint16_t index, GenericActionStruct & action) override; + CHIP_ERROR ReadEndpointListAtIndex(uint16_t index, GenericEndpointList & epList) override; CHIP_ERROR FindActionIdInActionList(uint16_t actionId) override; Protocols::InteractionModel::Status HandleInstantAction(uint16_t actionId, Optional invokeId) override; diff --git a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp index bd9a588f6047e4..e7f0dd701015b3 100644 --- a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp +++ b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp @@ -24,31 +24,23 @@ using namespace chip::app::Clusters::Actions; using namespace chip::app::Clusters::Actions::Attributes; using namespace chip::Protocols::InteractionModel; -CHIP_ERROR ActionsDelegateImpl::ReadActionAtIndex(uint16_t index, ActionListStructType & action) +CHIP_ERROR ActionsDelegateImpl::ReadActionAtIndex(uint16_t index, GenericActionStruct & action) { if (index >= ArraySize(kActionList)) { return CHIP_ERROR_PROVIDER_LIST_EXHAUSTED; } - action.actionID = kActionList[index].actionID; - action.name = kActionList[index].name; - action.type = kActionList[index].type; - action.endpointListID = kActionList[index].endpointListID; - action.supportedCommands = kActionList[index].supportedCommands; - action.state = kActionList[index].state; + action = kActionList[index]; return CHIP_NO_ERROR; } -CHIP_ERROR ActionsDelegateImpl::ReadEndpointListAtIndex(uint16_t index, EndpointListStructType & epList) +CHIP_ERROR ActionsDelegateImpl::ReadEndpointListAtIndex(uint16_t index, GenericEndpointList & epList) { if (index >= ArraySize(kEndpointList)) { return CHIP_ERROR_PROVIDER_LIST_EXHAUSTED; } - epList.endpointListID = kEndpointList[index].endpointListID; - epList.name = kEndpointList[index].name; - epList.type = kEndpointList[index].type; - epList.endpoints = kEndpointList[index].endpoints; + epList = kEndpointList[index]; return CHIP_NO_ERROR; } diff --git a/src/app/clusters/actions-server/actions-server.cpp b/src/app/clusters/actions-server/actions-server.cpp index 023cbcf9c1a0cd..3945903b85d410 100644 --- a/src/app/clusters/actions-server/actions-server.cpp +++ b/src/app/clusters/actions-server/actions-server.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -38,6 +39,35 @@ Instance * Instance::GetInstance() return &instance; } +void Instance::OnStateChanged(EndpointId endpoint, uint16_t actionId, uint32_t invokeId, ActionStateEnum actionState) +{ + ChipLogProgress(Zcl, "ActionsServer: OnStateChanged"); + + // Record StateChanged event + EventNumber eventNumber; + Events::StateChanged::Type event{ actionId, invokeId, actionState }; + + if (CHIP_NO_ERROR != LogEvent(event, endpoint, eventNumber)) + { + ChipLogError(Zcl, "ActionsServer: Failed to record OnStateChanged event"); + } +} + +void Instance::OnActionFailed(EndpointId endpoint, uint16_t actionId, uint32_t invokeId, ActionStateEnum actionState, + ActionErrorEnum actionError) +{ + ChipLogProgress(Zcl, "ActionsServer: OnActionFailed"); + + // Record ActionFailed event + EventNumber eventNumber; + Events::ActionFailed::Type event{ actionId, invokeId, actionState, actionError }; + + if (CHIP_NO_ERROR != LogEvent(event, endpoint, eventNumber)) + { + ChipLogError(Zcl, "ActionsServer: Failed to record OnActionFailed event"); + } +} + CHIP_ERROR Instance::ReadActionListAttribute(const AttributeValueEncoder::ListEncodeHelper & encoder) { if (GetInstance()->mDelegate == nullptr) @@ -47,7 +77,7 @@ CHIP_ERROR Instance::ReadActionListAttribute(const AttributeValueEncoder::ListEn } for (uint16_t i = 0; true; i++) { - Actions::Structs::ActionStruct::Type action; + GenericActionStruct action; CHIP_ERROR err = GetInstance()->mDelegate->ReadActionAtIndex(i, action); if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED) @@ -69,7 +99,7 @@ CHIP_ERROR Instance::ReadEndpointListAttribute(const AttributeValueEncoder::List } for (uint16_t i = 0; true; i++) { - Actions::Structs::EndpointListStruct::Type epList; + GenericEndpointList epList; CHIP_ERROR err = GetInstance()->mDelegate->ReadEndpointListAtIndex(i, epList); if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED) diff --git a/src/app/clusters/actions-server/actions-server.h b/src/app/clusters/actions-server/actions-server.h index f702069cfa68b7..d081bdfd6c44e0 100644 --- a/src/app/clusters/actions-server/actions-server.h +++ b/src/app/clusters/actions-server/actions-server.h @@ -27,8 +27,94 @@ namespace app { namespace Clusters { namespace Actions { +inline constexpr size_t kActionNameMaxSize = 128u; +inline constexpr size_t kEndpointListNameMaxSize = 128u; + class Delegate; +struct GenericActionStruct : public Structs::ActionStruct::Type +{ + GenericActionStruct(){}; + + GenericActionStruct(uint16_t action, CharSpan label, ActionTypeEnum actionType, uint16_t epListID, + BitMask commands, ActionStateEnum actionState) + { + Set(action, label, actionType, epListID, commands, actionState); + } + + GenericActionStruct(const GenericActionStruct & action) { *this = action; } + + GenericActionStruct & operator=(const GenericActionStruct & action) + { + Set(action.actionID, action.name, action.type, action.endpointListID, action.supportedCommands, action.state); + return *this; + } + + void Set(uint16_t action, CharSpan label, ActionTypeEnum actionType, uint16_t epListID, BitMask commands, + ActionStateEnum actionState) + { + actionID = action; + type = actionType; + endpointListID = epListID; + supportedCommands = commands; + state = actionState; + memset(mActionNameBuffer, 0, sizeof(mActionNameBuffer)); + if (label.size() > sizeof(mActionNameBuffer)) + { + memcpy(mActionNameBuffer, label.data(), sizeof(mActionNameBuffer)); + name = CharSpan(mActionNameBuffer, sizeof(mActionNameBuffer)); + } + else + { + memcpy(mActionNameBuffer, label.data(), label.size()); + name = CharSpan(mActionNameBuffer, label.size()); + } + } + +private: + char mActionNameBuffer[kActionNameMaxSize]; +}; + +struct GenericEndpointList : public Structs::EndpointListStruct::Type +{ + GenericEndpointList(){}; + + GenericEndpointList(uint16_t epListId, CharSpan label, EndpointListTypeEnum epListType, + DataModel::List endpointList) + { + Set(epListId, label, epListType, endpointList); + } + + GenericEndpointList(const GenericEndpointList & epList) { *this = epList; } + + GenericEndpointList & operator=(const GenericEndpointList & epList) + { + Set(epList.endpointListID, epList.name, epList.type, epList.endpoints); + return *this; + } + + void Set(uint16_t epListId, CharSpan label, EndpointListTypeEnum epListType, DataModel::List endpointList) + { + endpointListID = epListId; + type = epListType; + endpoints = endpointList; + memset(mEndpointListNameBuffer, 0, sizeof(mEndpointListNameBuffer)); + if (label.size() > sizeof(mEndpointListNameBuffer)) + { + memcpy(mEndpointListNameBuffer, label.data(), sizeof(mEndpointListNameBuffer)); + name = CharSpan(mEndpointListNameBuffer, sizeof(mEndpointListNameBuffer)); + } + else + { + memcpy(mEndpointListNameBuffer, label.data(), label.size()); + name = CharSpan(mEndpointListNameBuffer, label.size()); + } + } + +private: + char mEndpointListNameBuffer[kEndpointListNameMaxSize]; +}; + class Instance : public AttributeAccessInterface, public CommandHandlerInterface { public: @@ -38,6 +124,19 @@ class Instance : public AttributeAccessInterface, public CommandHandlerInterface CommandHandlerInterface(Optional::Missing(), Actions::Id) {} + /** + * @brief + * Called when the state of action is changed. + */ + void OnStateChanged(EndpointId endpoint, uint16_t actionId, uint32_t invokeId, ActionStateEnum actionState); + + /** + * @brief + * Called when the action is failed.. + */ + void OnActionFailed(EndpointId endpoint, uint16_t actionId, uint32_t invokeId, ActionStateEnum actionState, + ActionErrorEnum actionError); + static Instance * GetInstance(); void SetDefaultDelegate(Delegate * aDelegate); @@ -82,26 +181,24 @@ class Delegate /** * Get the action at the Nth index from list of actions. * @param index The index of the action to be returned. It is assumed that actions are indexable from 0 and with no gaps. - * @param action A reference to the action struct which copies existing initialised buffer at index. + * @param action A reference to the GenericActionStruct which copies the buffer into it's own memory. * Name field (CharSpan) of ActionStruct contails the name of an action. - * The underlying data must remain allocated throughout the lifetime of the device, - * as the API does not make a copy. + * The API makes a copy of name. * @return Returns a CHIP_NO_ERROR if there was no error and the action was returned successfully. * CHIP_ERROR_PROVIDER_LIST_EXHAUSTED if the index in beyond the list of available actions. */ - virtual CHIP_ERROR ReadActionAtIndex(uint16_t index, Structs::ActionStruct::Type & action); + virtual CHIP_ERROR ReadActionAtIndex(uint16_t index, GenericActionStruct & action); /** * Get the EndpointList at the Nth index from list of endpointList. * @param index The index of the endpointList to be returned. It is assumed that actions are indexable from 0 and with no gaps. - * @param epList A reference to the endpointList struct which copies existing initialised buffer at index. - * Name field (CharSpan) of EndpointList contails the name of an endpointList. - * The underlying data must remain allocated throughout the lifetime of the device, - * as the API does not make a copy. + * @param epList A reference to the GenericEndpointList which copies the buffer into it's own memory. + * Name field (CharSpan) of EndpointList contails the name of an endpoint list. + * The API makes a copy of name. * @return Returns a CHIP_NO_ERROR if there was no error and the epList was returned successfully. * CHIP_ERROR_PROVIDER_LIST_EXHAUSTED if the index in beyond the list of available endpointList. */ - virtual CHIP_ERROR ReadEndpointListAtIndex(uint16_t index, Structs::EndpointListStruct::Type & epList); + virtual CHIP_ERROR ReadEndpointListAtIndex(uint16_t index, GenericEndpointList & epList); /** * Find the action with matching actionId in the list of action. From a03eb9cb1856ef2fbf3c290452e23fe60711f40b Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Mon, 7 Oct 2024 17:27:26 +0530 Subject: [PATCH 09/25] Address review comments --- .../include/bridged-actions-stub.h | 22 +-- .../src/bridged-actions-stub.cpp | 10 +- examples/bridge-app/asr/BUILD.gn | 1 - .../asr/src/bridged-actions-stub.cpp | 101 ------------ .../asr/subdevice/SubDeviceManager.cpp | 8 - examples/bridge-app/linux/BUILD.gn | 8 + .../bridge-app/linux/bridged-actions-stub.cpp | 8 +- examples/bridge-app/telink/src/AppTask.cpp | 8 - .../bridge-app/telink/src/DeviceCallbacks.cpp | 105 ------------- examples/placeholder/linux/apps/app1/BUILD.gn | 1 - examples/placeholder/linux/apps/app2/BUILD.gn | 1 - .../linux/src/bridged-actions-stub.cpp | 102 ------------ .../actions-server/actions-server.cpp | 12 +- .../clusters/actions-server/actions-server.h | 148 +++++++++--------- 14 files changed, 103 insertions(+), 432 deletions(-) delete mode 100644 examples/bridge-app/asr/src/bridged-actions-stub.cpp delete mode 100644 examples/bridge-app/telink/src/DeviceCallbacks.cpp delete mode 100644 examples/placeholder/linux/src/bridged-actions-stub.cpp diff --git a/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h b/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h index 91c9999d7cf90d..ca8b0e26d2004d 100644 --- a/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h +++ b/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h @@ -34,10 +34,10 @@ const uint16_t kEndpointListSize = 3; class ActionsDelegateImpl : public Delegate { private: - const GenericActionStruct kActionList[kActionListSize] = { - GenericActionStruct(0, CharSpan::fromCharString("TurnOnLight"), ActionTypeEnum::kScene, 0, 0, ActionStateEnum::kInactive), - GenericActionStruct(1, CharSpan::fromCharString("TurnOffLight"), ActionTypeEnum::kScene, 1, 0, ActionStateEnum::kInactive), - GenericActionStruct(2, CharSpan::fromCharString("ToggleLight"), ActionTypeEnum::kScene, 2, 0, ActionStateEnum::kInactive), + const ActionStructStorage kActionList[kActionListSize] = { + ActionStructStorage(0, CharSpan::fromCharString("TurnOnLight"), ActionTypeEnum::kScene, 0, 0, ActionStateEnum::kInactive), + ActionStructStorage(1, CharSpan::fromCharString("TurnOffLight"), ActionTypeEnum::kScene, 1, 0, ActionStateEnum::kInactive), + ActionStructStorage(2, CharSpan::fromCharString("ToggleLight"), ActionTypeEnum::kScene, 2, 0, ActionStateEnum::kInactive), }; // Dummy endpoint list. @@ -45,18 +45,18 @@ class ActionsDelegateImpl : public Delegate const EndpointId secondEpList[1] = { 0 }; const EndpointId thirdEpList[1] = { 0 }; - const GenericEndpointList kEndpointList[kEndpointListSize] = { - GenericEndpointList(0, CharSpan::fromCharString("On"), EndpointListTypeEnum::kOther, + const EndpointListStorage kEndpointList[kEndpointListSize] = { + EndpointListStorage(0, CharSpan::fromCharString("On"), EndpointListTypeEnum::kOther, DataModel::List(firstEpList)), - GenericEndpointList(1, CharSpan::fromCharString("Off"), EndpointListTypeEnum::kOther, + EndpointListStorage(1, CharSpan::fromCharString("Off"), EndpointListTypeEnum::kOther, DataModel::List(secondEpList)), - GenericEndpointList(2, CharSpan::fromCharString("Toggle"), EndpointListTypeEnum::kOther, + EndpointListStorage(2, CharSpan::fromCharString("Toggle"), EndpointListTypeEnum::kOther, DataModel::List(thirdEpList)), }; - CHIP_ERROR ReadActionAtIndex(uint16_t index, GenericActionStruct & action) override; - CHIP_ERROR ReadEndpointListAtIndex(uint16_t index, GenericEndpointList & epList) override; - CHIP_ERROR FindActionIdInActionList(uint16_t actionId) override; + CHIP_ERROR ReadActionAtIndex(uint16_t index, ActionStructStorage & action) override; + CHIP_ERROR ReadEndpointListAtIndex(uint16_t index, EndpointListStorage & epList) override; + bool FindActionIdInActionList(uint16_t actionId) override; Protocols::InteractionModel::Status HandleInstantAction(uint16_t actionId, Optional invokeId) override; Protocols::InteractionModel::Status HandleInstantActionWithTransition(uint16_t actionId, uint16_t transitionTime, diff --git a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp index e7f0dd701015b3..e5e7c98ae0d0c1 100644 --- a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp +++ b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp @@ -24,7 +24,7 @@ using namespace chip::app::Clusters::Actions; using namespace chip::app::Clusters::Actions::Attributes; using namespace chip::Protocols::InteractionModel; -CHIP_ERROR ActionsDelegateImpl::ReadActionAtIndex(uint16_t index, GenericActionStruct & action) +CHIP_ERROR ActionsDelegateImpl::ReadActionAtIndex(uint16_t index, ActionStructStorage & action) { if (index >= ArraySize(kActionList)) { @@ -34,7 +34,7 @@ CHIP_ERROR ActionsDelegateImpl::ReadActionAtIndex(uint16_t index, GenericActionS return CHIP_NO_ERROR; } -CHIP_ERROR ActionsDelegateImpl::ReadEndpointListAtIndex(uint16_t index, GenericEndpointList & epList) +CHIP_ERROR ActionsDelegateImpl::ReadEndpointListAtIndex(uint16_t index, EndpointListStorage & epList) { if (index >= ArraySize(kEndpointList)) { @@ -44,14 +44,14 @@ CHIP_ERROR ActionsDelegateImpl::ReadEndpointListAtIndex(uint16_t index, GenericE return CHIP_NO_ERROR; } -CHIP_ERROR ActionsDelegateImpl::FindActionIdInActionList(uint16_t actionId) +bool ActionsDelegateImpl::FindActionIdInActionList(uint16_t actionId) { for (uint16_t i = 0; i < kActionListSize; i++) { if (kActionList[i].actionID == actionId) - return CHIP_NO_ERROR; + return true; } - return CHIP_ERROR_NOT_FOUND; + return false; } Status ActionsDelegateImpl::HandleInstantAction(uint16_t actionId, Optional invokeId) diff --git a/examples/bridge-app/asr/BUILD.gn b/examples/bridge-app/asr/BUILD.gn index 0fb9be4dcd99c4..19f8263e12bb7c 100755 --- a/examples/bridge-app/asr/BUILD.gn +++ b/examples/bridge-app/asr/BUILD.gn @@ -78,7 +78,6 @@ asr_executable("bridge_app") { "${examples_plat_dir}/shell/matter_shell.cpp", "src/AppTask.cpp", "src/DeviceCallbacks.cpp", - "src/bridged-actions-stub.cpp", "src/main.cpp", "subdevice/SubDevice.cpp", "subdevice/SubDeviceManager.cpp", diff --git a/examples/bridge-app/asr/src/bridged-actions-stub.cpp b/examples/bridge-app/asr/src/bridged-actions-stub.cpp deleted file mode 100644 index 84dca65f832107..00000000000000 --- a/examples/bridge-app/asr/src/bridged-actions-stub.cpp +++ /dev/null @@ -1,101 +0,0 @@ -/* - * - * Copyright (c) 2021 Project CHIP Authors - * - * 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 -#include -#include -#include -#include -#include -#include -#include - -using namespace chip; -using namespace chip::app; -using namespace chip::app::Clusters; -using namespace chip::app::Clusters::Actions::Attributes; - -namespace { - -class ActionsAttrAccess : public AttributeAccessInterface -{ -public: - // Register for the Actions cluster on all endpoints. - ActionsAttrAccess() : AttributeAccessInterface(Optional::Missing(), Actions::Id) {} - - CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; - -private: - static constexpr uint16_t ClusterRevision = 1; - - CHIP_ERROR ReadActionListAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder); - CHIP_ERROR ReadEndpointListAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder); - CHIP_ERROR ReadSetupUrlAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder); - CHIP_ERROR ReadClusterRevision(EndpointId endpoint, AttributeValueEncoder & aEncoder); -}; - -constexpr uint16_t ActionsAttrAccess::ClusterRevision; - -CHIP_ERROR ActionsAttrAccess::ReadActionListAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder) -{ - // Just return an empty list - return aEncoder.EncodeEmptyList(); -} - -CHIP_ERROR ActionsAttrAccess::ReadEndpointListAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder) -{ - // Just return an empty list - return aEncoder.EncodeEmptyList(); -} - -CHIP_ERROR ActionsAttrAccess::ReadSetupUrlAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder) -{ - static const char SetupUrl[] = "https://example.com"; - return aEncoder.Encode(chip::Span(SetupUrl, strlen(SetupUrl))); -} - -CHIP_ERROR ActionsAttrAccess::ReadClusterRevision(EndpointId endpoint, AttributeValueEncoder & aEncoder) -{ - return aEncoder.Encode(ClusterRevision); -} - -ActionsAttrAccess gAttrAccess; - -CHIP_ERROR ActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) -{ - VerifyOrDie(aPath.mClusterId == Actions::Id); - - switch (aPath.mAttributeId) - { - case ActionList::Id: - return ReadActionListAttribute(aPath.mEndpointId, aEncoder); - case EndpointLists::Id: - return ReadEndpointListAttribute(aPath.mEndpointId, aEncoder); - case SetupURL::Id: - return ReadSetupUrlAttribute(aPath.mEndpointId, aEncoder); - case ClusterRevision::Id: - return ReadClusterRevision(aPath.mEndpointId, aEncoder); - default: - break; - } - return CHIP_NO_ERROR; -} -} // anonymous namespace - -// void MatterActionsPluginServerInitCallback(void) -//{ -// AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); -// } diff --git a/examples/bridge-app/asr/subdevice/SubDeviceManager.cpp b/examples/bridge-app/asr/subdevice/SubDeviceManager.cpp index c0d45c06295c9d..5e2480f3800fa0 100644 --- a/examples/bridge-app/asr/subdevice/SubDeviceManager.cpp +++ b/examples/bridge-app/asr/subdevice/SubDeviceManager.cpp @@ -243,14 +243,6 @@ void HandleDeviceStatusChanged(SubDevice * dev, SubDevice::Changed_t itemChanged const EmberAfDeviceType gRootDeviceTypes[] = { { DEVICE_TYPE_ROOT_NODE, DEVICE_VERSION_DEFAULT } }; const EmberAfDeviceType gAggregateNodeDeviceTypes[] = { { DEVICE_TYPE_BRIDGE, DEVICE_VERSION_DEFAULT } }; -bool emberAfActionsClusterInstantActionCallback(app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath, - const Actions::Commands::InstantAction::DecodableType & commandData) -{ - // No actions are implemented, just return status NotFound. - commandObj->AddStatus(commandPath, Protocols::InteractionModel::Status::NotFound); - return true; -} - void Init_Bridge_Endpoint() { // bridge will have own database named gSubDevices. diff --git a/examples/bridge-app/linux/BUILD.gn b/examples/bridge-app/linux/BUILD.gn index 212fb9dd3a9264..42bdbe83f2ed1c 100644 --- a/examples/bridge-app/linux/BUILD.gn +++ b/examples/bridge-app/linux/BUILD.gn @@ -28,6 +28,14 @@ executable("chip-bridge-app") { "main.cpp", ] + # Generic implementation of the actions cluster is not introduced in this app as this app has + # an interface to test actions. + # TODO: Generic implementation can be used here instead of having its own. + excludes = [ "${chip_root}/src/app/clusters/actions-server/**" ] + + # Apply excludes to the sources + sources -= excludes + deps = [ "${chip_root}/examples/bridge-app/bridge-common", "${chip_root}/examples/platform/linux:app-main", diff --git a/examples/bridge-app/linux/bridged-actions-stub.cpp b/examples/bridge-app/linux/bridged-actions-stub.cpp index 0eff1e7b6d05bc..580f4f2239bd1a 100644 --- a/examples/bridge-app/linux/bridged-actions-stub.cpp +++ b/examples/bridge-app/linux/bridged-actions-stub.cpp @@ -131,7 +131,7 @@ CHIP_ERROR ActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, Attr } } // anonymous namespace -// void MatterActionsPluginServerInitCallback() -//{ -// AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); -// } +void MatterActionsPluginServerInitCallback() +{ + AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); +} diff --git a/examples/bridge-app/telink/src/AppTask.cpp b/examples/bridge-app/telink/src/AppTask.cpp index 0f59c3d7751936..f8360e8e860965 100644 --- a/examples/bridge-app/telink/src/AppTask.cpp +++ b/examples/bridge-app/telink/src/AppTask.cpp @@ -387,14 +387,6 @@ void HandleDeviceStatusChanged(Device * dev, Device::Changed_t itemChangedMask) } } -bool emberAfActionsClusterInstantActionCallback(app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath, - const Clusters::Actions::Commands::InstantAction::DecodableType & commandData) -{ - // No actions are implemented, just return status NotFound. - commandObj->AddStatus(commandPath, Protocols::InteractionModel::Status::NotFound); - return true; -} - CHIP_ERROR AppTask::Init(void) { SetExampleButtonCallbacks(LightingActionEventHandler); diff --git a/examples/bridge-app/telink/src/DeviceCallbacks.cpp b/examples/bridge-app/telink/src/DeviceCallbacks.cpp deleted file mode 100644 index 8511d7047dfb0a..00000000000000 --- a/examples/bridge-app/telink/src/DeviceCallbacks.cpp +++ /dev/null @@ -1,105 +0,0 @@ -/* - * - * Copyright (c) 2023 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 -#include -#include -#include -#include -#include -#include -#include - -using namespace ::chip; -using namespace ::chip::app; -using namespace ::chip::app::Clusters; -using namespace ::chip::app::Clusters::Actions::Attributes; -using namespace ::chip::Inet; -using namespace ::chip::System; - -namespace { - -class ActionsAttrAccess : public AttributeAccessInterface -{ -public: - // Register for the Actions cluster on all endpoints. - ActionsAttrAccess() : AttributeAccessInterface(Optional::Missing(), Actions::Id) {} - - CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; - -private: - static constexpr uint16_t ClusterRevision = 1; - - CHIP_ERROR ReadActionListAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder); - CHIP_ERROR ReadEndpointListAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder); - CHIP_ERROR ReadSetupUrlAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder); - CHIP_ERROR ReadClusterRevision(EndpointId endpoint, AttributeValueEncoder & aEncoder); -}; - -constexpr uint16_t ActionsAttrAccess::ClusterRevision; - -CHIP_ERROR ActionsAttrAccess::ReadActionListAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder) -{ - // Just return an empty list - return aEncoder.EncodeEmptyList(); -} - -CHIP_ERROR ActionsAttrAccess::ReadEndpointListAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder) -{ - // Just return an empty list - return aEncoder.EncodeEmptyList(); -} - -CHIP_ERROR ActionsAttrAccess::ReadSetupUrlAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder) -{ - static const char SetupUrl[] = "https://example.com"; - return aEncoder.Encode(chip::CharSpan::fromCharString(SetupUrl)); -} - -CHIP_ERROR ActionsAttrAccess::ReadClusterRevision(EndpointId endpoint, AttributeValueEncoder & aEncoder) -{ - return aEncoder.Encode(ClusterRevision); -} - -ActionsAttrAccess gAttrAccess; - -CHIP_ERROR ActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) -{ - VerifyOrDie(aPath.mClusterId == Actions::Id); - - switch (aPath.mAttributeId) - { - case ActionList::Id: - return ReadActionListAttribute(aPath.mEndpointId, aEncoder); - case EndpointLists::Id: - return ReadEndpointListAttribute(aPath.mEndpointId, aEncoder); - case SetupURL::Id: - return ReadSetupUrlAttribute(aPath.mEndpointId, aEncoder); - case ClusterRevision::Id: - return ReadClusterRevision(aPath.mEndpointId, aEncoder); - default: - break; - } - return CHIP_NO_ERROR; -} -} // anonymous namespace - -// void MatterActionsPluginServerInitCallback(void) -//{ -// AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); -// } diff --git a/examples/placeholder/linux/apps/app1/BUILD.gn b/examples/placeholder/linux/apps/app1/BUILD.gn index aba1e870f7a041..f8cc1b36a2b0ee 100644 --- a/examples/placeholder/linux/apps/app1/BUILD.gn +++ b/examples/placeholder/linux/apps/app1/BUILD.gn @@ -28,7 +28,6 @@ source_set("app1") { sources = [ "../../resource-monitoring-delegates.cpp", - "../../src/bridged-actions-stub.cpp", "../../static-supported-modes-manager.cpp", "../../thread-border-router-management.cpp", ] diff --git a/examples/placeholder/linux/apps/app2/BUILD.gn b/examples/placeholder/linux/apps/app2/BUILD.gn index 2bea36611ffb37..302054f5bf927b 100644 --- a/examples/placeholder/linux/apps/app2/BUILD.gn +++ b/examples/placeholder/linux/apps/app2/BUILD.gn @@ -28,7 +28,6 @@ source_set("app2") { sources = [ "../../resource-monitoring-delegates.cpp", - "../../src/bridged-actions-stub.cpp", "../../static-supported-modes-manager.cpp", ] diff --git a/examples/placeholder/linux/src/bridged-actions-stub.cpp b/examples/placeholder/linux/src/bridged-actions-stub.cpp deleted file mode 100644 index ad934cc881077c..00000000000000 --- a/examples/placeholder/linux/src/bridged-actions-stub.cpp +++ /dev/null @@ -1,102 +0,0 @@ -/* - * - * Copyright (c) 2021 Project CHIP Authors - * - * 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 -#include -#include -#include -#include -#include -#include -#include - -using namespace chip; -using namespace chip::app; -using namespace chip::app::Clusters; -using namespace chip::app::Clusters::Actions::Attributes; - -namespace { - -class ActionsAttrAccess : public AttributeAccessInterface -{ -public: - // Register for the Actions cluster on all endpoints. - ActionsAttrAccess() : AttributeAccessInterface(Optional::Missing(), Actions::Id) {} - - CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; - -private: - static constexpr uint16_t ClusterRevision = 1; - - CHIP_ERROR ReadActionListAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder); - CHIP_ERROR ReadEndpointListAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder); - CHIP_ERROR ReadSetupUrlAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder); - CHIP_ERROR ReadClusterRevision(EndpointId endpoint, AttributeValueEncoder & aEncoder); -}; - -constexpr uint16_t ActionsAttrAccess::ClusterRevision; - -CHIP_ERROR ActionsAttrAccess::ReadActionListAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder) -{ - // Just return an empty list - return aEncoder.EncodeEmptyList(); -} - -CHIP_ERROR ActionsAttrAccess::ReadEndpointListAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder) -{ - // Just return an empty list - return aEncoder.EncodeEmptyList(); -} - -CHIP_ERROR ActionsAttrAccess::ReadSetupUrlAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder) -{ - static const char SetupUrl[] = "https://example.com"; - return aEncoder.Encode(chip::Span(SetupUrl, strlen(SetupUrl))); -} - -CHIP_ERROR ActionsAttrAccess::ReadClusterRevision(EndpointId endpoint, AttributeValueEncoder & aEncoder) -{ - return aEncoder.Encode(ClusterRevision); -} - -ActionsAttrAccess gAttrAccess; - -CHIP_ERROR ActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) -{ - VerifyOrDie(aPath.mClusterId == Actions::Id); - - switch (aPath.mAttributeId) - { - case ActionList::Id: - return ReadActionListAttribute(aPath.mEndpointId, aEncoder); - case EndpointLists::Id: - return ReadEndpointListAttribute(aPath.mEndpointId, aEncoder); - case SetupURL::Id: - return ReadSetupUrlAttribute(aPath.mEndpointId, aEncoder); - case ClusterRevision::Id: - return ReadClusterRevision(aPath.mEndpointId, aEncoder); - default: - break; - } - return CHIP_NO_ERROR; -} -} // anonymous namespace - -// void MatterActionsPluginServerInitCallback(void) -//{ -// chip::app::AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); -// } diff --git a/src/app/clusters/actions-server/actions-server.cpp b/src/app/clusters/actions-server/actions-server.cpp index 3945903b85d410..543e4e6aa078b7 100644 --- a/src/app/clusters/actions-server/actions-server.cpp +++ b/src/app/clusters/actions-server/actions-server.cpp @@ -16,7 +16,6 @@ */ #include "actions-server.h" -#include #include #include #include @@ -77,7 +76,7 @@ CHIP_ERROR Instance::ReadActionListAttribute(const AttributeValueEncoder::ListEn } for (uint16_t i = 0; true; i++) { - GenericActionStruct action; + ActionStructStorage action; CHIP_ERROR err = GetInstance()->mDelegate->ReadActionAtIndex(i, action); if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED) @@ -99,7 +98,7 @@ CHIP_ERROR Instance::ReadEndpointListAttribute(const AttributeValueEncoder::List } for (uint16_t i = 0; true; i++) { - GenericEndpointList epList; + EndpointListStorage epList; CHIP_ERROR err = GetInstance()->mDelegate->ReadEndpointListAtIndex(i, epList); if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED) @@ -146,12 +145,12 @@ CHIP_ERROR Instance::Read(const ConcreteReadAttributePath & aPath, AttributeValu return CHIP_NO_ERROR; } -CHIP_ERROR Instance::FindActionIdInActionList(uint16_t actionId) +bool Instance::FindActionIdInActionList(uint16_t actionId) { if (GetInstance()->mDelegate == nullptr) { ChipLogError(Zcl, "Actions delegate is null!!!"); - return CHIP_ERROR_INCORRECT_STATE; + return false; } return GetInstance()->mDelegate->FindActionIdInActionList(actionId); } @@ -178,8 +177,7 @@ void Instance::HandleCommand(HandlerContext & handlerContext, FuncT func) } uint16_t actionId = requestPayload.actionID; - CHIP_ERROR err = GetInstance()->FindActionIdInActionList(actionId); - if (err != CHIP_NO_ERROR) + if (!GetInstance()->FindActionIdInActionList(actionId)) { handlerContext.mCommandHandler.AddStatus(handlerContext.mRequestPath, Protocols::InteractionModel::Status::NotFound); return; diff --git a/src/app/clusters/actions-server/actions-server.h b/src/app/clusters/actions-server/actions-server.h index d081bdfd6c44e0..ba30566a1e3101 100644 --- a/src/app/clusters/actions-server/actions-server.h +++ b/src/app/clusters/actions-server/actions-server.h @@ -21,36 +21,39 @@ #include #include #include +#include namespace chip { namespace app { namespace Clusters { namespace Actions { -inline constexpr size_t kActionNameMaxSize = 128u; -inline constexpr size_t kEndpointListNameMaxSize = 128u; +static constexpr size_t kActionNameMaxSize = 128u; +static constexpr size_t kEndpointListNameMaxSize = 128u; +static constexpr size_t kEndpointListMaxSize = 256u; class Delegate; -struct GenericActionStruct : public Structs::ActionStruct::Type +struct ActionStructStorage : public Structs::ActionStruct::Type { - GenericActionStruct(){}; + ActionStructStorage() : mActionName(mBuffer, sizeof(mBuffer)){}; - GenericActionStruct(uint16_t action, CharSpan label, ActionTypeEnum actionType, uint16_t epListID, - BitMask commands, ActionStateEnum actionState) + ActionStructStorage(uint16_t action, CharSpan actionName, ActionTypeEnum actionType, uint16_t epListID, + BitMask commands, ActionStateEnum actionState) : + mActionName(mBuffer, sizeof(mBuffer)) { - Set(action, label, actionType, epListID, commands, actionState); + Set(action, actionName, actionType, epListID, commands, actionState); } - GenericActionStruct(const GenericActionStruct & action) { *this = action; } + ActionStructStorage(const ActionStructStorage & action) : mActionName(mBuffer, sizeof(mBuffer)) { *this = action; } - GenericActionStruct & operator=(const GenericActionStruct & action) + ActionStructStorage & operator=(const ActionStructStorage & action) { Set(action.actionID, action.name, action.type, action.endpointListID, action.supportedCommands, action.state); return *this; } - void Set(uint16_t action, CharSpan label, ActionTypeEnum actionType, uint16_t epListID, BitMask commands, + void Set(uint16_t action, CharSpan actionName, ActionTypeEnum actionType, uint16_t epListID, BitMask commands, ActionStateEnum actionState) { actionID = action; @@ -58,61 +61,55 @@ struct GenericActionStruct : public Structs::ActionStruct::Type endpointListID = epListID; supportedCommands = commands; state = actionState; - memset(mActionNameBuffer, 0, sizeof(mActionNameBuffer)); - if (label.size() > sizeof(mActionNameBuffer)) - { - memcpy(mActionNameBuffer, label.data(), sizeof(mActionNameBuffer)); - name = CharSpan(mActionNameBuffer, sizeof(mActionNameBuffer)); - } - else - { - memcpy(mActionNameBuffer, label.data(), label.size()); - name = CharSpan(mActionNameBuffer, label.size()); - } + CopyCharSpanToMutableCharSpanWithTruncation(actionName, mActionName); + name = mActionName; } private: - char mActionNameBuffer[kActionNameMaxSize]; + char mBuffer[kActionNameMaxSize]; + MutableCharSpan mActionName; }; -struct GenericEndpointList : public Structs::EndpointListStruct::Type +struct EndpointListStorage : public Structs::EndpointListStruct::Type { - GenericEndpointList(){}; + EndpointListStorage() : mEpListName(mBuffer, sizeof(mBuffer)){}; - GenericEndpointList(uint16_t epListId, CharSpan label, EndpointListTypeEnum epListType, - DataModel::List endpointList) + EndpointListStorage(uint16_t epListId, CharSpan epListName, EndpointListTypeEnum epListType, + DataModel::List endpointList) : + mEpListName(mBuffer, sizeof(mBuffer)) { - Set(epListId, label, epListType, endpointList); + Set(epListId, epListName, epListType, endpointList); } - GenericEndpointList(const GenericEndpointList & epList) { *this = epList; } + EndpointListStorage(const EndpointListStorage & epList) : mEpListName(mBuffer, sizeof(mBuffer)) { *this = epList; } - GenericEndpointList & operator=(const GenericEndpointList & epList) + EndpointListStorage & operator=(const EndpointListStorage & epList) { Set(epList.endpointListID, epList.name, epList.type, epList.endpoints); return *this; } - void Set(uint16_t epListId, CharSpan label, EndpointListTypeEnum epListType, DataModel::List endpointList) + void Set(uint16_t epListId, CharSpan epListName, EndpointListTypeEnum epListType, + DataModel::List endpointList) { endpointListID = epListId; type = epListType; - endpoints = endpointList; - memset(mEndpointListNameBuffer, 0, sizeof(mEndpointListNameBuffer)); - if (label.size() > sizeof(mEndpointListNameBuffer)) - { - memcpy(mEndpointListNameBuffer, label.data(), sizeof(mEndpointListNameBuffer)); - name = CharSpan(mEndpointListNameBuffer, sizeof(mEndpointListNameBuffer)); - } - else + + for (uint8_t index = 0; index < std::min(endpointList.size(), kEndpointListMaxSize); index++) { - memcpy(mEndpointListNameBuffer, label.data(), label.size()); - name = CharSpan(mEndpointListNameBuffer, label.size()); + mEpList.push_back(endpointList[index]); } + mEpListSpan = Span(mEpList.data(), mEpList.size()); + endpoints = DataModel::List(mEpListSpan); + CopyCharSpanToMutableCharSpanWithTruncation(epListName, mEpListName); + name = mEpListName; } private: - char mEndpointListNameBuffer[kEndpointListNameMaxSize]; + char mBuffer[kEndpointListNameMaxSize]; + MutableCharSpan mEpListName; + std::vector mEpList; + Span mEpListSpan; }; class Instance : public AttributeAccessInterface, public CommandHandlerInterface @@ -148,7 +145,7 @@ class Instance : public AttributeAccessInterface, public CommandHandlerInterface CHIP_ERROR ReadActionListAttribute(const AttributeValueEncoder::ListEncodeHelper & encoder); CHIP_ERROR ReadEndpointListAttribute(const AttributeValueEncoder::ListEncodeHelper & encoder); - CHIP_ERROR FindActionIdInActionList(uint16_t actionId); + bool FindActionIdInActionList(uint16_t actionId); // CommandHandlerInterface template @@ -181,35 +178,30 @@ class Delegate /** * Get the action at the Nth index from list of actions. * @param index The index of the action to be returned. It is assumed that actions are indexable from 0 and with no gaps. - * @param action A reference to the GenericActionStruct which copies the buffer into it's own memory. - * Name field (CharSpan) of ActionStruct contails the name of an action. - * The API makes a copy of name. + * @param action A reference to the ActionStructStorage which should be initialized via copy/assignments or calling Set(). * @return Returns a CHIP_NO_ERROR if there was no error and the action was returned successfully. * CHIP_ERROR_PROVIDER_LIST_EXHAUSTED if the index in beyond the list of available actions. */ - virtual CHIP_ERROR ReadActionAtIndex(uint16_t index, GenericActionStruct & action); + virtual CHIP_ERROR ReadActionAtIndex(uint16_t index, ActionStructStorage & action) = 0; /** * Get the EndpointList at the Nth index from list of endpointList. * @param index The index of the endpointList to be returned. It is assumed that actions are indexable from 0 and with no gaps. - * @param epList A reference to the GenericEndpointList which copies the buffer into it's own memory. - * Name field (CharSpan) of EndpointList contails the name of an endpoint list. - * The API makes a copy of name. + * @param action A reference to the EndpointListStorage which should be initialized via copy/assignments or calling Set(). * @return Returns a CHIP_NO_ERROR if there was no error and the epList was returned successfully. * CHIP_ERROR_PROVIDER_LIST_EXHAUSTED if the index in beyond the list of available endpointList. */ - virtual CHIP_ERROR ReadEndpointListAtIndex(uint16_t index, GenericEndpointList & epList); + virtual CHIP_ERROR ReadEndpointListAtIndex(uint16_t index, EndpointListStorage & epList) = 0; /** * Find the action with matching actionId in the list of action. * @param actionId The action to be find in the list of action. - * @return Returns a CHIP_NO_ERROR if matching action is found. - * CHIP_ERROR_NOT_FOUND if the matching action does not found in the list of action. + * @return Returns a true if matching action is found otherwise false. */ - virtual CHIP_ERROR FindActionIdInActionList(uint16_t actionId); + virtual bool FindActionIdInActionList(uint16_t actionId) = 0; /** - * On reciept of each and every command, + * On receipt of each and every command, * if the InvokeID data field is provided by the client when invoking a command, the server SHALL generate a StateChanged event * when the action changes to a new state or an ActionFailed event when execution of the action fails. * @@ -218,16 +210,16 @@ class Delegate */ /** - * When an InstantAction command is recieved, an action (state change) on the involved endpoints shall trigger, + * When an InstantAction command is received, an action (state change) on the involved endpoints shall trigger, * in a "fire and forget" manner. Afterwards, the action’s state SHALL be Inactive. * * @param actionId The id of an action on which an action shall takes place. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Protocols::InteractionModel::Status HandleInstantAction(uint16_t actionId, Optional invokeId); + virtual Protocols::InteractionModel::Status HandleInstantAction(uint16_t actionId, Optional invokeId) = 0; /** - * When an InstantActionWithTransition command is recieved, an action (state change) on the involved endpoints shall trigger, + * When an InstantActionWithTransition command is received, an action (state change) on the involved endpoints shall trigger, * with a specified time to transition from the current state to the new state. During the transition, the action’s state SHALL * be Active. Afterwards, the action’s state SHALL be Inactive. * @@ -236,19 +228,19 @@ class Delegate * @return Returns a Success if an action took place successfully otherwise, suitable error. */ virtual Protocols::InteractionModel::Status HandleInstantActionWithTransition(uint16_t actionId, uint16_t transitionTime, - Optional invokeId); + Optional invokeId) = 0; /** - * When a StartAction command is recieved, the commencement of an action on the involved endpoints shall trigger. Afterwards, + * When a StartAction command is received, the commencement of an action on the involved endpoints shall trigger. Afterwards, * the action’s state SHALL be Inactive. * * @param actionId The id of an action on which an action shall takes place. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Protocols::InteractionModel::Status HandleStartAction(uint16_t actionId, Optional invokeId); + virtual Protocols::InteractionModel::Status HandleStartAction(uint16_t actionId, Optional invokeId) = 0; /** - * When a StartActionWithDuration command is recieved, the commencement of an action on the involved endpoints shall trigger, + * When a StartActionWithDuration command is received, the commencement of an action on the involved endpoints shall trigger, * and SHALL change the action’s state to Active. After the specified Duration, the action will stop, and the action’s state * SHALL change to Inactive. * @@ -257,28 +249,28 @@ class Delegate * @return Returns a Success if an action took place successfully otherwise, suitable error. */ virtual Protocols::InteractionModel::Status HandleStartActionWithDuration(uint16_t actionId, uint32_t duration, - Optional invokeId); + Optional invokeId) = 0; /** - * When a StopAction command is recieved, the ongoing action on the involved endpoints shall stop. Afterwards, the action’s + * When a StopAction command is received, the ongoing action on the involved endpoints shall stop. Afterwards, the action’s * state SHALL be Inactive. * * @param actionId The id of an action on which an action shall takes place. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Protocols::InteractionModel::Status HandleStopAction(uint16_t actionId, Optional invokeId); + virtual Protocols::InteractionModel::Status HandleStopAction(uint16_t actionId, Optional invokeId) = 0; /** - * When a PauseAction command is recieved, the ongoing action on the involved endpoints shall pause and SHALL change the + * When a PauseAction command is received, the ongoing action on the involved endpoints shall pause and SHALL change the * action’s state to Paused. * * @param actionId The id of an action on which an action shall takes place. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Protocols::InteractionModel::Status HandlePauseAction(uint16_t actionId, Optional invokeId); + virtual Protocols::InteractionModel::Status HandlePauseAction(uint16_t actionId, Optional invokeId) = 0; /** - * When a PauseActionWithDuration command is recieved, pauses an ongoing action, and SHALL change the action’s state to Paused. + * When a PauseActionWithDuration command is received, pauses an ongoing action, and SHALL change the action’s state to Paused. * After the specified Duration, the ongoing action will be automatically resumed. which SHALL change the action’s state to * Active. * @@ -287,28 +279,28 @@ class Delegate * @return Returns a Success if an action took place successfully otherwise, suitable error. */ virtual Protocols::InteractionModel::Status HandlePauseActionWithDuration(uint16_t actionId, uint32_t duration, - Optional invokeId); + Optional invokeId) = 0; /** - * When a ResumeAction command is recieved, the previously paused action shall resume and SHALL change the action’s state to + * When a ResumeAction command is received, the previously paused action shall resume and SHALL change the action’s state to * Active. * * @param actionId The id of an action on which an action shall takes place. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Protocols::InteractionModel::Status HandleResumeAction(uint16_t actionId, Optional invokeId); + virtual Protocols::InteractionModel::Status HandleResumeAction(uint16_t actionId, Optional invokeId) = 0; /** - * When an EnableAction command is recieved, it enables a certain action or automation. Afterwards, the action’s state SHALL be + * When an EnableAction command is received, it enables a certain action or automation. Afterwards, the action’s state SHALL be * Active. * * @param actionId The id of an action on which an action shall takes place. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Protocols::InteractionModel::Status HandleEnableAction(uint16_t actionId, Optional invokeId); + virtual Protocols::InteractionModel::Status HandleEnableAction(uint16_t actionId, Optional invokeId) = 0; /** - * When an EnableActionWithDuration command is recieved, it enables a certain action or automation, and SHALL change the + * When an EnableActionWithDuration command is received, it enables a certain action or automation, and SHALL change the * action’s state to be Active. After the specified Duration, the action or automation will stop, and the action’s state SHALL * change to Disabled. * @@ -317,19 +309,19 @@ class Delegate * @return Returns a Success if an action took place successfully otherwise, suitable error. */ virtual Protocols::InteractionModel::Status HandleEnableActionWithDuration(uint16_t actionId, uint32_t duration, - Optional invokeId); + Optional invokeId) = 0; /** - * When a DisableAction command is recieved, it disables a certain action or automation, and SHALL change the action’s state to + * When a DisableAction command is received, it disables a certain action or automation, and SHALL change the action’s state to * Inactive. * * @param actionId The id of an action on which an action shall takes place. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ - virtual Protocols::InteractionModel::Status HandleDisableAction(uint16_t actionId, Optional invokeId); + virtual Protocols::InteractionModel::Status HandleDisableAction(uint16_t actionId, Optional invokeId) = 0; /** - * When a DisableActionWithDuration command is recieved, it disables a certain action or automation, and SHALL change the + * When a DisableActionWithDuration command is received, it disables a certain action or automation, and SHALL change the * action’s state to Disabled. After the specified Duration, the action or automation will re-start, and the action’s state * SHALL change to either Inactive or Active, depending on the actions. * @@ -338,7 +330,7 @@ class Delegate * @return Returns a Success if an action took place successfully otherwise, suitable error. */ virtual Protocols::InteractionModel::Status HandleDisableActionWithDuration(uint16_t actionId, uint32_t duration, - Optional invokeId); + Optional invokeId) = 0; }; } // namespace Actions From aed08c2b7bc370da5e3c912b95af481c7ca914cb Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Thu, 10 Oct 2024 13:30:22 +0530 Subject: [PATCH 10/25] Fix CI and apps should use emberAfActionsClusterInitCallback instead of MatterActionsPluginServerInitCallback in example --- .../esp32/sdkconfig_m5stack.defaults | 6 ++++++ .../esp32/sdkconfig_m5stack_rpc.defaults | 6 ++++++ examples/bridge-app/linux/BUILD.gn | 8 -------- examples/bridge-app/linux/bridged-actions-stub.cpp | 2 +- examples/bridge-app/telink/CMakeLists.txt | 1 - 5 files changed, 13 insertions(+), 10 deletions(-) diff --git a/examples/all-clusters-minimal-app/esp32/sdkconfig_m5stack.defaults b/examples/all-clusters-minimal-app/esp32/sdkconfig_m5stack.defaults index 287262d17e57cc..03691548ea91db 100644 --- a/examples/all-clusters-minimal-app/esp32/sdkconfig_m5stack.defaults +++ b/examples/all-clusters-minimal-app/esp32/sdkconfig_m5stack.defaults @@ -73,3 +73,9 @@ CONFIG_BUILD_CHIP_TESTS=y # Move functions from IRAM to flash CONFIG_FREERTOS_PLACE_FUNCTIONS_INTO_FLASH=y + +# Reduce the event logging buffer to reduce the DRAM usage +# TODO: [ESP32] Fix the DRAM overflow in esp32 apps #34717 +CONFIG_EVENT_LOGGING_CRIT_BUFFER_SIZE=512 +CONFIG_EVENT_LOGGING_INFO_BUFFER_SIZE=512 +CONFIG_EVENT_LOGGING_DEBUG_BUFFER_SIZE=512 diff --git a/examples/all-clusters-minimal-app/esp32/sdkconfig_m5stack_rpc.defaults b/examples/all-clusters-minimal-app/esp32/sdkconfig_m5stack_rpc.defaults index d2eaffb9bfd15b..6c4907410a1c22 100644 --- a/examples/all-clusters-minimal-app/esp32/sdkconfig_m5stack_rpc.defaults +++ b/examples/all-clusters-minimal-app/esp32/sdkconfig_m5stack_rpc.defaults @@ -88,3 +88,9 @@ CONFIG_FREERTOS_PLACE_FUNCTIONS_INTO_FLASH=y CONFIG_EVENT_LOGGING_CRIT_BUFFER_SIZE=1024 CONFIG_DIAG_USE_EXTERNAL_LOG_WRAP=y + +# Reduce the event logging buffer to reduce the DRAM usage +# TODO: [ESP32] Fix the DRAM overflow in esp32 apps #34717 +CONFIG_EVENT_LOGGING_CRIT_BUFFER_SIZE=512 +CONFIG_EVENT_LOGGING_INFO_BUFFER_SIZE=512 +CONFIG_EVENT_LOGGING_DEBUG_BUFFER_SIZE=512 diff --git a/examples/bridge-app/linux/BUILD.gn b/examples/bridge-app/linux/BUILD.gn index 42bdbe83f2ed1c..212fb9dd3a9264 100644 --- a/examples/bridge-app/linux/BUILD.gn +++ b/examples/bridge-app/linux/BUILD.gn @@ -28,14 +28,6 @@ executable("chip-bridge-app") { "main.cpp", ] - # Generic implementation of the actions cluster is not introduced in this app as this app has - # an interface to test actions. - # TODO: Generic implementation can be used here instead of having its own. - excludes = [ "${chip_root}/src/app/clusters/actions-server/**" ] - - # Apply excludes to the sources - sources -= excludes - deps = [ "${chip_root}/examples/bridge-app/bridge-common", "${chip_root}/examples/platform/linux:app-main", diff --git a/examples/bridge-app/linux/bridged-actions-stub.cpp b/examples/bridge-app/linux/bridged-actions-stub.cpp index 580f4f2239bd1a..f5224199a03336 100644 --- a/examples/bridge-app/linux/bridged-actions-stub.cpp +++ b/examples/bridge-app/linux/bridged-actions-stub.cpp @@ -131,7 +131,7 @@ CHIP_ERROR ActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, Attr } } // anonymous namespace -void MatterActionsPluginServerInitCallback() +void emberAfActionsClusterInitCallback() { AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); } diff --git a/examples/bridge-app/telink/CMakeLists.txt b/examples/bridge-app/telink/CMakeLists.txt index f2e80864d46e01..e7984bc1332853 100644 --- a/examples/bridge-app/telink/CMakeLists.txt +++ b/examples/bridge-app/telink/CMakeLists.txt @@ -38,7 +38,6 @@ target_include_directories(app PRIVATE target_sources(app PRIVATE src/AppTask.cpp src/Device.cpp - src/DeviceCallbacks.cpp ${TELINK_COMMON}/common/src/mainCommon.cpp ${TELINK_COMMON}/common/src/AppTaskCommon.cpp ${TELINK_COMMON}/util/src/LEDManager.cpp From 520e8d61a47b1629e1a56a41b58ab5ee4e26bfaa Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Sun, 20 Oct 2024 18:08:41 +0530 Subject: [PATCH 11/25] Try to fix CI --- examples/all-clusters-app/esp32/main/main.cpp | 3 --- examples/all-clusters-app/linux/main-common.cpp | 7 +++++++ scripts/tools/check_includes_config.py | 1 + 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/examples/all-clusters-app/esp32/main/main.cpp b/examples/all-clusters-app/esp32/main/main.cpp index 485dc7cf30a641..c2b0dbe55cc746 100644 --- a/examples/all-clusters-app/esp32/main/main.cpp +++ b/examples/all-clusters-app/esp32/main/main.cpp @@ -35,7 +35,6 @@ #include "shell_extension/launch.h" #include #include -#include #include #include #include @@ -97,7 +96,6 @@ AppCallbacks sCallbacks; app::Clusters::TemperatureControl::AppSupportedTemperatureLevelsDelegate sAppSupportedTemperatureLevelsDelegate; app::Clusters::ModeSelect::StaticSupportedModesManager sStaticSupportedModesManager; -app::Clusters::Actions::ActionsDelegateImpl sActionsDelegateImpl; constexpr EndpointId kNetworkCommissioningEndpointSecondary = 0xFFFE; @@ -128,7 +126,6 @@ static void InitServer(intptr_t context) app::Clusters::TemperatureControl::SetInstance(&sAppSupportedTemperatureLevelsDelegate); app::Clusters::ModeSelect::setSupportedModesManager(&sStaticSupportedModesManager); - app::Clusters::Actions::Instance::GetInstance()->SetDefaultDelegate(&sActionsDelegateImpl); } #include diff --git a/examples/all-clusters-app/linux/main-common.cpp b/examples/all-clusters-app/linux/main-common.cpp index c5a7c2189303a0..46a8ed3638c21c 100644 --- a/examples/all-clusters-app/linux/main-common.cpp +++ b/examples/all-clusters-app/linux/main-common.cpp @@ -54,6 +54,7 @@ #include #include #include +#include #include #include #include @@ -86,6 +87,7 @@ Clusters::TemperatureControl::AppSupportedTemperatureLevelsDelegate sAppSupporte Clusters::ModeSelect::StaticSupportedModesManager sStaticSupportedModesManager; Clusters::ValveConfigurationAndControl::ValveControlDelegate sValveDelegate; Clusters::TimeSynchronization::ExtendedTimeSyncDelegate sTimeSyncDelegate; +Clusters::Actions::ActionsDelegateImpl sActionsDelegateImpl; // Please refer to https://github.com/CHIP-Specifications/connectedhomeip-spec/blob/master/src/namespaces constexpr const uint8_t kNamespaceCommon = 7; @@ -340,6 +342,11 @@ void emberAfThermostatClusterInitCallback(EndpointId endpoint) SetDefaultDelegate(endpoint, &delegate); } +void emberAfActionsClusterInitCallback(EndpointId endpoint) +{ + Clusters::Actions::Instance::GetInstance()->SetDefaultDelegate(&sActionsDelegateImpl); +} + Status emberAfExternalAttributeReadCallback(EndpointId endpoint, ClusterId clusterId, const EmberAfAttributeMetadata * attributeMetadata, uint8_t * buffer, uint16_t maxReadLength) diff --git a/scripts/tools/check_includes_config.py b/scripts/tools/check_includes_config.py index a7da891ea59c12..e7d165a20d384c 100644 --- a/scripts/tools/check_includes_config.py +++ b/scripts/tools/check_includes_config.py @@ -128,6 +128,7 @@ 'src/app/clusters/application-launcher-server/application-launcher-server.cpp': {'string'}, 'src/app/clusters/application-launcher-server/application-launcher-delegate.h': {'list'}, 'src/app/clusters/audio-output-server/audio-output-delegate.h': {'list'}, + 'src/app/clusters/actions-server/actions-server.h': {'vector'}, # EcosystemInformationCluster is for Fabric Sync and is intended to run on device that are capable of handling these types. 'src/app/clusters/ecosystem-information-server/ecosystem-information-server.h': {'map', 'string', 'vector'}, 'src/app/clusters/channel-server/channel-delegate.h': {'list'}, From 91461d94aa3b6c043a095d8e6ddff66cc05966fe Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Thu, 19 Dec 2024 12:59:46 +0530 Subject: [PATCH 12/25] Address review changes --- src/app/clusters/actions-server/actions-server.cpp | 12 ++++-------- src/app/clusters/actions-server/actions-server.h | 2 ++ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/app/clusters/actions-server/actions-server.cpp b/src/app/clusters/actions-server/actions-server.cpp index 543e4e6aa078b7..766fcda7bd9ade 100644 --- a/src/app/clusters/actions-server/actions-server.cpp +++ b/src/app/clusters/actions-server/actions-server.cpp @@ -74,7 +74,7 @@ CHIP_ERROR Instance::ReadActionListAttribute(const AttributeValueEncoder::ListEn ChipLogError(Zcl, "Actions delegate is null!!!"); return CHIP_ERROR_INCORRECT_STATE; } - for (uint16_t i = 0; true; i++) + for (uint16_t i = 0; i <= kMaxActionList; i++) { ActionStructStorage action; @@ -96,7 +96,7 @@ CHIP_ERROR Instance::ReadEndpointListAttribute(const AttributeValueEncoder::List ChipLogError(Zcl, "Actions delegate is null!!!"); return CHIP_ERROR_INCORRECT_STATE; } - for (uint16_t i = 0; true; i++) + for (uint16_t i = 0; i <= kMaxEndpointList; i++) { EndpointListStorage epList; @@ -129,15 +129,11 @@ CHIP_ERROR Instance::Read(const ConcreteReadAttributePath & aPath, AttributeValu { case ActionList::Id: { Instance * d = this; - CHIP_ERROR err = - aEncoder.EncodeList([d](const auto & encoder) -> CHIP_ERROR { return d->ReadActionListAttribute(encoder); }); - return err; + return aEncoder.EncodeList([d](const auto & encoder) -> CHIP_ERROR { return d->ReadActionListAttribute(encoder); }); } case EndpointLists::Id: { Instance * d = this; - CHIP_ERROR err = - aEncoder.EncodeList([d](const auto & encoder) -> CHIP_ERROR { return d->ReadEndpointListAttribute(encoder); }); - return err; + return aEncoder.EncodeList([d](const auto & encoder) -> CHIP_ERROR { return d->ReadEndpointListAttribute(encoder); }); } default: break; diff --git a/src/app/clusters/actions-server/actions-server.h b/src/app/clusters/actions-server/actions-server.h index ba30566a1e3101..0c20ef0b041e5c 100644 --- a/src/app/clusters/actions-server/actions-server.h +++ b/src/app/clusters/actions-server/actions-server.h @@ -140,6 +140,8 @@ class Instance : public AttributeAccessInterface, public CommandHandlerInterface private: Delegate * mDelegate; static Instance instance; + static constexpr size_t kMaxEndpointList = 256u; + static constexpr size_t kMaxActionList = 256u; CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; From f7c8c4c5bbf893eb65246c9405c05293e44f0bd1 Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Fri, 20 Dec 2024 17:46:40 +0530 Subject: [PATCH 13/25] Change implementation to accomodate actions server on multiple endpoints. --- .../src/bridged-actions-stub.cpp | 2 +- .../all-clusters-app/linux/main-common.cpp | 2 +- .../actions-server/actions-server.cpp | 173 +++++++++++------- .../clusters/actions-server/actions-server.h | 49 ++--- 4 files changed, 129 insertions(+), 97 deletions(-) diff --git a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp index e5e7c98ae0d0c1..f59efbe24b2a59 100644 --- a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp +++ b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp @@ -1,6 +1,6 @@ /* * - * Copyright (c) 2024 Project CHIP Authors + * Copyright (c) 2021-2024 Project CHIP Authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/examples/all-clusters-app/linux/main-common.cpp b/examples/all-clusters-app/linux/main-common.cpp index 46a8ed3638c21c..1644639e5d1dd3 100644 --- a/examples/all-clusters-app/linux/main-common.cpp +++ b/examples/all-clusters-app/linux/main-common.cpp @@ -344,7 +344,7 @@ void emberAfThermostatClusterInitCallback(EndpointId endpoint) void emberAfActionsClusterInitCallback(EndpointId endpoint) { - Clusters::Actions::Instance::GetInstance()->SetDefaultDelegate(&sActionsDelegateImpl); + Clusters::Actions::ActionsServer::SetDefaultDelegate(endpoint, &sActionsDelegateImpl); } Status emberAfExternalAttributeReadCallback(EndpointId endpoint, ClusterId clusterId, diff --git a/src/app/clusters/actions-server/actions-server.cpp b/src/app/clusters/actions-server/actions-server.cpp index 766fcda7bd9ade..ec20bdf038ab96 100644 --- a/src/app/clusters/actions-server/actions-server.cpp +++ b/src/app/clusters/actions-server/actions-server.cpp @@ -32,53 +32,80 @@ using namespace chip::app::Clusters::Actions; using namespace chip::app::Clusters::Actions::Attributes; using namespace chip::Protocols::InteractionModel; -Instance Instance::instance; -Instance * Instance::GetInstance() +static constexpr size_t kActionsDelegateTableSize = + MATTER_DM_ACTIONS_CLUSTER_SERVER_ENDPOINT_COUNT + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT; + +namespace { +Delegate * gDelegateTable[kActionsDelegateTableSize] = { nullptr }; + +Delegate * GetDelegate(EndpointId endpoint) +{ + uint16_t ep = emberAfGetClusterServerEndpointIndex(endpoint, Actions::Id, MATTER_DM_ACTIONS_CLUSTER_SERVER_ENDPOINT_COUNT); + return (ep >= kActionsDelegateTableSize ? nullptr : gDelegateTable[ep]); +} + +} // namespace + +ActionsServer ActionsServer::sInstance; + +void ActionsServer::SetDefaultDelegate(EndpointId endpoint, Delegate * delegate) +{ + uint16_t ep = emberAfGetClusterServerEndpointIndex(endpoint, Actions::Id, MATTER_DM_ACTIONS_CLUSTER_SERVER_ENDPOINT_COUNT); + // if endpoint is found + if (ep < kActionsDelegateTableSize) + { + gDelegateTable[ep] = delegate; + } +} + +ActionsServer & ActionsServer::Instance() { - return &instance; + return sInstance; } -void Instance::OnStateChanged(EndpointId endpoint, uint16_t actionId, uint32_t invokeId, ActionStateEnum actionState) +void ActionsServer::OnStateChanged(EndpointId endpoint, uint16_t actionId, uint32_t invokeId, ActionStateEnum actionState) { ChipLogProgress(Zcl, "ActionsServer: OnStateChanged"); - // Record StateChanged event + // Generate StateChanged event EventNumber eventNumber; Events::StateChanged::Type event{ actionId, invokeId, actionState }; if (CHIP_NO_ERROR != LogEvent(event, endpoint, eventNumber)) { - ChipLogError(Zcl, "ActionsServer: Failed to record OnStateChanged event"); + ChipLogError(Zcl, "ActionsServer: Failed to generate OnStateChanged event"); } } -void Instance::OnActionFailed(EndpointId endpoint, uint16_t actionId, uint32_t invokeId, ActionStateEnum actionState, - ActionErrorEnum actionError) +void ActionsServer::OnActionFailed(EndpointId endpoint, uint16_t actionId, uint32_t invokeId, ActionStateEnum actionState, + ActionErrorEnum actionError) { ChipLogProgress(Zcl, "ActionsServer: OnActionFailed"); - // Record ActionFailed event + // Generate ActionFailed event EventNumber eventNumber; Events::ActionFailed::Type event{ actionId, invokeId, actionState, actionError }; if (CHIP_NO_ERROR != LogEvent(event, endpoint, eventNumber)) { - ChipLogError(Zcl, "ActionsServer: Failed to record OnActionFailed event"); + ChipLogError(Zcl, "ActionsServer: Failed to generate OnActionFailed event"); } } -CHIP_ERROR Instance::ReadActionListAttribute(const AttributeValueEncoder::ListEncodeHelper & encoder) +CHIP_ERROR ActionsServer::ReadActionListAttribute(const ConcreteReadAttributePath & aPath, + const AttributeValueEncoder::ListEncodeHelper & encoder) { - if (GetInstance()->mDelegate == nullptr) + Delegate * delegate = GetDelegate(aPath.mEndpointId); + if (delegate == nullptr) { ChipLogError(Zcl, "Actions delegate is null!!!"); return CHIP_ERROR_INCORRECT_STATE; } - for (uint16_t i = 0; i <= kMaxActionList; i++) + for (uint16_t i = 0; i <= kMaxActionListLength; i++) { ActionStructStorage action; - CHIP_ERROR err = GetInstance()->mDelegate->ReadActionAtIndex(i, action); + CHIP_ERROR err = delegate->ReadActionAtIndex(i, action); if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED) { return CHIP_NO_ERROR; @@ -89,18 +116,20 @@ CHIP_ERROR Instance::ReadActionListAttribute(const AttributeValueEncoder::ListEn return CHIP_NO_ERROR; } -CHIP_ERROR Instance::ReadEndpointListAttribute(const AttributeValueEncoder::ListEncodeHelper & encoder) +CHIP_ERROR ActionsServer::ReadEndpointListAttribute(const ConcreteReadAttributePath & aPath, + const AttributeValueEncoder::ListEncodeHelper & encoder) { - if (GetInstance()->mDelegate == nullptr) + Delegate * delegate = GetDelegate(aPath.mEndpointId); + if (delegate == nullptr) { ChipLogError(Zcl, "Actions delegate is null!!!"); return CHIP_ERROR_INCORRECT_STATE; } - for (uint16_t i = 0; i <= kMaxEndpointList; i++) + for (uint16_t i = 0; i <= kMaxEndpointListLength; i++) { EndpointListStorage epList; - CHIP_ERROR err = GetInstance()->mDelegate->ReadEndpointListAtIndex(i, epList); + CHIP_ERROR err = delegate->ReadEndpointListAtIndex(i, epList); if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED) { return CHIP_NO_ERROR; @@ -111,29 +140,19 @@ CHIP_ERROR Instance::ReadEndpointListAttribute(const AttributeValueEncoder::List return CHIP_NO_ERROR; } -void Instance::SetDefaultDelegate(Delegate * aDelegate) -{ - if (aDelegate == nullptr) - { - ChipLogError(Zcl, "Cannot set empty delegate!!!"); - return; - } - mDelegate = aDelegate; -} - -CHIP_ERROR Instance::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) +CHIP_ERROR ActionsServer::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) { VerifyOrDie(aPath.mClusterId == Actions::Id); switch (aPath.mAttributeId) { case ActionList::Id: { - Instance * d = this; - return aEncoder.EncodeList([d](const auto & encoder) -> CHIP_ERROR { return d->ReadActionListAttribute(encoder); }); + return aEncoder.EncodeList( + [this, aPath](const auto & encoder) -> CHIP_ERROR { return this->ReadActionListAttribute(aPath, encoder); }); } case EndpointLists::Id: { - Instance * d = this; - return aEncoder.EncodeList([d](const auto & encoder) -> CHIP_ERROR { return d->ReadEndpointListAttribute(encoder); }); + return aEncoder.EncodeList( + [this, aPath](const auto & encoder) -> CHIP_ERROR { return this->ReadEndpointListAttribute(aPath, encoder); }); } default: break; @@ -141,18 +160,19 @@ CHIP_ERROR Instance::Read(const ConcreteReadAttributePath & aPath, AttributeValu return CHIP_NO_ERROR; } -bool Instance::FindActionIdInActionList(uint16_t actionId) +bool ActionsServer::FindActionIdInActionList(EndpointId endpointId, uint16_t actionId) { - if (GetInstance()->mDelegate == nullptr) + Delegate * delegate = GetDelegate(endpointId); + if (delegate == nullptr) { ChipLogError(Zcl, "Actions delegate is null!!!"); return false; } - return GetInstance()->mDelegate->FindActionIdInActionList(actionId); + return delegate->FindActionIdInActionList(actionId); } template -void Instance::HandleCommand(HandlerContext & handlerContext, FuncT func) +void ActionsServer::HandleCommand(HandlerContext & handlerContext, FuncT func) { if (!handlerContext.mCommandHandled && (handlerContext.mRequestPath.mCommandId == RequestT::GetCommandId())) { @@ -172,8 +192,7 @@ void Instance::HandleCommand(HandlerContext & handlerContext, FuncT func) return; } - uint16_t actionId = requestPayload.actionID; - if (!GetInstance()->FindActionIdInActionList(actionId)) + if (FindActionIdInActionList(handlerContext.mRequestPath.mEndpointId, requestPayload.actionID)) { handlerContext.mCommandHandler.AddStatus(handlerContext.mRequestPath, Protocols::InteractionModel::Status::NotFound); return; @@ -183,113 +202,125 @@ void Instance::HandleCommand(HandlerContext & handlerContext, FuncT func) } } -void Instance::HandleInstantAction(HandlerContext & ctx, const Commands::InstantAction::DecodableType & commandData) +void ActionsServer::HandleInstantAction(HandlerContext & ctx, const Commands::InstantAction::DecodableType & commandData) { uint16_t actionId = commandData.actionID; Optional invokeId = commandData.invokeID; - Status status = GetInstance()->mDelegate->HandleInstantAction(actionId, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandleInstantAction(actionId, invokeId); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } -void Instance::HandleInstantActionWithTransition(HandlerContext & ctx, - const Commands::InstantActionWithTransition::DecodableType & commandData) +void ActionsServer::HandleInstantActionWithTransition(HandlerContext & ctx, + const Commands::InstantActionWithTransition::DecodableType & commandData) { uint16_t actionId = commandData.actionID; Optional invokeId = commandData.invokeID; uint16_t transitionTime = commandData.transitionTime; - Status status = GetInstance()->mDelegate->HandleInstantActionWithTransition(actionId, transitionTime, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandleInstantActionWithTransition(actionId, transitionTime, invokeId); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } -void Instance::HandleStartAction(HandlerContext & ctx, const Commands::StartAction::DecodableType & commandData) +void ActionsServer::HandleStartAction(HandlerContext & ctx, const Commands::StartAction::DecodableType & commandData) { uint16_t actionId = commandData.actionID; Optional invokeId = commandData.invokeID; - Status status = GetInstance()->mDelegate->HandleStartAction(actionId, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandleStartAction(actionId, invokeId); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } -void Instance::HandleStartActionWithDuration(HandlerContext & ctx, - const Commands::StartActionWithDuration::DecodableType & commandData) +void ActionsServer::HandleStartActionWithDuration(HandlerContext & ctx, + const Commands::StartActionWithDuration::DecodableType & commandData) { uint16_t actionId = commandData.actionID; Optional invokeId = commandData.invokeID; uint32_t duration = commandData.duration; - Status status = GetInstance()->mDelegate->HandleStartActionWithDuration(actionId, duration, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandleStartActionWithDuration(actionId, duration, invokeId); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } -void Instance::HandleStopAction(HandlerContext & ctx, const Commands::StopAction::DecodableType & commandData) +void ActionsServer::HandleStopAction(HandlerContext & ctx, const Commands::StopAction::DecodableType & commandData) { uint16_t actionId = commandData.actionID; Optional invokeId = commandData.invokeID; - Status status = GetInstance()->mDelegate->HandleStopAction(actionId, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandleStopAction(actionId, invokeId); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } -void Instance::HandlePauseAction(HandlerContext & ctx, const Commands::PauseAction::DecodableType & commandData) +void ActionsServer::HandlePauseAction(HandlerContext & ctx, const Commands::PauseAction::DecodableType & commandData) { uint16_t actionId = commandData.actionID; Optional invokeId = commandData.invokeID; - Status status = GetInstance()->mDelegate->HandlePauseAction(actionId, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandlePauseAction(actionId, invokeId); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } -void Instance::HandlePauseActionWithDuration(HandlerContext & ctx, - const Commands::PauseActionWithDuration::DecodableType & commandData) +void ActionsServer::HandlePauseActionWithDuration(HandlerContext & ctx, + const Commands::PauseActionWithDuration::DecodableType & commandData) { uint16_t actionId = commandData.actionID; Optional invokeId = commandData.invokeID; uint32_t duration = commandData.duration; - Status status = GetInstance()->mDelegate->HandlePauseActionWithDuration(actionId, duration, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandlePauseActionWithDuration(actionId, duration, invokeId); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } -void Instance::HandleResumeAction(HandlerContext & ctx, const Commands::ResumeAction::DecodableType & commandData) +void ActionsServer::HandleResumeAction(HandlerContext & ctx, const Commands::ResumeAction::DecodableType & commandData) { uint16_t actionId = commandData.actionID; Optional invokeId = commandData.invokeID; - Status status = GetInstance()->mDelegate->HandleResumeAction(actionId, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandleResumeAction(actionId, invokeId); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } -void Instance::HandleEnableAction(HandlerContext & ctx, const Commands::EnableAction::DecodableType & commandData) +void ActionsServer::HandleEnableAction(HandlerContext & ctx, const Commands::EnableAction::DecodableType & commandData) { uint16_t actionId = commandData.actionID; Optional invokeId = commandData.invokeID; - Status status = GetInstance()->mDelegate->HandleEnableAction(actionId, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandleEnableAction(actionId, invokeId); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } -void Instance::HandleEnableActionWithDuration(HandlerContext & ctx, - const Commands::EnableActionWithDuration::DecodableType & commandData) +void ActionsServer::HandleEnableActionWithDuration(HandlerContext & ctx, + const Commands::EnableActionWithDuration::DecodableType & commandData) { uint16_t actionId = commandData.actionID; Optional invokeId = commandData.invokeID; uint32_t duration = commandData.duration; - Status status = GetInstance()->mDelegate->HandleEnableActionWithDuration(actionId, duration, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandleEnableActionWithDuration(actionId, duration, invokeId); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } -void Instance::HandleDisableAction(HandlerContext & ctx, const Commands::DisableAction::DecodableType & commandData) +void ActionsServer::HandleDisableAction(HandlerContext & ctx, const Commands::DisableAction::DecodableType & commandData) { uint16_t actionId = commandData.actionID; Optional invokeId = commandData.invokeID; - Status status = GetInstance()->mDelegate->HandleDisableAction(actionId, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandleDisableAction(actionId, invokeId); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } -void Instance::HandleDisableActionWithDuration(HandlerContext & ctx, - const Commands::DisableActionWithDuration::DecodableType & commandData) +void ActionsServer::HandleDisableActionWithDuration(HandlerContext & ctx, + const Commands::DisableActionWithDuration::DecodableType & commandData) { uint16_t actionId = commandData.actionID; Optional invokeId = commandData.invokeID; uint32_t duration = commandData.duration; - Status status = GetInstance()->mDelegate->HandleDisableActionWithDuration(actionId, duration, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandleDisableActionWithDuration(actionId, duration, invokeId); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } -void Instance::InvokeCommand(HandlerContext & handlerContext) +void ActionsServer::InvokeCommand(HandlerContext & handlerContext) { switch (handlerContext.mRequestPath.mCommandId) { @@ -350,6 +381,6 @@ void Instance::InvokeCommand(HandlerContext & handlerContext) } void MatterActionsPluginServerInitCallback() { - AttributeAccessInterfaceRegistry::Instance().Register(Instance::GetInstance()); - CommandHandlerInterfaceRegistry::Instance().RegisterCommandHandler(Instance::GetInstance()); + AttributeAccessInterfaceRegistry::Instance().Register(&ActionsServer::Instance()); + CommandHandlerInterfaceRegistry::Instance().RegisterCommandHandler(&ActionsServer::Instance()); } diff --git a/src/app/clusters/actions-server/actions-server.h b/src/app/clusters/actions-server/actions-server.h index 0c20ef0b041e5c..3c1570252e1326 100644 --- a/src/app/clusters/actions-server/actions-server.h +++ b/src/app/clusters/actions-server/actions-server.h @@ -36,16 +36,16 @@ class Delegate; struct ActionStructStorage : public Structs::ActionStruct::Type { - ActionStructStorage() : mActionName(mBuffer, sizeof(mBuffer)){}; + ActionStructStorage() : mActionName(mBuffer){}; - ActionStructStorage(uint16_t action, CharSpan actionName, ActionTypeEnum actionType, uint16_t epListID, + ActionStructStorage(uint16_t action, const CharSpan & actionName, ActionTypeEnum actionType, uint16_t epListID, BitMask commands, ActionStateEnum actionState) : - mActionName(mBuffer, sizeof(mBuffer)) + mActionName(mBuffer) { Set(action, actionName, actionType, epListID, commands, actionState); } - ActionStructStorage(const ActionStructStorage & action) : mActionName(mBuffer, sizeof(mBuffer)) { *this = action; } + ActionStructStorage(const ActionStructStorage & action) : mActionName(mBuffer) { *this = action; } ActionStructStorage & operator=(const ActionStructStorage & action) { @@ -53,8 +53,8 @@ struct ActionStructStorage : public Structs::ActionStruct::Type return *this; } - void Set(uint16_t action, CharSpan actionName, ActionTypeEnum actionType, uint16_t epListID, BitMask commands, - ActionStateEnum actionState) + void Set(uint16_t action, const CharSpan & actionName, ActionTypeEnum actionType, uint16_t epListID, + BitMask commands, ActionStateEnum actionState) { actionID = action; type = actionType; @@ -72,16 +72,16 @@ struct ActionStructStorage : public Structs::ActionStruct::Type struct EndpointListStorage : public Structs::EndpointListStruct::Type { - EndpointListStorage() : mEpListName(mBuffer, sizeof(mBuffer)){}; + EndpointListStorage() : mEpListName(mBuffer){}; - EndpointListStorage(uint16_t epListId, CharSpan epListName, EndpointListTypeEnum epListType, - DataModel::List endpointList) : - mEpListName(mBuffer, sizeof(mBuffer)) + EndpointListStorage(uint16_t epListId, const CharSpan & epListName, EndpointListTypeEnum epListType, + const DataModel::List & endpointList) : + mEpListName(mBuffer) { Set(epListId, epListName, epListType, endpointList); } - EndpointListStorage(const EndpointListStorage & epList) : mEpListName(mBuffer, sizeof(mBuffer)) { *this = epList; } + EndpointListStorage(const EndpointListStorage & epList) : mEpListName(mBuffer) { *this = epList; } EndpointListStorage & operator=(const EndpointListStorage & epList) { @@ -89,8 +89,8 @@ struct EndpointListStorage : public Structs::EndpointListStruct::Type return *this; } - void Set(uint16_t epListId, CharSpan epListName, EndpointListTypeEnum epListType, - DataModel::List endpointList) + void Set(uint16_t epListId, const CharSpan & epListName, EndpointListTypeEnum epListType, + const DataModel::List & endpointList) { endpointListID = epListId; type = epListType; @@ -112,14 +112,15 @@ struct EndpointListStorage : public Structs::EndpointListStruct::Type Span mEpListSpan; }; -class Instance : public AttributeAccessInterface, public CommandHandlerInterface +class ActionsServer : public AttributeAccessInterface, public CommandHandlerInterface { public: // Register for the Actions cluster on all endpoints. - Instance() : + ActionsServer() : AttributeAccessInterface(Optional::Missing(), Actions::Id), CommandHandlerInterface(Optional::Missing(), Actions::Id) {} + static ActionsServer & Instance(); /** * @brief @@ -134,20 +135,20 @@ class Instance : public AttributeAccessInterface, public CommandHandlerInterface void OnActionFailed(EndpointId endpoint, uint16_t actionId, uint32_t invokeId, ActionStateEnum actionState, ActionErrorEnum actionError); - static Instance * GetInstance(); - void SetDefaultDelegate(Delegate * aDelegate); + static void SetDefaultDelegate(EndpointId endpointId, Delegate * aDelegate); private: - Delegate * mDelegate; - static Instance instance; - static constexpr size_t kMaxEndpointList = 256u; - static constexpr size_t kMaxActionList = 256u; + static ActionsServer sInstance; + static constexpr size_t kMaxEndpointListLength = 256u; + static constexpr size_t kMaxActionListLength = 256u; CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; - CHIP_ERROR ReadActionListAttribute(const AttributeValueEncoder::ListEncodeHelper & encoder); - CHIP_ERROR ReadEndpointListAttribute(const AttributeValueEncoder::ListEncodeHelper & encoder); - bool FindActionIdInActionList(uint16_t actionId); + CHIP_ERROR ReadActionListAttribute(const ConcreteReadAttributePath & aPath, + const AttributeValueEncoder::ListEncodeHelper & encoder); + CHIP_ERROR ReadEndpointListAttribute(const ConcreteReadAttributePath & aPath, + const AttributeValueEncoder::ListEncodeHelper & encoder); + bool FindActionIdInActionList(EndpointId endpointId, uint16_t actionId); // CommandHandlerInterface template From c4023252761584ff56dba617ccfabe6fa39230c6 Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Tue, 24 Dec 2024 15:44:09 +0530 Subject: [PATCH 14/25] Restructured the storage structs, command handler and removed vector usage. --- scripts/tools/check_includes_config.py | 1 - .../actions-server/actions-server.cpp | 78 ++++++------------- .../clusters/actions-server/actions-server.h | 39 +++++----- 3 files changed, 45 insertions(+), 73 deletions(-) diff --git a/scripts/tools/check_includes_config.py b/scripts/tools/check_includes_config.py index e7d165a20d384c..a7da891ea59c12 100644 --- a/scripts/tools/check_includes_config.py +++ b/scripts/tools/check_includes_config.py @@ -128,7 +128,6 @@ 'src/app/clusters/application-launcher-server/application-launcher-server.cpp': {'string'}, 'src/app/clusters/application-launcher-server/application-launcher-delegate.h': {'list'}, 'src/app/clusters/audio-output-server/audio-output-delegate.h': {'list'}, - 'src/app/clusters/actions-server/actions-server.h': {'vector'}, # EcosystemInformationCluster is for Fabric Sync and is intended to run on device that are capable of handling these types. 'src/app/clusters/ecosystem-information-server/ecosystem-information-server.h': {'map', 'string', 'vector'}, 'src/app/clusters/channel-server/channel-delegate.h': {'list'}, diff --git a/src/app/clusters/actions-server/actions-server.cpp b/src/app/clusters/actions-server/actions-server.cpp index ec20bdf038ab96..50d5b548de803b 100644 --- a/src/app/clusters/actions-server/actions-server.cpp +++ b/src/app/clusters/actions-server/actions-server.cpp @@ -204,119 +204,91 @@ void ActionsServer::HandleCommand(HandlerContext & handlerContext, FuncT func) void ActionsServer::HandleInstantAction(HandlerContext & ctx, const Commands::InstantAction::DecodableType & commandData) { - uint16_t actionId = commandData.actionID; - Optional invokeId = commandData.invokeID; - Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandleInstantAction(actionId, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandleInstantAction(commandData.actionID, commandData.invokeID); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } void ActionsServer::HandleInstantActionWithTransition(HandlerContext & ctx, const Commands::InstantActionWithTransition::DecodableType & commandData) { - uint16_t actionId = commandData.actionID; - Optional invokeId = commandData.invokeID; - uint16_t transitionTime = commandData.transitionTime; - Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandleInstantActionWithTransition(actionId, transitionTime, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = + delegate->HandleInstantActionWithTransition(commandData.actionID, commandData.transitionTime, commandData.invokeID); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } void ActionsServer::HandleStartAction(HandlerContext & ctx, const Commands::StartAction::DecodableType & commandData) { - uint16_t actionId = commandData.actionID; - Optional invokeId = commandData.invokeID; - Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandleStartAction(actionId, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandleStartAction(commandData.actionID, commandData.invokeID); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } void ActionsServer::HandleStartActionWithDuration(HandlerContext & ctx, const Commands::StartActionWithDuration::DecodableType & commandData) { - uint16_t actionId = commandData.actionID; - Optional invokeId = commandData.invokeID; - uint32_t duration = commandData.duration; - Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandleStartActionWithDuration(actionId, duration, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandleStartActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } void ActionsServer::HandleStopAction(HandlerContext & ctx, const Commands::StopAction::DecodableType & commandData) { - uint16_t actionId = commandData.actionID; - Optional invokeId = commandData.invokeID; - Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandleStopAction(actionId, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandleStopAction(commandData.actionID, commandData.invokeID); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } void ActionsServer::HandlePauseAction(HandlerContext & ctx, const Commands::PauseAction::DecodableType & commandData) { - uint16_t actionId = commandData.actionID; - Optional invokeId = commandData.invokeID; - Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandlePauseAction(actionId, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandlePauseAction(commandData.actionID, commandData.invokeID); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } void ActionsServer::HandlePauseActionWithDuration(HandlerContext & ctx, const Commands::PauseActionWithDuration::DecodableType & commandData) { - uint16_t actionId = commandData.actionID; - Optional invokeId = commandData.invokeID; - uint32_t duration = commandData.duration; - Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandlePauseActionWithDuration(actionId, duration, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandlePauseActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } void ActionsServer::HandleResumeAction(HandlerContext & ctx, const Commands::ResumeAction::DecodableType & commandData) { - uint16_t actionId = commandData.actionID; - Optional invokeId = commandData.invokeID; - Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandleResumeAction(actionId, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandleResumeAction(commandData.actionID, commandData.invokeID); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } void ActionsServer::HandleEnableAction(HandlerContext & ctx, const Commands::EnableAction::DecodableType & commandData) { - uint16_t actionId = commandData.actionID; - Optional invokeId = commandData.invokeID; - Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandleEnableAction(actionId, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandleEnableAction(commandData.actionID, commandData.invokeID); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } void ActionsServer::HandleEnableActionWithDuration(HandlerContext & ctx, const Commands::EnableActionWithDuration::DecodableType & commandData) { - uint16_t actionId = commandData.actionID; - Optional invokeId = commandData.invokeID; - uint32_t duration = commandData.duration; - Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandleEnableActionWithDuration(actionId, duration, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandleEnableActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } void ActionsServer::HandleDisableAction(HandlerContext & ctx, const Commands::DisableAction::DecodableType & commandData) { - uint16_t actionId = commandData.actionID; - Optional invokeId = commandData.invokeID; - Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandleDisableAction(actionId, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandleDisableAction(commandData.actionID, commandData.invokeID); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } void ActionsServer::HandleDisableActionWithDuration(HandlerContext & ctx, const Commands::DisableActionWithDuration::DecodableType & commandData) { - uint16_t actionId = commandData.actionID; - Optional invokeId = commandData.invokeID; - uint32_t duration = commandData.duration; - Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandleDisableActionWithDuration(actionId, duration, invokeId); + Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); + Status status = delegate->HandleDisableActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID); ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } diff --git a/src/app/clusters/actions-server/actions-server.h b/src/app/clusters/actions-server/actions-server.h index 3c1570252e1326..bdb41f69cad771 100644 --- a/src/app/clusters/actions-server/actions-server.h +++ b/src/app/clusters/actions-server/actions-server.h @@ -21,31 +21,32 @@ #include #include #include -#include namespace chip { namespace app { namespace Clusters { namespace Actions { -static constexpr size_t kActionNameMaxSize = 128u; -static constexpr size_t kEndpointListNameMaxSize = 128u; -static constexpr size_t kEndpointListMaxSize = 256u; +// Last byte is reserved of '\0' terminator. +static constexpr size_t kActionNameMaxSize = 127u; +static constexpr size_t kEndpointListNameMaxSize = 127u; + +static constexpr size_t kEndpointListMaxSize = 256u; class Delegate; struct ActionStructStorage : public Structs::ActionStruct::Type { - ActionStructStorage() : mActionName(mBuffer){}; + ActionStructStorage() : mActionName(mBuffer, sizeof(mBuffer)){}; ActionStructStorage(uint16_t action, const CharSpan & actionName, ActionTypeEnum actionType, uint16_t epListID, BitMask commands, ActionStateEnum actionState) : - mActionName(mBuffer) + mActionName(mBuffer, sizeof(mBuffer)) { Set(action, actionName, actionType, epListID, commands, actionState); } - ActionStructStorage(const ActionStructStorage & action) : mActionName(mBuffer) { *this = action; } + ActionStructStorage(const ActionStructStorage & action) : mActionName(mBuffer, sizeof(mBuffer)) { *this = action; } ActionStructStorage & operator=(const ActionStructStorage & action) { @@ -72,16 +73,16 @@ struct ActionStructStorage : public Structs::ActionStruct::Type struct EndpointListStorage : public Structs::EndpointListStruct::Type { - EndpointListStorage() : mEpListName(mBuffer){}; + EndpointListStorage() : mEpListName(mBuffer, sizeof(mBuffer)){}; EndpointListStorage(uint16_t epListId, const CharSpan & epListName, EndpointListTypeEnum epListType, const DataModel::List & endpointList) : - mEpListName(mBuffer) + mEpListName(mBuffer, sizeof(mBuffer)) { Set(epListId, epListName, epListType, endpointList); } - EndpointListStorage(const EndpointListStorage & epList) : mEpListName(mBuffer) { *this = epList; } + EndpointListStorage(const EndpointListStorage & epList) : mEpListName(mBuffer, sizeof(mBuffer)) { *this = epList; } EndpointListStorage & operator=(const EndpointListStorage & epList) { @@ -92,15 +93,15 @@ struct EndpointListStorage : public Structs::EndpointListStruct::Type void Set(uint16_t epListId, const CharSpan & epListName, EndpointListTypeEnum epListType, const DataModel::List & endpointList) { - endpointListID = epListId; - type = epListType; + endpointListID = epListId; + type = epListType; + size_t epListSize = std::min(endpointList.size(), kEndpointListMaxSize); - for (uint8_t index = 0; index < std::min(endpointList.size(), kEndpointListMaxSize); index++) + for (uint8_t index = 0; index < epListSize; index++) { - mEpList.push_back(endpointList[index]); + mEpList[index] = endpointList[index]; } - mEpListSpan = Span(mEpList.data(), mEpList.size()); - endpoints = DataModel::List(mEpListSpan); + endpoints = DataModel::List(Span(mEpList, epListSize)); CopyCharSpanToMutableCharSpanWithTruncation(epListName, mEpListName); name = mEpListName; } @@ -108,8 +109,7 @@ struct EndpointListStorage : public Structs::EndpointListStruct::Type private: char mBuffer[kEndpointListNameMaxSize]; MutableCharSpan mEpListName; - std::vector mEpList; - Span mEpListSpan; + EndpointId mEpList[kEndpointListMaxSize]; }; class ActionsServer : public AttributeAccessInterface, public CommandHandlerInterface @@ -150,7 +150,8 @@ class ActionsServer : public AttributeAccessInterface, public CommandHandlerInte const AttributeValueEncoder::ListEncodeHelper & encoder); bool FindActionIdInActionList(EndpointId endpointId, uint16_t actionId); - // CommandHandlerInterface + // Cannot use CommandHandlerInterface::HandleCommand directly because we need to do the FindActionIdInActionList() check before + // sending a command. template void HandleCommand(HandlerContext & handlerContext, FuncT func); From ca3b650730f4ddfaf27d71edf4378e2030a8b05f Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Mon, 27 Jan 2025 16:49:22 +0530 Subject: [PATCH 15/25] Add unit tests for the actions cluster. --- .../include/bridged-actions-stub.h | 2 +- .../actions-server/actions-server.cpp | 67 +-- .../clusters/actions-server/actions-server.h | 16 +- src/app/tests/BUILD.gn | 15 + src/app/tests/TestActionsCluster.cpp | 445 ++++++++++++++++++ 5 files changed, 509 insertions(+), 36 deletions(-) create mode 100644 src/app/tests/TestActionsCluster.cpp diff --git a/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h b/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h index ca8b0e26d2004d..d58269313b648e 100644 --- a/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h +++ b/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h @@ -41,7 +41,7 @@ class ActionsDelegateImpl : public Delegate }; // Dummy endpoint list. - const EndpointId firstEpList[1] = { 0 }; + const EndpointId firstEpList[3] = { 0, 1, 2 }; const EndpointId secondEpList[1] = { 0 }; const EndpointId thirdEpList[1] = { 0 }; diff --git a/src/app/clusters/actions-server/actions-server.cpp b/src/app/clusters/actions-server/actions-server.cpp index 50d5b548de803b..a8fcd476eb2345 100644 --- a/src/app/clusters/actions-server/actions-server.cpp +++ b/src/app/clusters/actions-server/actions-server.cpp @@ -32,16 +32,12 @@ using namespace chip::app::Clusters::Actions; using namespace chip::app::Clusters::Actions::Attributes; using namespace chip::Protocols::InteractionModel; -static constexpr size_t kActionsDelegateTableSize = - MATTER_DM_ACTIONS_CLUSTER_SERVER_ENDPOINT_COUNT + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT; - namespace { Delegate * gDelegateTable[kActionsDelegateTableSize] = { nullptr }; Delegate * GetDelegate(EndpointId endpoint) { - uint16_t ep = emberAfGetClusterServerEndpointIndex(endpoint, Actions::Id, MATTER_DM_ACTIONS_CLUSTER_SERVER_ENDPOINT_COUNT); - return (ep >= kActionsDelegateTableSize ? nullptr : gDelegateTable[ep]); + return (endpoint >= kActionsDelegateTableSize ? nullptr : gDelegateTable[endpoint]); } } // namespace @@ -50,11 +46,9 @@ ActionsServer ActionsServer::sInstance; void ActionsServer::SetDefaultDelegate(EndpointId endpoint, Delegate * delegate) { - uint16_t ep = emberAfGetClusterServerEndpointIndex(endpoint, Actions::Id, MATTER_DM_ACTIONS_CLUSTER_SERVER_ENDPOINT_COUNT); - // if endpoint is found - if (ep < kActionsDelegateTableSize) + if (endpoint < kActionsDelegateTableSize) { - gDelegateTable[ep] = delegate; + gDelegateTable[endpoint] = delegate; } } @@ -92,6 +86,30 @@ void ActionsServer::OnActionFailed(EndpointId endpoint, uint16_t actionId, uint3 } } +CHIP_ERROR ActionsServer::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) +{ + VerifyOrDie(aPath.mClusterId == Actions::Id); + + switch (aPath.mAttributeId) + { + case ActionList::Id: { + // The encoder will automatically create the outer AttributeDataIB container + // We just need to encode the array of actions + ReturnErrorOnFailure(aEncoder.EncodeList( + [this, aPath](const auto & encoder) -> CHIP_ERROR { return this->ReadActionListAttribute(aPath, encoder); })); + return CHIP_NO_ERROR; + } + case EndpointLists::Id: { + ReturnErrorOnFailure(aEncoder.EncodeList( + [this, aPath](const auto & encoder) -> CHIP_ERROR { return this->ReadEndpointListAttribute(aPath, encoder); })); + return CHIP_NO_ERROR; + } + default: + break; + } + return CHIP_NO_ERROR; +} + CHIP_ERROR ActionsServer::ReadActionListAttribute(const ConcreteReadAttributePath & aPath, const AttributeValueEncoder::ListEncodeHelper & encoder) { @@ -101,15 +119,20 @@ CHIP_ERROR ActionsServer::ReadActionListAttribute(const ConcreteReadAttributePat ChipLogError(Zcl, "Actions delegate is null!!!"); return CHIP_ERROR_INCORRECT_STATE; } - for (uint16_t i = 0; i <= kMaxActionListLength; i++) + + for (uint16_t i = 0; i < kMaxActionListLength; i++) { ActionStructStorage action; - CHIP_ERROR err = delegate->ReadActionAtIndex(i, action); + if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED) { return CHIP_NO_ERROR; } + else if (err != CHIP_NO_ERROR) + { + return err; + } ReturnErrorOnFailure(encoder.Encode(action)); } @@ -125,7 +148,7 @@ CHIP_ERROR ActionsServer::ReadEndpointListAttribute(const ConcreteReadAttributeP ChipLogError(Zcl, "Actions delegate is null!!!"); return CHIP_ERROR_INCORRECT_STATE; } - for (uint16_t i = 0; i <= kMaxEndpointListLength; i++) + for (uint16_t i = 0; i < kMaxEndpointListLength; i++) { EndpointListStorage epList; @@ -140,26 +163,6 @@ CHIP_ERROR ActionsServer::ReadEndpointListAttribute(const ConcreteReadAttributeP return CHIP_NO_ERROR; } -CHIP_ERROR ActionsServer::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) -{ - VerifyOrDie(aPath.mClusterId == Actions::Id); - - switch (aPath.mAttributeId) - { - case ActionList::Id: { - return aEncoder.EncodeList( - [this, aPath](const auto & encoder) -> CHIP_ERROR { return this->ReadActionListAttribute(aPath, encoder); }); - } - case EndpointLists::Id: { - return aEncoder.EncodeList( - [this, aPath](const auto & encoder) -> CHIP_ERROR { return this->ReadEndpointListAttribute(aPath, encoder); }); - } - default: - break; - } - return CHIP_NO_ERROR; -} - bool ActionsServer::FindActionIdInActionList(EndpointId endpointId, uint16_t actionId) { Delegate * delegate = GetDelegate(endpointId); diff --git a/src/app/clusters/actions-server/actions-server.h b/src/app/clusters/actions-server/actions-server.h index bdb41f69cad771..162cb40fea215a 100644 --- a/src/app/clusters/actions-server/actions-server.h +++ b/src/app/clusters/actions-server/actions-server.h @@ -22,6 +22,16 @@ #include #include +namespace { +// Zero'th index will be used for unit tests only. +static constexpr size_t kActionsDelegateTableSize = +#ifdef ZCL_USING_ACTIONS_CLUSTER_SERVER + MATTER_DM_ACTIONS_CLUSTER_SERVER_ENDPOINT_COUNT + +#endif + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT + 1; + +} // namespace + namespace chip { namespace app { namespace Clusters { @@ -97,7 +107,7 @@ struct EndpointListStorage : public Structs::EndpointListStruct::Type type = epListType; size_t epListSize = std::min(endpointList.size(), kEndpointListMaxSize); - for (uint8_t index = 0; index < epListSize; index++) + for (size_t index = 0; index < epListSize; index++) { mEpList[index] = endpointList[index]; } @@ -137,13 +147,13 @@ class ActionsServer : public AttributeAccessInterface, public CommandHandlerInte static void SetDefaultDelegate(EndpointId endpointId, Delegate * aDelegate); + CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; + private: static ActionsServer sInstance; static constexpr size_t kMaxEndpointListLength = 256u; static constexpr size_t kMaxActionListLength = 256u; - CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; - CHIP_ERROR ReadActionListAttribute(const ConcreteReadAttributePath & aPath, const AttributeValueEncoder::ListEncodeHelper & encoder); CHIP_ERROR ReadEndpointListAttribute(const ConcreteReadAttributePath & aPath, diff --git a/src/app/tests/BUILD.gn b/src/app/tests/BUILD.gn index ea077eb5d67582..adfd2ea7ed85ee 100644 --- a/src/app/tests/BUILD.gn +++ b/src/app/tests/BUILD.gn @@ -109,6 +109,18 @@ source_set("power-cluster-test-srcs") { ] } +source_set("actions-cluster-test-srcs") { + sources = + [ "${chip_root}/src/app/clusters/actions-server/actions-server.cpp" ] + + public_deps = [ + "${chip_root}/src/app/common:cluster-objects", + "${chip_root}/src/app/util/mock:mock_codegen_data_model", + "${chip_root}/src/app/util/mock:mock_ember", + "${chip_root}/src/lib/core", + ] +} + source_set("scenes-table-test-srcs") { sources = [ "${chip_root}/src/app/clusters/scenes-server/ExtensionFieldSets.h", @@ -195,6 +207,7 @@ chip_test_suite("tests") { test_sources = [ "TestAclAttribute.cpp", "TestAclEvent.cpp", + "TestActionsCluster.cpp", "TestAttributeAccessInterfaceCache.cpp", "TestAttributePathExpandIterator.cpp", "TestAttributePathParams.cpp", @@ -239,6 +252,7 @@ chip_test_suite("tests") { cflags = [ "-Wconversion" ] public_deps = [ + ":actions-cluster-test-srcs", ":app-test-stubs", ":binding-test-srcs", ":ecosystem-information-test-srcs", @@ -271,6 +285,7 @@ chip_test_suite("tests") { "TestSceneTable.cpp", ] public_deps += [ + ":actions-cluster-test-srcs", ":power-cluster-test-srcs", ":scenes-table-test-srcs", ] diff --git a/src/app/tests/TestActionsCluster.cpp b/src/app/tests/TestActionsCluster.cpp new file mode 100644 index 00000000000000..702527711c2707 --- /dev/null +++ b/src/app/tests/TestActionsCluster.cpp @@ -0,0 +1,445 @@ +/* + * + * Copyright (c) 2025 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 + +#include "lib/support/CHIPMem.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#ifdef CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT +#undef CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT +#define CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT 1 +#endif + +namespace chip { +namespace app { + +using namespace Clusters::Actions; +using namespace chip::Protocols::InteractionModel; + +class TestActionsCluster : public ::testing::Test +{ +public: + static void SetUpTestSuite() { ASSERT_EQ(chip::Platform::MemoryInit(), CHIP_NO_ERROR); } + static void TearDownTestSuite() { chip::Platform::MemoryShutdown(); } +}; + +class TestActionsDelegateImpl : public Clusters::Actions::Delegate +{ +public: + EndpointId endpointId = 0; + static constexpr size_t kMaxActionNameLength = 127u; + static constexpr size_t kMaxEndpointListNameLength = 127u; + static constexpr size_t kEndpointListMaxSize = 256u; + static constexpr size_t kMaxActions = 2; + static constexpr size_t kMaxEndpointLists = 2; + + ActionStructStorage mActions[kMaxActions]; + size_t mNumActions = 0; + + EndpointListStorage mEndpointLists[kMaxEndpointLists]; + size_t mNumEndpointLists = 0; + + CHIP_ERROR ReadActionAtIndex(uint16_t index, ActionStructStorage & action) override + { + if (index >= mNumActions) + { + return CHIP_ERROR_PROVIDER_LIST_EXHAUSTED; + } + action = mActions[index]; + return CHIP_NO_ERROR; + } + + CHIP_ERROR ReadEndpointListAtIndex(uint16_t index, EndpointListStorage & epList) override + { + if (index >= mNumEndpointLists) + { + return CHIP_ERROR_PROVIDER_LIST_EXHAUSTED; + } + epList = mEndpointLists[index]; + return CHIP_NO_ERROR; + } + + bool FindActionIdInActionList(uint16_t actionId) override + { + for (uint16_t i = 0; i < mNumActions; i++) + { + if (mActions[i].actionID == actionId) + { + return true; + } + } + return false; + } + + // Test helper methods + CHIP_ERROR AddTestAction(const ActionStructStorage & action) + { + if (mNumActions >= kMaxActions) + { + return CHIP_ERROR_BUFFER_TOO_SMALL; + } + mActions[mNumActions++] = action; + return CHIP_NO_ERROR; + } + + CHIP_ERROR AddTestEndpointList(const EndpointListStorage & epList) + { + if (mNumEndpointLists >= kMaxEndpointLists) + { + return CHIP_ERROR_BUFFER_TOO_SMALL; + } + mEndpointLists[mNumEndpointLists++] = epList; + return CHIP_NO_ERROR; + } + + Protocols::InteractionModel::Status HandleInstantAction(uint16_t actionId, Optional invokeId) override + { + return Status::NotFound; + } + Protocols::InteractionModel::Status HandleInstantActionWithTransition(uint16_t actionId, uint16_t transitionTime, + Optional invokeId) override + { + return Status::NotFound; + } + Protocols::InteractionModel::Status HandleStartAction(uint16_t actionId, Optional invokeId) override + { + return Status::NotFound; + } + Protocols::InteractionModel::Status HandleStartActionWithDuration(uint16_t actionId, uint32_t duration, + Optional invokeId) override + { + return Status::NotFound; + } + Protocols::InteractionModel::Status HandleStopAction(uint16_t actionId, Optional invokeId) override + { + return Status::NotFound; + } + Protocols::InteractionModel::Status HandlePauseAction(uint16_t actionId, Optional invokeId) override + { + return Status::NotFound; + } + Protocols::InteractionModel::Status HandlePauseActionWithDuration(uint16_t actionId, uint32_t duration, + Optional invokeId) override + { + return Status::NotFound; + } + Protocols::InteractionModel::Status HandleResumeAction(uint16_t actionId, Optional invokeId) override + { + return Status::NotFound; + } + Protocols::InteractionModel::Status HandleEnableAction(uint16_t actionId, Optional invokeId) override + { + return Status::NotFound; + } + Protocols::InteractionModel::Status HandleEnableActionWithDuration(uint16_t actionId, uint32_t duration, + Optional invokeId) override + { + return Status::NotFound; + } + Protocols::InteractionModel::Status HandleDisableAction(uint16_t actionId, Optional invokeId) override + { + return Status::NotFound; + } + Protocols::InteractionModel::Status HandleDisableActionWithDuration(uint16_t actionId, uint32_t duration, + Optional invokeId) override + { + return Status::NotFound; + } +}; + +TestActionsDelegateImpl delegate; +TEST_F(TestActionsCluster, TestActionListConstraints) +{ + // Test 1: Action name length constraint + char longName[kActionNameMaxSize + 10]; + memset(longName, 'A', sizeof(longName)); + longName[sizeof(longName) - 1] = '\0'; + + ActionStructStorage actionWithLongName(1, CharSpan::fromCharString(longName), ActionTypeEnum::kScene, 0, BitMask(), + ActionStateEnum::kInactive); + + // Add action and verify it was added successfully + EXPECT_EQ(delegate.AddTestAction(actionWithLongName), CHIP_NO_ERROR); + + // Verify the name was truncated by reading it back + ActionStructStorage readAction; + EXPECT_EQ(delegate.ReadActionAtIndex(0, readAction), CHIP_NO_ERROR); + EXPECT_EQ(readAction.name.size(), kActionNameMaxSize); + + // Test 2: Maximum action list size + delegate.mNumActions = 0; + for (uint16_t i = 0; i < delegate.kMaxActions; i++) + { + ActionStructStorage action(i, CharSpan::fromCharString("Action"), ActionTypeEnum::kScene, 0, BitMask(), + ActionStateEnum::kInactive); + EXPECT_EQ(delegate.AddTestAction(action), CHIP_NO_ERROR); + } + + // Try to add one more action beyond the limit + ActionStructStorage extraAction(99, CharSpan::fromCharString("Extra"), ActionTypeEnum::kScene, 0, BitMask(), + ActionStateEnum::kInactive); + EXPECT_EQ(delegate.AddTestAction(extraAction), CHIP_ERROR_BUFFER_TOO_SMALL); +} + +TEST_F(TestActionsCluster, TestEndpointListConstraints) +{ + // Test 1: Endpoint list name length constraint + char longName[kEndpointListNameMaxSize + 10]; + memset(longName, 'B', sizeof(longName)); + longName[sizeof(longName) - 1] = '\0'; + + const EndpointId endpoints[] = { 1, 2, 3 }; + EndpointListStorage epListWithLongName(1, CharSpan::fromCharString(longName), EndpointListTypeEnum::kOther, + DataModel::List(endpoints)); + + // Add endpoint list and verify it was added successfully + EXPECT_EQ(delegate.AddTestEndpointList(epListWithLongName), CHIP_NO_ERROR); + + // Verify the name was truncated by reading it back + EndpointListStorage readEpList; + EXPECT_EQ(delegate.ReadEndpointListAtIndex(0, readEpList), CHIP_NO_ERROR); + EXPECT_EQ(readEpList.name.size(), kEndpointListNameMaxSize); + + // Test 2: Maximum endpoint list size + delegate.mNumEndpointLists = 0; + for (uint16_t i = 0; i < delegate.kMaxEndpointLists; i++) + { + EndpointListStorage epList(i, CharSpan::fromCharString("List"), EndpointListTypeEnum::kOther, + DataModel::List(endpoints)); + EXPECT_EQ(delegate.AddTestEndpointList(epList), CHIP_NO_ERROR); + } + + // Try to add one more endpoint list beyond the limit + EndpointListStorage extraEpList(99, CharSpan::fromCharString("Extra"), EndpointListTypeEnum::kOther, + DataModel::List(endpoints)); + EXPECT_EQ(delegate.AddTestEndpointList(extraEpList), CHIP_ERROR_BUFFER_TOO_SMALL); + + // Test 3: Maximum endpoints per list + delegate.mNumEndpointLists = 0; + EndpointId tooManyEndpoints[kEndpointListMaxSize + 5]; + for (uint16_t i = 0; i < kEndpointListMaxSize + 5; i++) + { + tooManyEndpoints[i] = i; + } + + EndpointListStorage epListWithTooManyEndpoints(1, CharSpan::fromCharString("List"), EndpointListTypeEnum::kOther, + DataModel::List(tooManyEndpoints)); + + // The list should be added but truncated to kEndpointListMaxSize + EXPECT_EQ(delegate.AddTestEndpointList(epListWithTooManyEndpoints), CHIP_NO_ERROR); + + // Verify the endpoint list was truncated + EXPECT_EQ(delegate.ReadEndpointListAtIndex(0, readEpList), CHIP_NO_ERROR); + EXPECT_EQ(readEpList.endpoints.size(), kEndpointListMaxSize); +} + +TEST_F(TestActionsCluster, TestActionListAttributeAccess) +{ + ActionsServer::Instance().SetDefaultDelegate(delegate.endpointId, &delegate); + delegate.mNumActions = 0; + + // Add test actions + ActionStructStorage action1(1, CharSpan::fromCharString("FirstAction"), ActionTypeEnum::kScene, 0, BitMask(), + ActionStateEnum::kInactive); + ActionStructStorage action2(2, CharSpan::fromCharString("SecondAction"), ActionTypeEnum::kScene, 1, BitMask(), + ActionStateEnum::kActive); + + EXPECT_EQ(delegate.AddTestAction(action1), CHIP_NO_ERROR); + EXPECT_EQ(delegate.AddTestAction(action2), CHIP_NO_ERROR); + + // Test reading actions through attribute reader + uint8_t buf[1024]; + + // Create the TLV writer + TLV::TLVWriter tlvWriter; + tlvWriter.Init(buf); + + AttributeReportIBs::Builder builder; + builder.Init(&tlvWriter); + + ConcreteAttributePath path(delegate.endpointId, Clusters::Actions::Id, Clusters::Actions::Attributes::ActionList::Id); + ConcreteReadAttributePath readPath(path); + chip::DataVersion dataVersion(0); + Access::SubjectDescriptor subjectDescriptor; + AttributeValueEncoder encoder(builder, subjectDescriptor, path, dataVersion); + + // Read the action list using the Actions cluster's Read function + EXPECT_EQ(ActionsServer::Instance().Read(readPath, encoder), CHIP_NO_ERROR); + + TLV::TLVReader reader; + reader.Init(buf); + + TLV::TLVReader attrReportsReader; + TLV::TLVReader attrReportReader; + TLV::TLVReader attrDataReader; + + reader.Next(); + reader.OpenContainer(attrReportsReader); + + attrReportsReader.Next(); + attrReportsReader.OpenContainer(attrReportReader); + + attrReportReader.Next(); + attrReportReader.OpenContainer(attrDataReader); + + // We're now in the attribute data IB, skip to the desired tag, we want TagNum = 2 + attrDataReader.Next(); + for (int i = 0; i < 3 && !(IsContextTag(attrDataReader.GetTag()) && TagNumFromTag(attrDataReader.GetTag()) == 2); ++i) + { + attrDataReader.Next(); + } + EXPECT_TRUE(IsContextTag(attrDataReader.GetTag())); + EXPECT_EQ(TagNumFromTag(attrDataReader.GetTag()), 2u); + + Clusters::Actions::Attributes::ActionList::TypeInfo::DecodableType list; + CHIP_ERROR err = list.Decode(attrDataReader); + EXPECT_EQ(err, CHIP_NO_ERROR); + + auto iter = list.begin(); + EXPECT_TRUE(iter.Next()); + Clusters::Actions::Structs::ActionStruct::Type action = iter.GetValue(); + EXPECT_EQ(action.actionID, 1); + EXPECT_TRUE(strncmp(action.name.data(), "FirstAction", action.name.size()) == 0); + EXPECT_EQ(action.type, ActionTypeEnum::kScene); + EXPECT_EQ(action.endpointListID, 0); + EXPECT_EQ(action.supportedCommands.Raw(), 0); + EXPECT_EQ(action.state, ActionStateEnum::kInactive); + + EXPECT_TRUE(iter.Next()); + action = iter.GetValue(); + EXPECT_EQ(action.actionID, 2); + EXPECT_TRUE(strncmp(action.name.data(), "SecondAction", action.name.size()) == 0); + EXPECT_EQ(action.type, ActionTypeEnum::kScene); + EXPECT_EQ(action.endpointListID, 1); + EXPECT_EQ(action.supportedCommands.Raw(), 0); + EXPECT_EQ(action.state, ActionStateEnum::kActive); + + // Cleanup + ActionsServer::Instance().SetDefaultDelegate(delegate.endpointId, nullptr); +} + +TEST_F(TestActionsCluster, TestEndpointListAttributeAccess) +{ + ActionsServer::Instance().SetDefaultDelegate(delegate.endpointId, &delegate); + delegate.mNumEndpointLists = 0; + + // Add test endpoint lists + const EndpointId endpoints1[] = { 1, 2 }; + const EndpointId endpoints2[] = { 3, 4, 5 }; + + EndpointListStorage epList1(1, CharSpan::fromCharString("FirstList"), EndpointListTypeEnum::kOther, + DataModel::List(endpoints1, 2)); + EndpointListStorage epList2(2, CharSpan::fromCharString("SecondList"), EndpointListTypeEnum::kOther, + DataModel::List(endpoints2, 3)); + + EXPECT_EQ(delegate.AddTestEndpointList(epList1), CHIP_NO_ERROR); + EXPECT_EQ(delegate.AddTestEndpointList(epList2), CHIP_NO_ERROR); + + // Test reading endpoint lists through attribute reader + TLV::TLVWriter writer; + uint8_t buf[1024]; + writer.Init(buf); + + // Create the builders + TLV::TLVWriter tlvWriter; + tlvWriter.Init(buf); + + AttributeReportIBs::Builder builder; + builder.Init(&tlvWriter); + + ConcreteAttributePath path(delegate.endpointId, Clusters::Actions::Id, Clusters::Actions::Attributes::EndpointLists::Id); + ConcreteReadAttributePath readPath(path); + chip::DataVersion dataVersion(0); + Access::SubjectDescriptor subjectDescriptor; + AttributeValueEncoder encoder(builder, subjectDescriptor, path, dataVersion); + + EXPECT_EQ(ActionsServer::Instance().Read(path, encoder), CHIP_NO_ERROR); + + TLV::TLVReader reader; + reader.Init(buf); + + TLV::TLVReader attrReportsReader; + TLV::TLVReader attrReportReader; + TLV::TLVReader attrDataReader; + + reader.Next(); + reader.OpenContainer(attrReportsReader); + + attrReportsReader.Next(); + attrReportsReader.OpenContainer(attrReportReader); + + attrReportReader.Next(); + attrReportReader.OpenContainer(attrDataReader); + + // We're now in the attribute data IB, skip to the desired tag, we want TagNum = 2 + attrDataReader.Next(); + for (int i = 0; i < 3 && !(IsContextTag(attrDataReader.GetTag()) && TagNumFromTag(attrDataReader.GetTag()) == 2); ++i) + { + attrDataReader.Next(); + } + EXPECT_TRUE(IsContextTag(attrDataReader.GetTag())); + EXPECT_EQ(TagNumFromTag(attrDataReader.GetTag()), 2u); + + Clusters::Actions::Attributes::EndpointLists::TypeInfo::DecodableType list; + CHIP_ERROR err = list.Decode(attrDataReader); + EXPECT_EQ(err, CHIP_NO_ERROR); + + auto iter = list.begin(); + EXPECT_TRUE(iter.Next()); + Clusters::Actions::Structs::EndpointListStruct::DecodableType endpointList = iter.GetValue(); + EXPECT_EQ(endpointList.endpointListID, 1); + EXPECT_TRUE(strncmp(endpointList.name.data(), "FirstList", endpointList.name.size()) == 0); + EXPECT_EQ(endpointList.type, EndpointListTypeEnum::kOther); + auto it = endpointList.endpoints.begin(); + uint8_t i = 0; + while (it.Next()) + { + EXPECT_EQ(endpoints1[i++], it.GetValue()); + } + + EXPECT_TRUE(iter.Next()); + endpointList = iter.GetValue(); + EXPECT_EQ(endpointList.endpointListID, 2); + EXPECT_TRUE(strncmp(endpointList.name.data(), "SecondList", endpointList.name.size()) == 0); + EXPECT_EQ(endpointList.type, EndpointListTypeEnum::kOther); + it = endpointList.endpoints.begin(); + i = 0; + while (it.Next()) + { + EXPECT_EQ(endpoints2[i++], it.GetValue()); + } + + // Cleanup + ActionsServer::Instance().SetDefaultDelegate(delegate.endpointId, nullptr); +} + +} // namespace app +} // namespace chip From 283cfeea7cbca2a3a5e258d8ad98e2c363e8d55a Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Wed, 29 Jan 2025 15:56:41 +0530 Subject: [PATCH 16/25] Add API to mark dirty attribute and changed delegate impl. --- .../include/bridged-actions-stub.h | 25 +-- .../src/bridged-actions-stub.cpp | 6 +- .../actions-server/actions-server.cpp | 107 ++++++++++--- .../clusters/actions-server/actions-server.h | 144 ++++++++++++------ src/app/tests/BUILD.gn | 1 - src/app/tests/TestActionsCluster.cpp | 29 ++-- 6 files changed, 211 insertions(+), 101 deletions(-) diff --git a/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h b/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h index d58269313b648e..c7355a34a0943c 100644 --- a/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h +++ b/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h @@ -24,40 +24,41 @@ #include #include #include +#include namespace chip { namespace app { namespace Clusters { namespace Actions { -const uint16_t kActionListSize = 3; -const uint16_t kEndpointListSize = 3; class ActionsDelegateImpl : public Delegate { private: - const ActionStructStorage kActionList[kActionListSize] = { + std::vector kActionList = { ActionStructStorage(0, CharSpan::fromCharString("TurnOnLight"), ActionTypeEnum::kScene, 0, 0, ActionStateEnum::kInactive), ActionStructStorage(1, CharSpan::fromCharString("TurnOffLight"), ActionTypeEnum::kScene, 1, 0, ActionStateEnum::kInactive), - ActionStructStorage(2, CharSpan::fromCharString("ToggleLight"), ActionTypeEnum::kScene, 2, 0, ActionStateEnum::kInactive), + ActionStructStorage(2, CharSpan::fromCharString("ToggleLight"), ActionTypeEnum::kScene, 2, 0, ActionStateEnum::kInactive) }; - // Dummy endpoint list. - const EndpointId firstEpList[3] = { 0, 1, 2 }; - const EndpointId secondEpList[1] = { 0 }; - const EndpointId thirdEpList[1] = { 0 }; + std::vector firstEpList = { 0 }; + std::vector secondEpList = { 0, 1 }; + std::vector thirdEpList = { 1, 2, 3 }; - const EndpointListStorage kEndpointList[kEndpointListSize] = { + std::vector kEndpointList = { EndpointListStorage(0, CharSpan::fromCharString("On"), EndpointListTypeEnum::kOther, - DataModel::List(firstEpList)), + DataModel::List(firstEpList.data(), firstEpList.size())), EndpointListStorage(1, CharSpan::fromCharString("Off"), EndpointListTypeEnum::kOther, - DataModel::List(secondEpList)), + DataModel::List(secondEpList.data(), secondEpList.size())), EndpointListStorage(2, CharSpan::fromCharString("Toggle"), EndpointListTypeEnum::kOther, - DataModel::List(thirdEpList)), + DataModel::List(thirdEpList.data(), thirdEpList.size())) }; CHIP_ERROR ReadActionAtIndex(uint16_t index, ActionStructStorage & action) override; CHIP_ERROR ReadEndpointListAtIndex(uint16_t index, EndpointListStorage & epList) override; bool FindActionIdInActionList(uint16_t actionId) override; + void OnActionListChanged(EndpointId endpoint, const ActionStructStorage & action) override{}; + void OnEndpointListChanged(EndpointId endpoint, const EndpointListStorage & action) override{}; + Protocols::InteractionModel::Status HandleInstantAction(uint16_t actionId, Optional invokeId) override; Protocols::InteractionModel::Status HandleInstantActionWithTransition(uint16_t actionId, uint16_t transitionTime, Optional invokeId) override; diff --git a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp index f59efbe24b2a59..d6414d2c7403db 100644 --- a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp +++ b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp @@ -26,7 +26,7 @@ using namespace chip::Protocols::InteractionModel; CHIP_ERROR ActionsDelegateImpl::ReadActionAtIndex(uint16_t index, ActionStructStorage & action) { - if (index >= ArraySize(kActionList)) + if (index >= kActionList.size()) { return CHIP_ERROR_PROVIDER_LIST_EXHAUSTED; } @@ -36,7 +36,7 @@ CHIP_ERROR ActionsDelegateImpl::ReadActionAtIndex(uint16_t index, ActionStructSt CHIP_ERROR ActionsDelegateImpl::ReadEndpointListAtIndex(uint16_t index, EndpointListStorage & epList) { - if (index >= ArraySize(kEndpointList)) + if (index >= kEndpointList.size()) { return CHIP_ERROR_PROVIDER_LIST_EXHAUSTED; } @@ -46,7 +46,7 @@ CHIP_ERROR ActionsDelegateImpl::ReadEndpointListAtIndex(uint16_t index, Endpoint bool ActionsDelegateImpl::FindActionIdInActionList(uint16_t actionId) { - for (uint16_t i = 0; i < kActionListSize; i++) + for (size_t i = 0; i < kEndpointList.size(); i++) { if (kActionList[i].actionID == actionId) return true; diff --git a/src/app/clusters/actions-server/actions-server.cpp b/src/app/clusters/actions-server/actions-server.cpp index a8fcd476eb2345..391d9f82ac0d11 100644 --- a/src/app/clusters/actions-server/actions-server.cpp +++ b/src/app/clusters/actions-server/actions-server.cpp @@ -32,23 +32,29 @@ using namespace chip::app::Clusters::Actions; using namespace chip::app::Clusters::Actions::Attributes; using namespace chip::Protocols::InteractionModel; +#ifndef MATTER_DM_ACTIONS_CLUSTER_SERVER_ENDPOINT_COUNT +#define MATTER_DM_ACTIONS_CLUSTER_SERVER_ENDPOINT_COUNT 1 +#endif + namespace { +static constexpr size_t kActionsDelegateTableSize = + MATTER_DM_ACTIONS_CLUSTER_SERVER_ENDPOINT_COUNT + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT; Delegate * gDelegateTable[kActionsDelegateTableSize] = { nullptr }; -Delegate * GetDelegate(EndpointId endpoint) +Delegate * GetDelegate(EndpointId aEndpoint) { - return (endpoint >= kActionsDelegateTableSize ? nullptr : gDelegateTable[endpoint]); + return (aEndpoint >= kActionsDelegateTableSize ? nullptr : gDelegateTable[aEndpoint]); } } // namespace ActionsServer ActionsServer::sInstance; -void ActionsServer::SetDefaultDelegate(EndpointId endpoint, Delegate * delegate) +void ActionsServer::SetDefaultDelegate(EndpointId aEndpoint, Delegate * aDelegate) { - if (endpoint < kActionsDelegateTableSize) + if (aEndpoint < kActionsDelegateTableSize) { - gDelegateTable[endpoint] = delegate; + gDelegateTable[aEndpoint] = aDelegate; } } @@ -57,30 +63,30 @@ ActionsServer & ActionsServer::Instance() return sInstance; } -void ActionsServer::OnStateChanged(EndpointId endpoint, uint16_t actionId, uint32_t invokeId, ActionStateEnum actionState) +void ActionsServer::OnStateChanged(EndpointId aEndpoint, uint16_t aActionId, uint32_t aInvokeId, ActionStateEnum aActionState) { ChipLogProgress(Zcl, "ActionsServer: OnStateChanged"); // Generate StateChanged event EventNumber eventNumber; - Events::StateChanged::Type event{ actionId, invokeId, actionState }; + Events::StateChanged::Type event{ aActionId, aInvokeId, aActionState }; - if (CHIP_NO_ERROR != LogEvent(event, endpoint, eventNumber)) + if (CHIP_NO_ERROR != LogEvent(event, aEndpoint, eventNumber)) { ChipLogError(Zcl, "ActionsServer: Failed to generate OnStateChanged event"); } } -void ActionsServer::OnActionFailed(EndpointId endpoint, uint16_t actionId, uint32_t invokeId, ActionStateEnum actionState, - ActionErrorEnum actionError) +void ActionsServer::OnActionFailed(EndpointId aEndpoint, uint16_t aActionId, uint32_t aInvokeId, ActionStateEnum aActionState, + ActionErrorEnum aActionError) { ChipLogProgress(Zcl, "ActionsServer: OnActionFailed"); // Generate ActionFailed event EventNumber eventNumber; - Events::ActionFailed::Type event{ actionId, invokeId, actionState, actionError }; + Events::ActionFailed::Type event{ aActionId, aInvokeId, aActionState, aActionError }; - if (CHIP_NO_ERROR != LogEvent(event, endpoint, eventNumber)) + if (CHIP_NO_ERROR != LogEvent(event, aEndpoint, eventNumber)) { ChipLogError(Zcl, "ActionsServer: Failed to generate OnActionFailed event"); } @@ -93,8 +99,6 @@ CHIP_ERROR ActionsServer::Read(const ConcreteReadAttributePath & aPath, Attribut switch (aPath.mAttributeId) { case ActionList::Id: { - // The encoder will automatically create the outer AttributeDataIB container - // We just need to encode the array of actions ReturnErrorOnFailure(aEncoder.EncodeList( [this, aPath](const auto & encoder) -> CHIP_ERROR { return this->ReadActionListAttribute(aPath, encoder); })); return CHIP_NO_ERROR; @@ -111,7 +115,7 @@ CHIP_ERROR ActionsServer::Read(const ConcreteReadAttributePath & aPath, Attribut } CHIP_ERROR ActionsServer::ReadActionListAttribute(const ConcreteReadAttributePath & aPath, - const AttributeValueEncoder::ListEncodeHelper & encoder) + const AttributeValueEncoder::ListEncodeHelper & aEncoder) { Delegate * delegate = GetDelegate(aPath.mEndpointId); if (delegate == nullptr) @@ -129,18 +133,14 @@ CHIP_ERROR ActionsServer::ReadActionListAttribute(const ConcreteReadAttributePat { return CHIP_NO_ERROR; } - else if (err != CHIP_NO_ERROR) - { - return err; - } - ReturnErrorOnFailure(encoder.Encode(action)); + ReturnErrorOnFailure(aEncoder.Encode(action)); } return CHIP_NO_ERROR; } CHIP_ERROR ActionsServer::ReadEndpointListAttribute(const ConcreteReadAttributePath & aPath, - const AttributeValueEncoder::ListEncodeHelper & encoder) + const AttributeValueEncoder::ListEncodeHelper & aEncoder) { Delegate * delegate = GetDelegate(aPath.mEndpointId); if (delegate == nullptr) @@ -158,20 +158,20 @@ CHIP_ERROR ActionsServer::ReadEndpointListAttribute(const ConcreteReadAttributeP return CHIP_NO_ERROR; } - ReturnErrorOnFailure(encoder.Encode(epList)); + ReturnErrorOnFailure(aEncoder.Encode(epList)); } return CHIP_NO_ERROR; } -bool ActionsServer::FindActionIdInActionList(EndpointId endpointId, uint16_t actionId) +bool ActionsServer::FindActionIdInActionList(EndpointId aEndpointId, uint16_t aActionId) { - Delegate * delegate = GetDelegate(endpointId); + Delegate * delegate = GetDelegate(aEndpointId); if (delegate == nullptr) { ChipLogError(Zcl, "Actions delegate is null!!!"); return false; } - return delegate->FindActionIdInActionList(actionId); + return delegate->FindActionIdInActionList(aActionId); } template @@ -354,6 +354,63 @@ void ActionsServer::InvokeCommand(HandlerContext & handlerContext) return; } } + +CHIP_ERROR ActionsServer::SetActionList(EndpointId aEndpoint, const ActionStructStorage & aAction) +{ + Delegate * delegate = GetDelegate(aEndpoint); + if (delegate == nullptr) + { + ChipLogError(Zcl, "Actions delegate is null!"); + return CHIP_ERROR_INCORRECT_STATE; + } + + // Read through the list to find and update the existing action + for (uint16_t i = 0; i < kMaxActionListLength; i++) + { + ActionStructStorage existingAction; + ReturnErrorOnFailure(delegate->ReadActionAtIndex(i, existingAction)); + + if (existingAction.actionID == aAction.actionID) + { + existingAction.Set(aAction.actionID, aAction.name, aAction.type, aAction.endpointListID, aAction.supportedCommands, + aAction.state); + + delegate->OnActionListChanged(aEndpoint, aAction); + mMatterContext.MarkDirty(aEndpoint, Attributes::ActionList::Id); + return CHIP_NO_ERROR; + } + } + + return CHIP_ERROR_NOT_FOUND; +} + +CHIP_ERROR ActionsServer::SetEndpointList(EndpointId aEndpoint, const EndpointListStorage & aEpList) +{ + Delegate * delegate = GetDelegate(aEndpoint); + if (delegate == nullptr) + { + ChipLogError(Zcl, "Actions delegate is null!"); + return CHIP_ERROR_INCORRECT_STATE; + } + + // Read through the list to find and update the existing endpoint list + for (uint16_t i = 0; i < kMaxEndpointListLength; i++) + { + EndpointListStorage existingEpList; + ReturnErrorOnFailure(delegate->ReadEndpointListAtIndex(i, existingEpList)); + + if (existingEpList.endpointListID == aEpList.endpointListID) + { + existingEpList.Set(aEpList.endpointListID, aEpList.name, aEpList.type, aEpList.endpoints); + mMatterContext.MarkDirty(aEndpoint, Attributes::EndpointLists::Id); + delegate->OnEndpointListChanged(aEndpoint, aEpList); + return CHIP_NO_ERROR; + } + } + + return CHIP_ERROR_NOT_FOUND; +} + void MatterActionsPluginServerInitCallback() { AttributeAccessInterfaceRegistry::Instance().Register(&ActionsServer::Instance()); diff --git a/src/app/clusters/actions-server/actions-server.h b/src/app/clusters/actions-server/actions-server.h index 162cb40fea215a..bd33db9e32d8d2 100644 --- a/src/app/clusters/actions-server/actions-server.h +++ b/src/app/clusters/actions-server/actions-server.h @@ -20,59 +20,61 @@ #include #include #include +#include #include -namespace { -// Zero'th index will be used for unit tests only. -static constexpr size_t kActionsDelegateTableSize = -#ifdef ZCL_USING_ACTIONS_CLUSTER_SERVER - MATTER_DM_ACTIONS_CLUSTER_SERVER_ENDPOINT_COUNT + -#endif - CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT + 1; - -} // namespace - namespace chip { namespace app { namespace Clusters { namespace Actions { -// Last byte is reserved of '\0' terminator. -static constexpr size_t kActionNameMaxSize = 127u; -static constexpr size_t kEndpointListNameMaxSize = 127u; +static constexpr size_t kActionNameMaxSize = 128u; +static constexpr size_t kEndpointListNameMaxSize = 128u; static constexpr size_t kEndpointListMaxSize = 256u; class Delegate; +class MatterContext +{ +public: + virtual ~MatterContext() = default; + // MarkDirty + virtual void MarkDirty(EndpointId aEndpointId, AttributeId aAttributeId) + { + MatterReportingAttributeChangeCallback(aEndpointId, Id, aAttributeId); + } +}; + struct ActionStructStorage : public Structs::ActionStruct::Type { ActionStructStorage() : mActionName(mBuffer, sizeof(mBuffer)){}; - ActionStructStorage(uint16_t action, const CharSpan & actionName, ActionTypeEnum actionType, uint16_t epListID, - BitMask commands, ActionStateEnum actionState) : + ActionStructStorage(uint16_t aAction, const CharSpan & aActionName, ActionTypeEnum aActionType, uint16_t aEpListID, + BitMask aCommands, ActionStateEnum aActionState) : mActionName(mBuffer, sizeof(mBuffer)) { - Set(action, actionName, actionType, epListID, commands, actionState); + Set(aAction, aActionName, aActionType, aEpListID, aCommands, aActionState); } - ActionStructStorage(const ActionStructStorage & action) : mActionName(mBuffer, sizeof(mBuffer)) { *this = action; } + ActionStructStorage(const ActionStructStorage & aAction) : mActionName(mBuffer, sizeof(mBuffer)) { *this = aAction; } - ActionStructStorage & operator=(const ActionStructStorage & action) + ActionStructStorage & operator=(const ActionStructStorage & aAction) { - Set(action.actionID, action.name, action.type, action.endpointListID, action.supportedCommands, action.state); + Set(aAction.actionID, aAction.name, aAction.type, aAction.endpointListID, aAction.supportedCommands, aAction.state); return *this; } - void Set(uint16_t action, const CharSpan & actionName, ActionTypeEnum actionType, uint16_t epListID, - BitMask commands, ActionStateEnum actionState) + void Set(uint16_t aAction, const CharSpan & aActionName, ActionTypeEnum aActionType, uint16_t aEpListID, + BitMask aCommands, ActionStateEnum aActionState) { - actionID = action; - type = actionType; - endpointListID = epListID; - supportedCommands = commands; - state = actionState; - CopyCharSpanToMutableCharSpanWithTruncation(actionName, mActionName); + actionID = aAction; + type = aActionType; + endpointListID = aEpListID; + supportedCommands = aCommands; + state = aActionState; + mActionName = Span(mBuffer, sizeof(mBuffer)); + CopyCharSpanToMutableCharSpanWithTruncation(aActionName, mActionName); name = mActionName; } @@ -85,34 +87,35 @@ struct EndpointListStorage : public Structs::EndpointListStruct::Type { EndpointListStorage() : mEpListName(mBuffer, sizeof(mBuffer)){}; - EndpointListStorage(uint16_t epListId, const CharSpan & epListName, EndpointListTypeEnum epListType, - const DataModel::List & endpointList) : + EndpointListStorage(uint16_t aEpListId, const CharSpan & aEpListName, EndpointListTypeEnum aEpListType, + const DataModel::List & aEndpointList) : mEpListName(mBuffer, sizeof(mBuffer)) { - Set(epListId, epListName, epListType, endpointList); + Set(aEpListId, aEpListName, aEpListType, aEndpointList); } - EndpointListStorage(const EndpointListStorage & epList) : mEpListName(mBuffer, sizeof(mBuffer)) { *this = epList; } + EndpointListStorage(const EndpointListStorage & aEpList) : mEpListName(mBuffer, sizeof(mBuffer)) { *this = aEpList; } - EndpointListStorage & operator=(const EndpointListStorage & epList) + EndpointListStorage & operator=(const EndpointListStorage & aEpList) { - Set(epList.endpointListID, epList.name, epList.type, epList.endpoints); + Set(aEpList.endpointListID, aEpList.name, aEpList.type, aEpList.endpoints); return *this; } - void Set(uint16_t epListId, const CharSpan & epListName, EndpointListTypeEnum epListType, - const DataModel::List & endpointList) + void Set(uint16_t aEpListId, const CharSpan & aEpListName, EndpointListTypeEnum aEpListType, + const DataModel::List & aEndpointList) { - endpointListID = epListId; - type = epListType; - size_t epListSize = std::min(endpointList.size(), kEndpointListMaxSize); + endpointListID = aEpListId; + type = aEpListType; + size_t epListSize = std::min(aEndpointList.size(), kEndpointListMaxSize); for (size_t index = 0; index < epListSize; index++) { - mEpList[index] = endpointList[index]; + mEpList[index] = aEndpointList[index]; } - endpoints = DataModel::List(Span(mEpList, epListSize)); - CopyCharSpanToMutableCharSpanWithTruncation(epListName, mEpListName); + endpoints = DataModel::List(Span(mEpList, epListSize)); + mEpListName = Span(mBuffer, sizeof(mBuffer)); + CopyCharSpanToMutableCharSpanWithTruncation(aEpListName, mEpListName); name = mEpListName; } @@ -122,13 +125,13 @@ struct EndpointListStorage : public Structs::EndpointListStruct::Type EndpointId mEpList[kEndpointListMaxSize]; }; -class ActionsServer : public AttributeAccessInterface, public CommandHandlerInterface +class ActionsServer : public AttributeAccessInterface, public CommandHandlerInterface, public MatterContext { public: // Register for the Actions cluster on all endpoints. ActionsServer() : AttributeAccessInterface(Optional::Missing(), Actions::Id), - CommandHandlerInterface(Optional::Missing(), Actions::Id) + CommandHandlerInterface(Optional::Missing(), Actions::Id), mMatterContext(*this) {} static ActionsServer & Instance(); @@ -136,29 +139,54 @@ class ActionsServer : public AttributeAccessInterface, public CommandHandlerInte * @brief * Called when the state of action is changed. */ - void OnStateChanged(EndpointId endpoint, uint16_t actionId, uint32_t invokeId, ActionStateEnum actionState); + void OnStateChanged(EndpointId aEndpoint, uint16_t aActionId, uint32_t aInvokeId, ActionStateEnum aActionState); /** * @brief * Called when the action is failed.. */ - void OnActionFailed(EndpointId endpoint, uint16_t actionId, uint32_t invokeId, ActionStateEnum actionState, - ActionErrorEnum actionError); + void OnActionFailed(EndpointId aEndpoint, uint16_t aActionId, uint32_t aInvokeId, ActionStateEnum aActionState, + ActionErrorEnum aActionError); - static void SetDefaultDelegate(EndpointId endpointId, Delegate * aDelegate); + static void SetDefaultDelegate(EndpointId aEndpointId, Delegate * aDelegate); CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; + /** + * Update an existing action in the action list for the given endpoint. + * If the action with the given actionID doesn't exist, returns CHIP_ERROR_NOT_FOUND. + * + * @param aEndpoint The endpoint ID where the action should be updated + * @param aAction The action structure containing the updated action details + * @return CHIP_ERROR_INCORRECT_STATE if delegate is null + * CHIP_ERROR_NOT_FOUND if action doesn't exist + * CHIP_NO_ERROR if successful + */ + CHIP_ERROR SetActionList(EndpointId aEndpoint, const ActionStructStorage & aAction); + + /** + * Update an existing endpoint list for the given endpoint. + * If the endpoint list with the given ID doesn't exist, returns CHIP_ERROR_NOT_FOUND. + * + * @param aEndpoint The endpoint ID where the endpoint list should be updated + * @param aEpList The endpoint list structure containing the updated list details + * @return CHIP_ERROR_INCORRECT_STATE if delegate is null + * CHIP_ERROR_NOT_FOUND if endpoint list doesn't exist + * CHIP_NO_ERROR if successful + */ + CHIP_ERROR SetEndpointList(EndpointId aEndpoint, const EndpointListStorage & aEpList); + private: static ActionsServer sInstance; + MatterContext & mMatterContext; static constexpr size_t kMaxEndpointListLength = 256u; static constexpr size_t kMaxActionListLength = 256u; CHIP_ERROR ReadActionListAttribute(const ConcreteReadAttributePath & aPath, - const AttributeValueEncoder::ListEncodeHelper & encoder); + const AttributeValueEncoder::ListEncodeHelper & aEncoder); CHIP_ERROR ReadEndpointListAttribute(const ConcreteReadAttributePath & aPath, - const AttributeValueEncoder::ListEncodeHelper & encoder); - bool FindActionIdInActionList(EndpointId endpointId, uint16_t actionId); + const AttributeValueEncoder::ListEncodeHelper & aEncoder); + bool FindActionIdInActionList(EndpointId aEndpointId, uint16_t aActionId); // Cannot use CommandHandlerInterface::HandleCommand directly because we need to do the FindActionIdInActionList() check before // sending a command. @@ -345,6 +373,22 @@ class Delegate */ virtual Protocols::InteractionModel::Status HandleDisableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) = 0; + + /** + * Called when an action list has been updated. + * + * @param endpoint The endpoint ID where the action list was updated + * @param action The updated action details + */ + virtual void OnActionListChanged(EndpointId endpoint, const ActionStructStorage & action) = 0; + + /** + * Called when an endpoint list has been updated. + * + * @param endpoint The endpoint ID where the endpoint list was updated + * @param epList The updated endpoint list details + */ + virtual void OnEndpointListChanged(EndpointId endpoint, const EndpointListStorage & epList) = 0; }; } // namespace Actions diff --git a/src/app/tests/BUILD.gn b/src/app/tests/BUILD.gn index adfd2ea7ed85ee..20c3702cb29baa 100644 --- a/src/app/tests/BUILD.gn +++ b/src/app/tests/BUILD.gn @@ -252,7 +252,6 @@ chip_test_suite("tests") { cflags = [ "-Wconversion" ] public_deps = [ - ":actions-cluster-test-srcs", ":app-test-stubs", ":binding-test-srcs", ":ecosystem-information-test-srcs", diff --git a/src/app/tests/TestActionsCluster.cpp b/src/app/tests/TestActionsCluster.cpp index 702527711c2707..027c0c60b17f40 100644 --- a/src/app/tests/TestActionsCluster.cpp +++ b/src/app/tests/TestActionsCluster.cpp @@ -54,18 +54,18 @@ class TestActionsCluster : public ::testing::Test class TestActionsDelegateImpl : public Clusters::Actions::Delegate { public: - EndpointId endpointId = 0; - static constexpr size_t kMaxActionNameLength = 127u; - static constexpr size_t kMaxEndpointListNameLength = 127u; - static constexpr size_t kEndpointListMaxSize = 256u; - static constexpr size_t kMaxActions = 2; - static constexpr size_t kMaxEndpointLists = 2; + EndpointId endpointId = 0; + static constexpr uint8_t kMaxActionNameLength = 128u; + static constexpr uint8_t kMaxEndpointListNameLength = 128u; + static constexpr uint16_t kEndpointListMaxSize = 256u; + static constexpr uint8_t kMaxActions = 2; + static constexpr uint8_t kMaxEndpointLists = 2; ActionStructStorage mActions[kMaxActions]; - size_t mNumActions = 0; + uint16_t mNumActions = 0; EndpointListStorage mEndpointLists[kMaxEndpointLists]; - size_t mNumEndpointLists = 0; + uint16_t mNumEndpointLists = 0; CHIP_ERROR ReadActionAtIndex(uint16_t index, ActionStructStorage & action) override { @@ -120,6 +120,9 @@ class TestActionsDelegateImpl : public Clusters::Actions::Delegate return CHIP_NO_ERROR; } + void OnActionListChanged(EndpointId endpoint, const ActionStructStorage & action) override{}; + void OnEndpointListChanged(EndpointId endpoint, const EndpointListStorage & action) override{}; + Protocols::InteractionModel::Status HandleInstantAction(uint16_t actionId, Optional invokeId) override { return Status::NotFound; @@ -422,7 +425,10 @@ TEST_F(TestActionsCluster, TestEndpointListAttributeAccess) uint8_t i = 0; while (it.Next()) { - EXPECT_EQ(endpoints1[i++], it.GetValue()); + if (i < 2) + { + EXPECT_EQ(endpoints1[i++], it.GetValue()); + } } EXPECT_TRUE(iter.Next()); @@ -434,7 +440,10 @@ TEST_F(TestActionsCluster, TestEndpointListAttributeAccess) i = 0; while (it.Next()) { - EXPECT_EQ(endpoints2[i++], it.GetValue()); + if (i < 3) + { + EXPECT_EQ(endpoints2[i++], it.GetValue()); + } } // Cleanup From a15ee11403a1ef265bebb16ed492a68b802ce948 Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Fri, 31 Jan 2025 14:03:13 +0530 Subject: [PATCH 17/25] Addressed review changes. --- .../include/bridged-actions-stub.h | 2 +- .../src/bridged-actions-stub.cpp | 2 +- .../all-clusters-app/linux/main-common.cpp | 2 +- .../actions-server/actions-server.cpp | 80 +++++++++++++++---- .../clusters/actions-server/actions-server.h | 80 +++++++++---------- src/app/tests/TestActionsCluster.cpp | 2 +- 6 files changed, 106 insertions(+), 62 deletions(-) diff --git a/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h b/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h index c7355a34a0943c..a141e3568b517a 100644 --- a/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h +++ b/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h @@ -54,7 +54,7 @@ class ActionsDelegateImpl : public Delegate CHIP_ERROR ReadActionAtIndex(uint16_t index, ActionStructStorage & action) override; CHIP_ERROR ReadEndpointListAtIndex(uint16_t index, EndpointListStorage & epList) override; - bool FindActionIdInActionList(uint16_t actionId) override; + bool HaveActionWithId(uint16_t actionId) override; void OnActionListChanged(EndpointId endpoint, const ActionStructStorage & action) override{}; void OnEndpointListChanged(EndpointId endpoint, const EndpointListStorage & action) override{}; diff --git a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp index d6414d2c7403db..321e32caf8d709 100644 --- a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp +++ b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp @@ -44,7 +44,7 @@ CHIP_ERROR ActionsDelegateImpl::ReadEndpointListAtIndex(uint16_t index, Endpoint return CHIP_NO_ERROR; } -bool ActionsDelegateImpl::FindActionIdInActionList(uint16_t actionId) +bool ActionsDelegateImpl::HaveActionWithId(uint16_t actionId) { for (size_t i = 0; i < kEndpointList.size(); i++) { diff --git a/examples/all-clusters-app/linux/main-common.cpp b/examples/all-clusters-app/linux/main-common.cpp index 1644639e5d1dd3..0981e0dd455d29 100644 --- a/examples/all-clusters-app/linux/main-common.cpp +++ b/examples/all-clusters-app/linux/main-common.cpp @@ -344,7 +344,7 @@ void emberAfThermostatClusterInitCallback(EndpointId endpoint) void emberAfActionsClusterInitCallback(EndpointId endpoint) { - Clusters::Actions::ActionsServer::SetDefaultDelegate(endpoint, &sActionsDelegateImpl); + Clusters::Actions::ActionsServer::Instance().SetDefaultDelegate(endpoint, &sActionsDelegateImpl); } Status emberAfExternalAttributeReadCallback(EndpointId endpoint, ClusterId clusterId, diff --git a/src/app/clusters/actions-server/actions-server.cpp b/src/app/clusters/actions-server/actions-server.cpp index 391d9f82ac0d11..6efa9d320dbd75 100644 --- a/src/app/clusters/actions-server/actions-server.cpp +++ b/src/app/clusters/actions-server/actions-server.cpp @@ -163,7 +163,7 @@ CHIP_ERROR ActionsServer::ReadEndpointListAttribute(const ConcreteReadAttributeP return CHIP_NO_ERROR; } -bool ActionsServer::FindActionIdInActionList(EndpointId aEndpointId, uint16_t aActionId) +bool ActionsServer::HaveActionWithId(EndpointId aEndpointId, uint16_t aActionId) { Delegate * delegate = GetDelegate(aEndpointId); if (delegate == nullptr) @@ -171,7 +171,7 @@ bool ActionsServer::FindActionIdInActionList(EndpointId aEndpointId, uint16_t aA ChipLogError(Zcl, "Actions delegate is null!!!"); return false; } - return delegate->FindActionIdInActionList(aActionId); + return delegate->HaveActionWithId(aActionId); } template @@ -195,7 +195,7 @@ void ActionsServer::HandleCommand(HandlerContext & handlerContext, FuncT func) return; } - if (FindActionIdInActionList(handlerContext.mRequestPath.mEndpointId, requestPayload.actionID)) + if (HaveActionWithId(handlerContext.mRequestPath.mEndpointId, requestPayload.actionID)) { handlerContext.mCommandHandler.AddStatus(handlerContext.mRequestPath, Protocols::InteractionModel::Status::NotFound); return; @@ -208,7 +208,11 @@ void ActionsServer::HandleCommand(HandlerContext & handlerContext, FuncT func) void ActionsServer::HandleInstantAction(HandlerContext & ctx, const Commands::InstantAction::DecodableType & commandData) { Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandleInstantAction(commandData.actionID, commandData.invokeID); + Status status = Status::InvalidInState; + if (delegate != nullptr) + { + status = delegate->HandleInstantAction(commandData.actionID, commandData.invokeID); + } ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } @@ -216,15 +220,23 @@ void ActionsServer::HandleInstantActionWithTransition(HandlerContext & ctx, const Commands::InstantActionWithTransition::DecodableType & commandData) { Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = - delegate->HandleInstantActionWithTransition(commandData.actionID, commandData.transitionTime, commandData.invokeID); + Status status = Status::InvalidInState; + if (delegate != nullptr) + { + status = + delegate->HandleInstantActionWithTransition(commandData.actionID, commandData.transitionTime, commandData.invokeID); + } ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } void ActionsServer::HandleStartAction(HandlerContext & ctx, const Commands::StartAction::DecodableType & commandData) { Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandleStartAction(commandData.actionID, commandData.invokeID); + Status status = Status::InvalidInState; + if (delegate != nullptr) + { + status = delegate->HandleStartAction(commandData.actionID, commandData.invokeID); + } ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } @@ -232,21 +244,33 @@ void ActionsServer::HandleStartActionWithDuration(HandlerContext & ctx, const Commands::StartActionWithDuration::DecodableType & commandData) { Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandleStartActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID); + Status status = Status::InvalidInState; + if (delegate != nullptr) + { + status = delegate->HandleStartActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID); + } ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } void ActionsServer::HandleStopAction(HandlerContext & ctx, const Commands::StopAction::DecodableType & commandData) { Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandleStopAction(commandData.actionID, commandData.invokeID); + Status status = Status::InvalidInState; + if (delegate != nullptr) + { + status = delegate->HandleStopAction(commandData.actionID, commandData.invokeID); + } ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } void ActionsServer::HandlePauseAction(HandlerContext & ctx, const Commands::PauseAction::DecodableType & commandData) { Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandlePauseAction(commandData.actionID, commandData.invokeID); + Status status = Status::InvalidInState; + if (delegate != nullptr) + { + status = delegate->HandlePauseAction(commandData.actionID, commandData.invokeID); + } ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } @@ -254,21 +278,33 @@ void ActionsServer::HandlePauseActionWithDuration(HandlerContext & ctx, const Commands::PauseActionWithDuration::DecodableType & commandData) { Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandlePauseActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID); + Status status = Status::InvalidInState; + if (delegate != nullptr) + { + status = delegate->HandlePauseActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID); + } ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } void ActionsServer::HandleResumeAction(HandlerContext & ctx, const Commands::ResumeAction::DecodableType & commandData) { Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandleResumeAction(commandData.actionID, commandData.invokeID); + Status status = Status::InvalidInState; + if (delegate != nullptr) + { + status = delegate->HandleResumeAction(commandData.actionID, commandData.invokeID); + } ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } void ActionsServer::HandleEnableAction(HandlerContext & ctx, const Commands::EnableAction::DecodableType & commandData) { Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandleEnableAction(commandData.actionID, commandData.invokeID); + Status status = Status::InvalidInState; + if (delegate != nullptr) + { + status = delegate->HandleEnableAction(commandData.actionID, commandData.invokeID); + } ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } @@ -276,14 +312,22 @@ void ActionsServer::HandleEnableActionWithDuration(HandlerContext & ctx, const Commands::EnableActionWithDuration::DecodableType & commandData) { Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandleEnableActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID); + Status status = Status::InvalidInState; + if (delegate != nullptr) + { + status = delegate->HandleEnableActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID); + } ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } void ActionsServer::HandleDisableAction(HandlerContext & ctx, const Commands::DisableAction::DecodableType & commandData) { Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandleDisableAction(commandData.actionID, commandData.invokeID); + Status status = Status::InvalidInState; + if (delegate != nullptr) + { + status = delegate->HandleDisableAction(commandData.actionID, commandData.invokeID); + } ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } @@ -291,7 +335,11 @@ void ActionsServer::HandleDisableActionWithDuration(HandlerContext & ctx, const Commands::DisableActionWithDuration::DecodableType & commandData) { Delegate * delegate = GetDelegate(ctx.mRequestPath.mEndpointId); - Status status = delegate->HandleDisableActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID); + Status status = Status::InvalidInState; + if (delegate != nullptr) + { + status = delegate->HandleDisableActionWithDuration(commandData.actionID, commandData.duration, commandData.invokeID); + } ctx.mCommandHandler.AddStatus(ctx.mRequestPath, status); } diff --git a/src/app/clusters/actions-server/actions-server.h b/src/app/clusters/actions-server/actions-server.h index bd33db9e32d8d2..0fd18131153c55 100644 --- a/src/app/clusters/actions-server/actions-server.h +++ b/src/app/clusters/actions-server/actions-server.h @@ -48,16 +48,15 @@ class MatterContext struct ActionStructStorage : public Structs::ActionStruct::Type { - ActionStructStorage() : mActionName(mBuffer, sizeof(mBuffer)){}; + ActionStructStorage(){}; ActionStructStorage(uint16_t aAction, const CharSpan & aActionName, ActionTypeEnum aActionType, uint16_t aEpListID, - BitMask aCommands, ActionStateEnum aActionState) : - mActionName(mBuffer, sizeof(mBuffer)) + BitMask aCommands, ActionStateEnum aActionState) { Set(aAction, aActionName, aActionType, aEpListID, aCommands, aActionState); } - ActionStructStorage(const ActionStructStorage & aAction) : mActionName(mBuffer, sizeof(mBuffer)) { *this = aAction; } + ActionStructStorage(const ActionStructStorage & aAction) { *this = aAction; } ActionStructStorage & operator=(const ActionStructStorage & aAction) { @@ -68,33 +67,31 @@ struct ActionStructStorage : public Structs::ActionStruct::Type void Set(uint16_t aAction, const CharSpan & aActionName, ActionTypeEnum aActionType, uint16_t aEpListID, BitMask aCommands, ActionStateEnum aActionState) { - actionID = aAction; - type = aActionType; - endpointListID = aEpListID; - supportedCommands = aCommands; - state = aActionState; - mActionName = Span(mBuffer, sizeof(mBuffer)); + actionID = aAction; + type = aActionType; + endpointListID = aEpListID; + supportedCommands = aCommands; + state = aActionState; + MutableCharSpan mActionName = Span(mBuffer, sizeof(mBuffer)); CopyCharSpanToMutableCharSpanWithTruncation(aActionName, mActionName); name = mActionName; } private: char mBuffer[kActionNameMaxSize]; - MutableCharSpan mActionName; }; struct EndpointListStorage : public Structs::EndpointListStruct::Type { - EndpointListStorage() : mEpListName(mBuffer, sizeof(mBuffer)){}; + EndpointListStorage(){}; EndpointListStorage(uint16_t aEpListId, const CharSpan & aEpListName, EndpointListTypeEnum aEpListType, - const DataModel::List & aEndpointList) : - mEpListName(mBuffer, sizeof(mBuffer)) + const DataModel::List & aEndpointList) { Set(aEpListId, aEpListName, aEpListType, aEndpointList); } - EndpointListStorage(const EndpointListStorage & aEpList) : mEpListName(mBuffer, sizeof(mBuffer)) { *this = aEpList; } + EndpointListStorage(const EndpointListStorage & aEpList) { *this = aEpList; } EndpointListStorage & operator=(const EndpointListStorage & aEpList) { @@ -107,31 +104,30 @@ struct EndpointListStorage : public Structs::EndpointListStruct::Type { endpointListID = aEpListId; type = aEpListType; - size_t epListSize = std::min(aEndpointList.size(), kEndpointListMaxSize); + size_t epListSize = std::min(aEndpointList.size(), ArraySize(mEpList)); for (size_t index = 0; index < epListSize; index++) { mEpList[index] = aEndpointList[index]; } - endpoints = DataModel::List(Span(mEpList, epListSize)); - mEpListName = Span(mBuffer, sizeof(mBuffer)); + endpoints = DataModel::List(Span(mEpList, epListSize)); + MutableCharSpan mEpListName = Span(mBuffer, sizeof(mBuffer)); CopyCharSpanToMutableCharSpanWithTruncation(aEpListName, mEpListName); name = mEpListName; } private: char mBuffer[kEndpointListNameMaxSize]; - MutableCharSpan mEpListName; EndpointId mEpList[kEndpointListMaxSize]; }; -class ActionsServer : public AttributeAccessInterface, public CommandHandlerInterface, public MatterContext +class ActionsServer : public AttributeAccessInterface, public CommandHandlerInterface { public: // Register for the Actions cluster on all endpoints. ActionsServer() : AttributeAccessInterface(Optional::Missing(), Actions::Id), - CommandHandlerInterface(Optional::Missing(), Actions::Id), mMatterContext(*this) + CommandHandlerInterface(Optional::Missing(), Actions::Id) {} static ActionsServer & Instance(); @@ -148,7 +144,7 @@ class ActionsServer : public AttributeAccessInterface, public CommandHandlerInte void OnActionFailed(EndpointId aEndpoint, uint16_t aActionId, uint32_t aInvokeId, ActionStateEnum aActionState, ActionErrorEnum aActionError); - static void SetDefaultDelegate(EndpointId aEndpointId, Delegate * aDelegate); + void SetDefaultDelegate(EndpointId aEndpointId, Delegate * aDelegate); CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; @@ -178,7 +174,7 @@ class ActionsServer : public AttributeAccessInterface, public CommandHandlerInte private: static ActionsServer sInstance; - MatterContext & mMatterContext; + MatterContext mMatterContext; static constexpr size_t kMaxEndpointListLength = 256u; static constexpr size_t kMaxActionListLength = 256u; @@ -186,9 +182,9 @@ class ActionsServer : public AttributeAccessInterface, public CommandHandlerInte const AttributeValueEncoder::ListEncodeHelper & aEncoder); CHIP_ERROR ReadEndpointListAttribute(const ConcreteReadAttributePath & aPath, const AttributeValueEncoder::ListEncodeHelper & aEncoder); - bool FindActionIdInActionList(EndpointId aEndpointId, uint16_t aActionId); + bool HaveActionWithId(EndpointId aEndpointId, uint16_t aActionId); - // Cannot use CommandHandlerInterface::HandleCommand directly because we need to do the FindActionIdInActionList() check before + // Cannot use CommandHandlerInterface::HandleCommand directly because we need to do the HaveActionWithId() check before // sending a command. template void HandleCommand(HandlerContext & handlerContext, FuncT func); @@ -222,7 +218,7 @@ class Delegate * @param index The index of the action to be returned. It is assumed that actions are indexable from 0 and with no gaps. * @param action A reference to the ActionStructStorage which should be initialized via copy/assignments or calling Set(). * @return Returns a CHIP_NO_ERROR if there was no error and the action was returned successfully. - * CHIP_ERROR_PROVIDER_LIST_EXHAUSTED if the index in beyond the list of available actions. + * CHIP_ERROR_PROVIDER_LIST_EXHAUSTED if the index is past the end of the list of actionList. */ virtual CHIP_ERROR ReadActionAtIndex(uint16_t index, ActionStructStorage & action) = 0; @@ -231,16 +227,16 @@ class Delegate * @param index The index of the endpointList to be returned. It is assumed that actions are indexable from 0 and with no gaps. * @param action A reference to the EndpointListStorage which should be initialized via copy/assignments or calling Set(). * @return Returns a CHIP_NO_ERROR if there was no error and the epList was returned successfully. - * CHIP_ERROR_PROVIDER_LIST_EXHAUSTED if the index in beyond the list of available endpointList. + * CHIP_ERROR_PROVIDER_LIST_EXHAUSTED if the index is past the end of the list of endpointLists. */ virtual CHIP_ERROR ReadEndpointListAtIndex(uint16_t index, EndpointListStorage & epList) = 0; /** - * Find the action with matching actionId in the list of action. - * @param actionId The action to be find in the list of action. + * Check whether there is an action with the given actionId in the list of actions. + * @param actionId The action ID to search for. * @return Returns a true if matching action is found otherwise false. */ - virtual bool FindActionIdInActionList(uint16_t actionId) = 0; + virtual bool HaveActionWithId(uint16_t actionId) = 0; /** * On receipt of each and every command, @@ -255,7 +251,7 @@ class Delegate * When an InstantAction command is received, an action (state change) on the involved endpoints shall trigger, * in a "fire and forget" manner. Afterwards, the action’s state SHALL be Inactive. * - * @param actionId The id of an action on which an action shall takes place. + * @param actionId The actionId of an action. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ virtual Protocols::InteractionModel::Status HandleInstantAction(uint16_t actionId, Optional invokeId) = 0; @@ -265,7 +261,7 @@ class Delegate * with a specified time to transition from the current state to the new state. During the transition, the action’s state SHALL * be Active. Afterwards, the action’s state SHALL be Inactive. * - * @param actionId The id of an action on which an action shall takes place. + * @param actionId The actionId of an action. * @param transitionTime The time for transition from the current state to the new state. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ @@ -276,7 +272,7 @@ class Delegate * When a StartAction command is received, the commencement of an action on the involved endpoints shall trigger. Afterwards, * the action’s state SHALL be Inactive. * - * @param actionId The id of an action on which an action shall takes place. + * @param actionId The actionId of an action. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ virtual Protocols::InteractionModel::Status HandleStartAction(uint16_t actionId, Optional invokeId) = 0; @@ -286,7 +282,7 @@ class Delegate * and SHALL change the action’s state to Active. After the specified Duration, the action will stop, and the action’s state * SHALL change to Inactive. * - * @param actionId The id of an action on which an action shall takes place. + * @param actionId The actionId of an action. * @param duration The time for which an action shall be in start state. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ @@ -297,7 +293,7 @@ class Delegate * When a StopAction command is received, the ongoing action on the involved endpoints shall stop. Afterwards, the action’s * state SHALL be Inactive. * - * @param actionId The id of an action on which an action shall takes place. + * @param actionId The actionId of an action. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ virtual Protocols::InteractionModel::Status HandleStopAction(uint16_t actionId, Optional invokeId) = 0; @@ -306,7 +302,7 @@ class Delegate * When a PauseAction command is received, the ongoing action on the involved endpoints shall pause and SHALL change the * action’s state to Paused. * - * @param actionId The id of an action on which an action shall takes place. + * @param actionId The actionId of an action. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ virtual Protocols::InteractionModel::Status HandlePauseAction(uint16_t actionId, Optional invokeId) = 0; @@ -316,7 +312,7 @@ class Delegate * After the specified Duration, the ongoing action will be automatically resumed. which SHALL change the action’s state to * Active. * - * @param actionId The id of an action on which an action shall takes place. + * @param actionId The actionId of an action. * @param duration The time for which an action shall be in pause state. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ @@ -327,7 +323,7 @@ class Delegate * When a ResumeAction command is received, the previously paused action shall resume and SHALL change the action’s state to * Active. * - * @param actionId The id of an action on which an action shall takes place. + * @param actionId The actionId of an action. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ virtual Protocols::InteractionModel::Status HandleResumeAction(uint16_t actionId, Optional invokeId) = 0; @@ -336,7 +332,7 @@ class Delegate * When an EnableAction command is received, it enables a certain action or automation. Afterwards, the action’s state SHALL be * Active. * - * @param actionId The id of an action on which an action shall takes place. + * @param actionId The actionId of an action. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ virtual Protocols::InteractionModel::Status HandleEnableAction(uint16_t actionId, Optional invokeId) = 0; @@ -346,7 +342,7 @@ class Delegate * action’s state to be Active. After the specified Duration, the action or automation will stop, and the action’s state SHALL * change to Disabled. * - * @param actionId The id of an action on which an action shall takes place. + * @param actionId The actionId of an action. * @param duration The time for which an action shall be in active state. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ @@ -357,7 +353,7 @@ class Delegate * When a DisableAction command is received, it disables a certain action or automation, and SHALL change the action’s state to * Inactive. * - * @param actionId The id of an action on which an action shall takes place. + * @param actionId The actionId of an action. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ virtual Protocols::InteractionModel::Status HandleDisableAction(uint16_t actionId, Optional invokeId) = 0; @@ -367,7 +363,7 @@ class Delegate * action’s state to Disabled. After the specified Duration, the action or automation will re-start, and the action’s state * SHALL change to either Inactive or Active, depending on the actions. * - * @param actionId The id of an action on which an action shall takes place. + * @param actionId The actionId of an action. * @param duration The time for which an action shall be in disable state. * @return Returns a Success if an action took place successfully otherwise, suitable error. */ diff --git a/src/app/tests/TestActionsCluster.cpp b/src/app/tests/TestActionsCluster.cpp index 027c0c60b17f40..6e8e36d08f5ca3 100644 --- a/src/app/tests/TestActionsCluster.cpp +++ b/src/app/tests/TestActionsCluster.cpp @@ -87,7 +87,7 @@ class TestActionsDelegateImpl : public Clusters::Actions::Delegate return CHIP_NO_ERROR; } - bool FindActionIdInActionList(uint16_t actionId) override + bool HaveActionWithId(uint16_t actionId) override { for (uint16_t i = 0; i < mNumActions; i++) { From d09a5a0d2ae81c380b120c781da2f39feea69fc9 Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Mon, 3 Feb 2025 17:56:45 +0530 Subject: [PATCH 18/25] Update the docs, changed API name as suggested, and fix CI for esp32 --- .../actions-server/actions-server.cpp | 26 ++++++----- .../clusters/actions-server/actions-server.h | 44 +++++++++---------- .../mock/include/zap-generated/gen_config.h | 3 +- 3 files changed, 40 insertions(+), 33 deletions(-) diff --git a/src/app/clusters/actions-server/actions-server.cpp b/src/app/clusters/actions-server/actions-server.cpp index 6efa9d320dbd75..2f5e0ceeace7ba 100644 --- a/src/app/clusters/actions-server/actions-server.cpp +++ b/src/app/clusters/actions-server/actions-server.cpp @@ -32,13 +32,11 @@ using namespace chip::app::Clusters::Actions; using namespace chip::app::Clusters::Actions::Attributes; using namespace chip::Protocols::InteractionModel; -#ifndef MATTER_DM_ACTIONS_CLUSTER_SERVER_ENDPOINT_COUNT -#define MATTER_DM_ACTIONS_CLUSTER_SERVER_ENDPOINT_COUNT 1 -#endif - namespace { static constexpr size_t kActionsDelegateTableSize = MATTER_DM_ACTIONS_CLUSTER_SERVER_ENDPOINT_COUNT + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT; +static_assert(kActionsDelegateTableSize <= kEmberInvalidEndpointIndex, "Actions Delegate table size error"); + Delegate * gDelegateTable[kActionsDelegateTableSize] = { nullptr }; Delegate * GetDelegate(EndpointId aEndpoint) @@ -403,7 +401,7 @@ void ActionsServer::InvokeCommand(HandlerContext & handlerContext) } } -CHIP_ERROR ActionsServer::SetActionList(EndpointId aEndpoint, const ActionStructStorage & aAction) +CHIP_ERROR ActionsServer::ModifyActionList(EndpointId aEndpoint, const ActionStructStorage & aAction) { Delegate * delegate = GetDelegate(aEndpoint); if (delegate == nullptr) @@ -412,11 +410,15 @@ CHIP_ERROR ActionsServer::SetActionList(EndpointId aEndpoint, const ActionStruct return CHIP_ERROR_INCORRECT_STATE; } - // Read through the list to find and update the existing action + // Read through the list to find and update the existing action that matches the passed-in action's ID. for (uint16_t i = 0; i < kMaxActionListLength; i++) { ActionStructStorage existingAction; - ReturnErrorOnFailure(delegate->ReadActionAtIndex(i, existingAction)); + CHIP_ERROR err = delegate->ReadActionAtIndex(i, existingAction); + if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED) + { + break; + } if (existingAction.actionID == aAction.actionID) { @@ -432,7 +434,7 @@ CHIP_ERROR ActionsServer::SetActionList(EndpointId aEndpoint, const ActionStruct return CHIP_ERROR_NOT_FOUND; } -CHIP_ERROR ActionsServer::SetEndpointList(EndpointId aEndpoint, const EndpointListStorage & aEpList) +CHIP_ERROR ActionsServer::ModifyEndpointList(EndpointId aEndpoint, const EndpointListStorage & aEpList) { Delegate * delegate = GetDelegate(aEndpoint); if (delegate == nullptr) @@ -441,11 +443,15 @@ CHIP_ERROR ActionsServer::SetEndpointList(EndpointId aEndpoint, const EndpointLi return CHIP_ERROR_INCORRECT_STATE; } - // Read through the list to find and update the existing endpoint list + // Read through the list to find and update the existing action that matches the passed-in endpoint-list's ID for (uint16_t i = 0; i < kMaxEndpointListLength; i++) { EndpointListStorage existingEpList; - ReturnErrorOnFailure(delegate->ReadEndpointListAtIndex(i, existingEpList)); + CHIP_ERROR err = delegate->ReadEndpointListAtIndex(i, existingEpList); + if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED) + { + break; + } if (existingEpList.endpointListID == aEpList.endpointListID) { diff --git a/src/app/clusters/actions-server/actions-server.h b/src/app/clusters/actions-server/actions-server.h index 0fd18131153c55..c811bf877f9820 100644 --- a/src/app/clusters/actions-server/actions-server.h +++ b/src/app/clusters/actions-server/actions-server.h @@ -67,12 +67,12 @@ struct ActionStructStorage : public Structs::ActionStruct::Type void Set(uint16_t aAction, const CharSpan & aActionName, ActionTypeEnum aActionType, uint16_t aEpListID, BitMask aCommands, ActionStateEnum aActionState) { - actionID = aAction; - type = aActionType; - endpointListID = aEpListID; - supportedCommands = aCommands; - state = aActionState; - MutableCharSpan mActionName = Span(mBuffer, sizeof(mBuffer)); + actionID = aAction; + type = aActionType; + endpointListID = aEpListID; + supportedCommands = aCommands; + state = aActionState; + MutableCharSpan mActionName(mBuffer); CopyCharSpanToMutableCharSpanWithTruncation(aActionName, mActionName); name = mActionName; } @@ -158,7 +158,7 @@ class ActionsServer : public AttributeAccessInterface, public CommandHandlerInte * CHIP_ERROR_NOT_FOUND if action doesn't exist * CHIP_NO_ERROR if successful */ - CHIP_ERROR SetActionList(EndpointId aEndpoint, const ActionStructStorage & aAction); + CHIP_ERROR ModifyActionList(EndpointId aEndpoint, const ActionStructStorage & aAction); /** * Update an existing endpoint list for the given endpoint. @@ -170,7 +170,7 @@ class ActionsServer : public AttributeAccessInterface, public CommandHandlerInte * CHIP_ERROR_NOT_FOUND if endpoint list doesn't exist * CHIP_NO_ERROR if successful */ - CHIP_ERROR SetEndpointList(EndpointId aEndpoint, const EndpointListStorage & aEpList); + CHIP_ERROR ModifyEndpointList(EndpointId aEndpoint, const EndpointListStorage & aEpList); private: static ActionsServer sInstance; @@ -243,8 +243,8 @@ class Delegate * if the InvokeID data field is provided by the client when invoking a command, the server SHALL generate a StateChanged event * when the action changes to a new state or an ActionFailed event when execution of the action fails. * - * @return If the command refers to an action which currently is not in a state where the command applies, a response SHALL be - * generated with the StatusCode INVALID_COMMAND. + * If the command refers to an action which currently is not in a state where the command applies, a response SHALL be + * generated with the Status::InvalidCommand. */ /** @@ -252,7 +252,7 @@ class Delegate * in a "fire and forget" manner. Afterwards, the action’s state SHALL be Inactive. * * @param actionId The actionId of an action. - * @return Returns a Success if an action took place successfully otherwise, suitable error. + * It should report Status::Success if successful and may report other Status codes if it fails */ virtual Protocols::InteractionModel::Status HandleInstantAction(uint16_t actionId, Optional invokeId) = 0; @@ -263,7 +263,7 @@ class Delegate * * @param actionId The actionId of an action. * @param transitionTime The time for transition from the current state to the new state. - * @return Returns a Success if an action took place successfully otherwise, suitable error. + * It should report Status::Success if successful and may report other Status codes if it fails */ virtual Protocols::InteractionModel::Status HandleInstantActionWithTransition(uint16_t actionId, uint16_t transitionTime, Optional invokeId) = 0; @@ -273,7 +273,7 @@ class Delegate * the action’s state SHALL be Inactive. * * @param actionId The actionId of an action. - * @return Returns a Success if an action took place successfully otherwise, suitable error. + * It should report Status::Success if successful and may report other Status codes if it fails */ virtual Protocols::InteractionModel::Status HandleStartAction(uint16_t actionId, Optional invokeId) = 0; @@ -284,7 +284,7 @@ class Delegate * * @param actionId The actionId of an action. * @param duration The time for which an action shall be in start state. - * @return Returns a Success if an action took place successfully otherwise, suitable error. + * It should report Status::Success if successful and may report other Status codes if it fails */ virtual Protocols::InteractionModel::Status HandleStartActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) = 0; @@ -294,7 +294,7 @@ class Delegate * state SHALL be Inactive. * * @param actionId The actionId of an action. - * @return Returns a Success if an action took place successfully otherwise, suitable error. + * It should report Status::Success if successful and may report other Status codes if it fails */ virtual Protocols::InteractionModel::Status HandleStopAction(uint16_t actionId, Optional invokeId) = 0; @@ -303,7 +303,7 @@ class Delegate * action’s state to Paused. * * @param actionId The actionId of an action. - * @return Returns a Success if an action took place successfully otherwise, suitable error. + * It should report Status::Success if successful and may report other Status codes if it fails */ virtual Protocols::InteractionModel::Status HandlePauseAction(uint16_t actionId, Optional invokeId) = 0; @@ -314,7 +314,7 @@ class Delegate * * @param actionId The actionId of an action. * @param duration The time for which an action shall be in pause state. - * @return Returns a Success if an action took place successfully otherwise, suitable error. + * It should report Status::Success if successful and may report other Status codes if it fails */ virtual Protocols::InteractionModel::Status HandlePauseActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) = 0; @@ -324,7 +324,7 @@ class Delegate * Active. * * @param actionId The actionId of an action. - * @return Returns a Success if an action took place successfully otherwise, suitable error. + * It should report Status::Success if successful and may report other Status codes if it fails */ virtual Protocols::InteractionModel::Status HandleResumeAction(uint16_t actionId, Optional invokeId) = 0; @@ -333,7 +333,7 @@ class Delegate * Active. * * @param actionId The actionId of an action. - * @return Returns a Success if an action took place successfully otherwise, suitable error. + * It should report Status::Success if successful and may report other Status codes if it fails */ virtual Protocols::InteractionModel::Status HandleEnableAction(uint16_t actionId, Optional invokeId) = 0; @@ -344,7 +344,7 @@ class Delegate * * @param actionId The actionId of an action. * @param duration The time for which an action shall be in active state. - * @return Returns a Success if an action took place successfully otherwise, suitable error. + * It should report Status::Success if successful and may report other Status codes if it fails */ virtual Protocols::InteractionModel::Status HandleEnableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) = 0; @@ -354,7 +354,7 @@ class Delegate * Inactive. * * @param actionId The actionId of an action. - * @return Returns a Success if an action took place successfully otherwise, suitable error. + * It should report Status::Success if successful and may report other Status codes if it fails */ virtual Protocols::InteractionModel::Status HandleDisableAction(uint16_t actionId, Optional invokeId) = 0; @@ -365,7 +365,7 @@ class Delegate * * @param actionId The actionId of an action. * @param duration The time for which an action shall be in disable state. - * @return Returns a Success if an action took place successfully otherwise, suitable error. + * It should report Status::Success if successful and may report other Status codes if it fails */ virtual Protocols::InteractionModel::Status HandleDisableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) = 0; diff --git a/src/app/util/mock/include/zap-generated/gen_config.h b/src/app/util/mock/include/zap-generated/gen_config.h index e3a53a6a6573bd..6200721d0b0509 100644 --- a/src/app/util/mock/include/zap-generated/gen_config.h +++ b/src/app/util/mock/include/zap-generated/gen_config.h @@ -29,7 +29,8 @@ #define MATTER_DM_DESCRIPTOR_CLUSTER_SERVER_ENDPOINT_COUNT (0) #define MATTER_DM_BINDING_CLUSTER_SERVER_ENDPOINT_COUNT (0) #define MATTER_DM_ACCESS_CONTROL_CLUSTER_SERVER_ENDPOINT_COUNT (0) -#define MATTER_DM_ACTIONS_CLUSTER_SERVER_ENDPOINT_COUNT (0) +// "The value of MATTER_DM_ACTIONS_CLUSTER_SERVER_ENDPOINT_COUNT needs to be at least one for unit testing the cluster. +#define MATTER_DM_ACTIONS_CLUSTER_SERVER_ENDPOINT_COUNT (1) #define MATTER_DM_BASIC_INFORMATION_CLUSTER_SERVER_ENDPOINT_COUNT (0) #define MATTER_DM_OTA_SOFTWARE_UPDATE_PROVIDER_CLUSTER_SERVER_ENDPOINT_COUNT (0) #define MATTER_DM_OTA_SOFTWARE_UPDATE_REQUESTOR_CLUSTER_SERVER_ENDPOINT_COUNT (0) From 3df36ecd371f1698ca43fe9d02eeab8813e99f6f Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Mon, 3 Feb 2025 18:05:21 +0530 Subject: [PATCH 19/25] Removed the notify delegates API on existing ActionList, EndpointList change. --- .../include/bridged-actions-stub.h | 3 --- .../clusters/actions-server/actions-server.cpp | 2 -- src/app/clusters/actions-server/actions-server.h | 16 ---------------- src/app/tests/TestActionsCluster.cpp | 3 --- 4 files changed, 24 deletions(-) diff --git a/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h b/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h index a141e3568b517a..07f80aa9cbef64 100644 --- a/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h +++ b/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h @@ -56,9 +56,6 @@ class ActionsDelegateImpl : public Delegate CHIP_ERROR ReadEndpointListAtIndex(uint16_t index, EndpointListStorage & epList) override; bool HaveActionWithId(uint16_t actionId) override; - void OnActionListChanged(EndpointId endpoint, const ActionStructStorage & action) override{}; - void OnEndpointListChanged(EndpointId endpoint, const EndpointListStorage & action) override{}; - Protocols::InteractionModel::Status HandleInstantAction(uint16_t actionId, Optional invokeId) override; Protocols::InteractionModel::Status HandleInstantActionWithTransition(uint16_t actionId, uint16_t transitionTime, Optional invokeId) override; diff --git a/src/app/clusters/actions-server/actions-server.cpp b/src/app/clusters/actions-server/actions-server.cpp index 2f5e0ceeace7ba..2bb2b87b1958a6 100644 --- a/src/app/clusters/actions-server/actions-server.cpp +++ b/src/app/clusters/actions-server/actions-server.cpp @@ -425,7 +425,6 @@ CHIP_ERROR ActionsServer::ModifyActionList(EndpointId aEndpoint, const ActionStr existingAction.Set(aAction.actionID, aAction.name, aAction.type, aAction.endpointListID, aAction.supportedCommands, aAction.state); - delegate->OnActionListChanged(aEndpoint, aAction); mMatterContext.MarkDirty(aEndpoint, Attributes::ActionList::Id); return CHIP_NO_ERROR; } @@ -457,7 +456,6 @@ CHIP_ERROR ActionsServer::ModifyEndpointList(EndpointId aEndpoint, const Endpoin { existingEpList.Set(aEpList.endpointListID, aEpList.name, aEpList.type, aEpList.endpoints); mMatterContext.MarkDirty(aEndpoint, Attributes::EndpointLists::Id); - delegate->OnEndpointListChanged(aEndpoint, aEpList); return CHIP_NO_ERROR; } } diff --git a/src/app/clusters/actions-server/actions-server.h b/src/app/clusters/actions-server/actions-server.h index c811bf877f9820..2a46f51a173358 100644 --- a/src/app/clusters/actions-server/actions-server.h +++ b/src/app/clusters/actions-server/actions-server.h @@ -369,22 +369,6 @@ class Delegate */ virtual Protocols::InteractionModel::Status HandleDisableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) = 0; - - /** - * Called when an action list has been updated. - * - * @param endpoint The endpoint ID where the action list was updated - * @param action The updated action details - */ - virtual void OnActionListChanged(EndpointId endpoint, const ActionStructStorage & action) = 0; - - /** - * Called when an endpoint list has been updated. - * - * @param endpoint The endpoint ID where the endpoint list was updated - * @param epList The updated endpoint list details - */ - virtual void OnEndpointListChanged(EndpointId endpoint, const EndpointListStorage & epList) = 0; }; } // namespace Actions diff --git a/src/app/tests/TestActionsCluster.cpp b/src/app/tests/TestActionsCluster.cpp index 6e8e36d08f5ca3..f63b2759f54d0a 100644 --- a/src/app/tests/TestActionsCluster.cpp +++ b/src/app/tests/TestActionsCluster.cpp @@ -120,9 +120,6 @@ class TestActionsDelegateImpl : public Clusters::Actions::Delegate return CHIP_NO_ERROR; } - void OnActionListChanged(EndpointId endpoint, const ActionStructStorage & action) override{}; - void OnEndpointListChanged(EndpointId endpoint, const EndpointListStorage & action) override{}; - Protocols::InteractionModel::Status HandleInstantAction(uint16_t actionId, Optional invokeId) override { return Status::NotFound; From e24d753864838d7e25cff26a9dbb1aaa906abb95 Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Tue, 4 Feb 2025 14:17:55 +0530 Subject: [PATCH 20/25] Fix API signature in linux/bridge-app. --- examples/bridge-app/linux/bridged-actions-stub.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/bridge-app/linux/bridged-actions-stub.cpp b/examples/bridge-app/linux/bridged-actions-stub.cpp index f5224199a03336..b40c040d6ebef2 100644 --- a/examples/bridge-app/linux/bridged-actions-stub.cpp +++ b/examples/bridge-app/linux/bridged-actions-stub.cpp @@ -131,7 +131,7 @@ CHIP_ERROR ActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, Attr } } // anonymous namespace -void emberAfActionsClusterInitCallback() +void emberAfActionsClusterInitCallback(EndpointId endpoint) { AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess); } From 611390516bf64532553e95d10de9a8ac9834fb60 Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Tue, 11 Feb 2025 12:03:30 +0530 Subject: [PATCH 21/25] Removed public MatterContext APIs, added todo to remove global delegates array. --- .../clusters/actions-server/actions-server.cpp | 5 +++-- src/app/clusters/actions-server/actions-server.h | 16 ++++------------ 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/app/clusters/actions-server/actions-server.cpp b/src/app/clusters/actions-server/actions-server.cpp index 2bb2b87b1958a6..70053fbf9fbf18 100644 --- a/src/app/clusters/actions-server/actions-server.cpp +++ b/src/app/clusters/actions-server/actions-server.cpp @@ -37,6 +37,7 @@ static constexpr size_t kActionsDelegateTableSize = MATTER_DM_ACTIONS_CLUSTER_SERVER_ENDPOINT_COUNT + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT; static_assert(kActionsDelegateTableSize <= kEmberInvalidEndpointIndex, "Actions Delegate table size error"); +// TODO: We should not use global array, instead we can use one cluster instance per endpoint. Delegate * gDelegateTable[kActionsDelegateTableSize] = { nullptr }; Delegate * GetDelegate(EndpointId aEndpoint) @@ -425,7 +426,7 @@ CHIP_ERROR ActionsServer::ModifyActionList(EndpointId aEndpoint, const ActionStr existingAction.Set(aAction.actionID, aAction.name, aAction.type, aAction.endpointListID, aAction.supportedCommands, aAction.state); - mMatterContext.MarkDirty(aEndpoint, Attributes::ActionList::Id); + MarkDirty(aEndpoint, Attributes::ActionList::Id); return CHIP_NO_ERROR; } } @@ -455,7 +456,7 @@ CHIP_ERROR ActionsServer::ModifyEndpointList(EndpointId aEndpoint, const Endpoin if (existingEpList.endpointListID == aEpList.endpointListID) { existingEpList.Set(aEpList.endpointListID, aEpList.name, aEpList.type, aEpList.endpoints); - mMatterContext.MarkDirty(aEndpoint, Attributes::EndpointLists::Id); + MarkDirty(aEndpoint, Attributes::EndpointLists::Id); return CHIP_NO_ERROR; } } diff --git a/src/app/clusters/actions-server/actions-server.h b/src/app/clusters/actions-server/actions-server.h index 2a46f51a173358..ad38976e574d22 100644 --- a/src/app/clusters/actions-server/actions-server.h +++ b/src/app/clusters/actions-server/actions-server.h @@ -35,17 +35,6 @@ static constexpr size_t kEndpointListMaxSize = 256u; class Delegate; -class MatterContext -{ -public: - virtual ~MatterContext() = default; - // MarkDirty - virtual void MarkDirty(EndpointId aEndpointId, AttributeId aAttributeId) - { - MatterReportingAttributeChangeCallback(aEndpointId, Id, aAttributeId); - } -}; - struct ActionStructStorage : public Structs::ActionStruct::Type { ActionStructStorage(){}; @@ -174,7 +163,6 @@ class ActionsServer : public AttributeAccessInterface, public CommandHandlerInte private: static ActionsServer sInstance; - MatterContext mMatterContext; static constexpr size_t kMaxEndpointListLength = 256u; static constexpr size_t kMaxActionListLength = 256u; @@ -184,6 +172,10 @@ class ActionsServer : public AttributeAccessInterface, public CommandHandlerInte const AttributeValueEncoder::ListEncodeHelper & aEncoder); bool HaveActionWithId(EndpointId aEndpointId, uint16_t aActionId); + void MarkDirty(EndpointId aEndpointId, AttributeId aAttributeId) + { + MatterReportingAttributeChangeCallback(aEndpointId, Id, aAttributeId); + } // Cannot use CommandHandlerInterface::HandleCommand directly because we need to do the HaveActionWithId() check before // sending a command. template From bfe646fd3278be2f4195f613d814710e6c7ef90c Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Thu, 13 Feb 2025 21:59:56 +0530 Subject: [PATCH 22/25] Address some minor comments from code review. --- .../actions-server/actions-server.cpp | 41 ++++++++----------- .../clusters/actions-server/actions-server.h | 5 ++- 2 files changed, 19 insertions(+), 27 deletions(-) diff --git a/src/app/clusters/actions-server/actions-server.cpp b/src/app/clusters/actions-server/actions-server.cpp index 70053fbf9fbf18..4c0a676949c621 100644 --- a/src/app/clusters/actions-server/actions-server.cpp +++ b/src/app/clusters/actions-server/actions-server.cpp @@ -45,6 +45,16 @@ Delegate * GetDelegate(EndpointId aEndpoint) return (aEndpoint >= kActionsDelegateTableSize ? nullptr : gDelegateTable[aEndpoint]); } +CHIP_ERROR IsValid(Delegate * delegate) +{ + if (delegate == nullptr) + { + ChipLogError(Zcl, "Actions delegate is null!!!"); + return CHIP_ERROR_INCORRECT_STATE; + } + return CHIP_NO_ERROR; +} + } // namespace ActionsServer ActionsServer::sInstance; @@ -117,11 +127,7 @@ CHIP_ERROR ActionsServer::ReadActionListAttribute(const ConcreteReadAttributePat const AttributeValueEncoder::ListEncodeHelper & aEncoder) { Delegate * delegate = GetDelegate(aPath.mEndpointId); - if (delegate == nullptr) - { - ChipLogError(Zcl, "Actions delegate is null!!!"); - return CHIP_ERROR_INCORRECT_STATE; - } + VerifyOrReturnError(IsValid(delegate) == CHIP_NO_ERROR, CHIP_ERROR_INCORRECT_STATE); for (uint16_t i = 0; i < kMaxActionListLength; i++) { @@ -142,11 +148,8 @@ CHIP_ERROR ActionsServer::ReadEndpointListAttribute(const ConcreteReadAttributeP const AttributeValueEncoder::ListEncodeHelper & aEncoder) { Delegate * delegate = GetDelegate(aPath.mEndpointId); - if (delegate == nullptr) - { - ChipLogError(Zcl, "Actions delegate is null!!!"); - return CHIP_ERROR_INCORRECT_STATE; - } + VerifyOrReturnError(IsValid(delegate) == CHIP_NO_ERROR, CHIP_ERROR_INCORRECT_STATE); + for (uint16_t i = 0; i < kMaxEndpointListLength; i++) { EndpointListStorage epList; @@ -165,11 +168,7 @@ CHIP_ERROR ActionsServer::ReadEndpointListAttribute(const ConcreteReadAttributeP bool ActionsServer::HaveActionWithId(EndpointId aEndpointId, uint16_t aActionId) { Delegate * delegate = GetDelegate(aEndpointId); - if (delegate == nullptr) - { - ChipLogError(Zcl, "Actions delegate is null!!!"); - return false; - } + VerifyOrReturnValue(IsValid(delegate) == CHIP_NO_ERROR, false); return delegate->HaveActionWithId(aActionId); } @@ -405,11 +404,7 @@ void ActionsServer::InvokeCommand(HandlerContext & handlerContext) CHIP_ERROR ActionsServer::ModifyActionList(EndpointId aEndpoint, const ActionStructStorage & aAction) { Delegate * delegate = GetDelegate(aEndpoint); - if (delegate == nullptr) - { - ChipLogError(Zcl, "Actions delegate is null!"); - return CHIP_ERROR_INCORRECT_STATE; - } + VerifyOrReturnError(IsValid(delegate) == CHIP_NO_ERROR, CHIP_ERROR_INCORRECT_STATE); // Read through the list to find and update the existing action that matches the passed-in action's ID. for (uint16_t i = 0; i < kMaxActionListLength; i++) @@ -437,11 +432,7 @@ CHIP_ERROR ActionsServer::ModifyActionList(EndpointId aEndpoint, const ActionStr CHIP_ERROR ActionsServer::ModifyEndpointList(EndpointId aEndpoint, const EndpointListStorage & aEpList) { Delegate * delegate = GetDelegate(aEndpoint); - if (delegate == nullptr) - { - ChipLogError(Zcl, "Actions delegate is null!"); - return CHIP_ERROR_INCORRECT_STATE; - } + VerifyOrReturnError(IsValid(delegate) == CHIP_NO_ERROR, CHIP_ERROR_INCORRECT_STATE); // Read through the list to find and update the existing action that matches the passed-in endpoint-list's ID for (uint16_t i = 0; i < kMaxEndpointListLength; i++) diff --git a/src/app/clusters/actions-server/actions-server.h b/src/app/clusters/actions-server/actions-server.h index ad38976e574d22..74bb03a886027e 100644 --- a/src/app/clusters/actions-server/actions-server.h +++ b/src/app/clusters/actions-server/actions-server.h @@ -99,8 +99,8 @@ struct EndpointListStorage : public Structs::EndpointListStruct::Type { mEpList[index] = aEndpointList[index]; } - endpoints = DataModel::List(Span(mEpList, epListSize)); - MutableCharSpan mEpListName = Span(mBuffer, sizeof(mBuffer)); + endpoints = DataModel::List(Span(mEpList, epListSize)); + MutableCharSpan mEpListName(mBuffer); CopyCharSpanToMutableCharSpanWithTruncation(aEpListName, mEpListName); name = mEpListName; } @@ -172,6 +172,7 @@ class ActionsServer : public AttributeAccessInterface, public CommandHandlerInte const AttributeValueEncoder::ListEncodeHelper & aEncoder); bool HaveActionWithId(EndpointId aEndpointId, uint16_t aActionId); + // TODO: We should move to non-global dirty marker. void MarkDirty(EndpointId aEndpointId, AttributeId aAttributeId) { MatterReportingAttributeChangeCallback(aEndpointId, Id, aAttributeId); From ffca2940f2eaa6bac640c5487fa53785f857f9b1 Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Fri, 14 Feb 2025 16:34:51 +0530 Subject: [PATCH 23/25] Address review comments. --- .../all-clusters-app/linux/main-common.cpp | 1 + .../actions-server/actions-server.cpp | 18 +-- .../clusters/actions-server/actions-server.h | 107 +++++++----------- 3 files changed, 48 insertions(+), 78 deletions(-) diff --git a/examples/all-clusters-app/linux/main-common.cpp b/examples/all-clusters-app/linux/main-common.cpp index 0981e0dd455d29..b9026a291817fb 100644 --- a/examples/all-clusters-app/linux/main-common.cpp +++ b/examples/all-clusters-app/linux/main-common.cpp @@ -344,6 +344,7 @@ void emberAfThermostatClusterInitCallback(EndpointId endpoint) void emberAfActionsClusterInitCallback(EndpointId endpoint) { + VerifyOrReturn(endpoint == 1); Clusters::Actions::ActionsServer::Instance().SetDefaultDelegate(endpoint, &sActionsDelegateImpl); } diff --git a/src/app/clusters/actions-server/actions-server.cpp b/src/app/clusters/actions-server/actions-server.cpp index 4c0a676949c621..118a051cf0f8d0 100644 --- a/src/app/clusters/actions-server/actions-server.cpp +++ b/src/app/clusters/actions-server/actions-server.cpp @@ -45,11 +45,11 @@ Delegate * GetDelegate(EndpointId aEndpoint) return (aEndpoint >= kActionsDelegateTableSize ? nullptr : gDelegateTable[aEndpoint]); } -CHIP_ERROR IsValid(Delegate * delegate) +CHIP_ERROR ValidateDelegate(Delegate * aDelegate, EndpointId aEndpoint) { - if (delegate == nullptr) + if (aDelegate == nullptr) { - ChipLogError(Zcl, "Actions delegate is null!!!"); + ChipLogError(Zcl, "Actions delegate is null for endpoint: %d !!!", aEndpoint); return CHIP_ERROR_INCORRECT_STATE; } return CHIP_NO_ERROR; @@ -127,7 +127,7 @@ CHIP_ERROR ActionsServer::ReadActionListAttribute(const ConcreteReadAttributePat const AttributeValueEncoder::ListEncodeHelper & aEncoder) { Delegate * delegate = GetDelegate(aPath.mEndpointId); - VerifyOrReturnError(IsValid(delegate) == CHIP_NO_ERROR, CHIP_ERROR_INCORRECT_STATE); + ReturnErrorOnFailure(ValidateDelegate(delegate, aPath.mEndpointId)); for (uint16_t i = 0; i < kMaxActionListLength; i++) { @@ -148,7 +148,7 @@ CHIP_ERROR ActionsServer::ReadEndpointListAttribute(const ConcreteReadAttributeP const AttributeValueEncoder::ListEncodeHelper & aEncoder) { Delegate * delegate = GetDelegate(aPath.mEndpointId); - VerifyOrReturnError(IsValid(delegate) == CHIP_NO_ERROR, CHIP_ERROR_INCORRECT_STATE); + ReturnErrorOnFailure(ValidateDelegate(delegate, aPath.mEndpointId)); for (uint16_t i = 0; i < kMaxEndpointListLength; i++) { @@ -168,7 +168,7 @@ CHIP_ERROR ActionsServer::ReadEndpointListAttribute(const ConcreteReadAttributeP bool ActionsServer::HaveActionWithId(EndpointId aEndpointId, uint16_t aActionId) { Delegate * delegate = GetDelegate(aEndpointId); - VerifyOrReturnValue(IsValid(delegate) == CHIP_NO_ERROR, false); + VerifyOrReturnValue(ValidateDelegate(delegate, aEndpointId) == CHIP_NO_ERROR, false); return delegate->HaveActionWithId(aActionId); } @@ -193,7 +193,7 @@ void ActionsServer::HandleCommand(HandlerContext & handlerContext, FuncT func) return; } - if (HaveActionWithId(handlerContext.mRequestPath.mEndpointId, requestPayload.actionID)) + if (!HaveActionWithId(handlerContext.mRequestPath.mEndpointId, requestPayload.actionID)) { handlerContext.mCommandHandler.AddStatus(handlerContext.mRequestPath, Protocols::InteractionModel::Status::NotFound); return; @@ -404,7 +404,7 @@ void ActionsServer::InvokeCommand(HandlerContext & handlerContext) CHIP_ERROR ActionsServer::ModifyActionList(EndpointId aEndpoint, const ActionStructStorage & aAction) { Delegate * delegate = GetDelegate(aEndpoint); - VerifyOrReturnError(IsValid(delegate) == CHIP_NO_ERROR, CHIP_ERROR_INCORRECT_STATE); + ReturnErrorOnFailure(ValidateDelegate(delegate, aEndpoint)); // Read through the list to find and update the existing action that matches the passed-in action's ID. for (uint16_t i = 0; i < kMaxActionListLength; i++) @@ -432,7 +432,7 @@ CHIP_ERROR ActionsServer::ModifyActionList(EndpointId aEndpoint, const ActionStr CHIP_ERROR ActionsServer::ModifyEndpointList(EndpointId aEndpoint, const EndpointListStorage & aEpList) { Delegate * delegate = GetDelegate(aEndpoint); - VerifyOrReturnError(IsValid(delegate) == CHIP_NO_ERROR, CHIP_ERROR_INCORRECT_STATE); + ReturnErrorOnFailure(ValidateDelegate(delegate, aEndpoint)); // Read through the list to find and update the existing action that matches the passed-in endpoint-list's ID for (uint16_t i = 0; i < kMaxEndpointListLength; i++) diff --git a/src/app/clusters/actions-server/actions-server.h b/src/app/clusters/actions-server/actions-server.h index 74bb03a886027e..e73bc52c508566 100644 --- a/src/app/clusters/actions-server/actions-server.h +++ b/src/app/clusters/actions-server/actions-server.h @@ -61,9 +61,9 @@ struct ActionStructStorage : public Structs::ActionStruct::Type endpointListID = aEpListID; supportedCommands = aCommands; state = aActionState; - MutableCharSpan mActionName(mBuffer); - CopyCharSpanToMutableCharSpanWithTruncation(aActionName, mActionName); - name = mActionName; + MutableCharSpan actionName(mBuffer); + CopyCharSpanToMutableCharSpanWithTruncation(aActionName, actionName); + name = actionName; } private: @@ -100,9 +100,9 @@ struct EndpointListStorage : public Structs::EndpointListStruct::Type mEpList[index] = aEndpointList[index]; } endpoints = DataModel::List(Span(mEpList, epListSize)); - MutableCharSpan mEpListName(mBuffer); - CopyCharSpanToMutableCharSpanWithTruncation(aEpListName, mEpListName); - name = mEpListName; + MutableCharSpan epListName(mBuffer); + CopyCharSpanToMutableCharSpanWithTruncation(aEpListName, epListName); + name = epListName; } private: @@ -122,13 +122,13 @@ class ActionsServer : public AttributeAccessInterface, public CommandHandlerInte /** * @brief - * Called when the state of action is changed. + * Called when the state of an action is changed. */ void OnStateChanged(EndpointId aEndpoint, uint16_t aActionId, uint32_t aInvokeId, ActionStateEnum aActionState); /** * @brief - * Called when the action is failed.. + * Called when an action fails. */ void OnActionFailed(EndpointId aEndpoint, uint16_t aActionId, uint32_t aInvokeId, ActionStateEnum aActionState, ActionErrorEnum aActionError); @@ -178,7 +178,7 @@ class ActionsServer : public AttributeAccessInterface, public CommandHandlerInte MatterReportingAttributeChangeCallback(aEndpointId, Id, aAttributeId); } // Cannot use CommandHandlerInterface::HandleCommand directly because we need to do the HaveActionWithId() check before - // sending a command. + // handling a command. template void HandleCommand(HandlerContext & handlerContext, FuncT func); @@ -211,13 +211,14 @@ class Delegate * @param index The index of the action to be returned. It is assumed that actions are indexable from 0 and with no gaps. * @param action A reference to the ActionStructStorage which should be initialized via copy/assignments or calling Set(). * @return Returns a CHIP_NO_ERROR if there was no error and the action was returned successfully. - * CHIP_ERROR_PROVIDER_LIST_EXHAUSTED if the index is past the end of the list of actionList. + * CHIP_ERROR_PROVIDER_LIST_EXHAUSTED if the index is past the end of the list of actions. */ virtual CHIP_ERROR ReadActionAtIndex(uint16_t index, ActionStructStorage & action) = 0; /** * Get the EndpointList at the Nth index from list of endpointList. - * @param index The index of the endpointList to be returned. It is assumed that actions are indexable from 0 and with no gaps. + * @param index The index of the endpointList to be returned. It is assumed that endpoint lists are indexable from 0 and with no + * gaps. * @param action A reference to the EndpointListStorage which should be initialized via copy/assignments or calling Set(). * @return Returns a CHIP_NO_ERROR if there was no error and the epList was returned successfully. * CHIP_ERROR_PROVIDER_LIST_EXHAUSTED if the index is past the end of the list of endpointLists. @@ -233,132 +234,100 @@ class Delegate /** * On receipt of each and every command, - * if the InvokeID data field is provided by the client when invoking a command, the server SHALL generate a StateChanged event - * when the action changes to a new state or an ActionFailed event when execution of the action fails. - * - * If the command refers to an action which currently is not in a state where the command applies, a response SHALL be - * generated with the Status::InvalidCommand. + * the server shall generate a either StateChanged or ActionFailed event. + * If the command is not supproted by the action, Status::InvalidCommand shall be reported. */ /** - * When an InstantAction command is received, an action (state change) on the involved endpoints shall trigger, - * in a "fire and forget" manner. Afterwards, the action’s state SHALL be Inactive. - * + * @brief Delegate should implement a handler to InstantAction. * @param actionId The actionId of an action. - * It should report Status::Success if successful and may report other Status codes if it fails + * It should report Status::Success if successful and may report other Status codes if it fails. */ virtual Protocols::InteractionModel::Status HandleInstantAction(uint16_t actionId, Optional invokeId) = 0; /** - * When an InstantActionWithTransition command is received, an action (state change) on the involved endpoints shall trigger, - * with a specified time to transition from the current state to the new state. During the transition, the action’s state SHALL - * be Active. Afterwards, the action’s state SHALL be Inactive. - * + * @brief Delegate should implement a handler to InstantActionWithTransition. * @param actionId The actionId of an action. * @param transitionTime The time for transition from the current state to the new state. - * It should report Status::Success if successful and may report other Status codes if it fails + * It should report Status::Success if successful and may report other Status codes if it fails. */ virtual Protocols::InteractionModel::Status HandleInstantActionWithTransition(uint16_t actionId, uint16_t transitionTime, Optional invokeId) = 0; /** - * When a StartAction command is received, the commencement of an action on the involved endpoints shall trigger. Afterwards, - * the action’s state SHALL be Inactive. - * + * @brief Delegate should implement a handler to StartAction. * @param actionId The actionId of an action. - * It should report Status::Success if successful and may report other Status codes if it fails + * It should report Status::Success if successful and may report other Status codes if it fails. */ virtual Protocols::InteractionModel::Status HandleStartAction(uint16_t actionId, Optional invokeId) = 0; /** - * When a StartActionWithDuration command is received, the commencement of an action on the involved endpoints shall trigger, - * and SHALL change the action’s state to Active. After the specified Duration, the action will stop, and the action’s state - * SHALL change to Inactive. - * + * @brief Delegate should implement a handler to StartActionWithDuration. * @param actionId The actionId of an action. * @param duration The time for which an action shall be in start state. - * It should report Status::Success if successful and may report other Status codes if it fails + * It should report Status::Success if successful and may report other Status codes if it fails. */ virtual Protocols::InteractionModel::Status HandleStartActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) = 0; /** - * When a StopAction command is received, the ongoing action on the involved endpoints shall stop. Afterwards, the action’s - * state SHALL be Inactive. - * + * @brief Delegate should implement a handler to StopAction. * @param actionId The actionId of an action. - * It should report Status::Success if successful and may report other Status codes if it fails + * It should report Status::Success if successful and may report other Status codes if it fails. */ virtual Protocols::InteractionModel::Status HandleStopAction(uint16_t actionId, Optional invokeId) = 0; /** - * When a PauseAction command is received, the ongoing action on the involved endpoints shall pause and SHALL change the - * action’s state to Paused. - * + * @brief Delegate should implement a handler to PauseAction. * @param actionId The actionId of an action. - * It should report Status::Success if successful and may report other Status codes if it fails + * It should report Status::Success if successful and may report other Status codes if it fails. */ virtual Protocols::InteractionModel::Status HandlePauseAction(uint16_t actionId, Optional invokeId) = 0; /** - * When a PauseActionWithDuration command is received, pauses an ongoing action, and SHALL change the action’s state to Paused. - * After the specified Duration, the ongoing action will be automatically resumed. which SHALL change the action’s state to - * Active. - * + * @brief Delegate should implement a handler to PauseActionWithDuration. * @param actionId The actionId of an action. * @param duration The time for which an action shall be in pause state. - * It should report Status::Success if successful and may report other Status codes if it fails + * It should report Status::Success if successful and may report other Status codes if it fails. */ virtual Protocols::InteractionModel::Status HandlePauseActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) = 0; /** - * When a ResumeAction command is received, the previously paused action shall resume and SHALL change the action’s state to - * Active. - * + * @brief Delegate should implement a handler to ResumeAction. * @param actionId The actionId of an action. - * It should report Status::Success if successful and may report other Status codes if it fails + * It should report Status::Success if successful and may report other Status codes if it fails. */ virtual Protocols::InteractionModel::Status HandleResumeAction(uint16_t actionId, Optional invokeId) = 0; /** - * When an EnableAction command is received, it enables a certain action or automation. Afterwards, the action’s state SHALL be - * Active. - * + * @brief Delegate should implement a handler to EnableAction. * @param actionId The actionId of an action. - * It should report Status::Success if successful and may report other Status codes if it fails + * It should report Status::Success if successful and may report other Status codes if it fails. */ virtual Protocols::InteractionModel::Status HandleEnableAction(uint16_t actionId, Optional invokeId) = 0; /** - * When an EnableActionWithDuration command is received, it enables a certain action or automation, and SHALL change the - * action’s state to be Active. After the specified Duration, the action or automation will stop, and the action’s state SHALL - * change to Disabled. - * + * @brief Delegate should implement a handler to EnableActionWithDuration. * @param actionId The actionId of an action. * @param duration The time for which an action shall be in active state. - * It should report Status::Success if successful and may report other Status codes if it fails + * It should report Status::Success if successful and may report other Status codes if it fails. */ virtual Protocols::InteractionModel::Status HandleEnableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) = 0; /** - * When a DisableAction command is received, it disables a certain action or automation, and SHALL change the action’s state to - * Inactive. - * + * @brief Delegate should implement a handler to DisableAction. * @param actionId The actionId of an action. - * It should report Status::Success if successful and may report other Status codes if it fails + * It should report Status::Success if successful and may report other Status codes if it fails. */ virtual Protocols::InteractionModel::Status HandleDisableAction(uint16_t actionId, Optional invokeId) = 0; /** - * When a DisableActionWithDuration command is received, it disables a certain action or automation, and SHALL change the - * action’s state to Disabled. After the specified Duration, the action or automation will re-start, and the action’s state - * SHALL change to either Inactive or Active, depending on the actions. - * + * @brief Delegate should implement a handler to DisableActionWithDuration. * @param actionId The actionId of an action. * @param duration The time for which an action shall be in disable state. - * It should report Status::Success if successful and may report other Status codes if it fails + * It should report Status::Success if successful and may report other Status codes if it fails. */ virtual Protocols::InteractionModel::Status HandleDisableActionWithDuration(uint16_t actionId, uint32_t duration, Optional invokeId) = 0; From cc22f3eeb512b699d55cc6adf50042f571886817 Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Wed, 19 Feb 2025 12:33:15 +0530 Subject: [PATCH 24/25] Update API docs, command handling to return InvalidCommand, update Modify* to a notification as suggested in the code review. --- .../include/bridged-actions-stub.h | 2 +- .../src/bridged-actions-stub.cpp | 7 +- .../actions-server/actions-server.cpp | 75 ++++++------------- .../clusters/actions-server/actions-server.h | 56 ++++++-------- src/app/tests/TestActionsCluster.cpp | 3 +- 5 files changed, 55 insertions(+), 88 deletions(-) diff --git a/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h b/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h index 07f80aa9cbef64..eb43404e6720ee 100644 --- a/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h +++ b/examples/all-clusters-app/all-clusters-common/include/bridged-actions-stub.h @@ -54,7 +54,7 @@ class ActionsDelegateImpl : public Delegate CHIP_ERROR ReadActionAtIndex(uint16_t index, ActionStructStorage & action) override; CHIP_ERROR ReadEndpointListAtIndex(uint16_t index, EndpointListStorage & epList) override; - bool HaveActionWithId(uint16_t actionId) override; + bool HaveActionWithId(uint16_t actionId, uint16_t & actionIndex) override; Protocols::InteractionModel::Status HandleInstantAction(uint16_t actionId, Optional invokeId) override; Protocols::InteractionModel::Status HandleInstantActionWithTransition(uint16_t actionId, uint16_t transitionTime, diff --git a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp index 321e32caf8d709..e84138595ef099 100644 --- a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp +++ b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp @@ -44,12 +44,15 @@ CHIP_ERROR ActionsDelegateImpl::ReadEndpointListAtIndex(uint16_t index, Endpoint return CHIP_NO_ERROR; } -bool ActionsDelegateImpl::HaveActionWithId(uint16_t actionId) +bool ActionsDelegateImpl::HaveActionWithId(uint16_t actionId, uint16_t & actionIndex) { - for (size_t i = 0; i < kEndpointList.size(); i++) + for (uint16_t i = 0; i < kEndpointList.size(); i++) { if (kActionList[i].actionID == actionId) + { + actionIndex = i; return true; + } } return false; } diff --git a/src/app/clusters/actions-server/actions-server.cpp b/src/app/clusters/actions-server/actions-server.cpp index 118a051cf0f8d0..02b24a3bbec18d 100644 --- a/src/app/clusters/actions-server/actions-server.cpp +++ b/src/app/clusters/actions-server/actions-server.cpp @@ -165,11 +165,11 @@ CHIP_ERROR ActionsServer::ReadEndpointListAttribute(const ConcreteReadAttributeP return CHIP_NO_ERROR; } -bool ActionsServer::HaveActionWithId(EndpointId aEndpointId, uint16_t aActionId) +bool ActionsServer::HaveActionWithId(EndpointId aEndpointId, uint16_t aActionId, uint16_t & aActionIndex) { Delegate * delegate = GetDelegate(aEndpointId); VerifyOrReturnValue(ValidateDelegate(delegate, aEndpointId) == CHIP_NO_ERROR, false); - return delegate->HaveActionWithId(aActionId); + return delegate->HaveActionWithId(aActionId, aActionIndex); } template @@ -193,11 +193,26 @@ void ActionsServer::HandleCommand(HandlerContext & handlerContext, FuncT func) return; } - if (!HaveActionWithId(handlerContext.mRequestPath.mEndpointId, requestPayload.actionID)) + uint16_t actionIndex = kMaxActionListLength; + if (!HaveActionWithId(handlerContext.mRequestPath.mEndpointId, requestPayload.actionID, actionIndex)) { handlerContext.mCommandHandler.AddStatus(handlerContext.mRequestPath, Protocols::InteractionModel::Status::NotFound); return; } + if (actionIndex != kMaxActionListLength) + { + Delegate * delegate = GetDelegate(handlerContext.mRequestPath.mEndpointId); + ReturnOnFailure(ValidateDelegate(delegate, handlerContext.mRequestPath.mEndpointId)); + ActionStructStorage action; + delegate->ReadActionAtIndex(actionIndex, action); + // Check if the command bit is set in the SupportedCommands of an ations. + if (!(action.supportedCommands.Raw() & (1 << handlerContext.mRequestPath.mCommandId))) + { + handlerContext.mCommandHandler.AddStatus(handlerContext.mRequestPath, + Protocols::InteractionModel::Status::InvalidCommand); + return; + } + } func(handlerContext, requestPayload); } @@ -401,58 +416,16 @@ void ActionsServer::InvokeCommand(HandlerContext & handlerContext) } } -CHIP_ERROR ActionsServer::ModifyActionList(EndpointId aEndpoint, const ActionStructStorage & aAction) +void ActionsServer::ActionListModified(EndpointId aEndpoint) { - Delegate * delegate = GetDelegate(aEndpoint); - ReturnErrorOnFailure(ValidateDelegate(delegate, aEndpoint)); - - // Read through the list to find and update the existing action that matches the passed-in action's ID. - for (uint16_t i = 0; i < kMaxActionListLength; i++) - { - ActionStructStorage existingAction; - CHIP_ERROR err = delegate->ReadActionAtIndex(i, existingAction); - if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED) - { - break; - } - - if (existingAction.actionID == aAction.actionID) - { - existingAction.Set(aAction.actionID, aAction.name, aAction.type, aAction.endpointListID, aAction.supportedCommands, - aAction.state); - - MarkDirty(aEndpoint, Attributes::ActionList::Id); - return CHIP_NO_ERROR; - } - } - - return CHIP_ERROR_NOT_FOUND; + MarkDirty(aEndpoint, Attributes::ActionList::Id); + return; } -CHIP_ERROR ActionsServer::ModifyEndpointList(EndpointId aEndpoint, const EndpointListStorage & aEpList) +void ActionsServer::EndpointListModified(EndpointId aEndpoint) { - Delegate * delegate = GetDelegate(aEndpoint); - ReturnErrorOnFailure(ValidateDelegate(delegate, aEndpoint)); - - // Read through the list to find and update the existing action that matches the passed-in endpoint-list's ID - for (uint16_t i = 0; i < kMaxEndpointListLength; i++) - { - EndpointListStorage existingEpList; - CHIP_ERROR err = delegate->ReadEndpointListAtIndex(i, existingEpList); - if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED) - { - break; - } - - if (existingEpList.endpointListID == aEpList.endpointListID) - { - existingEpList.Set(aEpList.endpointListID, aEpList.name, aEpList.type, aEpList.endpoints); - MarkDirty(aEndpoint, Attributes::EndpointLists::Id); - return CHIP_NO_ERROR; - } - } - - return CHIP_ERROR_NOT_FOUND; + MarkDirty(aEndpoint, Attributes::EndpointLists::Id); + return; } void MatterActionsPluginServerInitCallback() diff --git a/src/app/clusters/actions-server/actions-server.h b/src/app/clusters/actions-server/actions-server.h index e73bc52c508566..76e514d5cd7a95 100644 --- a/src/app/clusters/actions-server/actions-server.h +++ b/src/app/clusters/actions-server/actions-server.h @@ -138,28 +138,18 @@ class ActionsServer : public AttributeAccessInterface, public CommandHandlerInte CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; /** - * Update an existing action in the action list for the given endpoint. - * If the action with the given actionID doesn't exist, returns CHIP_ERROR_NOT_FOUND. + * A notification from an application to the sever that an ActionList is modified.. * * @param aEndpoint The endpoint ID where the action should be updated - * @param aAction The action structure containing the updated action details - * @return CHIP_ERROR_INCORRECT_STATE if delegate is null - * CHIP_ERROR_NOT_FOUND if action doesn't exist - * CHIP_NO_ERROR if successful */ - CHIP_ERROR ModifyActionList(EndpointId aEndpoint, const ActionStructStorage & aAction); + void ActionListModified(EndpointId aEndpoint); /** - * Update an existing endpoint list for the given endpoint. - * If the endpoint list with the given ID doesn't exist, returns CHIP_ERROR_NOT_FOUND. + * A notification from an application to the sever that an EndpointList is modified.. * - * @param aEndpoint The endpoint ID where the endpoint list should be updated - * @param aEpList The endpoint list structure containing the updated list details - * @return CHIP_ERROR_INCORRECT_STATE if delegate is null - * CHIP_ERROR_NOT_FOUND if endpoint list doesn't exist - * CHIP_NO_ERROR if successful + * @param aEndpoint The endpoint ID where the action should be updated */ - CHIP_ERROR ModifyEndpointList(EndpointId aEndpoint, const EndpointListStorage & aEpList); + void EndpointListModified(EndpointId aEndpoint); private: static ActionsServer sInstance; @@ -170,7 +160,7 @@ class ActionsServer : public AttributeAccessInterface, public CommandHandlerInte const AttributeValueEncoder::ListEncodeHelper & aEncoder); CHIP_ERROR ReadEndpointListAttribute(const ConcreteReadAttributePath & aPath, const AttributeValueEncoder::ListEncodeHelper & aEncoder); - bool HaveActionWithId(EndpointId aEndpointId, uint16_t aActionId); + bool HaveActionWithId(EndpointId aEndpointId, uint16_t aActionId, uint16_t & aActionIndex); // TODO: We should move to non-global dirty marker. void MarkDirty(EndpointId aEndpointId, AttributeId aAttributeId) @@ -227,26 +217,26 @@ class Delegate /** * Check whether there is an action with the given actionId in the list of actions. - * @param actionId The action ID to search for. + * @param aActionId The action ID to search for. + * @param aActionIndex A reference to the index at which an action with matching aActionId. * @return Returns a true if matching action is found otherwise false. */ - virtual bool HaveActionWithId(uint16_t actionId) = 0; + virtual bool HaveActionWithId(uint16_t aActionId, uint16_t & aActionIndex) = 0; /** - * On receipt of each and every command, - * the server shall generate a either StateChanged or ActionFailed event. - * If the command is not supproted by the action, Status::InvalidCommand shall be reported. + * The implementations of the Handle* command callbacks below are expected to call OnStateChanged or + * OnActionFailed as needed to generate the events required by the spec. */ /** - * @brief Delegate should implement a handler to InstantAction. + * @brief Callback that will be called to handle an InstantAction command. * @param actionId The actionId of an action. * It should report Status::Success if successful and may report other Status codes if it fails. */ virtual Protocols::InteractionModel::Status HandleInstantAction(uint16_t actionId, Optional invokeId) = 0; /** - * @brief Delegate should implement a handler to InstantActionWithTransition. + * @brief Callback that will be called to handle an InstantActionWithTransition command. * @param actionId The actionId of an action. * @param transitionTime The time for transition from the current state to the new state. * It should report Status::Success if successful and may report other Status codes if it fails. @@ -255,14 +245,14 @@ class Delegate Optional invokeId) = 0; /** - * @brief Delegate should implement a handler to StartAction. + * @brief Callback that will be called to handle a StartAction command. * @param actionId The actionId of an action. * It should report Status::Success if successful and may report other Status codes if it fails. */ virtual Protocols::InteractionModel::Status HandleStartAction(uint16_t actionId, Optional invokeId) = 0; /** - * @brief Delegate should implement a handler to StartActionWithDuration. + * @brief Callback that will be called to handle a StartActionWithDuration command. * @param actionId The actionId of an action. * @param duration The time for which an action shall be in start state. * It should report Status::Success if successful and may report other Status codes if it fails. @@ -271,21 +261,21 @@ class Delegate Optional invokeId) = 0; /** - * @brief Delegate should implement a handler to StopAction. + * @brief Callback that will be called to handle a StopAction command. * @param actionId The actionId of an action. * It should report Status::Success if successful and may report other Status codes if it fails. */ virtual Protocols::InteractionModel::Status HandleStopAction(uint16_t actionId, Optional invokeId) = 0; /** - * @brief Delegate should implement a handler to PauseAction. + * @brief Callback that will be called to handle a PauseAction command. * @param actionId The actionId of an action. * It should report Status::Success if successful and may report other Status codes if it fails. */ virtual Protocols::InteractionModel::Status HandlePauseAction(uint16_t actionId, Optional invokeId) = 0; /** - * @brief Delegate should implement a handler to PauseActionWithDuration. + * @brief Callback that will be called to handle a PauseActionWithDuration command. * @param actionId The actionId of an action. * @param duration The time for which an action shall be in pause state. * It should report Status::Success if successful and may report other Status codes if it fails. @@ -294,21 +284,21 @@ class Delegate Optional invokeId) = 0; /** - * @brief Delegate should implement a handler to ResumeAction. + * @brief Callback that will be called to handle a ResumeAction command. * @param actionId The actionId of an action. * It should report Status::Success if successful and may report other Status codes if it fails. */ virtual Protocols::InteractionModel::Status HandleResumeAction(uint16_t actionId, Optional invokeId) = 0; /** - * @brief Delegate should implement a handler to EnableAction. + * @brief Callback that will be called to handle an EnableAction command. * @param actionId The actionId of an action. * It should report Status::Success if successful and may report other Status codes if it fails. */ virtual Protocols::InteractionModel::Status HandleEnableAction(uint16_t actionId, Optional invokeId) = 0; /** - * @brief Delegate should implement a handler to EnableActionWithDuration. + * @brief Callback that will be called to handle an EnableActionWithDuration command. * @param actionId The actionId of an action. * @param duration The time for which an action shall be in active state. * It should report Status::Success if successful and may report other Status codes if it fails. @@ -317,14 +307,14 @@ class Delegate Optional invokeId) = 0; /** - * @brief Delegate should implement a handler to DisableAction. + * @brief Callback that will be called to handle a DisableAction command. * @param actionId The actionId of an action. * It should report Status::Success if successful and may report other Status codes if it fails. */ virtual Protocols::InteractionModel::Status HandleDisableAction(uint16_t actionId, Optional invokeId) = 0; /** - * @brief Delegate should implement a handler to DisableActionWithDuration. + * @brief Callback that will be called to handle a DisableActionWithDuration command. * @param actionId The actionId of an action. * @param duration The time for which an action shall be in disable state. * It should report Status::Success if successful and may report other Status codes if it fails. diff --git a/src/app/tests/TestActionsCluster.cpp b/src/app/tests/TestActionsCluster.cpp index f63b2759f54d0a..f7034a075dc2b0 100644 --- a/src/app/tests/TestActionsCluster.cpp +++ b/src/app/tests/TestActionsCluster.cpp @@ -87,12 +87,13 @@ class TestActionsDelegateImpl : public Clusters::Actions::Delegate return CHIP_NO_ERROR; } - bool HaveActionWithId(uint16_t actionId) override + bool HaveActionWithId(uint16_t actionId, uint16_t & aActionIndex) override { for (uint16_t i = 0; i < mNumActions; i++) { if (mActions[i].actionID == actionId) { + aActionIndex = i; return true; } } From 9ec4acff6b88b5bfdd5f26d0843c0789c345a364 Mon Sep 17 00:00:00 2001 From: Rohit Jadhav Date: Wed, 19 Feb 2025 14:14:15 +0530 Subject: [PATCH 25/25] Fix clang-tidy validation --- .../all-clusters-common/src/bridged-actions-stub.cpp | 4 ++-- src/app/clusters/actions-server/actions-server.cpp | 2 -- src/app/clusters/actions-server/actions-server.h | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp index e84138595ef099..b44193d996b788 100644 --- a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp +++ b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp @@ -46,11 +46,11 @@ CHIP_ERROR ActionsDelegateImpl::ReadEndpointListAtIndex(uint16_t index, Endpoint bool ActionsDelegateImpl::HaveActionWithId(uint16_t actionId, uint16_t & actionIndex) { - for (uint16_t i = 0; i < kEndpointList.size(); i++) + for (size_t i = 0; i < kEndpointList.size(); i++) { if (kActionList[i].actionID == actionId) { - actionIndex = i; + actionIndex = (uint16_t) i; return true; } } diff --git a/src/app/clusters/actions-server/actions-server.cpp b/src/app/clusters/actions-server/actions-server.cpp index 02b24a3bbec18d..e02f094d85105f 100644 --- a/src/app/clusters/actions-server/actions-server.cpp +++ b/src/app/clusters/actions-server/actions-server.cpp @@ -419,13 +419,11 @@ void ActionsServer::InvokeCommand(HandlerContext & handlerContext) void ActionsServer::ActionListModified(EndpointId aEndpoint) { MarkDirty(aEndpoint, Attributes::ActionList::Id); - return; } void ActionsServer::EndpointListModified(EndpointId aEndpoint) { MarkDirty(aEndpoint, Attributes::EndpointLists::Id); - return; } void MatterActionsPluginServerInitCallback() diff --git a/src/app/clusters/actions-server/actions-server.h b/src/app/clusters/actions-server/actions-server.h index 76e514d5cd7a95..2cacbe868739b6 100644 --- a/src/app/clusters/actions-server/actions-server.h +++ b/src/app/clusters/actions-server/actions-server.h @@ -93,7 +93,7 @@ struct EndpointListStorage : public Structs::EndpointListStruct::Type { endpointListID = aEpListId; type = aEpListType; - size_t epListSize = std::min(aEndpointList.size(), ArraySize(mEpList)); + size_t epListSize = std::min(aEndpointList.size(), MATTER_ARRAY_SIZE(mEpList)); for (size_t index = 0; index < epListSize; index++) {