Skip to content

Add haptic message support for FrontendScreen#6635

Open
TimoPtr wants to merge 6 commits intomainfrom
feature/haptic
Open

Add haptic message support for FrontendScreen#6635
TimoPtr wants to merge 6 commits intomainfrom
feature/haptic

Conversation

@TimoPtr
Copy link
Copy Markdown
Member

@TimoPtr TimoPtr commented Mar 26, 2026

Summary

Add support for haptic message with a fallback when haptic is unknown.

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.

Any other notes

I tested the feedbacks one by one and I'm not able to tell the difference ... It's very subtle

Copilot AI review requested due to automatic review settings March 26, 2026 17:14
@TimoPtr TimoPtr added WebViewActivity replacement Ongoing work to replace the WebViewActivity in favor of a well tested compose screen using nav. and removed cla-signed labels Mar 26, 2026
@TimoPtr TimoPtr linked an issue Mar 26, 2026 that may be closed by this pull request
@TimoPtr TimoPtr requested a review from jpelgrom March 26, 2026 17:14
Copy link
Copy Markdown
Contributor

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

Adds support for the Home Assistant frontend external-bus haptic message in the new Compose-based FrontendScreen, propagating typed haptic events from message parsing through the ViewModel to the WebView so native haptics can be performed (with forward-compatible handling of unknown types).

Changes:

  • Introduce HapticMessage + HapticType (sealed, forward-compatible) and register serializers for external-bus JSON parsing
  • Add a new FrontendHandlerEvent.PerformHaptic and wire it through FrontendMessageHandlerFrontendViewModelFrontendScreen side effects
  • Add unit tests covering message parsing/routing and basic haptic performer mappings

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
app/src/main/kotlin/io/homeassistant/companion/android/frontend/FrontendScreen.kt Collects haptic events and triggers native haptics from the WebView layer
app/src/main/kotlin/io/homeassistant/companion/android/frontend/FrontendViewModel.kt Exposes hapticEvents SharedFlow and emits types based on handler results
app/src/main/kotlin/io/homeassistant/companion/android/frontend/externalbus/FrontendExternalBusRepositoryImpl.kt Registers HapticType serializers module into frontendExternalBusJson
app/src/main/kotlin/io/homeassistant/companion/android/frontend/externalbus/incoming/HapticType.kt Defines typed haptic kinds with forward-compatible deserialization fallback
app/src/main/kotlin/io/homeassistant/companion/android/frontend/externalbus/incoming/IncomingExternalBusMessage.kt Adds the incoming haptic message type to the external-bus message model
app/src/main/kotlin/io/homeassistant/companion/android/frontend/handler/FrontendHandlerEvent.kt Adds PerformHaptic event to the handler event stream
app/src/main/kotlin/io/homeassistant/companion/android/frontend/handler/FrontendMessageHandler.kt Maps HapticMessage to FrontendHandlerEvent.PerformHaptic
app/src/main/kotlin/io/homeassistant/companion/android/frontend/haptic/HapticFeedbackPerformer.kt Implements Android-side haptic execution and fallback vibration behavior
app/src/test/kotlin/io/homeassistant/companion/android/frontend/FrontendViewModelTest.kt Tests that PerformHaptic events are surfaced via FrontendViewModel.hapticEvents
app/src/test/kotlin/io/homeassistant/companion/android/frontend/externalbus/incoming/HapticMessageTest.kt Tests external-bus JSON parsing for HapticMessage payloads and id handling
app/src/test/kotlin/io/homeassistant/companion/android/frontend/externalbus/incoming/HapticTypeTest.kt Tests HapticType deserialization for known/unknown string values
app/src/test/kotlin/io/homeassistant/companion/android/frontend/handler/FrontendMessageHandlerTest.kt Tests handler emits PerformHaptic with correct types from incoming messages
app/src/test/kotlin/io/homeassistant/companion/android/frontend/haptic/HapticFeedbackPerformerTest.kt Tests mapping of haptic types to Android haptic constants on API 30

Comment on lines +18 to +20
@RunWith(RobolectricTestRunner::class)
@Config(application = HiltTestApplication::class, sdk = [Build.VERSION_CODES.R])
class HapticFeedbackPerformerTest {
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

HapticFeedbackPerformer contains fallback vibration behavior for pre-API 30 (Success/Failure/Selection) and pre-API 29 (Warning), but this test class is fixed to sdk = [R] and only exercises the API 30 code paths. Add tests with lower SDK configs to verify the fallback paths call the vibrator (and not performHapticFeedback).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm ok not testing the fallback

Comment on lines +474 to +478
LaunchedEffect(webView) {
hapticEvents.collect { hapticType ->
HapticFeedbackPerformer.perform(webView, hapticType)
}
}
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

LaunchedEffect is keyed only on webView, but it captures/collects hapticEvents. If hapticEvents changes across recompositions (eg a new ViewModel instance while the same WebView is kept), this effect will keep collecting the old Flow and haptics may stop working (and the old collector may leak). Key the effect on both webView and hapticEvents (or use rememberUpdatedState for the Flow) so the collector restarts when the Flow reference changes.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think it is valid to key on the webview but happy to discuss with @jpelgrom

private fun performWarning(view: View) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
view.context.getSystemService<Vibrator>()?.vibrate(
VibrationEffect.createPredefined(VibrationEffect.EFFECT_HEAVY_CLICK),
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This one is different than the actual WebView

"warning" -> {
                if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
                    vm?.vibrate(VibrationEffect.createOneShot(400, VibrationEffect.EFFECT_HEAVY_CLICK))
                } else {
                    @Suppress("DEPRECATION")
                    vm?.vibrate(1500)
                }
            }

I think it was wrong in the Webview since the second param is an amplitude not an effect. (Claude spotted this one)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed WebViewActivity replacement Ongoing work to replace the WebViewActivity in favor of a well tested compose screen using nav.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Modernize WebViewActivity to use compose navigation

2 participants