Remote PR Set #3 #488
Remote PR Set #3 #488
Conversation
| */ | ||
| func commandFromPushNotification(_ notification: [String: AnyObject]) async throws -> RemoteCommand | ||
| func handleRemoteNotification(_ notification: [String: AnyObject]) async throws | ||
|
|
There was a problem hiding this comment.
REMOTE 2.0: WILL REMOVE PRE-MERGE
There was a problem hiding this comment.
nit: "handle" is one of those non-descriptive verbs, like "process". Every function "handles" something or "processes" something. Maybe something like remoteNotificationWasReceived(_ notification ...)
| func handleRemoteOverride(name: String, durationTime: TimeInterval?, remoteAddress: String) async throws | ||
| func handleRemoteOverrideCancel() async throws | ||
| func handleRemoteCarb(amountInGrams: Double, absorptionTime: TimeInterval?, foodType: String?, startDate: Date?) async throws | ||
| func handleRemoteBolus(amountInUnits: Double) async throws |
There was a problem hiding this comment.
REMOTE 2.0: WILL REMOVE PRE-MERGE
| // | ||
|
|
||
| import Foundation | ||
|
|
There was a problem hiding this comment.
The RemoteActionDelegate is conformed to both by the ServiceManager and DeviceDataManager to propogate actions to where they are delivered.
I'm using "handleRemote" in the method names to help disambiguate from existing APIs in DeviceDataManaager, like "enactBolus". I can drop the prefix if desired though.
There was a problem hiding this comment.
Again, "handle" is very vague. This API is exposing dosing functionality and settings change functionality. So it would be more descriptive to have the verbs describe actually enacting or delivering. I think it's fine to keep the "remote" in there, as that's the main foreseeable use for this API, but an API like this could theoretically be used by Services for other things, like maybe they have some kind of other trigger to change dosing, like a timer, or other notification.
There was a problem hiding this comment.
Done but let me know if feedback on the names below.
gestrich
force-pushed
the
feature/2023/06/bg/remote-command-service-refactor
branch
from
a82acc8 to
98070db
Compare
MockKit/MockService.swift
Outdated
| throw MockServicePushNotificationError.remoteCommandsNotSupported | ||
| public func handleRemoteNotification(_ notification: [String: AnyObject]) async throws { | ||
| } | ||
|
|
There was a problem hiding this comment.
REMOTE 2.0: WILL REMOVE PRE-MERGE
gestrich
force-pushed
the
feature/2023/06/bg/remote-command-service-refactor
branch
from
98070db to
cb3f748
Compare
This is part of a collection of foundational PRs that need to merge at the same time.
Loop
LoopKit
NightscoutService
TidepoolService