Add new binding for LIRC. #2537

kabili207 · 2016-11-26T02:32:13Z

This binding allows decoding and sending of infrared signals used by most remotes controls via LIRC (and its Windows counterpart WinLIRC).

Each remote that is known by LIRC is represented by a thing with a single channel. This channel is a String type that contains the name of the button the user pressed.

I had considered dynamically creating channels for each button on a remote, but I decided against it as most TV remotes can have upwards of of 30 or more buttons and only one can be activated at any given time.

Signed-off-by: Andrew Nagle kabili@zyrenth.com

sjsf

Thanks very much for this really good contribution!

I just had a look through and left a few comments - don't panic by the sheer number, it's mostly about blank lines where they don't add to the readability and a few minor remarks where you might want to have a look.

There is only one major question I have. It's about the channel, whether it should be only one for sending AND receiving, or maybe we should split this in two as we are talking about different physical things: one is a button on the remote control which was pressed, the other is a state of a device which should be altered. See also my inline comment about it.

sjsf · 2016-12-07T11:12:34Z

extensions/binding/org.eclipse.smarthome.binding.lirc/ESH-INF/i18n/lirc_xx_XX.properties

@@ -0,0 +1,11 @@
+# binding
+binding.lirc.name = <Your localized Binding name>


sjsf · 2016-12-07T11:14:16Z

extensions/binding/org.eclipse.smarthome.binding.lirc/ESH-INF/thing/bridge.xml

+    <!-- LIRC Server -->
+    <bridge-type id="bridge">
+        <label>LIRC Server</label>
+        <description>The LIRC Server represents the LIRC Server bridge.</description>


x representing x doesn't say much. How about "The LIRC Server Bridge"?

sjsf · 2016-12-07T11:15:27Z

extensions/binding/org.eclipse.smarthome.binding.lirc/ESH-INF/thing/bridge.xml

+            </parameter>
+            <parameter name="portNumber" type="integer" min="0" max="65536" step="1">
+                <label>Port Number</label>
+                <description>The port number the LIRC server is listening on.</description>


The port the LIRC server is listening to

sjsf · 2016-12-07T11:15:48Z

extensions/binding/org.eclipse.smarthome.binding.lirc/ESH-INF/thing/thing-types.xml

+        xmlns:thing="http://eclipse.org/smarthome/schemas/thing-description/v1.0.0"
+        xsi:schemaLocation="http://eclipse.org/smarthome/schemas/thing-description/v1.0.0 http://eclipse.org/smarthome/schemas/thing-description-1.0.0.xsd">
+
+    <!-- Sample Thing Type -->


"Sample", really??

sjsf · 2016-12-07T11:17:29Z

extensions/binding/org.eclipse.smarthome.binding.lirc/ESH-INF/thing/thing-types.xml

+
+    </thing-type>
+
+    <!-- Sample Channel Type -->


Sample, really?

sjsf · 2016-12-07T13:29:20Z

...n/java/org/eclipse/smarthome/binding/lirc/internal/discovery/LIRCRemoteDiscoveryService.java

