[irobot] Some enhancements (#9973)

* [irobot] Roomba: Add more properties.
On request by @falkena, also some i7 specifics

* [irobot] Roomba: Add map_upload channel. 
Controls uploading Clean Map(tm) to the cloud.

* [irobot] discovery: Get rid of empty while() loop
Rewrite the loop so that it doesn't have empty body any more, this gets rid of
one more static analyzer warning. Added dumping the whole IDENT packet on TRACE
level, aids implementing support for newer devices.

Signed-off-by: Pavel Fedin <pavel_fedin@mail.ru>

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

@@ -30,29 +30,30 @@ known, however, whether the password is eternal or can change during factory res
 
 ## Channels
 
-| channel       | type   | description                                        | Read-only |
+| channel       | type   | description                                                               | Read-only |
-|---------------|--------|----------------------------------------------------|-----------|
+|---------------|--------|---------------------------------------------------------------------------|-----------|
-| command       | String | Command to execute: clean, cleanRegions, spot, dock, pause, stop | N |
+| command       | String | Command to execute: clean, spot, dock, pause, stop                        | N |
-| cycle         | String | Current mission: none, clean, spot                 | Y |
+| cycle         | String | Current mission: none, clean, spot                                        | Y |
-| phase         | String | Current phase of the mission; see below.           | Y |
+| phase         | String | Current phase of the mission; see below.                                  | Y |
-| battery       | Number | Battery charge in percents                         | Y |
+| battery       | Number | Battery charge in percents                                                | Y |
-| bin           | String | Bin status: ok, removed, full                      | Y |
+| bin           | String | Bin status: ok, removed, full                                             | Y |
-| error         | String | Error code; see below                              | Y |
+| error         | String | Error code; see below                                                     | Y |
-| rssi          | Number | Wi-Fi Received Signal Strength indicator in db     | Y |
+| rssi          | Number | Wi-Fi Received Signal Strength indicator in db                            | Y |
-| snr           | Number | Wi-Fi Signal to noise ratio                        | Y |
+| snr           | Number | Wi-Fi Signal to noise ratio                                               | Y |
-| sched_mon     | Switch | Scheduled clean enabled for Monday                 | N |
+| sched_mon     | Switch | Scheduled clean enabled for Monday                                        | N |
-| sched_tue     | Switch | Scheduled clean enabled for Tuesday                | N |
+| sched_tue     | Switch | Scheduled clean enabled for Tuesday                                       | N |
-| sched_wed     | Switch | Scheduled clean enabled for Wednesday              | N |
+| sched_wed     | Switch | Scheduled clean enabled for Wednesday                                     | N |
-| sched_thu     | Switch | Scheduled clean enabled for Thursday               | N |
+| sched_thu     | Switch | Scheduled clean enabled for Thursday                                      | N |
-| sched_fri     | Switch | Scheduled clean enabled for Friday                 | N |
+| sched_fri     | Switch | Scheduled clean enabled for Friday                                        | N |
-| sched_sat     | Switch | Scheduled clean enabled for Saturday               | N |
+| sched_sat     | Switch | Scheduled clean enabled for Saturday                                      | N |
-| sched_sun     | Switch | Scheduled clean enabled for Sunday                 | N |
+| sched_sun     | Switch | Scheduled clean enabled for Sunday                                        | N |
 | schedule      | Number | Schedule bitmask for use in scripts. 7 bits, bit #0 corresponds to Sunday | N |
-| edge_clean    | Switch | Seek out and clean along walls and furniture legs  | N |
+| edge_clean    | Switch | Seek out and clean along walls and furniture legs                         | N |
-| always_finish | Switch | Whether to keep cleaning if the bin becomes full   | N |
+| always_finish | Switch | Whether to keep cleaning if the bin becomes full                          | N |
-| power_boost   | String | Power boost mode: "auto", "performance", "eco"     | N |
+| power_boost   | String | Power boost mode: "auto", "performance", "eco"                            | N |
-| clean_passes  | String | Number of cleaning passes: "auto", "1", "2"        | N |
+| clean_passes  | String | Number of cleaning passes: "auto", "1", "2"                               | N |
-| last_command  | String | Json string containing the parameters of the last executed command | N |
+| map_upload    | Switch | Enable or disable uploading Clean Map(tm) to cloud for notifications      | N |
+| last_command  | String | Json string containing the parameters of the last executed command        | N |
 
 Known phase strings and their meanings:
 

bundles/org.openhab.binding.irobot/src/main/java/org/openhab/binding/irobot/internal/IRobotBindingConstants.java

@@ -48,6 +48,7 @@ public class IRobotBindingConstants {
     public static final String CHANNEL_ALWAYS_FINISH = "always_finish";
     public static final String CHANNEL_POWER_BOOST = "power_boost";
     public static final String CHANNEL_CLEAN_PASSES = "clean_passes";
+    public static final String CHANNEL_MAP_UPLOAD = "map_upload";
     public static final String CHANNEL_LAST_COMMAND = "last_command";
 
     public static final String CMD_CLEAN = "clean";

bundles/org.openhab.binding.irobot/src/main/java/org/openhab/binding/irobot/internal/discovery/IRobotDiscoveryService.java

@@ -17,6 +17,7 @@ import java.net.DatagramPacket;
 import java.net.DatagramSocket;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -92,7 +93,10 @@ public class IRobotDiscoveryService extends AbstractDiscoveryService {
                 logger.debug("Starting broadcast for {}", broadcastAddress.toString());
 
                 try (DatagramSocket socket = IdentProtocol.sendRequest(broadcastAddress)) {
-                    while (receivePacketAndDiscover(socket)) {
+                    DatagramPacket incomingPacket;
+
+                    while ((incomingPacket = receivePacket(socket)) != null) {
+                        discover(incomingPacket);
                     }
                 } catch (IOException e) {
                     logger.warn("Error sending broadcast: {}", e.toString());
@@ -119,31 +123,35 @@ public class IRobotDiscoveryService extends AbstractDiscoveryService {
         return addresses;
     }
 
-    private boolean receivePacketAndDiscover(DatagramSocket socket) {
+    private @Nullable DatagramPacket receivePacket(DatagramSocket socket) {
-        DatagramPacket incomingPacket;
-
         try {
-            incomingPacket = IdentProtocol.receiveResponse(socket);
+            return IdentProtocol.receiveResponse(socket);
         } catch (IOException e) {
             // This is not really an error, eventually we get a timeout
             // due to a loop in the caller
-            return false;
+            return null;
         }
+    }
 
+    private void discover(DatagramPacket incomingPacket) {
         String host = incomingPacket.getAddress().toString().substring(1);
+        String reply = new String(incomingPacket.getData(), StandardCharsets.UTF_8);
+
+        logger.trace("Received IDENT from {}: {}", host, reply);
+
         IdentProtocol.IdentData ident;
 
         try {
-            ident = IdentProtocol.decodeResponse(incomingPacket);
+            ident = IdentProtocol.decodeResponse(reply);
         } catch (JsonParseException e) {
             logger.warn("Malformed IDENT reply from {}!", host);
-            return true;
+            return;
         }
 
         // This check comes from Roomba980-Python
         if (ident.ver < IdentData.MIN_SUPPORTED_VERSION) {
             logger.warn("Found unsupported iRobot \"{}\" version {} at {}", ident.robotname, ident.ver, host);
-            return true;
+            return;
         }
 
         if (ident.product.equals(IdentData.PRODUCT_ROOMBA)) {
@@ -153,7 +161,5 @@ public class IRobotDiscoveryService extends AbstractDiscoveryService {
 
             thingDiscovered(result);
         }
-
-        return true;
     }
 }

bundles/org.openhab.binding.irobot/src/main/java/org/openhab/binding/irobot/internal/dto/IdentProtocol.java

@@ -58,6 +58,10 @@ public class IdentProtocol {
     }
 
     public static IdentData decodeResponse(DatagramPacket packet) throws JsonParseException {
+        return decodeResponse(new String(packet.getData(), StandardCharsets.UTF_8));
+    }
+
+    public static IdentData decodeResponse(String reply) throws JsonParseException {
         /*
          * packet is a JSON of the following contents (addresses are undisclosed):
          * @formatter:off
@@ -87,7 +91,6 @@ public class IdentProtocol {
          * }
          * @formatter:on
          */
-        String reply = new String(packet.getData(), StandardCharsets.UTF_8);
         // We are not consuming all the fields, so we have to create the reader explicitly
         // If we use fromJson(String) or fromJson(java.util.reader), it will throw
         // "JSON not fully consumed" exception, because not all the reader's content has been

bundles/org.openhab.binding.irobot/src/main/java/org/openhab/binding/irobot/internal/dto/MQTTProtocol.java

@@ -177,6 +177,24 @@ public class MQTTProtocol {
         }
     }
 
+    public static class MapUploadAllowed extends StateValue {
+        public boolean mapUploadAllowed;
+
+        public MapUploadAllowed(boolean mapUploadAllowed) {
+            this.mapUploadAllowed = mapUploadAllowed;
+        }
+    }
+
+    public static class SubModSwVer {
+        public String nav;
+        public String mob;
+        public String pwr;
+        public String sft;
+        public String mobBtl;
+        public String linux;
+        public String con;
+    }
+
     // "reported" messages never contain the full state, only a part.
     // Therefore all the fields in this class are nullable
     public static class GenericState extends StateValue {
@@ -202,6 +220,8 @@ public class MQTTProtocol {
         public Boolean noAutoPasses;
         // "twoPass":true
         public Boolean twoPass;
+        // "mapUploadAllowed":true
+        public Boolean mapUploadAllowed;
         // "softwareVer":"v2.4.6-3"
         public String softwareVer;
         // "navSwVer":"01.12.01#1"
@@ -214,6 +234,18 @@ public class MQTTProtocol {
         public String bootloaderVer;
         // "umiVer":"6",
         public String umiVer;
+        // "sku":"R981040"
+        public String sku;
+        // "batteryType":"lith"
+        public String batteryType;
+        // Used by i7:
+        // "subModSwVer":{
+        // "nav": "lewis-nav+3.2.4-EPMF+build-HEAD-7834b608797+12", "mob":"3.2.4-XX+build-HEAD-7834b608797+12",
+        // "pwr": "0.5.0+build-HEAD-7834b608797+12",
+        // "sft":"1.1.0+Lewis-Builds/Lewis-Certified-Safety/lewis-safety-bbbe81f2c82+21",
+        // "mobBtl": "4.2", "linux":"linux+2.1.6_lock-1+lewis-release-rt419+12",
+        // "con":"2.1.6-tags/release-2.1.6@c6b6585a/build"}
+        public SubModSwVer subModSwVer;
         // "lastCommand":
         // {"command":"start","initiator":"localApp","time":1610283995,"ordered":1,"pmap_id":"AAABBBCCCSDDDEEEFFF","regions":[{"region_id":"6","type":"rid"}]}
         public JsonElement lastCommand;

bundles/org.openhab.binding.irobot/src/main/java/org/openhab/binding/irobot/internal/handler/RoombaHandler.java

@@ -188,6 +188,10 @@ public class RoombaHandler extends BaseThingHandler implements MqttConnectionObs
             sendDelta(new MQTTProtocol.PowerBoost(command.equals(BOOST_AUTO), command.equals(BOOST_PERFORMANCE)));
         } else if (ch.equals(CHANNEL_CLEAN_PASSES)) {
             sendDelta(new MQTTProtocol.CleanPasses(!command.equals(PASSES_AUTO), command.equals(PASSES_2)));
+        } else if (ch.equals(CHANNEL_MAP_UPLOAD)) {
+            if (command instanceof OnOffType) {
+                sendDelta(new MQTTProtocol.MapUploadAllowed(command.equals(OnOffType.ON)));
+            }
         }
     }
 
@@ -512,12 +516,30 @@ public class RoombaHandler extends BaseThingHandler implements MqttConnectionObs
             reportString(CHANNEL_LAST_COMMAND, reported.lastCommand.toString());
         }
 
+        if (reported.mapUploadAllowed != null) {
+            reportSwitch(CHANNEL_MAP_UPLOAD, reported.mapUploadAllowed);
+        }
+
         reportProperty(Thing.PROPERTY_FIRMWARE_VERSION, reported.softwareVer);
         reportProperty("navSwVer", reported.navSwVer);
         reportProperty("wifiSwVer", reported.wifiSwVer);
         reportProperty("mobilityVer", reported.mobilityVer);
         reportProperty("bootloaderVer", reported.bootloaderVer);
         reportProperty("umiVer", reported.umiVer);
+        reportProperty("sku", reported.sku);
+        reportProperty("batteryType", reported.batteryType);
+
+        if (reported.subModSwVer != null) {
+            // This is used by i7 model. It has more capabilities, perhaps a dedicated
+            // handler should be written by someone who owns it.
+            reportProperty("subModSwVer.nav", reported.subModSwVer.nav);
+            reportProperty("subModSwVer.mob", reported.subModSwVer.mob);
+            reportProperty("subModSwVer.pwr", reported.subModSwVer.pwr);
+            reportProperty("subModSwVer.sft", reported.subModSwVer.sft);
+            reportProperty("subModSwVer.mobBtl", reported.subModSwVer.mobBtl);
+            reportProperty("subModSwVer.linux", reported.subModSwVer.linux);
+            reportProperty("subModSwVer.con", reported.subModSwVer.con);
+        }
     }
 
     private void reportVacHigh() {

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

@@ -51,6 +51,7 @@
 			<channel id="always_finish" typeId="always_finish"/>
 			<channel id="power_boost" typeId="power_boost"/>
 			<channel id="clean_passes" typeId="clean_passes"/>
+			<channel id="map_upload" typeId="map_upload"/>
 		</channels>
 		<config-description>
 			<parameter name="ipaddress" type="text">
@@ -263,5 +264,10 @@
 		<state readOnly="true">
 		</state>
 	</channel-type>
+	<channel-type id="map_upload" advanced="true">
+		<item-type>Switch</item-type>
+		<label>Map upload</label>
+		<description>Enable uploading Clean Map(tm) to cloud for reporting</description>
+	</channel-type>
 
 </thing:thing-descriptions>