application controlled acknowledgements to match Java behaviour

[petersilva]

The existing python library acknowledges all messages received after calling the on_message() callback configured by the application. This PR adds manual_acks=False as an argument to client.py/init() function. When manual_acks=True, the application must acknowledge receipt of every message using the newly created ack(mid,qos) entry point.

• Closes Prevent ack of received message until final #348

This PR addresses issue #348 to allow applications to delay acknowledgement until it has completely received it. There was a first PR (#554) submitted two years ago. and got some great feeback from it:

• wanted QoS==2 support (the PR addressed only QoS==1)
• worry about reception order (is the library responsible for ensuring that acknowledgements are sent in the order the messages were received.)

After much too and fro, it was fairly clear that the library cannot be responsible for ordering. Then it occurred to me, that the Java implementation exists, and the equivalent functionality has been present for a number of years, and so chances are that it is doing "the right thing" tm.

So this PR uses the java implementation as a model, and slavishly implements the same thing in Python.

To illustrate the relationship to Java implementation, we can compare some tidbits:

So in python we have:

in _handle_publish(self):
     .
     .
     .
      elif message.qos == 1:
            self._handle_on_message(message)
            if self._manual_ack:
                return MQTT_ERR_SUCCESS
            else:
                return self._send_puback(message.mid)

vs. Java:

in:     private void handleMessage(MqttPublish publishMessage)

                  // If we are not in manual ACK mode:
                if (!this.manualAcks && publishMessage.getMessage().getQos() == 1) {
                        this.clientComms.internalSend(new MqttPubAck(MqttReturnCode.RETURN_CODE_SUCCESS,
                                        publishMessage.getMessageId(), new MqttProperties()),
                                        new MqttToken(clientComms.getClient().getClientId()));
                }

There is a need for an implementation of a call to be used by the library user to actually send the acknowledgements. we can compare code here also:

python:

    def ack( self, mid, qos ):
        """
           send an acknowledgement for a given message id. (stored in message.mid )
           only useful in QoS=1 and auto_ack=False
        """
        if self._manual_ack :
            if qos == 1:
                return self._send_puback(mid)
            elif qos == 2:
                return self._send_pubcomp(mid)

        return MQTT_ERR_SUCCESS

versus Java:

       public void messageArrivedComplete(int messageId, int qos) throws MqttException {
                if (qos == 1) {
                        this.clientComms.internalSend(
                                        new MqttPubAck(MqttReturnCode.RETURN_CODE_SUCCESS, messageId, new MqttProperties()),
                                        new MqttToken(clientComms.getClient().getClientId()));
                } else if (qos == 2) {
                        this.clientComms.deliveryComplete(messageId);
                        MqttPubComp pubComp = new MqttPubComp(MqttReturnCode.RETURN_CODE_SUCCESS, messageId, new MqttProperties());
                        // @TRACE 723=Creating MqttPubComp due to manual ACK: {0}
                        log.info(CLASS_NAME, "messageArrivedComplete", "723", new Object[] { pubComp.toString() });

                        this.clientComms.internalSend(pubComp, new MqttToken(clientComms.getClient().getClientId()));
                }
        }

In the java, there was an existing messageArrivedCompleted entry point to adjust. In python, nothing similar was obvious, so modified _handrel which would only apply in QoS==2... to suppress sending of pubcomp when manual_acks is active.

[petersilva]

@ralight not trying to be pushy... but you were very helpful with my last PR (#554) I thought a mild poke might be reasonable ?

[petersilva]

@aks @cclauss this PR is actually one I care about... nobody seems to be looking at it. I added the closes magic phrase once I learned it from the other PR.

[petersilva]

@BBBSnowball @HaraldGustafsson Does this look like it solves the problem? Perhaps try out the branch and confirm if it works for you?

[akx]

@PierreF Ping – you recently closed #348, mind taking a look at this?

[PierreF]

Could you also add a Signed-off to your commit ? This is a requirement from Eclipse to acknowledge you agree with the ECA.

[petersilva]

all good improvments. Thanks @PierreF !

[PierreF]

Thank for your contribution.

[petersilva]

Thanks! I think a lot of people are helped by this new function.
I know committters on this repo are overloaded, but it isn't obvious how to help.
If committers want some help, I would be willing, but I don't see how to get sufficient permission to be helpful.