+    }
+
+    private void addRemote(ThingUID bridge, String remote) {
+


sjsf · 2016-12-07T13:29:42Z

...lirc/src/main/java/org/eclipse/smarthome/binding/lirc/internal/messages/LIRCButtonEvent.java

+ */
+package org.eclipse.smarthome.binding.lirc.internal.messages;
+
+public class LIRCButtonEvent {


here some javadoc is missing

sjsf · 2016-12-07T13:30:08Z

...lirc/src/main/java/org/eclipse/smarthome/binding/lirc/internal/messages/LIRCButtonEvent.java

+package org.eclipse.smarthome.binding.lirc.internal.messages;
+
+public class LIRCButtonEvent {
+    String code;


these fields should all be private final

sjsf · 2016-12-07T13:30:16Z

...lirc/src/main/java/org/eclipse/smarthome/binding/lirc/internal/messages/LIRCButtonEvent.java

+    }
+
+    public String getCode() {
+


sjsf · 2016-12-07T13:30:30Z

...ng.lirc/src/main/java/org/eclipse/smarthome/binding/lirc/internal/messages/LIRCResponse.java

+ */
+package org.eclipse.smarthome.binding.lirc.internal.messages;
+
+public class LIRCResponse {


javadoc is missing

The binding does not send the number of repeats and does not support UNIX sockets, so the LIRC server must be exposed as over TCP. Signed-off-by: Andrew Nagle <kabili@zyrenth.com>

kabili207 · 2016-12-10T02:32:45Z

Okay, I've gone through and updated everything based on your suggestions. Splitting the channel into "event" and "transmit" channels worked nicely and does make a lot more sense.

The way LIRC handles transmitting repeats isn't how most users expect it to behave. Sending the command "KEY_VOLUMEUP 5" would actually transmit the volume up code six times. The commit changes that behavior to be more user-friendly. This also includes examples on using the binding. Signed-off-by: Andrew Nagle <kabili@zyrenth.com>

computerlyrik · 2017-03-20T15:52:02Z

+1 User wants to use this and gets to test binding with openhab2 on RPi3

Signed-off-by: Simon Kaufmann <simon.kfm@googlemail.com>

sjsf · 2017-07-07T11:49:20Z

Thanks a lot for addressing all the comments that quickly and my deepest apologies for then taking ages to getting back to this!

From my point of view this now is good to go. As your PR exceeds 1000 lines of code, we unfortunately have to run it through Eclipse's IP process. This will hopefully only add a few days or additional delay and normally won't impose any work on your side.

The only think I need to ask you is to not push any further commits to here. In case it should be necessary it will have to go through another PR.

sjsf · 2017-07-07T11:56:46Z

For the record: I have submitted this as CQ 13830

sjsf · 2017-07-12T06:09:39Z

Good news: the CQ has been approved.

sjsf · 2017-07-12T06:10:29Z

Thanks!

kaikreuzer · 2017-07-18T10:57:46Z

extensions/binding/org.eclipse.smarthome.binding.lirc/ESH-INF/thing/bridge.xml

+
+        <config-description>
+            <parameter name="host" type="text">
+                <label>The Host</label>


Please add context "network-address".

kaikreuzer · 2017-07-18T10:59:01Z

extensions/binding/org.eclipse.smarthome.binding.lirc/ESH-INF/thing/bridge.xml

+            <parameter name="host" type="text">
+                <label>The Host</label>
+                <description>The host of the LIRC server</description>
+                <required>true</required>


If there is a default, you usually should not set required to true.

kaikreuzer · 2017-07-18T10:59:17Z

extensions/binding/org.eclipse.smarthome.binding.lirc/ESH-INF/thing/bridge.xml

+                <default>localhost</default>
+            </parameter>
+            <parameter name="portNumber" type="integer" min="0" max="65536" step="1">
+                <label>Port Number</label>


"Port" should be good enough.

kaikreuzer · 2017-07-18T10:59:22Z

extensions/binding/org.eclipse.smarthome.binding.lirc/ESH-INF/thing/bridge.xml

+            <parameter name="portNumber" type="integer" min="0" max="65536" step="1">
+                <label>Port Number</label>
+                <description>The port number the LIRC server is listening to.</description>
+                <required>true</required>


If there is a default, you usually should not set required to true.

kaikreuzer · 2017-07-18T11:00:07Z

extensions/binding/org.eclipse.smarthome.binding.lirc/META-INF/MANIFEST.MF

+Bundle-SymbolicName: org.eclipse.smarthome.binding.lirc;singleton:=true
+Bundle-Vendor: Eclipse.org/SmartHome
+Bundle-Version: 0.9.0.qualifier
+Bundle-RequiredExecutionEnvironment: JavaSE-1.7


Should be 1.8

kaikreuzer · 2017-07-18T11:00:37Z

extensions/binding/org.eclipse.smarthome.binding.lirc/OSGI-INF/LIRCHandlerFactory.xml

+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+    Copyright (c) 2014-2016 by the respective copyright holders.


looks like an outdated header (should be 2017)

kaikreuzer · 2017-07-18T11:01:16Z

extensions/binding/org.eclipse.smarthome.binding.lirc/README.md

+[Install]
+WantedBy=multi-user.target
+```
+By default, LIRC will listen on IP address 0.0.0.0 (any available IP address) and port 8765. If you would


please add an empty line after every code section.

kaikreuzer · 2017-07-18T11:01:32Z

extensions/binding/org.eclipse.smarthome.binding.lirc/README.md

+    Thing remote Samsung [ remote="Samsung" ]
+}
+```
+Bridge:


please add an empty line after every code section.

kaikreuzer · 2017-07-18T11:01:48Z

extensions/binding/org.eclipse.smarthome.binding.lirc/README.md

+* **remote**: The name of the remote as known by LIRC
+
+### Items
+```xtend


please add an empty line after every header.

kaikreuzer · 2017-07-18T11:01:55Z

extensions/binding/org.eclipse.smarthome.binding.lirc/README.md

+```
+
+### Rules
+```xtend


please add an empty line after every code header.

sjsf · 2017-07-18T14:09:24Z

see #3848

kabili207 · 2017-07-18T20:43:42Z

Thank you for taking care of that @SJKA!

sjsf suggested changes Dec 7, 2016

View reviewed changes

Add new binding for LIRC.

db823e5

The binding does not send the number of repeats and does not support UNIX sockets, so the LIRC server must be exposed as over TCP. Signed-off-by: Andrew Nagle <kabili@zyrenth.com>

kabili207 force-pushed the lirc branch from 3efaae6 to db823e5 Compare December 10, 2016 02:16

maggu2810 added Binding New Contribution labels Feb 7, 2017

adding about.html

edaf1a4

Signed-off-by: Simon Kaufmann <simon.kfm@googlemail.com>

sjsf approved these changes Jul 7, 2017

View reviewed changes

sjsf added the CQ label Jul 7, 2017

sjsf merged commit 28e865a into eclipse-archived:master Jul 12, 2017

kaikreuzer reviewed Jul 18, 2017

View reviewed changes

sjsf mentioned this pull request Jul 18, 2017

LIRC binding follow-up #3848

Merged

kaikreuzer added this to the 0.9.0 milestone Nov 30, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new binding for LIRC. #2537

Add new binding for LIRC. #2537

kabili207 commented Nov 26, 2016

sjsf left a comment

sjsf Dec 7, 2016

sjsf Dec 7, 2016

sjsf Dec 7, 2016

sjsf Dec 7, 2016

sjsf Dec 7, 2016

sjsf Dec 7, 2016

sjsf Dec 7, 2016

sjsf Dec 7, 2016

sjsf Dec 7, 2016

sjsf Dec 7, 2016

kabili207 commented Dec 10, 2016

computerlyrik commented Mar 20, 2017

sjsf commented Jul 7, 2017

sjsf commented Jul 7, 2017

sjsf commented Jul 12, 2017

sjsf commented Jul 12, 2017

kaikreuzer Jul 18, 2017

kaikreuzer Jul 18, 2017

kaikreuzer Jul 18, 2017

kaikreuzer Jul 18, 2017

kaikreuzer Jul 18, 2017

kaikreuzer Jul 18, 2017

kaikreuzer Jul 18, 2017

kaikreuzer Jul 18, 2017

kaikreuzer Jul 18, 2017

kaikreuzer Jul 18, 2017

sjsf commented Jul 18, 2017

kabili207 commented Jul 18, 2017

		@@ -0,0 +1,11 @@
		# binding
		binding.lirc.name = <Your localized Binding name>

Add new binding for LIRC. #2537

Add new binding for LIRC. #2537

Conversation

kabili207 commented Nov 26, 2016

sjsf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kabili207 commented Dec 10, 2016

computerlyrik commented Mar 20, 2017

sjsf commented Jul 7, 2017

sjsf commented Jul 7, 2017

sjsf commented Jul 12, 2017

sjsf commented Jul 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sjsf commented Jul 18, 2017

kabili207 commented Jul 18, 2017