Remote PR Set #3#41
Conversation
LOOP-4116 DIY Sync
LOOP-4190 DIY Sync
LOOP-4371 DIY Sync
…ates LOOP-887 Keycloak updates
Tidepool Sync
| @@ -0,0 +1,18 @@ | |||
| // | |||
There was a problem hiding this comment.
REMOTE 2.0: WILL REMOVE PRE-MERGE
| @@ -0,0 +1,186 @@ | |||
| // | |||
There was a problem hiding this comment.
REMOTE 2.0: WILL REMOVE PRE-MERGE
This class still needs some work.
| @@ -0,0 +1,53 @@ | |||
| // | |||
There was a problem hiding this comment.
REMOTE 2.0: WILL REMOVE PRE-MERGE
Actually this is a hack I just use personally. This was how I used to use the override system to control Loop settings.
808e0ff to
a9ac5a8
Compare
| @@ -59,6 +60,7 @@ public final class NightscoutService: Service { | |||
| } | |||
|
|
|||
| private let commandSourceV1: RemoteCommandSourceV1 | |||
There was a problem hiding this comment.
REMOTE 2.0: WILL REMOVE PRE-MERGE
All the commandSourceV2 will be removed.
| let commandSource = try commandSource(notification: notification) | ||
| await commandSource.handleRemoteNotification(notification) | ||
| } | ||
|
|
There was a problem hiding this comment.
REMOTE 2.0: WILL REMOVE PRE-MERGE
| } | ||
|
|
||
| } | ||
|
|
There was a problem hiding this comment.
A delegate for the command source V1 (pre Remote 2.0 source). It really just handles incoming push notifications.
| try await self.serviceDelegate?.handleRemoteClosedLoop(activate: closedLoopComand.active) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This will upload the NS note. A screenshot of the result is in the main Loop PR.
|
|
||
| let notificationJSON = try JSONSerialization.data(withJSONObject: notification) | ||
| let notificationJSONString = String(data: notificationJSON, encoding: .utf8) ?? "" | ||
|
|
There was a problem hiding this comment.
The JSON is included in the note which may not be desired. I added it just in case we want to use that JSON in Caregiver for the interim (rather than users needing to look in NS), while the Remote 2.0 pieces are being worked out.
a9ac5a8 to
2313dec
Compare
| @@ -254,7 +254,7 @@ public class OTPManager { | |||
| dateFormatter.dateFormat = "h:mm" | |||
| return String(format: "Error: Password sent at %@ has expired. Only the last %u passwords are accepted. See LoopDocs for troubleshooting.", dateFormatter.string(from: deliveryDate), maxOTPsToAccept) | |||
| } else { | |||
There was a problem hiding this comment.
I noticed an unused parameter here.
| @@ -0,0 +1,18 @@ | |||
| // | |||
There was a problem hiding this comment.
REMOTE 2.0: WILL REMOVE PRE-MERGE
| @@ -0,0 +1,18 @@ | |||
| // | |||
| @@ -0,0 +1,24 @@ | |||
| // | |||
| @@ -0,0 +1,23 @@ | |||
| // | |||
| @@ -0,0 +1,19 @@ | |||
| // | |||
| @@ -1,36 +0,0 @@ | |||
| // | |||
There was a problem hiding this comment.
With the refactor, now there are the following objects:
V1 (Loop now): RemoteNotification: An incoming push notification.
V2 (remote 2.0): NSRemoteCommandPayload: Payload for a incoming push notification and also when making REST calls to NS for pending commands
Both V1 and V2 will use the same Actions which are above.
| public let remoteAddress: String | ||
| public let expiration: Date? | ||
| public let sentAt: Date? | ||
| public let otp: String |
There was a problem hiding this comment.
I noticed we were missing the enteredBy field which may be helpful to leverage.
| case expiration = "expiration" | ||
| case sentAt = "sent-at" | ||
| case otp = "otp" | ||
| } |
There was a problem hiding this comment.
No longer converting to NightscoutRemoteCommand since that was eliminated.
| func toRemoteAction() -> Action { | ||
| return .bolusEntry(BolusAction(amountInUnits: amount)) | ||
| } | ||
|
|
There was a problem hiding this comment.
The validate method on each notification will do both the OTP validation, if required, and validate the expiration date.
| @@ -19,6 +19,7 @@ public struct CarbRemoteNotification: RemoteNotification, Codable { | |||
| public let expiration: Date? | |||
There was a problem hiding this comment.
Same pattern in these classes as BolusRemoteNotification above.
| let action = OverrideCancelAction(remoteAddress: remoteAddress) | ||
| return .cancelTemporaryOverride(action) | ||
| } | ||
|
|
There was a problem hiding this comment.
No OTP validation needed for overrides so there is where we omit that.
| @@ -7,15 +7,19 @@ | |||
| // | |||
|
|
|||
There was a problem hiding this comment.
This is only used for Remote 1.0 (Loop today). It represents the payload for an incoming push notification.
| @@ -0,0 +1,41 @@ | |||
| // | |||
There was a problem hiding this comment.
The 2 commands sources, for Remote 1.0 vs Remote 2.0, allow us to support both paradigms simultaneously
| @@ -0,0 +1,47 @@ | |||
| // | |||
There was a problem hiding this comment.
REMOTE 2.0: WILL REMOVE PRE-MERGE
|
Probably beyond the scope of this PR, but this does make me think Loop should probably have a RemoteCommsLog, similar to the DeviceCommsLog. |
2313dec to
bc282c0
Compare
| public let sentAt: Date? | ||
| public let otp: String | ||
| public let otp: String? | ||
| public let enteredBy: String? |
There was a problem hiding this comment.
The answer to "who sent this command?" should ideally be determined by knowing which OTP code was matched, and showing the Loop user a name that the Loop user entered for that OTP pairing when the authorization was initially created.
So when Riley shares her Loop data with me, she puts in "Dad", and then the qrcode is shown and associated with "Dad". I pair that in LCG and when a remote bolus is received by Loop, if it matches my OTP, then Riley gets a notification that "Dad" sent a bolus command.
There was a problem hiding this comment.
Otherwise it's possible for caregivers to spoof.
There was a problem hiding this comment.
Yeah that makes sense. This is currently a field in NS but we could eliminate field someday with the support you mentioned. I'm actually not using this field here at the moment but just noticed it was part of the payload.
…ce-logs LOOP-1169 Upload Device Logs
…ce-logs LOOP-1169 Upload Device Logs
This is part of a collection of foundational PRs that need to merge at the same time.
Loop
LoopKit
NightscoutService
TidepoolService