MQTT v5 support

[paul-lysak]

Hello. We'd like vertx-mqtt to support MQTT v5 (https://docs.oasis-open.org/mqtt/mqtt/v5.0/mqtt-v5.0.html) and we're willing to develop such changes. Before we started, could you give us some guidelines on how to make sure that our MQTT5-related changes are suitable for merging back into the upstream vertx-mqtt?

In particular, what's the best way to change netty-codec-mqtt - Netty maintainers don't seem to be interested in its further development (https://github.com/netty/netty/issues/6285), it makes no sense to expect MQTT5 support from them. So, if vertx-mqtt is to support MQTT v5 then there are 2 possible solutions:

• move netty-codec-mqtt code directly in vertx-mqtt (with renaming the packages)
• create a separate project with code from netty-codec-mqtt, rename the packages, organization, and artifact, deploy it to some publically available artifactory

What's your opinion on this?

[ppatierno]

Tbh I didn't get what the comment "This was moved to an alternative project" means? Which alternative project? I think that the matt v5 coded should be part of netty and not of this project as we do for mqtt 3.1. Why can't you work on mqtt 5 codec for netty and then reusing it here?

[ppatierno]

I found the codec was moved here https://github.com/moquette-io/netty-mqtt5-codec Maybe you can use it?

[ppatierno]

The only thing I see is about how much it's supported. There are just 2 commits and 2 years ago. :-(

[paul-lysak]

The only thing I see is about how much it's supported. There are just 2 commits and 2 years ago. :-(

That's the issue. Another problem is that its artifacts aren't deployed to any public Maven repository. I'll try to ping Netty maintainers again to see what they think on MQTT5. If they still don't respond - we can release an artifact based on https://github.com/moquette-io/netty-mqtt5-codec and do the changes in vertx-mqtt to use it.

[paul-lysak]

I've submitted a PR to the Netty: https://github.com/netty/netty/pull/10483 . It's based on https://github.com/moquette-io/netty-mqtt5-codec with some corrections and improvements. Let's see how will they react now :)

[ppatierno]

@paul-lysak and it was approved! Congrats! Now if you are willing to work on this for adding the support to the Vert.x MQTT library, I would say to:

1. trying to do it in steps, not a huge PR but little PRs adding incremental features. It's easy to review for us.
2. I would do only for the master, so for future Vert.x 4 and not for 3.9. I would expect too much work for backporting due to the differences. @vietj what's your opinion on this?

The first thing to do should be waiting for a new Netty release with the MQTT 5 support, bumping the version here and checking that nothing is broken with 3.1.1 :-)

[paul-lysak]

@paul-lysak and it was approved! Congrats! Now if you are willing to work on this for adding the support to the Vert.x MQTT library, I would say to:

1. trying to do it in steps, not a huge PR but little PRs adding incremental features. It's easy to review for us.
2. I would do only for the master, so for future Vert.x 4 and not for 3.9. I would expect too much work for backporting due to the differences. @vietj what's your opinion on this?

The first thing to do should be waiting for a new Netty release with the MQTT 5 support, bumping the version here and checking that nothing is broken with 3.1.1 :-)

Thank you! Will start work on it after the nearest Netty release. I'll try to split it into parts wherever possible.

[paul-lysak]

We've decided not to go forward with providing PR for this issue: I've tried to do the necessary changes and turned out that satisfying code generator (for classes marked with @VertxGen) will take more effort and require more wrapping than just copying and adapting to our use case. For example, the code generator was not happy when I've added to MqttConnAckMessage new create method that takes MqttProperties.

For those who decide to go forward - here's my assessment of the required changes:

• MqttConnAckMessage: add properties

• Specify protocol version in MqttClientImpl

• MqttClientImpl and corresponding interfaces - support passing the properties (probably, via MqttClientOptions) for CONNECT itself and for the Will message

