Skip to content
This repository has been archived by the owner on May 17, 2021. It is now read-only.

Testers wanted - Better support for GreenWave devices #3875

Merged
merged 2 commits into from Jan 28, 2016
Merged

Testers wanted - Better support for GreenWave devices #3875

merged 2 commits into from Jan 28, 2016

Conversation

vrampal
Copy link
Contributor

@vrampal vrampal commented Jan 22, 2016

I have only a NP310, I need your help to test on NP210 and NS210

Fixes/improvement:

  • the color wheel display is now correct on my NP310
  • I receive new meter value without polling

Bugs:

  • The meter received by notifications are not attached to the right socket

Not tested:

  • State after power loss
  • over-current related stuff

@cdjackson cdjackson self-assigned this Jan 22, 2016
@vrampal
Copy link
Contributor Author

vrampal commented Jan 25, 2016

I received the following documentation from GreenWave:
Technical doc for PowerNode

@cdjackson
Copy link
Contributor

Thanks - I had a quick look at this yesterday and I think it's ok - I'll double check tonight and hopefully merge then...

Just for information, I'm trying to move the database to an online database editor to make it easier for people to use, and also to make it easier to support both OH1 and OH2 bindings that have different formats...

http://www.cd-jackson.com/index.php/zwave/zwave-device-database

Thanks
Chris

@vrampal
Copy link
Contributor Author

vrampal commented Jan 25, 2016

Before merging, please someone confirm it's not harmful on NP210 & NS210.
I can test only on NP310 & NS310.
I confirm the restore state on powerloss works fine.
I have a strange issue with my Fibaro smoke detector, it shut down the 1st socket for no reason.

@ghost
Copy link

ghost commented Jan 26, 2016

Hi Vincent

I have a NS210 5-plug Powernode and could do some testing. But not sure how. Just grap the latest 1.9 z-wave binding?

Regards
Joerg

@cdjackson
Copy link
Contributor

This isn't merged yet so it's not in the latest snapshot. Let me take another look at it tonight - so long as the changes look ok such that any problems won't be significant I'll merge so you can test.

Sent from my iPhone

On 26 Jan 2016, at 08:38, luotaus notifications@github.com wrote:

Hi Vincent

I have a NS210 5-plug Powernode and could do some testing. But not sure how. Just grap the latest 1.9 z-wave binding?

Regards
Joerg


Reply to this email directly or view it on GitHub.

@vrampal
Copy link
Contributor Author

vrampal commented Jan 26, 2016

You can get a build of the z-wave binding here:
org.openhab.binding.zwave-1.9.0-SNAPSHOT.jar

@ghost
Copy link

ghost commented Jan 26, 2016

Thanks. Will have a look in the evening.

On 26 January 2016 09:53:17 GMT+00:00, Vincent RAMPAL notifications@github.com wrote:

You can get a build of the z-wave binding here:
org.openhab.binding.zwave-1.9.0-SNAPSHOT.jar


Reply to this email directly or view it on GitHub:
#3875 (comment)

@ghost
Copy link

ghost commented Jan 26, 2016

ok, looks good. For my NP210 I am able to see the new parameters and association groups. Everything seems to work as before.

Regarding the new features:

  • Parameter for color wheel is not populated/updated. At least in Habmin, haven't created any items as I don't know the correct command. But when changing the wheel I see following message in the log: NODE 22: No item bound for event, endpoint = 0, command class = CONFIGURATION, value = org.openhab.binding.zwave.internal.protocol.ConfigurationParameter@1b2da2e2, ignoring.
  • Still need to poll status of switch and consumption
  • Happy to confirm that state restore after power loss is working.

@ghost
Copy link

ghost commented Jan 27, 2016

To follow up my mail from yesterday. From my side I think it is ok to merge this change. The 1.8 functionality is still available and at least one new feature (power restore state) is working.

Regarding polling: I am not clear if the new settings should remove the need for polling. Currently my item definitions looks like:

  • zwave="22:1:command=meter,meter_scale=E_W,refresh_interval=55"
  • zwave="22:2:command=switch_binary,fresh_interval=60"
    Coudl this now work also without polling (which in my understadning is enabled with the *refresh_interval parameter).

And perhaps could you post your item definition for the color wheel? Not sure what value this wheel adds, but I like to understand in more details how the parameters work in z-wave.

@vrampal
Copy link
Contributor Author

vrampal commented Jan 27, 2016

From the technical documentation, you should see the flowing:

  • param 0: Power change required to send a notification, in % from 1 to 100, default 10
  • param 1: Keep alive time, in minutes from 1 to 255, default 2
  • param 2: Color wheel selection, read only
  • param 3: State after power loss, 0 = all off, 1 = remember last state, 2 = all on, default 2
  • param 4: Led for network error, 0 = disable, 1 = enable, default 0
  • assoc group 1: Color wheel change
  • assoc group 2: Relay health
  • assoc group 3: Power value change
  • assoc group 4: Over-current protection

