-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Remote 1: Action Models #1930
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remote 1: Action Models #1930
Changes from 1 commit
ba6c7f3
6f321af
ab04d44
302323a
db2ef8d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,78 +8,34 @@ | |
|
|
||
| import Foundation | ||
| import LoopKit | ||
| import HealthKit | ||
|
|
||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not needed as each remote action type will own its own errors. |
||
| public enum RemoteCommandError: LocalizedError { | ||
| case expired | ||
| case invalidOTP | ||
| case missingMaxBolus | ||
| case exceedsMaxBolus | ||
| case exceedsMaxCarbs | ||
| case invalidCarbs | ||
|
|
||
| public var errorDescription: String? { | ||
| get { | ||
| switch self { | ||
| case .expired: | ||
| return NSLocalizedString("Expired", comment: "Remote command error description: expired.") | ||
| case .invalidOTP: | ||
| return NSLocalizedString("Invalid OTP", comment: "Remote command error description: invalid OTP.") | ||
| case .missingMaxBolus: | ||
| return NSLocalizedString("Missing maximum allowed bolus in settings", comment: "Remote command error description: missing maximum bolus in settings.") | ||
| case .exceedsMaxBolus: | ||
| return NSLocalizedString("Exceeds maximum allowed bolus in settings", comment: "Remote command error description: bolus exceeds maximum bolus in settings.") | ||
| case .exceedsMaxCarbs: | ||
| return NSLocalizedString("Exceeds maximum allowed carbs", comment: "Remote command error description: carbs exceed maximum amount.") | ||
| case .invalidCarbs: | ||
| return NSLocalizedString("Invalid carb amount", comment: "Remote command error description: invalid carb amount.") | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
| enum RemoteCommand { | ||
| case temporaryScheduleOverride(TemporaryScheduleOverride) | ||
| case cancelTemporaryOverride | ||
| case bolusEntry(Double) | ||
| case carbsEntry(NewCarbEntry) | ||
| } | ||
|
|
||
|
|
||
| // Push Notifications | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Generally like before, this method parses a notification and extracts a remote action (previously command). The difference is Loop valid action checks are not done here (ex: Carb amount within reasonable limits). That will instead be managed by the client. Separating notification payload parsing and validation seems like a good idea here. This will go away when I use Codable to do all this parsing in Remote 2.0. |
||
| extension RemoteCommand { | ||
| static func createRemoteCommand(notification: [String: Any], allowedPresets: [TemporaryScheduleOverridePreset], defaultAbsorptionTime: TimeInterval, nowDate: Date = Date()) -> Result<RemoteCommand, RemoteCommandParseError> { | ||
| extension RemoteAction { | ||
| static func createRemoteAction(notification: [String: Any]) -> Result<RemoteAction, RemoteCommandParseError> { | ||
| if let overrideName = notification["override-name"] as? String, | ||
| let preset = allowedPresets.first(where: { $0.name == overrideName }), | ||
| let remoteAddress = notification["remote-address"] as? String | ||
| { | ||
| var override = preset.createOverride(enactTrigger: .remote(remoteAddress)) | ||
| var overrideTime: TimeInterval? = nil | ||
| if let overrideDurationMinutes = notification["override-duration-minutes"] as? Double { | ||
| override.duration = .finite(TimeInterval(minutes: overrideDurationMinutes)) | ||
| overrideTime = TimeInterval(minutes: overrideDurationMinutes) | ||
| } | ||
| return .success(.temporaryScheduleOverride(override)) | ||
| } else if let _ = notification["cancel-temporary-override"] as? String { | ||
| return .success(.cancelTemporaryOverride) | ||
| return .success(.temporaryScheduleOverride(RemoteOverrideAction(name: overrideName, durationTime: overrideTime, remoteAddress: remoteAddress))) | ||
| } else if let _ = notification["cancel-temporary-override"] as? String, | ||
| let remoteAddress = notification["remote-address"] as? String | ||
| { | ||
| return .success(.cancelTemporaryOverride(RemoteOverrideCancelAction(remoteAddress: remoteAddress))) | ||
| } else if let bolusValue = notification["bolus-entry"] as? Double { | ||
| return .success(.bolusEntry(bolusValue)) | ||
| return .success(.bolusEntry(RemoteBolusAction(amountInUnits: bolusValue))) | ||
| } else if let carbsValue = notification["carbs-entry"] as? Double { | ||
|
|
||
| let minAbsorptionTime = TimeInterval(hours: 0.5) | ||
| let maxAbsorptionTime = LoopConstants.maxCarbAbsorptionTime | ||
|
|
||
| var absorptionTime = defaultAbsorptionTime | ||
| var absorptionTime: TimeInterval? = nil | ||
| if let absorptionOverrideInHours = notification["absorption-time"] as? Double { | ||
| absorptionTime = TimeInterval(hours: absorptionOverrideInHours) | ||
| } | ||
|
|
||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Carb food type was missing from the APIs. This will require a change to the Nightscout codebase in order to support this but should be ok when empty too. |
||
| if absorptionTime < minAbsorptionTime || absorptionTime > maxAbsorptionTime { | ||
| return .failure(RemoteCommandParseError.invalidAbsorptionSeconds(absorptionTime)) | ||
| } | ||
|
|
||
| let quantity = HKQuantity(unit: .gram(), doubleValue: carbsValue) | ||
| var foodType = notification["food-type"] as? String ?? nil | ||
|
|
||
| var startDate = nowDate | ||
| var startDate: Date? = nil | ||
| if let notificationStartTimeString = notification["start-time"] as? String { | ||
| let formatter = ISO8601DateFormatter() | ||
| formatter.formatOptions = [.withInternetDateTime, .withFractionalSeconds] | ||
|
|
@@ -90,16 +46,14 @@ extension RemoteCommand { | |
| } | ||
| } | ||
|
|
||
| let newEntry = NewCarbEntry(quantity: quantity, startDate: startDate, foodType: "", absorptionTime: absorptionTime) | ||
| return .success(.carbsEntry(newEntry)) | ||
| return .success(.carbsEntry(RemoteCarbAction(amountInGrams: carbsValue, absorptionTime: absorptionTime, foodType: foodType, startDate: startDate))) | ||
| } else { | ||
| return .failure(RemoteCommandParseError.unhandledNotication("\(notification)")) | ||
| } | ||
| } | ||
|
|
||
| enum RemoteCommandParseError: LocalizedError { | ||
| case invalidStartTime(String) | ||
| case invalidAbsorptionSeconds(Double) | ||
| case unhandledNotication(String) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| // | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I needed to update the RemoteCommandTests with these and change a few that are unnecessary. I added more tests to LoopKit, which test the individual remote action types themselves which should cover all the cases I know of. |
||
| // RemoteActionTests.swift | ||
| // LoopTests | ||
| // | ||
| // Created by Bill Gestrich on 8/13/22. | ||
| // Copyright © 2022 LoopKit Authors. All rights reserved. | ||
| // | ||
|
|
||
| import XCTest | ||
| import HealthKit | ||
| @testable import Loop | ||
| import LoopKit | ||
|
|
||
| class RemoteActionTests: XCTestCase { | ||
|
|
||
| override func setUpWithError() throws { | ||
| } | ||
|
|
||
| override func tearDownWithError() throws { | ||
| } | ||
|
|
||
|
|
||
| //MARK: Carb Entry Command | ||
|
|
||
| func testParseCarbEntryNotification_ValidPayload_Succeeds() throws { | ||
|
|
||
| //Arrange | ||
| let expectedStartDateString = "2022-08-14T03:08:00.000Z" | ||
| let expectedCarbsInGrams = 15.0 | ||
| let expectedDate = dateFormatter().date(from: expectedStartDateString)! | ||
| let expectedAbsorptionTimeInHours = 3.0 | ||
| let expectedFoodType = "🍕" | ||
| let otp = 12345 | ||
| let notification: [String: Any] = [ | ||
| "carbs-entry":expectedCarbsInGrams, | ||
| "absorption-time": expectedAbsorptionTimeInHours, | ||
| "food-type": expectedFoodType, | ||
| "otp": otp, | ||
| "start-time": expectedStartDateString | ||
| ] | ||
|
|
||
| //Act | ||
| let action = try RemoteAction.createRemoteAction(notification: notification).get() | ||
|
|
||
| //Assert | ||
| guard case .carbsEntry(let carbEntry) = action else { | ||
| XCTFail("Incorrect case") | ||
| return | ||
| } | ||
| XCTAssertEqual(carbEntry.startDate, expectedDate) | ||
| XCTAssertEqual(carbEntry.absorptionTime, TimeInterval(hours: expectedAbsorptionTimeInHours)) | ||
| XCTAssertEqual(carbEntry.amountInGrams, expectedCarbsInGrams) | ||
| XCTAssertEqual(expectedFoodType, carbEntry.foodType) | ||
| } | ||
|
|
||
| func testParseCarbEntryNotification_MissingCreatedDate_Succeeds() throws { | ||
|
|
||
| //Arrange | ||
| let expectedCarbsInGrams = 15.0 | ||
| let expectedAbsorptionTimeInHours = 3.0 | ||
| let otp = 12345 | ||
| let notification: [String: Any] = [ | ||
| "carbs-entry":expectedCarbsInGrams, | ||
| "absorption-time": expectedAbsorptionTimeInHours, | ||
| "otp": otp | ||
| ] | ||
|
|
||
| //Act | ||
| let action = try RemoteAction.createRemoteAction(notification: notification).get() | ||
|
|
||
| //Assert | ||
| guard case .carbsEntry(let carbEntry) = action else { | ||
| XCTFail("Incorrect case") | ||
| return | ||
| } | ||
|
|
||
| XCTAssertEqual(carbEntry.startDate, nil) | ||
| XCTAssertEqual(carbEntry.absorptionTime, TimeInterval(hours: expectedAbsorptionTimeInHours)) | ||
| XCTAssertEqual(carbEntry.amountInGrams, expectedCarbsInGrams) | ||
| } | ||
|
|
||
| func testParseCarbEntryNotification_InvalidCreatedDate_Fails() throws { | ||
|
|
||
| //Arrange | ||
| let expectedCarbsInGrams = 15.0 | ||
| let expectedAbsorptionTimeInHours = 3.0 | ||
| let otp = 12345 | ||
| let notification: [String: Any] = [ | ||
| "carbs-entry": expectedCarbsInGrams, | ||
| "absorption-time":expectedAbsorptionTimeInHours, | ||
| "otp": otp, | ||
| "start-time": "invalid-date-string" | ||
| ] | ||
|
|
||
| //Act + Assert | ||
| XCTAssertThrowsError(try RemoteAction.createRemoteAction(notification: notification).get()) | ||
| } | ||
|
|
||
|
|
||
| //MARK: Utils | ||
|
|
||
| func dateFormatter() -> ISO8601DateFormatter { | ||
| let formatter = ISO8601DateFormatter() | ||
| formatter.formatOptions = [.withInternetDateTime, .withFractionalSeconds] | ||
| return formatter | ||
| } | ||
|
|
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Async wrapper