Generalise calback system?

[davydnorris]

Hi all,

Firstly many thanks for this library - I'm working heavily with the ESP8266 NonOS SDK and have been looking for a replacement MQTT library due to several problems with the current one. The ESP folks have rewritten the MQTT in their RTOS SDK but have made some weird decisions around the use of blocking socket calls and other things :-(

This library has a lot of potential to replace the current library for both RTOS and NonOS work, in particular because of the use of non-blocking/polling for the networking and the ability to compile with mbedTLS.

The one thing the current ESP MQTT library has that this one doesn't, and which would be super useful, are more callback points. Both libraries do it with separate functions, but it would actually be very simple to do it with a single callback that posted an event. The event points that I've found to be particularly useful are:

CONNECT - called when connect successful and complete
DISCONNECT - called when disconnect occurs with reason (client/server/heartbeat error/...)
PUBLISH - called when publish is successful (any QoS) with message id
RECV - called when data received with topic and message
ERROR - called when an error condition is detected with reason (publish timeout/expire, reconnect fail/...)

I know you already have several of these as individual callbacks, but would you consider implementing an event system like this in your code using a single generalised callback? An event driven approach like this is a lot more important when you're using limited resource devices and especially when you're using a NonOS SDK or even a pseudo RTOS where non blocking code is critical (even more so when sensors are being read), and it's compatible with your current library approach - instead of calling the individual functions, you call one with an event type and the same payload.

The other thing I was implementing in my own library was a single inbox and outbox, managed via bipbuffers - all outgoing messages go into the outbox until their publish is successful or they expire, and all incoming topic receives go directly into the inbox. Clients are responsible for clearing the specific messages from the inbox and outbox once they get the event callback, and the contents of the inbox and outbox can either be stored directly or saved to storage if the device should be powered down or slept.

[LiamBindle]

I'd support this. Would a single generalized callback be preferred or 5 separate callbacks? I think 5 separate callbacks might fit better in line with the current approach in MQTT-C (i.e., it wouldn't introduce new events that trigger people's exiting callbacks). Also a single generalized callback might encourage people to put quite a bit of logic in that callback--increasing the likelyhood that people make mqtt_xxx() calls inside the callback which is not permitted (see #88).

[davydnorris]

I guess it doesn't really matter - the current espmqtt library is from tuanpm/espmqtt and he has separate callbacks, and I simply use a wrapper to convert the separate data structures into events and then call the same function in each. The idea of the event driven approach is that it's a single function with a specific event payload - this makes it a bit simpler in NonOS or RTOS environments where you are implementing the handling with task queues or similar. Then the callback function throws it onto an appropriate task queue with the event type as the id.

There are similar restrictions in the current library in terms of what you can and can't use, but not for every callback - it's only the receive callback that has that restriction from memory. Would that be the same as yours? As an example, in the connect, disconnect and error callbacks you can call mqtt functions to do things like set up your subscriptions or reconnect.

One difference in the espmqtt library is that it implements the send/recv loop internally - I like the fact that in yours I can actually potentially separate the send and receive portions so that only the receive is polled but the send is event driven. Then you only hit the send routines when needed.

Oh, and as an aside, the ESP8266 and ESP32 chips are all word aligned so they need the memory alignment as discussed in another issue, but their memory routines largely account for this - you just have to use a packed directive on the MQTT headers. The biggest problem the current MQTT libraries have is that they do a lot of data copying, which uses excess memory and takes time - this is why I was looking at implementing everything using in place buffers for the inbox and outbox. Means you have an extra call when sending messages to allocate space in the outbox (you'd just use alloc routines in something that's not memory limted), and the client is responsible for disposing of their inbox messages once read.

[LiamBindle]

I'm going to close this since it doesn't appear anyone has picked it up. Thanks for opening though! If there's a similar feature request in the future, we'll link to this and that will make it more likely someone picks it up.

Thanks again!

[perigoso]

I'm going to work on a feature like this, as It would be useful for my current project, If you could reopen the issue and assign it to me that would be nice

[LiamBindle]

There you go. Thanks for picking this up @perigoso!