-
-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Query Property Wrappers, HealthChart, and Other Refactoring #27
Conversation
- Remove `CollectSamples`. The same functionality can be achieved using a for loop in the configuration builder. - Rework the HealthKit auth handling. We still cannot access the read auth request responses, but there's also no point in somehow trying to keep track of which type's we've already requested access to, so we now just keep track of _whether_ we've already requested access (which is the same thing). - Add an API for requesting write access - Rework the `HealthKitSampleType` to be more lean and generic (and to better support the individual types) - Add a couple more sample types
…rs until their respective sample types have been authorized in the previous approach, it would happen that if you were observing let's say 5 different sample types using eg `CollectSample`, it would "enable" all of these in a for loop, and each would then request authorization to its respective sample type. but since they kinda all were starting their queries at about the same time, you'd get a situation where only the first one to run would be able to present the HealthKit data access request sheet, and all other ones would fail to request access. The new approach works as follows: we try to combine all Health data we need to access into a single auth request, and we no longer automatically request access as part of e.g. starting a background CollectSample observer. instead, we give the user the responsibility to call HealthKit.askForAuthorization at some point. the (automatic) background queries will start once the permission request has completed (regardless of whether we actually were given access; we can't see that). This is better in several ways: - it avoids the issue of all background queries except the first failing to request access - it allows the app control over when exactly the HealthKit access request should happen. (Eg: you probably want to embed this into an onboarding flow, instead of just having it happen randomly at some point, without the user being informed in advance what;s about to happen) The simplest case (assuming no onboarding) would be that the app would simply have a `.task { try? await healthKit.askForAuthorization() }` somewhere in the root view. this would only result in the auth sheet being presented once, or when the access requirements change. otherwise, you can call this function without there being any side effects.
this sadly seems to be necessary to work around swiftlang/swift#78734 and swiftlang/swift#78735
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job with the PR and additions here @lukaskollmer!
I added some comments throughout the PR. Thank you for resolving all the remaining TODOs in this PR or we start to transform some of them into GitHub issues to be resolved later on in subsequent PRs.
We should also adjust the README (similar to other Spezi READMEs) to reflect the functionality & HealthChart examples. We should also copy the same content to the DocC setup & ensure that it also is having all the updated references and examples in place.
This will be a huge step forward for SpeziHealthKit and will bring it closer to a 1.0, thank you! 🚀
Sources/SpeziHealthKit/HealthKitDeliverySetting/HealthKitDeliverySetting.swift
Outdated
Show resolved
Hide resolved
Sources/SpeziHealthKit/Queries/HealthKitCharacteristicsQuery.swift
Outdated
Show resolved
Hide resolved
even though the Health data store isn't available on macOS, HealthKit as a framework still is.
query property wrappers, HealthChart, other refactoring
♻️ Current situation & Problem
The HealthKit module currently only provides access to HealthKit data via long-running anchor queries that deliver information about new/deleted objects to the app's Standard.
It does not provide any facilities for querying for past samples, or accessing health data from SwiftUI. This PR attempts to address these issues.
Furthermore, the HealthKit module is lacking an API allowing spezi users to integrate custom HealthKit permission requests into the module's permission handling (i.e., you currently can only request HealthKit access for some specific sample type by actively defining a long-running observer for that sample type).
Furthermore, this PR attempts to implement a
HealthChart
view, which can display various types of queried HealthKit data as a chart.resolves #8
requires StanfordSpezi/SpeziFoundation#19
requires StanfordBDHG/XCTestExtensions#28
⚙️ Release Notes
HealthKitQuery
property wrapperHealthKitStatisticsQuery
property wrapperHealthKitCharacteristicQuery
property wrapperHealthChart
viewHealthKit
configuration API to allow users to specify sample types the system should request read and/or write access toCollectSamples
. The same functionality can be achieved using afor
loop creating individualCollectSample
instances.📚 Documentation
All added new and changed existing APIs are documented. The DocC structure was reworked and some key aspects of SpeziHealthKit now have dedicated article/extension pages.
✅ Testing
The new and changed APIs are tested using both "normal" unit tests and UI tests. Existing tests were kept and adapted where possible
📝 Code of Conduct & Contributing Guidelines
By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines: