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

Implement Forced Option #97

Merged
merged 3 commits into from
Nov 8, 2024
Merged

Implement Forced Option #97

merged 3 commits into from
Nov 8, 2024

Conversation

derjoerg
Copy link
Collaborator

@derjoerg derjoerg commented Nov 6, 2024

closes #95

Copy link
Owner

@kingsleyadam kingsleyadam left a comment

Choose a reason for hiding this comment

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

I think for this one it probably makes sense to create a new enum with the forced options. Especially if HA will implement this as a select, HA will need to know what all the possible options are as it's not a simple boolean.

Instead of the property forced returning an int which doesn't provide much value, it'd return the ENUM.

I've done something similar here: https://github.com/kingsleyadam/local-abbfreeathome/blob/main/src/abbfreeathome/devices/window_door_sensor.py#L11

@kingsleyadam kingsleyadam added enhancement New feature or request Semver-Minor New Features labels Nov 6, 2024
@derjoerg
Copy link
Collaborator Author

derjoerg commented Nov 8, 2024

I added decicated enums for SwitchActuator and DimmingActuator even that they have currently the same content.
My thought about this was, that I don't know if the value might change in the future and also if possible translations might be different.

@derjoerg derjoerg requested a review from kingsleyadam November 8, 2024 12:25
@kingsleyadam
Copy link
Owner

I added decicated enums for SwitchActuator and DimmingActuator even that they have currently the same content. My thought about this was, that I don't know if the value might change in the future and also if possible translations might be different.

Looks good, I think this will be easier to implement in HA, and in the future if there are more options the changes to HA will be minimal.

@kingsleyadam kingsleyadam merged commit 95075b3 into kingsleyadam:main Nov 8, 2024
2 checks passed
@derjoerg derjoerg deleted the Force branch November 8, 2024 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Semver-Minor New Features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend DimmingActuator and SwitchActuator with "forced" possibility
2 participants