• Reason code on all acks:

  • PUBACK:

    • MqttClientImpl.publishAcknowledge - use MqttPubReplyMessageVariableHeader for the header

    • MqttClientImpl.publishCompletionHandler - provide code and properties of the last completion message

    • MqttEndpointImpl.publishAcknowledge - add reason code and properties, change header class

    • MqttEndpointImpl.publishAcknowledgeHandler - add with a different signature, that takes reason code and properties too

  • PUBREC: add reason code and properties:

    • MqttClientImpl.publishReceived

    • MqttClientImpl.publishCompletionHandler

    • MqttEndpoint.publishReceive

    • MqttEndpoint.publishReceivedHandler

  • PUBREL - same

  • PUBCOMP - same

  • SUBACK

    • MqttClientImpl.subscribeCompletionHandler

    • MqttEndpoint.subscribeAcknowledge

  • UNSUBACK

    • MqttClientImpl.unsubscribeCompletionHandler

    • MqttEndpoint.unsubscribeAcknowledge

  • DISCONNECT

    • MqttClientImpl - add disconnect handler

    • MqttEndpoint.disconnectHandler - handle code and headers

    • MqttEndpoint - add disconnect with the code and headers

• Indicate maximal packet size

  • MqttClientImpl.doConnect

  • MqttEndpointImpl.connack

• Properties support

  • CONNACK

    • MqttEndpointImpl.accept/reject

    • MqttConnAckMessage - add properties

• AUTH support

  • MqttEndpoint: add auth and authHandler methods

• SUBSCRIBE: options instead of just QoS byte, add properties

  • MqttClientImpl.subscribe - take options instead of QoS. Take properties.

  • MqttSubscribeMessage - add properties

[vietj]

@paul-lysak can you reconsider this choice if we help you on this ?

e.g if you annotate the method with @GenIgnore() then the code generator will ignore this method , etc...

[paul-lysak]

@vietj what is exact purpose of @VertxGen in MqttConnAckMessage and other classes? How do I know which methods I may ignore and which not? Tried to ignore original connect method - mvn verify seems to pass just fine. I may consider contributions, but it's not going to be a priority.

[vietj]

the goal is to code generate API in other languages, e.g the Vert.x RxJava 2 API is generated using it.

in general just add @GenIgnore everywhere you need it in VertxGen class and if we can share a working branch we (maintainer of vertx) will take care of making this work for you.

[paul-lysak]

Ok, reopened https://github.com/vert-x3/vertx-dependencies/pull/49 and https://github.com/vert-x3/vertx-mqtt/pull/168. Will try to contribute more occasionally.

[manju-reddys]

Mqtt V5 support will be added as part of Vertx V4 release?

[vietj]

@manju-reddys we are missing contributors for this, so it will be added if somebody contributes it

[vietj]

@paul-lysak is motivated to contribute it, I don't know what's the current status of his contribution

[paul-lysak]

@vietj all the contributions are blocked by https://github.com/vert-x3/vertx-dependencies/pull/49

[vietj]

@paul-lysak yes I remember now.

I think you can try override these dependencies in the vertx-mqtt pom file of a working branch until we sort this out ?

[paul-lysak]

I've started to work on back-porting MQTT5 changes from our system to this repository. @vietj you can see the work in progress here: https://github.com/simplematter/vertx-mqtt/tree/mqtt5 . Let me know if you have any feedback on that.

[vietj]

thanks for heads up @paul-lysak !

[vietj]

@paul-lysak FYI Netty was bumped to Netty 4.1.60.Final in master

[paul-lysak]

Here comes server-side support (except of AUTH message): https://github.com/vert-x3/vertx-mqtt/pull/194

[chenzhiguo]

Has this feature been developed yet? Do you need any more help?

[joggeli34]

Are there any plans, that the client also supports v5?

[vietj]

any contribution would be accepted, the core team has no bandwidth for this currently

On Sat, Feb 5, 2022 at 8:26 AM joggeli34 @.***> wrote:

Are there any plans, that the client also supports v5?

— Reply to this email directly, view it on GitHub https://github.com/vert-x3/vertx-mqtt/issues/161#issuecomment-1030568877, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABXDCV2BCZP7QAPEUI5KRTUZTGJTANCNFSM4OZQMXLA . You are receiving this because you were mentioned.Message ID: @.***>

[mattisonchao]

@vietj I will work on it.

[vietj]

looking forward for this @mattisonchao