Skip to content

Add Siri support for overrides#343

Merged
ps2 merged 35 commits into
LoopKit:devfrom
novalegra:novalegra/overrides-siri
Nov 27, 2020
Merged

Add Siri support for overrides#343
ps2 merged 35 commits into
LoopKit:devfrom
novalegra:novalegra/overrides-siri

Conversation

@novalegra

Copy link
Copy Markdown
Contributor

Currently, overrides aren't visible to Siri (meaning they can't be set with Shortcuts or via voice). There also isn't a great way to view the override history in Loop; the issue report does contain the history, but it's not in a very human-readable format.

This PR:

  1. has some minor LoopKit-level tweaks so Loop can donate an override preset intent when one is set
  2. adds an override history screen
  3. makes some under-the-hood improvements to override accounting in order to correctly determine the end date of an override

public let enactTrigger: EnactTrigger
public let syncIdentifier: UUID

public var actualEnd: End = .natural

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.

I moved this logic out of TemporayScheduleOverrideHistory and into here so the history screens can have access to it

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.

Can OverrideEvent be cleaned up to not need this then, since OverrideEvent contains a TemporaryScheduleOverride?

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.

Yes & done!

Comment thread LoopKitUI/UIColor.swift
Comment on lines +18 to +21
@nonobjc static let lightenedInsulin = BundleColor("Lightened Insulin") ?? systemOrange

@nonobjc static let darkenedInsulin = BundleColor("Darkened Insulin") ?? systemOrange

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.

Colors for override percentage gauge bar

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.

Colors used in LoopKit should be provided by the app layer in some way. Passing colors into the view constructor, or assigning attributes on the view, or setting up Environment attributes for SwiftUI views.

@ps2 ps2 left a comment

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.

Generally looks good code wise; colors should not be pulling from the LoopKit bundle, though. I haven't run it yet, but just looked at the code.

Comment thread LoopKitUI/UIColor.swift
Comment on lines +18 to +21
@nonobjc static let lightenedInsulin = BundleColor("Lightened Insulin") ?? systemOrange

@nonobjc static let darkenedInsulin = BundleColor("Darkened Insulin") ?? systemOrange

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.

Colors used in LoopKit should be provided by the app layer in some way. Passing colors into the view constructor, or assigning attributes on the view, or setting up Environment attributes for SwiftUI views.

public let enactTrigger: EnactTrigger
public let syncIdentifier: UUID

public var actualEnd: End = .natural

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.

Can OverrideEvent be cleaned up to not need this then, since OverrideEvent contains a TemporaryScheduleOverride?

@ps2 ps2 merged commit 0d8844e into LoopKit:dev Nov 27, 2020
@novalegra novalegra deleted the novalegra/overrides-siri branch December 29, 2020 20:04
ps2 pushed a commit that referenced this pull request Jun 11, 2021
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