Make all SensorManager injectable into a Set - #7049
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the sensor subsystem to make all SensorManager implementations constructor-injectable and contributed via Hilt multibindings (Set<SensorManager>), removing the need to pass Context through most sensor APIs. It also adds ClassGraph-based completeness tests to ensure new managers are always bound into the DI graph and introduces thin Android components (receivers/services) that forward framework callbacks to injectable managers.
Changes:
- Convert
SensorManagerto an injectable, dependency-carrying interface (applicationContext,sensorRepository,serverManager) and update call sites to removeContextparameters. - Add Hilt modules that bind sensor managers into
Set<SensorManager>and provide generated sensor catalog entries intoSet<BasicSensor>for:common,:app, and:wear. - Add completeness tests (ClassGraph scan vs injected set) and refactor certain Android components (notification listener, location/activity receivers) into “thin forwarders” to injectable managers.
Reviewed changes
Copilot reviewed 98 out of 98 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| wear/src/test/kotlin/io/homeassistant/companion/android/sensors/SensorManagerCompletenessTest.kt | Adds DI completeness test for wear SensorManagers using ClassGraph |
| wear/src/main/kotlin/io/homeassistant/companion/android/util/PreviewSensorManager.kt | Adds preview helper to build a real BatterySensorManager with stubbed deps |
| wear/src/main/kotlin/io/homeassistant/companion/android/util/PreviewData.kt | Removes direct BatterySensorManager() usage from preview data |
| wear/src/main/kotlin/io/homeassistant/companion/android/sensors/WetModeSensorManager.kt | Migrates to constructor injection + removes Context parameters |
| wear/src/main/kotlin/io/homeassistant/companion/android/sensors/WearSensorModule.kt | New Hilt module binding wear-specific managers into Set<SensorManager> + provides wear catalog sensors |
| wear/src/main/kotlin/io/homeassistant/companion/android/sensors/WearSensorCatalogModule.kt | Removes standalone wear catalog Hilt module (merged into WearSensorModule) |
| wear/src/main/kotlin/io/homeassistant/companion/android/sensors/TheaterModeSensorManager.kt | Migrates to constructor injection + removes Context parameters |
| wear/src/main/kotlin/io/homeassistant/companion/android/sensors/SensorWorker.kt | Exposes LastUpdateManager via entry point to satisfy new base worker dependency |
| wear/src/main/kotlin/io/homeassistant/companion/android/sensors/SensorReceiver.kt | Switches from manual manager list to injected Set<SensorManager> |
| wear/src/main/kotlin/io/homeassistant/companion/android/sensors/OnBodySensorManager.kt | Migrates to constructor injection + removes stored latestContext usage |
| wear/src/main/kotlin/io/homeassistant/companion/android/sensors/HeartRateSensorManager.kt | Migrates to constructor injection + removes stored latestContext usage |
| wear/src/main/kotlin/io/homeassistant/companion/android/sensors/HealthServicesSensorManager.kt | Migrates to constructor injection + removes Context parameters in key paths |
| wear/src/main/kotlin/io/homeassistant/companion/android/sensors/BedtimeModeSensorManager.kt | Migrates to constructor injection + removes Context parameters |
| wear/src/main/kotlin/io/homeassistant/companion/android/sensors/AppSensorManager.kt | Updates wear App sensor manager to delegate to injected base constructor |
| wear/src/main/kotlin/io/homeassistant/companion/android/notifications/MessagingManager.kt | Injects BluetoothSensorManager and passes it into BLE transmitter command path |
| wear/src/main/kotlin/io/homeassistant/companion/android/home/views/SensorUi.kt | Updates permissions/availability calls to new SensorManager API; fixes preview construction |
| wear/src/main/kotlin/io/homeassistant/companion/android/home/views/SensorsView.kt | Accepts managers as parameter (no longer references SensorReceiver.MANAGERS) |
| wear/src/main/kotlin/io/homeassistant/companion/android/home/views/SensorManagerUi.kt | Updates preview to build a battery sensor manager via helper |
| wear/src/main/kotlin/io/homeassistant/companion/android/home/views/HomeView.kt | Wires injected manager list from MainViewModel into sensor screens |
| wear/src/main/kotlin/io/homeassistant/companion/android/home/MainViewModel.kt | Injects Set<SensorManager> and updates management flows to new API |
| wear/gradle.lockfile | Locks ClassGraph dependency for wear unit tests |
| microwakeword/gradle.lockfile | Locks ClassGraph dependency for microwakeword unit tests (via shared test deps) |
| gradle/libs.versions.toml | Adds ClassGraph version + library alias |
| common/src/test/kotlin/io/homeassistant/companion/android/sensors/SensorManagerTest.kt | Updates test helper manager to satisfy new SensorManager interface requirements |
| common/src/test/kotlin/io/homeassistant/companion/android/sensors/SensorManagerCompletenessTest.kt | Adds DI completeness test for :common SensorManagers |
| common/src/test/kotlin/io/homeassistant/companion/android/common/sensors/AudioSensorManagerTest.kt | Updates test construction to constructor-injected AudioSensorManager |
| common/src/test/kotlin/io/homeassistant/companion/android/common/CommonTestModule.kt | Adds test-only Hilt module for common tests (providers for app/version/token/etc.) |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/TrafficStatsManager.kt | Migrates to constructor injection + removes Context parameters |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/TimeZoneManager.kt | Migrates to constructor injection + removes Context parameters |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/StorageSensorManager.kt | Migrates to constructor injection + removes Context parameters |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/StepsSensorManager.kt | Migrates to constructor injection + removes Context parameters |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/SensorWorkerBase.kt | Injects LastUpdateManager (no longer constructs it directly) |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/SensorUpdateReceiver.kt | Injects BluetoothSensorManager and returns it via Set<SensorManager> |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/SensorReceiverBase.kt | Changes managers type to Set, injects LastUpdateManager, updates calls to new API |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/SensorModule.kt | Adds multibinding @Binds @IntoSet for all common SensorManagers + provides common catalog sensors |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/SensorManager.kt | Removes context-based entrypoint access; adds injected deps + new API surface without Context params |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/ProximitySensorManager.kt | Migrates to constructor injection + removes Context parameters |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/PressureSensorManager.kt | Migrates to constructor injection + removes Context parameters |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/PowerSensorManager.kt | Migrates to constructor injection + removes Context parameters |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/PhoneStateSensorManager.kt | Migrates to constructor injection + removes Context parameters |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/NfcSensorManager.kt | Migrates to constructor injection + removes Context parameters |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/NextAlarmManager.kt | Migrates to constructor injection + removes Context parameters |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/MobileDataManager.kt | Migrates to constructor injection + removes Context parameters; simplifies branching |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/LightSensorManager.kt | Migrates to constructor injection + removes Context parameters |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/LastUpdateManager.kt | Migrates to constructor injection; sendLastUpdate no longer takes Context |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/LastRebootSensorManager.kt | Migrates to constructor injection + removes Context parameters |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/KeyguardSensorManager.kt | Migrates to constructor injection + removes Context parameters |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/DNDSensorManager.kt | Migrates to constructor injection + removes Context parameters |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/DisplaySensorManager.kt | Migrates to constructor injection + removes Context parameters |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/CommonSensorCatalogModule.kt | Removes standalone common catalog module (catalog now provided by SensorModule) |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/AppSensorManagerBase.kt | Converts base class to receive injected deps and updates implementations accordingly |
| common/src/main/kotlin/io/homeassistant/companion/android/common/sensors/AndroidOsSensorManager.kt | Migrates to constructor injection + removes Context parameters |
| common/src/main/kotlin/io/homeassistant/companion/android/common/notifications/DeviceCommands.kt | Passes injected BluetoothSensorManager to BLE transmitter command |
| common/src/main/kotlin/io/homeassistant/companion/android/common/bluetooth/ble/MonitoringManager.kt | Updates beacon monitor sensor updates to new API |
| common/src/main/kotlin/io/homeassistant/companion/android/common/bluetooth/ble/IBeaconMonitor.kt | Updates beacon monitor sensor updates to new API |
| common/gradle.lockfile | Locks ClassGraph dependency for common unit tests |
| build-logic/convention/src/main/kotlin/AndroidCommonConventionPlugin.kt | Adds testImplementation(libs.classgraph) to shared Android conventions |
| automotive/src/main/AndroidManifest.xml | Updates component names to new receiver/service forwarders |
| automotive/gradle.lockfile | Locks ClassGraph dependency for automotive unit tests |
| app/src/test/kotlin/io/homeassistant/companion/android/sensors/SensorManagerCompletenessTest.kt | Adds DI completeness test for :app variants |
| app/src/test/kotlin/io/homeassistant/companion/android/sensors/RequestAccurateLocationReceiverTest.kt | Updates expectation to new LocationSensorReceiver component |
| app/src/test/kotlin/io/homeassistant/companion/android/sensors/HealthConnectSensorManagerTest.kt | Updates construction and permission calls to new API |
| app/src/test/kotlin/io/homeassistant/companion/android/frontend/CheckLocationDisabledUseCaseTest.kt | Injects managers set into use case; removes SensorReceiver.MANAGERS dependence |
| app/src/minimal/kotlin/io/homeassistant/companion/android/sensors/LocationSensorManager.kt | Makes minimal flavor manager injectable; moves broadcast handling to new receiver |
| app/src/minimal/kotlin/io/homeassistant/companion/android/sensors/AndroidAutoSensorManager.kt | Makes minimal flavor manager injectable and updates API |
| app/src/minimal/kotlin/io/homeassistant/companion/android/sensors/ActivitySensorManager.kt | Makes minimal flavor manager injectable and updates API |
| app/src/main/kotlin/io/homeassistant/companion/android/util/CheckLocationDisabledUseCase.kt | Injects manager set and updates logic to new SensorManager API |
| app/src/main/kotlin/io/homeassistant/companion/android/settings/sensor/views/SensorDetailView.kt | Updates permission check to new SensorManager.checkPermission signature |
| app/src/main/kotlin/io/homeassistant/companion/android/settings/sensor/SensorSettingsViewModel.kt | Injects manager set and updates list building/filtering to new API |
| app/src/main/kotlin/io/homeassistant/companion/android/settings/sensor/SensorDetailViewModel.kt | Injects manager set and updates enable/permission/update flows to new API |
| app/src/main/kotlin/io/homeassistant/companion/android/sensors/SensorWorker.kt | Exposes LastUpdateManager via entry point to satisfy new base worker dependency |
| app/src/main/kotlin/io/homeassistant/companion/android/sensors/SensorReceiver.kt | Switches from manual manager list to injected Set<SensorManager> |
| app/src/main/kotlin/io/homeassistant/companion/android/sensors/QuestSensorManager.kt | Migrates to constructor injection + removes Context parameters |
| app/src/main/kotlin/io/homeassistant/companion/android/sensors/NotificationSensorManager.kt | Refactors into injectable manager (logic holder) decoupled from Android service |
| app/src/main/kotlin/io/homeassistant/companion/android/sensors/NotificationSensorListenerService.kt | New thin NotificationListenerService forwarding callbacks to injected manager |
| app/src/main/kotlin/io/homeassistant/companion/android/sensors/LocationSensorReceiver.kt | New thin BroadcastReceiver forwarding intents to injectable location manager |
| app/src/main/kotlin/io/homeassistant/companion/android/sensors/LastAppSensorManager.kt | Migrates to constructor injection + removes Context parameters |
| app/src/main/kotlin/io/homeassistant/companion/android/sensors/GeocodeSensorManager.kt | Migrates to constructor injection + removes Context parameters |
| app/src/main/kotlin/io/homeassistant/companion/android/sensors/DynamicColorSensorManager.kt | Migrates to constructor injection + removes Context parameters |
| app/src/main/kotlin/io/homeassistant/companion/android/sensors/DevicePolicyManager.kt | Migrates to constructor injection + updates intent-based update path signature |
| app/src/main/kotlin/io/homeassistant/companion/android/sensors/CarSensorManager.kt | Migrates to constructor injection + removes stored context usage |
| app/src/main/kotlin/io/homeassistant/companion/android/sensors/AppSensorModule.kt | New Hilt module binding app managers into Set<SensorManager> + provides app catalog sensors |
| app/src/main/kotlin/io/homeassistant/companion/android/sensors/AppSensorManager.kt | Updates app App sensor manager to delegate to injected base constructor |
| app/src/main/kotlin/io/homeassistant/companion/android/sensors/AppSensorCatalogModule.kt | Removes standalone app catalog module (catalog now provided by AppSensorModule) |
| app/src/main/kotlin/io/homeassistant/companion/android/sensors/ActivitySensorReceiver.kt | New thin BroadcastReceiver forwarding activity/sleep intents to injectable manager |
| app/src/main/kotlin/io/homeassistant/companion/android/notifications/MessagingManager.kt | Injects BluetoothSensorManager; updates notification/location listener component references |
| app/src/main/AndroidManifest.xml | Updates notification listener service + location/activity receiver component names |
| app/src/full/kotlin/io/homeassistant/companion/android/sensors/AndroidAutoSensorManager.kt | Migrates to constructor injection + removes stored context usage |
| app/src/full/kotlin/io/homeassistant/companion/android/sensors/ActivitySensorManager.kt | Refactors to injectable manager; forwards broadcasts via new receiver component |
| app/src/full/kotlin/io/homeassistant/companion/android/location/HighAccuracyLocationService.kt | Targets LocationSensorReceiver for high-accuracy location pending intents |
| app/src/full/kotlin/io/homeassistant/companion/android/location/HighAccuracyLocationReceiver.kt | Targets LocationSensorReceiver for follow-up broadcasts |
| app/gradle.lockfile | Locks ClassGraph dependency for app unit tests |
2bc7c02 to
73cb4c8
Compare
5c5a9de to
6815184
Compare
73cb4c8 to
aa0b470
Compare
6815184 to
f3dd9fd
Compare
Test Results 303 files 307 suites 12m 29s ⏱️ Results for commit 6280218. ♻️ This comment has been updated with latest results. |
aa0b470 to
b6364d2
Compare
bbdf55b to
15ba358
Compare
a2c4bf8 to
3f2d348
Compare
7032c01 to
3088dc4
Compare
fd02d84 to
6ffd601
Compare
bcef6fb to
6280218
Compare
jpelgrom
left a comment
There was a problem hiding this comment.
I think we were overdue for doing this as injectable. Once again, very iterative cleanup 👌, especially in the tests, but a lot of lines changed as a result.
Splitting receivers from the SensorManagers is also a change that I think we could technically do separately, but let's not make this stack even bigger.
6280218 to
bcabe36
Compare
jpelgrom
left a comment
There was a problem hiding this comment.
Checked code again and tested with #7107 (top of the stack; (in app, enabling sensors from the receiver, broadcasts like location tracking)), and I think we're mostly good. Ready for you to merge all in the correct order and hopefully no complications.
bcabe36 to
3376339
Compare
Summary
This PR make the SensorManager injectable, I had to remove some android interface from some manager to allow construction with parameters like in the
LocaltionSensorManagerthat now have aLocationSensorReceiverthat simply forward whatever intent it receive to the manager.I've built a test for each module that verify using
io.github.classgraph:classgraphhttps://github.com/classgraph/classgraph that every implementation of the SensorManager is properly injected into the DI graph by doing so if someone creates a manager but forget to bind it the CI is going to catch it. We could make a lint rule but it would be more complicated. Indeed it would probably make development easier, so if it becomes an issue let's build a lint rule.The PR allow removes all the
contextparameters in favor of the injected one. It simplifies the APIs.Checklist
Any other notes
I've made two commits to put the test aside.
This PR is based on #7040