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.

Sign up for GitHub

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

Jump to bottom

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
Open

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions protos/core/core.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ service CoreService {
* need to be increased to prevent timeouts.
*/
rpc SetMavlinkTimeout(SetMavlinkTimeoutRequest) returns(SetMavlinkTimeoutResponse) {}
/*
* List all components (computers, cameras, servos, gimbals, etc.) connected to the system.
*/
rpc ListComponents (ListComponentsRequest) returns (ListComponentsResponse) {}
}

message SubscribeConnectionStateRequest {}
Expand All @@ -36,3 +40,18 @@ message SetMavlinkTimeoutResponse {}
message ConnectionState {
bool is_connected = 2; // Whether the vehicle got connected or disconnected
}

// Defines a request to list components of all systems.
message ListComponentsRequest {}

// Defines a single system's components.
message SystemComponents {
uint32 system_id = 1; // System ID
repeated uint32 component_ids = 2; // List of component IDs for this system
}

// 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 •
edited
Loading

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 •
edited
Loading

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();
........

}