Remote PR Set #2: Codable Models for Remote Notifications #768
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| // | ||
| // BolusRemoteNotification.swift | ||
| // NightscoutUploadKit | ||
| // | ||
| // Created by Bill Gestrich on 2/25/23. | ||
| // Copyright © 2023 Pete Schwamb. All rights reserved. | ||
| // | ||
|
|
||
| import Foundation | ||
|
|
||
| public struct BolusRemoteNotification: RemoteNotification, Codable { | ||
|
|
||
| public let amount: Double | ||
| public let remoteAddress: String | ||
| public let expiration: Date? | ||
| public let sentAt: Date? | ||
| public let otp: String | ||
|
|
||
| enum CodingKeys: String, CodingKey { | ||
| case remoteAddress = "remote-address" | ||
|
There was a problem hiding this comment. Is this encoding ( There was a problem hiding this comment. Yeah unfortunately the dash is used in the key. Here's the existing server and loop code that handles that There was a problem hiding this comment. I had Taylor Swift's Anti-Hero song pop into my head when looking at the git blame for this. :P |
||
| case amount = "bolus-entry" | ||
| case expiration = "expiration" | ||
| case sentAt = "sent-at" | ||
| case otp = "otp" | ||
| } | ||
|
|
||
| public static func includedInNotification(_ notification: [String: Any]) -> Bool { | ||
| return notification["bolus-entry"] != nil | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| // | ||
| // CarbRemoteNotification.swift | ||
| // NightscoutUploadKit | ||
| // | ||
| // Created by Bill Gestrich on 2/25/23. | ||
| // Copyright © 2023 Pete Schwamb. All rights reserved. | ||
| // | ||
|
|
||
| import Foundation | ||
|
|
||
| public struct CarbRemoteNotification: RemoteNotification, Codable { | ||
|
|
||
| public let amount: Double | ||
| public let absorptionInHours: Double? | ||
| public let foodType: String? | ||
| public let startDate: Date? | ||
| public let remoteAddress: String | ||
| public let expiration: Date? | ||
| public let sentAt: Date? | ||
| public let otp: String | ||
|
|
||
| enum CodingKeys: String, CodingKey { | ||
| case remoteAddress = "remote-address" | ||
| case amount = "carbs-entry" | ||
| case absorptionInHours = "absorption-time" | ||
| case foodType = "food-type" | ||
| case startDate = "start-time" | ||
| case expiration = "expiration" | ||
| case sentAt = "sent-at" | ||
| case otp = "otp" | ||
| } | ||
|
|
||
| public func absorptionTime() -> TimeInterval? { | ||
| guard let absorptionInHours = absorptionInHours else { | ||
| return nil | ||
| } | ||
| return TimeInterval(hours: absorptionInHours) | ||
| } | ||
|
|
||
| public static func includedInNotification(_ notification: [String: Any]) -> Bool { | ||
| return notification["carbs-entry"] != nil | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| // | ||
| // OverrideCancelRemoteNotification.swift | ||
| // NightscoutUploadKit | ||
| // | ||
| // Created by Bill Gestrich on 2/25/23. | ||
| // Copyright © 2023 Pete Schwamb. All rights reserved. | ||
| // | ||
|
|
||
| import Foundation | ||
|
|
||
| public struct OverrideCancelRemoteNotification: RemoteNotification, Codable { | ||
|
|
||
| public let remoteAddress: String | ||
| public let expiration: Date? | ||
| public let sentAt: Date? | ||
| public let cancelOverride: Bool | ||
|
|
||
| enum CodingKeys: String, CodingKey { | ||
| case remoteAddress = "remote-address" | ||
| case expiration = "expiration" | ||
| case sentAt = "sent-at" | ||
| case cancelOverride = "cancel-temporary-override" | ||
| } | ||
|
|
||
| public static func includedInNotification(_ notification: [String: Any]) -> Bool { | ||
| return notification["cancel-temporary-override"] != nil | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| // | ||
| // OverrideRemoteNotification.swift | ||
| // NightscoutUploadKit | ||
| // | ||
| // Created by Bill Gestrich on 2/25/23. | ||
| // Copyright © 2023 Pete Schwamb. All rights reserved. | ||
| // | ||
|
|
||
| import Foundation | ||
|
|
||
| public struct OverrideRemoteNotification: RemoteNotification, Codable { | ||
|
|
||
| public let name: String | ||
| public let durationInMinutes: Double? | ||
| public let remoteAddress: String | ||
| public let expiration: Date? | ||
| public let sentAt: Date? | ||
|
|
||
| enum CodingKeys: String, CodingKey { | ||
| case name = "override-name" | ||
| case remoteAddress = "remote-address" | ||
| case durationInMinutes = "override-duration-minutes" | ||
| case expiration = "expiration" | ||
| case sentAt = "sent-at" | ||
| } | ||
|
|
||
| public func durationTime() -> TimeInterval? { | ||
| guard let durationInMinutes = durationInMinutes else { | ||
| return nil | ||
| } | ||
| return TimeInterval(minutes: durationInMinutes) | ||
| } | ||
|
|
||
| public static func includedInNotification(_ notification: [String: Any]) -> Bool { | ||
| return notification["override-name"] != nil | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| // | ||
| // RemoteNotification.swift | ||
| // NightscoutUploadKit | ||
| // | ||
| // Created by Bill Gestrich on 2/25/23. | ||
| // Copyright © 2023 Pete Schwamb. All rights reserved. | ||
| // | ||
|
|
||
| import Foundation | ||
|
|
||
| public protocol RemoteNotification: Codable { | ||
|
|
||
| var id: String {get} | ||
| var expiration: Date? {get} | ||
| var sentAt: Date? {get} | ||
| var remoteAddress: String {get} | ||
|
|
||
| static func includedInNotification(_ notification: [String: Any]) -> Bool | ||
| } | ||
|
|
||
| extension RemoteNotification { | ||
|
|
||
| public var id: String { | ||
| //There is no unique identifier so we use the sent date when available | ||
| if let sentAt = sentAt { | ||
| return "\(sentAt.timeIntervalSince1970)" | ||
| } else { | ||
| return UUID().uuidString | ||
| } | ||
| } | ||
|
|
||
| public init(dictionary: [String: Any]) throws { | ||
| let data = try JSONSerialization.data(withJSONObject: dictionary) | ||
| let jsonDecoder = JSONDecoder() | ||
| jsonDecoder.dateDecodingStrategy = .formatted(DateFormatter.iso8601DateDecoder) | ||
| self = try jsonDecoder.decode(Self.self, from: data) | ||
| } | ||
| } | ||
|
|
||
| extension DateFormatter { | ||
| static var iso8601DateDecoder: DateFormatter = { | ||
| let formatter = DateFormatter() | ||
| formatter.locale = Locale(identifier: "en_US_POSIX") | ||
| formatter.dateFormat = "yyyy-MM-dd'T'HH:mm:ss.SSSZ" //Ex: 2022-12-24T21:34:02.090Z | ||
| return formatter | ||
| }() | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| // | ||
|
There was a problem hiding this comment. The tests in the next few files just take some data samples and ensures I can still create notifications from it. |
||
| // BolusRemoteNotificationTestCase.swift | ||
| // NightscoutUploadKitTests | ||
| // | ||
| // Created by Bill Gestrich on 2/25/23. | ||
| // Copyright © 2023 Pete Schwamb. All rights reserved. | ||
| // | ||
|
|
||
| import XCTest | ||
| @testable import NightscoutUploadKit | ||
|
|
||
| final class BolusRemoteNotificationTestCase: XCTestCase { | ||
|
|
||
| override func setUpWithError() throws { | ||
| } | ||
|
|
||
| override func tearDownWithError() throws { | ||
| } | ||
|
|
||
| func testParseBolusNotification_ValidPayload_Succeeds() throws { | ||
|
|
||
| //Arrange | ||
| let expectedRemoteAddress = "::ffff:11.2.44.155" | ||
| let sentAtDateString = "2023-02-25T20:46:35.778Z" | ||
| let expectedSentAtDate = dateFormatter().date(from: sentAtDateString)! | ||
| let expirationDateString = "2023-02-25T20:51:35.778Z" | ||
| let expectedExpirationDate = dateFormatter().date(from: expirationDateString)! | ||
| let expectedBolusInUnits = 1.1 | ||
| let expectedOTP = "123456" | ||
|
|
||
| let notification: [String: Any] = [ | ||
| "remote-address": expectedRemoteAddress, | ||
| "sent-at": sentAtDateString, | ||
| "expiration": expirationDateString, | ||
| "bolus-entry": expectedBolusInUnits, | ||
| "otp": expectedOTP | ||
| ] | ||
|
|
||
| //Act | ||
| let bolusNotification = try BolusRemoteNotification(dictionary: notification) | ||
|
|
||
| //Assert | ||
| XCTAssertEqual(bolusNotification.remoteAddress, expectedRemoteAddress) | ||
| XCTAssertEqual(bolusNotification.sentAt, expectedSentAtDate) | ||
| XCTAssertEqual(bolusNotification.expiration, expectedExpirationDate) | ||
| XCTAssertEqual(bolusNotification.amount, expectedBolusInUnits) | ||
| XCTAssertEqual(bolusNotification.otp, expectedOTP) | ||
| } | ||
|
|
||
|
|
||
| //MARK: Utils | ||
|
|
||
| func dateFormatter() -> ISO8601DateFormatter { | ||
| let formatter = ISO8601DateFormatter() | ||
| formatter.formatOptions = [.withInternetDateTime, .withFractionalSeconds] | ||
| return formatter | ||
| } | ||
|
|
||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| // | ||
| // CarbRemoteNotificationTestCase.swift | ||
| // NightscoutUploadKitTests | ||
| // | ||
| // Created by Bill Gestrich on 2/25/23. | ||
| // Copyright © 2023 Pete Schwamb. All rights reserved. | ||
| // | ||
|
|
||
| import XCTest | ||
| @testable import NightscoutUploadKit | ||
|
|
||
| final class CarbRemoteNotificationTestCase: XCTestCase { | ||
|
|
||
| override func setUpWithError() throws { | ||
| } | ||
|
|
||
| override func tearDownWithError() throws { | ||
| } | ||
|
|
||
| func testParseCarbNotification_ValidPayload_Succeeds() throws { | ||
|
|
||
| //Arrange | ||
| let expectedRemoteAddress = "::ffff:11.2.44.155" | ||
| let sentAtDateString = "2023-02-25T20:46:35.778Z" | ||
| let expectedSentAtDate = dateFormatter().date(from: sentAtDateString)! | ||
| let expirationDateString = "2023-02-25T20:51:35.778Z" | ||
| let expectedExpirationDate = dateFormatter().date(from: expirationDateString)! | ||
| let startDateString = "2023-02-25T20:46:35.778Z" | ||
| let expectedStartDate = dateFormatter().date(from: startDateString)! | ||
| let expectedCarbsInGrams = 15.1 | ||
| let expectedAbsorptionTimeInHours = 3.1 | ||
| let expectedFoodType = "🍕" | ||
| let expectedOTP = "12345" | ||
|
|
||
|
|
||
| let notification: [String: Any] = [ | ||
| "remote-address": expectedRemoteAddress, | ||
| "sent-at": sentAtDateString, | ||
| "expiration": expirationDateString, | ||
| "start-time": startDateString, | ||
| "carbs-entry": expectedCarbsInGrams, | ||
| "absorption-time": expectedAbsorptionTimeInHours, | ||
| "food-type": expectedFoodType, | ||
| "otp": expectedOTP | ||
| ] | ||
|
|
||
| //Act | ||
| let carbNotification = try CarbRemoteNotification(dictionary: notification) | ||
|
|
||
| //Assert | ||
| XCTAssertEqual(carbNotification.remoteAddress, expectedRemoteAddress) | ||
| XCTAssertEqual(carbNotification.sentAt, expectedSentAtDate) | ||
| XCTAssertEqual(carbNotification.expiration, expectedExpirationDate) | ||
| XCTAssertEqual(carbNotification.startDate, expectedStartDate) | ||
| XCTAssertEqual(carbNotification.amount, expectedCarbsInGrams) | ||
| XCTAssertEqual(carbNotification.absorptionInHours, expectedAbsorptionTimeInHours) | ||
| XCTAssertEqual(carbNotification.absorptionTime(), TimeInterval(hours: expectedAbsorptionTimeInHours)) | ||
| XCTAssertEqual(carbNotification.foodType, expectedFoodType) | ||
| XCTAssertEqual(carbNotification.otp, expectedOTP) | ||
| } | ||
|
|
||
|
|
||
| //MARK: Utils | ||
|
|
||
| func dateFormatter() -> ISO8601DateFormatter { | ||
| let formatter = ISO8601DateFormatter() | ||
| formatter.formatOptions = [.withInternetDateTime, .withFractionalSeconds] | ||
| return formatter | ||
| } | ||
|
|
||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| // | ||
| // OverrideCancelRemoteNotificationTestCase.swift | ||
| // NightscoutUploadKitTests | ||
| // | ||
| // Created by Bill Gestrich on 2/25/23. | ||
| // Copyright © 2023 Pete Schwamb. All rights reserved. | ||
| // | ||
|
|
||
| import XCTest | ||
| @testable import NightscoutUploadKit | ||
|
|
||
| final class OverrideCancelRemoteNotificationTestCase: XCTestCase { | ||
|
|
||
| override func setUpWithError() throws { | ||
| } | ||
|
|
||
| override func tearDownWithError() throws { | ||
| } | ||
|
|
||
| func testParseOverrideCancelNotification_ValidPayload_Succeeds() throws { | ||
|
|
||
| //Arrange | ||
| let expectedRemoteAddress = "::ffff:11.2.44.155" | ||
| let sentAtDateString = "2023-02-25T20:46:35.778Z" | ||
| let expectedSentAtDate = dateFormatter().date(from: sentAtDateString)! | ||
| let expirationDateString = "2023-02-25T20:51:35.778Z" | ||
| let expectedExpirationDate = dateFormatter().date(from: expirationDateString)! | ||
| let expectedCancelOverrideValue = true | ||
|
|
||
| let notification: [String: Any] = [ | ||
| "remote-address": expectedRemoteAddress, | ||
| "sent-at": sentAtDateString, | ||
| "expiration": expirationDateString, | ||
| "cancel-temporary-override": expectedCancelOverrideValue | ||
| ] | ||
|
|
||
| //Act | ||
| let overrideNotification = try OverrideCancelRemoteNotification(dictionary: notification) | ||
|
|
||
| //Assert | ||
| XCTAssertEqual(overrideNotification.remoteAddress, expectedRemoteAddress) | ||
| XCTAssertEqual(overrideNotification.sentAt, expectedSentAtDate) | ||
| XCTAssertEqual(overrideNotification.expiration, expectedExpirationDate) | ||
| XCTAssertEqual(overrideNotification.cancelOverride, expectedCancelOverrideValue) | ||
| } | ||
|
|
||
|
|
||
| //MARK: Utils | ||
|
|
||
| func dateFormatter() -> ISO8601DateFormatter { | ||
| let formatter = ISO8601DateFormatter() | ||
| formatter.formatOptions = [.withInternetDateTime, .withFractionalSeconds] | ||
| return formatter | ||
| } | ||
|
|
||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
These are the Codable models used for Nightscout. I'm including these with the NightscoutUploadKit which I feel is the "client" of Nightscout APIs. It could be argued this is more part of the "NightscoutService" domain as these are only exposed in the context of the plugin. Nevertheless there will be client things for Remote 2.0 later that will go in here so I'm starting here to avoid fragmenting things unnecessarily.
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.
If they are things that any iOS app wanting to talk to Nightscout might use (upload, download), then it belongs in NightscoutKit. If it's specific to Loop, then it goes in NightscoutService.
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.
It sounds like all these Codable models should move to the NightscoutService as these are all Loop specific.
Do you see the role of NightscoutService as also serving other apps like Caregiver, associated with Loop, that needs to make Loop-specific Nightscout api calls?
If so, we would need to add something like a NightscoutLoopAPI to NightscoutService that handles those network things, similarly to NightscoutKit. I ask as the the notifications endpoint in NightscoutKit is currently doing some Loop domain things so probably doesn't belong there.
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.
Hmm, I don't think so. NightscoutService is a Loop Plugin. It could vend onboarding screens, settings for controlling upload, all that kind of stuff.
I think CareGiver should probably have its own CareGiver <-> NightscoutKit conversion layer, or maybe CareGiver would interface with NightscoutKit directly.
The question is really about the commands themselves right? CareGiver creates commands, NightScout helps transfer them, and Loop consumes them. And I think there is a sense that some of the work of dealing with those commands or modeling them should be shared, right?
One of the questions I have is, does Nightscout have awareness of the contents of these commands, or are we assigning Nightscout the role of a dumb pipe? I think I'd argue against it being a dumb pipe, and would say that AID remote commands is a domain that Nightscout is privy to. Nightscout already does some remote command issuance, and I could see that being developed further. So I think that means that the Nightscout model includes the commands, and that any client (with proper authorizations and such) should be able query/create commands destined for the Looper's system. And possibly Non-Loop AID systems might want to use Nightscout and a even a different caregiver app in a similar way.
So I'm suggesting we treat these models as primarily Nightscout models, and part of the Nightscout API. Loop can consume Nightscout "Remote Commands", either via API, or by Nightscout pushing to Loop. And then they belong in NightscoutKit, which is the Swift front end to the Nightscout API.
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.
Nightscout is being used as a "dumb pipe" as you mentioned. I had considered a more domain-aware API as you mentioned but didn't want to get ahead of myself. I can work towards moving that direction when we get there.
On that matter, the NS reviewer sulkaharo had suggested on my NS Draft PR that this remote commands feature be coordinated with the wider community to potentially make it cross-compatible with other implementations (Like AndroidAPS). Your comments though also suggest a move in that direction.