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

Support opening and closing the cover of MyFoxSecurityCamera #224

Merged

Conversation

iMicknl
Copy link
Owner

@iMicknl iMicknl commented Aug 12, 2020

@tetienne you mapped it to a switch, however looking at the current codebase, it makes more sense to map it to cover. What do you think?

@tetienne
Copy link
Collaborator

Only open and close action are available. You cannot set the position. It's why I set it as a switch. It makes more sense IMO.

@iMicknl
Copy link
Owner Author

iMicknl commented Aug 13, 2020

Only open and close action are available. You cannot set the position. It's why I set it as a switch. It makes more sense IMO.

Setting the position is not required for the cover platform. We also support other stateless Somfy devices in the cover platform. Imo it makes more sense to utilise the cover platform where all commands are supported already. (and the camera shutter is some kind of a cover)

We moved the stateless Garage Door also from the switch to the cover component for the reason above.

@tetienne
Copy link
Collaborator

If I follow your logic the switch platform can be deleted as the cover platform can do the same.
Can you please add in comment an exemple of camera shutter setup?

@iMicknl
Copy link
Owner Author

iMicknl commented Aug 13, 2020

If I follow your logic the switch platform can be deleted as the cover platform can do the same.
Can you please add in comment an exemple of camera shutter setup?

Not really. My logic would be to map a cover (or device with cover like features and the same commands) to the cover platform, where the switch platform is used as an on/off switch. We map devices with only on / off commands to switch for example, and we modified it to also support a basic siren. (until there is a native siren platform).

For me, it doesn't make sense to implement open, close and closed/opened state in Switch as well. Especially since it is covered by the cover platform already, and the use case is a camera cover. #221 (comment) (set up can be found here)

We could add a new device class for a CAMERA_SHUTTER / CAMERA_COVER, however I am not sure if that is necessary. For now I mapped it to DEVICE_CLASS_SHUTTER, but maybe that is not a nice way.

@tetienne
Copy link
Collaborator

OK looking at the setup it make sense. We already support these commands indeed.
Sorry for my comments.

@iMicknl
Copy link
Owner Author

iMicknl commented Aug 13, 2020

OK looking at the setup it make sense. We already support these commands indeed.
Sorry for my comments.

No worries, it is good to be challenged and to discuss architectural choices. 👍🏻. Thanks for your review!

@iMicknl iMicknl merged commit caf1460 into master Aug 13, 2020
@iMicknl iMicknl deleted the enhancement/support_MyFoxSecurityCamera_camera_cover branch August 13, 2020 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request platform: cover
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants