[sbus] testing and fixes

Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>

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

@@ -29,9 +29,11 @@ The SBUS Bridge requires the following configuration parameters:
 Example:
 
 ```
-Bridge sbus:bridge-udp:mybridge [ host="192.168.1.100", port=5000 ]
+Bridge sbus:udp:mybridge [ host="192.168.1.255", port=5000 ]
 ```
 
+Please note the broadcast address. This is how Sbus devices communicate with each other.
+
 ### Thing Configuration
 
 #### RGBW Controller
@@ -41,6 +43,7 @@ Thing sbus:rgbw:mybridge:light1 [ address=1 ]
 ```
 
 Supported channels:
+
 * `red` - Red component (0-100%)
 * `green` - Green component (0-100%)
 * `blue` - Blue component (0-100%)
@@ -49,24 +52,37 @@ Supported channels:
 #### Temperature Sensor
 
 ```
-Thing sbus:temperature:mybridge:temp1 [ address=2 ]
+Thing temperature temp1 [ id=62, refresh=30 ] {
+    Channels:
+        Type temperature-channel : temperature [ channelNumber=1 ]
+}
 ```
 
 Supported channels:
+
 * `temperature` - Current temperature reading
 
 #### Switch Controller
 
 ```
-Thing sbus:switch:mybridge:switch1 [ address=3 ]
+Thing switch switch1 [ id=75, refresh=30 ] {
+    Channels:
+        Type switch-channel : first_switch  [ channelNumber=1 ]
+        Type dimmer-channel : second_switch [ channelNumber=2 ]
+        Type paired-channel : third_switch [ channelNumber=3 ]
+}
 ```
 
 Supported channels:
+
 * `switch` - ON/OFF state
+* `dimmer` - ON/OFF state with timer transition
+* `paired` - ON/OFF state for two paired channels. This feature is used for curtains and other devices that require two actuator channels.
 
 ## Example Usage
 
 items/sbus.items:
+
 ```
 Color Light_RGB "RGB Light" { channel="sbus:rgbw:mybridge:light1:color" }
 Number:Temperature Temp_Sensor "Temperature [%.1f °C]" { channel="sbus:temperature:mybridge:temp1:temperature" }
@@ -74,6 +90,7 @@ Switch Light_Switch "Switch" { channel="sbus:switch:mybridge:switch1:switch" }
 ```
 
 sitemap/sbus.sitemap:
+
 ```
 sitemap sbus label="SBUS Demo"
 {
@@ -83,3 +100,30 @@ sitemap sbus label="SBUS Demo"
         Switch item=Light_Switch
     }
 }
+```
+
+## Special Case: RGBW Controller with Power Control
+
+Here's how to configure an RGBW controller with both color and power control:
+
+```
+// RGBW controller for color
+Thing rgbw colorctrl [ id=72, refresh=30 ] {
+    Channels:
+        Type color-channel : color [ channelNumber=1 ]   // HSB color picker, RGBW values stored at channel 1
+}
+
+// Switch for power control
+Thing switch powerctrl [ id=72, refresh=30 ] {
+    Channels:
+        Type switch-channel : power [ channelNumber=5, timer=-1 ]  // On/Off control for the RGBW output. Disable the timer functionality. The device doesn't support it.
+}
+
+// Light Group
+Group   gLight      "RGBW Light"    <light>     ["Lighting"]
+
+// Color Control
+Color   rgbwColor    "Color"        <colorwheel> (gLight)   ["Control", "Light"]    { channel="sbus:rgbw:mybridge:colorctrl:color" }
+
+// Power Control
+Switch  rgbwPower    "Power"        <switch>     (gLight)   ["Switch", "Light"]     { channel="sbus:switch:mybridge:powerctrl:power" }

bundles/org.openhab.binding.sbus/src/main/java/org/openhab/binding/sbus/BindingConstants.java

@@ -48,4 +48,5 @@ public class BindingConstants {
     public static final String CHANNEL_GREEN = "green";
     public static final String CHANNEL_BLUE = "blue";
     public static final String CHANNEL_WHITE = "white";
+    public static final String CHANNEL_COLOR = "color";
 }

