-
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Supplemental Metrics for ECG Reading #47
Conversation
Some questions for @PSchmiedmayer:
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #47 +/- ##
==========================================
- Coverage 31.66% 30.33% -1.33%
==========================================
Files 33 33
Lines 992 1118 +126
==========================================
+ Hits 314 339 +25
- Misses 678 779 +101
Continue to review full report in Codecov by Sentry.
|
@MatthewTurk247 Sorry for the late response! While SpeziHealthKit is great to collect longitudinal data, I would suggest to use normal "custom" HealthKit queries for the 5 minute intervals around the measurements. I think it is fine to just store them in the database as they can be correlated with the ECG using the timestamps. In the long run we could work with FHIR references between the different documents but that might be too complicated for now. Just storing them as observations should be fine and should address our need. We would also need some additional logic around the correlation of incoming symptoms and how to correlate this with the ECG types. Let's use the meeting today to take a deep-dive into some of the implementation ideas and how we can address them as part of this and future PRs. |
…ned` to `PAWSDelegate`
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.
Thank you for the improvements @MatthewTurk247; the changes look great and are well structured! 🚀
Great job with your first PR!
@MatthewTurk247 I would merge the PR if you agree that you don't want to make any additional changes? I can override the missing patch requirement. It will be nearly impossible to easily unit test them with the current testing capabilities that are available for HealthKit. |
Yep, sounds good to me! I don't have any other changes in mind for this specific PR. So once that missing patch requirement is overridden and the PR is merged, I think it will make sense to open a new PR specifically for symptom tagging and linking to ECG recordings. |
Sounds great; I have merged the PR 🎉 |
Thank you for all the work and research that went into the PR @MatthewTurk247 🚀 |
Supplemental Metrics for ECG Reading
♻️ Current situation & Problem
Building on top of #40, this pull request seeks to package ECG readings with supplemental information like VO2 max and pulse rate in a way that is sensitive to the possible limitations of current recording capabilities.
⚙️ Release Notes
📚 Documentation
In-line documentation will be written in relevant files in conformance to the Spezi Documentation Guide.
✅ Testing
Tests may be added for ensuring that data is correctly serialized and properly handled.
Code of Conduct & Contributing Guidelines
By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines: