Add carbs to healthkit by dyjaks · Pull Request #165 · ivalkou/freeaps

dyjaks · 2021-12-31T20:29:33Z

Add carbs to healthkit
Should probably refactor carbs/tmp targets to their own files/subscriptions
Entires show up twice in health but health freeapx knows both don't count? All seems to load properly.
Carbs deleted in health don't delete from FreeAPS. Carbs deleted from freeAPS do delete from health.

Most of HealthKitManager.swift is now duplicate code and should be made better but as I don't know swift or ios I'm hitting a limit here of what I can do

Carbs in the future don't show. This is an easy fix but I feel is more a design decision. OpenAPS is not carb forward looking so should they be shown?

Properly check permissions. Enable background delivery.

dyjaks · 2022-01-11T19:22:16Z

I think this is ready enough for review. Health will sync now with FreeAPS and FreeAPS will read carbs from health but it will not read deleted carbs from health, if that makes sense.

More simply put, carbs can be read and entered from health but if you want to delete it must be done from faps.

ivalkou · 2022-01-17T11:59:50Z

@DobbyWanKenoby PTAL

ivalkou · 2022-01-19T11:18:25Z

I think it's OK, but we need FAX to Health carbs transfer also

dyjaks · 2022-01-19T12:23:30Z

I think it's OK, but we need FAX to Health carbs transfer also

Added. Wanted to put in in carb storage but that caused a circular dependency.

DobbyWanKenoby · 2022-01-28T11:29:33Z

@dyjaks Good job! You did very good job! We need to add some fix.

I finded 2 problem:

Dublicates of items in Health. https://ibb.co/rvX0Pnf
I think there are 2 reasons. First: id property of CarbEntry. Each time id different. And after get data from NS they are dublicated. Second: need filtered healthkit data before save it to healthkit again (see comments in code).

I keep checking this problem

Manual entered carbs data from FAX don't transfer to Health.

FreeAPS.xcodeproj/project.pbxproj

FreeAPS/Sources/APS/Storage/CarbSource.swift

FreeAPS/Sources/APS/FetchTreatmentsManager.swift

DobbyWanKenoby · 2022-01-28T10:43:45Z

FreeAPS/Sources/Services/HealthKit/HealthKitManager.swift

+                CarbsEntry(
+                    _id: sample.healthKitId,
+                    createdAt: sample.date,
+                    carbs: sample.carb as? Decimal ?? 0.0,


Conditional downcast from Decimal? to Decimal does nothing
sample.carb is Decimal already

Edit to
carbs: sample.carb ?? 0.0

FreeAPS/Sources/Services/HealthKit/HealthKitManager.swift

DobbyWanKenoby · 2022-01-31T14:39:11Z

Houston we have a problem
Carbs loading from NS doesn't work now

Problem with CarbEntry. You added id property, but NS send _id. As a result decoding doesn't work. I will add comments to code

DobbyWanKenoby · 2022-01-31T17:52:38Z

After a lot of different tests with adding/removing carbs and different situation (such as bad signal), I think, that we shouldn't add carbs to HealthKit in FreeAPS/Sources/APS/FetchTreatmentsManager.swift subscribe method.
All we need for HealthKit support is use CarbObserver protocol in BaseHealthKitManager like Ivan did it in BaseNightscoutManager. After that all new carbs (from FAX, from NS, and others) will send to HealthKitManager.

To BaseHealthKitManager add:

private func subscribe() {
    broadcaster.register(CarbsObserver.self, observer: self)
}

To BaseHealthKitManager.init(resolver:) add

init(resolver: Resolver) {
    // ...
    subscribe()
}

At the end of file add

extension BaseHealthKitManager: CarbsObserver {
    func carbsDidUpdate(_ carbs: [CarbsEntry]) {
        saveIfNeeded(carbs: carbs)
    }
}

DobbyWanKenoby · 2022-01-31T17:56:23Z

Ok finished review.
So, we edit model (CarbsEntity) and now have problem with old carbs entities.
In several days I will release new MigrationManager and resolve this problem

DobbyWanKenoby · 2022-01-29T06:21:11Z

FreeAPS.xcodeproj/project.pbxproj

 			isa = PBXGroup;
 			children = (
 				38F3783A2613555C009DB701 /* Config.xcconfig */,
-				3818AA42274BBC1100843DB3 /* ConfigOverride.xcconfig */,


DobbyWanKenoby · 2022-01-29T06:24:34Z

FreeAPS.xcodeproj/project.pbxproj

 		3811DEE725CA063400A708ED /* PersistedProperty.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PersistedProperty.swift; sourceTree = "<group>"; };
 		3811DF0125CA9FEA00A708ED /* Credentials.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Credentials.swift; sourceTree = "<group>"; };
 		3811DF0F25CAAAE200A708ED /* APSManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = APSManager.swift; sourceTree = "<group>"; };
-		3818AA42274BBC1100843DB3 /* ConfigOverride.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = ConfigOverride.xcconfig; sourceTree = "<group>"; };


I had same problem in past, when Ivan added ConfigOverride.xcconfig file.
You deleted wrong copy of the file.

Previous version of PR you have 2 files ConfigOverride.xcconfig, one was old (original), and second - new (your).
We need only old file, but now you deleted original.

See checksum (left side) and compare it with original in FreeAPS.xcodeproj/project.pbxproj

freeaps/FreeAPS.xcodeproj/project.pbxproj

Line 483 in b8a0956

3818AA42274BBC1100843DB3 /* ConfigOverride.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = ConfigOverride.xcconfig; sourceTree = "<group>"; };

You have to return this file, and delete next with checksum '29AC4F65277F48C100766404'

DobbyWanKenoby · 2022-01-29T06:25:55Z

FreeAPS.xcodeproj/project.pbxproj

 				CURRENT_PROJECT_VERSION = 1;
 				DEVELOPMENT_ASSET_PATHS = "";
-				DEVELOPMENT_TEAM = "${DEVELOPER_TEAM}";
+				DEVELOPMENT_TEAM = 4W28235M63;


Please return back "${DEVELOPER_TEAM}"
This change automatically with ConfigOverride.xcconfig

DobbyWanKenoby · 2022-01-29T06:26:07Z

FreeAPS.xcodeproj/project.pbxproj

 				CURRENT_PROJECT_VERSION = 1;
 				DEVELOPMENT_ASSET_PATHS = "";
-				DEVELOPMENT_TEAM = "${DEVELOPER_TEAM}";
+				DEVELOPMENT_TEAM = 4W28235M63;


DobbyWanKenoby · 2022-01-29T06:26:12Z

FreeAPS.xcodeproj/project.pbxproj

 				CODE_SIGN_STYLE = Automatic;
 				CURRENT_PROJECT_VERSION = "$(BUILD_VERSION)";
-				DEVELOPMENT_TEAM = "${DEVELOPER_TEAM}";
+				DEVELOPMENT_TEAM = 4W28235M63;


DobbyWanKenoby · 2022-01-31T15:25:32Z

FreeAPS/Sources/Modules/AddCarbs/AddCarbsStateModel.swift

-                CarbsEntry(createdAt: date, carbs: carbs, enteredBy: CarbsEntry.manual)
-            ])
+            let carbArray = [
+                CarbsEntry(id: ID, createdAt: date, carbs: carbs, enteredBy: CarbsEntry.manual)


replace to CarbsEntry(createdAt: date, carbs: carbs, enteredBy: CarbsEntry.manual)

In CarbsEntry we added id property with initial value

DobbyWanKenoby · 2022-01-31T15:27:36Z

FreeAPS/Sources/Models/CarbsEntry.swift

 import Foundation

 struct CarbsEntry: JSON, Equatable, Hashable {
+    let id: String


replace to var id = UUID().uuidString

But after adding new property (id) old carbs will not display, and it's a problem.
The reason is because old carbs entities (storing at carbs.json) don't contain id and cant decode.

In several days I will release new MigrationManager and resolve this problem

DobbyWanKenoby · 2022-01-31T15:28:05Z

FreeAPS/Sources/Models/CarbsEntry.swift


 extension CarbsEntry {
    private enum CodingKeys: String, CodingKey {
+        case id


Answer (JSON) from NS have _id field. This is reason because import from NS doesn't work now.
Replace to case id = "_id"

DobbyWanKenoby · 2022-01-31T16:00:43Z

FreeAPS/Sources/APS/Storage/CarbsStorage.swift

-            broadcaster.notify(CarbsObserver.self, on: processQueue) {
-                $0.carbsDidUpdate(uniqEvents)
+
+            DispatchQueue.main.async {


Why did you fix this code?
I think it must be edit back

DobbyWanKenoby · 2022-01-31T17:28:49Z

FreeAPS/Sources/Services/WatchManager/WatchManager.swift

+                CarbsEntry(id: UUID().uuidString, createdAt: Date(), carbs: Decimal(carbs), enteredBy: CarbsEntry.manual)
+            ]
+            carbsStorage.storeCarbs(carbArray)
+            healthKitManager.saveIfNeeded(carbs: carbArray)


Please read comment, where I tell about CarbObserver.
After that these edits are unnecessary

DobbyWanKenoby · 2022-12-22T10:48:12Z

New realization in #188

dyjaks added 5 commits December 31, 2021 08:09

Add carbs to health kit

28586cc

Refactor and properly fetch glucose

0311c5a

Create process queue for carbs instead of overloading glucose queue.

3401783

Properly check permissions. Enable background delivery.

Split out carb/bg observers

14e4354

Fix delete.

dd0c30b

dyjaks changed the title ~~[WIP] Add carbs to healthkit~~ Add carbs to healthkit Jan 11, 2022

Add carbs from FAPX to health

af9ec3c

DobbyWanKenoby reviewed Jan 28, 2022

View reviewed changes

dyjaks added 2 commits January 28, 2022 17:03

PR Feedback

40a8d2f

Put in nightscout carb filter

dae8954

DobbyWanKenoby reviewed Jan 31, 2022

View reviewed changes

ivalkou added enhancement New feature or request not ready yet labels Mar 14, 2022

Merge branch 'ivalkou:dev' into dev

6a9598a

DobbyWanKenoby closed this Dec 22, 2022

Conversation

dyjaks commented Dec 31, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dyjaks commented Jan 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ivalkou commented Jan 17, 2022

Uh oh!

ivalkou commented Jan 19, 2022

Uh oh!

dyjaks commented Jan 19, 2022

Uh oh!

DobbyWanKenoby commented Jan 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DobbyWanKenoby Jan 28, 2022 • 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DobbyWanKenoby commented Jan 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DobbyWanKenoby commented Jan 31, 2022

Uh oh!

DobbyWanKenoby commented Jan 31, 2022

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!

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!

DobbyWanKenoby commented Dec 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dyjaks commented Dec 31, 2021 •

edited

Loading

dyjaks commented Jan 11, 2022 •

edited

Loading

DobbyWanKenoby commented Jan 28, 2022 •

edited

Loading

DobbyWanKenoby Jan 28, 2022 •

edited

Loading

DobbyWanKenoby commented Jan 31, 2022 •

edited

Loading