eclipse / paho.mqtt.java

Eclipse Paho Java MQTT client library. Paho is an Eclipse IoT project.
https://eclipse.org/paho
Other
2.12k stars 883 forks source link

Possible source of confusion: connect(java.lang.Object userContext, IMqttActionListener callback) #290

Closed RyanRamchandar closed 6 years ago

RyanRamchandar commented 7 years ago

I could not figure out why my MqttAsyncClient was not respecting my MqttConnectOptions (automatic reconnect in particular) and then realized I was incorrectly using the following connect method and passing my options in as the first parameter which is actually just a userContext Object:

connect(java.lang.Object userContext, IMqttActionListener callback)

I should have been using the following which actually accepts a MqttConnectOptions:

connect(MqttConnectOptions options, java.lang.Object userContext, IMqttActionListener callback)

Not really sure what the fix should be for this would be.

jpwsutton commented 7 years ago

Unfortunately this is just one of the things about the current mqttv3 client that is quite confusing. Additionally, I'm not sure if there is any way to fix this without changing the API which we would not do as we don't want to break backwards compatibility for anyone right now. I think it's something that we'll just have to live with until the mqttv5 client is released which I'm starting development on now, as it will be a brand new client, hopefully we can take advantages of these lessons to create a less confusing API.

RyanRamchandar commented 7 years ago

@jpwsutton totally agree. This is just to keep in mind for the next API design.

jpwsutton commented 6 years ago

Closing as we never found a better way to change the IMqttAsyncClient API. There are still use cases where someone might want to connect using the default options, so I didn't feel that removing the existing connect(java.lang.Object userContext, IMqttActionListener callback) was the right thing to do, ultimately, the documentation is fairly clear and if using a modern IDE, most developers should have no issues.