From 0a64d5083a054ee51ad477ed8285cacbd2285ab1 Mon Sep 17 00:00:00 2001 From: Jeff Tung <100387939+jtung-apple@users.noreply.github.com> Date: Thu, 18 May 2023 08:46:40 -0700 Subject: [PATCH] [Darwin] MTRDevice estimate start time based on General Diagnostics UpTime attribute when present (#26583) * [Darwin] MTRDevice estimate start time based on General Diagnostics UpTime attribute when present * Address bug found in review * Simplified UpTime use logic, and ignore StartUp event for estimate purpose during priming * Reworked UpTime-derived estimate logic per discussion with Boris * Reduced complexity and dependency on _state --- src/darwin/Framework/CHIP/MTRDevice.mm | 38 ++++++++++++++++++- .../Framework/CHIPTests/MTRDeviceTests.m | 10 +++-- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index 9ca065238b4dd3..ca1bfd6495b03d 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -156,6 +156,8 @@ @interface MTRDevice () @property (nonatomic) BOOL expirationCheckScheduled; +@property (nonatomic) NSDate * estimatedStartTimeFromGeneralDiagnosticsUpTime; + /** * If currentReadClient is non-null, that means that we successfully * called SendAutoResubscribeRequest on the ReadClient and have not yet gotten @@ -260,7 +262,9 @@ - (void)_changeState:(MTRDeviceState)state _state = state; if (lastState != state) { if (state != MTRDeviceStateReachable) { + MTR_LOG_INFO("%@ Set estimated start time to nil due to state change", self); _estimatedStartTime = nil; + _estimatedStartTimeFromGeneralDiagnosticsUpTime = nil; } id delegate = _weakDelegate.strongObject; if (delegate) { @@ -412,8 +416,18 @@ - (void)_handleEventReport:(NSArray *> *)eventRepor MTREventPath * eventPath = eventDict[MTREventPathKey]; BOOL isStartUpEvent = (eventPath.cluster.unsignedLongValue == MTRClusterIDTypeBasicInformationID) && (eventPath.event.unsignedLongValue == MTREventIDTypeClusterBasicInformationEventStartUpID); - if (isStartUpEvent) { - _estimatedStartTime = nil; + if (isStartUpEvent && (_state == MTRDeviceStateReachable)) { + // StartUp event received when server resumes subscription + if (_estimatedStartTimeFromGeneralDiagnosticsUpTime) { + // If UpTime was received, make use of it as mark of system start time + MTR_LOG_INFO("%@ StartUp event: set estimated start time forward to %@", self, + _estimatedStartTimeFromGeneralDiagnosticsUpTime); + _estimatedStartTime = _estimatedStartTimeFromGeneralDiagnosticsUpTime; + } else { + // If UpTime was not received, reset estimated start time in case of reboot + MTR_LOG_INFO("%@ StartUp event: set estimated start time to nil", self); + _estimatedStartTime = nil; + } } // If event time is of MTREventTimeTypeSystemUpTime type, then update estimated start time as needed @@ -950,6 +964,26 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray %@", self, upTime, + oldSystemStartTime, potentialSystemStartTime); + _estimatedStartTime = potentialSystemStartTime; + } + + // Save estimate in the subscription resumption case, for when StartUp event uses it + _estimatedStartTimeFromGeneralDiagnosticsUpTime = potentialSystemStartTime; + } + } } if (shouldReportAttribute) { diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m index f6d6c5d1e0db88..1c2c1520f586d5 100644 --- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m +++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m @@ -1543,7 +1543,12 @@ - (void)test017_TestMTRDeviceBasics subscriptionEstablished:^() { }]; - [self waitForExpectations:@[ subscriptionDroppedExpectation, resubscriptionExpectation ] timeout:60 enforceOrder:YES]; + [self waitForExpectations:@[ subscriptionDroppedExpectation ] timeout:60]; + + // Check that device resets start time on subscription drop + XCTAssertNil(device.estimatedStartTime); + + [self waitForExpectations:@[ resubscriptionExpectation ] timeout:60]; // Now make sure we ignore later tests. Ideally we would just unsubscribe // or remove the delegate, but there's no good way to do that. @@ -1557,9 +1562,6 @@ - (void)test017_TestMTRDeviceBasics // with data versions) during the resubscribe. XCTAssertEqual(attributeReportsReceived, 0); XCTAssertEqual(eventReportsReceived, 0); - - // Check that device resets start time on subscription drop - XCTAssertNil(device.estimatedStartTime); } - (void)test018_SubscriptionErrorWhenNotResubscribing