diff --git a/src/app/clusters/operational-state-server/operational-state-server.cpp b/src/app/clusters/operational-state-server/operational-state-server.cpp index b6ae0d7a221253..c86de02bfab6ae 100644 --- a/src/app/clusters/operational-state-server/operational-state-server.cpp +++ b/src/app/clusters/operational-state-server/operational-state-server.cpp @@ -29,6 +29,7 @@ #include #include #include +#include using namespace chip; using namespace chip::app; @@ -43,6 +44,9 @@ Instance::Instance(Delegate * aDelegate, EndpointId aEndpointId, ClusterId aClus mDelegate(aDelegate), mEndpointId(aEndpointId), mClusterId(aClusterId) { mDelegate->SetInstance(this); + mCountdownTime.policy() + .Set(QuieterReportingPolicyEnum::kMarkDirtyOnIncrement) + .Set(QuieterReportingPolicyEnum::kMarkDirtyOnChangeToFromZero); } Instance::Instance(Delegate * aDelegate, EndpointId aEndpointId) : Instance(aDelegate, aEndpointId, OperationalState::Id) {} @@ -84,6 +88,7 @@ CHIP_ERROR Instance::SetCurrentPhase(const DataModel::Nullable & aPhase if (mCurrentPhase != oldPhase) { MatterReportingAttributeChangeCallback(mEndpointId, mClusterId, Attributes::CurrentPhase::Id); + UpdateCountdownTimeFromClusterLogic(); } return CHIP_NO_ERROR; } @@ -96,9 +101,11 @@ CHIP_ERROR Instance::SetOperationalState(uint8_t aOpState) return CHIP_ERROR_INVALID_ARGUMENT; } + bool countdownTimeUpdateNeeded = false; if (mOperationalError.errorStateID != to_underlying(ErrorStateEnum::kNoError)) { mOperationalError.Set(to_underlying(ErrorStateEnum::kNoError)); + countdownTimeUpdateNeeded = true; MatterReportingAttributeChangeCallback(mEndpointId, mClusterId, Attributes::OperationalError::Id); } @@ -107,6 +114,12 @@ CHIP_ERROR Instance::SetOperationalState(uint8_t aOpState) if (mOperationalState != oldState) { MatterReportingAttributeChangeCallback(mEndpointId, mClusterId, Attributes::OperationalState::Id); + countdownTimeUpdateNeeded = true; + } + + if (countdownTimeUpdateNeeded) + { + UpdateCountdownTimeFromClusterLogic(); } return CHIP_NO_ERROR; } @@ -143,6 +156,8 @@ void Instance::OnOperationalErrorDetected(const Structs::ErrorStateStruct::Type MatterReportingAttributeChangeCallback(mEndpointId, mClusterId, Attributes::OperationalError::Id); } + UpdateCountdownTimeFromClusterLogic(); + // Generate an ErrorDetected event GenericErrorEvent event(mClusterId, aError); EventNumber eventNumber; @@ -156,7 +171,7 @@ void Instance::OnOperationalErrorDetected(const Structs::ErrorStateStruct::Type void Instance::OnOperationCompletionDetected(uint8_t aCompletionErrorCode, const Optional> & aTotalOperationalTime, - const Optional> & aPausedTime) const + const Optional> & aPausedTime) { ChipLogDetail(Zcl, "OperationalStateServer: OnOperationCompletionDetected"); @@ -169,6 +184,8 @@ void Instance::OnOperationCompletionDetected(uint8_t aCompletionErrorCode, ChipLogError(Zcl, "OperationalStateServer: Failed to record OperationCompletion event: %" CHIP_ERROR_FORMAT, error.Format()); } + + UpdateCountdownTimeFromClusterLogic(); } void Instance::ReportOperationalStateListChange() @@ -179,6 +196,43 @@ void Instance::ReportOperationalStateListChange() void Instance::ReportPhaseListChange() { MatterReportingAttributeChangeCallback(ConcreteAttributePath(mEndpointId, mClusterId, Attributes::PhaseList::Id)); + UpdateCountdownTimeFromClusterLogic(); +} + +void Instance::UpdateCountdownTime(bool fromDelegate) +{ + app::DataModel::Nullable newCountdownTime = mDelegate->GetCountdownTime(); + auto now = System::SystemClock().GetMonotonicTimestamp(); + + bool markDirty = false; + + if (fromDelegate) + { + // Updates from delegate are reduce-reported to every 10s max (choice of this implementation), in addition + // to default change-from-null, change-from-zero and increment policy. + auto predicate = [](const decltype(mCountdownTime)::SufficientChangePredicateCandidate & candidate) -> bool { + if (candidate.lastDirtyValue.IsNull() || candidate.newValue.IsNull()) + { + return false; + } + + uint32_t lastDirtyValue = candidate.lastDirtyValue.Value(); + uint32_t newValue = candidate.newValue.Value(); + uint32_t kNumSecondsDeltaToReport = 10; + return (newValue < lastDirtyValue) && ((lastDirtyValue - newValue) > kNumSecondsDeltaToReport); + }; + markDirty = (mCountdownTime.SetValue(newCountdownTime, now, predicate) == AttributeDirtyState::kMustReport); + } + else + { + auto predicate = [](const decltype(mCountdownTime)::SufficientChangePredicateCandidate &) -> bool { return true; }; + markDirty = (mCountdownTime.SetValue(newCountdownTime, now, predicate) == AttributeDirtyState::kMustReport); + } + + if (markDirty) + { + MatterReportingAttributeChangeCallback(mEndpointId, mClusterId, Attributes::CountdownTime::Id); + } } bool Instance::IsSupportedPhase(uint8_t aPhase) @@ -270,6 +324,7 @@ void Instance::InvokeCommand(HandlerContext & handlerContext) ChipLogDetail(Zcl, "OperationalState: Entering handling derived cluster commands"); InvokeDerivedClusterCommand(handlerContext); + break; } } @@ -294,18 +349,18 @@ CHIP_ERROR Instance::Read(const ConcreteReadAttributePath & aPath, AttributeValu } return err; }); + break; } - break; case OperationalState::Attributes::OperationalState::Id: { ReturnErrorOnFailure(aEncoder.Encode(GetCurrentOperationalState())); + break; } - break; case OperationalState::Attributes::OperationalError::Id: { ReturnErrorOnFailure(aEncoder.Encode(mOperationalError)); + break; } - break; case OperationalState::Attributes::PhaseList::Id: { @@ -332,18 +387,19 @@ CHIP_ERROR Instance::Read(const ConcreteReadAttributePath & aPath, AttributeValu ReturnErrorOnFailure(encoder.Encode(phase2)); } }); + break; } - break; case OperationalState::Attributes::CurrentPhase::Id: { ReturnErrorOnFailure(aEncoder.Encode(GetCurrentPhase())); + break; } - break; case OperationalState::Attributes::CountdownTime::Id: { + // Read through to get value closest to reality. ReturnErrorOnFailure(aEncoder.Encode(mDelegate->GetCountdownTime())); + break; } - break; } return CHIP_NO_ERROR; } diff --git a/src/app/clusters/operational-state-server/operational-state-server.h b/src/app/clusters/operational-state-server/operational-state-server.h index c1640cb99e9ce7..56229c02541d60 100644 --- a/src/app/clusters/operational-state-server/operational-state-server.h +++ b/src/app/clusters/operational-state-server/operational-state-server.h @@ -22,6 +22,8 @@ #include #include #include +#include +#include namespace chip { namespace app { @@ -113,6 +115,12 @@ class Instance : public CommandHandlerInterface, public AttributeAccessInterface */ void GetCurrentOperationalError(GenericOperationalError & error) const; + /** + * @brief Whenever application delegate wants to possibly report a new updated time, + * call this method. The `GetCountdownTime()` method will be called on the delegate. + */ + void UpdateCountdownTimeFromDelegate() { UpdateCountdownTime(/* fromDelegate = */ true); } + // Event triggers /** * @brief Called when the Node detects a OperationalError has been raised. @@ -129,7 +137,7 @@ class Instance : public CommandHandlerInterface, public AttributeAccessInterface */ void OnOperationCompletionDetected(uint8_t aCompletionErrorCode, const Optional> & aTotalOperationalTime = NullOptional, - const Optional> & aPausedTime = NullOptional) const; + const Optional> & aPausedTime = NullOptional); // List change reporting /** @@ -192,6 +200,19 @@ class Instance : public CommandHandlerInterface, public AttributeAccessInterface */ virtual void InvokeDerivedClusterCommand(HandlerContext & handlerContext) { return; }; + /** + * Causes reporting/udpating of CountdownTime attribute from driver if sufficient changes have + * occurred (based on Q quality definition for operational state). Calls the Delegate::GetCountdownTime() method. + * + * @param fromDelegate true if the change notice was triggered by the delegate, false if internal to cluster logic. + */ + void UpdateCountdownTime(bool fromDelegate); + + /** + * @brief Whenever the cluster logic thinks time should be updated, call this. + */ + void UpdateCountdownTimeFromClusterLogic() { UpdateCountdownTime(/* fromDelegate=*/false); } + private: Delegate * mDelegate; @@ -202,6 +223,7 @@ class Instance : public CommandHandlerInterface, public AttributeAccessInterface app::DataModel::Nullable mCurrentPhase; uint8_t mOperationalState = 0; // assume 0 for now. GenericOperationalError mOperationalError = to_underlying(ErrorStateEnum::kNoError); + app::QuieterReportingAttribute mCountdownTime{ DataModel::NullNullable }; /** * This method is inherited from CommandHandlerInterface. @@ -262,9 +284,10 @@ class Delegate virtual ~Delegate() = default; /** - * Get the countdown time. - * NOTE: Changes to this attribute should not be reported. - * From the spec: Changes to this value SHALL NOT be reported in a subscription. + * Get the countdown time. This will get called on many edges such as + * commands to change operational state, or when the delegate deals with + * changes. Make sure it becomes null whenever it is appropriate. + * * @return The current countdown time. */ virtual app::DataModel::Nullable GetCountdownTime() = 0; diff --git a/src/python_testing/TC_OpstateCommon.py b/src/python_testing/TC_OpstateCommon.py index 648eea59f2bb3c..09dc73e3344c1f 100644 --- a/src/python_testing/TC_OpstateCommon.py +++ b/src/python_testing/TC_OpstateCommon.py @@ -354,7 +354,7 @@ async def TEST_TC_OPSTATE_BASE_2_1(self, endpoint=1): if phase_list is not NullValue: phase_list_len = len(phase_list) asserts.assert_less_equal(phase_list_len, 32, - f"PhaseList length({phase_list_len}) must be less than 32!") + f"PhaseList length({phase_list_len}) must be at most 32 entries!") # STEP 3: TH reads from the DUT the CurrentPhase attribute self.step(3) @@ -364,8 +364,9 @@ async def TEST_TC_OPSTATE_BASE_2_1(self, endpoint=1): if (phase_list == NullValue) or (not phase_list): asserts.assert_true(current_phase == NullValue, f"CurrentPhase({current_phase}) should be null") else: - asserts.assert_true(0 <= current_phase and current_phase < phase_list_len, - f"CurrentPhase({current_phase}) must be between 0 and {(phase_list_len - 1)}") + asserts.assert_greater_equal(current_phase, 0, f"CurrentPhase({current_phase}) must be >= 0") + asserts.assert_less(current_phase, phase_list_len, + f"CurrentPhase({current_phase}) must be less than {phase_list_len}") # STEP 4: TH reads from the DUT the CountdownTime attribute self.step(4) @@ -652,7 +653,7 @@ async def TEST_TC_OPSTATE_BASE_2_2(self, endpoint=1): if phase_list is not NullValue: phase_list_len = len(phase_list) asserts.assert_less_equal(phase_list_len, 32, - f"PhaseList length({phase_list_len}) must be less than 32!") + f"PhaseList length({phase_list_len}) must be at most 32 entries!") # STEP 9: TH reads from the DUT the CurrentPhase attribute self.step(9) @@ -662,10 +663,10 @@ async def TEST_TC_OPSTATE_BASE_2_2(self, endpoint=1): if (phase_list == NullValue) or (not phase_list): asserts.assert_equal(current_phase, NullValue, f"CurrentPhase({current_phase}) should be null") else: - asserts.assert_less_equal(0, current_phase, - f"CurrentPhase({current_phase}) must be greater or equal than 0") - asserts.assert_less(current_phase < phase_list_len, - f"CurrentPhase({current_phase}) must be less then {(phase_list_len - 1)}") + asserts.assert_greater_equal(current_phase, 0, + f"CurrentPhase({current_phase}) must be greater or equal to 0") + asserts.assert_less(current_phase, phase_list_len, + f"CurrentPhase({current_phase}) must be less than {(phase_list_len)}") # STEP 10: TH waits for {PIXIT.WAITTIME.COUNTDOWN} self.step(10) @@ -679,6 +680,8 @@ async def TEST_TC_OPSTATE_BASE_2_2(self, endpoint=1): attribute=attributes.CountdownTime) if (countdown_time is not NullValue) and (initial_countdown_time is not NullValue): + logging.info(f" -> Initial countdown time: {initial_countdown_time}") + logging.info(f" -> New countdown time: {countdown_time}") asserts.assert_less_equal(countdown_time, (initial_countdown_time - wait_time), f"The countdown time shall have decreased at least {wait_time:.1f} since start command") @@ -821,6 +824,7 @@ async def TEST_TC_OPSTATE_BASE_2_3(self, endpoint=1): initial_countdown_time = await self.read_expect_success(endpoint=endpoint, attribute=attributes.CountdownTime) if initial_countdown_time is not NullValue: + logging.info(f" -> Initial ountdown time: {initial_countdown_time}") asserts.assert_true(0 <= initial_countdown_time <= 259200, f"CountdownTime({initial_countdown_time}) must be between 0 and 259200") @@ -835,6 +839,8 @@ async def TEST_TC_OPSTATE_BASE_2_3(self, endpoint=1): attribute=attributes.CountdownTime) if (countdown_time is not NullValue) and (initial_countdown_time is not NullValue): + logging.info(f" -> Initial countdown time: {initial_countdown_time}") + logging.info(f" -> New countdown time: {countdown_time}") asserts.assert_equal(countdown_time, initial_countdown_time, "The countdown time shall be equal since pause command")