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

Issue #338: Remove assert for serial number #354

Closed
wants to merge 1 commit into from

Conversation

Julius2342
Copy link
Owner

No description provided.

@Julius2342 Julius2342 requested a review from palfrey November 26, 2023 15:52
@palfrey
Copy link
Collaborator

palfrey commented Nov 26, 2023

Note the mypy failures in the build. If you're going to do this, Node will have to accept None as a serial number, and then you can do all the others.

@Julius2342
Copy link
Owner Author

Hrmm :)

Do you think we should introduce a SerialNumber object? Or is this sledgehammer -> walnut?

@pawlizio
Copy link
Collaborator

To pass mypy should be engough to change the type hints at all serial_number declarations to Optional[str].
serial_number: Optional[str],

I know this serial_number is like kind of unique_ID intention at HA.

However consider the following statement from here:
An entity is looked up in the registry based on a combination of the platform type (e.g., light), and the integration name (domain) (e.g. hue) and the unique ID of the entity. Entities should not include the domain (e.g., your_integration) and platform type (e.g., light) in their Unique ID as the system already accounts for these identifiers.

In HA there are a lot of features in the UI requiring a unique ID, like set a friendly name. Due to the fact that KLF200 does not provide this information for Somfy devices for example, only empty bytes are sent here, you would not be able to use that features if you link that unique_ID to serial_number.

Therefore for HA, considering the above statement, that unique ID is looked up in the registry based on a combination of domain (velux), for pyvlx we can use node_id as unique_ID and I do so with my custom without any issue so far..

This can be combined with a check if serial_number not None, then...

@Julius2342 Julius2342 closed this Dec 12, 2023
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.

3 participants