From 27886d7234d879f8bc133ae39848f2ebbe828457 Mon Sep 17 00:00:00 2001
From: Oleg Vivtash <oleg@vivtash.net>
Date: Sun, 17 Oct 2021 14:34:47 +0300
Subject: [PATCH] [mikrotik] WiFi client logging and presence problem fix
 (#11386)

* [mikrotik] WiFi client fix

Signed-off-by: Oleg Vivtash <oleg@vivtash.net>

* [mikrotik] Thing types and WiFi client presence update

Signed-off-by: Oleg Vivtash <oleg@vivtash.net>
---
 .../org.openhab.binding.mikrotik/README.md    | 24 +++++++++---------
 .../MikrotikInterfaceThingHandler.java        |  4 +--
 .../MikrotikWirelessClientThingHandler.java   | 25 ++++++++-----------
 .../mikrotik/internal/util/StateUtil.java     | 10 +++++++-
 .../resources/OH-INF/thing/thing-types.xml    |  4 +--
 5 files changed, 35 insertions(+), 32 deletions(-)

diff --git a/bundles/org.openhab.binding.mikrotik/README.md b/bundles/org.openhab.binding.mikrotik/README.md
index 43d74e606..c45dbb678 100644
--- a/bundles/org.openhab.binding.mikrotik/README.md
+++ b/bundles/org.openhab.binding.mikrotik/README.md
@@ -78,8 +78,8 @@ The WiFi client thing configuration parameters are:
 |---|---|---|---|
 | macAddress | String | MAC address of the client or interface |  |
 | comment | String | User-defined comment |  |
-| connected | Switch | Reflects connected or disconnected state |  |
-| continuous | Switch | Connection is considered long-running |  |
+| connected | Contact | Reflects connected or disconnected state |  |
+| continuous | Contact | Connection is considered long-running |  |
 | ssid | String | Wireless Network (SSID) the wireless client is connected to |  |
 | interface | String | Network interface name |  |
 | signal | system.signal-strength | Signal strength (RSSI) |  |
@@ -132,7 +132,7 @@ Common for all kinds of interfaces:
 | comment | String | User-defined comment |  |
 | macAddress | String | MAC address of the client or interface |  |
 | enabled | Switch | Reflects enabled or disabled state |  |
-| connected | Switch | Reflects connected or disconnected state |  |
+| connected | Contact | Reflects connected or disconnected state |  |
 | lastLinkDownTime | DateTime | Last time when link went down |  |
 | lastLinkUpTime | DateTime | Last time when link went up |  |
 | linkDowns | Number | Amount of link downs |  |
@@ -193,7 +193,7 @@ String     Eth_1_Name                  "Name"                       (gRB1Eth1) {
 String     Eth_1_Comment               "Comment"                    (gRB1Eth1) {channel="mikrotik:interface:rb1:eth1:comment"}
 String     Eth_1_Mac_Address           "Mac address"                (gRB1Eth1) {channel="mikrotik:interface:rb1:eth1:macAddress"}
 Switch     Eth_1_Enabled               "Enabled"                    (gRB1Eth1) {channel="mikrotik:interface:rb1:eth1:enabled"}
-Switch     Eth_1_Connected             "Connected"                  (gRB1Eth1) {channel="mikrotik:interface:rb1:eth1:connected"}
+Contact     Eth_1_Connected             "Connected"                  (gRB1Eth1) {channel="mikrotik:interface:rb1:eth1:connected"}
 DateTime   Eth_1_Last_Link_Down_Time   "Last link down"             (gRB1Eth1) {channel="mikrotik:interface:rb1:eth1:lastLinkDownTime"}
 DateTime   Eth_1_Last_Link_Up_Time     "Last link up"               (gRB1Eth1) {channel="mikrotik:interface:rb1:eth1:lastLinkUpTime"}
 Number     Eth_1_Link_Downs            "Link downs"                 (gRB1Eth1) {channel="mikrotik:interface:rb1:eth1:linkDowns"}
@@ -220,7 +220,7 @@ String     Eth_2_Name                  "Name"                       (gRB1Eth2) {
 String     Eth_2_Comment               "Comment"                    (gRB1Eth2) {channel="mikrotik:interface:rb1:eth2:comment"}
 String     Eth_2_Mac_Address           "Mac address"                (gRB1Eth2) {channel="mikrotik:interface:rb1:eth2:macAddress"}
 Switch     Eth_2_Enabled               "Enabled"                    (gRB1Eth2) {channel="mikrotik:interface:rb1:eth2:enabled"}
-Switch     Eth_2_Connected             "Connected"                  (gRB1Eth2) {channel="mikrotik:interface:rb1:eth2:connected"}
+Contact     Eth_2_Connected             "Connected"                  (gRB1Eth2) {channel="mikrotik:interface:rb1:eth2:connected"}
 DateTime   Eth_2_Last_Link_Down_Time   "Last link down"             (gRB1Eth2) {channel="mikrotik:interface:rb1:eth2:lastLinkDownTime"}
 DateTime   Eth_2_Last_Link_Up_Time     "Last link up"               (gRB1Eth2) {channel="mikrotik:interface:rb1:eth2:lastLinkUpTime"}
 Number     Eth_2_Link_Downs            "Link downs"                 (gRB1Eth2) {channel="mikrotik:interface:rb1:eth2:linkDowns"}
@@ -247,7 +247,7 @@ String     Cap_1_Name                  "Name"                       (gRB1Cap1) {
 String     Cap_1_Comment               "Comment"                    (gRB1Cap1) {channel="mikrotik:interface:rb1:cap1:comment"}
 String     Cap_1_Mac_Address           "Mac address"                (gRB1Cap1) {channel="mikrotik:interface:rb1:cap1:macAddress"}
 Switch     Cap_1_Enabled               "Enabled"                    (gRB1Cap1) {channel="mikrotik:interface:rb1:cap1:enabled"}
-Switch     Cap_1_Connected             "Connected"                  (gRB1Cap1) {channel="mikrotik:interface:rb1:cap1:connected"}
+Contact     Cap_1_Connected             "Connected"                  (gRB1Cap1) {channel="mikrotik:interface:rb1:cap1:connected"}
 DateTime   Cap_1_Last_Link_Down_Time   "Last link down"             (gRB1Cap1) {channel="mikrotik:interface:rb1:cap1:lastLinkDownTime"}
 DateTime   Cap_1_Last_Link_Up_Time     "Last link up"               (gRB1Cap1) {channel="mikrotik:interface:rb1:cap1:lastLinkUpTime"}
 Number     Cap_1_Link_Downs            "Link downs"                 (gRB1Cap1) {channel="mikrotik:interface:rb1:cap1:linkDowns"}
@@ -274,7 +274,7 @@ String     PP_Po_E_1_Name                  "Name"                       (gRB1Ppp
 String     PP_Po_E_1_Comment               "Comment"                    (gRB1Ppp1) {channel="mikrotik:interface:rb1:ppp1:comment"}
 String     PP_Po_E_1_Mac_Address           "Mac address"                (gRB1Ppp1) {channel="mikrotik:interface:rb1:ppp1:macAddress"}
 Switch     PP_Po_E_1_Enabled               "Enabled"                    (gRB1Ppp1) {channel="mikrotik:interface:rb1:ppp1:enabled"}
-Switch     PP_Po_E_1_Connected             "Connected"                  (gRB1Ppp1) {channel="mikrotik:interface:rb1:ppp1:connected"}
+Contact     PP_Po_E_1_Connected             "Connected"                  (gRB1Ppp1) {channel="mikrotik:interface:rb1:ppp1:connected"}
 DateTime   PP_Po_E_1_Last_Link_Down_Time   "Last link down"             (gRB1Ppp1) {channel="mikrotik:interface:rb1:ppp1:lastLinkDownTime"}
 DateTime   PP_Po_E_1_Last_Link_Up_Time     "Last link up"               (gRB1Ppp1) {channel="mikrotik:interface:rb1:ppp1:lastLinkUpTime"}
 Number     PP_Po_E_1_Link_Downs            "Link downs"                 (gRB1Ppp1) {channel="mikrotik:interface:rb1:ppp1:linkDowns"}
@@ -299,7 +299,7 @@ String     L_2_TP_Srv_1_Name                  "Name"                       (gRB1
 String     L_2_TP_Srv_1_Comment               "Comment"                    (gRB1Tun1) {channel="mikrotik:interface:rb1:tun1:comment"}
 String     L_2_TP_Srv_1_Mac_Address           "Mac address"                (gRB1Tun1) {channel="mikrotik:interface:rb1:tun1:macAddress"}
 Switch     L_2_TP_Srv_1_Enabled               "Enabled"                    (gRB1Tun1) {channel="mikrotik:interface:rb1:tun1:enabled"}
-Switch     L_2_TP_Srv_1_Connected             "Connected"                  (gRB1Tun1) {channel="mikrotik:interface:rb1:tun1:connected"}
+Contact     L_2_TP_Srv_1_Connected             "Connected"                  (gRB1Tun1) {channel="mikrotik:interface:rb1:tun1:connected"}
 DateTime   L_2_TP_Srv_1_Last_Link_Down_Time   "Last link down"             (gRB1Tun1) {channel="mikrotik:interface:rb1:tun1:lastLinkDownTime"}
 DateTime   L_2_TP_Srv_1_Last_Link_Up_Time     "Last link up"               (gRB1Tun1) {channel="mikrotik:interface:rb1:tun1:lastLinkUpTime"}
 Number     L_2_TP_Srv_1_Link_Downs            "Link downs"                 (gRB1Tun1) {channel="mikrotik:interface:rb1:tun1:linkDowns"}
@@ -319,8 +319,8 @@ Number     L_2_TP_Srv_1_Rx_Errors             "Receiving errors"           (gRB1
 Group gRB1Wifi1 "WiFi Client 1"
 String     Phone_1_Mac_Address      "Mac address"                          (gRB1Wifi1) {channel="mikrotik:wifiRegistration:rb1:wifi1:macAddress"}
 String     Phone_1_Comment          "Comment"                              (gRB1Wifi1) {channel="mikrotik:wifiRegistration:rb1:wifi1:comment"}
-Switch     Phone_1_Connected        "Connected"                            (gRB1Wifi1) {channel="mikrotik:wifiRegistration:rb1:wifi1:connected"}
-Switch     Phone_1_Continuous       "Continuous"                           (gRB1Wifi1) {channel="mikrotik:wifiRegistration:rb1:wifi1:continuous"}
+Contact     Phone_1_Connected        "Connected"                            (gRB1Wifi1) {channel="mikrotik:wifiRegistration:rb1:wifi1:connected"}
+Contact     Phone_1_Continuous       "Continuous"                           (gRB1Wifi1) {channel="mikrotik:wifiRegistration:rb1:wifi1:continuous"}
 String     Phone_1_Ssid             "Wi fi network"                        (gRB1Wifi1) {channel="mikrotik:wifiRegistration:rb1:wifi1:ssid"}
 String     Phone_1_Interface        "Name"                                 (gRB1Wifi1) {channel="mikrotik:wifiRegistration:rb1:wifi1:interface"}
 Number     Phone_1_Signal           "Received signal strength indicator"   (gRB1Wifi1) {channel="mikrotik:wifiRegistration:rb1:wifi1:signal"}
@@ -338,8 +338,8 @@ Number     Phone_1_Rx_Packets       "Received packets"                     (gRB1
 Group gRB1Wifi2 "WiFi Client 2"
 String     Tablet_2_Mac_Address      "Mac address"                          (gRB1Wifi2) {channel="mikrotik:wifiRegistration:rb1:wifi2:macAddress"}
 String     Tablet_2_Comment          "Comment"                              (gRB1Wifi2) {channel="mikrotik:wifiRegistration:rb1:wifi2:comment"}
-Switch     Tablet_2_Connected        "Connected"                            (gRB1Wifi2) {channel="mikrotik:wifiRegistration:rb1:wifi2:connected"}
-Switch     Tablet_2_Continuous       "Continuous"                           (gRB1Wifi2) {channel="mikrotik:wifiRegistration:rb1:wifi2:continuous"}
+Contact     Tablet_2_Connected        "Connected"                            (gRB1Wifi2) {channel="mikrotik:wifiRegistration:rb1:wifi2:connected"}
+Contact     Tablet_2_Continuous       "Continuous"                           (gRB1Wifi2) {channel="mikrotik:wifiRegistration:rb1:wifi2:continuous"}
 String     Tablet_2_Ssid             "Wi fi network"                        (gRB1Wifi2) {channel="mikrotik:wifiRegistration:rb1:wifi2:ssid"}
 String     Tablet_2_Interface        "Name"                                 (gRB1Wifi2) {channel="mikrotik:wifiRegistration:rb1:wifi2:interface"}
 Number     Tablet_2_Signal           "Received signal strength indicator"   (gRB1Wifi2) {channel="mikrotik:wifiRegistration:rb1:wifi2:signal"}
diff --git a/bundles/org.openhab.binding.mikrotik/src/main/java/org/openhab/binding/mikrotik/internal/handler/MikrotikInterfaceThingHandler.java b/bundles/org.openhab.binding.mikrotik/src/main/java/org/openhab/binding/mikrotik/internal/handler/MikrotikInterfaceThingHandler.java
index a7f251f50..ab947d446 100644
--- a/bundles/org.openhab.binding.mikrotik/src/main/java/org/openhab/binding/mikrotik/internal/handler/MikrotikInterfaceThingHandler.java
+++ b/bundles/org.openhab.binding.mikrotik/src/main/java/org/openhab/binding/mikrotik/internal/handler/MikrotikInterfaceThingHandler.java
@@ -127,10 +127,10 @@ public class MikrotikInterfaceThingHandler extends MikrotikBaseThingHandler<Inte
                     newState = StateUtil.stringOrNull(iface.getMacAddress());
                     break;
                 case MikrotikBindingConstants.CHANNEL_ENABLED:
-                    newState = StateUtil.boolOrNull(iface.isEnabled());
+                    newState = StateUtil.boolSwitchOrNull(iface.isEnabled());
                     break;
                 case MikrotikBindingConstants.CHANNEL_CONNECTED:
-                    newState = StateUtil.boolOrNull(iface.isConnected());
+                    newState = StateUtil.boolContactOrNull(iface.isConnected());
                     break;
                 case MikrotikBindingConstants.CHANNEL_LAST_LINK_DOWN_TIME:
                     newState = StateUtil.timeOrNull(iface.getLastLinkDownTime());
diff --git a/bundles/org.openhab.binding.mikrotik/src/main/java/org/openhab/binding/mikrotik/internal/handler/MikrotikWirelessClientThingHandler.java b/bundles/org.openhab.binding.mikrotik/src/main/java/org/openhab/binding/mikrotik/internal/handler/MikrotikWirelessClientThingHandler.java
index 20aec9d57..a6885f303 100644
--- a/bundles/org.openhab.binding.mikrotik/src/main/java/org/openhab/binding/mikrotik/internal/handler/MikrotikWirelessClientThingHandler.java
+++ b/bundles/org.openhab.binding.mikrotik/src/main/java/org/openhab/binding/mikrotik/internal/handler/MikrotikWirelessClientThingHandler.java
@@ -67,7 +67,7 @@ public class MikrotikWirelessClientThingHandler extends MikrotikBaseThingHandler
         super(thing);
     }
 
-    private boolean fetchModels() {
+    private void fetchModels() {
         var cfg = this.config;
         if (cfg != null) {
             RouterosDevice routeros = getRouterOs();
@@ -91,19 +91,15 @@ public class MikrotikWirelessClientThingHandler extends MikrotikBaseThingHandler
                 }
             }
         }
-        return this.wifiReg != null;
     }
 
     @Override
     protected void refreshModels() {
-        this.online = fetchModels();
-        if (online) {
-            lastSeen = LocalDateTime.now();
-        } else {
-            continuousConnection = false;
-        }
+        fetchModels();
         var wifiReg = this.wifiReg;
         if (wifiReg != null) {
+            this.lastSeen = LocalDateTime.now();
+            this.online = true;
             var cfg = this.config;
             int considerContinuous = 180;
             if (cfg != null) {
@@ -116,28 +112,27 @@ public class MikrotikWirelessClientThingHandler extends MikrotikBaseThingHandler
             LocalDateTime uptimeStart = wifiReg.getUptimeStart();
             continuousConnection = (uptimeStart != null)
                     && LocalDateTime.now().isAfter(uptimeStart.plusSeconds(considerContinuous));
+        } else {
+            this.online = false;
+            this.continuousConnection = false;
         }
     }
 
     @Override
     protected void refreshChannel(ChannelUID channelUID) {
         var wifiReg = this.wifiReg;
-        if (wifiReg == null) {
-            logger.warn("wifiReg is null in refreshChannel({})", channelUID);
-            return;
-        }
 
         String channelID = channelUID.getIdWithoutGroup();
         State oldState = currentState.getOrDefault(channelID, UnDefType.NULL);
         State newState = oldState;
 
         if (channelID.equals(CHANNEL_CONNECTED)) {
-            newState = StateUtil.boolOrNull(online);
+            newState = StateUtil.boolContactOrNull(this.online);
         } else if (channelID.equals(CHANNEL_LAST_SEEN)) {
             newState = StateUtil.timeOrNull(lastSeen);
         } else if (channelID.equals(CHANNEL_CONTINUOUS)) {
-            newState = StateUtil.boolOrNull(continuousConnection);
-        } else if (!online) {
+            newState = StateUtil.boolContactOrNull(this.continuousConnection);
+        } else if (!this.online || wifiReg == null) {
             newState = UnDefType.NULL;
         } else {
             switch (channelID) {
diff --git a/bundles/org.openhab.binding.mikrotik/src/main/java/org/openhab/binding/mikrotik/internal/util/StateUtil.java b/bundles/org.openhab.binding.mikrotik/src/main/java/org/openhab/binding/mikrotik/internal/util/StateUtil.java
index 5f63d1d95..0a99435ad 100644
--- a/bundles/org.openhab.binding.mikrotik/src/main/java/org/openhab/binding/mikrotik/internal/util/StateUtil.java
+++ b/bundles/org.openhab.binding.mikrotik/src/main/java/org/openhab/binding/mikrotik/internal/util/StateUtil.java
@@ -21,6 +21,7 @@ import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.core.library.types.DateTimeType;
 import org.openhab.core.library.types.DecimalType;
 import org.openhab.core.library.types.OnOffType;
+import org.openhab.core.library.types.OpenClosedType;
 import org.openhab.core.library.types.QuantityType;
 import org.openhab.core.library.types.StringType;
 import org.openhab.core.library.unit.Units;
@@ -63,13 +64,20 @@ public class StateUtil {
         return value == null ? UnDefType.NULL : new DecimalType(value);
     }
 
-    public static State boolOrNull(@Nullable Boolean value) {
+    public static State boolSwitchOrNull(@Nullable Boolean value) {
         if (value == null) {
             return UnDefType.NULL;
         }
         return value ? OnOffType.ON : OnOffType.OFF;
     }
 
+    public static State boolContactOrNull(@Nullable Boolean value) {
+        if (value == null) {
+            return UnDefType.NULL;
+        }
+        return value ? OpenClosedType.OPEN : OpenClosedType.CLOSED;
+    }
+
     public static State timeOrNull(@Nullable LocalDateTime value) {
         return value == null ? UnDefType.NULL : new DateTimeType(value.atZone(ZoneId.systemDefault()));
     }
diff --git a/bundles/org.openhab.binding.mikrotik/src/main/resources/OH-INF/thing/thing-types.xml b/bundles/org.openhab.binding.mikrotik/src/main/resources/OH-INF/thing/thing-types.xml
index 7297b93ca..d24eaab12 100644
--- a/bundles/org.openhab.binding.mikrotik/src/main/resources/OH-INF/thing/thing-types.xml
+++ b/bundles/org.openhab.binding.mikrotik/src/main/resources/OH-INF/thing/thing-types.xml
@@ -237,14 +237,14 @@
 	</channel-type>
 
 	<channel-type id="connected">
-		<item-type>Switch</item-type>
+		<item-type>Contact</item-type>
 		<label>Connected</label>
 		<description>Reflects connected or disconnected state</description>
 		<state readOnly="true"/>
 	</channel-type>
 
 	<channel-type id="continuous">
-		<item-type>Switch</item-type>
+		<item-type>Contact</item-type>
 		<label>Continuous</label>
 		<description>Connection is considered long-running</description>
 		<state readOnly="true"/>