Skip to content
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

Use EntityCategory enum instead of strings #504

Merged
merged 2 commits into from
Apr 4, 2022

Conversation

mdegat01
Copy link
Contributor

@mdegat01 mdegat01 commented Apr 1, 2022

Using the entity category strings doesn't work anymore starting in 2022.4. Using the enums instead fixes #503 .

The enums have been accepted since 2021.12 (PR) which covers your supported versions so it is safe to make immediately, don't have to wait for 2022.4.

@ArnyminerZ
Copy link
Collaborator

Please, @mdegat01 check your code style, and configure correctly pre-commit, check the contribution guidelines.

Copy link
Owner

@leikoilja leikoilja left a comment

Choose a reason for hiding this comment

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

That's nice, thank you!
Are we sure it's backwards competitive and won't break the integration for those using latest stable HA image? 🤔 :)

@ArnyminerZ
Copy link
Collaborator

ArnyminerZ commented Apr 1, 2022

Are we sure it's backwards competitive and won't break the integration for those using latest stable HA image? 🤔 :)

Well, I don't think so. EntityCategory is added in home-assistant/core#60720, so all compilations before that one will not contain that class, resulting on an error, as far as I see it. Is there any way to check the version, and choose accordingly?

If not, maybe we should wait until the stable version is released, and then use the new method, which won't be compatible with HA <2022.4.

However, this should be tested.

Edit: in fact, the PR from HA is labeled as "breaking-change", so this is something to take into consideration.

@ArnyminerZ
Copy link
Collaborator

After a bit of searching I've found this on StackOverflow, so maybe something like this can be used:

if version >= "2022.4":
    from homeassistant.const import ENTITY_CATEGORY_CONFIG
else:
    from homeassistant.helpers.entity import EntityCategory.CONFIG as ENTITY_CATEGORY_CONFIG

Which will allow to keep using

_attr_entity_category = ENTITY_CATEGORY_CONFIG

Not sure about this, it has to be tested and further investigation must be performed, just a suggestion that might work out until it's marked as not backward-compatible by HA.

@mdegat01
Copy link
Contributor Author

mdegat01 commented Apr 2, 2022

@ArnyminerZ This repository lists version 2022.2 as it's minimum supported version of home assistant which is after the linked pr.
https://github.com/leikoilja/ha-google-home/blob/master/hacs.json#L4

That's why I said it would be fine to merge immediately. All versions of home assistant this integration supports are after that was merged into core and the enum shipped.

I'll fix the formatting and stuff though sorry about that. Was cranking through a couple of these based on users reporting the same issues in the beta channel of ha

@mdegat01
Copy link
Contributor Author

mdegat01 commented Apr 2, 2022

Also just to get everyone on the same page, 2022.4 is the release when providing an entitycategory as a string instead of an enum goes from being deprecated to being unsupported. It's not the release when HA introduced the entitycategory enum, that was 2021.12 (the pr we both linked was committed into core on 12/1/2021).

So essentially this component no longer works at all starting in 2022.4 without this change. With this change it works fine for all releases of ha >= 2021.12 which covers all releases of ha supported by this component according to its HACS.json.

@mdegat01 mdegat01 force-pushed the entity-category-enum branch from a66b62e to a92f1af Compare April 2, 2022 01:44
@ArnyminerZ
Copy link
Collaborator

Okey @mdegat01, thanks for making it clear. Then we can merge this without any issues. Agree @leikoilja ?

@ArnyminerZ
Copy link
Collaborator

Since no further issues have been reported, merging this one 🚀

@ArnyminerZ ArnyminerZ merged commit 7822b39 into leikoilja:master Apr 4, 2022
@grantclem
Copy link

grantclem commented Apr 4, 2022

This issue does not appear to be resolved, check #507 for update

@mdegat01 mdegat01 deleted the entity-category-enum branch April 4, 2022 19:29
@leikoilja
Copy link
Owner

Awesome, thank you, gents 💪 💥 🔥

@grantclem
Copy link

Thanks very much for your efforts resolving this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error with latest Beta core 2022.4.0b1
4 participants