core: add raw connection #2682

julianoes · 2025-10-08T01:12:32Z

This adds a "raw connection" which accepts and delivers raw bytes. This can be handy when a custom transport other than the included UDP/TCP/serial needs to be used.

JonasVautherin

I am a bit confused by the API 😕. Is the intent to "observe" all the connections (in the manner of running wireshark for debugging) and to "inject" arbitrary bytes into all the connections?

If the goal is to enable a custom Connection implementation, isn't it possible to just expose that? add_connection(Connection connection), and let the user implement their own variant?

But maybe that cannot be exposed easily in the includes without a lot of refactoring? Right now, every connection manipulates mavlink in its own implementation. I guess ideally this would be separate: a Connection would only send and receive bytes over a transport, and there would be one unified "MavlinkHandler" that would use the Connection and do the MAVLink stuff. That MavlinkHandler could take a UdpConnection or a TcpConnection or a user-implemented QuicConnection all the same.

So a user would use something like add_connection(Connection connection), e.g. add_connection(my_quic_connection), and ideally we would only have to expose connection.h in the public API.

What do you think?

julianoes · 2025-10-09T01:22:41Z

@JonasVautherin what you describe could be a bit nicer indeed. It's just that so far we have not exposed connection.h, and we have some specific stuff in connections, e.g. for UDP we have to keep track of the remotes and let them time out, etc.

The aim is exactly that you can just feed it bytes from your own connection, and get a callback for any bytes that need sending.

JonasVautherin

I guess I mostly got confused by some of the documentation mentioning doing something with the other connections, and the way it works is confusing me a little. Also the raw:// confuses me, I guess it's a way to not add an enable_raw_connection() function and shouldn't be used from a CLI?

But other than that I think it should work 👍.

JonasVautherin · 2025-10-09T09:36:06Z

src/mavsdk/core/include/mavsdk/mavsdk.h

+    /**
+     * @brief Send raw MAVLink bytes.
+     *
+     * This allows sending raw MAVLink message bytes through all connections.


I don't get that line. What does "through all connections" mean? I understand this function as this: say I am writing a GCS with MAVSDK and communicating with a remote drone. I want to use Quic and want to use my own custom implementation. So outside of MAVSDK I create a connection to the drone.

When I receive bytes from the drone, I need to feed them to MAVSDK. This is this send_raw_bytes, right?

When MAVSDK sends bytes to the drone, it needs to feed them to my custom Quic implementation. This is subscribe_raw_bytes, I think? So I call send to receive, and subscribe to send, because I am outside of MAVSDK so it's inverted.

But then what does it have to do with the other connections? Say there is a UDP connection, if I receive bytes from the drone and feed them to MAVSDK, I don't want MAVSDK to send them to the remote of the UDP connection 🤔??

Oh dear. Yes, Claude was confused how it worked, hence the comments. I'll have to fix those.

And you're right naming is confusing. I should call it receive_raw_bytes and subscribe_to_bytes_to_send.

JonasVautherin · 2025-10-09T09:38:55Z

src/mavsdk/core/include/mavsdk/mavsdk.h

+    /**
+     * @brief Subscribe to all outgoing raw bytes.
+     *
+     * This allows monitoring all MAVLink bytes that are sent out through any connection.


Same here, though I guess we always send the same MAVLink messages over all the active connections.

Is that function meant to "monitor" stuff happening with other connections? My feeling is that it's meant for MAVSDK to send MAVLink bytes over the raw connection (e.g. my custom Quic implementation). So I'm not monitoring anything, I'm just providing a transport implementation to MAVSDK, right?

Yes, again Claude confused. Let me fix it.

JonasVautherin · 2025-10-09T09:40:37Z

src/mavsdk/core/mavsdk_impl.cpp

+    // Enable heartbeats for raw connection
+    auto new_configuration = get_configuration();
+    new_configuration.set_always_send_heartbeats(true);
+    set_configuration(new_configuration);


I don't understand why this is required. That means that I cannot write my own custom UDP implementation that would behave like the internal UDP implementation, because for some reason I'm forced to enable heartbeats?

The idea here is that we should send out heartbeats on a custom/raw connection in order to start MAVLink flowing. The only place where we don't send out heartbeats is UDP connections that are just listening and don't have a remote to actually send messages to. In fact, we could just remote the whole "always send heartbeats" logic and have it enabled for all connections and just drop messages as we don't have a remote yet.

Got it, makes sense!

JonasVautherin · 2025-10-09T09:47:28Z

src/mavsdk/core/mavsdk_impl.cpp

+
+bool MavsdkImpl::send_raw_bytes(const char* bytes, size_t length)
+{
+    auto* raw_conn = find_raw_connection();


I guess that's the reason for the maximum one raw connection? Because for every single message MAVSDK wants to send, it has to go through all the connections, dynamic_cast each of them and try to find the RawConnection?

I guess it could just loop through all of them, but right now there is no need for multiple raw connections (maybe there will never be one) so it's simpler like this?

My next concern is the overhead of casting all the connections, but again I guess in practice there is only one connection so it's okay?

That's a valid concern. I'm not sure what the overhead is.

The reason I wanted to have it as a "real connction" was so that forwarding could be used with it.

JonasVautherin · 2025-10-09T09:50:50Z

src/mavsdk/core/cli_arg.cpp

+    const std::string raw = "raw";
+    if (uri.find(raw + delimiter) == 0) {
+        return parse_raw(std::string_view(uri).substr(raw.size() + delimiter.size()));
+    }
+


Do we need to expose this at all? Does that mean that one can run mavsdk_server raw://? When is it useful for a CLI to do that? 🤔

The point of having the add_any_connection("raw://") call is that you can also use add_any_connection("raw://", ForwardingOption::ForwardingOn);

julianoes · 2025-10-09T19:06:41Z

Thanks for the comments!

The reason I hadn't gone for the more object oriented approach of "creating your own connection and adding it" is that it's not straightforward to support that in language wrappers. However, we already have the API to add any connection and use that in all language wrappers. And we can expose the new mavsdk API via the C wrapper that we have. As an example, this would allow serial on Android without having to go via UDP.

docs/en/cpp/api_reference/classmavsdk_1_1_mavsdk.md

src/mavsdk/core/include/mavsdk/mavsdk.h

docs/en/cpp/api_reference/classmavsdk_1_1_mavsdk.md

julianoes · 2025-10-10T20:14:20Z

Thanks for the proof reading. I'm clearly a bit distracted 😑

This adds a "raw connection" which accepts and delivers raw bytes. This can be handy when a custom transport other than the included UDP/TCP/serial needs to be used.

The client needs to make sure this happens quickly.

This should make the raw connection a bit clearer.

We probably shouldn't rely on heartbeats as these are sent anyway.

sonarqubecloud · 2025-11-05T03:20:50Z

Quality Gate passed

Issues
11 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

julianoes requested a review from JonasVautherin October 8, 2025 01:12

julianoes force-pushed the pr-raw-bytes branch from 71946e6 to f748413 Compare October 8, 2025 01:43

JonasVautherin reviewed Oct 8, 2025

View reviewed changes

JonasVautherin previously approved these changes Oct 9, 2025

View reviewed changes

julianoes dismissed JonasVautherin’s stale review via 90eee55 October 9, 2025 20:53

JonasVautherin previously approved these changes Oct 10, 2025

View reviewed changes

julianoes dismissed JonasVautherin’s stale review via 4ff3617 October 10, 2025 20:26

JonasVautherin previously approved these changes Oct 10, 2025

View reviewed changes

julianoes added 5 commits November 4, 2025 14:46

core: add raw connection

7f33ef7

This adds a "raw connection" which accepts and delivers raw bytes. This can be handy when a custom transport other than the included UDP/TCP/serial needs to be used.

system_tests: prevent maybe uninitialized

cab1032

core: directly call raw connection callback

250ccb0

The client needs to make sure this happens quickly.

core: fixup API naming and docs

9b6740e

This should make the raw connection a bit clearer.

core: fixup spelling

0bb58fa

julianoes dismissed JonasVautherin’s stale review via 0bb58fa November 4, 2025 01:48

julianoes force-pushed the pr-raw-bytes branch from 4ff3617 to 0bb58fa Compare November 4, 2025 01:48

system_tests: adapt test for raw bytes API

6d3a34a

We probably shouldn't rely on heartbeats as these are sent anyway.

Uh oh!

core: add raw connection #2682

Are you sure you want to change the base?

core: add raw connection #2682

Uh oh!

Conversation

julianoes commented Oct 8, 2025

Uh oh!

JonasVautherin left a comment

Choose a reason for hiding this comment

Uh oh!

julianoes commented Oct 9, 2025

Uh oh!

JonasVautherin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

julianoes commented Oct 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

julianoes commented Oct 10, 2025

Uh oh!

sonarqubecloud bot commented Nov 5, 2025

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants