Allow multiple UDP clients to connect/disconnect/reconnect #26163

bugobliterator · 2024-02-07T05:36:14Z

Tested on a variant of CubePilot CANMod. The issue was that we were using connected UDP Server socket. Meaning once we connect to an application, then we shutdown that application and then restart the application the listen port on the application can change. But the UDP Server will be stuck with old IP:Port and will not even listen to the new application.

This PR moves to using sendto command with IP:Port from last received packet.
This is the case for example with Mavproxy, or any other client where the data is sent without binding to specific port.

libraries/AP_Networking/AP_Networking_port.cpp

magicrub

This is a lot of code. I recently needed the same feature for the same reason and just commented out the locking.

bugobliterator · 2024-02-09T01:48:19Z

This is a lot of code. I recently needed the same feature for the same reason and just commented out the locking.

Most of the code is to do things right i.e. use uint32_t instead of string parsing at every packet.

libraries/AP_HAL/utility/Socket.cpp

libraries/AP_HAL/utility/packetise.cpp

libraries/AP_Networking/AP_Networking_port.cpp

libraries/AP_HAL/utility/packetise.cpp

libraries/AP_Networking/AP_Networking_port.cpp

tridge · 2024-06-19T09:21:23Z

libraries/AP_Networking/AP_Networking_port.cpp

+            last_udp_connect_port != 0) {
+            ret = sock->sendto(buf, n, last_udp_connect_address, last_udp_connect_port);
+        } else {
+            ret = sock->send(buf, n);


this send makes no sense without having received a pkt when we are a UDP server

made the condition more tied in with non-UDP_SERVER sends. And added comments for more readability

peterbarker

Add timeout? Would stop the bouncing back-and-forwards between two clients problem.

Maybe a 3 second timeout to then allow bouncing

peterbarker · 2024-06-19T09:20:52Z

libraries/AP_HAL/utility/packetise.h

+#include <AP_HAL/AP_HAL.h>
+#include <GCS_MAVLink/GCS_config.h>


These includes don't look right - don't need GCS_config and probably want AP_HAL_/AP_HAL_Boards.h for the other

It needs to be GCS_Config, the reason is due to the include pattern being:

GCS_Config -- includes --> AP_HAL/AP_HAL_Boards.h

HAL_ENABLE_MAVLINK_PACKETISE is defined in AP_HAL_Boards.h but relies on the definition of HAL_GCS_ENABLED which is defined in GCS_Config.h

The solution I think might be to move HAL_GCS_ENABLED one layer up into AP_HAL_Boards.h ?

.... but nothing in this header file is using anything related to either of those headers.

Heck, you could probably just include stdint.h and add class in front of bytebuffer and need no other include.

... actually, now I see the thing.

You're not actually using HAL_ENABLE_MAVLINK_PACKETISE in the header.

Perhaps rename to AP_MAVLINK_PACKETISE_ENABLED, move the definition of that into this header inside some infdefs and include GCS_config.h and whatever else is required to decide on whether to enable the feature...

ok, done. please check

peterbarker · 2024-06-19T09:22:04Z

libraries/AP_Networking/AP_Networking_port.cpp

+            last_udp_connect_port != 0) {
+            ret = sock->sendto(buf, n, last_udp_connect_address, last_udp_connect_port);
+        } else {
+            ret = sock->send(buf, n);


Work out whether this line is actually sensible; this only makes sense if the socket has been connect()'d somewhere. Perhaps we can't actually cross this line?

made the condition more tied in with non-UDP_SERVER sends. And added comments for more readability

peterbarker · 2024-06-20T06:00:25Z

libraries/AP_HAL/utility/packetise.h

+#include <AP_HAL/AP_HAL.h>
+#include <GCS_MAVLink/GCS_config.h>
+
+#ifndef AP_MAVLINK_PACKETISE_ENABLE


Suggested change

#ifndef AP_MAVLINK_PACKETISE_ENABLE

#ifndef AP_MAVLINK_PACKETISE_ENABLED

It's a niggle but also what we're trying to standardise on

... also, do you still need HAL.h?

…eceived packet

…ame run

tridge

Thanks Sid!

Stale

bugobliterator added DevCallEU Networking labels Feb 7, 2024

bugobliterator force-pushed the pr-udp-multiple-clients branch from 09c079d to 1f722c0 Compare February 7, 2024 07:07

tridge requested changes Feb 7, 2024

View reviewed changes

libraries/AP_Networking/AP_Networking_port.cpp Outdated Show resolved Hide resolved

tridge removed the DevCallEU label Feb 7, 2024

bugobliterator force-pushed the pr-udp-multiple-clients branch 2 times, most recently from 4a15a6a to d7b9839 Compare February 8, 2024 02:02

magicrub reviewed Feb 8, 2024

View reviewed changes

bugobliterator force-pushed the pr-udp-multiple-clients branch from d7b9839 to 410976b Compare February 9, 2024 01:57

bugobliterator requested a review from tridge February 9, 2024 02:00

bugobliterator force-pushed the pr-udp-multiple-clients branch from 410976b to 947a3a8 Compare February 11, 2024 23:25

bugobliterator added the DevCallEU label Feb 14, 2024

tridge requested changes Feb 14, 2024

View reviewed changes

tridge removed the DevCallEU label Feb 14, 2024

bugobliterator force-pushed the pr-udp-multiple-clients branch 2 times, most recently from 1df9242 to aba91f6 Compare February 14, 2024 11:09

bugobliterator requested a review from tridge February 14, 2024 11:10

magicrub previously requested changes Feb 16, 2024

View reviewed changes

libraries/AP_HAL/utility/packetise.cpp Outdated Show resolved Hide resolved

libraries/AP_Networking/AP_Networking_port.cpp Outdated Show resolved Hide resolved

libraries/AP_Networking/AP_Networking_port.cpp Outdated Show resolved Hide resolved

bugobliterator force-pushed the pr-udp-multiple-clients branch from 607ad24 to 00da51c Compare February 19, 2024 06:32

bugobliterator added the DevCallEU label Feb 21, 2024

tridge requested changes Feb 21, 2024

View reviewed changes

libraries/AP_Networking/AP_Networking_port.cpp Outdated Show resolved Hide resolved

tridge removed the DevCallEU label Feb 21, 2024

bugobliterator force-pushed the pr-udp-multiple-clients branch 2 times, most recently from 833ad7e to 86f60c2 Compare June 2, 2024 06:35

bugobliterator added the DevCallEU label Jun 5, 2024

bugobliterator force-pushed the pr-udp-multiple-clients branch from 86f60c2 to 39a4095 Compare June 18, 2024 06:33

tridge requested changes Jun 19, 2024

View reviewed changes

peterbarker reviewed Jun 19, 2024

View reviewed changes

bugobliterator force-pushed the pr-udp-multiple-clients branch from 39a4095 to d458cd3 Compare June 20, 2024 02:32

bugobliterator requested review from tridge and peterbarker June 20, 2024 04:31

bugobliterator force-pushed the pr-udp-multiple-clients branch from d458cd3 to 91de39a Compare June 20, 2024 05:52

peterbarker previously requested changes Jun 20, 2024

View reviewed changes

bugobliterator added 7 commits June 21, 2024 11:00

AP_HAL: add AP_MAVLINK_PACKETISE_ENABLE option

8cd1054

AP_HAL: make recv call to be undirected but also get the ip:port of r…

e5ae815

…eceived packet

AP_HAL: add methods that deal in ip address as uint32_t

6ae1020

AP_Networking: allow UDP Server to talk to different clients on the s…

6319902

…ame run

AP_Periph: do not close if already closed

a03920e

AP_Networking: use IP4_STR_LEN instead of value 16

f364f8e

AP_Networking: add timeout to swap the UDP Server client target

c2f7e2a

bugobliterator force-pushed the pr-udp-multiple-clients branch from 91de39a to c2f7e2a Compare June 21, 2024 01:00

bugobliterator requested a review from peterbarker June 21, 2024 01:32

tridge approved these changes Jun 21, 2024

View reviewed changes

tridge merged commit 96682b1 into ArduPilot:master Jun 21, 2024
93 checks passed

IamPete1 removed the DevCallEU label Jul 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow multiple UDP clients to connect/disconnect/reconnect #26163

Allow multiple UDP clients to connect/disconnect/reconnect #26163

bugobliterator commented Feb 7, 2024

magicrub left a comment

bugobliterator commented Feb 9, 2024 •

edited

Loading

tridge Jun 19, 2024

bugobliterator Jun 20, 2024

peterbarker left a comment

peterbarker Jun 19, 2024

bugobliterator Jun 20, 2024

peterbarker Jun 20, 2024

bugobliterator Jun 20, 2024

peterbarker Jun 19, 2024

bugobliterator Jun 20, 2024

peterbarker Jun 20, 2024

bugobliterator Jun 21, 2024

tridge left a comment

		#include <AP_HAL/AP_HAL.h>
		#include <GCS_MAVLink/GCS_config.h>

	#ifndef AP_MAVLINK_PACKETISE_ENABLE
	#ifndef AP_MAVLINK_PACKETISE_ENABLED

Allow multiple UDP clients to connect/disconnect/reconnect #26163

Allow multiple UDP clients to connect/disconnect/reconnect #26163

Conversation

bugobliterator commented Feb 7, 2024

magicrub left a comment

Choose a reason for hiding this comment

bugobliterator commented Feb 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterbarker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tridge left a comment

Choose a reason for hiding this comment

bugobliterator commented Feb 9, 2024 •

edited

Loading