Added optional onDelivery callback for publishing messages with QoS 1 and 2.

[slaff]

Useful if you want to publish a message, make sure that it was delivered and execute a callback once the message was delivered.

[slaff]

Tested on real device and ready to use.

[hreintke]

@slaff : Nice extension.
My remarks :

• By using bool MqttClient::publishWithQoS(String topic, String message, int QoS, bool retained /* = false*/, MqttMessageDeliveredCallback onDelivery /* = NULL */) you are forcing to set the optional parameter retained. Maybe better to overload with:
  bool MqttClient::publishWithQoS(String topic, String message, int QoS, MqttMessageDeliveredCallback onDelivery, bool retained /* = false*/)
• What is the reason for keeping the delegate with each messageID in progress ?
• In combination with above (and I know we missed a part in the initial callback interface)
  For classes implementing callback/delegates we try to always implement also a f.e. mqttClient.setMqttMessageDeliveredCallback(...) Can you add that and the take the setMqttStringSubscriptionCallback also ?

[slaff]

@hreintke Thanks for the remarks

• Maybe better to overload with ...

The main reason is backwards compatability. If I change the function signature the way you suggest that will break compatibility with already existing code and Sming users may not like it.

• What is the reason for keeping the delegate with each messageID in progress ?

I am not sure if I understood you but there has to be a way to figure out whether a message was confirmed as delivered from the other side ( server ). Not every published message has delegates attached. Only the message ids for which the developer explicitly requested onDelivery callback.
And the reason for having onDelivery callback for a message is that a developer may want to be sure that a message was delivered to the other side(server) and that the other side(server) confirmed that. Thus the developer can be 100% sure that a message was delivered and be able to put the device in sleep or do something else.

• qttClient.setMqttMessageDeliveredCallback(...)

Again I am not sure if I get the reason for this?! The onDelivery callback is per message! Not for the whole MqttClient. The use-case for that is that the developer can track the success of some messages. Those that she/he defines as important and requests onDelivery callback. Not all.
Therefore my question is what is the use case for having setMqttMessageDeliveredCallback ?

[hreintke]

@slaff : Sorry for later reply. I have been busy last days.

Overload :
With overloading in C++ you have the same function name, different parameters -> full backward compatibility.

Delegates per msg.
Missed that detail in your implementation.
All the callbacks within Sming are server/client based. I think it is better to keep this the same for this delegate implementation, The earlier remarks are based on this.

[slaff]

With overloading in C++ you have the same function name, different parameters

@hreintke Agreed.

... I think it is better to keep this the same for this delegate implementation

Hm... Think about the following use-case.

1. A device has to send important message.
2. The device runs on batteries and has to shut itself down immediately after it receives confirmation from the other side that the message was delivered.

If I add a callback that is for a general callback for all delivered messages then outside of the MqttClient class the developer has to implement on his own a logic that reads the msgId and then adds a check in the onDeliveredAllMessages callback that compares the msgId that he/she is interested in with the one that is delivered. Which is doable but it would be much better if the class MqttClient already has this built-in. The onDeliveredMessage callback per message has encapsulated all the logic and the developer needs without forcing him to know the details of Mqtt communication.

[hreintke]

@slaff :
There is already a callback implemented for all incoming messages, see the use of MqttStringSubscriptionCallback (wrong wording I know).
My proposal is to have the functionality you describe, remember the msgid's for qos messages,within the mqttclient class..
Difference is that your implementation uses a callback delegate for each msgid while mine is one mqtt callback, used for all msgid's which requested the callback.

The delegates within other parts of sming use the same methodology : there are different delegates for different functionality, but all are defined on "class level" and not on "message level"

One other Mqtt related question.
I read the following on qos2 :

Did we implemented qos 2 correctly and is your delegate called when PUBCOMP is received ?

[slaff]

Did we implemented qos 2

For QoS 1 the server returns PUBACK message as confirmation. For QoS 2 returns PUBREC message as confirmation and then the client has to do something more. For the onDelivery we are interested ONLY in confirmation from the server. And both QoS 1 and 2 are handled in the code.

else if (type == MQTT_MSG_PUBACK || type == MQTT_MSG_PUBREC) {

My proposal is to have the functionality you describe, remember the msgid's for qos messages,within the mqttclient class..

Please, send me some sample code to understand your implementation for the above use-case.