Universal-Arduino-Telegram-Bot icon indicating copy to clipboard operation
Universal-Arduino-Telegram-Bot copied to clipboard

ESP32 will send duplicate messages.

Open JacoFourie opened this issue 7 years ago • 15 comments

Hi all.

So I have been testing the port of the ESP32. It works well except I will get duplicate messages at random. I switched on debugging to get the message info. I also logged a support call with Telegram. I gave them the message id when I receive duplicates and they told me that the messages is sent more than one.

I will send the message only once on my end as I can see that in the serial feed.

with the debug switched on I will see 2 outputs for every message sent.

Like this.

SEND Post Message
[BOT Client]Connecting to server

{"ok":true,"result":{"message_id":2750,"from":{"id":yyyyyy,"is_bot":true,"first_name":"Open Automation Alerts","username":"OpenAutomationBot"},"chat":{"id":xxxxxxxx,"first_name":"Jaco","last_name":"Fourie","type":"private"},"date":1519390520,"text":"Web Test"}}

{"ok":true,"result":{"message_id":2750,"from":{"id":yyyyyyy,"is_bot":true,"first_name":"Open Automation Alerts","username":"OpenAutomationBot"},"chat":{"id":xxxxxxx,"first_name":"Jaco","last_name":"Fourie","type":"private"},"date":1519390520,"text":"Web Test"}}
Closing client

But yet I will get duplicates from time to time like this one .

photo_2018-02-23_14-57-43

Telegram checked on their end with the message ID and they say it is being sent more than once.

"Our devs have been checking it, there is nothing wrong on our side, somehow your code is sending the message several times :("

Do anybody else see this ? I can see I only fire the code once as the debug info only shows once.

JacoFourie avatar Feb 23 '18 13:02 JacoFourie

OK so I seem to have found where it is coming from.

Why is there a while loop here ?

if (payload.containsKey("text")) {
    while (millis() < sttime+8000) { // loop for a while to send the message
      String command = "bot"+_token+"/sendMessage";
	  
          //Added this to test
	  Serial.print("The Command is : ");
	  Serial.println(command);
	  
      String response = sendPostToTelegram(command, payload);
      if (_debug) Serial.println(response);
      sent = checkForOkResponse(response);
      if (sent) {
        break;
      }
    }
  }

Sometimes this loop will cause the command to fire more than once as can be seen here.

SEND Post Message
The Command is : botxxxxxxxxxxxx/sendMessage
[BOT Client]Connecting to server

The Command is : botxxxxxxxx/sendMessage

The Command is : botxxxxxxxx/sendMessage

{"ok":true,"result":{"message_id":2791,"from":{"id":xxxxxx,"is_bot":true,"first_name":"Open Automation Alerts","username":"OpenAutomationBot"},"chat":{"id":xxxxxxxx,"first_name":"Jaco","last_name":"Fourie","type":"private"},"date":1519413626,"text":"Lets test the Bot !!"}}

{"ok":true,"result":{"message_id":2791,"from":{"id":xxxxxxxx,"is_bot":true,"first_name":"Open Automation Alerts","username":"OpenAutomationBot"},"chat":{"id":xxxxxxxxxxx,"first_name":"Jaco","last_name":"Fourie","type":"private"},"date":1519413626,"text":"Lets test the Bot !!"}}
Closing client

JacoFourie avatar Feb 23 '18 19:02 JacoFourie

OK it seems that if the network or something is busy that code will fire in a loop and create duplicate messages. I managed to get it firing 3 times now. I assume that if you do not get a response of good you will sit in the loop until you do get one.

JacoFourie avatar Feb 23 '18 19:02 JacoFourie

I have commented out the while loop and will let it run now to see if I still get the duplicates. I guess the loop is there to make sure the message is sent. But it seems that the response is showing it did not send yet it did and then the loop will fire again.

JacoFourie avatar Feb 23 '18 19:02 JacoFourie

Thanks for the investigation, I will take a look this evening

On 23 Feb 2018 19:41, "JacoFourie" [email protected] wrote:

I have commented out the while loop and will let it run now to see if I still get the duplicates. I guess the loop is there to make sure the message is sent. But it seems that the response is showing it did not sent yet it did and then the loop will fire again.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/witnessmenow/Universal-Arduino-Telegram-Bot/issues/74#issuecomment-368117858, or mute the thread https://github.com/notifications/unsubscribe-auth/ABfXwgboDuTaKrW4ASw_Myt3ALxUTW4jks5tXxRpgaJpZM4SQ2Wr .

witnessmenow avatar Feb 23 '18 20:02 witnessmenow

OK so the code has been running 9.5 hours. It will send a message every 2 minutes to test. I did not get any duplicates. But I did loose one message. So I will see if I can find a way to make sure the message was delivered but not to send duplicates.

missing

JacoFourie avatar Feb 24 '18 05:02 JacoFourie

OK I see what is going on here. The response while loop will return sometimes with no response and then the send loop will send again.