You should be able to remove polling on METER WATT by setting param 0 and assoc group 3.
From my test with a NP310 I had some errors with the endpoints and I get the power of a line on another.

The color wheel is only an information to identify the device, useful when you have multiple devices.
If needed, you can display it using this binding:
{ zwave="22:1:command=CONFIGURATION,parameter=2" }

@vrampal
Copy link
Contributor Author

vrampal commented Jan 27, 2016

Please don't merge the pull request the value of parameter 4 is inverted in the config files.
I will fix this next weekend.

@cdjackson
Copy link
Contributor

Ok, thanks

Sent from my iPhone

On 27 Jan 2016, at 15:44, Vincent RAMPAL notifications@github.com wrote:

Please don't merge the pull request the value of parameter 4 is inverted in the config files.
I will fix this next weekend.


Reply to this email directly or view it on GitHub.

@vrampal
Copy link
Contributor Author

vrampal commented Jan 27, 2016

Pull request updated, all config correspond to Greenwave technical documentation.
Binary for testers:
org.openhab.binding.zwave-1.9.0-SNAPSHOT.jar

@ghost
Copy link

ghost commented Jan 27, 2016

Thanks, your explenation were of great help. I was able to disable polling and also experienced the errors with wrong endpoints, i.e. only endpoint 4 is switched on and got following message:

NODE 22: Got a value event from Z-Wave network, endpoint = 1, command class = METER, value = 131.8

Also the switch state seems not to get updated in the non-polling setup.

Will now test the new binding version from today.

@cdjackson
Copy link
Contributor

Please let me know when this is good to merge…

Thanks.

@vrampal
Copy link
Contributor Author

vrampal commented Jan 27, 2016

