[rfxcom] Add ability to properly receive configured command ids, depr…

…ecated hard-coded guesses. (openhab#10940)

Currently, when a message is received the command will be determined only from the hard-coded set of ON/OFF commands, which means if I configure
a thing and attach it to a switch there is no guarantee that it will respond as expected to receive commands. This PR changes the message factory
to require either bytes (from the RFXcom device) or all the details required to make a transmissable message, including the confguration of the
associated Thing. At the moment, this is only used by lighting4 and raw types, but I expect it will be useful for others in the future.

The hard-coded ON/OFF commands in lighting4 are based on experimentation with different devices, and are not at all consistent. This PR deprecates the
use of those so that in a future release, Lighting4 devices will only work when configured with the correct command ids up front. This will resolve
inconsistent behaviour where devices that have been discovered may work correctly, work only for ON or OFF but not both, have ON and OFF the wrong
way around, turn ON one physical device and OFF another, or any combination of those possibilities.

Signed-off-by: James Hewitt <[email protected]>
Signed-off-by: Dave J Schoepel <[email protected]>

bundles/org.openhab.binding.rfxcom/README.md

@@ -136,15 +136,10 @@ Bridge rfxcom:tcpbridge:sunflower [ host="sunflower", port=10001 ] {
 
 ## Thing Configuration
 
-Available configuration parameters are:
-
-| Applies to | Parameter Label | Parameter ID | Description                                                          | Required | Default |
-|------------|-----------------|--------------|----------------------------------------------------------------------|----------|---------|
-| All things | Device ID       | deviceId     | (Unique) id of the device, for example "100001.1", "B.8" or "286169" | true     |         |
-| All things | Sub type        | subType      | Sub type, note that every thing-type has its own sub types           | true     |         |
-| Lighting4  | Pulse           | pulse        | Pulse length used by the device, only used when sending              | false    | 350     |
-| Lighting4  | On command ID   | onCommandId  | Id of the command which should be send to turn the device ON         | false    | 1       |
-| Lighting4  | Off command ID  | offCommandId | Id of the command which should be send to turn the device OFF        | false    | 4       |
+Configuration parameters are listed alongside each thing type. Most devices only require a deviceId and
+a subType, but some things require additional configuration. The deviceId is used both when receiving and
+transmitting messages, the subType is mainly used when sending messages, but it can vary between device
+types.
 
 ## Channels
 
@@ -690,7 +685,27 @@ A Lighting2 device
 
 ### lighting4 - RFXCOM Lighting4 Actuator
 
-A Lighting4 device
+A Lighting4 device. The specification for the PT2262 protocol includes 3 bytes for data. By
+convention, the first 20 bits of this is used for deviceId, and the last 4 bits is used for
+command, which gives us a total of 16 commands per device.
+
+Depending on your device, you may have only one command, one pair of commands (on/off), or
+any other multiple, for example, a set of 4 sockets with an on/off pair for each and an
+additional pair for "all".
+
+Different device manufactures using this protocol will use different schemes for their
+commands, so to configure a thing using the lighting4 protocol, you must specify at least
+one commandId in the thing configuration. If a device has multiple sets of commands, you
+can configure multiple things with the same device id, but different commandIds.
+
+Some devices will expect a specific pulse length. If required, that can also be specified
+as a thing configuration parameter.
+
+Previously, openHAB would attempt to guess at the meaning of a commandId if it was not
+specified in the thing configuration based on devices seen in the wild. Due to the varying
+nature of devices, this behaviour is deprecated and will be removed in a future openHAB
+version. Until then, commands 1, 3, 5-13 and 15 are considered ON and 0, 2, 4 and 14 are
+considered OFF when the `onCommandId` or `offCommandId` for a device is not specified.
 
 #### Channels
 
@@ -709,95 +724,78 @@ A Lighting4 device
 *   subType - Sub Type
     *   Specifies device sub type.
 
-    *   PT2262 - PT2262
+        *   PT2262 - PT2262
 
 *   pulse - Pulse length
     *   Pulse length of the device
 
 *   onCommandId - On command
-    *   Specifies command to be send when ON must be transmitted
-
-        *   0 - OFF (value 0)
-        *   1 - ON (value 1)
-        *   2 - OFF (value 2)
-        *   3 - ON (value 3)
-        *   4 - OFF (value 4)
-        *   5 - ON (value 5)
-        *   6 - value 6
-        *   7 - ON (value 7)
-        *   8 - value 8
-        *   9 - ON (value 9)
-        *   10 - ON (value 10)
-        *   11 - ON (value 11)
-        *   12 - ON (value 12)
-        *   13 - value 13
-        *   14 - OFF (value 14)
-        *   15 - value 15
+    *   Specifies command that represents ON for this device.
 
 *   offCommandId - Off command
-    *   Specifies command to be send when OFF must be transmitted
-
-        *   0 - OFF (value 0)
-        *   1 - ON (value 1)
-        *   2 - OFF (value 2)
-        *   3 - ON (value 3)
-        *   4 - OFF (value 4)
-        *   5 - ON (value 5)
-        *   6 - value 6
-        *   7 - ON (value 7)
-        *   8 - value 8
-        *   9 - ON (value 9)
-        *   10 - ON (value 10)
-        *   11 - ON (value 11)
-        *   12 - ON (value 12)
-        *   13 - value 13
-        *   14 - OFF (value 14)
-        *   15 - value 15
+    *   Specifies command that represents OFF for this device.
 
-#### Examples
+*   openCommandId - Open command
+    *   Specifies command that represents OPEN for this device.
+
+*   closedCommandId - Closed command
+    *   Specifies command that represents CLOSED for this device.
+
+#### Discovering commandId values
 
-The support for lighting 4 in RFXCOM is less complete because a lot of different devices use the same chips and can not easily be distinguished.
+There are a number of ways to detect the commandId values for your device.
 
-So some extra configuration can be used for fine tuning the behavior of your Lighting4 devices.
-When configuring, three extra fields are available, being the the pulse length, and a separate command id for both on and off.
-If your item is auto-discovered normally the on or off command should be recognized properly.
+- You can turn on DEBUG messages for the rfxcom binding by adding the line
+  `<Logger level="DEBUG" name="org.openhab.binding.rfxcom"/>`
+  to your `log4j2.xml`. You will then be able to see the commandId in the log
+  file when you trigger the device.
+
+- You can link a Number Item to the commandId channel. The item will be updated with the
+  detected commandId when you trigger the device.
+
+- You can use RFXmngr to look at the data from the device. Use the last letter/number
+  of the hexadecimal "Code", and convert it from hexadecimal to decimal.
 
-For a USB attached RFXCOM on Windows the configuration could look like this (note that the `onCommandId`, `offCommandId` and `pulse` are all optional):
+#### Examples
+
+For a USB attached RFXCOM on Windows the configuration could look like this (note the `pulse` is optional):
 
 ```
 Bridge rfxcom:bridge:238adf67 [ serialPort="COM4" ] {
-    Thing lighting4 17745  [deviceId="17745",  subType="PT2262", onCommandId=7, offCommandId=4, pulse=800]
+    Thing lighting4 17745a  [deviceId="17745",  subType="PT2262", onCommandId=7, offCommandId=4]
+    Thing lighting4 17745b  [deviceId="17745",  subType="PT2262", onCommandId=10, offCommandId=2]
     Thing lighting4 motion [deviceId="286169", subType="PT2262", onCommandId=9, pulse=392]
 }
 ```
 
 Your items file could look like this:
 
 ```
-Switch Switch                               {channel="rfxcom:lighting4:238adf67:17745:command"}
-Number SwitchCommandId "Command ID [%d]"    {channel="rfxcom:lighting4:238adf67:17745:commandId"}
+Number SocketCommandId            {channel="rfxcom:lighting4:238adf67:17745a:commandId"}
+Switch SocketA                    {channel="rfxcom:lighting4:238adf67:17745a:command"}
+Switch SocketB                    {channel="rfxcom:lighting4:238adf67:17745b:command"}
 ```
 
-And if you want random actions on your relay you could for example do like this:
-
-```
-rule "Set random relay variations"
-    when
-        System started or
-        Time cron "/20 * * * * ?"
-    then
-        SwitchCommandId.sendCommand((Math::random * 15.9).intValue)
-end
-```
-
-#### Devices:
-
-| Brand | What          | Action      | Command ID | Supported | Source | 
-|-------|---------------|-------------|------------|-----------|--------|
-| Kerui | Motion Sensor | Motion      | 10         | as ON     | [#3103](https://github.com/openhab/openhab-addons/issues/3103) |
-| Kerui | Door Contact  | door open   | 14         | as OFF    | [#3103](https://github.com/openhab/openhab-addons/issues/3103) |
-| Kerui | Door Contact  | door closed | 7          | as ON     | [#3103](https://github.com/openhab/openhab-addons/issues/3103) |
-| Kerui | Door Contact  | tamper      | 7          | as ON     | [#3103](https://github.com/openhab/openhab-addons/issues/3103) |
+#### Known commandIds
+
+These are some commandIds from the field that may match your devices.
+
+| Brand | What          | Action      | Command ID | Source | 
+|-------|---------------|-------------|------------|--------|
+| Kerui | Motion Sensor | Motion      | 10         | [#3103](https://github.com/openhab/openhab-addons/issues/3103) |
+| Kerui | Door Contact  | door open   | 14         | [#3103](https://github.com/openhab/openhab-addons/issues/3103) |
+| Kerui | Door Contact  | door closed | 7          | [#3103](https://github.com/openhab/openhab-addons/issues/3103) |
+| Kerui | Door Contact  | tamper      | 11         | [#3103](https://github.com/openhab/openhab-addons/issues/3103) |
+| Energenie | 4 Socket Power Bar | Socket 1 on  | 15 | [Community](https://community.openhab.org/t/rfxcom-looking-to-improve-lighting4-call-for-users/123674) |
+| Energenie | 4 Socket Power Bar | Socket 1 off | 14 | [Community](https://community.openhab.org/t/rfxcom-looking-to-improve-lighting4-call-for-users/123674) |
+| Energenie | 4 Socket Power Bar | Socket 2 on  | 7  | [Community](https://community.openhab.org/t/rfxcom-looking-to-improve-lighting4-call-for-users/123674) |
+| Energenie | 4 Socket Power Bar | Socket 2 off | 6  | [Community](https://community.openhab.org/t/rfxcom-looking-to-improve-lighting4-call-for-users/123674) |
+| Energenie | 4 Socket Power Bar | Socket 3 on  | 11 | [Community](https://community.openhab.org/t/rfxcom-looking-to-improve-lighting4-call-for-users/123674) |
+| Energenie | 4 Socket Power Bar | Socket 3 off | 10 | [Community](https://community.openhab.org/t/rfxcom-looking-to-improve-lighting4-call-for-users/123674) |
+| Energenie | 4 Socket Power Bar | Socket 4 on  | 3  | [Community](https://community.openhab.org/t/rfxcom-looking-to-improve-lighting4-call-for-users/123674) |
+| Energenie | 4 Socket Power Bar | Socket 4 off | 2  | [Community](https://community.openhab.org/t/rfxcom-looking-to-improve-lighting4-call-for-users/123674) |
+| Energenie | 4 Socket Power Bar | All on       | 13 | [Community](https://community.openhab.org/t/rfxcom-looking-to-improve-lighting4-call-for-users/123674) |
+| Energenie | 4 Socket Power Bar | All off      | 12 | [Community](https://community.openhab.org/t/rfxcom-looking-to-improve-lighting4-call-for-users/123674) |
 
 ### lighting5 - RFXCOM Lighting5 Actuator
 

bundles/org.openhab.binding.rfxcom/src/main/java/org/openhab/binding/rfxcom/internal/config/RFXComLighting4DeviceConfiguration.java

@@ -21,7 +21,11 @@ public class RFXComLighting4DeviceConfiguration extends RFXComGenericDeviceConfi
     public static final String PULSE_LABEL = "pulse";
     public static final String ON_COMMAND_ID_LABEL = "onCommandId";
     public static final String OFF_COMMAND_ID_LABEL = "offCommandId";
+    public static final String OPEN_COMMAND_ID_LABEL = "openCommandId";
+    public static final String CLOSED_COMMAND_ID_LABEL = "closedCommandId";
     public Integer pulse;
     public Integer onCommandId;
     public Integer offCommandId;
+    public Integer openCommandId;
+    public Integer closedCommandId;
 }

bundles/org.openhab.binding.rfxcom/src/main/java/org/openhab/binding/rfxcom/internal/handler/RFXComBridgeHandler.java

@@ -102,8 +102,10 @@ public synchronized void send() throws IOException {
                 RFXComBaseMessage msg = queue.peek();
 
                 try {
-                    logger.debug("Transmitting message '{}'", msg);
                     byte[] data = msg.decodeMessage();
+                    if (logger.isDebugEnabled()) {
+                        logger.debug("Transmitting bytes '{}' for message '{}'", HexUtils.bytesToHex(data), msg);
+                    }
                     connector.sendMessage(data);
                     break;
                 } catch (RFXComException rfxe) {

bundles/org.openhab.binding.rfxcom/src/main/java/org/openhab/binding/rfxcom/internal/handler/RFXComHandler.java

@@ -95,10 +95,7 @@ public void handleCommand(ChannelUID channelUID, Command command) {
                     PacketType packetType = RFXComMessageFactoryImpl
                             .convertPacketType(getThing().getThingTypeUID().getId().toUpperCase());
 
-                    RFXComMessage msg = messageFactory.createMessage(packetType);
-
-                    msg.setConfig(config);
-                    msg.convertFromState(channelUID.getId(), command);
+                    RFXComMessage msg = messageFactory.createMessage(packetType, config, channelUID, command);
 
                     bridgeHandler.sendMessage(msg);
                 } catch (RFXComMessageNotImplementedException e) {
@@ -180,9 +177,9 @@ public void onDeviceMessageReceived(ThingUID bridge, RFXComDeviceMessage message
         try {
             if (config.matchesMessage(message)) {
                 String receivedId = PACKET_TYPE_THING_TYPE_UID_MAP.get(message.getPacketType()).getId();
-                logger.debug("Received message from bridge: {} message: {}", bridge, message);
-
                 if (receivedId.equals(getThing().getThingTypeUID().getId())) {
+                    logger.debug("Message from bridge [{}] matches thing [{}] message: {}", bridge,
+                            getThing().getUID().toString(), message);
                     updateStatus(ThingStatus.ONLINE);
 
                     for (Channel channel : getThing().getChannels()) {
@@ -194,19 +191,21 @@ public void onDeviceMessageReceived(ThingUID bridge, RFXComDeviceMessage message
                                 case CHANNEL_COMMAND:
                                 case CHANNEL_CHIME_SOUND:
                                 case CHANNEL_MOOD:
-                                    postNullableCommand(uid, message.convertToCommand(channelId, this));
+                                    postNullableCommand(uid, message.convertToCommand(channelId, config, this));
                                     break;
 
                                 case CHANNEL_LOW_BATTERY:
                                     updateNullableState(uid,
-                                            isLowBattery(message.convertToState(CHANNEL_BATTERY_LEVEL, this)));
+                                            isLowBattery(message.convertToState(CHANNEL_BATTERY_LEVEL, config, this)));
                                     break;
 
                                 default:
-                                    updateNullableState(uid, message.convertToState(channelId, this));
+                                    updateNullableState(uid, message.convertToState(channelId, config, this));
                                     break;
                             }
-                        } catch (RFXComException e) {
+                        } catch (RFXComInvalidStateException e) {
+                            logger.trace("{} not configured for {}", channelId, message);
+                        } catch (RFXComUnsupportedChannelException e) {
                             logger.trace("{} does not handle {}", channelId, message);
                         }
                     }

bundles/org.openhab.binding.rfxcom/src/main/java/org/openhab/binding/rfxcom/internal/messages/RFXComBBQTemperatureMessage.java

@@ -15,7 +15,9 @@
 import static org.openhab.binding.rfxcom.internal.RFXComBindingConstants.*;
 import static org.openhab.binding.rfxcom.internal.messages.ByteEnumUtil.fromByte;
 
+import org.openhab.binding.rfxcom.internal.config.RFXComDeviceConfiguration;
 import org.openhab.binding.rfxcom.internal.exceptions.RFXComException;
+import org.openhab.binding.rfxcom.internal.exceptions.RFXComInvalidStateException;
 import org.openhab.binding.rfxcom.internal.exceptions.RFXComUnsupportedChannelException;
 import org.openhab.binding.rfxcom.internal.exceptions.RFXComUnsupportedValueException;
 import org.openhab.binding.rfxcom.internal.handler.DeviceState;
@@ -124,13 +126,14 @@ public String getDeviceId() {
     }
 
     @Override
-    public State convertToState(String channelId, DeviceState deviceState) throws RFXComUnsupportedChannelException {
+    public State convertToState(String channelId, RFXComDeviceConfiguration config, DeviceState deviceState)
+            throws RFXComUnsupportedChannelException, RFXComInvalidStateException {
         if (CHANNEL_FOOD_TEMPERATURE.equals(channelId)) {
             return new DecimalType(foodTemperature);
         } else if (CHANNEL_BBQ_TEMPERATURE.equals(channelId)) {
             return new DecimalType(bbqTemperature);
         } else {
-            return super.convertToState(channelId, deviceState);
+            return super.convertToState(channelId, config, deviceState);
         }
     }
 

bundles/org.openhab.binding.rfxcom/src/main/java/org/openhab/binding/rfxcom/internal/messages/RFXComBatteryDeviceMessage.java

@@ -14,6 +14,8 @@
 
 import static org.openhab.binding.rfxcom.internal.RFXComBindingConstants.CHANNEL_BATTERY_LEVEL;
 
+import org.openhab.binding.rfxcom.internal.config.RFXComDeviceConfiguration;
+import org.openhab.binding.rfxcom.internal.exceptions.RFXComInvalidStateException;
 import org.openhab.binding.rfxcom.internal.exceptions.RFXComUnsupportedChannelException;
 import org.openhab.binding.rfxcom.internal.handler.DeviceState;
 import org.openhab.core.library.types.DecimalType;
@@ -36,13 +38,14 @@ abstract class RFXComBatteryDeviceMessage<T> extends RFXComDeviceMessageImpl<T>
     }
 
     @Override
-    public State convertToState(String channelId, DeviceState deviceState) throws RFXComUnsupportedChannelException {
+    public State convertToState(String channelId, RFXComDeviceConfiguration config, DeviceState deviceState)
+            throws RFXComUnsupportedChannelException, RFXComInvalidStateException {
         switch (channelId) {
             case CHANNEL_BATTERY_LEVEL:
                 return convertBatteryLevelToSystemWideLevel(batteryLevel);
 
             default:
-                return super.convertToState(channelId, deviceState);
+                return super.convertToState(channelId, config, deviceState);
         }
     }
 

bundles/org.openhab.binding.rfxcom/src/main/java/org/openhab/binding/rfxcom/internal/messages/RFXComBlinds1Message.java

@@ -15,7 +15,9 @@
 import static org.openhab.binding.rfxcom.internal.RFXComBindingConstants.*;
 import static org.openhab.binding.rfxcom.internal.messages.ByteEnumUtil.fromByte;
 
+import org.openhab.binding.rfxcom.internal.config.RFXComDeviceConfiguration;
 import org.openhab.binding.rfxcom.internal.exceptions.RFXComException;
+import org.openhab.binding.rfxcom.internal.exceptions.RFXComInvalidStateException;
 import org.openhab.binding.rfxcom.internal.exceptions.RFXComUnsupportedChannelException;
 import org.openhab.binding.rfxcom.internal.exceptions.RFXComUnsupportedValueException;
 import org.openhab.binding.rfxcom.internal.handler.DeviceState;
@@ -174,11 +176,12 @@ public String getDeviceId() {
     }
 
     @Override
-    public State convertToState(String channelId, DeviceState deviceState) throws RFXComUnsupportedChannelException {
+    public State convertToState(String channelId, RFXComDeviceConfiguration config, DeviceState deviceState)
+            throws RFXComUnsupportedChannelException, RFXComInvalidStateException {
         if (CHANNEL_COMMAND.equals(channelId)) {
             return (command == Commands.CLOSE ? OpenClosedType.CLOSED : OpenClosedType.OPEN);
         } else {
-            return super.convertToState(channelId, deviceState);
+            return super.convertToState(channelId, config, deviceState);
         }
     }
 

bundles/org.openhab.binding.rfxcom/src/main/java/org/openhab/binding/rfxcom/internal/messages/RFXComChimeMessage.java

@@ -15,7 +15,9 @@
 import static org.openhab.binding.rfxcom.internal.RFXComBindingConstants.CHANNEL_CHIME_SOUND;
 import static org.openhab.binding.rfxcom.internal.messages.ByteEnumUtil.fromByte;
 
+import org.openhab.binding.rfxcom.internal.config.RFXComDeviceConfiguration;
 import org.openhab.binding.rfxcom.internal.exceptions.RFXComException;
+import org.openhab.binding.rfxcom.internal.exceptions.RFXComInvalidStateException;
 import org.openhab.binding.rfxcom.internal.exceptions.RFXComUnsupportedChannelException;
 import org.openhab.binding.rfxcom.internal.exceptions.RFXComUnsupportedValueException;
 import org.openhab.binding.rfxcom.internal.handler.DeviceState;
@@ -133,11 +135,12 @@ public String getDeviceId() {
     }
 
     @Override
-    public State convertToState(String channelId, DeviceState deviceState) throws RFXComUnsupportedChannelException {
+    public State convertToState(String channelId, RFXComDeviceConfiguration config, DeviceState deviceState)
+            throws RFXComUnsupportedChannelException, RFXComInvalidStateException {
         if (CHANNEL_CHIME_SOUND.equals(channelId)) {
             return new DecimalType(chimeSound);
         } else {
-            return super.convertToState(channelId, deviceState);
+            return super.convertToState(channelId, config, deviceState);
         }
     }