Skip to content

Conversation

@mpangburn
Copy link
Contributor

Relevant discussion in #816.
Summary of changes:

  • Create a new class, WatchComplicationUpdateManager, whose sole purpose is to answer the question "should a complication user info transfer be used now?"
  • Create a new struct, WatchSettings, which includes user preferences for Watch app and complication behavior. These settings are stored in WatchDataManager.
  • Create a conditionally-visible section in the Settings screen for configuring the Watch settings. Right now, the only customization is the user's waking hours, which are used to restrict the time frame in which complication user info transfers are used. Waking hours are modeled by a new struct, DailyScheduleInterval, which describes a continuous period of time in a daily schedule.

Gardening:

  • WatchContext is given a name field to avoid the "nil name field means context" dance we've been doing.

Points for discussion:

  • I'm not thrilled with exposing the waking hours as a user-configurable setting, but the impact on the user info transfer interval is significant. Hiding this setting and assuming a default range would alienate users whose sleeping schedules are non-traditional.
  • DatePickerTableViewCell is ripped directly from LoopKit. DateAndDurationTableViewCell is also ripped from LoopKit, with one small to specialize behavior for the picker configuration in updateDateLabel(). I've copied them over to contain this PR to Loop, but exposing these classes publicly in LoopKit is probably a cleaner solution.
  • I've left in logging of user info transfer update counts, at least for testing purposes. These logs make it easy to track usage patterns in Loggly.

@ps2
Copy link
Collaborator

ps2 commented Oct 12, 2018

A few notes:

  • Can this be simplified to only specify a bedtime?
  • Clock times should always be stored in DateComponents; a time interval of 8h from midnight doesn't always mean 8 AM.
  • I think there's a type which is a (start: DateComponents, end: DateComponents), so you don't need to have an Either type to consider disjoint; you can ask the calendar the next dates that match those components.

@mpangburn
Copy link
Contributor Author

  • Can this be simplified to only specify a bedtime?

Without a wake time, we don't have the full picture of the user's waking hours. To condense the interval over which to distribute transfers, we need both parameters here.

  • Clock times should always be stored in DateComponents; a time interval of 8h from midnight doesn't always mean 8 AM.

I've refactored DailyScheduleTime to use DateComponents, though I've kept it as a separate struct to improve compile-time safety since only the hour and minute components are pertinent here.

  • I think there's a type which is a (start: DateComponents, end: DateComponents), so you don't need to have an Either type to consider disjoint; you can ask the calendar the next dates that match those components.

I can't find any such type, though I've made sure to remove any usage of TimeInterval in calendar computations.

@ps2
Copy link
Collaborator

ps2 commented Oct 13, 2018

It seems like complication pushes might not decrement the number of future updates when on the charger? Have you seen that?

@ps2
Copy link
Collaborator

ps2 commented Oct 13, 2018

If we can can figure the number of hours from the current date to bedtime, and try to spread the remaining complication pushes over that time, then we don't need a wake time.

@mpangburn
Copy link
Contributor Author

I've seen the same, yes—complication updates aren't used up when the Watch is charging.

We might be able to get away with only a bedtime if everyone took off their Watch when going to bed, but not everyone does. The motivation for leaving it on isn't super obvious to me (third-party sleep-tracking apps, maybe?), but I know a number of people who only take off their Watch to charge while getting ready in the morning.

@mpangburn
Copy link
Contributor Author

Closing for now. We can do this better using HK data.

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