Addressed review comments

project-chip · Jul 18, 2023 · 1531600 · 1531600
1 parent 725ccc3
commit 1531600
Show file tree

Hide file tree

Showing 15 changed files with 70 additions and 74 deletions.
diff --git a/...ll-clusters-app/all-clusters-common/include/static-supported-temperature-levels-manager.h b/...ll-clusters-app/all-clusters-common/include/static-supported-temperature-levels-manager.h
@@ -58,10 +58,6 @@ class StaticSupportedTemperatureLevelsManager : public SupportedTemperatureLevel
     SupportedTemperatureLevelsManager::TemperatureLevelOptionsProvider
     GetTemperatureLevelOptionsProvider(EndpointId endpointId) const override;
 
-    Protocols::InteractionModel::Status
-    GetTemperatureLevelOptionByTemperatureLevel(EndpointId endpointId, uint8_t temperatureLevel,
-                                                const TemperatureLevelStructType ** dataPtr) const override;
-
     ~StaticSupportedTemperatureLevelsManager(){};
 
     StaticSupportedTemperatureLevelsManager() {}

diff --git a/.../all-clusters-app/all-clusters-common/src/static-supported-temperature-levels-manager.cpp b/.../all-clusters-app/all-clusters-common/src/static-supported-temperature-levels-manager.cpp
@@ -64,30 +64,6 @@ StaticSupportedTemperatureLevelsManager::GetTemperatureLevelOptionsProvider(Endp
     return TemperatureLevelOptionsProvider(nullptr, nullptr);
 }
 
