-
-
Notifications
You must be signed in to change notification settings - Fork 512
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
Add Example of MAVLink Proxy Using MavlinkPassthrough Plugin #2467
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a nice idea! I have a few questions on the implementation but overall it looks pretty good!
queue_cvs[PX4_NAME] = std::make_unique<std::condition_variable>(); | ||
queue_cvs[QGC_NAME] = std::make_unique<std::condition_variable>(); | ||
|
||
for (uint16_t msg_id = 0; msg_id < 500; ++msg_id) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure that you want to subscribe 500 times? Shouldn't you subscribe just once for each? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, and why 500? That seems somewhat arbitrary, as message IDs go up to 43000, with mostly gaps in-between.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to enable this, should we maybe have something like a subscribe_all_messages
in passthrough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a Proxy, the correct thing to do is to subscribe to all message IDs. So a subscribe_all_messages
might help. Another option is to create a list of message ID constants. I even went as far as manually importing the ID constants into a list. However, I'm open to suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see the problem.
I see that Mavsdk::intercept_messages_async
offers that but MavlinkPassthrough does not. It would probably be a good idea to add that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this!
I would maybe call it MAVLink proxy? Or is it QGC specific?
And it would be nice to have a comment at the top of the file for anyone that finds it to know more about what it is and what it does.
// Thread-safe logging | ||
void log_message(const std::string& level, const std::string& message) { | ||
static std::mutex log_mutex; | ||
std::lock_guard<std::mutex> lock(log_mutex); | ||
std::cout << "[" << level << "] " << message << std::endl; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should actually do this within MAVSDK, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can to do this with MAVSDK ? Is there a function to this ?
queue_cvs[PX4_NAME] = std::make_unique<std::condition_variable>(); | ||
queue_cvs[QGC_NAME] = std::make_unique<std::condition_variable>(); | ||
|
||
for (uint16_t msg_id = 0; msg_id < 500; ++msg_id) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, and why 500? That seems somewhat arbitrary, as message IDs go up to 43000, with mostly gaps in-between.
Quality Gate passedIssues Measures |
Thanks for the changes and comment on the top. Out of curiosity, what is the use case for queuing? Like why do you need archived messages? Are the momentary ones not enough? |
This pull request introduces an example demonstrating how to implement a MAVLink proxy using the MAVSDK
MavlinkPassthrough
plugin. The proxy facilitates communication between two MAVLink components (e.g., PX4 and QGroundControl) by intercepting and forwarding messages.Key Features
Use Cases
Execution Command
To run the example, use the following command:
Important Note: To avoid conflicts, the PX4 SITL instance must not run on the default port
14550
. Adjust the PX4 configuration to use a different port (e.g.,14050
in this example).