For me it's ok, the xml file match the documentation, we can benefit of some new parameters and the notification works on single channel version.
The notifications seams broken for multi-channel but when I look inside OpenHab Z-wave code I see a lot of "not yet implemented" comments for the command classes used by this device. :(

@cdjackson
Copy link
Contributor

What command classes does it implement that you need?

Sent from my iPhone

On 27 Jan 2016, at 20:17, Vincent RAMPAL notifications@github.com wrote:

For me it's ok, the xml file match the documentation, we can benefit of some new parameters and the notification works on single channel version.
The notifications seams broken for multi-channel but when I look inside OpenHab Z-wave code I see a lot of "not yet implemented" comments for the command classes used by this device. :(


Reply to this email directly or view it on GitHub.

@cdjackson
Copy link
Contributor

The notifications seams broken for multi-channel

What do you mean by notifications? There aren't notifications from the multi-channel command class - it's just an encapsulation class. If there's a problem, then please open an issue, but I'm not aware of anything wrong in this area.

but when I look inside OpenHab Z-wave code I see a lot of "not yet implemented" comments for the command classes used by this device. :(

Based on this in the XML you produced -:

  •   <Class><id>0x20</id></Class><!-- COMMAND_CLASS_BASIC -->
    
  •   <Class><id>0x25</id></Class><!-- COMMAND_CLASS_SWITCH_BINARY -->
    
  •   <Class><id>0x27</id></Class><!-- COMMAND_CLASS_SWITCH_ALL -->
    
  •   <Class><id>0x32</id></Class><!-- COMMAND_CLASS_METER_V2 -->
    
  •   <Class><id>0x56</id></Class><!-- COMMAND_CLASS_CRC_16_ENCAP -->
    
  •   <Class><id>0x60</id></Class><!-- COMMAND_CLASS_MULTI_CHANNEL_V3 -->
    
  •   <Class><id>0x70</id></Class><!-- COMMAND_CLASS_CONFIGURATION -->
    
  •   <Class><id>0x71</id></Class><!-- COMMAND_CLASS_ALARM -->
    
  •   <Class><id>0x72</id></Class><!-- COMMAND_CLASS_MANUFACTURER_SPECIFIC_V2 -->
    
  •   <Class><id>0x75</id></Class><!-- COMMAND_CLASS_PROTECTION_V2 -->
    
  •   <Class><id>0x85</id></Class><!-- COMMAND_CLASS_ASSOCIATION -->
    
  •   <Class><id>0x86</id></Class><!-- COMMAND_CLASS_VERSION -->
    
  •   <Class><id>0x87</id></Class><!-- COMMAND_CLASS_INDICATOR -->
    

I only see the protection class that isn't supported - I don't think that's a major issue (is it?) - or do you really need this?

@vrampal
Copy link
Contributor Author

vrampal commented Jan 28, 2016

Ok, I started home automation 1 month ago, it's the very first time I use Z-Wave so I'm not very confident in my analysis.

GreenWave supports association to send the current consumption. I see no reason at all to enable information push on channel 1 only so they must have some kind of trick to encapsulate association in multichannel, or use another parameter. Also, you don't need to configure any polling in Fibaro boxes, so I guess there is a way to use the feature.
I didn't take time to run openhab in debug and to record some z-wave packet. I will do when available but I'm not sure my z-wave knowledge will be enough to enable the full potential of greenwave devices.

The COMMAND_CLASS_METER, only the REPORT command is implemented, when I look inside OpenHab 1.8.0 code, the GET command and the RESET command was logging an warning.
I didn't look inside COMMAND_CLASS_MULTI_CHANNEL yet, nor COMMAND_CLASS_ASSOCIATION.

@vrampal
Copy link
Contributor Author

vrampal commented Jan 28, 2016

At the end, it's all I can do by changing only the XML files. So the pull request is complete for now.
I am perfectly ok to upload data to your DB for all my 3 z-wave devices.

@cdjackson
Copy link
Contributor

GreenWave supports association to send the current consumption. I see no reason at all to enable information push on channel 1 only so they must have some kind of trick to encapsulate association in multichannel, or use another parameter.

Typically, this needs to be configured in the device - associations are configured in the device, and the device then sends the data direct. IF there's a bug here in the binding, or some feature missing, then we can look at it, but I don't think so?

Also, you don't need to configure any polling in Fibaro boxes, so I guess there is a way to use the feature.

Absolutely - if you use associations, then polling is disabled. This is definately the best way to work and I would suggest configuring the device to do this. This is the normal way to work in openhab.

The COMMAND_CLASS_METER, only the REPORT command is implemented, when I look inside OpenHab 1.8.0 code, the GET command and the RESET command was logging an warning.

Ok - here you misunderstand the way the system works. What this means is that the BINDING doesn't support receiving GET requests since this makes no sense. This means that if the device sends a GET to the binding, then it doesn't respond - but this will never happen.
It is however supported that the binding can SEND a GET request - it just doesn't respond if it RECEIVES a GET (I hope that makes sense).
Both GET and RESET are supported. GET is sent when polling is enabled. The reset is a separate command but it's documented in the wiki if I remember correctly.

At the end, it's all I can do by changing only the XML files. So the pull request is complete for now.
I am perfectly ok to upload data to your DB for all my 3 z-wave devices.

Thanks. I hope the above helps - if you have any questions, feel free to ask (best on the forum since then everyone can benefit from the discussion).

cdjackson added a commit that referenced this pull request Jan 28, 2016
Testers wanted - Better support for GreenWave devices
@cdjackson cdjackson merged commit 6fb7cb4 into openhab:master Jan 28, 2016
@ghost
Copy link

ghost commented Jan 28, 2016

@vrampal FYI, I already added three Greenwave products to the database. The 6 plug version is fully populated in regards parameters. But the latest version is still awaiting approval. Once this is done, perhabs you can have a look at it and check. Suggest to follow up in your forum thread.

@cdjackson Given the learnings from this discussion I think there might be somewhere a bug with associations and multiple endpoints. But I wonder if it makes sense to wait for the 2.0 binding and test it there. For me it is not urgent as polling is an acceptable workaround for the time being.

@cdjackson
Copy link
Contributor

@cdjackson Given the learnings from this discussion I think there might be somewhere a bug with associations and multiple endpoints.

I'm more than happy if you want to raise an issue, but I don't have ANY idea what the problem is? The only comment I see is something about the fact that the device supports associations to send current consumption, but this isn't even an explanation of what issue you are seeing. I gather that this means something isn't working, but what? Can you provide a log, maybe a description of what you're doing?

Currently, I don't believe there is anything missing or any bug in the association command class -- there is a missing command class which is the multi-instance-association, but this device doesn't seem to support this (at least based on the list of command classes).

I suggested above that there is a discussion on the forum on this so we can understand the issue - then if there's a problem identified, we can raise an issue to get it fixed. I'm happy to try and help, but only if I understand the issue :)

@cdjackson cdjackson added this to the 1.9.0 milestone Jan 29, 2016
@vrampal
Copy link
Contributor Author

vrampal commented Jan 29, 2016

According to Z-Wave command class specification, there is a command class called MULTI_INSTANCE_ASSOCIATION. Apparently, it's implemented in OpenZWave.
I would be very curious to test this command class against Greenwave devices. ;)

@cdjackson
Copy link
Contributor

Yes - that's what I mentioned in my email above, but according to the information you provided in the XML, this class isn't supported by the device? Can you confirm this please?

@vrampal
Copy link
Contributor Author

vrampal commented Jan 29, 2016

Your right, I updated the XML according to the documentation and MULTI_INSTANCE_ASSOCIATION is not mentioned in documentation.

@vrampal vrampal deleted the better_greenwave branch February 5, 2016 23:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants