[salus] Initial contribution

[magx2]

[salus] Initial contribution

Description

This is a new binding that is responsible to connect to Salus Cloud. Please refer to README.md to get more information.

[holgerfriedrich]

Thank you for your contribution!
This is not a full review, just a few comments.

Besides that, a few things need to be resolved:

• please check SAT warnings, mvn verify will create target/summary_report.html... it should be at least closer to 0
• too many compilation warnings (Null) - maybe a few vanish when you resolve SAT warnings
• SAT shows a forbidden package StringUtils - maybe @lolodomo can comment on this, we have some utils in core as well....

i18n seems ok, javadoc build is fine as well.

[holgerfriedrich]

@holgerfriedrich FYI the build is failing because if formatter, but those are not my classes (main build is also failing)

give it a few hours to settle, Kai is creating the 4.1 release....

[holgerfriedrich]

Great progress, thanks!

Lets take a look at the other topics:

• apache.commons should removed. See Removal of dependency on 'org.apache.commons.*' #7722. I marked a few easy things. Could you please care for replacing Pair?
• Still a lot of null warnings, a few seem to be correct and need additional checks in the code. See also my comment in the source file for one example. I have also seen a null check before accessing a variable - but there is still a warning, as the variable might be changed in the background. To avoid the warning, work with a local copy, do the null check on the copy, call the methods on the copy.
  I think in the tests - especially when using mockito - the warnings could be suppressed, but I leave that to a maintainer.
• I noticed that you are using type inference a lot (var). I was previously asked to remove this. But that is something for a maintainer to comment on as well....

[holgerfriedrich]

Pushed changes for nulls

Thanks, this is really good progress.
I think you could remove the suppression statements - as we have a very restricted set if allowed suppressions.

[holgerfriedrich]

I think you could remove the suppression statements - as we have a very restricted set if allowed suppressions.

Hmm, but they are suppressing staff

Did not check, I interpreted the warning as if they would have been ignored.....

[holgerfriedrich]

@magx2 I think that is all I can help here. My comments have been addressed.

[holgerfriedrich]

Thanks! What's the next steps?

Wait for a maintainer to pick this up. This usually takes time...

[lsiepel]

It would also be usefull to add yourself to the codeowner file, so that in future PR's you are automatically added as reviewer.

[lsiepel]

Thanks for your contribution!
Ran out of time, so no full review, just quick comments. I reviewed the bridgehandler and when looking at the device handler i would repeat most comments. Please check if the comments also apply to other classes.
Ping when you're ready and i'll proceed with the review.

[magx2]

@lsiepel I've applied your changes.

Also testing of the plugin was already done by my (after each major change I'm deploying Salus binding in my own OH instance)

[lsiepel]

Thank you for this contribution. LGTM.

Now, you could add your binding's logo to the openHAB website. See https://www.openhab.org/docs/developer/addons/#add-your-add-on-s-logo-to-the-openhab-website

SVG is preferred:
https://next.openhab.org/docs/developer/addons/#add-your-add-on-s-logo-to-the-openhab-website-and-the-ui