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 strict data model checks with pydantic #48

Merged
merged 9 commits into from
Aug 26, 2023
Merged

Conversation

kdeyev
Copy link
Owner

@kdeyev kdeyev commented Aug 24, 2023

pyonwater version is updated

@disforw
Copy link
Collaborator

disforw commented Aug 24, 2023

Why are you adding the names back in tot he entity description? Having hardcoded names is no longer allowed on new integrations. The names (if manually added) need to be added via the strings file using the translated_key.

@kdeyev
Copy link
Owner Author

kdeyev commented Aug 24, 2023

Got it, will fix

@kdeyev kdeyev marked this pull request as draft August 24, 2023 18:05
@kdeyev kdeyev requested a review from disforw August 25, 2023 04:20
@kdeyev
Copy link
Owner Author

kdeyev commented Aug 25, 2023

@disforw I've generated data models for pyonwater:https://github.com/kdeyev/pyonwater/blob/v0.2.1/pyonwater/models/eow_models.py
It should make it easier to maintain/extend the code base

@kdeyev kdeyev marked this pull request as ready for review August 26, 2023 00:14
@kdeyev
Copy link
Owner Author

kdeyev commented Aug 26, 2023

Ready for review

@disforw
Copy link
Collaborator

disforw commented Aug 26, 2023

HA will not accept a new integration with description names hardcoded. I'll post documentation. The translation_key will lookup the name (if required) in the strings file. Some entities do not need a name because HA will auto determine the name from the class used.

@disforw
Copy link
Collaborator

disforw commented Aug 26, 2023

Also, I used the "key" with proper case to match the actual flag because core doesn't care about case in the key

@disforw
Copy link
Collaborator

disforw commented Aug 26, 2023

I'd remove all the names, revert all the keys and change line 110 back to use description.key

@kdeyev
Copy link
Owner Author

kdeyev commented Aug 26, 2023

Hey @disforw . Sorry for the misunderstanding.
The name is not used: in the BinarySensorEntityDescription:

        self.entity_description = BinarySensorEntityDescription(
            key=description.key,
            device_class=description.device_class,
            translation_key=description.translation_key,
        )

The name is only used for the unique_id generation here:

self._attr_unique_id = f"{description.name}_{self.meter.meter_uuid}"

The latest version of pyonwater uses snake_case for attributes and all the changes you see here are made to support it.
Please let me know if you still see a problem.

@kdeyev
Copy link
Owner Author

kdeyev commented Aug 26, 2023

I could use the key here:

self._attr_unique_id = f"{description.key}_{self.meter.meter_uuid}"

But then unique_id will be changed, If it's not a concern I can easily do that

@disforw
Copy link
Collaborator

disforw commented Aug 26, 2023

Ah, I see what you did now... Do you think it adds a layer of unnecessary complexity?
I mean, when creating binary_sensor.py, I used the "best practices" that I've seen in other integrations. It's up to you.
When we submit to core, we can see what they say about it then.

@kdeyev kdeyev merged commit 2d42eb0 into master Aug 26, 2023
@kdeyev kdeyev deleted the feature/use-pydantic branch August 31, 2023 14:15
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.

2 participants