From 8128724f29a7f948b23b9cd30586b7abefe9ef26 Mon Sep 17 00:00:00 2001 From: Matt Hazley Date: Wed, 19 Jul 2023 14:24:49 +0100 Subject: [PATCH] [`FanControl`] Adding Delegate to allow application to handle `Step` command (#28000) * Added support for a basic FanControl::Delegate class that the application can implement to handle the Step Command. Added Set and Get functions and a class definition with a virtual function that can be implemented. * Extended the fan-stub class to also inherit the Delegate and wrote an example handler for the Step function * Turned on the available features for the ci tests * Restyled by whitespace * Included optional header so can use std::optional * Removing use of optional as it is not building on certain platforms in CI * Simplified the HandleStep callback and also added support for when Speed is Null * Initialising uninitialised value in HandleStep --------- Co-authored-by: Restyled.io --- .../all-clusters-common/src/fan-stub.cpp | 114 ++++++++++++++---- .../fan-control-server/fan-control-delegate.h | 61 ++++++++++ .../fan-control-server/fan-control-server.cpp | 70 +++++++++-- .../fan-control-server/fan-control-server.h | 36 ++++++ .../tests/suites/certification/ci-pics-values | 8 +- 5 files changed, 254 insertions(+), 35 deletions(-) create mode 100644 src/app/clusters/fan-control-server/fan-control-delegate.h create mode 100644 src/app/clusters/fan-control-server/fan-control-server.h diff --git a/examples/all-clusters-app/all-clusters-common/src/fan-stub.cpp b/examples/all-clusters-app/all-clusters-common/src/fan-stub.cpp index d28904010f0cca..1c343f139e71cf 100644 --- a/examples/all-clusters-app/all-clusters-common/src/fan-stub.cpp +++ b/examples/all-clusters-app/all-clusters-common/src/fan-stub.cpp @@ -20,45 +20,43 @@ #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::FanControl; using namespace chip::app::Clusters::FanControl::Attributes; +using Protocols::InteractionModel::Status; namespace { - -/* - * TODO: This is a stop-gap solution to allow the existing fan control cluster tests to run after changes to - * the cluster objects for TE1. This should be removed once #6496 is resolved as it will likely result in a - * FanControl delegate added to the SDK. - * - * FYI... The previous implementation of the FanControl cluster set the speedCurrent/percentCurrent when it received - * speedSetting/percentSetting. The new implementation of the FanControl cluster does not do this as this should - * really be done by the application. - */ - -class FanAttrAccess : public AttributeAccessInterface +class FanControlManager : public AttributeAccessInterface, public Delegate { public: // Register for the FanControl cluster on all endpoints. - FanAttrAccess() : AttributeAccessInterface(Optional::Missing(), FanControl::Id) {} + FanControlManager(EndpointId aEndpointId) : + AttributeAccessInterface(Optional(aEndpointId), FanControl::Id), Delegate(aEndpointId) + {} CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; + Status HandleStep(StepDirectionEnum direction, bool wrap, bool off) override; private: - CHIP_ERROR ReadPercentCurrent(EndpointId endpoint, AttributeValueEncoder & aEncoder); - CHIP_ERROR ReadSpeedCurrent(EndpointId endpoint, AttributeValueEncoder & aEncoder); + CHIP_ERROR ReadPercentCurrent(AttributeValueEncoder & aEncoder); + CHIP_ERROR ReadSpeedCurrent(AttributeValueEncoder & aEncoder); }; -CHIP_ERROR FanAttrAccess::ReadPercentCurrent(EndpointId endpoint, AttributeValueEncoder & aEncoder) +static FanControlManager * mFanControlManager; + +CHIP_ERROR FanControlManager::ReadPercentCurrent(AttributeValueEncoder & aEncoder) { // Return PercentSetting attribute value for now DataModel::Nullable percentSetting; - PercentSetting::Get(endpoint, percentSetting); + PercentSetting::Get(mEndpoint, percentSetting); Percent ret = 0; if (!percentSetting.IsNull()) { @@ -68,11 +66,11 @@ CHIP_ERROR FanAttrAccess::ReadPercentCurrent(EndpointId endpoint, AttributeValue return aEncoder.Encode(ret); } -CHIP_ERROR FanAttrAccess::ReadSpeedCurrent(EndpointId endpoint, AttributeValueEncoder & aEncoder) +CHIP_ERROR FanControlManager::ReadSpeedCurrent(AttributeValueEncoder & aEncoder) { // Return SpeedCurrent attribute value for now DataModel::Nullable speedSetting; - SpeedSetting::Get(endpoint, speedSetting); + SpeedSetting::Get(mEndpoint, speedSetting); uint8_t ret = 0; if (!speedSetting.IsNull()) { @@ -82,26 +80,94 @@ CHIP_ERROR FanAttrAccess::ReadSpeedCurrent(EndpointId endpoint, AttributeValueEn return aEncoder.Encode(ret); } -FanAttrAccess gAttrAccess; +Status FanControlManager::HandleStep(StepDirectionEnum direction, bool wrap, bool lowestOff) +{ + ChipLogProgress(NotSpecified, "FanControlManager::HandleStep direction %d, wrap %d, lowestOff %d", to_underlying(direction), + wrap, lowestOff); + + VerifyOrReturnError(direction != StepDirectionEnum::kUnknownEnumValue, Status::InvalidCommand); + + uint8_t speedMax; + SpeedMax::Get(mEndpoint, &speedMax); + + DataModel::Nullable speedSetting; + SpeedSetting::Get(mEndpoint, speedSetting); + + uint8_t newSpeedSetting = speedSetting.Value(); + + if (direction == StepDirectionEnum::kIncrease) + { + if (speedSetting.IsNull()) + { + newSpeedSetting = 1; + } + else if (speedSetting.Value() < speedMax) + { + newSpeedSetting = static_cast(speedSetting.Value() + 1); + } + else if (speedSetting.Value() == speedMax) + { + if (wrap) + { + newSpeedSetting = lowestOff ? 0 : 1; + } + } + } + else if (direction == StepDirectionEnum::kDecrease) + { + if (speedSetting.IsNull()) + { + newSpeedSetting = lowestOff ? 0 : 1; + } + else if ((speedSetting.Value() > 1) && (speedSetting.Value() <= speedMax)) + { + newSpeedSetting = static_cast(speedSetting.Value() - 1); + } + else if (speedSetting.Value() == 1) + { + if (lowestOff) + { + newSpeedSetting = static_cast(speedSetting.Value() - 1); + } + } + else if (speedSetting.Value() == 0) + { + if (wrap) + { + newSpeedSetting = speedMax; + } + else if (!lowestOff) + { + newSpeedSetting = 1; + } + } + } -CHIP_ERROR FanAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) + return ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); +} + +CHIP_ERROR FanControlManager::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) { VerifyOrDie(aPath.mClusterId == FanControl::Id); + VerifyOrDie(aPath.mEndpointId == mEndpoint); switch (aPath.mAttributeId) { case SpeedCurrent::Id: - return ReadSpeedCurrent(aPath.mEndpointId, aEncoder); + return ReadSpeedCurrent(aEncoder); case PercentCurrent::Id: - return ReadPercentCurrent(aPath.mEndpointId, aEncoder); + return ReadPercentCurrent(aEncoder); default: break; } return CHIP_NO_ERROR; } + } // anonymous namespace void emberAfFanControlClusterInitCallback(EndpointId endpoint) { - registerAttributeAccessOverride(&gAttrAccess); + mFanControlManager = new FanControlManager(endpoint); + registerAttributeAccessOverride(mFanControlManager); + FanControl::SetDefaultDelegate(endpoint, mFanControlManager); } diff --git a/src/app/clusters/fan-control-server/fan-control-delegate.h b/src/app/clusters/fan-control-server/fan-control-delegate.h new file mode 100644 index 00000000000000..65a67d2f4915bc --- /dev/null +++ b/src/app/clusters/fan-control-server/fan-control-delegate.h @@ -0,0 +1,61 @@ +/* + * + * 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. + */ + +#pragma once + +#include +#include +#include +#include + +namespace chip { +namespace app { +namespace Clusters { +namespace FanControl { + +/** @brief + * Defines methods for implementing application-specific logic for the FanControl Cluster. + */ +class Delegate +{ +public: + /** + * @brief + * This method handles the step command. This will happen as fast as possible. + * + * @param[in] direction direction in which to step + * @param[in] wrap whether the step command wraps or not + * @param[in] wrap whether the step command wraps or not + * + * @return Success On success. + * @return Other Value indicating it failed to execute the command. + */ + virtual Protocols::InteractionModel::Status HandleStep(StepDirectionEnum direction, bool wrap, bool off) = 0; + + Delegate(EndpointId endpoint) : mEndpoint(endpoint) {} + + virtual ~Delegate() = default; + +protected: + EndpointId mEndpoint = 0; +}; + +} // namespace FanControl +} // namespace Clusters +} // namespace app +} // namespace chip diff --git a/src/app/clusters/fan-control-server/fan-control-server.cpp b/src/app/clusters/fan-control-server/fan-control-server.cpp index eed33820e0a947..ca50fb06662283 100644 --- a/src/app/clusters/fan-control-server/fan-control-server.cpp +++ b/src/app/clusters/fan-control-server/fan-control-server.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -43,6 +44,45 @@ using namespace chip::app::Clusters::FanControl::Attributes; namespace { +constexpr size_t kFanControlDelegateTableSize = + EMBER_AF_FAN_CONTROL_CLUSTER_SERVER_ENDPOINT_COUNT + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT; + +static_assert(kFanControlDelegateTableSize <= kEmberInvalidEndpointIndex, "FanControl Delegate table size error"); + +Delegate * gDelegateTable[kFanControlDelegateTableSize] = { nullptr }; + +} // anonymous namespace + +namespace chip { +namespace app { +namespace Clusters { +namespace FanControl { + +Delegate * GetDelegate(EndpointId endpoint) +{ + uint16_t ep = + emberAfGetClusterServerEndpointIndex(endpoint, FanControl::Id, EMBER_AF_FAN_CONTROL_CLUSTER_SERVER_ENDPOINT_COUNT); + return (ep >= kFanControlDelegateTableSize ? nullptr : gDelegateTable[ep]); +} + +void SetDefaultDelegate(EndpointId endpoint, Delegate * delegate) +{ + uint16_t ep = + emberAfGetClusterServerEndpointIndex(endpoint, FanControl::Id, EMBER_AF_APPLICATION_BASIC_CLUSTER_SERVER_ENDPOINT_COUNT); + // if endpoint is found + if (ep < kFanControlDelegateTableSize) + { + gDelegateTable[ep] = delegate; + } +} + +} // namespace FanControl +} // namespace Clusters +} // namespace app +} // namespace chip + +namespace { + // Indicates if the write operation is from the cluster server itself bool gWriteFromClusterLogic = false; @@ -403,21 +443,37 @@ void MatterFanControlClusterServerAttributeChangedCallback(const app::ConcreteAt } } -bool emberAfFanControlClusterStepCallback(CommandHandler * commandObj, const ConcreteCommandPath & commandPath, +bool emberAfFanControlClusterStepCallback(app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath, const Commands::Step::DecodableType & commandData) { - /* - * TODO: Clarification needed in spec issue #6496 - if this is tied to the SpeedSetting attribute, then - * the attribute can be updated here, if it is supposed to be implementation specific, then the command - * will have to be handed off to an application specific callback which will require some sort of delegate. - */ - Protocols::InteractionModel::Status status = Status::Success; + ChipLogProgress(Zcl, "FanControl emberAfFanControlClusterStepCallback: Endpoint %u", commandPath.mEndpointId); + if (!SupportsStep(commandPath.mEndpointId)) { + ChipLogProgress(Zcl, "FanControl does not support Step:%u", commandPath.mEndpointId); status = Status::UnsupportedCommand; } + else + { + EndpointId endpoint = commandPath.mEndpointId; + StepDirectionEnum direction = commandData.direction; + + bool wrapValue = commandData.wrap.HasValue() ? commandData.wrap.Value() : false; + bool lowestOffValue = commandData.lowestOff.HasValue() ? commandData.lowestOff.Value() : false; + + Delegate * delegate = GetDelegate(endpoint); + if (delegate) + { + status = delegate->HandleStep(direction, wrapValue, lowestOffValue); + } + else + { + ChipLogProgress(Zcl, "FanControl has no delegate set for endpoint:%u", endpoint); + } + } + commandObj->AddStatus(commandPath, status); return true; } diff --git a/src/app/clusters/fan-control-server/fan-control-server.h b/src/app/clusters/fan-control-server/fan-control-server.h new file mode 100644 index 00000000000000..d06504c3b87d2a --- /dev/null +++ b/src/app/clusters/fan-control-server/fan-control-server.h @@ -0,0 +1,36 @@ +/* + * + * Copyright (c) 2023 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 "fan-control-delegate.h" +#include +#include +#include + +namespace chip { +namespace app { +namespace Clusters { +namespace FanControl { + +void SetDefaultDelegate(EndpointId endpoint, Delegate * delegate); +Delegate * GetDelegate(EndpointId endpoint); + +} // namespace FanControl +} // namespace Clusters +} // namespace app +} // namespace chip diff --git a/src/app/tests/suites/certification/ci-pics-values b/src/app/tests/suites/certification/ci-pics-values index 239ca2dafc6404..9ef76ba01b092a 100644 --- a/src/app/tests/suites/certification/ci-pics-values +++ b/src/app/tests/suites/certification/ci-pics-values @@ -2601,10 +2601,10 @@ RNCONC.S.Afffd=1 #Features FAN.S.F00=1 FAN.S.F01=1 -FAN.S.F02=0 -FAN.S.F03=0 -FAN.S.F04=0 -FAN.S.F05=0 +FAN.S.F02=1 +FAN.S.F03=1 +FAN.S.F04=1 +FAN.S.F05=1 #Server Attributes FAN.S.A0000=1