bundles/org.openhab.binding.sbus/src/main/java/org/openhab/binding/sbus/handler/AbstractSbusHandler.java

@@ -127,7 +127,7 @@ public abstract class AbstractSbusHandler extends BaseThingHandler {
                 } catch (Exception e) {
                     logger.error("Error polling SBUS device", e);
                 }
-            }, 0, config.refresh, TimeUnit.MILLISECONDS);
+            }, 0, config.refresh, TimeUnit.SECONDS);
         }
     }
 

bundles/org.openhab.binding.sbus/src/main/java/org/openhab/binding/sbus/handler/SbusRgbwHandler.java

@@ -17,6 +17,7 @@ import static org.openhab.binding.sbus.BindingConstants.*;
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.openhab.binding.sbus.internal.config.SbusChannelConfig;
 import org.openhab.binding.sbus.internal.config.SbusDeviceConfig;
+import org.openhab.core.library.types.HSBType;
 import org.openhab.core.library.types.PercentType;
 import org.openhab.core.thing.Channel;
 import org.openhab.core.thing.ChannelUID;
@@ -24,6 +25,7 @@ import org.openhab.core.thing.Thing;
 import org.openhab.core.thing.ThingStatus;
 import org.openhab.core.thing.ThingStatusDetail;
 import org.openhab.core.types.Command;
+import org.openhab.core.util.ColorUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -44,24 +46,89 @@ public class SbusRgbwHandler extends AbstractSbusHandler {
         super(thing);
     }
 
