Refactor: add PushProvider abstraction for FCM/WebSocket by sk7n4k3d · Pull Request #6633 · home-assistant/android

sk7n4k3d · 2026-03-26T17:08:56Z

Summary

Introduces a generic PushProvider interface that abstracts push notification delivery, replacing the current tight coupling to FCM and WebSocket.

PushProvider interface (common/) — defines register(), refresh(), isActive() for any push backend
PushProviderManager — manages multiple providers, handles registration/refresh lifecycle
FcmPushProvider — wraps existing FCM logic behind the new interface
WebSocketPushProvider — wraps existing WebSocket push behind the new interface
Settings UI — adds a push provider selection preference (currently FCM or WebSocket)
Dagger modules — full flavor binds FCM+WebSocket, minimal binds WebSocket only

This is a pure refactor — no new push backends are added, no behavior changes. Existing FCM and WebSocket push continue to work exactly as before.

Split from #6599 as requested by @jpelgrom and @TimoPtr.

Test plan

Existing FCM push notifications still work
WebSocket fallback still works when FCM is unavailable
Push provider selection in Settings persists across restarts
Unit tests pass (PushProviderTest, PushProviderManagerTest)
No regressions on wear or automotive builds

Introduce PushProvider abstraction for push notification providers (FCM, WebSocket) with PushProviderManager for runtime selection. Add pushUrl and pushEncrypt fields to DeviceRegistration to support alternative push endpoints. Update IntegrationRepositoryImpl to persist and send these new fields during device registration.

Add concrete PushProvider implementations: - FcmPushProvider: wraps Firebase Cloud Messaging (full flavor only) - WebSocketPushProvider: uses persistent WebSocket connection Wire providers via Dagger multibinding in PushProviderModule for both full and minimal flavors.

Add push provider selection preference to settings UI, allowing users to choose between available push providers (FCM, WebSocket). Update SettingsPresenter/Impl to expose provider list and handle provider changes. Add WebsocketManager.restart() for re-enabling WebSocket when switching back to it.

Add tests for PushProvider interface contract and PushProviderManager selection, registration, and server update logic.

TimoPtr · 2026-03-26T17:12:49Z

Could you focus on the first PRs #6621 and #6619 before opening new ones it's going to be hard for us to review if you mix things up.

Copilot

Pull request overview

Introduces a PushProvider abstraction in :common and begins wiring it into the app via Hilt multibindings and a new Settings preference, while extending device registration to carry a push URL and encryption flag.

Changes:

Added PushProvider + PushProviderManager and a PushRegistrationResult model in :common
Extended device registration (DeviceRegistration + registration payload building) to include pushUrl and pushEncrypt
Added FCM/WebSocket provider implementations + flavor-specific Hilt modules and a Settings UI selector (with a WebSocket worker “restart” helper)

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
wear/src/main/kotlin/io/homeassistant/companion/android/phone/PhoneSettingsListener.kt	Updates wear-side device registration call for new `DeviceRegistration` params
wear/src/main/kotlin/io/homeassistant/companion/android/onboarding/integration/MobileAppIntegrationPresenterImpl.kt	Updates wear onboarding device registration call for new `DeviceRegistration` params
common/src/test/kotlin/io/homeassistant/companion/android/common/push/PushProviderTest.kt	Adds unit tests for provider defaults and `PushRegistrationResult`
common/src/test/kotlin/io/homeassistant/companion/android/common/push/PushProviderManagerTest.kt	Adds unit tests for provider manager selection/registration/update behavior
common/src/main/res/values/strings.xml	Adds strings for push provider UI
common/src/main/kotlin/io/homeassistant/companion/android/common/push/PushProviderManager.kt	Implements provider selection + server registration update helper
common/src/main/kotlin/io/homeassistant/companion/android/common/push/PushProvider.kt	Defines the provider interface and registration result model
common/src/main/kotlin/io/homeassistant/companion/android/common/data/integration/impl/IntegrationRepositoryImpl.kt	Persists/uses `pushUrl` and includes `push_encrypt` in registration payload
common/src/main/kotlin/io/homeassistant/companion/android/common/data/integration/DeviceRegistration.kt	Extends device registration model with `pushUrl` + `pushEncrypt`
app/src/minimal/kotlin/io/homeassistant/companion/android/push/PushProviderModule.kt	Minimal flavor binds WebSocket provider into `Set<PushProvider>`
app/src/main/res/xml/preferences.xml	Adds a `ListPreference` for selecting push provider
app/src/main/kotlin/io/homeassistant/companion/android/websocket/WebsocketManager.kt	Adds a `restart()` helper for the periodic WebSocket worker
app/src/main/kotlin/io/homeassistant/companion/android/settings/SettingsPresenterImpl.kt	Exposes provider list + selected value handling to Settings
app/src/main/kotlin/io/homeassistant/companion/android/settings/SettingsPresenter.kt	Adds new presenter APIs for push provider preference
app/src/main/kotlin/io/homeassistant/companion/android/settings/SettingsFragment.kt	Builds provider list entries dynamically and restarts WebSocket worker on selection
app/src/main/kotlin/io/homeassistant/companion/android/push/WebSocketPushProvider.kt	Introduces WebSocket-backed `PushProvider` implementation
app/src/full/kotlin/io/homeassistant/companion/android/push/PushProviderModule.kt	Full flavor binds FCM + WebSocket providers into `Set<PushProvider>`
app/src/full/kotlin/io/homeassistant/companion/android/push/FcmPushProvider.kt	Introduces FCM-backed `PushProvider` implementation

