Fibaro Smart Implant FGBS-222

@chris would know for sure.

Iā€™m not sure - itā€™s not a feature that has been requested very often and Iā€™ve not spent a lot of time looking at it so I donā€™t really remember where itā€™s at.

@robmac maybe you can tell us how protection works and how to configure it? thank you!

@oeiber Do you think this fix that @rmichalak made is in the version you are using?

That sounds this part is OK. I am guessing if you trace the messages it will send it but get back a message saying nothing changed. I think the fix might not have made it into the binding but canā€™t be sure without pulling the latest code but I would have to mess a bit to do that.

It has worked. This device is a bit odd as the inputs can be configured in various ways and this impacts how they work with the outputs. Also not all possible protection values are supported by the device.

Unfortunately I do not use this specific version, because I thaught the change has already been included to the official version.
Iā€˜m sorry for that.

Just a question to @chris: Is it planned to merge the change with the master branch?

regards,
Oliver

I know Chris is working through a 3 month backlog of changes due to the new build system. I just wanted to warn that even if this is planned it may take a while to actually happen.

I donā€™t see any open or closed PR for that:

https://github.com/openhab/org.openhab.binding.zwave/pulls

Without a PR, no fix :grinning:

1 Like

Apparently, there was an issue opened, but it then needs programming to generate a PR.

1 Like

I thought this was already done:

If it was just a dirty hack, Chris would not accept that change anyway. But it does not help anyone if changes to the code are not published to the official zwave repo.

3 Likes

I think if you read the thread @rmichalak tried but was not sure how it should all be submitted.

As to hack if you mean in the sense that @rmichalak used his skills to make it work then fine if you donā€™t like his style then I am not sure how to respond.

In my book something that works is always better than something pretty that serves no purpose. His code works. The current code may be more beautiful but does not support this version of the command class. I was very impressed he spotted the issue.

Please be careful not to wrongly attribute quotes. That is not my words or claim. I totally failed to find the missing bit in the code and fix it. All credit to @rmichalak but possibly if it is not in the release or submitted someone could help by explaining what needs doing or confirm it is just that Chris is snowed under and it will arrive in time.

Yes, that is what I meant.
I have no programming skills so maybe I used the wrong words :sunglasses:

It is just the word hack has gained various connotations over the years. It sways between a respectful term indicating great skill and a derogatory term implying not nice depending on context.

As i say the issue was not easy to spot and @rmichalak admitted he had no deep knowledge of zwave. To find it and fix was very cool in the time he managed to do it.

I am not saying there may not be an even better fix but that would require someone that knows more about the binding and the fix does work so might be of interest to people in an interim release even if it never makes it to a full.

Iā€™m not sure what change you are asking to merge? Can you reference this please?

1 Like

Hi Chris,

It is the one made by @rmichalak to add support for version 2 of the protection command class. He may have raised the request or possibly it is sat to one side needing pushing over to you for review
.

Itā€™s not about style necessarily, but there does have to be some code control, and we canā€™t allow ā€œnasty hacksā€ into the binding or it will become unreadable and unusable.

Maybe, but it needs to be coded properly - it canā€™t be a case of adding ā€œanythingā€ even if it works.

As nobody has reviewed it it could be hack in the good sense

Can you provide a reference please? I donā€™t see any open OR about the protection class?