+    /**
+     * Converts an openHAB HSBType into an RGBW array ([R, G, B, W]),
+     * with each channel in [0..255].
+     *
+     * We extract 'white' by taking the minimum of R, G, B and
+     * subtracting it from each color channel.
+     *
+     * @param hsbType the openHAB HSBType (hue [0..360], sat [0..100], bri [0..100])
+     * @return an int array [R, G, B, W] each in [0..255]
+     */
+    public static int[] hsbToRgbw(HSBType hsbType) {
+        if (hsbType == null) {
+            throw new IllegalArgumentException("HSBType cannot be null.");
+        }
+
+        // Convert HSBType to standard RGB [0..255]
+        PercentType[] rgb = ColorUtil.hsbToRgbPercent(hsbType);
+        // Convert each channel from 0..100 to 0..255
+        int r = (int) Math.round(rgb[0].floatValue() * 2.55);
+        int g = (int) Math.round(rgb[1].floatValue() * 2.55);
+        int b = (int) Math.round(rgb[2].floatValue() * 2.55);
+
+        // Determine the white component as the min of R, G, B
+        int w = Math.min(r, Math.min(g, b));
+
+        // Subtract W from each
+        r -= w;
+        g -= w;
+        b -= w;
+
+        return new int[] { r, g, b, w };
+    }
+
+    /**
+     * Converts an RGBW array ([R, G, B, W]) back to an openHAB HSBType.
+     *
+     * We add the W channel back into R, G, and B, then clamp to [0..255].
+     * Finally, we create an HSBType via fromRGB().
+     *
+     * @param rgbw an int array [R, G, B, W] each in [0..255]
+     * @return an HSBType (hue [0..360], saturation/brightness [0..100])
+     */
+    public static HSBType rgbwToHsb(int[] rgbw) {
+        if (rgbw == null || rgbw.length < 4) {
+            throw new IllegalArgumentException("rgbw must be non-null and have 4 elements: [R, G, B, W].");
+        }
+
+        int r = rgbw[0];
+        int g = rgbw[1];
+        int b = rgbw[2];
+        int w = rgbw[3];
+
+        // Restore the combined R, G, B
+        int rTotal = r + w;
+        int gTotal = g + w;
+        int bTotal = b + w;
+
+        // Clamp to [0..255]
+        rTotal = Math.min(255, Math.max(0, rTotal));
+        gTotal = Math.min(255, Math.max(0, gTotal));
+        bTotal = Math.min(255, Math.max(0, bTotal));
+
+        // Convert back to an HSBType via fromRGB
+        HSBType hsbType = HSBType.fromRGB(rTotal, gTotal, bTotal);
+
+        return hsbType;
+    }
+
     @Override
     protected void initializeChannels() {
-        // Get all channel configurations from the thing
+        // Validate all color channel configurations
         for (Channel channel : getThing().getChannels()) {
-            // Channels are already defined in thing-types.xml, just validate their configuration
+            if ("color-channel".equals(channel.getChannelTypeUID().getId())) {
                 SbusChannelConfig channelConfig = channel.getConfiguration().as(SbusChannelConfig.class);
                 if (channelConfig.channelNumber <= 0) {
                     logger.warn("Channel {} has invalid channel number configuration", channel.getUID());
                 }
             }
         }
+    }
 
     @Override
     protected void pollDevice() {
-        handleReadRgbwValues();
-    }
-
-    private void handleReadRgbwValues() {
         final SbusAdapter adapter = super.sbusAdapter;
         if (adapter == null) {
             logger.warn("SBUS adapter not initialized");
@@ -71,26 +138,27 @@ public class SbusRgbwHandler extends AbstractSbusHandler {
 
         try {
             SbusDeviceConfig config = getConfigAs(SbusDeviceConfig.class);
-            int[] rgbwValues = adapter.readRgbw(config.subnetId, config.id);
+
-            if (rgbwValues != null && rgbwValues.length >= 4) {
+            // Update all color channels
-                // Update each channel based on its ID
             for (Channel channel : getThing().getChannels()) {
-                    String channelId = channel.getUID().getId();
+                if ("color-channel".equals(channel.getChannelTypeUID().getId())) {
-                    if (CHANNEL_RED.equals(channelId)) {
+                    SbusChannelConfig channelConfig = channel.getConfiguration().as(SbusChannelConfig.class);
-                        updateState(channel.getUID(), new PercentType(rgbwValues[0]));
+
-                    } else if (CHANNEL_GREEN.equals(channelId)) {
+                    // Read RGBW values for this channel
-                        updateState(channel.getUID(), new PercentType(rgbwValues[1]));
+                    int[] rgbwValues = adapter.readRgbw(config.subnetId, config.id, channelConfig.channelNumber);
-                    } else if (CHANNEL_BLUE.equals(channelId)) {
+                    if (rgbwValues != null && rgbwValues.length >= 4) {
-                        updateState(channel.getUID(), new PercentType(rgbwValues[2]));
+                        // Convert RGBW to HSB using our custom conversion
-                    } else if (CHANNEL_WHITE.equals(channelId)) {
+                        HSBType hsbType = rgbwToHsb(rgbwValues);
-                        updateState(channel.getUID(), new PercentType(rgbwValues[3]));
+
+                        updateState(channel.getUID(), hsbType);
                     }
                 }
-            } else {
-                logger.warn("Invalid RGBW values received from SBUS device");
             }
+
+            updateStatus(ThingStatus.ONLINE);
         } catch (Exception e) {
-            logger.error("Error reading RGBW values", e);
+            logger.error("Error reading device state", e);
+            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Error reading device state");
         }
     }
 
@@ -104,39 +172,25 @@ public class SbusRgbwHandler extends AbstractSbusHandler {
         }
 
         try {
-            String channelId = channelUID.getId();
+            Channel channel = getThing().getChannel(channelUID.getId());
-            if (command instanceof PercentType) {
+            if (channel != null && "color-channel".equals(channel.getChannelTypeUID().getId())
-                int value = ((PercentType) command).intValue();
+                    && command instanceof HSBType hsbCommand) {
                 SbusDeviceConfig config = getConfigAs(SbusDeviceConfig.class);
-                int[] currentValues = adapter.readRgbw(config.subnetId, config.id);
-                if (currentValues == null || currentValues.length < 4) {
-                    logger.warn("Failed to read current RGBW values");
-                    return;
-                }
-
-                int[] newValues = currentValues.clone();
-                if (CHANNEL_RED.equals(channelId)) {
-                    newValues[0] = value;
-                } else if (CHANNEL_GREEN.equals(channelId)) {
-                    newValues[1] = value;
-                } else if (CHANNEL_BLUE.equals(channelId)) {
-                    newValues[2] = value;
-                } else if (CHANNEL_WHITE.equals(channelId)) {
-                    newValues[3] = value;
-                }
-
-                // Update each channel's state
-                Channel channel = getThing().getChannel(channelId);
-                if (channel != null) {
                 SbusChannelConfig channelConfig = channel.getConfiguration().as(SbusChannelConfig.class);
-                    if (channelConfig.channelNumber > 0) {
+
-                        adapter.writeSingleChannel(config.subnetId, config.id, channelConfig.channelNumber, value > 0);
+                // Convert HSB to RGBW
-                        updateState(channelUID, (PercentType) command);
+                int[] rgbw = hsbToRgbw(hsbCommand);
-                    }
+
-                }
+                // Write all RGBW values at once using the dedicated method
+                adapter.writeRgbw(config.subnetId, config.id, channelConfig.channelNumber, rgbw[0], rgbw[1], rgbw[2],
+                        rgbw[3]);
+
+                // Update state
+                updateState(channelUID, hsbCommand);
             }
         } catch (Exception e) {
             logger.error("Error handling command", e);
+            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Error sending command to device");
         }
     }
 }

bundles/org.openhab.binding.sbus/src/main/java/org/openhab/binding/sbus/handler/SbusSwitchHandler.java

@@ -16,13 +16,14 @@ import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.openhab.binding.sbus.internal.config.SbusChannelConfig;
 import org.openhab.binding.sbus.internal.config.SbusDeviceConfig;
 import org.openhab.core.library.types.OnOffType;
+import org.openhab.core.library.types.OpenClosedType;
+import org.openhab.core.library.types.PercentType;
 import org.openhab.core.thing.Channel;
 import org.openhab.core.thing.ChannelUID;
 import org.openhab.core.thing.Thing;
 import org.openhab.core.thing.ThingStatus;
 import org.openhab.core.thing.ThingStatusDetail;
 import org.openhab.core.types.Command;
-import org.openhab.core.types.State;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -70,7 +71,7 @@ public class SbusSwitchHandler extends AbstractSbusHandler {
 
         try {
             SbusDeviceConfig config = getConfigAs(SbusDeviceConfig.class);
-            boolean[] statuses = adapter.readStatusChannels(config.subnetId, config.id);
+            int[] statuses = adapter.readStatusChannels(config.subnetId, config.id);
             if (statuses == null) {
                 logger.warn("Received null status channels from SBUS device");
                 return;
@@ -80,8 +81,17 @@ public class SbusSwitchHandler extends AbstractSbusHandler {
             for (Channel channel : getThing().getChannels()) {
                 SbusChannelConfig channelConfig = channel.getConfiguration().as(SbusChannelConfig.class);
                 if (channelConfig.channelNumber > 0 && channelConfig.channelNumber <= statuses.length) {
-                    State state = statuses[channelConfig.channelNumber - 1] ? OnOffType.ON : OnOffType.OFF;
+                    String channelTypeId = channel.getChannelTypeUID().getId();
-                    updateState(channel.getUID(), state);
+                    boolean isActive = statuses[channelConfig.channelNumber - 1] == 0x64; // 100 when on, something else
+                                                                                          // when off
+
+                    if ("switch-channel".equals(channelTypeId)) {
+                        updateState(channel.getUID(), isActive ? OnOffType.ON : OnOffType.OFF);
+                    } else if ("dimmer-channel".equals(channelTypeId)) {
+                        updateState(channel.getUID(), new PercentType(statuses[channelConfig.channelNumber - 1]));
+                    } else if ("paired-channel".equals(channelTypeId)) {
+                        updateState(channel.getUID(), isActive ? OpenClosedType.OPEN : OpenClosedType.CLOSED);
+                    }
                 }
             }
         } catch (Exception e) {
@@ -102,15 +112,62 @@ public class SbusSwitchHandler extends AbstractSbusHandler {
             Channel channel = getThing().getChannel(channelUID);
             if (channel != null) {
                 SbusChannelConfig channelConfig = channel.getConfiguration().as(SbusChannelConfig.class);
-                if (channelConfig.channelNumber > 0) {
+                if (channelConfig.channelNumber <= 0) {
-                    boolean isOn = command.equals(OnOffType.ON);
+                    logger.warn("Invalid channel number for {}", channelUID);
+                    return;
+                }
+
                 SbusDeviceConfig config = getConfigAs(SbusDeviceConfig.class);
-                    adapter.writeSingleChannel(config.subnetId, config.id, channelConfig.channelNumber, isOn);
+
-                    updateState(channelUID, isOn ? OnOffType.ON : OnOffType.OFF);
+                if (command instanceof OnOffType) {
+                    handleOnOffCommand((OnOffType) command, config, channelConfig, channelUID, adapter);
+                } else if (command instanceof PercentType) {
+                    handlePercentCommand((PercentType) command, config, channelConfig, channelUID, adapter);
+                } else if (command instanceof OpenClosedType) {
+                    handleOpenClosedCommand((OpenClosedType) command, config, channelConfig, channelUID, adapter);
                 }
             }
         } catch (Exception e) {
             logger.error("Error handling command", e);
         }
     }
+
+    private void handleOnOffCommand(OnOffType command, SbusDeviceConfig config, SbusChannelConfig channelConfig,
+            ChannelUID channelUID, SbusAdapter adapter) throws Exception {
+        boolean isOn = command == OnOffType.ON;
+        adapter.writeSingleChannel(config.subnetId, config.id, channelConfig.channelNumber, isOn ? 100 : 0,
+                channelConfig.timer);
+        updateState(channelUID, isOn ? OnOffType.ON : OnOffType.OFF);
+    }
+
+    private void handlePercentCommand(PercentType command, SbusDeviceConfig config, SbusChannelConfig channelConfig,
+            ChannelUID channelUID, SbusAdapter adapter) throws Exception {
+        adapter.writeSingleChannel(config.subnetId, config.id, channelConfig.channelNumber, command.intValue(),
+                channelConfig.timer);
+        updateState(channelUID, command);
+    }
+
+    private void handleOpenClosedCommand(OpenClosedType command, SbusDeviceConfig config,
+            SbusChannelConfig channelConfig, ChannelUID channelUID, SbusAdapter adapter) throws Exception {
+        boolean isOpen = command == OpenClosedType.OPEN;
+        // Set main channel
+        if (getChannelToClose(channelConfig, isOpen) > 0) {
+            adapter.writeSingleChannel(config.subnetId, config.id, getChannelToClose(channelConfig, isOpen), 0,
+                    channelConfig.timer);
+        }
+        // Set paired channel to opposite state if configured
+        if (getChannelToOpen(channelConfig, isOpen) > 0) {
+            adapter.writeSingleChannel(config.subnetId, config.id, getChannelToOpen(channelConfig, isOpen), 0x64,
+                    channelConfig.timer);
+        }
+        updateState(channelUID, isOpen ? OpenClosedType.OPEN : OpenClosedType.CLOSED);
+    }
+
+    private int getChannelToOpen(SbusChannelConfig channelConfig, boolean state) {
+        return state ? channelConfig.channelNumber : channelConfig.pairedChannelNumber;
+    }
+
+    private int getChannelToClose(SbusChannelConfig channelConfig, boolean state) {
+        return state ? channelConfig.pairedChannelNumber : channelConfig.channelNumber;
+    }
 }

bundles/org.openhab.binding.sbus/src/main/java/org/openhab/binding/sbus/internal/config/SbusChannelConfig.java

@@ -22,7 +22,18 @@ import org.eclipse.jdt.annotation.NonNullByDefault;
 @NonNullByDefault
 public class SbusChannelConfig {
     /**
-     * The physical channel number on the SBUS device
+     * The physical channel number on the SBUS device.
      */
     public int channelNumber;
+
+    /**
+     * The paired channel number for OpenClosedType channels.
+     * When the main channel is opened, this channel will be closed and vice versa.
+     */
+    public int pairedChannelNumber;
+
+    /**
+     * Timer in seconds to automatically turn off the switch (-1 = disabled).
+     */
+    public int timer = 0;
 }

bundles/org.openhab.binding.sbus/src/main/java/org/openhab/binding/sbus/internal/config/SbusDeviceConfig.java

@@ -34,7 +34,7 @@ public class SbusDeviceConfig {
     public int subnetId = Sbus.DEFAULT_SUBNET_ID;
 
     /**
-     * Refresh interval in milliseconds
+     * Refresh interval in seconds
      */
-    public int refresh = 30000; // Default value from thing-types.xml
+    public int refresh = 30; // Default value from thing-types.xml
 }

bundles/org.openhab.binding.sbus/src/main/resources/OH-INF/thing/bridge-udp.xml

@@ -19,11 +19,6 @@
 				<default>6000</default>
 			</parameter>
 
-			<parameter name="refresh" type="integer" required="false" min="1">
-				<label>Refresh Interval</label>
-				<description>Refresh interval in seconds</description>
-				<default>30</default>
-			</parameter>
 			<parameter name="enableDiscovery" type="boolean">
 				<label>Discovery Enabled</label>
 				<description>When enabled we try to find a device specific handler. Turn this on if you're using one of the

bundles/org.openhab.binding.sbus/src/main/resources/OH-INF/thing/thing-types.xml

@@ -24,8 +24,8 @@
 			</parameter>
 			<parameter name="refresh" type="integer">
 				<label>Refresh Interval</label>
-				<description>Refresh interval in milliseconds</description>
+				<description>Refresh interval in seconds</description>
-				<default>30000</default>
+				<default>30</default>
 			</parameter>
 		</config-description>
 	</thing-type>
@@ -49,8 +49,8 @@
 			</parameter>
 			<parameter name="refresh" type="integer">
 				<label>Refresh Interval</label>
-				<description>Refresh interval in milliseconds</description>
+				<description>Refresh interval in seconds</description>
-				<default>30000</default>
+				<default>30</default>
 			</parameter>
 		</config-description>
 	</thing-type>
@@ -74,8 +74,8 @@
 			</parameter>
 			<parameter name="refresh" type="integer">
 				<label>Refresh Interval</label>
-				<description>Refresh interval in milliseconds</description>
+				<description>Refresh interval in seconds</description>
-				<default>30000</default>
+				<default>30</default>
 			</parameter>
 		</config-description>
 	</thing-type>
@@ -94,6 +94,42 @@
 		</config-description>
 	</channel-type>
 
+	<channel-type id="dimmer-channel">
+		<item-type>Dimmer</item-type>
+		<label>Dimmer State</label>
+		<description>Dimmer state (0-100%)</description>
+		<category>DimmableLight</category>
+		<config-description>
+			<parameter name="channelNumber" type="integer" required="true">
+				<label>Channel Number</label>
+				<description>The physical channel number on the SBUS device</description>
+			</parameter>
+			<parameter name="timer" type="integer">
+				<label>Timer</label>
+				<description>Timer in seconds to automatically turn off the switch (0 = disabled)</description>
+				<default>0</default>
+				<min>0</min>
+			</parameter>
+		</config-description>
+	</channel-type>
+
+	<channel-type id="paired-channel">
+		<item-type>Contact</item-type>
+		<label>Paired Channel State</label>
+		<description>Paired channel state (OPEN/CLOSED) - controls two opposite channels</description>
+		<category>Contact</category>
+		<config-description>
+			<parameter name="channelNumber" type="integer" required="true">
+				<label>Channel Number</label>
+				<description>The physical channel number on the SBUS device</description>
+			</parameter>
+			<parameter name="pairedChannelNumber" type="integer" required="true">
+				<label>Paired Channel Number</label>
+				<description>The physical channel number of the paired channel (will be set to opposite state)</description>
+			</parameter>
+		</config-description>
+	</channel-type>
+
 	<channel-type id="temperature-channel">
 		<item-type>Number:Temperature</item-type>
 		<label>Temperature</label>
@@ -109,11 +145,10 @@
 	</channel-type>
 
 	<channel-type id="color-channel">
-		<item-type>Dimmer</item-type>
+		<item-type>Color</item-type>
-		<label>Color Channel</label>
+		<label>Color</label>
-		<description>Color intensity (0-100%)</description>
+		<description>Color control</description>
 		<category>ColorLight</category>
-		<state min="0" max="100" step="1" pattern="%d %%"/>
 	</channel-type>
 
 	<!-- Channel Group Types (unused, but left in case you need them) -->