Closed GoogleCodeExporter closed 8 years ago
Hi, i've just created the blank Wiki-Page
http://code.google.com/p/openhab/wiki/OpenSprinkler for the documentation.
Write-Access should be granted now.
Original comment by teichsta
on 4 Aug 2013 at 9:01
I've just published a first draft wiki content at the page you created.
I should note two things about the link that you created:
1) The name should be OpenSprinklerPi, not OpenSprinkler. This is because
OpenSprinkler also exists and is not what this binding controls. This binding
is specifically for the OpenSprinkler Pi edition.
2) The wiki link is not consistent with the other binding links, but it isn't a
big deal. For example, rather than OpenSprinkler being the link, to be
consistent it would be named OpenSprinklerPiBinding.
Original comment by jonathan@jonathangiles.net
on 4 Aug 2013 at 9:13
I've just quickly taken a (very poor quality) video of the binding in action,
to prove that it does actually work and has been tested. Here is the YouTube
video to prove it:
https://www.youtube.com/watch?v=RgZo8Rg0yEs
Original comment by jonathan@jonathangiles.net
on 4 Aug 2013 at 9:34
Cool, thanks! I have added it to the openHAB YouTube playlist :-)
Btw, you should better hold your phone in landscape mode when producing videos!
Original comment by kai.openhab
on 4 Aug 2013 at 9:41
Yeah, perhaps I should refilm the video....?
Original comment by jonathan@jonathangiles.net
on 4 Aug 2013 at 9:43
At least as soon as you have real sprinklers attached :-)
Original comment by kai.openhab
on 4 Aug 2013 at 9:47
Ok, updated video: https://www.youtube.com/watch?v=lT0uxFlwu9s :-)
Original comment by jonathan@jonathangiles.net
on 4 Aug 2013 at 9:56
Thanks, I have replaced it in the playlist!
Original comment by kai.openhab
on 5 Aug 2013 at 8:41
Great work!
I have been using the standard opensprinkler (non RPi) for two seasons now,
using the http binding, rules, and direct http commands:
http://rayshobby.net/?page_id=730#httpget
I take it this only works if openhab is installed on the opensprinkler pi?
Any chance of extending this enable the standard opensprinkler?
Original comment by g.g.r...@gmail.com
on 5 Aug 2013 at 7:38
Yes, this binding is only for OpenSprinkler Pi, and only when OpenHAB is
running on the Raspberry Pi it is attached to. The binding communicates
directly over the Raspberry Pi GPIO pins, rather than via http. I would be
quite happy to write a OpenSprinkler binding that can communicate via http, but
I don't have the standard OpenSprinkler to test with. Without this it is
largely an exercise in futility :-)
Original comment by jonathan@jonathangiles.net
on 5 Aug 2013 at 7:49
Hi Jonathan,
please find attached my review comments:
# lib
* please enhance lib name with it's version
# OSGI-INF
* the given name (tag) of both the binding and genericbindingprovider is both
'org.openhab.binding.opensprinklerpi' please rename the genericbindingprovider
to something more reasonable
# OpenSprinklerPiBindingProvider
* javadoc is missing on both Class- and Methodlevel
# OpenSprinklerPi
* please move description of numberOfStations (currently contained in
classlevel javadoc) to javadoc of that specific member
* please add javadoc for at least every public and protected constructor and
method
* please add CR/LF at EOF
# OpenSprinklerPiBinding
* please add javadoc on class level
* please remove template inline comments
* L62 better use numberOfStations as parameter here
* L82 insert log statement which explains the else case
# OpenSprinklerPiGenericBindingProvider
* please add some valid configurations to classlevel javadoc
# OpenSprinklerPiTest
* please add some javadoc on classlevel to explain the purpose of this somewhat
'special' class
* please move class to another package to explicitly express it's special
purpose
Original comment by teichsta
on 5 Aug 2013 at 8:15
Thanks - I'll get onto these ASAP. Just a few comments / questions:
1) I didn't realise that the JavaDoc was published anywhere - is there a URL of
the current release so I can see what is expected?
2) I don't follow your OSGI-INF comment. I don't know what a more reasonable
name would be?
3) The OpenSprinklerPi and OpenSprinklerPiTest classes will probably be removed
at some point, replaced by a jar file containing the .class files. This was
actually how I originally architected it but then simplified the changeset.
This way the OpenSprinkler Pi code can continue to be developed in a neutral
way and included in various other projects (as well as expanded perhaps to also
support the http approach available in the original OpenSprinkler device).
Original comment by jonathan@jonathangiles.net
on 5 Aug 2013 at 8:24
[deleted comment]
Hi,
> 1) is there a URL of the current release so I can see what is expected?
no, the javadoc isn't published (yet). A good documented example binding is the
DMX binding by Davy or Kai's KNX binding.
> 2) I don't follow your OSGI-INF comment. I don't know what a more reasonable
name would be?
Sorry, this was a very unspecific comment. Both of the components described in
both files do have a name which should be unique (attribute "name" of element
"scr") . Currently both names are equal and should be changed thus. A good name
for the genericbinding provider could be
"org.openhab.binding.opensprinklerpi.genericbindingprovider". Does that makes
sense?
> 3) This way the OpenSprinkler Pi code can continue to be developed in a
neutral way and included in various other projects (as well as expanded perhaps
to also support the http approach available in the original OpenSprinkler
device).
ok, understand. Then probably all files potentially moved to that external lib
should be moved to another package.
Original comment by teichsta
on 5 Aug 2013 at 8:44
I believe I have made most (if not all) changes discussed above. I have also
changed the binding to be called openSprinkler, rather than openSprinklerPi,
given the possibility that the binding may one day support connecting to an
OpenSprinkler device (which is not dependent on a Pi), and may also support
connecting via HTTP rather than GPIO.
I am just compiling and testing the changes, and will then push them all to my
clone shortly.
Original comment by jonathan@jonathangiles.net
on 6 Aug 2013 at 3:05
I have just now implemented support for OpenSprinkler devices that use the
interval program that is available for OpenSprinkler and OpenSprinkler Pi. That
means that the OpenSprinkler binding for OpenHAB should now be able to support
both (although the configuration will be slightly different). I'll update the
binding to support both styles of communication (http and gpio).
Original comment by jonathan@jonathangiles.net
on 6 Aug 2013 at 4:39
I've just pushed the changes to
http://code.google.com/r/jonathan-opensprinklerpi/source/detail?r=979fc397a1a09b
3d1bf7cd6a735c615f97745c3b
Changeset comment:
Big cleanup based on review feedback, as well as extracting out OpenSprinkler
code into a separate library jar and introducing support for the original
OpenSprinkler (connecting over http rather than gpio pins)
I need to go update the wiki, but in short here are the new config details:
################################ OpenSprinkler Binding
################################
# The type of OpenSprinkler connection to make. There are two valid options,
and the
# default mode is gpio.
#
# gpio: this mode is only applicable when running OpenHAB on a Raspberry Pi,
which
# is connected directly to an OpenSprinkler Pi. In this mode the
communication
# is directly over the GPIO pins of the Raspberry Pi
# http: this mode is applicable to both OpenSprinkler and OpenSprinkler Pi, as
long as
# they are running the interval program. Realistically though if you have
an
# OpenSprinkler Pi, it makes more sense to directly connect via gpio mode.
# openSprinkler:mode=gpio
# If the http mode is used, you need to specify the url of the internal program,
# and the password to access it. By default the password is 'opendoor'.
# openSprinkler:httpUrl=http://localhost:8080/
# openSprinkler:httpPassword=opendoor
# The number of stations available. By default this is 8, but for each
expansion board
# installed this number will can be incremented by 8. Default value is 8.
# openSprinkler:numberOfStations=8
Original comment by jonathan@jonathangiles.net
on 6 Aug 2013 at 6:26
I have now updated the wiki page to detail the new functionality:
http://code.google.com/p/openhab/wiki/OpenSprinkler
Original comment by jonathan@jonathangiles.net
on 6 Aug 2013 at 6:34
Will re-review and merge into default!
Thanks Jonathan!
Original comment by teichsta
on 6 Aug 2013 at 4:21
Merged binding to default repo (see
http://code.google.com/p/openhab/source/detail?r=ec963bf17b9469504c8832753e55634
ed6f441c5).
Finally i made some small changes which i hope you don't mind:
* changed OS mode to Enum
* made parsing of the OS mode more restrictive in updated()
* made (theoretic) default case in updateBinding() more restrictive
Thank you very much for this contribution! I am looking to all your upcoming
bindings (Ninjablock, etc.)
Original comment by teichsta
on 6 Aug 2013 at 7:52
Thanks for merging, and the changes are fine.
Regarding other bindings: my wife is reminding me that I have a lot of other
projects I need to get sorted before the baby arrives, so maybe this is the
only binding for now.....but we'll see :-)
Original comment by jonathan@jonathangiles.net
on 6 Aug 2013 at 7:56
Take your time :-)
Original comment by teichsta
on 6 Aug 2013 at 7:57
You will soon have many sleepless nights and thus time for coding :-D
Original comment by kai.openhab
on 6 Aug 2013 at 7:58
Original issue reported on code.google.com by
jonathan@jonathangiles.net
on 4 Aug 2013 at 8:55