olliw42 / mLRS

2.4 GHz & 915/868 MHz & 433 MHz/70 cm LoRa based telemetry and radio link for remote controlled vehicles
GNU General Public License v3.0
295 stars 67 forks source link

Don't send so many very short UDP messages to avoid message loss #70

Closed brad112358 closed 1 year ago

brad112358 commented 1 year ago

With QGround control, on both Linux and Android, I got lots of lost MAVLink messages without this change. When I checked, I saw that most UDP packets contained only a few bytes (1-3) of payload. I don't know which end the messages were being lost on.

The intent of this change is to wait for more bytes to be received on the serial link before sending a UDP packet, but still constrain the latency to a reasonable 5ms.

olliw42 commented 1 year ago

great, this makes lots of sense I guess the 30 and 5ms has worked out well experimentally, couldn't this be increased to 10ms or even 20ms? could you also test with MissionPlanner? (not asking for, I'm just curious)

brad112358 commented 1 year ago

I haven't tested with mission planner yet, but I will let you know when I do. 30 and 5 was the first thing I tried and it helped so much I didn't think I needed to try others. But, after I submitted this PR, I discovered a few messages were still being lost so I have already increased the delay and message size which helped even more. I'll update this PR later.

I also have another idea for a heuristic which wouldn't add latency as much as this does: mLRS sends a mavlink message at a time in a burst, so we could actually wait for the break in serial reception, when the receive size stops growing for a ms or so, then send the packet. This should be especially effective at a slightly higher baud rate.

olliw42 commented 1 year ago

I'm really not concerned about 20ms vs 5ms in other applications of mine, like the uavcan mavlik tunnel I did many years back, few 10-ish ms worked great anyway, put in what you think is the best, it always can be further improved later

brad112358 commented 1 year ago

I did code up the heuristic to align UDP payload with MAVLink messages and it works quite well, allowing wireshark to decode mavlink messages and further limiting UDP message rate without much latency. I think I'll wait for your answer on why you don't like static locals before I push it.

olliw42 commented 1 year ago

I think we should first finish this up before attempting anything more difficult/else.

brad112358 commented 1 year ago

I'm sorry if I've misunderstood what you wrote. My goal really is that you shouldn't feel the need to rework everything I submit.

On Tue, Apr 11, 2023, 12:16 PM olliw42 @.***> wrote:

@.**** commented on this pull request.

In esp/mlrs-wifi-bridge/mlrs-wifi-bridge.ino https://github.com/olliw42/mLRS/pull/70#discussion_r1163118097:

@@ -169,7 +169,8 @@ void setup()

void loop() {

  • int tnow_ms = millis();
  • unsigned int tnow_ms = millis();
  • static unsigned int last_send = 0;

how does this "I myself have spend too much time and have seen others spending way too much time" and "It seems you have been burned" goes together ... how does it come that I feel my words are not read anyway ... we can continue this forever, and for once in time I tried to avoid that, yet here we are again, because, frankly, you insisted ... I will not ask for changes anymore at all, my fault.

pl do the changes you like to do, so we can move on.

— Reply to this email directly, view it on GitHub https://github.com/olliw42/mLRS/pull/70#discussion_r1163118097, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKQOEPVZSOJUWAWQFSVYOTXAWGYJANCNFSM6AAAAAAWYFZWFQ . You are receiving this because you authored the thread.Message ID: @.***>

brad112358 commented 1 year ago

I did test mission planner on Windows 10 both with and without this change at 19Hz. Both worked fine with no sign of dropped messages. I'm not sure why QGC drops so many messages on Linux and Android with the original code.

Have you tested UDP bridge yet with Android?

olliw42 commented 1 year ago

MANY THX :)