[velux] Vane position channel is visible when required (openhab#13271)

* [velux] make vanePosition channel dynamic * [velux] change category of window position from 'blinds' to 'window' Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
nemerdaud · Feb 28, 2023 · 5a928dc · 5a928dc
1 parent a36b0f2
commit 5a928dc
Show file tree

Hide file tree

Showing 7 changed files with 69 additions and 10 deletions.
diff --git a/bundles/org.openhab.binding.velux/README.md b/bundles/org.openhab.binding.velux/README.md
@@ -176,11 +176,12 @@ The supported Channels and their associated channel types are shown below.
 | position     | Rollershutter | Actual position of the window or device.        |
 | limitMinimum | Rollershutter | Minimum limit position of the window or device. |
 | limitMaximum | Rollershutter | Maximum limit position of the window or device. |
-| vanePosition | Dimmer        | Vane position of a Venetian blind.              |
+| vanePosition | Dimmer        | Vane position of a Venetian blind. (optional)   |
 
 The `position`, `limitMinimum`, and `limitMaximum` are the same as described above for "window" Things.
 
 The `vanePosition` Channel only applies to Venetian blinds that have tiltable slats.
+The binding detects whether the device supports a vane position, and if so, it adds the `vanePosition` Channel automatically.
 
 ### Channels for "actuator" Things
 

diff --git a/...ng.velux/src/main/java/org/openhab/binding/velux/internal/handler/VeluxBridgeHandler.java b/...ng.velux/src/main/java/org/openhab/binding/velux/internal/handler/VeluxBridgeHandler.java
@@ -69,6 +69,7 @@
 import org.openhab.core.thing.ThingStatus;
 import org.openhab.core.thing.ThingStatusDetail;
 import org.openhab.core.thing.ThingTypeUID;
+import org.openhab.core.thing.binding.ThingHandler;
 import org.openhab.core.thing.binding.ThingHandlerService;
 import org.openhab.core.types.Command;
 import org.openhab.core.types.RefreshType;
@@ -465,6 +466,8 @@ private void bridgeParamsUpdated() {
             }
         }
 
+        updateDynamicChannels();
+
         veluxBridgeConfiguration.hasChanged = false;
         logger.debug("Velux veluxBridge is online, now.");
         updateStatus(ThingStatus.ONLINE);
@@ -957,4 +960,16 @@ public ProductBridgeIndex getProductBridgeIndex(String thingName) {
         }
         return ProductBridgeIndex.UNKNOWN;
     }
+
+    /**
+     * Ask all things in the hub to initialise their dynamic vane position channel if they support it.
+     */
+    private void updateDynamicChannels() {
+        getThing().getThings().stream().forEach(thing -> {
+            ThingHandler thingHandler = thing.getHandler();
+            if (thingHandler instanceof VeluxHandler) {
+                ((VeluxHandler) thingHandler).updateDynamicChannels(this);
+            }
+        });
+    }
 }
diff --git a/....binding.velux/src/main/java/org/openhab/binding/velux/internal/handler/VeluxHandler.java b/....binding.velux/src/main/java/org/openhab/binding/velux/internal/handler/VeluxHandler.java
@@ -16,11 +16,14 @@
 import java.util.Map.Entry;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
-import org.openhab.binding.velux.internal.config.VeluxThingConfiguration;
+import org.openhab.binding.velux.internal.VeluxBindingConstants;
 import org.openhab.binding.velux.internal.handler.utils.ExtendedBaseThingHandler;
+import org.openhab.binding.velux.internal.handler.utils.Thing2VeluxActuator;
+import org.openhab.binding.velux.internal.things.VeluxProduct;
 import org.openhab.binding.velux.internal.utils.Localization;
 import org.openhab.core.config.core.Configuration;
 import org.openhab.core.thing.Bridge;
+import org.openhab.core.thing.Channel;
 import org.openhab.core.thing.ChannelUID;
 import org.openhab.core.thing.Thing;
 import org.openhab.core.thing.ThingStatus;
@@ -41,8 +44,6 @@
 public class VeluxHandler extends ExtendedBaseThingHandler {
     private final Logger logger = LoggerFactory.getLogger(VeluxHandler.class);
 
-    VeluxThingConfiguration configuration = new VeluxThingConfiguration();
-
     public VeluxHandler(Thing thing, Localization localization) {
         super(thing);
         logger.trace("VeluxHandler(thing={},localization={}) constructor called.", thing, localization);
@@ -70,7 +71,6 @@ public void initialize() {
     }
 
     private synchronized void initializeProperties() {
-        configuration = getConfigAs(VeluxThingConfiguration.class);
         logger.trace("initializeProperties() done.");
     }
 
@@ -125,4 +125,37 @@ public void handleConfigurationUpdate(Map<String, Object> configurationParameter
             super.handleConfigurationUpdate(configurationParameters);
         }
     }
+
+    /**
+     * Remove previously statically created vane channel if the device does not support it. Or log a warning if it does
+     * support a vane and the respective channel is missing.
+     *
+     * @param bridgeHandler the calling bridge handler.
+     * @throws IllegalStateException if something went wrong.
+     */
+    public void updateDynamicChannels(VeluxBridgeHandler bridgeHandler) throws IllegalStateException {
+        // roller shutters are the only things with a previously statically created vane channel
+        if (!VeluxBindingConstants.THING_TYPE_VELUX_ROLLERSHUTTER.equals(thing.getThingTypeUID())) {
+            return;
+        }
+
+        String id = VeluxBindingConstants.CHANNEL_VANE_POSITION;
+        ChannelUID uid = new ChannelUID(thing.getUID(), id);
+        Thing2VeluxActuator actuator = new Thing2VeluxActuator(bridgeHandler, uid);
+        VeluxProduct product = bridgeHandler.existingProducts().get(actuator.getProductBridgeIndex());
+
+        if (product.equals(VeluxProduct.UNKNOWN)) {
+            throw new IllegalStateException("updateDynamicChannels(): Product unknown in the bridge");
+        }
+
+        Channel channel = thing.getChannel(id);
+        boolean required = product.supportsVanePosition();
+
+        if (!required && channel != null) {
+            logger.debug("Removing unsupported channel for {}: {}", thing.getUID(), id);
+            updateThing(editThing().withoutChannels(channel).build());
+        } else if (required && channel == null) {
+            logger.warn("Thing {} does not have a '{}' channel => please re-create it", thing.getUID(), id);
+        }
+    }
 }
