[Worxlandroid] Initial contribution

[clinique]

This binding is used since a while as a standalone - let's get it merged.

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/worx-landroid-binding/95246/570

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/worx-landroid-binding/95246/609

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/worx-landroid-binding/95246/619

[lsiepel]

Thanks for contributing this binding. This is a partly review, i only looked at the documentation, thing structure and some metadata files. I'll continue my review when i find some more time.
I noticed an images folder, i guess that can be removed as they do not seem to be used anywhere.

[lsiepel]

This binding performs some similar requests/response, so it should be possible to create them without these SDK's.
Could you remove these dependencies?

[clinique]

Can you be more explicit ? I'm afraid I did not get the meaning of your comment.
IIRW I tried to upgrade dependencies but it had major impact on code structure, so prefered to leave as it was.

[lsiepel]

I meant to say that the SDK should not be used. Instead the httpclient can be used to perform the same api request/responses.

You could look at the salus binding that also holds a very light weight base class doing much of the SDK work without needing the actual SDK.

Due to problems of the past with size, upgrades and other issues we try to keep these SDK’s out of the codebase.

[clinique]

@lsiepel : This is for Mqtt client of AWS IoT, I remember I failed to switch to another MQTT client (because of the need of a websocket for the authentication IIRW that was not available with available one). I'll have another look at it to be sure.

[lsiepel]

Have not looked in depth on how the MQTT client and websocket / authentication work together. I just know that we tent not to accept these SDK's. For the salus binding we had a similar issue with the software.amazon.awssdk.crt package. A fix was found by some lightweight classes that implemented the authentication.
Hope you figure out how to get rid of the SDK with these suggestions, otherwise please post the specific problems you encounter and maybe we can find a fix together. If ultimately it seems not feasable, we can always merge it as is, but yeah really prefer not to..

[wborn]

I can't find any problems so far with the URL encoder in the HiveMQ implementation, is that still a problem and why?

What I found after looking at the code:

• The HiveMQ client uses a URI in the MqttWebSocketInitializer.
• Using a URI is not necessarily an issue because a URI has a getRawQuery() method which returns the provided String and doesn't do any decoding.
• The HiveMQ client uses Netty for its requests. It looks like Netty will also call getRawQuery() when creating WebSocket requests.

[clinique]

I tried HiveMQ client in my eatly tentative. I do not find back what blocked me at the time but the result is I did not suceed in that way.

[lsiepel]

I can't find any problems so far with the URL encoder in the HiveMQ implementation, is that still a problem and why?

Did you read the linked comment? As that also points to: hivemq/hivemq-mqtt-client#643
Maybe @jpg0 can elaborate on the issue and if your suggestion is a workaround.
@clinique would it be possible for you to give it another try and describe where you encounter issues? It would be helpfull to either find a workaround or a decision on SDK usage.

[wborn]

As that also points to: hivemq/hivemq-mqtt-client#643

That issue is not very detailed as it doesn't tell exactly where the issue occurs.

Also I couldn't relate the code in that issue to actual HiveMQ Client APIs. 🤔

[jpg0]

I filed that issue quite a while ago. I have just tried to dig through this as I had originally come to the same conclusion as you, but could not make it work. I've managed to reduce it to the following code:

import java.net.URI;

class Main {
    public static void main(String[] args) throws Exception {
        String input = "x/y";
        URI uri = new URI(
            "wss", null, "host", 8080, "/test", input, null);
            
        //Try to output: x%2Fy
        System.out.println(uri.getRawQuery());
    }
}

In the above it creates a URI just like hivemq, then passes it on to Netty which extracts the raw query as you pointed out. The challenge is to get the string that Netty uses (and is returned by getRawQuery()) to be equal to x%2Fy (AWS explicitly checks the exact URI for sig validation, so it cannot be different at all). If you pass it unencoded (x/y) is remains this way, if you pass it encoded (x%2Fy) it double encodes it.

[lsiepel]

Can you rebase your branch, it would make it simpler for me to trouble shoot the dependency issue.

[clinique]

@lsiepel : rebased

[lsiepel]

The rebase also caused this to be missing. I added this locally, but now it claims it needs org.graalvm.sdk.
According to maven, this is an optional module. Do you know if this is essential or not? I can adapt the osgi package. I also wonder what we did wrong with the previous test as it was confirmed, but now we have these issues.

[lsiepel]

^ what are your thoughts about graal?

[clinique]

^ what are your thoughts about graal?

I discover this dependency with your message. Do you think it could have been introduced with the version update ?

[lsiepel]

^ what are your thoughts about graal?

I discover this dependency with your message. Do you think it could have been introduced with the version update ?

Yes, it was introduced with version upgrade

[clinique]

^ what are your thoughts about graal?

I discover this dependency with your message. Do you think it could have been introduced with the version update ?

Yes, it was introduced with version upgrade

Is it worth trying to package it in osgii ?