[Solax] Changes in values parser for X3-Hybrid-G4

[theater]

Some indexes were changed in the parsing based on user feedback.

It has been confirmed with the users that the changes do not affect the QVolt implementation and we can proceed with the review process.

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/solax-binding/146326/203

[lsiepel]

These comments are probably the main reason to create this PR, so consider it as a double check. As you mention this might be breaking to users on current version, do you have some kind of information available (by API or otherwise) to determine the right indices like a version or schema string?

[theater]

These comments are probably the main reason to create this PR, so consider it as a double check. As you mention this might be breaking to users on current version, do you have some kind of information available (by API or otherwise) to determine the right indices like a version or schema string?

Unfortunately everything is taken from other reverse-engineered implementations (python mostly). The problem is that the QVolt and X3-Hybrid-G4 return the same inverter type - 14. This leads me to believe that the API should be the same but changing indexes is indeed worrying. I will confirm that with users first (that's why the WIP flag).
If we have to support both and they provide different indexes (i.e. the expected values are in different places), probably I need to implement a parser for the QVolt which is bound to the type but not really taking the API's type into consideration.
Have you seen anything like that so far in any other bindings? Probably the automatic discovery will not work correctly for that...

[lsiepel]

These comments are probably the main reason to create this PR, so consider it as a double check. As you mention this might be breaking to users on current version, do you have some kind of information available (by API or otherwise) to determine the right indices like a version or schema string?

Unfortunately everything is taken from other reverse-engineered implementations (python mostly). The problem is that the QVolt and X3-Hybrid-G4 return the same inverter type - 14. This leads me to believe that the API should be the same but changing indexes is indeed worrying. I will confirm that with users first (that's why the WIP flag). If we have to support both and they provide different indexes (i.e. the expected values are in different places), probably I need to implement a parser for the QVolt which is bound to the type but not really taking the API's type into consideration. Have you seen anything like that so far in any other bindings? Probably the automatic discovery will not work correctly for that...

In general there are two scenario's (may be more):

1. Versioning; The used schema (set if indices or whatever) is bases on some kind of property that can be determined up front. Either by dscovery or (additional) remote API call.
2. Fallback; Use the default (most used) schema and if that fails (by error, missing property or unexpected result) it can fall back to the old behaviour.
   Obvious scenario 1 is prefered and i'm not that familair with this binding to give any advice on linking to a type and/or a specific parser. If you want to use a specific parser for a specific thing type, the thinghandler would be a place to control this. Not sure if there is a prefered way of implementing this. Just create something and we will see from there ;-)

[theater]

OK. It has been discussed with the community. It seems that QVolt and X3 Hybrid are the same inverter after all. The values in the initial parser were never correct and now they seem to be correct for both these inverters, so I would say that this change also contains a fixes for a non-working parse of some channels. We don't have to make any backwards compatibility magic...

[theater]

@lsiepel not sure why the build has failed here. It seems that it never ran for the last commit I pushed. Do you know how we can retrigger it? I'm not sure how to login on the Jenkins and if I have the rights to do so...

[theater]

I got all the confirmations that the changes work well. The PR is ready for a review.

[lsiepel]

Thanks LGTM