diff --git a/bundles/org.openhab.binding.velux/src/main/resources/OH-INF/i18n/velux.properties b/bundles/org.openhab.binding.velux/src/main/resources/OH-INF/i18n/velux.properties
@@ -133,6 +133,8 @@ channel-type.velux.scenes.description = Scenes which are configured on the Bridg
 channel-type.velux.check.label = Check of configuration
 channel-type.velux.check.description = Result of the check of current item configuration.
 #
+channel-type.velux.windowPosition.label = Position
+channel-type.velux.windowPosition.description = Device control (UP, DOWN, STOP, closure 0-100%).
 channel-type.velux.position.label = Position
 channel-type.velux.position.description = Device control (UP, DOWN, STOP, closure 0-100%).
 channel-type.velux.vanePosition.label = Vane Position

diff --git a/bundles/org.openhab.binding.velux/src/main/resources/OH-INF/thing/channels.xml b/bundles/org.openhab.binding.velux/src/main/resources/OH-INF/thing/channels.xml
@@ -132,18 +132,26 @@
 
 	<!-- Channel Type - Generic Things -->
 
+	<channel-type id="windowPosition">
+		<item-type>Rollershutter</item-type>
+		<label>@text/channel-type.velux.windowPosition.label</label>
+		<description>@text/channel-type.velux.windowPosition.description</description>
+		<category>Window</category>
+		<state min="0" max="100"/>
+	</channel-type>
+
 	<channel-type id="position">
 		<item-type>Rollershutter</item-type>
 		<label>@text/channel-type.velux.position.label</label>
 		<description>@text/channel-type.velux.position.description</description>
 		<category>Blinds</category>
+		<state min="0" max="100"/>
 	</channel-type>
 
 	<channel-type id="vanePosition">
 		<item-type>Dimmer</item-type>
 		<label>@text/channel-type.velux.vanePosition.label</label>
 		<description>@text/channel-type.velux.vanePosition.description</description>
-		<category>Blinds</category>
 		<state min="0" max="100"/>
 	</channel-type>
 
@@ -158,14 +166,14 @@
 		<item-type>Rollershutter</item-type>
 		<label>@text/channel-type.velux.limitMinimum.label</label>
 		<description>@text/channel-type.velux.limitMinimum.description</description>
-		<category>Blinds</category>
+		<state min="0" max="100"/>
 	</channel-type>
 
 	<channel-type id="limitMaximum" advanced="true">
 		<item-type>Rollershutter</item-type>
 		<label>@text/channel-type.velux.limitMaximum.label</label>
 		<description>@text/channel-type.velux.limitMaximum.description</description>
-		<category>Blinds</category>
+		<state min="0" max="100"/>
 	</channel-type>
 
 	<channel-type id="action">

diff --git a/bundles/org.openhab.binding.velux/src/main/resources/OH-INF/thing/rollershutter.xml b/bundles/org.openhab.binding.velux/src/main/resources/OH-INF/thing/rollershutter.xml
@@ -17,9 +17,9 @@
 		<category>Blinds</category>
 		<channels>
 			<channel id="position" typeId="position"/>
+			<channel id="vanePosition" typeId="vanePosition"/>
 			<channel id="limitMinimum" typeId="limitMinimum"/>
 			<channel id="limitMaximum" typeId="limitMaximum"/>
-			<channel id="vanePosition" typeId="vanePosition"/>
 		</channels>
 		<representation-property>serial</representation-property>
 		<config-description-ref uri="thing-type:velux:rollershutter"/>

diff --git a/bundles/org.openhab.binding.velux/src/main/resources/OH-INF/thing/window.xml b/bundles/org.openhab.binding.velux/src/main/resources/OH-INF/thing/window.xml
@@ -16,7 +16,7 @@
 		<description>@text/thing-type.velux.window.description</description>
 		<category>Window</category>
 		<channels>
-			<channel id="position" typeId="position"></channel>
+			<channel id="position" typeId="windowPosition"></channel>
 			<channel id="limitMinimum" typeId="limitMinimum"/>
 			<channel id="limitMaximum" typeId="limitMaximum"/>
 		</channels>