[iRobot] Vacuum cleaner not cleaning room

[Nuesel]

Hello all,
I'm trying to use my iRobot Roomba from Openhab, especially cleaning special rooms is important for me. Unfortunately, my iRobot Roomba i7 starts cleaning everything instead of cleaning only one room.

The problem seems to be the parameter user_pmapv_id, that is omnitted when sending the vacuum cleaner on its mission. The robot expects a command of the syntax
{"command":"start","initiator":"rmtApp","time":1633160056,"ordered":1,"pmap_id":"mymapid123456789012345","regions":[{"region_id":"18","type":"rid"}],"user_pmapv_id":"123456T123456"}
Currently, the iRobot addon does not pass user_pmapv_id. Consequently, last_command contains user_pmapv_id, but it is set to null. Maybe, older versions of the Roomba software did not need this parameter. But now it is obligatory.
I think, I managed somehow to add this feature in the addon (files RoombaHandler.java and MQTTProtocol.java). How do I create branch?

Additionally, the iRobot addon is not able to let the Roomba clean special zones. A region corresponds to whole rooms, zones are user defined rectangle areas within rooms. For zones, the type must not be "rid" but "zid" (zone ID). Currently, in the Java code the type is fixed to "rid". Honestly, I don't know how add this parameter since my Java knowledge is very limited. I feel a bit strange about this gson stuff.

My iRobot uses the software version 3.18.11 (currently up to date).
Thanks for any help in advance!

Regards,
Nuesel

[Nuesel]

No one can help?

[lolodomo]

Pinging the author of the binding: @Sonic-Amiga

[Sonic-Amiga]

Hello! Sorry for the delay; i don't check my email often, perhaps it's my bad. Since i've got active conversations now, i'll do it quicker.

Currently, the iRobot addon does not pass user_pmapv_id. Consequently, last_command contains user_pmapv_id, but it is set to null. Maybe, older versions of the Roomba software did not need this parameter. But now it is obligatory.
I think, I managed somehow to add this feature in the addon (files RoombaHandler.java and MQTTProtocol.java). How do I create branch?

You're supposed to fork the whole repository; this will be your private repo. In this repository you'll be able to create a branch and post a pull request. You'll post it against the central repo. This is how github works.

Additionally, the iRobot addon is not able to let the Roomba clean special zones. A region corresponds to whole rooms, zones are user defined rectangle areas within rooms. For zones, the type must not be "rid" but "zid" (zone ID). Currently, in the Java code the type is fixed to "rid". Honestly, I don't know how add this parameter since my Java knowledge is very limited. I feel a bit strange about this gson stuff.

gson is built using reflection. During serializing an object, the library will examine data fields names of the given class and produce a respective JSON text. Automatically.

Let's consider a simple example:

    public static class CommandRequest implements Request {
        public String command;
        public long time;
        public String initiator;

        public CommandRequest(String cmd) {
            command = cmd;
            time = System.currentTimeMillis() / 1000;
            initiator = "openhab";
        }

        @Override
        public String getTopic() {
            return "cmd";
        }
    }

When serialized to JSON string, this object will produce:

{"command":"cmd", "time":12345, "initiator":"openhab"}

Methods will be completely ignored by the serializer, they are just for convenience of using this class from the code. So there's nothing special about them. Methods are just methods, constructors are just constructors. You can extend Region class constructor in order to accept "type" as a second argument; nothing will break.

When deserializing from a JSON string, if some field is missing in the string, a default value will be assigned. In other words, after object instantiation, the respective field won't be "poked" with its value from the JSON (because there's nothing provided). And vice versa, if there's some stuff in the JSON, not present in the class definition, it will be just ignored. However, a special approach needs to be used for reading the string for this to work, see comments in RoombaHandler.receive().

