arduino-libraries / ArduinoMqttClient

ArduinoMqttClient Library for Arduino
GNU Lesser General Public License v2.1
192 stars 75 forks source link

Implementing QoS 1 #39

Open jtsse opened 4 years ago

jtsse commented 4 years ago

I try to use QoS 1 which fortunately is already part of the library, but I am not really sure how to implement some things. because the way I approach the things at the moment seems to lead to issues.

For example, if I send a message like this:

mqttClient.beginMessage("/devices/" + deviceID + "/events", false, 1, false);
mqttClient.print("Hello world!");
mqttClient.end();

I see that the end() function is polling until a PUBACK is received or until the defined connectionTimeout is elapsed. Is it true that the library does not send automatically a new PUBLISH message after a while if there is no PUBACK received?

If so, how can I implement this in a good way? Is for example the approach below good enough? Since the end() function seems to return 0 if no PUBACK is received after the defined connectionTimeout.

int messageArrived = false;
while (!messageArived) {
  mqttClient.beginMessage("/devices/" + deviceID + "/events", false, 1, false);
  mqttClient.print("Hello world!");
  mqttClient.end();
}

Of course, I have to close the connection after a while if I don't get any response, but that is another subject.

I am wondering because I am not sure if all variables are set to the right ones if we call the function again without having end() returning 0.

The same problem occurs with receiving messages. If the callback function is called it is receiving data until _rxLength is zero. But, let's say that we have received a message of 1000 bytes and after 600 bytes we are not receiving the last 400 bytes anymore. It seems that clientTimedRead() returns -1. Therefore the client does not send back a PUBACK message. My idea was to create a loop that is looping until a PUBACK is sent (so that we don't get data from the broker again). I did this by creating a global variable:

volatile bool messageNotReceived= true;

The loop that is looping is:

MQTTClient.onMessage(receivedMessage);

while (messageNotReceived) {
  mqttClient.poll();
}

Where the callback function is defined as:

static void receivedMessage(void)
{
  int rd = 0;

  while (mqttClient.available()) {
    rd = mqttClient.read();
  }
  messageNotReceived = false;
}

But when I did this I saw that the while loop is running forever, because available() keeps returning 400. So I had to think about a way to leave the while loop and wait till a new Message was received from the broker. Therefore all the values should be the same as before to make sure that the new incoming data is handled fine.

static void receivedMessage(void)
{
  int rd = 0;

  while (mqttClient.available()) {
    rd = mqttClient.read();

    if (rd == -1) {
      while(mqttClient.parseMessage());
      return;
    }

  }
  messageNotReceived = false;
}

I saw that the parseMessage() function sets _rxLength to zero when I implemented it as above. The code above runs fine for a while. I get new data as long as I do not send a PUBACK. But after some time it seems to go wrong. Although the connection with the broker and my provider is still alive (I keep sending messages in the mean time), I got an error reading the data (I use the NB library, so after a while if the library calls AT+USORD to read messages, I got an error). Something says me that calling just parseMessage() is not enough to set the values to the right values before reading a new incoming message. Does someone know the right routine for this?