-
Notifications
You must be signed in to change notification settings - Fork 30
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
Allow consumers to acknowledge received messages themselves #368
Comments
This may be like the fourth or so request we've had for this. I'm sympathetic to the idea of adding bridging support, but have some reservations, primarily with respect to the 311 client. The 311 client is a brittle, difficult-to-maintain shared-state-with-locking code base. New features are only added when absolutely necessary and I would best describe my feelings when going about doing such work as "reluctance tinged with a hint of dread." Making things worse, the publish received callbacks in 311 are not structured in a way that would allow us to add bridging in a backwards compatible manner. The callbacks take the topic and payload as parameters; bridging support would require an entirely new callback type signature which in turn would require us to support multiple different callback signatures as well as expose controls to the user for selection. My desire to do this is pretty low, to put it mildly. On the other hand, the 5 client's callbacks are all structure-wrapped, both at the native level and in all the CRT libraries that bind the client out to other languages. Adding bridging support to the 5 client (and all of its various bindings) would not break existing users nor would it require a new callback type + controls. The downside is that the 5 client is relatively new and it's likely that existing users interested in this functionality may be using the 311 client and have no desire to upgrade because the two clients have very different public APIs (intentional, as the 311 API contains a number of frustrating design mistakes). Regardless of which clients we add it to, I could see bridging (if we add it) working as follows: The publish received callbacks include an additional field/parameter that is an opaque "factory"-like object with a single API that one-time-only returns an AckInvoker object. The client checks the state of the factory after the callback returns and sends the puback if the AckInvoker wasn't created. The AckInvoker is a first-class object that supports a one-time call that will send the underlying puback for the publish received; users are free to pass it around and keep it as long as necessary. It would have a strong reference to the MQTT client, so failing to use it properly would cause a reference/memory leak. While an AckInvoker for a particular packet id was outstanding, no further ones would be created for that packet id (IoT Core will resend publishes that don't get acked within certain intervals and having multiple AckInvokers alive that refer to the same packet id could lead to all sorts of bad behavior). |
Surely it would be reasonable to add new features like this to the new client only. (FWIW I'm using the 5 client only.) |
No timeframe or commitment atm, but this request was well-received by the team and I added a backlog entry for it. |
Describe the feature
This is #130 as a feature request.
This client library automatically PUBACKs received messages, not allowing consumers (handlers, implementers of
on_publish_received
) to control ackwowledgement themselves. This shortcoming makes setting QoS 1 on messages published by the server seem mostly pointless. If the client is doing anything useful with the message, there's a chance it will fail.The current workaround is to do these acknowledgements of success at the application level, which is a shame when it's a feature of the protocol.
Use Case
By @aquark
Proposed Solution
By @aquark
Other Information
No response
Acknowledgements
The text was updated successfully, but these errors were encountered: