Skip to content

Move Health Connect to main module #5612

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Move Health Connect to main module #5612

wants to merge 9 commits into from
+27 −58

Conversation

@dshokouhi
Copy link
Member

@dshokouhi dshokouhi commented Jul 31, 2025 •
edited
Loading

Summary

Based on #5605 health connect should work on the minimal build if the SDK is available. We have a user who can test on a de-googled phone to confirm it works for them. Our hasSensor check should work as is here.

Checklist

  • New or updated tests have been added to cover the changes following the testing guidelines.
  • The code follows the project's code style and best_practices.
  • The changes have been thoroughly tested, and edge cases have been considered.
  • Changes are backward compatible whenever feasible. Any breaking changes are documented in the changelog for users and/or in the code for developers depending on the relevance.

Screenshots

Link to pull request in documentation repositories

User Documentation: home-assistant/companion.home-assistant#1230

Any other notes

We should probably get some users to test and confirm, on my end sensors continue to work as expected.

I used android studio to move the files and committed as is, the Companion addition in multiple locations surprised me but works.

Copilot AI review requested due to automatic review settings July 31, 2025 18:35
@dshokouhi dshokouhi linked an issue Jul 31, 2025 that may be closed by this pull request
Add Health Connect sensor to minimal app #5605
Open
Copilot AI reviewed Jul 31, 2025
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR moves the Health Connect sensor manager from the minimal build variant to the main module, enabling Health Connect functionality to work on minimal builds when the SDK is available. The key motivation is to support de-googled phones where Health Connect may be present.

  • Health Connect functionality is now available in minimal builds
  • Import aliases are replaced with direct imports and companion object references
  • The stub implementation is removed from the minimal variant

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
app/src/minimal/kotlin/io/homeassistant/companion/android/sensors/HealthConnectSensorManager.kt Removes the stub implementation that was previously used in minimal builds
app/src/main/kotlin/io/homeassistant/companion/android/sensors/HealthConnectSensorManager.kt Updates imports and companion object references after moving to main module

github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 31, 2025
View reviewed changes
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

ktlint found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@dshokouhi dshokouhi mentioned this pull request Jul 31, 2025
Open
7 tasks
@jpelgrom
Copy link
Member

jpelgrom commented Jul 31, 2025

Why remove the import io.homeassistant.companion.android.common.R alias? If we keep that there are less changes and I think it's a nice alias to avoid confusion about which module's R is used.

@dshokouhi
Copy link
Member Author

dshokouhi commented Jul 31, 2025

Why remove the import io.homeassistant.companion.android.common.R alias? If we keep that there are less changes and I think it's a nice alias to avoid confusion about which module's R is used.

i can switch it back, that was all android studio lol

@jpelgrom
Copy link
Member

jpelgrom commented Jul 31, 2025

Simple move and changing the dependency looks good, also after reviewing the docs, but let's wait for user feedback before merging.

@jpelgrom jpelgrom mentioned this pull request Oct 24, 2025
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@TimoPtr TimoPtr TimoPtr approved these changes

Assignees

No one assigned

Labels

cla-signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add Health Connect sensor to minimal app

3 participants

@dshokouhi @jpelgrom @TimoPtr