Remote Bolus Dev#362
Conversation
|
Thanks @gestrich ! Will be looking at this soon. |
|
@ps2 I still have a few weeks before these pieces will be ready for review but will ping you on these when ready to go (welcome interim feedback though too). I'll probably start sharing with some testers at that time to give it an initial look too. |
cd652eb to
b0f6296
Compare
|
@ps2 I've started to ask for some community help with the docs and early testing of these branches https://loop.zulipchat.com/#narrow/stream/144182-development/topic/Remote.20bolus.20.2F.20remote.20carbs. On the code side, it is probably a good time to discuss the overall integration of this feature. I can either do my best to make a quality write-up on the architecture and my own questions. I'd also be willing to do a code walkthrough over a quick video call which may be a shortcut for both of us. Just let me know what works best for you. thanks! |
| @@ -18,4 +18,8 @@ public enum LoopNotificationCategory: String { | |||
| case pumpExpired | |||
| case pumpFault | |||
| case alert | |||
There was a problem hiding this comment.
Added these for the local notifications that will present for remote bolus/carbs.
There was a problem hiding this comment.
These alert definitions are probably going away, or being moved into Loop itself. But I'll take this for now
| @@ -78,6 +78,13 @@ public protocol RemoteDataService: Service { | |||
| - Parameter completion: The completion function to call with any success or failure. | |||
| */ | |||
| func uploadSettingsData(_ stored: [StoredSettings], completion: @escaping (_ result: Result<Bool, Error>) -> Void) | |||
There was a problem hiding this comment.
This new delegate methods is so Loop can ask remote data services if they can validate a notification. More details on this flow in the Loop PR.
| @@ -118,5 +125,9 @@ public extension RemoteDataService { | |||
| func uploadSettingsData(_ stored: [StoredSettings], completion: @escaping (_ result: Result<Bool, Error>) -> Void) { | |||
| completion(.success(false)) | |||
| } | |||
There was a problem hiding this comment.
No support for validation by default. The services can choose to implement this, like the NightscoutService will.
| } | ||
|
|
||
| func validatePushNotificationSource(_ notification: [String: AnyObject]) -> Bool { | ||
| return false |
There was a problem hiding this comment.
Not a big fan of default implementations for protocols (https://betterprogramming.pub/swift-why-you-should-avoid-using-default-implementations-in-protocols-eeffddbed46d), but I can see you're just extending the pattern. And it's a pain to update all the actual implementations that use the protocol.
| @@ -18,4 +18,8 @@ public enum LoopNotificationCategory: String { | |||
| case pumpExpired | |||
| case pumpFault | |||
| case alert | |||
There was a problem hiding this comment.
These alert definitions are probably going away, or being moved into Loop itself. But I'll take this for now
This PR includes small changes to the LoopKit library to support remote/bolus in dev. See the workspace PR for more details on the overall effort.