home-assistant / android

:iphone: Home Assistant Companion for Android
https://companion.home-assistant.io/
Apache License 2.0
2.33k stars 650 forks source link

SET_ALARM command activity race conditions #3282

Closed simeoncran closed 1 year ago

simeoncran commented 1 year ago

Home Assistant Android version: 2022.12.3

Android version: 13

Phone model: Pixel 7

Home Assistant version: 2022.1208.0

Last working Home Assistant release (if known):

Description of problem: I am trying to set multiple alarm clock alarms using a command activity in Node-Red. It works fine if I set just one alarm, but if I set multiple alarms at once some of the alarms do not get reliably set, and sometimes they get set to incorrect time values. Rate-limiting in Node-Red seems to allow it to work.

In the Node-Red call service node I have set "Queue = All Messages" to ensure that no messages are dropped.

A typical command activity request looks like this: {"intent_extras":"android.intent.extra.alarm.VIBRATE:true,android.intent.extra.alarm.SKIP_UI:true,android.intent.extra.alarm.HOUR:21,android.intent.extra.alarm.MINUTES:16,android.intent.extra.alarm.MESSAGE:Time to do a thing!","intent_action":"android.intent.action.SET_ALARM"}

While rate-limiting the messages from Node-Red seems to solve it, I don't think it will always be sufficient because it relies on the delay in Node-Red resulting in a delay in Android, which will not be the case if the request has to be enqueued in Home Assistant while Android is unreachable.

Traceback (if applicable, to get the logs you may refer to: https://companion.home-assistant.io/docs/troubleshooting/faqs/#android-crash-logs):

Screenshot of problem:

Additional information:

dshokouhi commented 1 year ago

Please get us the companion app logs when you reproduce the issue so we can see what is going on.

jpelgrom commented 1 year ago

It might also be helpful to look at the notification history in the app.

As you're sending multiple messages (almost) at the same time, keep in mind that FCM does not guarantee the order of delivery. Not sure if this is relevant for you but since you mentioned throttling just to be sure.

simeoncran commented 1 year ago

@dshokouhi:

I've got a simple repro - I am sending 5 requests in succession to the Node-Red HA service with different times and labels. The result is usually 2 or 3 alarms set. Expected: 5 alarms set.

If I rate limit to 1 request per second I usually get all 5 alarms set.

@jpelgrom - good thought, but these alarms have unique times and labels and in the repro I'm only setting them so the ordering does not matter.

The log from the HA app shows 5 FCM messages but the result was that only 2 alarms were set.

homeassistant_companion_log_0-30-2023_21-7-33.txt

dshokouhi commented 1 year ago

I don't see any errors in the logs so if there is any issue it might be app specific. Which alarm app are you using? Maybe try another one to rule out an app issue.

simeoncran commented 1 year ago

@dshokouhi - I tried "Alarm Clock Plus" and it seems to be reliable (for setting - I couldn't get it working for dismissing). So maybe this is a limitation in the Android AlarmClock.

A thought: could the companion app wait for a result from the ACTION_SET_ALARM intent before sending the next one? Or is there a way that HA could enumerate the alarms so it could know when it had succeeded?

I really want to avoid using a third party alarm clock.

dshokouhi commented 1 year ago

A thought: could the companion app wait for a result from the ACTION_SET_ALARM intent before sending the next one? Or is there a way that HA could enumerate the alarms so it could know when it had succeeded?

Intents do not send back responses they just execute the action. You'll need to use an app that supports what you are trying to do. As this issue is related to a particular app I am closing this issue.

simeoncran commented 1 year ago

Isn't that the difference between startActivity and startActivityForResult?