Copilot · 2026-03-26T17:17:42Z

                    appVersionProvider(),
                    deviceName,
                    messagingTokenProvider(),
+                    null,
                    false,


This DeviceRegistration(...) call now relies on positional arguments for pushUrl (String?) and pushWebsocket (Boolean), which makes it easy to misread/accidentally swap. Please switch to named arguments for the newly inserted parameters (and the boolean) so the intent is explicit.

Suggested change

appVersionProvider(),

deviceName,

messagingTokenProvider(),

null,

false,

appVersion = appVersionProvider(),

deviceName = deviceName,

pushToken = messagingTokenProvider(),

pushUrl = null,

pushWebsocket = false,

Copilot · 2026-03-26T17:17:43Z

+            null,
            false,


This DeviceRegistration(...) call passes null and false positionally for the new parameters. To avoid mistakes (especially with multiple primitive/nullable parameters), use named arguments for pushUrl/pushWebsocket (and rely on defaults where appropriate).

Suggested change

null,

false,

Copilot · 2026-03-26T17:17:43Z

+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertNull
+import org.junit.Assert.assertTrue
+import org.junit.Test


These tests use JUnit 4 (org.junit.Test + org.junit.Assert), but the :common module test suite appears to be using JUnit Jupiter (org.junit.jupiter.api.*). If JUnit Vintage isn't enabled, these tests will not run. Please migrate to Jupiter annotations/assertions (and @BeforeEach instead of @Before where needed).

Suggested change

import org.junit.Assert.assertEquals

import org.junit.Assert.assertFalse

import org.junit.Assert.assertNull

import org.junit.Assert.assertTrue

import org.junit.Test

import org.junit.jupiter.api.Assertions.assertEquals

import org.junit.jupiter.api.Assertions.assertFalse

import org.junit.jupiter.api.Assertions.assertNull

import org.junit.jupiter.api.Assertions.assertTrue

import org.junit.jupiter.api.Test

Copilot · 2026-03-26T17:17:43Z

+     * Get all registered providers.
+     */
+    fun getAllProviders(): List<PushProvider> = providers.toList()
+
+    /**
+     * Get a provider by name.
+     */
+    fun getProvider(name: String): PushProvider? = providers.firstOrNull { it.name == name }


getAllProviders() converts a Dagger Set to a List, which makes the ordering non-deterministic. This will surface in the Settings UI as a randomly ordered list between runs/builds. Consider returning providers sorted by a stable key (e.g., name) or exposing an explicit ordering contract.

Copilot · 2026-03-26T17:17:44Z

+        suspend fun restart(context: Context) {
+            val websocketNotifications =
+                PeriodicWorkRequestBuilder<WebsocketManager>(15, TimeUnit.MINUTES)
+                    .build()
+            WorkManager.getInstance(context).enqueueUniquePeriodicWork(
+                UNIQUE_WORK_NAME,
+                ExistingPeriodicWorkPolicy.CANCEL_AND_REENQUEUE,
+                websocketNotifications,
+            )
+        }


restart() duplicates start() logic but doesn't include the legacy cleanup of OLD_UNIQUE_WORK_NAME that start() performs. If restart() is used while the old worker still exists, this can reintroduce duplicate periodic workers. Consider reusing start() (or extracting shared scheduling code) and ensure the old unique work name is also cancelled during restart.

Copilot · 2026-03-26T17:17:45Z

+        servers.forEach { server ->
+            try {
+                serverManager.integrationRepository(server.id).updateRegistration(
+                    deviceRegistration = deviceRegistration,
+                    allowReregistration = false,
+                )
+            } catch (e: Exception) {
+                Timber.e(e, "Failed to update push registration for server ${server.id}")
+            }


updateServerRegistration catches a generic Exception. Because this is a suspend function, this will also catch and swallow CancellationException, preventing proper coroutine cancellation. Please rethrow CancellationException (or catch it explicitly and throw), and only handle non-cancellation failures here.

Copilot · 2026-03-26T17:17:45Z

+ * - Indicating whether it requires a persistent connection (e.g. WebSocket)
+ *
+ * Implementations should be registered via Dagger multibinding so that
+ * [PushProviderManager] can select the best available provider at runtime.


The KDoc says providers are registered via multibinding so PushProviderManager can "select the best available provider at runtime", but PushProviderManager explicitly documents (and implements) user-driven selection with no priority-based selection. Please update this documentation to match the actual behavior to avoid misleading future implementations.

Suggested change

* [PushProviderManager] can select the best available provider at runtime.

* [PushProviderManager] can discover all available providers and expose them

* for user-driven selection and management, rather than performing automatic

* priority-based selection.

Copilot · 2026-03-26T17:17:45Z

        val pushToken = deviceRegistration.pushToken ?: oldDeviceRegistration.pushToken
+        val pushUrl = deviceRegistration.pushUrl ?: oldDeviceRegistration.pushUrl

        val appData = mutableMapOf<String, Any>("push_websocket_channel" to deviceRegistration.pushWebsocket)
        if (!pushToken.isNullOrBlank()) {
-            appData["push_url"] = PUSH_URL
-            appData["push_token"] = pushToken
+            appData["push_url"] = pushUrl?.ifBlank { PUSH_URL } ?: PUSH_URL
+            appData["push_token"] = pushToken.value
+            // Encryption is controlled by the push provider: UnifiedPush sets this to true
+            // when public keys are available, FCM/WebSocket leave it false.
+            appData["push_encrypt"] = deviceRegistration.pushEncrypt
+        } else if (!pushUrl.isNullOrBlank()) {
+            appData["push_url"] = pushUrl
+            appData["push_token"] = ""
+            appData["push_encrypt"] = false


createUpdateRegistrationRequest falls back to the previously persisted pushUrl when the caller doesn't provide one. After introducing a persisted pushUrl, this can accidentally pair a refreshed FCM token (which only updates pushToken) with a stale custom pushUrl from an earlier provider. Consider only using a custom pushUrl when it is explicitly provided (otherwise default to PUSH_URL), or clear the persisted pushUrl when switching back to token-based providers.

Copilot · 2026-03-26T17:17:46Z

+    override fun getAvailablePushProviders(): List<Pair<String, String>> {
+        val result = mutableListOf<Pair<String, String>>()
+        for (provider in pushProviderManager.getAllProviders()) {
+            val label = when (provider.name) {
+                "FCM" -> "Firebase Cloud Messaging"
+                "WebSocket" -> "WebSocket"
+                else -> provider.name
+            }
+            result.add(provider.name to label)
+        }
+        return result


getAvailablePushProviders() hardcodes user-facing labels ("Firebase Cloud Messaging", "WebSocket") instead of using string resources. This also leaves the newly added push_provider_* strings unused (likely triggering unused resource lint). Consider returning resource IDs or building the entries in SettingsFragment using getString(commonR.string.push_provider_fcm) etc.

Copilot · 2026-03-26T17:17:46Z

+class WebSocketPushProvider @Inject constructor(
+    @ApplicationContext private val context: Context,
+    private val serverManager: ServerManager,
+    private val settingsDao: SettingsDao,
+) : PushProvider {
+
+    override val name: String = NAME
+
+    override val requiresPersistentConnection: Boolean = true
+
+    override suspend fun isAvailable(): Boolean = true
+
+    override suspend fun isActive(): Boolean {
+        if (!serverManager.isRegistered()) return false
+        return serverManager.servers().any { server ->
+            val setting = settingsDao.get(server.id)?.websocketSetting
+            setting != null && setting != WebsocketSetting.NEVER
+        }


WebSocketPushProvider injects context but never uses it, and isActive() treats a missing DB row (settingsDao.get(...) == null) as inactive even though other parts of the app use a flavor-dependent default when the setting is absent (see WebsocketManager / SettingViewModel.DEFAULT_WEBSOCKET_SETTING). Please remove the unused dependency and align the defaulting logic so isActive() accurately reflects whether WebSocket push is enabled.

sk7n4k3d · 2026-03-26T17:18:54Z

Duplicate of #6619 — closing. Sorry for the noise, I missed the existing PRs.

sk7n4k3d added 4 commits March 26, 2026 17:56

test: add PushProvider unit tests

a60ff6f

Add tests for PushProvider interface contract and PushProviderManager selection, registration, and server update logic.

Copilot AI review requested due to automatic review settings March 26, 2026 17:08

home-assistant bot added the cla-signed label Mar 26, 2026

sk7n4k3d mentioned this pull request Mar 26, 2026

Add UnifiedPush support as alternative push provider #6634

Closed

7 tasks

Copilot started reviewing on behalf of sk7n4k3d March 26, 2026 17:09 View session

Copilot AI reviewed Mar 26, 2026

View reviewed changes

sk7n4k3d closed this Mar 26, 2026

-                    appVersionProvider(),
-                    deviceName,
-                    messagingTokenProvider(),
-                    null,
-                    false,
+                    appVersion = appVersionProvider(),
+                    deviceName = deviceName,
+                    pushToken = messagingTokenProvider(),
+                    pushUrl = null,
+                    pushWebsocket = false,

- * [PushProviderManager] can select the best available provider at runtime.
+ * [PushProviderManager] can discover all available providers and expose them
+ * for user-driven selection and management, rather than performing automatic
+ * priority-based selection.

Uh oh!

Conversation

sk7n4k3d commented Mar 26, 2026

Summary

Test plan

Uh oh!

TimoPtr commented Mar 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

sk7n4k3d commented Mar 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants