mavlink / MAVSDK

API and library for MAVLink compatible systems written in C++17
https://mavsdk.mavlink.io
BSD 3-Clause "New" or "Revised" License
637 stars 510 forks source link

mavsdk_server with more than one remote system #1633

Open dlech opened 3 years ago

dlech commented 3 years ago

I would like to be able to make use of the CamerServer API I am working on in Python. However it appears from looking at the mavsdk_server code that it only uses the first discovered system.

https://github.com/mavlink/MAVSDK/blob/125fa95e681309c3004d9460a147888429f74021/src/core/lazy_plugin.h#L19

This works for connecting to just an autopilot, but the CamerServer will need to communicate to both a ground control station and an autopilot. This can be done with a single connection (a single Mavskd object). But the connection will have at least two systems - and based on my experience so far, which one is at index 0 is non-deterministic if the ground station is already up and running when the mavsdk_server is started.

dlech commented 3 years ago

Would it make sense to add a command line option to filter on a specific system and component id to select the desired system instead of the first/only system? Then for my use case, I would run one instance of mavsdk_server with 1:1 to connect to the autopilot and another instance with 255:190 to connect to the ground control station, each with a unique TCP port. Then in Python, I would create two System objects by specifying the chosen TCP ports.

EDIT: One potential problem with this is that both mavsdk_server instances would need to have the same self system and component ids (in this case 1:100 for an onboard camera). This could cause problems with message routing.

dlech commented 3 years ago

Modified idea: maybe a single mavsdk_server could be modified to run multiple gRPC servers? For example, a single mavsdk_server instance would be run. It would have its self system and component id be 1:100. A command line option would be supplied (probably a configuration file) that provides a map of system ids to TCP ports, e.g. if a autopilot 1:1 appeared, it would start a gRPC server on port 50051, if a ground control station 255:190 appeared, it would start a gRPC server on port 50052.

Possibly, a 3rd gRPC server would be needed with a separate protocol to notify Python when a system appeared so it doesn't have to constantly try to connect to non-existent ports, e.g. if the ground control station is not up and running.

julianoes commented 3 years ago

Does your camera server have the same system ID as the drone it is on? Because that would be the prerequisite so it shows up with the same system.

julianoes commented 3 years ago

I guess we are missing the interface to set the sysid/compid of the mavsdk instance for Python, right?

dlech commented 3 years ago

Does your camera server have the same system ID as the drone it is on? Because that would be the prerequisite so it shows up with the same system.

MAVSDK allows you to set any system id, but for my use case, yes, I was planning on using the same system ID as the drone.

dlech commented 3 years ago

I guess we are missing the interface to set the sysid/compid of the mavsdk instance for Python, right?

Yes, there is a C API for this in mavsdk_server, but no command line options yet which could be used by Python.

julianoes commented 3 years ago

So in C++ you can set the sysid/compid of mavsdk (yourself): https://github.com/mavlink/MAVSDK/blob/125fa95e681309c3004d9460a147888429f74021/src/core/mavsdk.h#L213-L222

And you're right, we need to add that to the API. I remember having started with this. Let's check.

julianoes commented 3 years ago

Here it is! https://github.com/mavlink/MAVSDK/pull/1577

julianoes commented 3 years ago

I think now we need to expose it in:

https://github.com/mavlink/MAVSDK-Python/blob/edda07ff2d382bbfca02bda3c9e815e8a8b55b91/mavsdk/system.py#L61-L66

and:

https://github.com/mavlink/MAVSDK-Python/blob/edda07ff2d382bbfca02bda3c9e815e8a8b55b91/mavsdk/system.py#L292

And we have to add it here:

https://github.com/mavlink/MAVSDK/blob/125fa95e681309c3004d9460a147888429f74021/src/mavsdk_server/src/mavsdk_server_bin.cpp#L21-L40

And here: https://github.com/mavlink/MAVSDK/blob/125fa95e681309c3004d9460a147888429f74021/src/mavsdk_server/src/mavsdk_server_bin.cpp#L46

dlech commented 3 years ago

Yes, that is the easy part :wink:.

julianoes commented 3 years ago

Ok, so I'm missing something? (Sorry I'm on a call in parallel, so I might be glancing over some of what you wrote.)

dlech commented 3 years ago

You should probably read the first few comments again when you are not busy :smile:.

julianoes commented 3 years ago

So you need a way to connect all 3 components together, right? Could you use mavlink-router in the middle to route the traffic? Or how does your connection diagram look like?

dlech commented 3 years ago

So you need a way to connect all 3 components together, right?

This isn't really the problem. The problem is that mavsdk_server only allows communicating with one other system on a single connection (the first-seen system), not two (an autopilot and a ground control station).

Could you use mavlink-router in the middle to route the traffic? Or how does your connection diagram look like?

For testing, I am using px4_sitl, so the diagram looks like https://docs.px4.io/master/en/simulation/#sitl-simulation-environment. The camera server is one of the API/Offboard nodes on port 14030. The px4_sitl instance takes care of forwarding messages to/from the ground control station.

julianoes commented 3 years ago

Ok, it's dawning on me. You're right.

I would like to be able to expose the logic with more than one system to the language wrappers but I don't quite know how yet.

dlech commented 3 years ago

Does https://github.com/mavlink/MAVSDK/issues/1633#issuecomment-978081589 seem reasonable? I haven't thought of any major shortcomings with that idea yet.

julianoes commented 3 years ago

I need to think through it, thanks!

dlech commented 3 years ago

I suppose another option would be to do https://github.com/mavlink/MAVSDK/issues/1618#issuecomment-970334161 for Python as well - create a new set of Python bindings that doesn't use mavlnik_server/gRPC but rather uses the C++ API directly.