search

openhab / org.openhab.binding.zwave

openHAB binding for Z-Wave
Eclipse Public License 2.0
170 stars 202 forks source link

Qubino Smart Plug 16A (ZMNHYD) database fixes #1784

Open robbert0h opened 1 year ago

robbert0h commented 1 year ago

Hi,

I'm creating this issue for reference. I'm looking into correcting/completing the database entry for this device. I am talking to Qubino and to several owners of the device. What I found out until now with their help:

Hard/software versions:

Parameters:

Zwave security:

For now I am going to mark parameter 41 for deletion and add missing parameters 71-74 and 249 to the definition.

I'm also investigating if we should split the definition into 2 or even 3 definitions based parameter/command class differences. I have to check if we can rely on the firmware version for this or if these difference are in the hardware - this is currently not entirely clear to me yet.

Please let me know if there are any questions/concerns.

robbert0h commented 1 year ago

@cdjackson Can you please delete parameter 41 from the database for this device, as mentioned above? Right now it is showing in UI as ' DELETED ' in the latest snapshot build.

cdjackson commented 1 year ago

I've deleted this from the database. Thanks.

robbert0h commented 1 year ago

In my tests with this device I have noticed that it does not always react very well to querying non-existing config parameters. I have seen the initialization process get more or less 'stuck' after querying a non-existing parameter, which causes any subsequent parameters (that do exist) not to be queried successfully. Powercycling the device after querying a non-existing parameter seems to cause the initialization to continue correctly (provided the timing of the powercycle is right).

Since multiple versions of this device exist that have parameters removed/added in between versions, there is a big chance this may affect anyone trying to initialize any version of this device. This is one more reason for me to try to split the database definition into multiple versions.

@cdjackson Have you seen this behaviour before? Do you think it is device specific or may be a broader issue?

cdjackson commented 1 year ago

I'm not sure that it should completely stall, but yes, it's a very bad idea to have incorrect parameters in the database since the binding will try and retrieve then from the device, and it will try to do this quite a bit before ultimately giving up. The database definately needs to be correct for the device in hand.

robbert0h commented 1 year ago

completely stall

To clarify, I haven't seen the initialization completely stall. What I observed is that the querying of valid parameters started to suddenly time out, shortly after querying an invalid parameter (and timing out on that action). From that point it would just keep on timing out on successive parameter queries, resulting in multiple parameters not being read/identified and as such, not being registered in the device xml. However, the initialization process would eventually still finish, after waiting on a lot of timeouts, and with an incomplete xml. Anyway, let me know if you want me to gather a log for this, otherwise I'll just focus on fixing the db.

robbert0h commented 1 year ago

Hello @cdjackson,

I received some very useful overview from Qubino/Goap listing differences for all existing versions of this device:

Screenshot 2022-07-25 at 19 58 19

So I think we are going to need 3 different, versioned, db entries to account for the differences:

What do you think? If you agree - do you have an easy way to duplicate the current definition into these 3 versioned definitions without having to manually reenter all settings in the GUI? Or can you perform this duplication for me? I can correct the various parameter definitions afterwards.

cdjackson commented 1 year ago

What is the difference between H1S1P1 and P2? Do these have different channels or are they exactly the same?

There's no easy way to completely duplicate the database entry - you need the XML for the devices. If they are all exactly the same - ie same command classes and channels, then you could use the same XML and just change the type/id fields so there's no import conflict, and then change them back once the entry is made. There is the ability to copy the associations/parameters and info from one entry to another once the entry is created though, so at least that saves a lot of work, and I can help do that if you like.

robbert0h commented 1 year ago

What is the difference between H1S1P1 and P2? Do these have different channels or are they exactly the same?

I don't know if there are any differences and I only have XMLs of the former. From the information provided by Goap we can see the parameters/security support is the same between these versions, so for now I'm considering them equal. So these can both keep using the current database definition minus the parameter corrections I'm planning.

There's no easy way to completely duplicate the database entry - you need the XML for the devices.

Ok, so I created new definitions for firmware == 3.0 and firmware >= 4.0 from XMLs. Unfortunately, while creating the devices, the website kept returning errors (without any further details) but it turned out in the background it still created the devices. So now there are a couple of bogus entries in the db that are not needed. Please delete these:

The correct definitions are:

Two questions:

  1. With the XML import it also set the Protocol Version to the one found in the XML. AFAIK we don't need the protocol version to distinguish between the device versions. Is it safer (for future compatibility) to remove the specific protocol version from the definitions?
  2. Do you see any issues with the new definitions?

I have not changed any parameters/associations yet. I will do so as a next step after getting your feedback on this part.

robbert0h commented 1 year ago

@cdjackson did you see my message above?

cdjackson commented 1 year ago

Sorry - I'm travelling internationally at the moment.

