Location sharing: Support restarting location sending after app kill (PSF-1036) #6200
Conversation
|
📱 Scan the QR code below to install the build for this PR. If you can't scan the QR code you can install the build via this link: https://i.diawi.com/duM6Qd |
SBiOSoftWhare
requested review from
a team,
stefanceriu and
Anderas
and removed request for
a team
| } | ||
|
|
||
| return self.session.locationService.getBeaconInfoSummaries(for: userId).filter { summary in | ||
| return self.isDeviceBeaconInfoSummary(summary) && summary.isActive |
There was a problem hiding this comment.
Does this need to be weak? Also if I am reading this correctly we are calling isDeviceBeaconInfoSummary twice when restoring, once when getExistingDeviceBeaconSummaries and second time when didReceiveDeviceNewBeaconInfoSummary is called for the same session. Should both methods be responsible for making this check?
There was a problem hiding this comment.
Does this need to be weak?
You mean: private let session: MXSession needs to be weak?
if I am reading this correctly we are calling isDeviceBeaconInfoSummary twice when restoring
In fact in didReceiveBeaconInfoSummary where we are listenning to every beacon info summaries we are checking isDeviceBeaconInfoSummary and then call didReceiveDeviceNewBeaconInfoSummary when we have a new BeaconInfoSummary that belongs to the device . But there is no further call to isDeviceBeaconInfoSummary inside didReceiveDeviceNewBeaconInfoSummary.
There was a problem hiding this comment.
You mean:
private let session: MXSessionneeds to be weak?
Ah its a filter, not an escaping callback, so ignore this.
In fact in
didReceiveBeaconInfoSummarywhere we are listenning to every beacon info summaries we are checkingisDeviceBeaconInfoSummaryand then calldidReceiveDeviceNewBeaconInfoSummarywhen we have a new BeaconInfoSummary that belongs to the device . But there is no further call toisDeviceBeaconInfoSummaryinsidedidReceiveDeviceNewBeaconInfoSummary.
You're right, I was confused by the similarity of didReceiveBeaconInfoSummary and didReceiveDeviceNewBeaconInfoSummary (or rather not noticing the second method)
There was a problem hiding this comment.
No problem I admit the that the two function names are closed
| private func setupDeviceBeaconSummaries() { | ||
| let existingDeviceBeaconInfoSummaries = self.getExistingDeviceBeaconSummaries() | ||
|
|
||
| self.deviceBeaconInfoSummaries = existingDeviceBeaconInfoSummaries |
There was a problem hiding this comment.
I think you can get rid of the local existingDeviceBeaconInfoSummaries. The self.s too, here and in a lot of other places.
There was a problem hiding this comment.
Yes we should probably define some SwiftLint rules to decide or not to get rid of self. and all use the same convention. I have 50 self. in this file 😅
Part of #6199
Requires matrix-org/matrix-ios-sdk#1474