There's a method to make field name in the JSON different from field name in the class. A "@SerializedName" annotation is used for the purpose. In the example we have:

    public static class CleanRoomsRequest extends CommandRequest {
        public int ordered;
        @SerializedName("pmap_id")
        public String pmapId;
        public List<Region> regions;

        public CleanRoomsRequest(String cmd, String mapId, String[] regions) {
            super(cmd);
            ordered = 1;
            pmapId = mapId;
            this.regions = Arrays.stream(regions).map(i -> new Region(i)).collect(Collectors.toList());
        }

        public static class Region {
            @SerializedName("region_id")
            public String regionId;
            public String type;

            public Region(String id) {
                this.regionId = id;
                this.type = "rid";
            }
        }
    }

fields pmapId and regionId will be serialized as "pmap_id" and "region_id" respectively. It's done in order to conform to OpenHAB's coding style, where data members must be named in camelCase.

Bottom line: basically, this is very much like C(++) struct.

[Nuesel]

@Sonic-Amiga, thanks a lot for the extensive answer!

So far, so good. The parameter user_pmapv_id was indeed quite easy. But I have a problem adding a user defined type in the class Region. I extend the constructor from public Region(String id) to public Region(String id, String type). Additionally, I changed the line:
this.regions = Arrays.stream(regions, types).map(i -> new Region(i)).collect(Collectors.toList());
Here, types is of type String[], similar to regions.
When compiling, it gives me the error:
The method stream(T[]) in the type java.util.Arrays is not applicable for the arguments (java.lang.String[], java.lang.String[])
But what is the correct way?

[Sonic-Amiga]

Hello! The answer is:

        public CleanRoomsRequest(String cmd, String mapId, String[] regions) {
            super(cmd);
            ordered = 1;
            pmapId = mapId;
            String type = "rid"; // This is a regular initialization, just a placeholder for you, nothing fancy.
            this.regions = Arrays.stream(regions).map(i -> new Region(i, type)).collect(Collectors.toList());
        }

This construct is called a "closure" AKA "lambda expression". You're calling a map() method of Stream class, which basically iterates over members of the array and calls a function, supplied as argument, on each member. The "i -> new Region(i, type)" expression instantiates an anonymous function of the following contents:

Region f(String i) {
    return new Region(i, type);
}

but where does "type" come from? It's called "capturing". When you instantiate this anonymous function, technically, a (anonymous, again) class is created, in whose body a value of "type" is remembered. And an object of this class is instantiated, capturing value of "type". This is more like:

class fClass {
    private String type;
    public Region f(String i) {
        return new Region(i, this.type);
    }
}

Could this have been written using a conventional "for" loop without this lambda-mumbo-jumbo? Yes, it could, but nowadays it's considered "uncool", people just love lambdas; they are trendy and sexy.

P.S. I don't like lambdas; neither i like functional programming; i am very oldschool.
P.P.S. Real Javists would probably slap me hard for such an explanation because it's not using proper terminology. Sorry guys, i'm not really a Java coder; i am a guest from C world; and only dealing with Java because OH is written in it. I am simply trying to convey my understanding by explaining what really happens under the hood; no matter how it's properly called. Again, i am ve-e-e-ry oldschool :)

[Nuesel]

Hello!
I tried to find out more about lambda expression. But the problem I'm still facing is type in
this.regions = Arrays.stream(regions).map(i -> new Region(i, type)).collect(Collectors.toList());
is not a constant String but also a String array. Each entry in the arrays regions and type, respectively, are a pair. E.g. regions[2] and type[2] should contain a pair of information.
I'm afraid, the map method cannot resolve such a problem. Maybe I will try it via an ordinary for loop.
Thanks a lot for your help! I appreciate that.

[Sonic-Amiga]

So in my example one type value would apply to all elements in the list. Do you need to break it and specify "type" for each region string ? Yes, in this case you'll be rewriting the whole thing. And yes, a map() method iterates only one thing, not two things simultaneously. So if you have two arrays, and want to iterate over both of them using the same indices for both, yes, you'd better scrap this modern sexy approach and write a conventional for loop.

[lsiepel]

Looks like this issue was fixed with PR #11783, if not please feel te re-open.