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 bca9148300dca7..98600d1080ceff 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 @@ -8866,6 +8866,7 @@ endpoint 1 { ram attribute clusterRevision default = 6; handle command SetpointRaiseLower; + handle command SetActivePresetRequest; handle command AtomicResponse; handle command AtomicRequest; } 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 d8e1be226c9af4..5f659abc1503a0 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 @@ -16394,6 +16394,14 @@ "isIncoming": 1, "isEnabled": 1 }, + { + "name": "SetActivePresetRequest", + "code": 6, + "mfgCode": null, + "source": "client", + "isIncoming": 1, + "isEnabled": 1 + }, { "name": "AtomicResponse", "code": 253, diff --git a/examples/thermostat/linux/thermostat-delegate-impl.cpp b/examples/thermostat/linux/thermostat-delegate-impl.cpp index a86a9058e9fc83..61d496f233b408 100644 --- a/examples/thermostat/linux/thermostat-delegate-impl.cpp +++ b/examples/thermostat/linux/thermostat-delegate-impl.cpp @@ -195,8 +195,6 @@ CHIP_ERROR ThermostatDelegate::ApplyPendingPresets() const PresetStructWithOwnedMembers & pendingPreset = mPendingPresets[indexInPendingPresets]; mPresets[mNextFreeIndexInPresetsList] = pendingPreset; mNextFreeIndexInPresetsList++; - ChipLogError(Zcl, "applied pending preset again"); - } return CHIP_NO_ERROR; } diff --git a/src/app/clusters/thermostat-server/PresetStructWithOwnedMembers.cpp b/src/app/clusters/thermostat-server/PresetStructWithOwnedMembers.cpp index dfc395f6bee81d..45eb8f5ebfd95b 100644 --- a/src/app/clusters/thermostat-server/PresetStructWithOwnedMembers.cpp +++ b/src/app/clusters/thermostat-server/PresetStructWithOwnedMembers.cpp @@ -50,6 +50,11 @@ void PresetStructWithOwnedMembers::operator=(const PresetStruct::Type & other) SetBuiltIn(other.builtIn); } +void PresetStructWithOwnedMembers::operator=(const PresetStructWithOwnedMembers & other) +{ + *this = static_cast(other); +} + void PresetStructWithOwnedMembers::SetPresetScenario(PresetScenarioEnum enumValue) { presetScenario = enumValue; diff --git a/src/app/clusters/thermostat-server/PresetStructWithOwnedMembers.h b/src/app/clusters/thermostat-server/PresetStructWithOwnedMembers.h index 7161fb874989e2..d31cbc3b2cb409 100644 --- a/src/app/clusters/thermostat-server/PresetStructWithOwnedMembers.h +++ b/src/app/clusters/thermostat-server/PresetStructWithOwnedMembers.h @@ -43,6 +43,7 @@ struct PresetStructWithOwnedMembers : protected Structs::PresetStruct::Type PresetStructWithOwnedMembers() = default; PresetStructWithOwnedMembers(const Structs::PresetStruct::Type & other); void operator=(const Structs::PresetStruct::Type & other); + void operator=(const PresetStructWithOwnedMembers & other); void SetPresetScenario(PresetScenarioEnum enumValue); CHIP_ERROR SetPresetHandle(const DataModel::Nullable & newPresetHandle); diff --git a/src/app/clusters/thermostat-server/thermostat-server.cpp b/src/app/clusters/thermostat-server/thermostat-server.cpp index 1bc7e88d218370..d4c85f56e1b89a 100644 --- a/src/app/clusters/thermostat-server/thermostat-server.cpp +++ b/src/app/clusters/thermostat-server/thermostat-server.cpp @@ -192,13 +192,10 @@ ScopedNodeId GetSourceScopedNodeId(CommandHandler * commandObj) } /** - * @brief Sends a response for the command and cleans up state by calling CleanUp() + * @brief Discards pending atomic writes and atomic state. * * @param[in] delegate The delegate to use. * @param[in] endpoint The endpoint to use. - * @param[in] commandObj The command handler to use to add the status response. - * @param[in] commandPath The command path. - * @param[in] status The status code to send as the response. * */ void resetAtomicWrite(Delegate * delegate, EndpointId endpoint) @@ -250,8 +247,8 @@ bool MatchingPendingPresetExists(Delegate * delegate, const PresetStructWithOwne } /** - * @brief Finds and returns an entry in the Presets attribute list that matches a preset. - * The presetHandle of the two presets must match. + * @brief Finds and returns an entry in the Presets attribute list that matches + * a preset, if such an entry exists. The presetToMatch must have a preset handle. * * @param[in] delegate The delegate to use. * @param[in] presetToMatch The preset to match with. @@ -259,7 +256,7 @@ bool MatchingPendingPresetExists(Delegate * delegate, const PresetStructWithOwne * * @return true if a matching entry was found in the presets attribute list, false otherwise. */ -bool GetMatchingPresetInPresets(Delegate * delegate, const PresetStructWithOwnedMembers & presetToMatch, +bool GetMatchingPresetInPresets(Delegate * delegate, const PresetStruct::Type & presetToMatch, PresetStructWithOwnedMembers & matchingPreset) { VerifyOrReturnValue(delegate != nullptr, false); @@ -278,7 +275,8 @@ bool GetMatchingPresetInPresets(Delegate * delegate, const PresetStructWithOwned return false; } - if (PresetHandlesExistAndMatch(matchingPreset, presetToMatch)) + // Note: presets coming from our delegate always have a handle. + if (presetToMatch.presetHandle.Value().data_equal(matchingPreset.GetPresetHandle().Value())) { return true; } @@ -860,7 +858,6 @@ CHIP_ERROR ThermostatAttrAccess::Write(const ConcreteDataAttributePath & aPath, // and add to the pending presets list. if (!aPath.IsListOperation() || aPath.mListOp == ConcreteDataAttributePath::ListOperation::ReplaceAll) { - ChipLogError(Zcl, "Replace all!"); // Clear the pending presets list delegate->ClearPendingPresetList(); @@ -872,14 +869,7 @@ CHIP_ERROR ThermostatAttrAccess::Write(const ConcreteDataAttributePath & aPath, while (iter.Next()) { const PresetStruct::Type & preset = iter.GetValue(); - if (IsValidPresetEntry(preset)) - { - ReturnErrorOnFailure(delegate->AppendToPendingPresetList(preset)); - } - else - { - return CHIP_IM_GLOBAL_STATUS(ConstraintError); - } + ReturnErrorOnFailure(AppendPendingPreset(delegate, preset)); } return iter.GetStatus(); } @@ -887,14 +877,9 @@ CHIP_ERROR ThermostatAttrAccess::Write(const ConcreteDataAttributePath & aPath, // If the list operation is AppendItem, call the delegate to append the item to the list of pending presets. if (aPath.mListOp == ConcreteDataAttributePath::ListOperation::AppendItem) { - ChipLogError(Zcl, "Appending again!"); PresetStruct::Type preset; ReturnErrorOnFailure(aDecoder.Decode(preset)); - if (IsValidPresetEntry(preset)) - { - return delegate->AppendToPendingPresetList(preset); - } - return CHIP_IM_GLOBAL_STATUS(ConstraintError); + return AppendPendingPreset(delegate, preset); } } break; @@ -939,6 +924,60 @@ CHIP_ERROR ThermostatAttrAccess::Write(const ConcreteDataAttributePath & aPath, return CHIP_NO_ERROR; } +CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Delegate * delegate, const PresetStruct::Type & preset) +{ + if (!IsValidPresetEntry(preset)) + { + return CHIP_IM_GLOBAL_STATUS(ConstraintError); + } + + if (preset.presetHandle.IsNull()) + { + if (IsBuiltIn(preset)) + { + return CHIP_IM_GLOBAL_STATUS(ConstraintError); + } + } + else + { + auto & presetHandle = preset.presetHandle.Value(); + + // Per spec we need to check that: + // (a) There is an existing non-pending preset with this handle. + PresetStructWithOwnedMembers matchingPreset; + if (!GetMatchingPresetInPresets(delegate, preset, matchingPreset)) + { + return CHIP_IM_GLOBAL_STATUS(NotFound); + } + + // (b) There is no existing pending preset with this handle. + if (CountPresetsInPendingListWithPresetHandle(delegate, presetHandle) > 0) + { + return CHIP_IM_GLOBAL_STATUS(ConstraintError); + } + + // (c)/(d) The built-in fields do not have a mismatch. + // TODO: What's the story with nullability on the BuiltIn field? + if (!preset.builtIn.IsNull() && !matchingPreset.GetBuiltIn().IsNull() && + preset.builtIn.Value() != matchingPreset.GetBuiltIn().Value()) + { + return CHIP_IM_GLOBAL_STATUS(ConstraintError); + } + } + + if (!PresetScenarioExistsInPresetTypes(delegate, preset.presetScenario)) + { + return CHIP_IM_GLOBAL_STATUS(ConstraintError); + } + + if (preset.name.HasValue() && !PresetTypeSupportsNames(delegate, preset.presetScenario)) + { + return CHIP_IM_GLOBAL_STATUS(ConstraintError); + } + + return delegate->AppendToPendingPresetList(preset); +} + } // namespace Thermostat } // namespace Clusters } // namespace app @@ -1341,7 +1380,8 @@ bool validAtomicAttributes(const Commands::AtomicRequest::DecodableType & comman return (requestedPresets || requestedSchedules); } -Commands::AtomicResponse::Type buildAtomicResponse(imcode status, imcode presetsStatus, imcode schedulesStatus) +void sendAtomicResponse(CommandHandler * commandObj, const ConcreteCommandPath & commandPath, imcode status, imcode presetsStatus, + imcode schedulesStatus, Optional timeout = NullOptional) { Commands::AtomicResponse::Type response; Globals::Structs::AtomicAttributeStatusStruct::Type attributeStatus[] = { @@ -1350,7 +1390,8 @@ Commands::AtomicResponse::Type buildAtomicResponse(imcode status, imcode presets }; response.statusCode = to_underlying(status); response.attributeStatus = attributeStatus; - return response; + response.timeout = timeout; + commandObj->AddResponse(commandPath, response); } void handleAtomicBegin(CommandHandler * commandObj, const ConcreteCommandPath & commandPath, @@ -1358,6 +1399,15 @@ void handleAtomicBegin(CommandHandler * commandObj, const ConcreteCommandPath & { EndpointId endpoint = commandPath.mEndpointId; + Delegate * delegate = GetDelegate(endpoint); + + if (delegate == nullptr) + { + ChipLogError(Zcl, "Delegate is null"); + commandObj->AddStatus(commandPath, imcode::InvalidInState); + return; + } + if (gThermostatAttrAccess.InAtomicWrite(commandObj, endpoint)) { // This client already has an open atomic write @@ -1381,11 +1431,13 @@ void handleAtomicBegin(CommandHandler * commandObj, const ConcreteCommandPath & if (gThermostatAttrAccess.InAtomicWrite(endpoint)) { - commandObj->AddResponse(commandPath, buildAtomicResponse(imcode::Failure, imcode::Busy, imcode::Busy)); + sendAtomicResponse(commandObj, commandPath, imcode::Failure, imcode::Busy, imcode::Busy); return; } - // This is a valid request to open an atomic write. Note that + // This is a valid request to open an atomic write. Tell the delegate it + // needs to keep track of a pending preset list now. + delegate->InitializePendingPresets(); uint16_t maxTimeout = 5000; timeout = std::min(timeout, maxTimeout); @@ -1393,21 +1445,19 @@ void handleAtomicBegin(CommandHandler * commandObj, const ConcreteCommandPath & ScheduleTimer(endpoint, System::Clock::Milliseconds16(timeout)); gThermostatAttrAccess.SetAtomicWrite(endpoint, true); gThermostatAttrAccess.SetAtomicWriteScopedNodeId(endpoint, GetSourceScopedNodeId(commandObj)); - auto response = buildAtomicResponse(imcode::Success, imcode::Success, imcode::Success); - response.timeout.Emplace(timeout); - commandObj->AddResponse(commandPath, response); + sendAtomicResponse(commandObj, commandPath, imcode::Success, imcode::Success, imcode::Success, MakeOptional(timeout)); return; } imcode commitPresets(Delegate * delegate, EndpointId endpoint) { - PresetStructWithOwnedMembers preset; CHIP_ERROR err = CHIP_NO_ERROR; // For each preset in the presets attribute, check that the matching preset in the pending presets list does not // violate any spec constraints. for (uint8_t i = 0; true; i++) { + PresetStructWithOwnedMembers preset; err = delegate->GetPresetAtIndex(i, preset); if (err == CHIP_ERROR_PROVIDER_LIST_EXHAUSTED) @@ -1454,6 +1504,7 @@ imcode commitPresets(Delegate * delegate, EndpointId endpoint) return imcode::InvalidInState; } } + // For each preset in the pending presets list, check that the preset does not violate any spec constraints. for (uint8_t i = 0; true; i++) { @@ -1473,66 +1524,8 @@ imcode commitPresets(Delegate * delegate, EndpointId endpoint) return imcode::InvalidInState; } - bool isPendingPresetWithNullPresetHandle = pendingPreset.GetPresetHandle().IsNull(); - - // If the preset handle is null and the built in field is set to true, return CONSTRAINT_ERROR. - if (isPendingPresetWithNullPresetHandle && IsBuiltIn(pendingPreset)) - { - return imcode::ConstraintError; - } - - bool foundMatchingPresetInPresets = false; - PresetStructWithOwnedMembers matchingPreset; - if (!isPendingPresetWithNullPresetHandle) - { - foundMatchingPresetInPresets = GetMatchingPresetInPresets(delegate, pendingPreset, matchingPreset); - - // If the presetHandle for the pending preset is not null and a matching preset is not found in the - // presets attribute list, return NOT_FOUND. - if (!foundMatchingPresetInPresets) - { - return imcode::NotFound; - } - - // Find the number of presets in the pending preset list that match the preset handle. If there are duplicate - // entries, return CONSTRAINT_ERROR. - uint8_t count = CountPresetsInPendingListWithPresetHandle(delegate, pendingPreset.GetPresetHandle().Value()); - if (count > 1) - { - return imcode::ConstraintError; - } - } - - // If the BuiltIn field is true, and the PresetStruct in the current value with a - // matching PresetHandle field has a BuiltIn field set to false, a response - // with the status code CONSTRAINT_ERROR SHALL be returned. - if (foundMatchingPresetInPresets && (IsBuiltIn(pendingPreset) && !IsBuiltIn(matchingPreset))) - { - return imcode::ConstraintError; - } - - // If the BuiltIn field is false, and the PresetStruct in the current value with a - // matching PresetHandle field has a BuiltIn field set to true, a response - // with the status code CONSTRAINT_ERROR SHALL be returned. - if (foundMatchingPresetInPresets && (!IsBuiltIn(pendingPreset) && IsBuiltIn(matchingPreset))) - { - return imcode::ConstraintError; - } - - // If the presetScenario is not found in the preset types, return CONSTRAINT_ERROR. - PresetScenarioEnum presetScenario = pendingPreset.GetPresetScenario(); - if (!PresetScenarioExistsInPresetTypes(delegate, presetScenario)) - { - return imcode::ConstraintError; - } - - // If the preset type for the preset scenario does not support names and a name is specified, return CONSTRAINT_ERROR. - if (!PresetTypeSupportsNames(delegate, presetScenario) && pendingPreset.GetName().HasValue()) - { - return imcode::ConstraintError; - } - // Enforce the Setpoint Limits for both the cooling and heating setpoints in the pending preset. + // TODO: This code does not work, because it's modifying our temporary copy. Optional coolingSetpointValue = pendingPreset.GetCoolingSetpoint(); if (coolingSetpointValue.HasValue()) { @@ -1573,14 +1566,13 @@ imcode commitPresets(Delegate * delegate, EndpointId endpoint) { return imcode::InvalidInState; } - ChipLogError(Zcl, "committed presets"); + return imcode::Success; } void handleAtomicCommit(CommandHandler * commandObj, const ConcreteCommandPath & commandPath, const Commands::AtomicRequest::DecodableType & commandData) { - ChipLogError(Zcl, "handleAtomicCommit"); if (!validAtomicAttributes(commandData, true)) { commandObj->AddStatus(commandPath, imcode::InvalidCommand); @@ -1608,7 +1600,7 @@ void handleAtomicCommit(CommandHandler * commandObj, const ConcreteCommandPath & auto schedulesStatus = imcode::Success; resetAtomicWrite(delegate, endpoint); imcode status = (presetsStatus == imcode::Success && schedulesStatus == imcode::Success) ? imcode::Success : imcode::Failure; - commandObj->AddResponse(commandPath, buildAtomicResponse(status, presetsStatus, schedulesStatus)); + sendAtomicResponse(commandObj, commandPath, status, presetsStatus, schedulesStatus); } void handleAtomicRollback(CommandHandler * commandObj, const ConcreteCommandPath & commandPath, @@ -1636,7 +1628,7 @@ void handleAtomicRollback(CommandHandler * commandObj, const ConcreteCommandPath return; } resetAtomicWrite(delegate, endpoint); - commandObj->AddResponse(commandPath, buildAtomicResponse(imcode::Success, imcode::Success, imcode::Success)); + sendAtomicResponse(commandObj, commandPath, imcode::Success, imcode::Success, imcode::Success); } bool emberAfThermostatClusterAtomicRequestCallback(CommandHandler * commandObj, const ConcreteCommandPath & commandPath, @@ -1715,13 +1707,13 @@ bool emberAfThermostatClusterSetpointRaiseLowerCallback(app::CommandHandler * co { DesiredCoolingSetpoint = static_cast(CoolingSetpoint + amount * 10); CoolLimit = static_cast(DesiredCoolingSetpoint - - EnforceCoolingSetpointLimits(DesiredCoolingSetpoint, aEndpointId)); + EnforceCoolingSetpointLimits(DesiredCoolingSetpoint, aEndpointId)); { if (OccupiedHeatingSetpoint::Get(aEndpointId, &HeatingSetpoint) == imcode::Success) { DesiredHeatingSetpoint = static_cast(HeatingSetpoint + amount * 10); HeatLimit = static_cast(DesiredHeatingSetpoint - - EnforceHeatingSetpointLimits(DesiredHeatingSetpoint, aEndpointId)); + EnforceHeatingSetpointLimits(DesiredHeatingSetpoint, aEndpointId)); { if (CoolLimit != 0 || HeatLimit != 0) { diff --git a/src/app/clusters/thermostat-server/thermostat-server.h b/src/app/clusters/thermostat-server/thermostat-server.h index 230f21b0ccfe25..306a2a625c57b6 100644 --- a/src/app/clusters/thermostat-server/thermostat-server.h +++ b/src/app/clusters/thermostat-server/thermostat-server.h @@ -105,6 +105,8 @@ class ThermostatAttrAccess : public chip::app::AttributeAccessInterface bool InAtomicWrite(CommandHandler * commandObj, EndpointId endpoint); private: + CHIP_ERROR AppendPendingPreset(Delegate * delegate, const Structs::PresetStruct::Type & preset); + ScopedNodeId mAtomicWriteNodeIds[kThermostatEndpointCount]; bool mAtomicWriteState[kThermostatEndpointCount]; }; diff --git a/src/app/tests/suites/certification/PICS.yaml b/src/app/tests/suites/certification/PICS.yaml index 8a3e1fce94326f..cfe4681f7fb2c3 100644 --- a/src/app/tests/suites/certification/PICS.yaml +++ b/src/app/tests/suites/certification/PICS.yaml @@ -6523,19 +6523,9 @@ PICS: id: TSTAT.S.C06.Rsp - label: - "Does the device implement receiving the - StartPresetsSchedulesEditRequest command?" - id: TSTAT.S.C07.Rsp - - - label: - "Does the device implement receiving the - CancelPresetsSchedulesEditRequest command?" - id: TSTAT.S.C08.Rsp - - - label: - "Does the device implement receiving the CommitPresetsSchedulesRequest - command?" - id: TSTAT.S.C09.Rsp + "Does the device implement receiving the AtomicRequest command for + Thermostat?" + id: TSTAT.S.CFE.Rsp # # server / commandsGenerated @@ -6550,6 +6540,9 @@ PICS: command?" id: TSTAT.S.C01.Tx + - label: "Does the device implement sending the AtomicResponse command?" + id: TSTAT.S.CFD.Tx + # # server / features # diff --git a/src/app/tests/suites/certification/ci-pics-values b/src/app/tests/suites/certification/ci-pics-values index 141af5d9090ba1..6960d3388859f1 100644 --- a/src/app/tests/suites/certification/ci-pics-values +++ b/src/app/tests/suites/certification/ci-pics-values @@ -1999,7 +1999,8 @@ TSTAT.S.C02.Rsp=0 TSTAT.S.C03.Rsp=0 TSTAT.S.C04.Rsp=0 TSTAT.S.C06.Rsp=1 -TSTAT.S.C07.Rsp=1 +TSTAT.S.CFE.Rsp=1 +TSTAT.S.CFD.Tx=1 # Client TSTAT.C=0 diff --git a/src/python_testing/TC_TSTAT_4_2.py b/src/python_testing/TC_TSTAT_4_2.py index 609b67284f5361..513715b7d30f31 100644 --- a/src/python_testing/TC_TSTAT_4_2.py +++ b/src/python_testing/TC_TSTAT_4_2.py @@ -62,11 +62,12 @@ async def write_presets(self, endpoint, presets) -> Status: return result[0].Status async def send_edit_atomic_request_begin_command(self, - endpoint: int = None, - expected_status: Status = Status.Success): + endpoint: int = None, + expected_status: Status = Status.Success): try: - await self.send_single_cmd(cmd=cluster.Commands.AtomicRequest(requestType=0, - attributeRequests=[cluster.Attributes.Presets.attribute_id], + await self.send_single_cmd(cmd=cluster.Commands.AtomicRequest(requestType=0, + attributeRequests=[ + cluster.Attributes.Presets.attribute_id], timeout=1800), endpoint=endpoint) asserts.assert_equal(expected_status, Status.Success) @@ -75,21 +76,31 @@ async def send_edit_atomic_request_begin_command(self, asserts.assert_equal(e.status, expected_status, "Unexpected error returned") async def send_edit_atomic_request_commit_command(self, - endpoint: int = None, - expected_status: Status = Status.Success): + endpoint: int = None, + expected_status: Status = Status.Success, + expected_overall_status: Status = Status.Success, + expected_preset_status: Status = Status.Success): try: - await self.send_single_cmd(cmd=cluster.Commands.AtomicRequest(requestType=1, - attributeRequests=[cluster.Attributes.Presets.attribute_id, cluster.Attributes.Schedules.attribute_id]), - endpoint=endpoint) - asserts.assert_equal(expected_status, Status.Success) + response = await self.send_single_cmd(cmd=cluster.Commands.AtomicRequest(requestType=1, + attributeRequests=[cluster.Attributes.Presets.attribute_id, cluster.Attributes.Schedules.attribute_id]), + endpoint=endpoint) + asserts.assert_equal(expected_status, Status.Success, "We expected we had a valid commit command") + asserts.assert_equal(response.statusCode, expected_overall_status, "Commit should have the right overall status") + found_preset_status = False + for attrStatus in response.attributeStatus: + if attrStatus.attributeID == cluster.Attributes.Presets.attribute_id: + asserts.assert_equal(attrStatus.statusCode, expected_preset_status, + "Preset attribute commit should have the right status") + found_preset_status = True + asserts.assert_true(found_preset_status, "Preset attribute commit should have a status") except InteractionModelError as e: asserts.assert_equal(e.status, expected_status, "Unexpected error returned") async def send_edit_atomic_request_rollback_command(self, - endpoint: int = None, - expected_status: Status = Status.Success): + endpoint: int = None, + expected_status: Status = Status.Success): try: - await self.send_single_cmd(cmd=cluster.Commands.AtomicRequest(requestType=2, + await self.send_single_cmd(cmd=cluster.Commands.AtomicRequest(requestType=2, attributeRequests=[cluster.Attributes.Presets.attribute_id, cluster.Attributes.Schedules.attribute_id]), endpoint=endpoint) asserts.assert_equal(expected_status, Status.Success) @@ -143,7 +154,7 @@ def steps_TC_TSTAT_4_2(self) -> list[TestStep]: TestStep("12", "TH writes to the Presets attribute with a preset that doesn't support names in the PresetTypeFeatures bitmap but has a name", "Verify that the CommitPresetsSchedulesRequest returned CONSTRAINT_ERROR (0x87)."), TestStep("13", "TH writes to the Presets attribute but calls the CancelPresetsSchedulesEditRequest command to cancel the edit request", - "Verify that the edit request was cancelled"),*/ + "Verify that the edit request was cancelled"), ] return steps @@ -167,7 +178,7 @@ async def test_TC_TSTAT_4_2(self): asserts.assert_true(status_ok, "Presets write did not return InvalidInState as expected") self.step("3") - if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.C07.Rsp")): + if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.CFE.Rsp")): await self.send_edit_atomic_request_begin_command() # Write to the presets attribute after calling AtomicRequest command @@ -175,17 +186,21 @@ async def test_TC_TSTAT_4_2(self): status_ok = (status == Status.Success) asserts.assert_true(status_ok, "Presets write did not return Success as expected") - # Read the presets attribute and verify it was not updated since CommitPresetsSchedulesRequest was not called after writing presets + # Read the presets attribute and verify it was updated by the write presets = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.Presets) logger.info(f"Rx'd Presets: {presets}") - asserts.assert_equal(presets, new_presets_with_handle, "Presets were updated which is not expected") + asserts.assert_equal(presets, new_presets_with_handle, "Presets were updated, as expected") await self.send_edit_atomic_request_rollback_command() + # Read the presets attribute and verify it has been properly rolled back + presets = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.Presets) + asserts.assert_equal(presets, initial_presets, "Presets were updated which is not expected") + self.step("4") - if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.C07.Rsp") and self.check_pics("TSTAT.S.C07.Rsp")): + if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.CFE.Rsp")): - # Send the AtomicRequest command + # Send the AtomicRequest begin command await self.send_edit_atomic_request_begin_command() # Write to the presets attribute after calling AtomicRequest command @@ -193,18 +208,18 @@ async def test_TC_TSTAT_4_2(self): status_ok = (status == Status.Success) asserts.assert_true(status_ok, "Presets write did not return Success as expected") - # Send the CommitPresetsSchedulesRequest command + # Send the AtomicRequest commit command await self.send_edit_atomic_request_commit_command() - # Read the presets attribute and verify it was updated since CommitPresetsSchedulesRequest was called after writing presets + # Read the presets attribute and verify it was updated since AtomicRequest commit was called after writing presets presets = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.Presets) logger.info(f"Rx'd Presets: {presets}") asserts.assert_equal(presets, new_presets_with_handle, "Presets were not updated which is not expected") self.step("5") - if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.C07.Rsp") and self.check_pics("TSTAT.S.C09.Rsp")): + if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.CFE.Rsp")): - # Send the AtomicRequest command + # Send the AtomicRequest begin command await self.send_edit_atomic_request_begin_command() # Write to the presets attribute after removing a built in preset from the list. Remove the first entry. @@ -214,11 +229,11 @@ async def test_TC_TSTAT_4_2(self): status_ok = (status == Status.Success) asserts.assert_true(status_ok, "Presets write did not return Success as expected") - # Send the CommitPresetsSchedulesRequest command and expect UnsupportedAccess - await self.send_edit_atomic_request_commit_command(expected_status=Status.UnsupportedAccess) + # Send the AtomicRequest commit command and expect ConstraintError for presets. + await self.send_edit_atomic_request_commit_command(expected_overall_status=Status.Failure, expected_preset_status=Status.ConstraintError) self.step("6") - if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.C06.Rsp") and self.check_pics("TSTAT.S.C07.Rsp") and self.check_pics("TSTAT.S.C09.Rsp")): + if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.C06.Rsp") and self.check_pics("TSTAT.S.CFE.Rsp")): # Send the SetActivePresetRequest command await self.send_set_active_preset_handle_request_command(value=b'\x03') @@ -228,7 +243,7 @@ async def test_TC_TSTAT_4_2(self): logger.info(f"Rx'd ActivePresetHandle: {activePresetHandle}") asserts.assert_equal(activePresetHandle, b'\x03', "Active preset handle was not updated as expected") - # Send the AtomicRequest command + # Send the AtomicRequest begin command await self.send_edit_atomic_request_begin_command() # Write to the presets attribute after removing the preset that was set as the active preset handle. Remove the last entry with preset handle (b'\x03') @@ -238,13 +253,13 @@ async def test_TC_TSTAT_4_2(self): status_ok = (status == Status.Success) asserts.assert_true(status_ok, "Presets write did not return Success as expected") - # Send the CommitPresetsSchedulesRequest command and expect InvalidInState - await self.send_edit_atomic_request_commit_command(expected_status=Status.InvalidInState) + # Send the AtomicRequest commit command and expect InvalidInState for presets. + await self.send_edit_atomic_request_commit_command(expected_overall_status=Status.Failure, expected_preset_status=Status.InvalidInState) self.step("7") - if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.C07.Rsp") and self.check_pics("TSTAT.S.C09.Rsp")): + if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.CFE.Rsp")): - # Send the AtomicRequest command + # Send the AtomicRequest begin command await self.send_edit_atomic_request_begin_command() # Write to the presets attribute after setting the builtIn flag to False for preset with handle (b'\x01') @@ -252,16 +267,16 @@ async def test_TC_TSTAT_4_2(self): test_presets[0].builtIn = False status = await self.write_presets(endpoint=endpoint, presets=test_presets) - status_ok = (status == Status.Success) - asserts.assert_true(status_ok, "Presets write did not return Success as expected") + asserts.assert_equal(status, Status.ConstraintError, + "Presets write should return ConstraintError, because BuiltIn values do not match") - # Send the CommitPresetsSchedulesRequest command and expect UnsupportedAccess - await self.send_edit_atomic_request_commit_command(expected_status=Status.UnsupportedAccess) + # Clear state for next test. + await self.send_edit_atomic_request_rollback_command() self.step("8") - if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.C07.Rsp") and self.check_pics("TSTAT.S.C09.Rsp")): + if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.CFE.Rsp")): - # Send the AtomicRequest command + # Send the AtomicRequest begin command await self.send_edit_atomic_request_begin_command() # Write to the presets attribute after adding a preset with builtIn set to True @@ -271,15 +286,16 @@ async def test_TC_TSTAT_4_2(self): status = await self.write_presets(endpoint=endpoint, presets=test_presets) status_ok = (status == Status.Success) - asserts.assert_true(status_ok, "Presets write did not return Success as expected") + asserts.assert_equal(status, Status.ConstraintError, + "Presets write should return ConstraintError, since we are trying to add a new built-in preset") - # Send the CommitPresetsSchedulesRequest command and expect ConstraintError - await self.send_edit_atomic_request_commit_command(expected_status=Status.ConstraintError) + # Clear state for next test. + await self.send_edit_atomic_request_rollback_command() self.step("9") - if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.C07.Rsp") and self.check_pics("TSTAT.S.C09.Rsp")): + if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.CFE.Rsp")): - # Send the AtomicRequest command + # Send the AtomicRequest begin command await self.send_edit_atomic_request_begin_command() # Write to the presets attribute after adding a preset with a preset handle that doesn't exist in Presets attribute @@ -288,16 +304,16 @@ async def test_TC_TSTAT_4_2(self): name="Wake", coolingSetpoint=2800, heatingSetpoint=1800, builtIn=True)) status = await self.write_presets(endpoint=endpoint, presets=test_presets) - status_ok = (status == Status.Success) - asserts.assert_true(status_ok, "Presets write did not return Success as expected") + asserts.assert_equal(status, Status.NotFound, + "Presets write should return NotFound, since we are trying to modify non-existent preset") - # Send the CommitPresetsSchedulesRequest command and expect NotFound - await self.send_edit_atomic_request_commit_command(expected_status=Status.NotFound) + # Clear state for next test. + await self.send_edit_atomic_request_rollback_command() self.step("10") - if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.C07.Rsp") and self.check_pics("TSTAT.S.C09.Rsp")): + if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.CFE.Rsp")): - # Send the AtomicRequest command + # Send the AtomicRequest begin command await self.send_edit_atomic_request_begin_command() # Write to the presets attribute after adding a duplicate preset with handle (b'\x03') @@ -306,16 +322,16 @@ async def test_TC_TSTAT_4_2(self): presetHandle=b'\x03', presetScenario=cluster.Enums.PresetScenarioEnum.kSleep, name="Sleep", coolingSetpoint=2700, heatingSetpoint=1900, builtIn=False)) status = await self.write_presets(endpoint=endpoint, presets=test_presets) - status_ok = (status == Status.Success) - asserts.assert_true(status_ok, "Presets write did not return Success as expected") + asserts.assert_equal(status, Status.ConstraintError, + "Presets write should return ConstraintError, since we have duplicated presets") - # Send the CommitPresetsSchedulesRequest command and expect ConstraintError - await self.send_edit_atomic_request_commit_command(expected_status=Status.ConstraintError) + # Clear state for next test. + await self.send_edit_atomic_request_rollback_command() self.step("11") - if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.C07.Rsp") and self.check_pics("TSTAT.S.C09.Rsp")): + if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.CFE.Rsp")): - # Send the AtomicRequest command + # Send the AtomicRequest begin command await self.send_edit_atomic_request_begin_command() # Write to the presets attribute after setting the builtIn flag to True for preset with handle (b'\x03') @@ -323,16 +339,16 @@ async def test_TC_TSTAT_4_2(self): test_presets[2].builtIn = True status = await self.write_presets(endpoint=endpoint, presets=test_presets) - status_ok = (status == Status.Success) - asserts.assert_true(status_ok, "Presets write did not return Success as expected") + asserts.assert_equal(status, Status.ConstraintError, + "Presets write should return ConstraintError, since we are trying to change whether a preset is BuiltIn") - # Send the CommitPresetsSchedulesRequest command and expect UnsupportedAccess - await self.send_edit_atomic_request_commit_command(expected_status=Status.UnsupportedAccess) + # Clear state for next test. + await self.send_edit_atomic_request_rollback_command() self.step("12") - if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.C07.Rsp") and self.check_pics("TSTAT.S.C09.Rsp")): + if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.CFE.Rsp")): - # Send the AtomicRequest command + # Send the AtomicRequest begin command await self.send_edit_atomic_request_begin_command() # Write to the presets attribute after setting a name for preset with handle (b'\x01') that doesn't support names @@ -340,31 +356,31 @@ async def test_TC_TSTAT_4_2(self): test_presets[0].name = "Occupied" status = await self.write_presets(endpoint=endpoint, presets=test_presets) - status_ok = (status == Status.Success) - asserts.assert_true(status_ok, "Presets write did not return Success as expected") + asserts.assert_equal(status, Status.ConstraintError, + "Presets write should return ConstraintError, since we are trying to set a name for a preset that does not support that") - # Send the CommitPresetsSchedulesRequest command and expect ConstraintError - await self.send_edit_atomic_request_commit_command(expected_status=Status.ConstraintError) + # Clear state for next test. + await self.send_edit_atomic_request_rollback_command() self.step("13") - if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.C07.Rsp") and self.check_pics("TSTAT.S.C09.Rsp")): + if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050") and self.check_pics("TSTAT.S.CFE.Rsp")): - # Send the AtomicRequest command + # Send the AtomicRequest begin command await self.send_edit_atomic_request_begin_command() # Write to the presets attribute with a new valid preset added test_presets = copy.deepcopy(new_presets_with_handle) - test_presets.append(cluster.Structs.PresetStruct(presetHandle=b'\x04', presetScenario=cluster.Enums.PresetScenarioEnum.kWake, + test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=cluster.Enums.PresetScenarioEnum.kWake, name="Wake", coolingSetpoint=2800, heatingSetpoint=1800, builtIn=False)) status = await self.write_presets(endpoint=endpoint, presets=test_presets) status_ok = (status == Status.Success) - asserts.assert_true(status_ok, "Presets write did not return Success as expected") + asserts.assert_equal(status, Status.Success, "Presets write did not return Success as expected") - # Send the CancelPresetsSchedulesRequest command + # Roll back await self.send_edit_atomic_request_rollback_command() - # Send the CommitPresetsSchedulesRequest command and expect InvalidInState as the previous edit request was cancelled + # Send the AtomicRequest commit command and expect InvalidInState as the previous edit request was cancelled await self.send_edit_atomic_request_commit_command(expected_status=Status.InvalidInState) # TODO: Add tests for the total number of Presets exceeds the NumberOfPresets supported. Also Add tests for adding presets with preset scenario not present in PresetTypes.