-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Moving more stuff out of the MTRDevice #36090
Moving more stuff out of the MTRDevice #36090
Conversation
Review changes with SemanticDiff. |
PR #36090: Size comparison from 579b1b1 to 151a7ac Full report (82 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
@@ -309,4 +309,41 @@ - (void)downloadLogOfType:(MTRDiagnosticLogType)type | |||
// Not Supported via XPC | |||
// - (oneway void)downloadLogOfType:(MTRDiagnosticLogType)type nodeID:(NSNumber *)nodeID timeout:(NSTimeInterval)timeout completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion; | |||
|
|||
#pragma mark - Storage Overrides | |||
|
|||
- (NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *)_clusterDataToPersistSnapshot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the goal is to not have persistence at all in MTRDevice_XPC, then we should just rip it out of MTRDevice, not do this, I think....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have at it :)
@@ -1646,11 +1646,6 @@ - (void)_scheduleClusterDataPersistence | |||
} | |||
[_mostRecentReportTimes addObject:[NSDate now]]; | |||
|
|||
{ | |||
std::lock_guard lock(_descriptionLock); | |||
_mostRecentReportTimeForDescription = [_mostRecentReportTimes lastObject]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this being removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just copied and pasted the base implementation into concrete, please fix :)
I think we should do #36132 instead. |
MTRDevice is still using some basic C++ stuff here, we should be not doing this in MTRDevice_XPC, I did a quick and simple hack here. @bzbarsky-apple