diff --git a/examples/thermostat/linux/include/thermostat-delegate-impl.h b/examples/thermostat/linux/include/thermostat-delegate-impl.h index f559977f341949..8252f2274f9d79 100644 --- a/examples/thermostat/linux/include/thermostat-delegate-impl.h +++ b/examples/thermostat/linux/include/thermostat-delegate-impl.h @@ -44,6 +44,10 @@ class ThermostatDelegate : public Delegate public: static inline ThermostatDelegate & GetInstance() { return sInstance; } + std::optional + GetAtomicWriteTimeout(DataModel::DecodableList attributeRequests, + System::Clock::Milliseconds16 timeoutRequest) override; + CHIP_ERROR GetPresetTypeAtIndex(size_t index, Structs::PresetTypeStruct::Type & presetType) override; uint8_t GetNumberOfPresets() override; diff --git a/examples/thermostat/linux/thermostat-delegate-impl.cpp b/examples/thermostat/linux/thermostat-delegate-impl.cpp index 61d496f233b408..c39a757a8b0644 100644 --- a/examples/thermostat/linux/thermostat-delegate-impl.cpp +++ b/examples/thermostat/linux/thermostat-delegate-impl.cpp @@ -148,6 +148,47 @@ CHIP_ERROR ThermostatDelegate::SetActivePresetHandle(const DataModel::Nullable +ThermostatDelegate::GetAtomicWriteTimeout(DataModel::DecodableList attributeRequests, + System::Clock::Milliseconds16 timeoutRequest) +{ + auto attributeIdsIter = attributeRequests.begin(); + bool requestedPresets = false, requestedSchedules = false; + while (attributeIdsIter.Next()) + { + auto & attributeId = attributeIdsIter.GetValue(); + + switch (attributeId) + { + case Attributes::Presets::Id: + requestedPresets = true; + break; + case Attributes::Schedules::Id: + requestedSchedules = true; + break; + default: + return System::Clock::Milliseconds16(0); + } + } + if (attributeIdsIter.GetStatus() != CHIP_NO_ERROR) + { + return System::Clock::Milliseconds16(0); + } + auto timeout = System::Clock::Milliseconds16(0); + if (requestedPresets) + { + // If the client expects to edit the presets, then we'll give it 3 seconds to do so + timeout += std::chrono::milliseconds(3000); + } + if (requestedSchedules) + { + // If the client expects to edit the schedules, then we'll give it 9 seconds to do so + timeout += std::chrono::milliseconds(9000); + } + // If the client requested an even smaller timeout, then use that one + return std::min(timeoutRequest, timeout); +} + void ThermostatDelegate::InitializePendingPresets() { mNextFreeIndexInPendingPresetsList = 0; diff --git a/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h b/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h index bb4e1a77c14f59..3146dd902ffb68 100644 --- a/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h +++ b/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h @@ -2217,6 +2217,7 @@ }; \ const EmberAfGenericClusterFunction chipFuncArrayThermostatServer[] = { \ (EmberAfGenericClusterFunction) emberAfThermostatClusterServerInitCallback, \ + (EmberAfGenericClusterFunction) MatterThermostatClusterServerShutdownCallback, \ (EmberAfGenericClusterFunction) MatterThermostatClusterServerPreAttributeChangedCallback, \ }; \ const EmberAfGenericClusterFunction chipFuncArrayFanControlServer[] = { \ @@ -3755,7 +3756,7 @@ .attributes = ZAP_ATTRIBUTE_INDEX(616), \ .attributeCount = 26, \ .clusterSize = 72, \ - .mask = ZAP_CLUSTER_MASK(SERVER) | ZAP_CLUSTER_MASK(INIT_FUNCTION) | ZAP_CLUSTER_MASK(PRE_ATTRIBUTE_CHANGED_FUNCTION), \ + .mask = ZAP_CLUSTER_MASK(SERVER) | ZAP_CLUSTER_MASK(INIT_FUNCTION) | ZAP_CLUSTER_MASK(SHUTDOWN_FUNCTION) | ZAP_CLUSTER_MASK(PRE_ATTRIBUTE_CHANGED_FUNCTION), \ .functions = chipFuncArrayThermostatServer, \ .acceptedCommandList = ZAP_GENERATED_COMMANDS_INDEX( 241 ), \ .generatedCommandList = ZAP_GENERATED_COMMANDS_INDEX( 246 ), \ diff --git a/src/app/clusters/thermostat-server/thermostat-delegate.h b/src/app/clusters/thermostat-server/thermostat-delegate.h index 0c09b9dd4d70fc..c8c21d898af167 100644 --- a/src/app/clusters/thermostat-server/thermostat-delegate.h +++ b/src/app/clusters/thermostat-server/thermostat-delegate.h @@ -38,6 +38,17 @@ class Delegate virtual ~Delegate() = default; + /** + * @brief Get the maximum timeout for atomically writing to a set of attributes + * + * @param[in] attributeRequests The list of attributes to write to. + * @param[out] timeoutRequest The timeout proposed by the client. + * @return The maximum allowed timeout; zero if the request is invalid. + */ + virtual std::optional + GetAtomicWriteTimeout(DataModel::DecodableList attributeRequests, + System::Clock::Milliseconds16 timeoutRequest) = 0; + /** * @brief Get the preset type at a given index in the PresetTypes attribute * diff --git a/src/app/clusters/thermostat-server/thermostat-server.cpp b/src/app/clusters/thermostat-server/thermostat-server.cpp index 6b8a50a6ffe7e0..91c045c5ff540e 100644 --- a/src/app/clusters/thermostat-server/thermostat-server.cpp +++ b/src/app/clusters/thermostat-server/thermostat-server.cpp @@ -27,6 +27,8 @@ #include #include #include +#include +#include #include #include @@ -116,8 +118,7 @@ void TimerExpiredCallback(System::Layer * systemLayer, void * callbackContext) VerifyOrReturn(delegate != nullptr, ChipLogError(Zcl, "Delegate is null. Unable to handle timer expired")); delegate->ClearPendingPresetList(); - gThermostatAttrAccess.SetAtomicWrite(endpoint, false); - gThermostatAttrAccess.SetAtomicWriteScopedNodeId(endpoint, ScopedNodeId()); + gThermostatAttrAccess.SetAtomicWrite(endpoint, ScopedNodeId(), kAtomicWriteState_Closed); } /** @@ -205,8 +206,7 @@ void resetAtomicWrite(Delegate * delegate, EndpointId endpoint) delegate->ClearPendingPresetList(); } ClearTimer(endpoint); - gThermostatAttrAccess.SetAtomicWrite(endpoint, false); - gThermostatAttrAccess.SetAtomicWriteScopedNodeId(endpoint, ScopedNodeId()); + gThermostatAttrAccess.SetAtomicWrite(endpoint, ScopedNodeId(), kAtomicWriteState_Closed); } /** @@ -605,14 +605,16 @@ void SetDefaultDelegate(EndpointId endpoint, Delegate * delegate) } } -void ThermostatAttrAccess::SetAtomicWrite(EndpointId endpoint, bool inProgress) +void ThermostatAttrAccess::SetAtomicWrite(EndpointId endpoint, ScopedNodeId originatorNodeId, AtomicWriteState state) { uint16_t ep = emberAfGetClusterServerEndpointIndex(endpoint, Thermostat::Id, MATTER_DM_THERMOSTAT_CLUSTER_SERVER_ENDPOINT_COUNT); - if (ep < ArraySize(mAtomicWriteState)) + if (ep < ArraySize(mAtomicWriteSessions)) { - mAtomicWriteState[ep] = inProgress; + mAtomicWriteSessions[ep].state = state; + mAtomicWriteSessions[ep].endpointId = endpoint; + mAtomicWriteSessions[ep].nodeId = originatorNodeId; } } @@ -622,9 +624,9 @@ bool ThermostatAttrAccess::InAtomicWrite(EndpointId endpoint) uint16_t ep = emberAfGetClusterServerEndpointIndex(endpoint, Thermostat::Id, MATTER_DM_THERMOSTAT_CLUSTER_SERVER_ENDPOINT_COUNT); - if (ep < ArraySize(mAtomicWriteState)) + if (ep < ArraySize(mAtomicWriteSessions)) { - inAtomicWrite = mAtomicWriteState[ep]; + inAtomicWrite = (mAtomicWriteSessions[ep].state == kAtomicWriteState_Open); } return inAtomicWrite; } @@ -649,26 +651,15 @@ bool ThermostatAttrAccess::InAtomicWrite(CommandHandler * commandObj, EndpointId return GetAtomicWriteScopedNodeId(endpoint) == sourceNodeId; } -void ThermostatAttrAccess::SetAtomicWriteScopedNodeId(EndpointId endpoint, ScopedNodeId originatorNodeId) -{ - uint16_t ep = - emberAfGetClusterServerEndpointIndex(endpoint, Thermostat::Id, MATTER_DM_THERMOSTAT_CLUSTER_SERVER_ENDPOINT_COUNT); - - if (ep < ArraySize(mAtomicWriteNodeIds)) - { - mAtomicWriteNodeIds[ep] = originatorNodeId; - } -} - ScopedNodeId ThermostatAttrAccess::GetAtomicWriteScopedNodeId(EndpointId endpoint) { ScopedNodeId originatorNodeId = ScopedNodeId(); uint16_t ep = emberAfGetClusterServerEndpointIndex(endpoint, Thermostat::Id, MATTER_DM_THERMOSTAT_CLUSTER_SERVER_ENDPOINT_COUNT); - if (ep < ArraySize(mAtomicWriteNodeIds)) + if (ep < ArraySize(mAtomicWriteSessions)) { - originatorNodeId = mAtomicWriteNodeIds[ep]; + originatorNodeId = mAtomicWriteSessions[ep].nodeId; } return originatorNodeId; } @@ -704,7 +695,7 @@ CHIP_ERROR ThermostatAttrAccess::Read(const ConcreteReadAttributePath & aPath, A } break; case PresetTypes::Id: { - Delegate * delegate = GetDelegate(aPath.mEndpointId); + auto delegate = GetDelegate(aPath.mEndpointId); VerifyOrReturnError(delegate != nullptr, CHIP_ERROR_INCORRECT_STATE, ChipLogError(Zcl, "Delegate is null")); return aEncoder.EncodeList([delegate](const auto & encoder) -> CHIP_ERROR { @@ -723,14 +714,14 @@ CHIP_ERROR ThermostatAttrAccess::Read(const ConcreteReadAttributePath & aPath, A } break; case NumberOfPresets::Id: { - Delegate * delegate = GetDelegate(aPath.mEndpointId); + auto delegate = GetDelegate(aPath.mEndpointId); VerifyOrReturnError(delegate != nullptr, CHIP_ERROR_INCORRECT_STATE, ChipLogError(Zcl, "Delegate is null")); ReturnErrorOnFailure(aEncoder.Encode(delegate->GetNumberOfPresets())); } break; case Presets::Id: { - Delegate * delegate = GetDelegate(aPath.mEndpointId); + auto delegate = GetDelegate(aPath.mEndpointId); VerifyOrReturnError(delegate != nullptr, CHIP_ERROR_INCORRECT_STATE, ChipLogError(Zcl, "Delegate is null")); auto & subjectDescriptor = aEncoder.GetSubjectDescriptor(); @@ -766,7 +757,7 @@ CHIP_ERROR ThermostatAttrAccess::Read(const ConcreteReadAttributePath & aPath, A } break; case ActivePresetHandle::Id: { - Delegate * delegate = GetDelegate(aPath.mEndpointId); + auto delegate = GetDelegate(aPath.mEndpointId); VerifyOrReturnError(delegate != nullptr, CHIP_ERROR_INCORRECT_STATE, ChipLogError(Zcl, "Delegate is null")); uint8_t buffer[kPresetHandleSize]; @@ -812,7 +803,7 @@ CHIP_ERROR ThermostatAttrAccess::Write(const ConcreteDataAttributePath & aPath, { case Presets::Id: { - Delegate * delegate = GetDelegate(endpoint); + auto delegate = GetDelegate(endpoint); VerifyOrReturnError(delegate != nullptr, CHIP_ERROR_INCORRECT_STATE, ChipLogError(Zcl, "Delegate is null")); // Presets are not editable, return INVALID_IN_STATE. @@ -897,7 +888,7 @@ CHIP_ERROR ThermostatAttrAccess::Write(const ConcreteDataAttributePath & aPath, return CHIP_NO_ERROR; } -CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Delegate * delegate, const PresetStruct::Type & preset) +CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Thermostat::Delegate * delegate, const PresetStruct::Type & preset) { if (!IsValidPresetEntry(preset)) { @@ -951,6 +942,23 @@ CHIP_ERROR ThermostatAttrAccess::AppendPendingPreset(Delegate * delegate, const return delegate->AppendToPendingPresetList(preset); } +void ThermostatAttrAccess::OnFabricRemoved(const FabricTable & fabricTable, FabricIndex fabricIndex) +{ + for (size_t i = 0; i < ArraySize(mAtomicWriteSessions); ++i) + { + auto atomicWriteState = mAtomicWriteSessions[i]; + if (atomicWriteState.state == kAtomicWriteState_Open && atomicWriteState.nodeId.GetFabricIndex() == fabricIndex) + { + auto delegate = GetDelegate(atomicWriteState.endpointId); + if (delegate == nullptr) + { + continue; + } + resetAtomicWrite(delegate, atomicWriteState.endpointId); + } + } +} + } // namespace Thermostat } // namespace Clusters } // namespace app @@ -1394,8 +1402,6 @@ void handleAtomicBegin(CommandHandler * commandObj, const ConcreteCommandPath & return; } - auto timeout = commandData.timeout.Value(); - if (!validAtomicAttributes(commandData, false)) { commandObj->AddStatus(commandPath, imcode::InvalidCommand); @@ -1412,13 +1418,18 @@ void handleAtomicBegin(CommandHandler * commandObj, const ConcreteCommandPath & // needs to keep track of a pending preset list now. delegate->InitializePendingPresets(); - uint16_t maxTimeout = 5000; - timeout = std::min(timeout, maxTimeout); + auto timeout = + delegate->GetAtomicWriteTimeout(commandData.attributeRequests, System::Clock::Milliseconds16(commandData.timeout.Value())); - ScheduleTimer(endpoint, System::Clock::Milliseconds16(timeout)); - gThermostatAttrAccess.SetAtomicWrite(endpoint, true); - gThermostatAttrAccess.SetAtomicWriteScopedNodeId(endpoint, GetSourceScopedNodeId(commandObj)); - sendAtomicResponse(commandObj, commandPath, imcode::Success, imcode::Success, imcode::Success, MakeOptional(timeout)); + if (!timeout.has_value()) + { + commandObj->AddStatus(commandPath, imcode::InvalidCommand); + return; + } + ScheduleTimer(endpoint, timeout.value()); + gThermostatAttrAccess.SetAtomicWrite(endpoint, GetSourceScopedNodeId(commandObj), kAtomicWriteState_Open); + sendAtomicResponse(commandObj, commandPath, imcode::Success, imcode::Success, imcode::Success, + MakeOptional(timeout.value().count())); } imcode commitPresets(Delegate * delegate, EndpointId endpoint) @@ -1868,5 +1879,17 @@ bool emberAfThermostatClusterSetpointRaiseLowerCallback(app::CommandHandler * co void MatterThermostatPluginServerInitCallback() { + Server::GetInstance().GetFabricTable().AddFabricDelegate(&gThermostatAttrAccess); AttributeAccessInterfaceRegistry::Instance().Register(&gThermostatAttrAccess); } + +void MatterThermostatClusterServerShutdownCallback(EndpointId endpoint) +{ + ChipLogProgress(Zcl, "Shutting down thermostat server cluster on endpoint %d", endpoint); + Delegate * delegate = GetDelegate(endpoint); + + if (delegate != nullptr) + { + resetAtomicWrite(delegate, endpoint); + } +} diff --git a/src/app/clusters/thermostat-server/thermostat-server.h b/src/app/clusters/thermostat-server/thermostat-server.h index 306a2a625c57b6..ddede8a9bb13f9 100644 --- a/src/app/clusters/thermostat-server/thermostat-server.h +++ b/src/app/clusters/thermostat-server/thermostat-server.h @@ -37,10 +37,15 @@ namespace Thermostat { static constexpr size_t kThermostatEndpointCount = MATTER_DM_THERMOSTAT_CLUSTER_SERVER_ENDPOINT_COUNT + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT; +enum AtomicWriteState +{ + kAtomicWriteState_Closed = 0, + kAtomicWriteState_Open, +}; /** * @brief Thermostat Attribute Access Interface. */ -class ThermostatAttrAccess : public chip::app::AttributeAccessInterface +class ThermostatAttrAccess : public chip::app::AttributeAccessInterface, public chip::FabricTable::Delegate { public: ThermostatAttrAccess() : AttributeAccessInterface(Optional::Missing(), Thermostat::Id) {} @@ -48,15 +53,6 @@ class ThermostatAttrAccess : public chip::app::AttributeAccessInterface CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; CHIP_ERROR Write(const ConcreteDataAttributePath & aPath, chip::app::AttributeValueDecoder & aDecoder) override; - /** - * @brief Sets the scoped node id of the originator that sent the last successful - * AtomicRequest of type BeginWrite for the given endpoint. - * - * @param[in] endpoint The endpoint. - * @param[in] originatorNodeId The originator scoped node id. - */ - void SetAtomicWriteScopedNodeId(EndpointId endpoint, ScopedNodeId originatorNodeId); - /** * @brief Gets the scoped node id of the originator that sent the last successful * AtomicRequest of type BeginWrite for the given endpoint. @@ -68,12 +64,13 @@ class ThermostatAttrAccess : public chip::app::AttributeAccessInterface ScopedNodeId GetAtomicWriteScopedNodeId(EndpointId endpoint); /** - * @brief Sets whether an atomic write is in progress for the given endpoint + * @brief Sets the atomic write state for the given endpoint and originatorNodeId * * @param[in] endpoint The endpoint. - * @param[in] inProgress Whether or not an atomic write is in progress. + * @param[in] originatorNodeId The originator scoped node id. + * @param[in] state Whether or not an atomic write is open or closed. */ - void SetAtomicWrite(EndpointId endpoint, bool inProgress); + void SetAtomicWrite(EndpointId endpoint, ScopedNodeId originatorNodeId, AtomicWriteState state); /** * @brief Gets whether an atomic write is in progress for the given endpoint @@ -105,10 +102,18 @@ class ThermostatAttrAccess : public chip::app::AttributeAccessInterface bool InAtomicWrite(CommandHandler * commandObj, EndpointId endpoint); private: - CHIP_ERROR AppendPendingPreset(Delegate * delegate, const Structs::PresetStruct::Type & preset); + CHIP_ERROR AppendPendingPreset(Thermostat::Delegate * delegate, const Structs::PresetStruct::Type & preset); + + void OnFabricRemoved(const FabricTable & fabricTable, FabricIndex fabricIndex) override; + + struct AtomicWriteSession + { + AtomicWriteState state = kAtomicWriteState_Closed; + ScopedNodeId nodeId; + EndpointId endpointId = kInvalidEndpointId; + }; - ScopedNodeId mAtomicWriteNodeIds[kThermostatEndpointCount]; - bool mAtomicWriteState[kThermostatEndpointCount]; + AtomicWriteSession mAtomicWriteSessions[kThermostatEndpointCount]; }; /** diff --git a/src/app/common/templates/config-data.yaml b/src/app/common/templates/config-data.yaml index d8d276f53a0700..660ca0b3aa1df4 100644 --- a/src/app/common/templates/config-data.yaml +++ b/src/app/common/templates/config-data.yaml @@ -81,6 +81,7 @@ ClustersWithShutdownFunctions: - Color Control - Sample MEI - Scenes Management + - Thermostat ClustersWithPreAttributeChangeFunctions: - Door Lock diff --git a/src/python_testing/TC_TSTAT_4_2.py b/src/python_testing/TC_TSTAT_4_2.py index 58d78ef4491a73..9cb7336212b1e5 100644 --- a/src/python_testing/TC_TSTAT_4_2.py +++ b/src/python_testing/TC_TSTAT_4_2.py @@ -30,6 +30,7 @@ import logging import chip.clusters as Clusters +from chip import ChipDeviceCtrl # Needed before chip.FabricAdmin from chip.clusters import Globals from chip.clusters.Types import NullValue from chip.interaction_model import InteractionModelError, Status @@ -58,53 +59,76 @@ class TC_TSTAT_4_2(MatterBaseTest): - async def write_presets(self, endpoint, presets) -> Status: - result = await self.default_controller.WriteAttribute(self.dut_node_id, [(endpoint, cluster.Attributes.Presets(presets))]) - return result[0].Status - - async def send_edit_atomic_request_begin_command(self, - endpoint: int = None, - expected_status: Status = Status.Success): + def check_atomic_response(self, response: object, expected_status: Status = Status.Success, + expected_overall_status: Status = Status.Success, + expected_preset_status: Status = Status.Success): + asserts.assert_equal(expected_status, Status.Success, "We expected we had a valid response") + asserts.assert_equal(response.statusCode, expected_overall_status, "Response 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 should have the right status") + found_preset_status = True + asserts.assert_true(found_preset_status, "Preset attribute should have a status") + + async def write_presets(self, + endpoint, + presets, + dev_ctrl: ChipDeviceCtrl = None, + expected_status: Status = Status.Success) -> Status: + if dev_ctrl is None: + dev_ctrl = self.default_controller + result = await dev_ctrl.WriteAttribute(self.dut_node_id, [(endpoint, cluster.Attributes.Presets(presets))]) + status = result[0].Status + asserts.assert_equal(status, expected_status, f"Presets write returned {status.name}; expected {expected_status.name}") + return status + + async def send_atomic_request_begin_command(self, + dev_ctrl: ChipDeviceCtrl = None, + 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=Globals.Enums.AtomicRequestTypeEnum.kBeginWrite, - attributeRequests=[ - cluster.Attributes.Presets.attribute_id], - timeout=1800), - endpoint=endpoint) - asserts.assert_equal(expected_status, Status.Success) + response = await self.send_single_cmd(cmd=cluster.Commands.AtomicRequest(requestType=Globals.Enums.AtomicRequestTypeEnum.kBeginWrite, + attributeRequests=[ + cluster.Attributes.Presets.attribute_id], + timeout=1800), + dev_ctrl=dev_ctrl, + endpoint=endpoint) + self.check_atomic_response(response, expected_status, expected_overall_status, expected_preset_status) except InteractionModelError as e: 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, - expected_overall_status: Status = Status.Success, - expected_preset_status: Status = Status.Success): + async def send_atomic_request_commit_command(self, + dev_ctrl: ChipDeviceCtrl = None, + endpoint: int = None, + expected_status: Status = Status.Success, + expected_overall_status: Status = Status.Success, + expected_preset_status: Status = Status.Success): try: response = await self.send_single_cmd(cmd=cluster.Commands.AtomicRequest(requestType=Globals.Enums.AtomicRequestTypeEnum.kCommitWrite, attributeRequests=[cluster.Attributes.Presets.attribute_id, cluster.Attributes.Schedules.attribute_id]), + dev_ctrl=dev_ctrl, 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") + self.check_atomic_response(response, expected_status, expected_overall_status, expected_preset_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): + async def send_atomic_request_rollback_command(self, + dev_ctrl: ChipDeviceCtrl = None, + 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=Globals.Enums.AtomicRequestTypeEnum.kRollbackWrite, - 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=Globals.Enums.AtomicRequestTypeEnum.kRollbackWrite, + attributeRequests=[cluster.Attributes.Presets.attribute_id, cluster.Attributes.Schedules.attribute_id]), + dev_ctrl=dev_ctrl, + endpoint=endpoint) + self.check_atomic_response(response, expected_status, expected_overall_status, expected_preset_status) except InteractionModelError as e: asserts.assert_equal(e.status, expected_status, "Unexpected error returned") @@ -134,39 +158,57 @@ def steps_TC_TSTAT_4_2(self) -> list[TestStep]: is_commissioning=True), TestStep("2", "TH writes to the Presets attribute without calling the AtomicRequest command", " Verify that the write request returns INVALID_IN_STATE error since the client didn't send a request to edit the presets by calling AtomicRequest command."), - TestStep("3", "TH writes to the Presets attribute after calling the AtomicRequest command but doesn't call CommitPresetsSchedulesRequest to commit", - "Verify that the Presets attribute was not updated since CommitPresetsSchedulesRequest command was not called."), - TestStep("4", "TH writes to the Presets attribute after calling the AtomicRequest command and calls CommitPresetsSchedulesRequest to commit", + TestStep("3", "TH writes to the Presets attribute after calling the AtomicRequest begin command but doesn't call AtomicRequest commit", + "Verify that the Presets attribute was not updated since AtomicRequest commit command was not called."), + TestStep("4", "TH writes to the Presets attribute after calling the AtomicRequest begin command and calls AtomicRequest commit", "Verify that the Presets attribute was updated with new presets."), TestStep("5", "TH writes to the Presets attribute with a built-in preset removed", - "Verify that the CommitPresetsSchedulesRequest returned UNSUPPORTED_ACCESS (0x7e)."), + "Verify that the AtomicRequest commit returned CONSTRAINT_ERROR (0x87)."), TestStep("6", "TH writes to the Presets attribute with a preset removed whose handle matches the value in the ActivePresetHandle attribute", - "Verify that the CommitPresetsSchedulesRequest returned INVALID_IN_STATE (0xcb)."), + "Verify that the AtomicRequest commit returned INVALID_IN_STATE (0xcb)."), TestStep("7", "TH writes to the Presets attribute with a built-in preset modified to be not built-in", - "Verify that the CommitPresetsSchedulesRequest returned UNSUPPORTED_ACCESS (0x7e)."), + "Verify that the AtomicRequest commit returned CONSTRAINT_ERROR (0x87)."), TestStep("8", "TH writes to the Presets attribute with a new preset having builtIn set to true", - "Verify that the CommitPresetsSchedulesRequest returned CONSTRAINT_ERROR (0x87)."), + "Verify that the AtomicRequest commit returned CONSTRAINT_ERROR (0x87)."), TestStep("9", "TH writes to the Presets attribute with a new preset having a preset handle that doesn't exist in the Presets attribute", - "Verify that the CommitPresetsSchedulesRequest returned NOT_FOUND (0x8b)."), + "Verify that the AtomicRequest commit returned NOT_FOUND (0x8b)."), TestStep("10", "TH writes to the Presets attribute with duplicate presets", - "Verify that the CommitPresetsSchedulesRequest returned CONSTRAINT_ERROR (0x87)."), + "Verify that the AtomicRequest commit returned CONSTRAINT_ERROR (0x87)."), TestStep("11", "TH writes to the Presets attribute with a non built-in preset modified to be built-in", - "Verify that the CommitPresetsSchedulesRequest returned UNSUPPORTED_ACCESS (0x7e)."), + "Verify that the AtomicRequest commit returned CONSTRAINT_ERROR (0x87)."), 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 AtomicRequest commit returned CONSTRAINT_ERROR (0x87)."), + TestStep("13", "TH writes to the Presets attribute but calls the AtomicRequest rollback command to cancel the edit request", + "Verify that the edit request was rolled back"), + TestStep("14", "TH starts an atomic write, and TH2 attempts to open an atomic write before TH is complete", + "Verify that the atomic request is rejected"), + TestStep("15", "TH starts an atomic write, and TH2 attempts to write to presets", + "Verify that the write request is rejected"), + TestStep("16", "TH starts an atomic write, and before it's complete, TH2 removes TH's fabric; TH2 then opens an atomic write", + "Verify that the atomic request is successful"), ] return steps - @async_test_body + @ async_test_body async def test_TC_TSTAT_4_2(self): endpoint = self.user_params.get("endpoint", 1) self.step("1") # Commission DUT - already done + logger.info("Commissioning under second controller") + params = await self.default_controller.OpenCommissioningWindow( + nodeid=self.dut_node_id, timeout=600, iteration=10000, discriminator=1234, option=1) + + secondary_authority = self.certificate_authority_manager.NewCertificateAuthority() + secondary_fabric_admin = secondary_authority.NewFabricAdmin(vendorId=0xFFF1, fabricId=2) + secondary_controller = secondary_fabric_admin.NewController(nodeId=112233) + + await secondary_controller.CommissionOnNetwork( + nodeId=self.dut_node_id, setupPinCode=params.setupPinCode, + filterType=ChipDeviceCtrl.DiscoveryFilterType.LONG_DISCRIMINATOR, filter=1234) + self.step("2") if self.pics_guard(self.check_pics("TSTAT.S.F08") and self.check_pics("TSTAT.S.A0050")): presets = await self.read_single_attribute_check_success(endpoint=endpoint, cluster=cluster, attribute=cluster.Attributes.Presets) @@ -174,13 +216,11 @@ async def test_TC_TSTAT_4_2(self): asserts.assert_equal(presets, initial_presets, "Presets do not match initial value") # Write to the presets attribute without calling AtomicRequest command - status = await self.write_presets(endpoint=endpoint, presets=new_presets) - status_ok = (status == Status.InvalidInState) - asserts.assert_true(status_ok, "Presets write did not return InvalidInState as expected") + await self.write_presets(endpoint=endpoint, presets=new_presets, expected_status=Status.InvalidInState) 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.CFE.Rsp")): - await self.send_edit_atomic_request_begin_command() + await self.send_atomic_request_begin_command() # Write to the presets attribute after calling AtomicRequest command status = await self.write_presets(endpoint=endpoint, presets=new_presets) @@ -192,7 +232,7 @@ async def test_TC_TSTAT_4_2(self): logger.info(f"Rx'd Presets: {presets}") asserts.assert_equal(presets, new_presets_with_handle, "Presets were updated, as expected") - await self.send_edit_atomic_request_rollback_command() + await self.send_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) @@ -202,15 +242,13 @@ async def test_TC_TSTAT_4_2(self): 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 begin command - await self.send_edit_atomic_request_begin_command() + await self.send_atomic_request_begin_command() # Write to the presets attribute after calling AtomicRequest command - status = await self.write_presets(endpoint=endpoint, presets=new_presets) - status_ok = (status == Status.Success) - asserts.assert_true(status_ok, "Presets write did not return Success as expected") + await self.write_presets(endpoint=endpoint, presets=new_presets) # Send the AtomicRequest commit command - await self.send_edit_atomic_request_commit_command() + await self.send_atomic_request_commit_command() # 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) @@ -221,17 +259,15 @@ async def test_TC_TSTAT_4_2(self): 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 begin command - await self.send_edit_atomic_request_begin_command() + await self.send_atomic_request_begin_command() # Write to the presets attribute after removing a built in preset from the list. Remove the first entry. test_presets = new_presets_with_handle.copy() test_presets.pop(0) - 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") + await self.write_presets(endpoint=endpoint, presets=test_presets) # 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) + await self.send_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.CFE.Rsp")): @@ -245,144 +281,164 @@ async def test_TC_TSTAT_4_2(self): asserts.assert_equal(activePresetHandle, b'\x03', "Active preset handle was not updated as expected") # Send the AtomicRequest begin command - await self.send_edit_atomic_request_begin_command() + await self.send_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') test_presets = new_presets_with_handle.copy() del test_presets[-1] - 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") + await self.write_presets(endpoint=endpoint, presets=test_presets) # 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) + await self.send_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.CFE.Rsp")): # Send the AtomicRequest begin command - await self.send_edit_atomic_request_begin_command() + await self.send_atomic_request_begin_command() # Write to the presets attribute after setting the builtIn flag to False for preset with handle (b'\x01') test_presets = copy.deepcopy(new_presets_with_handle) test_presets[0].builtIn = False - status = await self.write_presets(endpoint=endpoint, presets=test_presets) - asserts.assert_equal(status, Status.ConstraintError, - "Presets write should return ConstraintError, because BuiltIn values do not match") + await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ConstraintError) # Clear state for next test. - await self.send_edit_atomic_request_rollback_command() + await self.send_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.CFE.Rsp")): # Send the AtomicRequest begin command - await self.send_edit_atomic_request_begin_command() + await self.send_atomic_request_begin_command() # Write to the presets attribute after adding a preset with builtIn set to True test_presets = copy.deepcopy(new_presets_with_handle) test_presets.append(cluster.Structs.PresetStruct(presetHandle=NullValue, presetScenario=cluster.Enums.PresetScenarioEnum.kWake, 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_equal(status, Status.ConstraintError, - "Presets write should return ConstraintError, since we are trying to add a new built-in preset") + status = await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ConstraintError) # Clear state for next test. - await self.send_edit_atomic_request_rollback_command() + await self.send_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.CFE.Rsp")): # Send the AtomicRequest begin command - await self.send_edit_atomic_request_begin_command() + await self.send_atomic_request_begin_command() # Write to the presets attribute after adding a preset with a preset handle that doesn't exist in Presets attribute test_presets = copy.deepcopy(new_presets_with_handle) test_presets.append(cluster.Structs.PresetStruct(presetHandle=b'\x08', presetScenario=cluster.Enums.PresetScenarioEnum.kWake, name="Wake", coolingSetpoint=2800, heatingSetpoint=1800, builtIn=True)) - status = await self.write_presets(endpoint=endpoint, presets=test_presets) - asserts.assert_equal(status, Status.NotFound, - "Presets write should return NotFound, since we are trying to modify non-existent preset") + status = await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.NotFound) # Clear state for next test. - await self.send_edit_atomic_request_rollback_command() + await self.send_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.CFE.Rsp")): # Send the AtomicRequest begin command - await self.send_edit_atomic_request_begin_command() + await self.send_atomic_request_begin_command() # Write to the presets attribute after adding a duplicate preset with handle (b'\x03') test_presets = copy.deepcopy(new_presets_with_handle) test_presets.append(cluster.Structs.PresetStruct( 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) - asserts.assert_equal(status, Status.ConstraintError, - "Presets write should return ConstraintError, since we have duplicated presets") + await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ConstraintError) # Clear state for next test. - await self.send_edit_atomic_request_rollback_command() + await self.send_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.CFE.Rsp")): # Send the AtomicRequest begin command - await self.send_edit_atomic_request_begin_command() + await self.send_atomic_request_begin_command() # Write to the presets attribute after setting the builtIn flag to True for preset with handle (b'\x03') test_presets = copy.deepcopy(new_presets_with_handle) test_presets[2].builtIn = True - status = await self.write_presets(endpoint=endpoint, presets=test_presets) - asserts.assert_equal(status, Status.ConstraintError, - "Presets write should return ConstraintError, since we are trying to change whether a preset is BuiltIn") + await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ConstraintError) # Clear state for next test. - await self.send_edit_atomic_request_rollback_command() + await self.send_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.CFE.Rsp")): # Send the AtomicRequest begin command - await self.send_edit_atomic_request_begin_command() + await self.send_atomic_request_begin_command() # Write to the presets attribute after setting a name for preset with handle (b'\x01') that doesn't support names test_presets = copy.deepcopy(new_presets_with_handle) test_presets[0].name = "Occupied" - status = await self.write_presets(endpoint=endpoint, presets=test_presets) - 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") + await self.write_presets(endpoint=endpoint, presets=test_presets, expected_status=Status.ConstraintError) # Clear state for next test. - await self.send_edit_atomic_request_rollback_command() + await self.send_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.CFE.Rsp")): # Send the AtomicRequest begin command - await self.send_edit_atomic_request_begin_command() + await self.send_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=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_equal(status, Status.Success, "Presets write did not return Success as expected") + await self.write_presets(endpoint=endpoint, presets=test_presets) # Roll back - await self.send_edit_atomic_request_rollback_command() + await self.send_atomic_request_rollback_command() # 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) + await self.send_atomic_request_commit_command(expected_status=Status.InvalidInState) + + self.step("14") + 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 begin command + await self.send_atomic_request_begin_command() + + # Send the AtomicRequest begin command from separate controller, which should receive busy + status = await self.send_atomic_request_begin_command(dev_ctrl=secondary_controller, expected_overall_status=Status.Failure, expected_preset_status=Status.Busy) + + # Roll back + await self.send_atomic_request_rollback_command() + + self.step("15") + 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 begin command from the secondary controller + await self.send_atomic_request_begin_command() + + await self.write_presets(endpoint=endpoint, presets=test_presets, dev_ctrl=secondary_controller, expected_status=Status.Busy) + + # Roll back + await self.send_atomic_request_rollback_command() + + self.step("16") + 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 begin command from the secondary controller + await self.send_atomic_request_begin_command(dev_ctrl=secondary_controller) + + # Primary controller removes the second fabric + await self.send_single_cmd(Clusters.OperationalCredentials.Commands.RemoveFabric(fabricIndex=2), endpoint=0) + + # Send the AtomicRequest begin command from primary controller, which should succeed, as the secondary controller's atomic write state has been cleared + status = await self.send_atomic_request_begin_command() + + # Roll back + await self.send_atomic_request_rollback_command() # 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.