So this loop needs to wait longer for a response.

	while (millis()-now < 1500) {
			while (client->available()) {
				char c = client->read();
				responseReceived=true;


        if(!finishedHeaders){
          if (currentLineIsBlank && c == '\n') {
            finishedHeaders = true;
          }
          else {
            headers = headers + c;
          }
        } else {
          if (ch_count < maxMessageLength) {
            body=body+c;
            ch_count++;
  				}
        }

        if (c == '\n') {
          currentLineIsBlank = true;
        }else if (c != '\r') {
          currentLineIsBlank = false;
        }

			}

JacoFourie avatar Feb 24 '18 06:02 JacoFourie

I changed the code to look like this. Will let it run and check if I see duplicates or missing messages.

while (millis()-now < 4500 && !responseReceived) {

Maybe that value in the while loop should be a variable so you can set the time to wait longer for networks that are slower around the world.

while (millis()-now < timeout && !responseReceived) {

JacoFourie avatar Feb 24 '18 06:02 JacoFourie

Great to hear you fixed the issue!

That's a really good test it seems very stable!

Just off the top of my head the it should return a false if it fails, that wait time is just to check the response from telegram to see if it sent OK, it wouldn't have any impact on the message actually sending as you could technically not wait for a response at all, you just wouldn't know if it sent.

On 24 Feb 2018 06:32, "JacoFourie" [email protected] wrote:

I changed the code to look like this. Will let it run and check if I see duplicates or missing messages.

while (millis()-now < 4500 && !responseReceived) {

Maybe that value in the while loop should be a variable so you can set the time to wait longer for networks that are slower around the world.

while (millis()-now < timeout && !responseReceived) {

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/witnessmenow/Universal-Arduino-Telegram-Bot/issues/74#issuecomment-368204803, or mute the thread https://github.com/notifications/unsubscribe-auth/ABfXwhMAYxtfzaXQx7fNnTZORg5M0V0qks5tX6zogaJpZM4SQ2Wr .

witnessmenow avatar Feb 24 '18 08:02 witnessmenow

So it seems my timeout was too low. The server sometimes takes its time to send a response and then the send code will fire again as it did not get the OK resonse back in time. I now made the timeout value 9000 millis and all seems fine now.

I think the code should be made that you give it a timeout value in the constructor. It can have a default value of 1500 but it should be configurable. On both the send and receive side.

JacoFourie avatar Feb 24 '18 08:02 JacoFourie

I let it run for 190 minutes and all is OK now. I also did random tests where I would fluid the messages. I would count how many was sent and the correct number arrived on the other end.

correct

JacoFourie avatar Feb 24 '18 11:02 JacoFourie

So the code has been running for more than a day and I did not get any duplicates or loose any messages. ok

what I did was to add a timeout parameter to the constructor

and declared a var to hold the value. I also modified the code of the while loop to use the timeout value

//In the header file
UniversalTelegramBot (String token, Client &client , int timeout);

int send_timeout = 1500;

//In the cpp file
UniversalTelegramBot::UniversalTelegramBot(String token, Client &client , int timeout = 1500)	{
  _token = token;
  //Set the timeout value
  send_timeout = timeout;
  this->client = &client;
}

           //Wile loop of the send
	    while (millis()-now < send_timeout && !responseReceived) {
			while (client->available()) {
				char c = client->read();
				responseReceived=true;


        if(!finishedHeaders){
          if (currentLineIsBlank && c == '\n') {
            finishedHeaders = true;
          }
          else {
            headers = headers + c;
          }
        } else {
          if (ch_count < maxMessageLength) {
            body=body+c;
            ch_count++;
  				}
        }

        if (c == '\n') {
          currentLineIsBlank = true;
        }else if (c != '\r') {
          currentLineIsBlank = false;
        }

			}

      //if (responseReceived) {
        //if (_debug) {
        //  Serial.println();
        //  Serial.println(body);
        //  Serial.println();
        //}
  		//	break;
  		//}
		}


the

JacoFourie avatar Feb 26 '18 08:02 JacoFourie

Is this solution part of the latest release? I happen to have the same problem and cant follow your code examples. I know which part of the cpp file you are reffering to at https://github.com/witnessmenow/Universal-Arduino-Telegram-Bot/issues/74#issuecomment-368113549. But I dont understand what you did at https://github.com/witnessmenow/Universal-Arduino-Telegram-Bot/issues/74#issuecomment-368428472 where you posted the final solution. Thank you

usab avatar Nov 11 '18 04:11 usab

Not sure if my fix was made part of the current version. I have been using it for months now and it works 100% with the fix I put in.

JacoFourie avatar Nov 11 '18 20:11 JacoFourie

Check the waitForResponse variable, the default value is 1500. Use a higher value to fix the issue, no code changes in the library is needed. E.g.: bot.waitForResponse = 9000; (configuration done like the longPoll option)

kingsumos avatar Jul 07 '19 17:07 kingsumos

I found out that using an external antenna on the esp32 when wifi reception is low, that helps also with getting more stable responses...

fritsjan avatar Jul 29 '20 07:07 fritsjan