From deca2b3d06b4e963432a8791063efe22bf1bbe59 Mon Sep 17 00:00:00 2001 From: Matthew Hazley Date: Mon, 17 Jul 2023 17:27:17 +0100 Subject: [PATCH 1/8] 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. --- .../fan-control-server/fan-control-delegate.h | 61 ++++++++++++++++ .../fan-control-server/fan-control-server.cpp | 72 +++++++++++++++++-- .../fan-control-server/fan-control-server.h | 36 ++++++++++ 3 files changed, 162 insertions(+), 7 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/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..7cc7b0860fad54 --- /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 \ No newline at end of file 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..ae2189c9127ea5 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,39 @@ 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:%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; + Optional wrap = commandData.wrap; + Optional lowestOff = commandData.lowestOff; + + bool wrapValue = wrap.HasValue() ? wrap.Value() : false; + bool lowestOffValue = lowestOff.HasValue() ? 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..f92035865754fe --- /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 \ No newline at end of file From 78a1eb05c33477a0ee1b7bcd2f8d55168f912246 Mon Sep 17 00:00:00 2001 From: Matthew Hazley Date: Mon, 17 Jul 2023 17:28:15 +0100 Subject: [PATCH 2/8] Extended the fan-stub class to also inherit the Delegate and wrote an example handler for the Step function --- .../all-clusters-common/src/fan-stub.cpp | 150 +++++++++++++++--- 1 file changed, 126 insertions(+), 24 deletions(-) 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..77b4e5dde28172 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 std::optional 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,130 @@ CHIP_ERROR FanAttrAccess::ReadSpeedCurrent(EndpointId endpoint, AttributeValueEn return aEncoder.Encode(ret); } -FanAttrAccess gAttrAccess; +Status FanControlManager::HandleStep(StepDirectionEnum direction, bool wrap, bool lowestOff) +{ + Status status = Status::Success; + + uint8_t speedMax; + SpeedMax::Get(mEndpoint, &speedMax); + + DataModel::Nullable speedSetting; + SpeedSetting::Get(mEndpoint, speedSetting); + + uint8_t newSpeedSetting; -CHIP_ERROR FanAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) + if ((speedSetting.Value() > 1) && (speedSetting.Value() < speedMax)) + { + if (direction == StepDirectionEnum::kIncrease) + { + newSpeedSetting = static_cast(speedSetting.Value() + 1); + status = ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); + } + else if (direction == StepDirectionEnum::kDecrease) + { + newSpeedSetting = static_cast(speedSetting.Value() - 1); + status = ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); + } + else + { + status = Status::InvalidCommand; + } + } + else if (speedSetting.Value() == 1) + { + if (direction == StepDirectionEnum::kIncrease) + { + newSpeedSetting = static_cast(speedSetting.Value() + 1); + status = ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); + } + else if (direction == StepDirectionEnum::kDecrease) + { + if (lowestOff) + { + newSpeedSetting = static_cast(speedSetting.Value() - 1); + status = ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); + } + else if (wrap) + { + newSpeedSetting = speedMax; + status = ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); + } + } + else + { + status = Status::InvalidCommand; + } + } + else if (speedSetting.Value() == 0) + { + if (direction == StepDirectionEnum::kIncrease) + { + newSpeedSetting = static_cast(speedSetting.Value() + 1); + status = ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); + } + else if (direction == StepDirectionEnum::kDecrease) + { + if (wrap) + { + newSpeedSetting = speedMax; + status = ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); + } + else if (!lowestOff) + { + newSpeedSetting = 1; + status = ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); + } + } + else + { + status = Status::InvalidCommand; + } + } + else if (speedSetting.Value() == speedMax) + { + if (direction == StepDirectionEnum::kIncrease) + { + if (wrap) + { + newSpeedSetting = lowestOff ? 0 : 1; + status = ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); + } + } + else if (direction == StepDirectionEnum::kDecrease) + { + newSpeedSetting = static_cast(speedSetting.Value() - 1); + status = ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); + } + else + { + status = Status::InvalidCommand; + } + } + return status; +} + +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.emplace(endpoint); + registerAttributeAccessOverride(&mFanControlManager.value()); + FanControl::SetDefaultDelegate(endpoint, &mFanControlManager.value()); } From 68147991a1ecc961e30a4326f3e183cebe92e7e9 Mon Sep 17 00:00:00 2001 From: Matthew Hazley Date: Mon, 17 Jul 2023 17:29:07 +0100 Subject: [PATCH 3/8] Turned on the available features for the ci tests --- src/app/tests/suites/certification/ci-pics-values | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app/tests/suites/certification/ci-pics-values b/src/app/tests/suites/certification/ci-pics-values index 0264ad8fe3be8d..570f42d8b3f81d 100644 --- a/src/app/tests/suites/certification/ci-pics-values +++ b/src/app/tests/suites/certification/ci-pics-values @@ -2587,10 +2587,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 From d78fe1b9a1e270d96900ac2fd90c8be4faafbb4b Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Mon, 17 Jul 2023 16:46:03 +0000 Subject: [PATCH 4/8] Restyled by whitespace --- src/app/clusters/fan-control-server/fan-control-delegate.h | 2 +- src/app/clusters/fan-control-server/fan-control-server.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/clusters/fan-control-server/fan-control-delegate.h b/src/app/clusters/fan-control-server/fan-control-delegate.h index 7cc7b0860fad54..65a67d2f4915bc 100644 --- a/src/app/clusters/fan-control-server/fan-control-delegate.h +++ b/src/app/clusters/fan-control-server/fan-control-delegate.h @@ -58,4 +58,4 @@ class Delegate } // namespace FanControl } // namespace Clusters } // namespace app -} // namespace chip \ No newline at end of file +} // namespace chip diff --git a/src/app/clusters/fan-control-server/fan-control-server.h b/src/app/clusters/fan-control-server/fan-control-server.h index f92035865754fe..d06504c3b87d2a 100644 --- a/src/app/clusters/fan-control-server/fan-control-server.h +++ b/src/app/clusters/fan-control-server/fan-control-server.h @@ -33,4 +33,4 @@ Delegate * GetDelegate(EndpointId endpoint); } // namespace FanControl } // namespace Clusters } // namespace app -} // namespace chip \ No newline at end of file +} // namespace chip From 181dbe314142472121f97050287b2fed3c7b44ea Mon Sep 17 00:00:00 2001 From: Matthew Hazley Date: Tue, 18 Jul 2023 09:29:48 +0100 Subject: [PATCH 5/8] Included optional header so can use std::optional --- examples/all-clusters-app/all-clusters-common/src/fan-stub.cpp | 1 + 1 file changed, 1 insertion(+) 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 77b4e5dde28172..651072b42008d8 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 @@ -25,6 +25,7 @@ #include #include #include +#include using namespace chip; using namespace chip::app; From 74557a092dd753303fc49bf524cad374ea326c48 Mon Sep 17 00:00:00 2001 From: Matthew Hazley Date: Tue, 18 Jul 2023 09:48:03 +0100 Subject: [PATCH 6/8] Removing use of optional as it is not building on certain platforms in CI --- .../all-clusters-common/src/fan-stub.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) 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 651072b42008d8..4fc6fd31083859 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 @@ -25,7 +25,6 @@ #include #include #include -#include using namespace chip; using namespace chip::app; @@ -51,7 +50,7 @@ class FanControlManager : public AttributeAccessInterface, public Delegate CHIP_ERROR ReadSpeedCurrent(AttributeValueEncoder & aEncoder); }; -static std::optional mFanControlManager; +static FanControlManager * mFanControlManager; CHIP_ERROR FanControlManager::ReadPercentCurrent(AttributeValueEncoder & aEncoder) { @@ -204,7 +203,7 @@ CHIP_ERROR FanControlManager::Read(const ConcreteReadAttributePath & aPath, Attr void emberAfFanControlClusterInitCallback(EndpointId endpoint) { - mFanControlManager.emplace(endpoint); - registerAttributeAccessOverride(&mFanControlManager.value()); - FanControl::SetDefaultDelegate(endpoint, &mFanControlManager.value()); + mFanControlManager = new FanControlManager(endpoint); + registerAttributeAccessOverride(mFanControlManager); + FanControl::SetDefaultDelegate(endpoint, mFanControlManager); } From e4011d53f6ec78e81b558bf2ae89a4419aa1e725 Mon Sep 17 00:00:00 2001 From: Matthew Hazley Date: Wed, 19 Jul 2023 11:16:03 +0100 Subject: [PATCH 7/8] Simplified the HandleStep callback and also added support for when Speed is Null --- .../all-clusters-common/src/fan-stub.cpp | 86 ++++++------------- .../fan-control-server/fan-control-server.cpp | 8 +- 2 files changed, 28 insertions(+), 66 deletions(-) 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 4fc6fd31083859..5d55c63e8c8281 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 @@ -82,7 +82,10 @@ CHIP_ERROR FanControlManager::ReadSpeedCurrent(AttributeValueEncoder & aEncoder) Status FanControlManager::HandleStep(StepDirectionEnum direction, bool wrap, bool lowestOff) { - Status status = Status::Success; + 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); @@ -92,94 +95,55 @@ Status FanControlManager::HandleStep(StepDirectionEnum direction, bool wrap, boo uint8_t newSpeedSetting; - if ((speedSetting.Value() > 1) && (speedSetting.Value() < speedMax)) + if (direction == StepDirectionEnum::kIncrease) { - if (direction == StepDirectionEnum::kIncrease) + if (speedSetting.IsNull()) { - newSpeedSetting = static_cast(speedSetting.Value() + 1); - status = ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); + newSpeedSetting = 1; } - else if (direction == StepDirectionEnum::kDecrease) + else if (speedSetting.Value() < speedMax) { - newSpeedSetting = static_cast(speedSetting.Value() - 1); - status = ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); + newSpeedSetting = static_cast(speedSetting.Value() + 1); } - else + else if (speedSetting.Value() == speedMax) { - status = Status::InvalidCommand; + if (wrap) + { + newSpeedSetting = lowestOff ? 0 : 1; + } } } - else if (speedSetting.Value() == 1) + else if (direction == StepDirectionEnum::kDecrease) { - if (direction == StepDirectionEnum::kIncrease) + if (speedSetting.IsNull()) { - newSpeedSetting = static_cast(speedSetting.Value() + 1); - status = ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); + newSpeedSetting = lowestOff ? 0 : 1; } - else if (direction == StepDirectionEnum::kDecrease) + 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); - status = ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); - } - else if (wrap) - { - newSpeedSetting = speedMax; - status = ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); } } - else - { - status = Status::InvalidCommand; - } - } - else if (speedSetting.Value() == 0) - { - if (direction == StepDirectionEnum::kIncrease) - { - newSpeedSetting = static_cast(speedSetting.Value() + 1); - status = ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); - } - else if (direction == StepDirectionEnum::kDecrease) + else if (speedSetting.Value() == 0) { if (wrap) { newSpeedSetting = speedMax; - status = ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); } else if (!lowestOff) { newSpeedSetting = 1; - status = ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); } } - else - { - status = Status::InvalidCommand; - } } - else if (speedSetting.Value() == speedMax) - { - if (direction == StepDirectionEnum::kIncrease) - { - if (wrap) - { - newSpeedSetting = lowestOff ? 0 : 1; - status = ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); - } - } - else if (direction == StepDirectionEnum::kDecrease) - { - newSpeedSetting = static_cast(speedSetting.Value() - 1); - status = ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); - } - else - { - status = Status::InvalidCommand; - } - } - return status; + + return ToInteractionModelStatus(SpeedSetting::Set(mEndpoint, newSpeedSetting)); } CHIP_ERROR FanControlManager::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) 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 ae2189c9127ea5..ca50fb06662283 100644 --- a/src/app/clusters/fan-control-server/fan-control-server.cpp +++ b/src/app/clusters/fan-control-server/fan-control-server.cpp @@ -448,7 +448,7 @@ bool emberAfFanControlClusterStepCallback(app::CommandHandler * commandObj, cons { Protocols::InteractionModel::Status status = Status::Success; - ChipLogProgress(Zcl, "FanControl emberAfFanControlClusterStepCallback:%u", commandPath.mEndpointId); + ChipLogProgress(Zcl, "FanControl emberAfFanControlClusterStepCallback: Endpoint %u", commandPath.mEndpointId); if (!SupportsStep(commandPath.mEndpointId)) { @@ -459,11 +459,9 @@ bool emberAfFanControlClusterStepCallback(app::CommandHandler * commandObj, cons { EndpointId endpoint = commandPath.mEndpointId; StepDirectionEnum direction = commandData.direction; - Optional wrap = commandData.wrap; - Optional lowestOff = commandData.lowestOff; - bool wrapValue = wrap.HasValue() ? wrap.Value() : false; - bool lowestOffValue = lowestOff.HasValue() ? lowestOff.Value() : false; + bool wrapValue = commandData.wrap.HasValue() ? commandData.wrap.Value() : false; + bool lowestOffValue = commandData.lowestOff.HasValue() ? commandData.lowestOff.Value() : false; Delegate * delegate = GetDelegate(endpoint); if (delegate) From 06c874c4cc9d52e9d393dc48608a6dea47a62368 Mon Sep 17 00:00:00 2001 From: Matthew Hazley Date: Wed, 19 Jul 2023 11:35:53 +0100 Subject: [PATCH 8/8] Initialising uninitialised value in HandleStep --- examples/all-clusters-app/all-clusters-common/src/fan-stub.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 5d55c63e8c8281..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 @@ -93,7 +93,7 @@ Status FanControlManager::HandleStep(StepDirectionEnum direction, bool wrap, boo DataModel::Nullable speedSetting; SpeedSetting::Get(mEndpoint, speedSetting); - uint8_t newSpeedSetting; + uint8_t newSpeedSetting = speedSetting.Value(); if (direction == StepDirectionEnum::kIncrease) {