Ole8700 / openhab

Automatically exported from code.google.com/p/openhab
GNU General Public License v3.0
1 stars 0 forks source link

Implement Somfy URTSI II Binding #276

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Oliver Libutzki:

today I found the time to finish the Somfy URTSI II binding: 
https://github.com/OLibutzki/openhab.urtsi.binding/

Kai, it would be great, if you could have a look at it.

There is one thing I didn't solve: How can I react on the response of the 
device?

Example:
I send an Up command to a rollershutter, but the URTSI device doesn't confirm 
this command. Unfortunatlely the state of the item is "UP" anyway. Is there a 
way to tell the bus that the command has not been executed successfully?

Btw. I implemented most of the binding in Xtend... I added the required plugins 
in the pom.xml, so Maven build should be successful.

Original issue reported on code.google.com by kai.openhab on 5 May 2013 at 7:02

GoogleCodeExporter commented 9 years ago

Original comment by teichsta on 7 May 2013 at 4:00

GoogleCodeExporter commented 9 years ago

Original comment by kai.openhab on 7 May 2013 at 4:58

GoogleCodeExporter commented 9 years ago
Hi Oliver,

Here are my review comments:
1. Please see our coding conventions at 
https://code.google.com/p/openhab/wiki/HowToContribute - it would be nice if 
you could adjust your code accordingly (mainly adding missing Javadocs, @author 
and @since tags)
2. Does your binding really require Java7? If not, please set the required 
execution environment in the manifest to Java5 or Java6.
3. change binding version from 1.2.0 to 1.3.0 (sorry, that's because I was too 
slow ;-))
4. Your binding MUST NOT reference the item registry - sorry that this is not 
yet clearly stated in the documentation, but it is an architectural principal 
that we want to enforce. Bindings should only communicate through the event bus 
and thus be stateless (see 
http://wiki.openhab.googlecode.com/hg/images/events.png). For your case (the 
method internalReceiveCommand), you should instead simply use 
"eventPublisher.postUpdate(itemName, command)"
5. The binding configuration would be nicer if you'd move the configuration of 
the serial port to the openhab.cfg file - you would then only specify the 
channel in the items file. This is how other bindings handle it as well (when 
requiring a serial port or a host ip).
6. UrsiDevice.xtend, l. 152: Wouldn't you need a Thread::sleep in this while 
loop?
7. What is AbstractUrtsiBinding good for? I guess it can be removed
8. ArrayHelper should be removed - the one method is only used in one place and 
has only one line - this does not really qualify for a class of its own.

That's all - looking forward to the next update!

Original comment by kai.openhab on 8 May 2013 at 9:09

GoogleCodeExporter commented 9 years ago
Hi Kai,

thanks for reviewing the binding. My comments:
1. I'm going to extend the code accordingly.
2. The code doesn't require Java 7, I'm going to switch to Java 5 or 6.
3. sure
4. I see
5. In theory it's possible to use two or more devices. Each of them offers 16 
channels. Putting the port into the global config file restricts the binding to 
one device. Anyway, it's your decision... for me (and most of the others users) 
one device is sufficient. Please give me feedback concerning this one.
6. yes
7. I just needed this due to a Bug in the Xtend Maven Plugin which compiles the 
Xtend file. I guess this bug has been solved in Xtend 2.4.1, but openhab uses 
Xtext and Xtend 2.3.1. It's ugly I know...
8. I created this class because of an Xtend limitation: 
https://groups.google.com/forum/#!msg/xtend-lang/p33jDUNh0Hg/z4u7B7BHhJgJ
With Xtend 2.4.1 AbstractUrtsiBinding and ArrayHelper can be deleted.

Do you have any plans concerning an Xtext/Xtend update?

Kind regards
Oliver

Original comment by oliver_l...@gmx.de on 9 May 2013 at 11:19

GoogleCodeExporter commented 9 years ago
Hi Kai,

I have moved the binding to a googlecode clone: 
http://code.google.com/r/oliver-libutzki-urtsi-binding/

I fixed 1, 2, 3, 4 and 6.

For 5 I expect your decision.

7 and 8 cannot be fixed without an Xtext/Xtend update, but I documented this in 
the code.

Kind regards
Oliver

Original comment by oliver_l...@gmx.de on 11 May 2013 at 11:29

GoogleCodeExporter commented 9 years ago
Thanks for addressing 1,2,3,4 and 6 so quickly!
Regarding 5: I would say that supporting a single one is enough - we do it 
similarly for other bindings like KNX, where you could have multiple 
connections in theory, but in practice, nobody ever asked for it.
If you would still refer to support multiple devices, you could also do it 
similarly to https://code.google.com/p/openhab/wiki/SamsungTVBinding, i.e. 
introduce an id for each device and configure it in openhab.cfg.

7+8: How do you assess the effort for an upgrade? Will existing things break 
and need to be adapted or can we expect it to be rather smooth?

Original comment by kai.openhab on 13 May 2013 at 5:51

GoogleCodeExporter commented 9 years ago
Regarding 5: Ok, I'm going to refactor this accordingly.

Regarding 7+8: I recently updated a project for a customer without having any 
bigger issues (but we don't use Xbase). Xtend's performance increases 
massively, so I highly recommend this update... right after the release this is 
the best moment to do such a step I think.

I can offer you to perform the upgrade on a clone, but we should file another 
issue for this work as it is not related to this one directly.

Kind regards
Oliver

Original comment by oliver_l...@gmx.de on 14 May 2013 at 11:54

GoogleCodeExporter commented 9 years ago
As I plan an overall upgrade to Kepler end of June, I think we can wait with 
the Xtext upgrade until then (Kepler will include Xtext 2.5.0, if I saw it 
right). So let's keep your code for 7+8 as it is for the moment.

Apparently Xtext 2.4.0 brought major changes to Xbase, so I could imagine that 
many things will break - if you like, you could already do some tests on this, 
so that we are prepared for the big upgrade end of June then :-)

Original comment by kai.openhab on 14 May 2013 at 7:49

GoogleCodeExporter commented 9 years ago
Kepler will include Xtext/Xtend 2.4.2: 
https://twitter.com/xtendlang/statuses/333677692035690496/

It might be useful to switch to Xtext/Xtend 2.4.1 prior to the Kepler update; 
just to avoid a big bang update ;-)

Original comment by oliver_l...@gmx.de on 15 May 2013 at 9:55

GoogleCodeExporter commented 9 years ago
Ok, I was talking about Xtext version: 
http://projects.eclipse.org/projects/modeling.tmf.xtext/releases/2.5.0

So please feel free to open a new issue fot Xtend 2.4.1 upgrade and assign it 
to yourself :-)

Original comment by kai.openhab on 15 May 2013 at 11:44

GoogleCodeExporter commented 9 years ago
Regarding 5: Change is pushed.

I have rewritten the UrtsiBinding class in Java due to the Xtend 2.3.0 problems.

Anyway, there are still a couple of Xtend files and this binding can be used as 
reference implementation, especailly concerning the Maven part.

Kind regards
Oliver

Original comment by oliver_l...@gmx.de on 20 May 2013 at 4:25

GoogleCodeExporter commented 9 years ago
stumbled upon the protocol specification: 
http://www.somfysmarthome.co.uk/pdfs/RS485%20RTS%20Transmitter.pdf

Original comment by teichsta on 5 Jun 2013 at 4:19

GoogleCodeExporter commented 9 years ago
This is the correct specification: 
http://www.avoutlet.com/images/product/additional/s/1810872-ins.pdf

Currently the openhab binding supports the RS232 connection.

Original comment by oliver_l...@gmx.de on 17 Jun 2013 at 1:19

GoogleCodeExporter commented 9 years ago

Original comment by kai.openhab on 18 Jun 2013 at 9:21