Skip to content
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

Feature/weightscale_bluetooth #6

Merged
merged 22 commits into from
May 22, 2024
Merged

Feature/weightscale_bluetooth #6

merged 22 commits into from
May 22, 2024

Conversation

nriedman
Copy link
Contributor

@nriedman nriedman commented May 12, 2024

Connect to weight scale, Save and display recorded measurements

♻️ Current situation & Problem

Currently, the application is unable to connect to the weight scale, record weight measurements, and save them to cloud storage.

⚙️ Release Notes

  • Application now automatically connects to nearest device advertising the Weight Scale Service as defined in the Bluetooth LE protocol.
  • If the device is new, the application will prompt the user to pair the device.
  • New measurements are managed by the MeasurementManager class.
  • When a new weight measurement is recorded, a sheet will appear prompting the user to confirm it, and either save or discard the new measurement. This will appear over any tab of the home view, wherever the user happens to be at the time.
  • If the user selects save, the measurement is converted to an Apple HealthKit HKQuantitySample, then uploaded to cloud storage (Firestore) as a FHIR Observation via HealthkitOnFHIR.
  • Also edited the Github action workflow for building and testing to fix a bug preventing previous PR's from passing. This related to the test runs not being signed into a Google Firebase account.

📚 Documentation

More thorough in-line documentation will be included along with testing in the next PR.

✅ Testing

UI Tests will be implemented in the next PR.

Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

@nriedman nriedman requested a review from PSchmiedmayer May 12, 2024 08:44
Copy link

codecov bot commented May 12, 2024

Codecov Report

Attention: Patch coverage is 7.36041% with 365 lines in your changes are missing coverage. Please review.

Project coverage is 52.72%. Comparing base (5c6ce1f) to head (377ac4c).
Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main       #6       +/-   ##
===========================================
+ Coverage   23.37%   52.72%   +29.36%     
===========================================
  Files          27       32        +5     
  Lines         856     1159      +303     
===========================================
+ Hits          200      611      +411     
+ Misses        656      548      -108     
Files Coverage Δ
...tooth/Devices/WeightScale/WeightScaleService.swift 100.00% <100.00%> (ø)
ENGAGEHF/ENGAGEHF.swift 100.00% <ø> (+5.89%) ⬆️
ENGAGEHF/ENGAGEHFDelegate.swift 96.67% <100.00%> (+1.84%) ⬆️
ENGAGEHF/ENGAGEHFStandard.swift 47.55% <0.00%> (+43.29%) ⬆️
ENGAGEHF/Home.swift 88.89% <73.34%> (+88.89%) ⬆️
...etooth/Devices/WeightScale/WeightScaleDevice.swift 31.25% <31.25%> (ø)
ENGAGEHF/Bluetooth/Views/MeasurementLayer.swift 0.00% <0.00%> (ø)
ENGAGEHF/Bluetooth/Views/ViewElements.swift 0.00% <0.00%> (ø)
...GEHF/Bluetooth/Views/MeasurementRecordedView.swift 0.00% <0.00%> (ø)
...ightScale/Characteristics/WeightScaleFeature.swift 0.00% <0.00%> (ø)
... and 3 more

... and 11 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15b47f6...377ac4c. Read the comment docs.

Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nriedman Amazing work with this PR for the first time working with Spezi Bluetooth and extending the app with this functionality.

I had a few comments across the PR that would be great to be addressed and would benefit the current logic and overall structure.

Let me know if you have any questions and feel free to re-request a review once you have addressed the different elements in this PR.

ENGAGEHF.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
ENGAGEHF.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
ENGAGEHF/Bluetooth/Views/MeasurementRecordedView.swift Outdated Show resolved Hide resolved
ENGAGEHF/Bluetooth/Views/MeasurementRecordedView.swift Outdated Show resolved Hide resolved
ENGAGEHF/Bluetooth/Views/MeasurementRecordedView.swift Outdated Show resolved Hide resolved
ENGAGEHF/Resources/Localizable.xcstrings Outdated Show resolved Hide resolved
@PSchmiedmayer PSchmiedmayer mentioned this pull request May 15, 2024
1 task
@nriedman nriedman requested a review from PSchmiedmayer May 20, 2024 21:33
Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nriedman Thank you for all the improvements. I only had a few smaller comments that would be great to be addressed before the PR is merged 👍

Apart from these comments, feel free to merge the PR once they are addressed; great job!

ENGAGEHF.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
ENGAGEHF/Bluetooth/MeasurementManager.swift Outdated Show resolved Hide resolved
ENGAGEHF/Bluetooth/MeasurementManager.swift Outdated Show resolved Hide resolved
ENGAGEHF/Bluetooth/MeasurementManager.swift Show resolved Hide resolved
ENGAGEHF/Bluetooth/MeasurementManager.swift Outdated Show resolved Hide resolved
ENGAGEHF/Bluetooth/Views/MeasurementLayer.swift Outdated Show resolved Hide resolved
ENGAGEHF/Bluetooth/Views/ViewElements.swift Show resolved Hide resolved
ENGAGEHF/Home.swift Outdated Show resolved Hide resolved
ENGAGEHF/Home.swift Outdated Show resolved Hide resolved
ENGAGEHF/Home.swift Outdated Show resolved Hide resolved
Copy link
Member

@PSchmiedmayer PSchmiedmayer left a 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 addition improvements @nriedman!

@nriedman nriedman enabled auto-merge (squash) May 22, 2024 16:30
@PSchmiedmayer PSchmiedmayer disabled auto-merge May 22, 2024 18:41
@PSchmiedmayer PSchmiedmayer merged commit 9e3715d into main May 22, 2024
6 of 7 checks passed
@PSchmiedmayer PSchmiedmayer deleted the feature/bluetooth branch May 22, 2024 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants