Conversation
cebf87d to
18723a2
Compare
At least in my testing, this resulted in a worse UX. The haptics is as feedback to indicate the app registered your touch (it's a small watch screen, nice to get confirmation). Networking on Wear OS can take a while and waiting seconds before getting haptic feedback feels disconnected.
Maybe not used to it but there is a constant (default haptic pattern) for REJECT "A haptic effect to signal the (...) failure of a user interaction.", and iOS has similar patterns for warning/error. |
18723a2 to
a2414a2
Compare
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce Wear OS ANRs by moving Tile action execution off the BroadcastReceiver.onReceive thread and improving coroutine cancellation correctness in entity press handling.
Changes:
- Update
TileActionReceiverto execute the entity press action asynchronously (instead ofrunBlocking) and only trigger haptic feedback after a successful call. - Update
onEntityPressedWithoutStateto rethrowCancellationExceptionand log failures when fetching lock entity state.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| wear/src/main/kotlin/io/homeassistant/companion/android/tiles/TileActionReceiver.kt | Moves tile action handling to an async coroutine-based approach intended to avoid ANRs |
| common/src/main/kotlin/io/homeassistant/companion/android/common/data/integration/Entity.kt | Ensures coroutine cancellation is not swallowed and adds warning logging for lock entity fetch failures |
wear/src/main/kotlin/io/homeassistant/companion/android/tiles/TileActionReceiver.kt
Show resolved
Hide resolved
wear/src/main/kotlin/io/homeassistant/companion/android/tiles/TileActionReceiver.kt
Show resolved
Hide resolved
common/src/main/kotlin/io/homeassistant/companion/android/common/data/integration/Entity.kt
Show resolved
Hide resolved
a2414a2 to
2f8d7ab
Compare
|
@TimoPtr I see multiple pushes but think you haven't addressed (any reply: agree or disagree) my comment on moving the haptic feedback yet. |
Not yet, that was one of my next thing. I didn't focus on this one since it was not a high priority TBH. |
|
I decided to bring it back to what it was so we don't introduce a change here (even if the change of Dispatcher might introduce a very small delay). |
I don't follow - previously haptic happened before the network call but now it waits for the network call to complete? |
Summary
In order to attempt to reduce the number of ANR, I'm proposing to move the TileReceiver action to a dedicated coroutineScope and
lauchAsyncto make sure it complete under 30s.I suppose that the current ANRs are caused by server not accessible for multiple reason
In any case not being able to perform the action under 30s is probably something that we should stop.
I decided to move the haptic after the success so it gives an idea to the user if it succeed or not. But that might be a bad idea since now he doesn't know that the action was took in consideration. I was thinking about adding an error vibration pattern but it is not something users are used too.
I'm open to discussion on this.
Checklist
Any other notes
Based on #6467