LOOP-887 Keycloak updates and DIY Sync by ps2 · Pull Request #560 · tidepool-org/Loop

ps2 · 2023-04-24T22:13:09Z

Keycloak changes ticket - https://tidepool.atlassian.net/browse/LOOP-887
DIY Sync ticket - https://tidepool.atlassian.net/browse/LOOP-4610

This is the Loop portion of the DIY sync, including keycloak updates.

Main changes:

Many translations
Implement PluginHost protocol for service plugins.
Support alerts issued from service plugins.
Starting to use some Swift async code instead of completion handlers (BolusEntryViewModel, and checkVersion protocol).
Warning in carb entry if override is selected (not in TL)
CGMManagers sorted by title
Prevent Automatic Dosing with Future Glucose
Add X-Large watch complication
Use maxBolus to set automaticDosingIOBLimit

Release 2.2.4

…into loop-release/v2.2.5

Loop release v2.2.5

Bring in LoopKit updates for 2.2.6 patch

…keys to watch extension plist

Updated translations from Lokalise on Mon Mar 20 12:48:17 CDT 2023

…Kit#1934)

…-updates

…updates

nhamming · 2023-05-01T09:12:22Z

Loop/Managers/Alerts/AlertManager.swift

-            let failureInterval = lastLoopDate.addingTimeInterval(.minutes(minutes)).timeIntervalSinceNow
-            guard failureInterval >= 0 else { break }
+            let warningInterval = TimeInterval(minutes: minutes)
+            let timeUntilNotification = lastLoopDate.addingTimeInterval(.minutes(minutes)).timeIntervalSinceNow


[nit] why not use the warning Interval to calculate timeUntilNotification?

Looking at the other changes, I don't understand this change. I think it created 2 potential issues and if those are issues it is not used until line 220.

nhamming · 2023-05-01T09:16:30Z

Loop/Managers/Alerts/AlertManager.swift

-                    timeInterval: failureInterval,
+                    timeInterval: warningInterval,


I worry about this change. If the lastLoopDate was not now, timeUntilNotification should be used, correct?

This is just to track which warning it was (20m/40m/etc) for forensics.

Then the name should be updated to reflect this. At least to me it doesn't read this way.

The Loop not looping warning has several different time intervals that it is triggered at, so a warningInterval seems like a descriptive name for that. Or is it something else that you are wanting to make clearer? Open to suggestions.

I'm looking at timeInterval for the StoredLoopNotRunningNotification. I'm not sure how this is used, so it is hard to suggest an alternative. If this is not used, then just remove it. If it is needed, then its need will guide the name. timeInterval is too general and allows the reader to make false assumptions.

It just keeps track of the particular time interval of this particular Loop not looping notification, so it seems like it's accurately named to me?

Removed this variable, which changes the serialized format of StoredLoopNotRunningNotification, which will potentially cause some historically issued alerts to not be recorded in the alert store is issued, affecting forensics, but not safety.

nhamming · 2023-05-01T09:21:06Z

Loop/Managers/Alerts/AlertManager.swift

-            if let failureIntervalString = formatter.string(from: failureInterval)?.localizedLowercase {
+            if let failureIntervalString = formatter.string(from: warningInterval)?.localizedLowercase {


I think this change is incorrect and believe it should be timeUntilNotification. Should the copy be updated to in at least %@?

Say that lastLoopDate was 15 minutes ago. The "20 minute" loop not looping alert should go off in 5 minutes. So timeUntilNotification should be five minutes. But the failure string should still indicate that it has been "20 minutes" since the last loop.

ah... yup. I agree

nhamming · 2023-05-01T09:24:17Z

Loop/Managers/Alerts/AlertManager.swift

-            let failureInterval = lastLoopDate.addingTimeInterval(.minutes(minutes)).timeIntervalSinceNow
-            guard failureInterval >= 0 else { break }
+            let warningInterval = TimeInterval(minutes: minutes)
+            let timeUntilNotification = lastLoopDate.addingTimeInterval(.minutes(minutes)).timeIntervalSinceNow


Looking at the other changes, I don't understand this change. I think it created 2 potential issues and if those are issues it is not used until line 220.

nhamming

I didn't detect anything. Same question though. Is there good test coverage for these updates? If not, considering adding.

ps2 and others added 30 commits October 3, 2020 20:13

Merge pull request LoopKit#1397 from LoopKit/dev

dc35b5b

Release 2.2.4

Updates for carthage build with Xcode 12.1

ec7e952

Update travis to use xcode12

18af414

Use embedded carthage binary

d3b89da

Update rileylink_ios to use pinned carthage

26b11d7

Remove files with bad characters

a20ff6b

Bump carthage deps to release branches

e4ee115

Update Amplitude, fix warnings, bump version

87e97eb

Bump cartfile revs

22b7295

Use version from xcconfig override

4e83db9

Use version from xcconfig override

412a985

Automatic bolus dosing strategy

79959ba

Automatic bolus cleanup

342f379

Show profile expiration in issue report

a29859c

Bump carthage revs

8664390

Bump carthage rev for rileylink_ios

4868150

Merge branch 'loop-release/v2.2.5' of https://github.com/LoopKit/Loop …

037ebe6

…into loop-release/v2.2.5

Fix LoopKit#1525 adjust margin before layout

003eb14

Bump carthage rev

c4fc328

Add profile expiration alert

4d4aa88

Profile alert text tweaks

eba2f9f

Localize profile alert action button text

5ca7629

Update rileylink_ios framework in Cartfile

88f6555

Bump rileylink_ios rev

e46cfb8

Merge pull request LoopKit#1529 from LoopKit/loop-release/v2.2.5

50020e4

Loop release v2.2.5

Bring in LoopKit updates for 2.2.6 patch

e28e3bc

Merge pull request LoopKit#1544 from LoopKit/loop-release/v2.2.6

b1a3b42

Bring in LoopKit updates for 2.2.6 patch

Xcode 13 updates

e81c5af

Add NSHealthShareUsageDescription and NSHealthUpdateUsageDescription …

e5abd9a

…keys to watch extension plist

Bump version

0333062

ps2 and others added 16 commits March 20, 2023 13:15

Merge pull request LoopKit#1963 from LoopKit/translations

9965046

Updated translations from Lokalise on Mon Mar 20 12:48:17 CDT 2023

localization fix

0ea36c6

Remove unused formatter

7d60a05

Add alert issuing capability to Services

cbcdfed

Move remote command parsing and validation to RemoteDataService (Loop…

3baeaf1

…Kit#1934)

readlink -f (LoopKit#1984)

d7e107d

Update versionCheck protocol for async

7a81397

Merge remote-tracking branch 'loopkit/dev' into ps2/LOOP-887/keycloak…

2351940

…-updates

Fix merge issues

6179688

checkVersion converted to async, and fix merge issues

411fcd4

Fix test

9fa5108

Fix test

76d18eb

Fix tests

ddfbc01

Allow workspace to specify custom app plist values.

733b3ee

Put missed meals notifications behind feature flag

d666489

Sync with dev

3cd5cf6

ps2 requested review from Camji55 and nhamming April 27, 2023 19:28

ps2 added 3 commits April 28, 2023 09:13

Merge remote-tracking branch 'origin/dev' into ps2/LOOP-887/keycloak-…

5feefbd

…updates

Fix merge issue

e19235e

Xcode project updates

4407ce8

nhamming reviewed May 1, 2023

View reviewed changes

nhamming self-requested a review May 1, 2023 09:16

nhamming reviewed May 1, 2023

View reviewed changes

nhamming self-requested a review May 1, 2023 09:25

ps2 added 2 commits May 1, 2023 12:20

Remove unused attribute

138dd63

Cleanup

9a8556d

nhamming approved these changes May 3, 2023

View reviewed changes

ps2 merged commit d7deb46 into dev May 3, 2023

ps2 deleted the ps2/LOOP-887/keycloak-updates branch May 3, 2023 23:21

		timeInterval: failureInterval,
		timeInterval: warningInterval,

		if let failureIntervalString = formatter.string(from: failureInterval)?.localizedLowercase {
		if let failureIntervalString = formatter.string(from: warningInterval)?.localizedLowercase {

Conversation

ps2 commented Apr 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ps2 May 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nhamming left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

ps2 commented Apr 24, 2023 •

edited

Loading

ps2 May 1, 2023 •

edited

Loading