Universal-Arduino-Telegram-Bot

Universal-Arduino-Telegram-Bot copied to clipboard

Issue:

Sending messages longer than maxMessageLength defined in UniversalTelegramBot.h sends the device into an endless polling loop where it polls the same message Id till infinity.

Proposed Fix: Cut off message and terminate at maxMessageLength in UniversalTelegramBot.cpp::sendGetToTelegram:

if (mess.length() >= maxMessageLength)
{
	mess = mess.substring(0,maxMessageLength-5);
	mess = mess + "\"\}\}\]\}";
}

For full code see attached modified cpp. UniversalTelegramBot.cpp.txt

Thanks for raising the issue.

Interesting solution to the problem, but you could not guarantee you would be at that level in the jsonArray.

Maybe counting the { versus the } would be one way to figure it out.

I think the max message length can be bumped much higher than it is. I made a call to the reddit api before on the esp8266 and it could handle a much longer response than that.

Increasing the JsonBuffer length increases processing time, which might be undesirable. I absolutely agree forcibly terminating a message is not the best solution (and the way I did it is unflexible), but it's probably better than an infite loop or skipping this message (by incrementing message_id). Dynamically counting/closing the Array is probably better. Note, that there are {} and [].

Increasing the message length shouldn't slow down processing time (well other than when the message is longer than your old max). Its just a random enough variable inherited from on of the other libraries this was based on.

Yeah for sure we need to do something about it, entering loops is the worst case scenario. I think maybe a combination of both might work well.

I would like to make message length configurable, so adding the Json closer first might be a good idea

I'll take a look this week if I get time.

On 13 Feb 2017 16:30, "NDscham" [email protected] wrote:

Increasing the JsonBuffer length increases processing time, which might be undesirable. I absolutely agree forcibly terminating a message is not the best solution (and the way I did it is unflexible), but it's probably better than an infite loop or skipping this message (by incrementing message_id). Dynamically counting/closing the Array is probably better. Note, that there are {} and [].

— 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/38#issuecomment-279443770, or mute the thread https://github.com/notifications/unsubscribe-auth/ABfXwnBx4EG14SPL3el5BTQiuj8foE45ks5rcIUUgaJpZM4L_UwN .

Awesome, thanks! I could look into it as well, although I am not really familiar with JSON.

One possible solution to these problems is to return -1 as the message number in case the json decoding fails. If you do a getUpdates?offset=-1 it'll flush the queue skipping the message. The problem with raising the buffer size is that'll be always a message that's bigger...so it won't be a complete solution. In many applications loosing a message is better than an infinite loop.

Probably I found solution. Function int UniversalTelegramBot::getUpdates(long offset) in some cases returns undefined result due to bug.

As you see, the last return might never called.

if (response != "") { // ..................... code here return 0; } }

i've experienced a similar loop. i'm unsure if this is the same issue as @NDscham described.

when my esp8266 tries to send a long message, he sends the same message about 5 times. after sendMessage(), the printed "response" was cut. only the last ~100-300 characters of the response were received/printed. and thus checkForOkResponse() failed to find {"ok":true this resulted in a loop inside sendPostMessage() which sends the same message about 5 times. basically this code inside sendPostMessage():

while (millis() < sttime+8000) { // loop for a while to send the message
      [...]
      sent = checkForOkResponse(response);
      if (sent) {
        break;
      }
    }

i don't know why "response" gets cut.

and a possible fix could be to automatically split long messages into multiple smaller ones, but that's not the best solution i believe.

I would love to see an automatic cut at the defined maximum message length very much.

I use the library for simple automation tasks, but recently got spammed by - I guess - other bots and due to my lack of skill to skip those messages, my program didn't work any more until I increased the maximum message size and read all those unwanted messages to clear the message queue

And thinking it further, a flushMessageQueue action would be useful - I'd call it after boot to make sure my Arduino can start with a clean slate. Unfortunately I am no C-Programmer, otherwise I'd try to implement it myself

@springm you can do this with bot.getUpdates if you just pass in a large number, it will basically mark all messages that telegram currently knows about with a smaller message id as processed. At least I think that should work

On Tue, 3 Apr 2018 20:04 springm, [email protected] wrote:

And thinking it further, a flushMessageQueue action would be useful - I'd call it after boot to make sure my Arduino can start with a clean slate. Unfortunately I am no C-Programmer, otherwise I'd try to implement it myself

— 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/38#issuecomment-378361666, or mute the thread https://github.com/notifications/unsubscribe-auth/ABfXwsRix3QjZCMLMruynszl6zmT5_Fpks5tk8fZgaJpZM4L_UwN .

@springm To automatic prevent infinite loop, in UniversalTelegramBot.cpp if you modify UniversalTelegramBot::getUpdates adding:

if (response.length() >= maxMessageLength) { 
    last_message_received = -2;
    closeClient();
    return 0;
}

In the next bot.getUpdates() since are you going to add +1 to bot.last_message_received variable that is set to -2, it will become equal to -1. When the update_id is -1 Telegram server returns the last message received. So if there is another message after the long one, then it will read it instead of get stuck in infinite loop.