-
-
Notifications
You must be signed in to change notification settings - Fork 677
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow custom intents register for categories in case they are required #2901
Allow custom intents register for categories in case they are required #2901
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is a comma a safe separator to use that won't conflict with values? I know most apps use dots and underscores in actions but we wouldn't want to create an accidental breaking change.
It's hard to tell what's safe or not here honestly. In terms of what's documented on Android all examples include dots and underscores so it felt safe. It's also what's used for intent extras in our notification command. |
Testing this and it's not working like expected. I have registered "ha_test,android.intent.category.DEFAULT" in the app, yet intents with only the action ha_test and no category set are still received. Am I misunderstanding something here? |
thats to be expected actually....you can register for more categories than the intent calls for and it won't make a difference....its only when the intent contains a category does it need to match |
Right I didn't understand the changes correctly. Did a lot more testing and reading and I think it's actually a lot less complicated than we think. The Android 13 change only applies to explicit intents; intents sent directly to the app (package = io.homeassistant.companion.android(.debug), class = io.homeassistant.companion.android.sensors.SensorReceiver). On older Android versions/when not targeting Android 13 any intents sent directly to the app will be received even if it doesn't match the intent filter. The upcoming Android change is for these intents, now there will have to be a filter for them. However there is no need to send intents directly to the HA app, the last update sensor allows registering any intent. Most intents users register probably aren't sent directly to the HA app so they aren't impacted by the change, they are implicit intents for which these requirements already existed. This PR is more like a new feature 🙂. Implicit intents can only be received when registered correctly. Previously the intent filter would only accept an action, and if the sender included a category it wouldn't be handled by the app (tested on Android 11). Now you can specify the category and receive these intents as well. tl;dr This PR works and is more of a new feature. Only apps specifically sending an intent to HA will be impacted by the Android 13 change. Two more notes about the description:
|
Thank you for doing that extra research into what actually is going on. That makes complete sense as to why we didn't hear any complaints after the update. I'll update the description to make to clean it up as well :) |
Don't forget to update the docs PR as well :) |
ok updated :) thank you for calling that out, I would've forgotten 😂 |
Summary
I noticed in Android 13 that there is a behavior change in intents which in some cases will cause the app not to receive an intent.
This PR extends the intent setting value to contain categories that an intent could have. As an intent can have multiple categories I have added in support for this as well.
The new format will be
intent,category1,category2
to allow for the app to continue working as is and to let users modify the setting to include categories if required. Once the app targets SDK 33 I suspect we may be more susceptible to this issue but as of now we can reproduce this behavior using Tasker.Screenshots
Link to pull request in Documentation repository
Documentation: home-assistant/companion.home-assistant#821
Any other notes