With the XML import it also set the Protocol Version to the one found in the XML. AFAIK we don't need the protocol version to distinguish between the device versions. Is it safer (for future compatibility) to remove the specific protocol version from the definitions?

I'm not sure if this can be blank, but even if it can be, I would suggest not to do that.

Do you see any issues with the new definitions?

I had a quick look at them a few days ago and they looked fine. They should already be in the latest snapshot as I think it was exported over the weekend

robbert0h commented 1 year ago

Thanks for replying.

I'm not sure if this can be blank, but even if it can be, I would suggest not to do that.

I have seen other entries displaying "0,0" in this field which I assume is equal to blank. For example the original entry of the device didn't have the protocol version specified (and it still doesn't): https://opensmarthouse.org/zwavedatabase/822

Anyway, I did some cleanups of unused parameters and associations. Please be advised that the following definitions will need to have some parameters manually deleted (they are marked as such):

Finally, it occurred to me that the middle definition (version == 3.0) seems to lack a bunch of command classes and channels. I can see them in the XML, so did the import go wrong there?

robbert0h commented 1 year ago

@cdjackson I looked a bit more into the newly created definitions but they both seem to be missing command classes for endpoint 0 that are present in the XMLs from which they were created. So it looks like the XML import went wrong here. Also in my experience the behaviour of this XML import feature is rather flaky, throwing errors without any details, leaving the user to guess what went wrong.

The missing command classes are in these 2 definitions:

https://opensmarthouse.org/zwavedatabase/1506

https://opensmarthouse.org/zwavedatabase/1508

Can you please check why these aren't imported correctly?

Thanks

cdjackson commented 1 year ago

I have seen other entries displaying "0,0" in this field which I assume is equal to blank.

Please don't use 0.0 - please keep this as per the device XML that the binding created.

I'm not sure why some command classes would have been missed - this seems strange as I've not seen this in the past. I probably won't be able to look at this for at least another week - I'm travelling a lot right now, and jetlag is not my friend. I would suggest to try and import it again.

robbert0h commented 1 year ago

Please don't use 0.0 - please keep this as per the device XML that the binding created.

Ok, noted.

I'm not sure why some command classes would have been missed - this seems strange as I've not seen this in the past. I probably won't be able to look at this for at least another week - I'm travelling a lot right now, and jetlag is not my friend. I would suggest to try and import it again.

I tried reimporting the (node 90, version == 3.0) file in various variations (plain/modified deviceId & deviceType/modified applicationVersion), but every single time I press the "create device" button the site greets me with an error: Screenshot 2022-08-06 at 18 29 26

I tried at least 5 imports. Apparently one of my tries does seem to have generated a new db entry, despite showing the error on every try. It's hard to say which one of my tries "succeeded" and even harder why it succeeded (despite still throwing the error). This is the entry that got created: https://opensmarthouse.org/zwavedatabase/1514

I write "succeeded" quoted because the generated db entry again only has 4 of 15 command classes. So that's another issue.

I'm not really sure at this point how I can proceed as the website is a black box to me and I don't know why the import is failing. Continuing in trial and error mode and trying to guess what part of the XML it is failing on (if that's even the problem) is quite tedious and it also produces a lot of junk in the database, so that doesn't seem a very good approach.

How can I get more details about what the site is failing on when trying to create devices from these XMLs?

robbert0h commented 1 year ago

Hi @cdjackson,

I hope you have settled down from travelling. Could you spare a bit of your time to look at the importing issue I described above? I'm kind of stuck now since the website does not import the supplied XML files correctly.

Thanks in advance.

robbert0h commented 1 year ago

Hi @cdjackson, can you please have a look at the above mentioned xml import issue. If you do not have time, I will have to revert part of my earlier changes before next release, otherwise support for newer versions of this device will be broken. Thanks.

cdjackson commented 1 year ago

Sorry - I've been travelling with limited access to the internet over the past 6 or 7 weeks or so (since early August) and have just arrived home late last night. I will try and take a look over the next week or so but as you might imagine, there's a long list of things to look at after such a time away :(

robbert0h commented 1 year ago

Hello @cdjackson,

Again a friendly reminder if you can please have a look at this issue where XML import is going wrong. AFAIK there is no way for me to manually input the missing command classes, so I'm kind of stuck here. If there is a way I can fix this myself, please let me know.

This message in the thread summarizes the issue best.

Thanks

robbert0h commented 1 year ago

@cdjackson Since we're nearing the 3.4 freeze and the import issue of these XMLs is still not solved (and I don't know how to solve it myself), I'm reverting my earlier changes to not impact current users of firmware version 3.0 or newer of this device. They will have to live with the (current) definition that isn't 100% correct (as they do now).

Can you please delete these:

And these earlier failed imports:

Thanks.