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

[brad112358]

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]

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]

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]

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]

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]

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

[brad112358]

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 <#70 (comment)>: > @@ -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 <#70 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABKQOEPVZSOJUWAWQFSVYOTXAWGYJANCNFSM6AAAAAAWYFZWFQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[brad112358]

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]

MANY THX :)