-Status StaticSupportedTemperatureLevelsManager::GetTemperatureLevelOptionByTemperatureLevel(
-    unsigned short endpointId, unsigned char temperatureLevel, const TemperatureLevelStructType ** dataPtr) const
-{
-    auto temperatureLevelOptionsProvider = this->GetTemperatureLevelOptionsProvider(endpointId);
-    if (temperatureLevelOptionsProvider.begin() == nullptr)
-    {
-        return Status::UnsupportedCluster;
-    }
-    auto * begin = temperatureLevelOptionsProvider.begin();
-    auto * end   = temperatureLevelOptionsProvider.end();
-
-    for (auto * it = begin; it != end; ++it)
-    {
-        auto & temperatureLevelOption = *it;
-        if (temperatureLevelOption.temperatureLevel == temperatureLevel)
-        {
-            *dataPtr = &temperatureLevelOption;
-            return Status::Success;
-        }
-    }
-    emberAfPrintln(EMBER_AF_PRINT_DEBUG, "Cannot find the temperatureLevel %u", temperatureLevel);
-    return Status::ConstraintError;
-}
-
 const TemperatureControl::SupportedTemperatureLevelsManager * TemperatureControl::GetSupportedTemperatureLevelsManager()
 {
     return &StaticSupportedTemperatureLevelsManager::instance;

diff --git a/examples/all-clusters-minimal-app/ameba/chip_main.cmake b/examples/all-clusters-minimal-app/ameba/chip_main.cmake
@@ -131,6 +131,7 @@ list(
     ${chip_dir}/examples/all-clusters-minimal-app/all-clusters-common/src/bridged-actions-stub.cpp
     ${chip_dir}/examples/all-clusters-minimal-app/all-clusters-common/src/smco-stub.cpp
     ${chip_dir}/examples/all-clusters-minimal-app/all-clusters-common/src/static-supported-modes-manager.cpp
+    ${chip_dir}/examples/all-clusters-minimal-app/all-clusters-common/src/static-supported-temperature-levels-manager.cpp
 
     ${chip_dir}/examples/all-clusters-minimal-app/ameba/main/chipinterface.cpp
     ${chip_dir}/examples/all-clusters-minimal-app/ameba/main/DeviceCallbacks.cpp

diff --git a/examples/all-clusters-minimal-app/infineon/psoc6/BUILD.gn b/examples/all-clusters-minimal-app/infineon/psoc6/BUILD.gn
@@ -111,6 +111,7 @@ psoc6_executable("clusters_minimal_app") {
     "${chip_root}/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp",
     "${chip_root}/examples/all-clusters-app/all-clusters-common/src/smco-stub.cpp",
     "${chip_root}/examples/all-clusters-app/all-clusters-common/src/static-supported-modes-manager.cpp",
+    "${chip_root}/examples/all-clusters-app/all-clusters-common/src/static-supported-temperature-levels-manager.cpp",
     "${examples_plat_dir}/LEDWidget.cpp",
     "${examples_plat_dir}/init_psoc6Platform.cpp",
     "src/AppTask.cpp",

diff --git a/examples/all-clusters-minimal-app/linux/BUILD.gn b/examples/all-clusters-minimal-app/linux/BUILD.gn
@@ -25,6 +25,7 @@ source_set("chip-all-clusters-common") {
     "${chip_root}/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp",
     "${chip_root}/examples/all-clusters-app/all-clusters-common/src/smco-stub.cpp",
     "${chip_root}/examples/all-clusters-app/all-clusters-common/src/static-supported-modes-manager.cpp",
+    "${chip_root}/examples/all-clusters-app/all-clusters-common/src/static-supported-temperature-levels-manager.cpp",
     "AppOptions.cpp",
     "include/tv-callbacks.cpp",
     "include/tv-callbacks.h",

diff --git a/examples/all-clusters-minimal-app/mbed/CMakeLists.txt b/examples/all-clusters-minimal-app/mbed/CMakeLists.txt
@@ -62,6 +62,7 @@ target_sources(${APP_TARGET} PRIVATE
                ${ALL_CLUSTERS_COMMON}/src/bridged-actions-stub.cpp
                ${ALL_CLUSTERS_COMMON}/src/smco-stub.cpp
                ${ALL_CLUSTERS_COMMON}/src/static-supported-modes-manager.cpp
+               ${ALL_CLUSTERS_COMMON}/src/static-supported-temperature-levels-manager.cpp
 )
 
 chip_configure_data_model(${APP_TARGET}

diff --git a/examples/all-clusters-minimal-app/nrfconnect/CMakeLists.txt b/examples/all-clusters-minimal-app/nrfconnect/CMakeLists.txt
@@ -57,6 +57,7 @@ target_sources(app PRIVATE
                main/main.cpp
                ${ALL_CLUSTERS_COMMON_DIR}/src/smco-stub.cpp
                ${ALL_CLUSTERS_COMMON_DIR}/src/static-supported-modes-manager.cpp
+               ${ALL_CLUSTERS_COMMON_DIR}/src/static-supported-temperature-levels-manager.cpp
                ${ALL_CLUSTERS_COMMON_DIR}/src/bridged-actions-stub.cpp
                ${ALL_CLUSTERS_COMMON_DIR}/src/binding-handler.cpp
                ${NRFCONNECT_COMMON}/util/LEDWidget.cpp)

diff --git a/examples/all-clusters-minimal-app/telink/CMakeLists.txt b/examples/all-clusters-minimal-app/telink/CMakeLists.txt
@@ -72,6 +72,7 @@ target_sources(app PRIVATE
                src/AppTask.cpp
                ${ALL_CLUSTERS_COMMON_DIR}/src/smco-stub.cpp
                ${ALL_CLUSTERS_COMMON_DIR}/src/static-supported-modes-manager.cpp
+               ${ALL_CLUSTERS_COMMON_DIR}/src/static-supported-temperature-levels-manager.cpp
                ${ALL_CLUSTERS_COMMON_DIR}/src/bridged-actions-stub.cpp
                ${ALL_CLUSTERS_COMMON_DIR}/src/binding-handler.cpp
                ${TELINK_COMMON}/common/src/mainCommon.cpp

diff --git a/examples/all-clusters-minimal-app/tizen/BUILD.gn b/examples/all-clusters-minimal-app/tizen/BUILD.gn
@@ -27,6 +27,7 @@ source_set("chip-all-clusters-common") {
     "${chip_root}/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp",
     "${chip_root}/examples/all-clusters-app/all-clusters-common/src/smco-stub.cpp",
     "${chip_root}/examples/all-clusters-app/all-clusters-common/src/static-supported-modes-manager.cpp",
+    "${chip_root}/examples/all-clusters-app/all-clusters-common/src/static-supported-temperature-levels-manager.cpp",
   ]
 
   deps = [

diff --git a/examples/placeholder/linux/apps/app1/BUILD.gn b/examples/placeholder/linux/apps/app1/BUILD.gn
@@ -32,6 +32,7 @@ source_set("app1") {
   sources = [
     "../../src/bridged-actions-stub.cpp",
     "../../static-supported-modes-manager.cpp",
+    "../../static-supported-temperature-levels-manager.cpp",
   ]
 
   public_deps = [ ":configuration" ]

diff --git a/examples/placeholder/linux/apps/app2/BUILD.gn b/examples/placeholder/linux/apps/app2/BUILD.gn
@@ -32,6 +32,7 @@ source_set("app2") {
   sources = [
     "../../src/bridged-actions-stub.cpp",
     "../../static-supported-modes-manager.cpp",
+    "../../static-supported-temperature-levels-manager.cpp",
   ]
 
   public_deps = [ ":configuration" ]

diff --git a/examples/shell/cc13x2x7_26x2x7/BUILD.gn b/examples/shell/cc13x2x7_26x2x7/BUILD.gn
@@ -78,6 +78,7 @@ ti_simplelink_executable("shell_app") {
     "${chip_root}/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp",
     "${chip_root}/examples/all-clusters-app/all-clusters-common/src/smco-stub.cpp",
     "${chip_root}/examples/all-clusters-app/all-clusters-common/src/static-supported-modes-manager.cpp",
+    "${chip_root}/examples/all-clusters-app/all-clusters-common/src/static-supported-temperature-levels-manager.cpp",
     "main/AppTask.cpp",
     "main/main.cpp",
   ]

diff --git a/examples/shell/shell_common/BUILD.gn b/examples/shell/shell_common/BUILD.gn
@@ -67,6 +67,7 @@ static_library("shell_common") {
       "${chip_root}/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp",
       "${chip_root}/examples/all-clusters-app/all-clusters-common/src/smco-stub.cpp",
       "${chip_root}/examples/all-clusters-app/all-clusters-common/src/static-supported-modes-manager.cpp",
+      "${chip_root}/examples/all-clusters-app/all-clusters-common/src/static-supported-temperature-levels-manager.cpp",
     ]
 
     include_dirs =

diff --git a/src/app/clusters/temperature-control-server/supported-temperature-levels-manager.h b/src/app/clusters/temperature-control-server/supported-temperature-levels-manager.h
@@ -68,18 +68,6 @@ class SupportedTemperatureLevelsManager
      */
     virtual TemperatureLevelOptionsProvider GetTemperatureLevelOptionsProvider(EndpointId endpointId) const = 0;
 
-    /**
-     * Given the endpointId and a temperature level value, find the TemperatureLevelStruct that matches the temperature level.
-     * @param endpointId The endpoint for which to search the TemperatureLevelStruct.
-     * @param temperatureLevel The temperature level for which to search the TemperatureLevelStruct.
-     * @param dataPtr The pointer to receive the TemperatureLevelStruct, if one is found.
-     * @return Status::Success if successfully found the option. Otherwise, returns appropriate status code (found in
-     * <app/util/af-enums.h>)
-     */
-    virtual Protocols::InteractionModel::Status
-    GetTemperatureLevelOptionByTemperatureLevel(EndpointId endpointId, uint8_t temperatureLevel,
-                                                const TemperatureLevelStructType ** dataPtr) const = 0;
-
     virtual ~SupportedTemperatureLevelsManager() {}
 };
 

diff --git a/src/app/clusters/temperature-control-server/temperature-control-server.cpp b/src/app/clusters/temperature-control-server/temperature-control-server.cpp
@@ -21,6 +21,7 @@
 #include <app/InteractionModelEngine.h>
 #include <app/clusters/temperature-control-server/supported-temperature-levels-manager.h>
 #include <app/util/attribute-storage.h>
+#include <app/util/error-mapping.h>
 
 using namespace chip;
 using namespace chip::app;
@@ -97,53 +98,77 @@ bool emberAfTemperatureControlClusterSetTemperatureCallback(app::CommandHandler
     auto & targetTemperatureLevel = commandData.targetTemperatureLevel;
     EndpointId endpoint           = commandPath.mEndpointId;
     Status status                 = Status::Success;
+    EmberAfStatus emberAfStatus   = EMBER_ZCL_STATUS_SUCCESS;
+
+    if (TemperatureControlHasFeature(endpoint, Feature::kTemperatureNumber) &&
+        TemperatureControlHasFeature(endpoint, Feature::kTemperatureLevel))
+    {
+        commandObj->AddStatus(commandPath, Status::Failure);
+        return false;
+    }
 
     if (TemperatureControlHasFeature(endpoint, TemperatureControl::Feature::kTemperatureNumber))
     {
-        if (!targetTemperature.HasValue())
+        if (targetTemperature.HasValue())
         {
-            status = Status::Failure;
-        }
+            int16_t minTemperature = 0;
+            int16_t maxTemperature = 0;
+            emberAfStatus          = MinTemperature::Get(endpoint, &minTemperature);
+            if (emberAfStatus != EMBER_ZCL_STATUS_SUCCESS)
+            {
+                status = app::ToInteractionModelStatus(emberAfStatus);
+            }
 
-        int16_t minTemperature = 0;
-        int16_t maxTemperature = 0;
-        if (MinTemperature::Get(endpoint, &minTemperature) != EMBER_ZCL_STATUS_SUCCESS)
-        {
-            status = Status::Failure;
-        }
-        if (MaxTemperature::Get(endpoint, &maxTemperature) != EMBER_ZCL_STATUS_SUCCESS)
-        {
-            status = Status::Failure;
-        }
+            emberAfStatus = MaxTemperature::Get(endpoint, &maxTemperature);
+            if (emberAfStatus != EMBER_ZCL_STATUS_SUCCESS)
+            {
+                status = app::ToInteractionModelStatus(emberAfStatus);
+            }
 
-        if (targetTemperature.Value() < minTemperature || targetTemperature.Value() > maxTemperature)
-        {
-            status = Status::ConstraintError;
-        }
+            if (targetTemperature.Value() < minTemperature || targetTemperature.Value() > maxTemperature)
+            {
+                status = Status::ConstraintError;
+            }
 
-        if (TemperatureSetpoint::Set(endpoint, targetTemperature.Value()) != EMBER_ZCL_STATUS_SUCCESS)
-        {
-            status = Status::Failure;
+            emberAfStatus = TemperatureSetpoint::Set(endpoint, targetTemperature.Value());
+            if (emberAfStatus != EMBER_ZCL_STATUS_SUCCESS)
+            {
+                status = app::ToInteractionModelStatus(emberAfStatus);
+            }
         }
     }
     if (TemperatureControlHasFeature(endpoint, TemperatureControl::Feature::kTemperatureLevel))
     {
-        if (!targetTemperatureLevel.HasValue())
-        {
-            status = Status::Failure;
-        }
-        const Structs::TemperatureLevelStruct::Type * dataPtr;
-        // TODO: Update implemetation when https://github.com/CHIP-Specifications/connectedhomeip-spec/issues/7005 fixed.
-        const TemperatureControl::SupportedTemperatureLevelsManager * gSupportedTemperatureLevelManager =
-            TemperatureControl::GetSupportedTemperatureLevelsManager();
-        status = gSupportedTemperatureLevelManager->GetTemperatureLevelOptionByTemperatureLevel(
-            endpoint, targetTemperatureLevel.Value(), &dataPtr);
-
-        if (status == Status::Success)
+        if (targetTemperatureLevel.HasValue())
         {
-            if (CurrentTemperatureLevelIndex::Set(endpoint, targetTemperatureLevel.Value()) != EMBER_ZCL_STATUS_SUCCESS)
+            bool foundEntryInStruct = false;
+            // TODO: Update implemetation when https://github.com/CHIP-Specifications/connectedhomeip-spec/issues/7005 fixed.
+            const TemperatureControl::SupportedTemperatureLevelsManager * gSupportedTemperatureLevelManager =
+                TemperatureControl::GetSupportedTemperatureLevelsManager();
+
+            const TemperatureControl::SupportedTemperatureLevelsManager::TemperatureLevelOptionsProvider
+                temperatureLevelOptionsProvider = gSupportedTemperatureLevelManager->GetTemperatureLevelOptionsProvider(endpoint);
+            if (temperatureLevelOptionsProvider.begin() != nullptr)
             {
-                status = Status::Failure;
+                auto * begin = temperatureLevelOptionsProvider.begin();
+                auto * end   = temperatureLevelOptionsProvider.end();
+                for (auto * it = begin; it != end; ++it)
+                {
+                    auto & temperatureLevelOption = *it;
+                    if (temperatureLevelOption.temperatureLevel == targetTemperatureLevel.Value())
+                    {
+                        foundEntryInStruct = true;
+                        emberAfStatus      = CurrentTemperatureLevelIndex::Set(endpoint, targetTemperatureLevel.Value());
+                        if (emberAfStatus != EMBER_ZCL_STATUS_SUCCESS)
+                        {
+                            status = app::ToInteractionModelStatus(emberAfStatus);
+                        }
+                    }
+                }
+                if (!foundEntryInStruct)
+                {
+                    status = Status::ConstraintError;
+                }
             }
         }
     }