Remote PR Set #2: Introduce RemoteCommands#455
Conversation
LOOP-4116 DIY Sync
8ea5b6b to
bc31966
Compare
bc31966 to
f40c9d6
Compare
| // Created by Bill Gestrich on 2/25/23. | ||
| // Copyright © 2023 LoopKit Authors. All rights reserved. | ||
| // | ||
|
|
There was a problem hiding this comment.
This is the shared model that will be returned by plugins to Loop. It opens an API into the plugin where state transitions will be reported during the command enactment process.
There was a problem hiding this comment.
I keep thinking about this architecture and the questions around it (where do the "command" or "action" models live, and who "owns" them?) and it feels like it's maybe not the right abstraction.
Just this morning I had an idea that might simplify this. Ultimately, we're implementing a feature where Loop exposes a remote control interface to service plugins. The data transfer, serialization, validation, authorization, etc, that a service plugin needs to implement the functionality should be all responsibilities of the service plugin, and not Loop. We're mostly there, I think, but if Loop isn't going to be managing "commands" and "actions", then why is this a model, and not just a simple interface that Loop exposes to the service plugins?
public protocol RemoteCommandHandler {
func enactBolus(units: Double, initiatedBy: String) async throws
func scheduleOverride(name: String, duration: TimeInterval, initiatedBy: String) async throws
func cancelOverride(name: String, initiatedBy: String) async throws
func addCarbEntry(amountInGrams: Double, absorptionTime: TimeInterval?, foodType: String?, startDate: Date?, initiatedBy: String) async throws
}
extension ServiceDelegate: RemoteCommandHandler { }
There was a problem hiding this comment.
This feels more natural to me, and clearly documents exactly what Loop is exposing more succinctly than a new type system ofRemoteCommands that can have validations, id, and actions, which have subtypes, are codable, etc.
f40c9d6 to
9a31695
Compare
| - Returns: RemoteCommand | ||
| */ | ||
| func validatePushNotificationSource(_ notification: [String: AnyObject]) -> Result<Void, Error> | ||
| func commandFromPushNotification(_ notification: [String: AnyObject]) async throws -> RemoteCommand |
There was a problem hiding this comment.
Nit: The docs say "fetch", but the signature looks like a simple conversion. Would be nice if the caller knows if this is a method that may have network level latencies.
| // Created by Bill Gestrich on 2/25/23. | ||
| // Copyright © 2023 LoopKit Authors. All rights reserved. | ||
| // | ||
|
|
There was a problem hiding this comment.
I keep thinking about this architecture and the questions around it (where do the "command" or "action" models live, and who "owns" them?) and it feels like it's maybe not the right abstraction.
Just this morning I had an idea that might simplify this. Ultimately, we're implementing a feature where Loop exposes a remote control interface to service plugins. The data transfer, serialization, validation, authorization, etc, that a service plugin needs to implement the functionality should be all responsibilities of the service plugin, and not Loop. We're mostly there, I think, but if Loop isn't going to be managing "commands" and "actions", then why is this a model, and not just a simple interface that Loop exposes to the service plugins?
public protocol RemoteCommandHandler {
func enactBolus(units: Double, initiatedBy: String) async throws
func scheduleOverride(name: String, duration: TimeInterval, initiatedBy: String) async throws
func cancelOverride(name: String, initiatedBy: String) async throws
func addCarbEntry(amountInGrams: Double, absorptionTime: TimeInterval?, foodType: String?, startDate: Date?, initiatedBy: String) async throws
}
extension ServiceDelegate: RemoteCommandHandler { }
This PR is part of the below set that should be merged together. This one will use "RemoteCommands", a protocol for interacting with the remote services that is the source of the RemoteCommand. This will lay the groundwork for remote 2.0 commands.