Autelis Pool Control for OpenHAB2 #168
Conversation
|
A thing with 50 channels is an easy one to start with...? Well, if you say so ;-) |
kaikreuzer
added
the
new binding
| @@ -0,0 +1,11 @@ | |||
| # binding | |||
There was a problem hiding this comment.
You should remove this file - it is meant as an example on how to provide localized texts in other languages.
|
Thanks @digitaldan, I am done with my first review - waiting for your updates! |
|
Just a last question: Does this controller provide any means of discovery, e.g. through UPnP or mDNS/Bonjour? |
|
@kaikreuzer thanks for the review! It does not support a traditional discovery, but by default it sets its dns name (though dhcp) to "poolcontrol" which is how they recommend finding it on your network. I was actually planning doing this now that I understand how autodiscovery works ( having implemented it for the squeezebox addon). Also, what is the policy on squashing commits? I know that the growing commit history on OH has been an issue. |
|
Ok, so discovery could be an additional PR then, if it works through the hostname. Maybe this could somehow be combined with the network binding, which scans the network for all IP devices?
See https://github.com/openhab/openhab2/blob/master/CONTRIBUTING.md#conventions: |
|
|
||
| @Override | ||
| protected void startBackgroundDiscovery() { | ||
| new Thread(new Runnable() { |
There was a problem hiding this comment.
Instead of creating a new thread, could you please use the available scheduler field from AbstractDiscoveryService?
|
Looks good, thanks. Besides the small review comments, may I ask you to
|
|
No problem, I will make those changes shortly. |
|
@kaikreuzer if I run mvn license:format on just my binding it fails, complaining it can't find "src/etc/header.txt" , If i run it against the root of the project it works, but modifies just about everything. Is there any easy fix or should I just manually update the headers? |
|
openhab » openhab2 #16 FAILURE |
|
openhab » openhab2 #17 FAILURE |
|
openhab » openhab2 #18 FAILURE |
Signed-off-by: Dan Cunningham <dan@digitaldan.com> (github: @digitaldan)
|
openhab » openhab2 #19 FAILURE |
|
Sorry about the cloudbees noise, my rebase did not go as smooth as usual. Its failing now on the Astro binding due to the thing status refactoring. I'm not sure how my commit would cause this, but it looks like an easy enough fix, let me know what you want to do. |
|
Yeah, my mistake, the new status handling broke the API, which I did not notice before :-( |
|
Compiles again, see also https://groups.google.com/d/msg/openhab2/J7aQhmc5AqI/kVqFXvmF34AJ |
Yes, this is right, it only works from the root. |
|
Thanks Kai! Btw, when i ran maven it put "2014-2014" in the header message, I see in the header.txt file its "2014-${year}" so I'm not sure what was pulling 2014 vs 2015 for the year variable. |
|
The year is set in here: https://github.com/openhab/openhab2/blob/master/pom.xml#L158 |
|
@digitaldan: May I ask you to create a short documentation for this binding until Sunday (May 24th)? |
|
@digitaldan: Just noticed that the README.md is still missing - would you manage to add a very short one asap? Would be good to have at least something; everything is better than a 404 link ;-) |
|
Agreed! See my latest PR, is contains a basic README (I'll revise it over the next few weeks) and a small thing xml fix. |
Signed-off-by: Kai Kreuzer <kai@openhab.org>
I thought I would start with porting a relatively easy binding to get familiar with how OH2 works.