Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MQTT5 single message-received callback #453

Open
1 of 2 tasks
dieter-aerit opened this issue Aug 8, 2023 · 2 comments
Open
1 of 2 tasks

MQTT5 single message-received callback #453

dieter-aerit opened this issue Aug 8, 2023 · 2 comments
Labels
feature-request A feature should be added or improved. p3 This is a minor priority issue

Comments

@dieter-aerit
Copy link

Describe the feature

This is feedback on the MQTT 5 API.

Cfr. the documentation:
The MQTT5 client has a single message-received callback. Per-subscription callbacks are not supported.

I was quite surprised to discover this, and I'm wondering if there's a good reason for this?

As it stands right now, the MQTT5 API feels very low-level, and not really helpful.
The fact that I seem to have to implement my own topic parsing and routing logic doesn't make it feel like this is a developer-friendly API, yet rather a low-level API meant to be used to build another abstraction layer on top.
Being forced to implement this logic myself feels like re-inventing the wheel, requires a lot of boilerplate code that most developers using this library will probably want, and just opens up my application to potential bugs.

It would be really nice to offer the option to register callbacks with subscriptions (similar to the MQTT 3 API), it makes using the API so much easier. I can imagine there are cases where developers want fine-grained or non-standard control on how to handle topic subscriptions and routing messages, but this feels like it should be a minority.

Use Case

I don't want to implement topic parsing and routing messages to different parts of my code myself.

I thought the MQTT 3 API with per-subscription callbacks was great!

Proposed Solution

A somewhat higher-level, possibly complementary API to create subscriptions and register callbacks?
Or at least offer some utility code to handle the boilerplate of parsing topics, routing messages and registering callbacks.

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

SDK version used

software.amazon.awssdk.iotdevicesdk:aws-iot-device-sdk:1.15.0

Environment details (OS name and version, etc.)

MacOS Ventura 13.4.1 (M2)

@dieter-aerit dieter-aerit added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Aug 8, 2023
@bretambrose
Copy link
Contributor

Cfr. the documentation: The MQTT5 client has a single message-received callback. Per-subscription callbacks are not supported.

I was quite surprised to discover this, and I'm wondering if there's a good reason for this?

As it stands right now, the MQTT5 API feels very low-level, and not really helpful. The fact that I seem to have to implement my own topic parsing and routing logic doesn't make it feel like this is a developer-friendly API, yet rather a low-level API meant to be used to build another abstraction layer on top. Being forced to implement this logic myself feels like re-inventing the wheel, requires a lot of boilerplate code that most developers using this library will probably want, and just opens up my application to potential bugs.

I am of the opinion that a base network client should be a tight wrapper around the protocol specification itself. Entangling higher-level logic inside the protocol implementation has lead to a lot of problems (bugs, complexity, awkward APIs, etc...). For example:

  • Our clients are written in native code. We've had multiple crash bugs (as in the JVM aborts) in the (painfully opaque) logic responsible for the routing (https://github.com/awslabs/aws-c-mqtt/blob/main/source/topic_tree.c). One was just fixed a few months ago, which means it's been out in the wilds for 3.5 years.
  • you have to add a singular on-any-message callback anyways in order to handle messages that don't match a subscription, leading to API usage confusion
  • further adding to potential confusion, overlapping subscriptions generate multiple calls (in addition to the on-any-message callback). All of these are full native->Java round trips.
  • The callbacks need to be explicitly tracked as (Managed language: Java/JS/Python/etc...)<-> native bindings. This significantly increases the complexity of binding the overall native API to the managed language.

Beyond those, I'm not aware of other MQTT clients who made a decision to bake routing-by-topic into their low-level API. From a separation-of-concerns standpoint, it was a mistake.

It would be really nice to offer the option to register callbacks with subscriptions (similar to the MQTT 3 API), it makes using the API so much easier. I can imagine there are cases where developers want fine-grained or non-standard control on how to handle topic subscriptions and routing messages, but this feels like it should be a minority.

I can't say I agree with the assertion that it's a minority but I honestly don't know. The IoT space has a lot of "reporting/notification" use cases that don't require significant routing logic. Even if it is the minority, I still don't think it belongs on the client itself.

Proposed Solution

A somewhat higher-level, possibly complementary API to create subscriptions and register callbacks? Or at least offer some utility code to handle the boilerplate of parsing topics, routing messages and registering callbacks.

On a more positive note, I think this would be an excellent idea and a really nice feature (after all an SDK's reason-for-existence is to make developers lives easier). Just not one that should be baked directly into the network client API.

@jmklix jmklix added p2 This is a standard priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Aug 8, 2023
@dieter-aerit
Copy link
Author

@bretambrose Thanks for the info! Always interesting to understand the reasoning behind design choices.

I'm pretty new to MQTT and don't have any experience with other libraries, so I can't compare.
I just saw that the MQTT5 code was in developer preview and wanted to drop some feedback.
Feel free to do with this what you want!

@jmklix jmklix added p3 This is a minor priority issue and removed p2 This is a standard priority issue labels Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. p3 This is a minor priority issue
Projects
None yet
Development

No branches or pull requests

3 participants