Skip to content

Remote PR Set #2: Introduce RemoteCommands#455

Merged
ps2 merged 1 commit into
LoopKit:devfrom
gestrich:feature/2023/02/bg/remote-command-service
Apr 21, 2023
Merged

Remote PR Set #2: Introduce RemoteCommands#455
ps2 merged 1 commit into
LoopKit:devfrom
gestrich:feature/2023/02/bg/remote-command-service

Conversation

@gestrich

@gestrich gestrich commented Feb 25, 2023

Copy link
Copy Markdown
Contributor

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.

dybs pushed a commit to dybs/LoopKit that referenced this pull request Feb 28, 2023
@gestrich gestrich force-pushed the feature/2023/02/bg/remote-command-service branch from 8ea5b6b to bc31966 Compare March 11, 2023 10:43
@gestrich gestrich force-pushed the feature/2023/02/bg/remote-command-service branch from bc31966 to f40c9d6 Compare March 11, 2023 15:02
// Created by Bill Gestrich on 2/25/23.
// Copyright © 2023 LoopKit Authors. All rights reserved.
//

@gestrich gestrich Mar 11, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 { }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@gestrich gestrich changed the title Remote 2: RemoteCommands in RemoteDataService Remote PR Set #2: RemoteCommands in RemoteDataService Mar 11, 2023
@gestrich gestrich changed the title Remote PR Set #2: RemoteCommands in RemoteDataService Remote PR Set #2: Introduce RemoteCommands Mar 11, 2023
@gestrich gestrich marked this pull request as ready for review March 11, 2023 15:44
@gestrich gestrich force-pushed the feature/2023/02/bg/remote-command-service branch from f40c9d6 to 9a31695 Compare March 11, 2023 20:20
- Returns: RemoteCommand
*/
func validatePushNotificationSource(_ notification: [String: AnyObject]) -> Result<Void, Error>
func commandFromPushNotification(_ notification: [String: AnyObject]) async throws -> RemoteCommand

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
//

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 { }

@ps2 ps2 merged commit 38ce9b4 into LoopKit:dev Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants