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

Basic implementation of Zwave Rollershutters #2313

Merged
merged 7 commits into from
Jun 20, 2016

Conversation

turbokongen
Copy link
Contributor

@turbokongen turbokongen commented Jun 16, 2016

Description:
Basic Implementation for Zwave Rollershutter devices.

This also blocks tested rollershutter device from switch and light.
This is only tested with Fibaro FGRM-222 rollershutter device, and should be considered a basic implementation. For the future, other devices may need to be added for blocking.
Thank you @nunofgs for providing the information to make this possible, testing and debug. 👍
Thank you @wokar for providing code for better filtering. This will make future filtration easier. 👍
Related issue (if applicable): fixes #
#2053

Checklist:

If code communicates with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass

@@ -92,6 +93,10 @@
[COMMAND_CLASS_DOOR_LOCK],
TYPE_BOOL,
GENRE_USER),
('rollershutter',
[COMMAND_CLASS_SWITCH_MULTILEVEL],
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this mean that all multilevel switches will be forwarded to the rollershutter component ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it get filtered by the next two arguments. The index type and genre.

Copy link
Contributor Author

@turbokongen turbokongen Jun 16, 2016

Choose a reason for hiding this comment

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

Type_whatever and genre_whatever would have forwarded all multilevel switches.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ofcourse untested 🙏 , neither of us have that combination.
Anyone that do have a dimmer and a rollershutter device that can test?

Copy link
Contributor Author

@turbokongen turbokongen Jun 16, 2016

Choose a reason for hiding this comment

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

Are there any other ways to channel this correctly. I see there will be problems with identifying devices correctly to the correct component. I understood the code as this:
If command class is switch multilevel AND the command class contains a type with button AND the type has genre with user, it will be detected as rollershutter.
Or have I misunderstood this? Open for suggestions :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I got time to add your changes. Thank you @wokar 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wokar Is there a list somewhere that we can use to find what specific command class is mapped to which number?

Copy link
Contributor

Choose a reason for hiding this comment

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

@turbokongen For the zwave constants I use the openhab project as a reference - the ZWaveCommandClass.java contains the mapping you want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that list I am aware of, but the specific class is what I am interessted in. In this case MOTOR_CONTROL_CLASS_B is 6. Is there a list that those mappings are available?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes - here the openhab class - or the openzwave mapping which better shows the relation of generic, specific and required command classes.

@turbokongen
Copy link
Contributor Author

Soooo.... we need to fix test coverage for other things too now to pass? 😄

@turbokongen turbokongen reopened this Jun 18, 2016
@turbokongen turbokongen force-pushed the zwave_rollershutter branch from 3c37f6a to ca05cc9 Compare June 19, 2016 17:55
@balloob
Copy link
Member

balloob commented Jun 20, 2016

Looks good! 🐬

@balloob balloob merged commit 2e62053 into home-assistant:dev Jun 20, 2016
@turbokongen turbokongen deleted the zwave_rollershutter branch June 20, 2016 18:06
@wokar
Copy link
Contributor

wokar commented Jun 21, 2016

@turbokongen my raspberry crashed on the weekend so I could not test your solution with my Fibaro rollershutter. Now I got the raspberry up and running again and my zwave devices are working fine, but the rollershutter seems not to work at all with the current code :(
Before I dive into the problem - have you successfully tested your solution with a real device? If yes - then this must be a problem with my raspberry...

@turbokongen
Copy link
Contributor Author

I don,t have the device myself, but I had @nunofgs who does have it, help me with the testing.

@turbokongen
Copy link
Contributor Author

I have discovered a flaw in the discovery code, can you try with my latest:
https://github.com/turbokongen/home-assistant/tree/zwave_garagedoor

@wokar
Copy link
Contributor

wokar commented Jun 22, 2016

Last night I figured out that the if-statement:

            if specific_device_class is not None and \
               specific_device_class != node.specific:
                continue

is the problem. Since the specific_device_class is now a list I changed it to:

            if specific_device_class is not None and \
               node.specific not in specific_device_class:
                continue

After that change the rollershutter showed up, but I could not control the device from the UI. I will continue testing and open a PR with my changes.

@turbokongen
Copy link
Contributor Author

Just download the changes I made now :)

@turbokongen
Copy link
Contributor Author

MAde a mistake in the code try latest.

@turbokongen
Copy link
Contributor Author

Ugh... Not working.

@turbokongen
Copy link
Contributor Author

@wokar
Copy link
Contributor

wokar commented Jun 22, 2016

Did a bit of cleanup of the rollershutter code, take a look at https://github.com/wokar/home-assistant/tree/zwave_rollershutter Rollershutter works fine now, execpt for the stop button. It did work the first time, but now ot is ignored or it changes direction.

I also tested your component discovery and it I had to add GENERIC_COMMAND_CLASS_BINARY_SENSOR to the 'sensor' platform to get my fibaro motion sensor working. I also changed the *_WHATEVER entries to be not in a list, otherwise the specific_device_class is not None code will not work.

@turbokongen
Copy link
Contributor Author

Was too tired yesterday to spot my mistakes. Please test latest :)

@turbokongen
Copy link
Contributor Author

Please try the latest at:
https://github.com/turbokongen/home-assistant/tree/zwave_garagedoor

Hopefully all will be good now :) My zwave network atleast looks like it did before.
There have been done some additional changes to filter components,

@wokar
Copy link
Contributor

wokar commented Jun 23, 2016

Perfect! All my devices show up and work as expected.

@turbokongen
Copy link
Contributor Author

:+1

@nunofgs
Copy link
Contributor

nunofgs commented Jun 25, 2016

@turbokongen Hi again! I just tested the latest changes in your zwave_garagedoor branch, and almost everything works as expected, except the move_up and move_down commands, which are switched!

I changed the following and everything is working perfectly:

diff --git i/homeassistant/components/rollershutter/zwave.py w/homeassistant/com
index 50ef2ed..60fa1fe 100644
--- i/homeassistant/components/rollershutter/zwave.py
+++ w/homeassistant/components/rollershutter/zwave.py
@@ -61,11 +61,11 @@ class ZwaveRollershutter(zwave.ZWaveDeviceEntity, Rollershut

     def move_up(self, **kwargs):
         """Move the roller shutter up."""
-        self._node.set_dimmer(self._value.value_id, 0)
+        self._node.set_dimmer(self._value.value_id, 100)

     def move_down(self, **kwargs):
         """Move the roller shutter down."""
-        self._node.set_dimmer(self._value.value_id, 100)
+        self._node.set_dimmer(self._value.value_id, 0)

     def stop(self, **kwargs):

@turbokongen
Copy link
Contributor Author

I fixed this issue last night in a different branch. It is already merged,
so test with Ha's dev branch. All should be good.
25. jun. 2016 14:36 skrev "Nuno Sousa" [email protected]:

@turbokongen https://github.com/turbokongen Hi again! I just tested the
latest changes in your zwave_garagedoor branch, and almost everything
works as expected, except the move_up and move_down commands, which are
switched!

I changed the following and everything is working perfectly:

diff --git i/homeassistant/components/rollershutter/zwave.py w/homeassistant/com
index 50ef2ed..60fa1fe 100644--- i/homeassistant/components/rollershutter/zwave.py+++ w/homeassistant/components/rollershutter/zwave.py@@ -61,11 +61,11 @@ class ZwaveRollershutter(zwave.ZWaveDeviceEntity, Rollershut

 def move_up(self, **kwargs):
     """Move the roller shutter up."""-        self._node.set_dimmer(self._value.value_id, 0)+        self._node.set_dimmer(self._value.value_id, 100)

 def move_down(self, **kwargs):
     """Move the roller shutter down."""-        self._node.set_dimmer(self._value.value_id, 100)+        self._node.set_dimmer(self._value.value_id, 0)

 def stop(self, **kwargs):


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2313 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AP2koLIme1CdbFdxTM1M4uY6TQdM7Cmqks5qPSCIgaJpZM4I3WFM
.

@turbokongen
Copy link
Contributor Author

Oooooo!!! This is rollershutter. Wil issue a fix right away!

@jbaudoux
Copy link

Replaced by cover component.
See #4037

@home-assistant home-assistant locked and limited conversation to collaborators Mar 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants