nightscout / Trio

MIT License
46 stars 130 forks source link

Pump/CGM state reverts on phone restart #231

Closed dnzxy closed 4 weeks ago

dnzxy commented 1 month ago

Describe the bug Upon phone restart, the app seldom reverts to old device state for CGM or pump, thus losing device connection. For pods this inherently leads to loss of device and need for replacement. This looks a lot like an old Loop bug is still present in Trio, but that mustn’t be the case as changes were migrated to FreeAPS and Trio in the past.

Screenshots Pod error  
IMG_0752 image0

screenshots by user @Sjoerd-Bo3 showing loss of pod connection and pairing info in bluetooth settings

Setup Information (please complete the following information):

Additional context Loop had a very similar issue, if not this issue, and the fix to this issue was moving device state info from UserDefaults to binary file (plist). See https://github.com/LoopKit/Loop/pull/1702

kskandis commented 1 month ago

This looks a lot like an old Loop bug is still present in Trio, but that mustn’t be the case as changes were migrated to FreeAPS and Trio in the past.

It looks like the https://github.com/LoopKit/Loop/pull/1702 fix is not imported to Trio. Trio imports https://github.com/LoopKit/LoopKit but not https://github.com/LoopKit/Loop where the fix to this is implemented. Some of the Trio corresponding code to LoopKit/Loop lives under https://github.com/nightscout/Trio/tree/dev/FreeAPS/Sources/APS, so Loop's fix needs to be merged into Trio's code, such as Loopkit/Loop/Managers/DeviceDataManager.swift,

dnzxy commented 1 month ago

@avouspierre had hinted on Discord that these changes were already imported to FAX and in turn to iAPS, so also Trio. I only recorded the issue here. I also thought this was still present in iAPS/Trio.

Quoting, as of 23/05/2024

I quickly checked but trio (freeaps in fact) seems implemented the plist strategy for the persistent wrapper.. so the fix of loop is probably already here..

If you can take a deeper look at this @kskandis and really compare the plist configs for Trio, please do 🙏

kskandis commented 1 month ago

If you can take a deeper look at this @kskandis and really compare the plist configs for Trio, please do 🙏

Yes, I would have thought, too, but when I looked at your link to the merged PR I saw that it was code in Loop! I will take a deeper look.

kskandis commented 1 month ago

@avouspierre had hinted on Discord that these changes were already imported

Yes, actually his changes are indeed implemented in Trio with this commit - see [FetchGlucoseManager.swift](https://github.com/nightscout/Trio/commit/47fa01786a7457b30e2307a11e02908719fa0aa9#diff-6456e698b2e52f8620335ab85f723632fe671650b5b8aae4b192b237f7c2f164) on Mar 17th for CGM and PR #15!

However, pumpManager still uses UserDefaults in DeviceDataManager.swift. So the pumpManager code needs to be updated similar to how CGM was implemented to use PersistentProperty instead of UserDefaults. Also, for existing users, the "legacyPumpManagerRawValue UserDefaults" should be cleared similar to Loop#1702 UserDefaults+Loop and DeviceDataManager.

dnzxy commented 1 month ago

@kskandis I messaged you on Discord last night and sent you a patch file, please check your inbox 😊 I had started that port last night, wasn’t sure if I got through and @aug0211 briefly tested the patch today and it seemed to lose the pod after force quitting the app, so it’s not yet done.

kskandis commented 1 month ago

I messaged you on Discord last night and sent you a patch file

Oh, sorry, I missed your DM. I've read it now. I also made a patch not realizing you did! I will DM you it. It is very similar to yours with some diffs ...

aug0211 commented 1 month ago

https://github.com/dnzxy/Trio/tree/pump-loss-fix-migration

Work being done in the above branch

kskandis commented 4 weeks ago

Should this be closed? I believe it was fixed with https://github.com/nightscout/Trio/pull/277