Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

This commit updates the core.proto to include enhancements to the Cor… #348

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

NourElMenshawy
Copy link

…eService:

The introduction of the ListComponents RPC provides the capability to list all components connected to the system, such as computers, cameras, servos, and gimbals. This feature enhances system introspection and management for users.

…eService:

The introduction of the ListComponents RPC provides the capability to list all components connected to the system, such as computers, cameras, servos, and gimbals. This feature enhances system introspection and management for users.
Copy link
Collaborator

@julianoes julianoes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pull request! See comment inline.

// Defines the response for a request to list components of all systems.
// It includes a list of systems, each with their own list of component IDs.
message ListComponentsResponse {
repeated SystemComponents systems = 1; // List of systems with their components
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is that with the current grpc API we only have access to the one system, which is the first autopilot. So having a list of all systems would be information that can't be used.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Julian,

In my Discord comment, I mentioned that I had one system from which I could list all components since my feature only needs one system. However, in my merge request, I extended it to include listing all systems because I'm preparing to share a draft with you on how to select systems over gRPC. But of course, if it's more convenient for you to wait for the bigger MR draft, then feel free to ignore this one.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks! I will have to think about this a bit more in the coming weeks, and talk to @JonasVautherin to figure out how we want to roll this out to the language wrappers.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be a good idea if @NourElMenshawy described the idea somewhere (maybe in an issue). Whatever is done has to be compatible with the language wrappers (that's the whole point of the gRPC layer, after all 😇).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @JonasVautherin
The purpose of this MR is to retrieve all components from the MAVSDK server over gRPC. I developed a Dart application that reads parameters from the drone via an LTE server. In this setup, the LTE server communicates with MAVLink, and my custom MAVSDK server includes the gRPC method for getting all components. My Dart application reads and writes parameters to the drone. Being able to select components allows my application to set and read the parameters of those components.

Copy link
Collaborator

@JonasVautherin JonasVautherin Jul 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but listing MAVLink systems is not the same as being able to talk to different systems. Say in Python you do system.action.arm(), how do you deal with two systems?

My personal opinion tends to be that you can demultiplex the MAVLink stream and run multiple instances of MAVSDK. So if some messages come from sysid 12 and some from sysid 13, you can have your demultiplexer send sysid 12 to some local port (and bind the first MAVSDK instance to it) and have it send sysid 13 to some other local port (and bind the second MAVSDK instance to it).

I have done something similar in the past (where my demultiplexer was discriminating based on IP and not based on sysid) and it worked well.

Copy link
Author

@NourElMenshawy NourElMenshawy Jul 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @JonasVautherin
I do read my comments and I think I didn't clearly convey my point, so I'll try again:

  1. I don't care about the system ID because I only have one drone. I made the system ID a list to potentially support multiple systems in the future.
  2. I need to read the parameters connected to the drone, including those from external components, but I don't know their IDs. Therefore, I need MAVSDK to provide me with those component IDs so I can select them and read their parameters.

As you can see in the following MR commit , the system loop will always run once, so I don't anticipate any problems for now. However, I can change this implementation to something like:

auto systems = _mavsdk.systems();
if (!systems.empty()) {
    // Directly access the first system since we only have one.
    const auto& system = systems.front();
........

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants