-
Notifications
You must be signed in to change notification settings - Fork 67
health integration #54
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
Conversation
|
|
|
Would this handle a user switching between two watches throughout the day? |
|
Not yet no, and some of the discussion on discord suggests some of my initial assumptions were incorrect on how the data storage works on the watch. This solution works but some steps I'm taking are unnecessary and could impact battery life. |
|
It might be relevant to have the Unit Distance (Miles/Kilometers) setting in the health integration of the mobileapp, if it is not already the case in this PR |
|
I recognize I might be going overboard with this one so lemme know if you folks want me to scale it back at all @sjp4 |
I added this in the health settings panel but it doesn't seem to immediately update on the watch. |
|
@sjp4 Picking this back up and trying to simplify it further. Would it be better to consolidate all datalogging into the DataLoggingService, rather than doing it in multiple locations? |
- Extract HealthDataProcessor from HealthService - Datalogging routes health tags (81-85) to HealthDataProcessor - DataLoggingService handles ALL datalogging packets and ACK/NACK - Remove duplicate packet handling from HealthService - HealthService now focuses on orchestration and public API Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Two critical fixes: 1. Emit healthUpdateFlow when health data is processed - HealthDataProcessor now emits to healthDataUpdated flow after inserting data - HealthService forwards these emissions to its healthUpdateFlow - This notifies UI (HealthStatsDialog) when new data arrives so it can refresh 2. Remove data blocking during reconciliation - Previously blocked ALL health data during reconciliation with setAcceptHealthData(false) - This caused reconciliation to hang because requested data was being dropped - waitForNewerHealthData() would timeout after 8s waiting for DB updates that never came - Now reconciliation allows data to flow freely - "Highest step count wins" strategy in DB handles any conflicts Fixes: "today's steps" now update correctly in UI after sync Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Removed complex reconciliation logic that was trying to handle multiple watches and prevent stale data. Now we just accept all incoming data from the watch and let the "highest step count wins" database strategy handle any conflicts. Removed: - reconcileWatchWithDatabase() function - waitForNewerHealthData() function - Data blocking during sync (setAcceptHealthData) - lastFullStatsUpdate state tracking - Related constants: HEALTH_SYNC_WAIT_MS, HEALTH_SYNC_POLL_MS, RECONCILE_DELAY_MS, FULL_STATS_THROTTLE_HOURS, TWENTY_FOUR_HOURS_MS Simplified: - No more reconciliation on watch connection - Daily stats update now just waits until 7 AM without throttling logic - sendHealthAveragesToWatch() no longer tracks last update time Result: Much simpler, more predictable health data flow. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
The refinement continues, I went ahead and consolidated all datalogging into DataLoggingService. Ready for re-review. |
|
@sjp4 I'm gonna dogfood this with my PTS for a few days to make sure it's working as expected. but the logic seems sound. Ready for another look and (hopefully) merge. |
|
@michaelthatsit Are these new tables designed for future synchronization with Apple Health or Google Health? |
I do believe they're at this point compatible but I can double check. There's a library I mentioned in an open issue that we can use for quick integrations for both services. If the plan is to simply integrate with those services and not provide a built in health panel, we should also probably not store more than a week of data at a time. |
Thank you for your efforts! I'll probably highly appreciate it once it has been merged. |
|
I vote to have them on boths places on pebble app and then just send them on apple/google/etc in the background. |
I'm personally more into the idea of a custom end point I can send the data to. There aren't many good self-hosted health tracking options out there. I'm planning to build one this year and being able to integrate directly into the mobile app would save me the effort of building a pebble app/face. |
Thanks! Taking a look today - is it OK if I push some commits to get it merged? |
- Fixed compilation on iOS (can't use String.format in common code) - Distance units does not belong in the activityPreferences entry - this broke changing any of the settings (even enabling health) because the watch couldn't parse it. This table actually should have multiple rows - I've refactored this (had to drop the previous table, so users will need to re-enable health after updating). - A lot of things were happening in the connection-scoped service which should have been in the global Health class - moved them over. - Many of the methods on libpebble/connected pebble weren't actually required: especially for requesting health sync (we can make the reuqest suspending until we get a response, and also note that the response does not mean that data has been synced via datalogging yet) - It was creating an endless loop of health sync requests whenever we manually request a sync (because the response message from the watch - which is just an ACK of the request - was being treated as a request to send another request..) - Refactored the 911 packet definitions - some of them were incorrect (or missing e.g. ACK vs NACK). - Changed injected coroutine scope for HealthDataProcessor to global rather than connection. I'm not sure how this didn't crash during koin init, but it shouldn't be able to use the connection scope because it's globally scoped in koin. - Moved the health debug screen behind debug settings flag - Fixed HealthParsingTest (was using wrong DataBuffer args?)
|
I pushed a bunch of fixes - see the commit message for details. A few things were broken which was stopping me from testing anything, and noticed more (including refactoring the existing settings model) as I went. I think sync requests and updating settings are working now - can you try out the updated branch, too? I'll go through the parsing/state code later - would be good to know it everything is still working for you before merging though - I made quite a lot of changes. |
@sjp4 Thanks for doing that and I appreciate the patience as I'm relatively new to Kotlin. I pulled and tested the changes and everything is working as expected. Are there any other updates you'd like me to make before we merge? |
|
@sjp4 I'll leave next steps up to you! After this merges I can take up Apple/Google health integration. |
|
Thank you all so much for doing this. Really looking forward to the return of Pebble Health! |
# Conflicts: # libpebble3/schema/io.rebble.libpebblecommon.database.Database/28.json # libpebble3/src/commonMain/kotlin/io/rebble/libpebblecommon/connection/FakeLibPebble.kt # libpebble3/src/commonMain/kotlin/io/rebble/libpebblecommon/connection/endpointmanager/blobdb/BlobDB.kt # libpebble3/src/commonMain/kotlin/io/rebble/libpebblecommon/database/Database.kt # libpebble3/src/commonMain/kotlin/io/rebble/libpebblecommon/di/LibPebbleModule.kt





this is pretty early. all it does is pull the health data from the watch, and push the averages to it.
I'm thinking it's probably best to add this and do any UI changes in a separate PR: In it I would probably add:
TODOs: