From c89d1570ed41d789ba82f8cd223c94d9812827a0 Mon Sep 17 00:00:00 2001 From: Nivedita Sarkar Date: Thu, 8 Feb 2024 22:26:38 -0800 Subject: [PATCH] final touches --- .../apple-thermostat-extension-delegate.h | 36 +++++++++---------- .../apple-thermostat-extension-server.cpp | 14 ++++---- 2 files changed, 23 insertions(+), 27 deletions(-) diff --git a/src/app/clusters/thermostat-server/apple-thermostat-extension-delegate.h b/src/app/clusters/thermostat-server/apple-thermostat-extension-delegate.h index efb7c8c6631c44..acb09eeb43d3fc 100644 --- a/src/app/clusters/thermostat-server/apple-thermostat-extension-delegate.h +++ b/src/app/clusters/thermostat-server/apple-thermostat-extension-delegate.h @@ -16,10 +16,6 @@ namespace app { namespace Clusters { namespace Thermostat { -static constexpr size_t kMaxNumberOfPresetTypes = 6; - -static constexpr uint8_t kMaxNumberOfPresetsOfEachType = 1; - static constexpr uint8_t kAppleActivePresetHandleSize = 16; /** @brief @@ -65,7 +61,7 @@ class Delegate * @brief Get the PresetType at a given index in the ApplePresetTypes attribute * * @param[in] index The index of the preset type in the list. - * @param[out] preset The preset type at the given index in the list. + * @param[out] presetType The preset type at the given index in the list. * @return CHIP_ERROR_PROVIDER_LIST_EXHAUSTED if the index is out of range for the preset types list. */ virtual CHIP_ERROR GetApplePresetTypeAtIndex(size_t index, chip::app::Clusters::Thermostat::Structs::ApplePresetTypeStruct::Type & presetType) = 0; @@ -89,7 +85,7 @@ class Delegate /** * @brief Get the Apple active preset handle. * - * @param[out] activePresetHandle The MutableByteSpan to copy the apple active preset handle into. On success, + * @param[out] activePresetHandle The MutableByteSpan to copy the active preset handle into. On success, * the callee must update the length to the length of the copied data. If the value of * the attribute is null, the callee must set the MutableByteSpan to empty. */ @@ -98,24 +94,24 @@ class Delegate /** * @brief Set the value of AlvaradoEnabled attribute. * - * @param[in] enabled true/false based on whether the thermostat has the Alvarado feature enabled or not. + * @param[in] enabled true/false based on whether the thermostat wants to enable the Alvarado feature or not. */ virtual CHIP_ERROR SetAlvaradoEnabled(bool enabled); /** * @brief Set the value of ValenciaEnabled attribute. * - * @param[in] enabled true/false based on whether the thermostat has the Valencia feature enabled or not. + * @param[in] enabled true/false based on whether the thermostat wants to enable the Valencia feature or not. */ virtual CHIP_ERROR SetValenciaEnabled(bool enabled); /** - * @brief Appends a preset entry to the pending presets list maintained by the delegate. + * @brief Appends a preset to the pending presets list maintained by the delegate. * - * @param[in] preset The preset entry to add to the list. + * @param[in] preset The preset to add to the list. * - * @return CHIP_NO_ERROR if the preset entry was appended to the list successfully. - * @return CHIP_ERROR if there was an error adding the preset entry to the list. + * @return CHIP_NO_ERROR if the preset was appended to the list successfully. + * @return CHIP_ERROR if there was an error adding the preset to the list. */ virtual CHIP_ERROR AppendToPendingPresetList(const chip::app::Clusters::Thermostat::Structs::ApplePresetStruct::Type & preset) = 0; @@ -124,15 +120,15 @@ class Delegate * * @param[in] index The index of the preset in the list. * @param[out] preset The preset at the given index in the list. - * @return CHIP_ERROR_PROVIDER_LIST_EXHAUSTED if the index is out of range for the presets list. + * @return CHIP_ERROR_PROVIDER_LIST_EXHAUSTED if the index is out of range for the pending presets list. */ virtual CHIP_ERROR GetPendingPresetAtIndex(size_t index, chip::app::Clusters::Thermostat::Structs::ApplePresetStruct::Type & preset) = 0; /** * @brief Finds and returns an entry in the pending presets list that matches a preset. The presetHandle of the two presets must match. * - * @param[in] presetToMatch The preset with which to match. - * @param[out] matchingPreset The preset entry in the pending preset list that matches the presetToMatch + * @param[in] presetToMatch The preset to match with. + * @param[out] matchingPreset The preset in the pending preset list that matches the presetToMatch. * * @return true if a matching entry was found in the pending presets list, false otherwise. */ @@ -142,7 +138,7 @@ class Delegate * @brief Finds and returns the count of preset entries in the pending presets list that has the matching presetHandle. * @param[in] presetHandleToMatch The preset handle to match. * - * @return count of the number of presets found with the matching presetHandle. Returns 0 if no matching presets are found. + * @return count of the number of presets found with the matching presetHandle. Returns 0 if no matching presets were found. */ virtual uint8_t FindPresetInPendingListWithPresetHandle(const ByteSpan presetHandleToMatch); @@ -154,12 +150,12 @@ class Delegate virtual bool FindPresetScenarioInPresetTypes(chip::app::Clusters::Thermostat::ApplePresetScenarioEnum presetScenario); /** - * @brief Updates the presets attribute with the content of the pending presets list. If the preset matches an existing + * @brief Updates the presets attribute with the content of the pending presets list. If the preset in the pending list matches an existing * entry in the Presets attribute, the thermostat will update the entry with the new preset, otherwise it will * add a new preset to the Presets attribute. This should be called when the Thermostat receives a AppleCommitPresetsSchedulesRequest - * command to commit the pending changes to the attribute. The callee needs to check whether the size of the new updated Presets attribute list - * does not exceed the total number of presets supported or if the number of presets for each presetScenario does not exceed the max number of - * presets supported for that scenario before commiting the changes. + * command to commit the pending preset changes. Before commiting the changes, the callee needs to check if the size of the new + * updated Presets attribute list does not exceed the total number of presets supported or if the number of presets for each + * presetScenario does not exceed the max number of presets supported for that scenario. * * @return CHIP_ERROR_INVALID_LIST_LENGTH if the expected length of the presets attribute list with the changes exceeds the total number of presets supported * or if the number of presets for each presetScenario exceeds the max number of presets supported for that scenario. diff --git a/src/app/clusters/thermostat-server/apple-thermostat-extension-server.cpp b/src/app/clusters/thermostat-server/apple-thermostat-extension-server.cpp index d07ecb809f777e..13a01f5ec719d4 100644 --- a/src/app/clusters/thermostat-server/apple-thermostat-extension-server.cpp +++ b/src/app/clusters/thermostat-server/apple-thermostat-extension-server.cpp @@ -28,7 +28,7 @@ bool gApplePresetsEditable; static constexpr size_t kThermostatDelegateTableSize = EMBER_AF_THERMOSTAT_CLUSTER_SERVER_ENDPOINT_COUNT + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT; -static_assert(kThermostatDelegateTableSize <= kEmberInvalidEndpointIndex, "Door Lock Delegate table size error"); +static_assert(kThermostatDelegateTableSize <= kEmberInvalidEndpointIndex, "Thermostat Delegate table size error"); namespace chip { namespace app { @@ -62,7 +62,7 @@ CHIP_ERROR AppleThermostatExtAttrAccess::Read(const ConcreteReadAttributePath & { VerifyOrReturnError(aPath.mClusterId == Thermostat::Id, CHIP_ERROR_INVALID_ARGUMENT); - Delegate * delegate = GetDelegate(aPath.mEndpointId); + Delegate * delegate = GetDelegate(aPath.mEndpointId); switch (aPath.mAttributeId) { @@ -183,10 +183,10 @@ CHIP_ERROR AppleThermostatExtAttrAccess::Write(const ConcreteDataAttributePath & case AlvaradoEnabled::Id: { VerifyOrReturnError(delegate != nullptr, CHIP_ERROR_INCORRECT_STATE, ChipLogError(Zcl, "Delegate is null")); - bool alvaradoEnabledValue = false; - ReturnErrorOnFailure(aDecoder.Decode(alvaradoEnabledValue)); + bool newValue = false; + ReturnErrorOnFailure(aDecoder.Decode(newValue)); - return delegate->SetAlvaradoEnabled(alvaradoEnabledValue); + return delegate->SetAlvaradoEnabled(newValue); } case ApplePresets::Id: { VerifyOrReturnError(delegate != nullptr, CHIP_ERROR_INCORRECT_STATE, ChipLogError(Zcl, "Delegate is null")); @@ -194,7 +194,7 @@ CHIP_ERROR AppleThermostatExtAttrAccess::Write(const ConcreteDataAttributePath & ApplePresets::TypeInfo::DecodableType newPresets; ReturnErrorOnFailure(aDecoder.Decode(newPresets)); - VerifyOrReturnError(gApplePresetsEditable, CHIP_ERROR_INCORRECT_STATE, ChipLogError(Zcl, "Presets are not editable")); + VerifyOrReturnError(gApplePresetsEditable, CHIP_IM_GLOBAL_STATUS(InvalidInState), ChipLogError(Zcl, "Presets are not editable")); // TODO: #19 - Check if the gPresetEditRequestOriginatorNodeId is the same as the node editing the presets. @@ -502,7 +502,7 @@ bool emberAfThermostatClusterAppleCommitPresetsSchedulesRequestCallback( return false; } - // If the preset handle is Null and the built in field is set to true, return CONSTRAINT_ERROR. + // If the preset handle is null and the built in field is set to true, return CONSTRAINT_ERROR. if (pendingPreset.presetHandle.IsNull() && (IsBuiltIn(pendingPreset))) { return sendResponseAndCleanUp(delegate, endpoint, commandObj, commandPath, Status::ConstraintError);