Repository navigation
Add LF Hub - #796
Add LF Hub#796bjorkert wants to merge 22 commits into
Conversation
…larms rung as system alarms by a notification service extension, and Speak BG, with invites, roles and a fallback to alarming in the app
…at once, rejected pushes and outages show on Home and in the Ready list, failed source changes can be undone, snoozes reach the hub before handled, and errors read plainly
# Conflicts: # LoopFollow.xcodeproj/project.pbxproj # fastlane/Fastfile
…p the tracked activity on start and end the others, and log system alarm snoozes and handled reports
… and remind an admin phone every two weeks while a newer hub is out
…the hub is reachable and red while it is not
… that name the extension
…arm locally until it does, and offer to disconnect
…t with the same Apple Developer account
The body was one long expression: the Form followed by about twenty modifiers, among them four alerts and two confirmation dialogs, each with generic closures and inline Binding(get:set:) values. Swift infers a modifier chain as a single expression, so every added modifier multiplies the work for the type checker. It gave up with "The compiler is unable to type-check this expression in reasonable time" and the build failed. The chain is now three pieces that body puts together: - form: the Form with its task, refresh, notification and sheet modifiers - deviceAlerts(_:): the error, replace, remove and make-member alerts - sourceAndConnectionDialogs(_:): the data-source dialog, the LF Hub mode offer and the disconnect dialog Each piece is checked on its own, which keeps every expression small. Two smaller changes help as well: the data-source dialog's title is worked out beforehand as a typed String, and its action uses if/else instead of a ternary of two awaits. The invite sheet now uses the existing patientDisplayName instead of repeating the same expression. Nothing changes in what the screen shows or does.
marionbarker
left a comment
There was a problem hiding this comment.
Review notes from reading the code (not built or run). The first five are about how hub alarms are adopted and handled.
🤖 Generated with Claude Code
| #if !targetEnvironment(macCatalyst) | ||
| if #available(iOS 26, *) { HubAlarmKit.stop(alarmID: alarmID) } | ||
| #endif | ||
| HubClient.shared.alarmHandled(alarmID) |
There was a problem hiding this comment.
Auto-stop reports a hub alarm as handled. stopAlarm() stops the AlarmKit alarm and POSTs /handled for whatever alarm is current whenever isHubMode. That includes when checkAlarms stops an adopted hub alarm because its condition isn't met locally.
Scenario: Hub mode is on but not confirmed (for example AlarmKit is denied, so drivesAlarms is false while the hub still delivers alarms because settings.alarms = isHubMode). A hub Low alarm arrives as a normal notification and is adopted as currentAlarm. The phone's own data lags the hub's, so the local Low check returns false, which leads to stopAlarm(), then HubAlarmKit.stop and alarmHandled. The hub stops repeating a Low nobody acknowledged.
Suggestion: report /handled only for a user snooze/acknowledge (e.g. from performSnooze), not for every stopAlarm().
There was a problem hiding this comment.
Real, though the effect is smaller than described: on the hub, /handled counts as a 2-minute snooze and clears the current alarm, so a Low that still holds fires again after that. It also only happens while the hub isn't driving the alarms, when the phone alarms itself anyway. Fixed: when the local check clears an alarm, the app no longer reports it handled or stops its system alarm; the hub stops that alarm itself once its own check clears it. Snooze, Acknowledge and Snooze All still report as before. (dee1724)
| // the Live Activity push-to-start uses interruption-level: passive with empty | ||
| // title/body and must not produce a banner or sound when LF is foregrounded. | ||
| // The hub's badge push is also silent, so only its badge is applied. | ||
| if content.interruptionLevel == .passive || (content.title.isEmpty && content.body.isEmpty) { |
There was a problem hiding this comment.
Hub system alarms aren't adopted while the app is in the foreground. The NSE sets interruptionLevel = .passive on every hub alarm that rings through AlarmKit, so this early return runs before the lfhubAlarmId adoption below. With the app open, the in-app Snooze/Snoozer doesn't act on the ringing alarm until a later status poll (up to about 60s) adopts it. Consider moving the adoptHubAlarm call above the passive check.
There was a problem hiding this comment.
Fixed: the alarm is adopted before the passive check (dee1724). Adoption there only happens while the hub drives the alarms, since the NSE also delivers passively when the phone has left the hub, and those alarms shouldn't be adopted. The system alarm's own Snooze button worked in this case all along.
| // them; this app checks the rest. A sound this app started for an alarm the hub | ||
| // checks stops; the hub's alarm rings instead. | ||
| var alarms = Storage.shared.alarms.value | ||
| let hubDrives = HubClient.shared.drivesAlarms |
There was a problem hiding this comment.
A local alarm can replace the adopted hub alarm. While the hub drives alarms, a local-only alarm type that fires sets currentAlarm to its own id (line 157). That replaces the adopted hub alarm, which is still ringing as a system alarm. Tapping in-app Snooze then snoozes the local alarm and calls alarmHandled for it, so the hub's alarm is never reported handled and keeps repeating.
There was a problem hiding this comment.
This can't happen with the current hub: API 1 checks every alarm type the app has, so while the hub drives, the list of alarms checked locally is empty and checkAlarms returns before that line. HubAlarmTypeTests.parity fails if a type is added that the hub doesn't check, which forces this to be handled then.
| alarms = HubClient.shared.alarmsCheckedHere(alarms) | ||
| let soundsHere = Observable.shared.currentAlarm.value.map { id in alarms.contains { $0.id == id } } ?? false | ||
| if Observable.shared.alarmSoundPlaying.value, !soundsHere { | ||
| AlarmSound.stop() |
There was a problem hiding this comment.
currentAlarm stays set after the sound stops. This stops the local sound for an alarm the hub now checks, but currentAlarm and currentAlarmTitleOverride aren't cleared. Because that alarm is now filtered out of alarms, no later tick clears it either. The Snooze UI stays up, and a later snooze sends /handled for an alarm the hub may not be sounding. Consider clearing currentAlarm here too.
There was a problem hiding this comment.
Fixed (dee1724), with one adjustment: currentAlarm is cleared only when the hub hasn't adopted that alarm. Clearing it unconditionally would drop an alarm the hub is sounding from the Snoozer until the next status read, and going through stopAlarm() would report it handled.
| if Observable.shared.alarmSoundPlaying.value, !soundsHere { | ||
| AlarmSound.stop() | ||
| } | ||
| if alarms.isEmpty { return } |
There was a problem hiding this comment.
Snooze All is skipped when every alarm is hub-checked. This early return runs before the Snooze All check below. If the user enables Snooze All while an adopted hub alarm rings, stopAlarm() is never called on this phone, and the AlarmKit alarm keeps ringing until the hub gets the uploaded config and sends a stop push (which depends on network and APNs). Consider moving the Snooze All check above this return.
There was a problem hiding this comment.
Fixed by moving the Snooze All check above the hub block (dee1724). Online, the hub already handled this: the config upload makes it send a stop push for the sounding alarm. The phone now also stops it locally, which covers the case where the upload or the push fails.
| listening = Task { [weak self] in | ||
| // Coming to the foreground runs a check of its own. | ||
| var lastCheck = Date() | ||
| while !Task.isCancelled, let self, self.isActive { |
There was a problem hiding this comment.
The long poll can't restart after this loop exits. When isActive turns false the loop ends, but listening is never set back to nil, so startListening()'s listening == nil guard blocks every later start. stopListening() is only called from change(to:) and on entering the background, so a mode change from elsewhere (settings/QR import, restore) leaves a finished Task in listening, and the open app stops getting live hub updates. Consider resetting listening = nil when the loop exits on its own.
There was a problem hiding this comment.
Leaving this as is. Every way out of Hub mode goes through change(to:), which calls stopListening(), including disconnect. QR import and restore don't touch the background mode. If it ever did happen, the next background/foreground cycle restarts the listener, and alarms don't depend on the long poll. Setting listening = nil at loop exit would also race with a stop and start and could leave two loops running.
| } | ||
| } | ||
|
|
||
| override func serviceExtensionTimeWillExpire() { |
There was a problem hiding this comment.
Expiry can deliver the content mid-change. serviceExtensionTimeWillExpire delivers the shared content while the Task may still be setting sound / interruptionLevel on another thread. A slow receipt (10s timeout) plus a ringOnce wait of up to 25s can pass the roughly 30s budget. The notification then goes out with its sound and the system alarm also rings, and the concurrent access is a data race on UNMutableNotificationContent. Consider capping the ringOnce wait shorter, or delivering a passive copy on expiry once a ring was scheduled.
There was a problem hiding this comment.
Leaving this as is. The receipt and the ring run concurrently (async let), and the ring-once deadline is absolute at 25 s from start, so they don't add up to 35 s. The sound is removed and the content made passive right after AlarmKit schedules the alarm, before the ring-once wait, so expiry during the wait already delivers the passive copy. The only remaining window is AlarmKit's schedule call hanging for about 30 s, where a notification with sound is the right fallback.
| // app's own sources. | ||
| if HubBundle.shared.isActive { | ||
| Task { | ||
| if await HubBundle.shared.data(for: .entries) != nil { |
There was a problem hiding this comment.
Redundant entries lookups (minor). In Hub mode the entries part is looked up here, then again in webLoadNSBGData (hasCurrent) and in NightscoutUtils.load (data(for:)). If the part goes stale between these checks, the tick ends early (Nightscout disabled) or fails with "No current LF Hub data", and that BG update is lost. Consider calling webLoadNSBGData directly and falling back to loadBGFromSources() on failure.
There was a problem hiding this comment.
Leaving this as is. The three checks run within milliseconds, so the part has to cross the 3-minute staleness boundary in exactly that gap, and only a follower without a Nightscout URL is affected. The reading is retried within 10–60 s, not lost. Falling back from webLoadNSBGData would change the Dexcom/Nightscout priority and could loop between them.
| func render(_ text: String, language: String, completion: @escaping (String?) -> Void) { | ||
| try? FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) | ||
| let old = (try? FileManager.default.contentsOfDirectory(atPath: directory.path)) ?? [] | ||
| for name in old where name.hasPrefix("lfhub-speak-") { |
There was a problem hiding this comment.
The previous Speak BG sound file can be deleted too early (minor). Every lfhub-speak-* file is deleted before the new one is written, including the file a just-delivered Speak BG notification still points to. If two speak pushes arrive close together, the first announcement can be cut off or fall back to the default sound. Names are also only unique to the second. Consider keeping the last one or two files, or pruning only older ones.
There was a problem hiding this comment.
Leaving this as is. The hub sends at most one speak push per new reading, so at least a minute apart, and every speak push has the same apns-collapse-id, so a newer one replaces the older notification anyway. Two speak notifications don't overlap in practice.
…ive hub alarms in the foreground, and check Snooze All before the hub filter
Adds LF Hub, a way to follow a family's push server instead of polling Nightscout or Dexcom from each phone. The hub drives the badge, Live Activity and Speak BG, and alarms are rung as system alarms by a new notification service extension, with a fallback to alarming in the app. Phones join through invites with roles, and LF Hub is offered as the Background Refresh mode right after connecting.
Hub failures are surfaced on Home and in the Ready list: a removed phone leaves at once, rejected pushes and outages are shown, failed source changes can be undone, and snoozes reach the hub before they are marked handled. Fastlane is updated for the new extension target.