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

Add a new c++ node offboard example #7

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Ecuashungo
Copy link

I have moved all python-related content to a separate package and created a parallel c++-package.
Instructions on how to use it are in a nested README.md

Copy link
Owner

@Jaeyoung-Lim Jaeyoung-Lim left a comment

Choose a reason for hiding this comment

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

@Ecuashungo Thanks for the great contribution, and sorry for the late review.

I think looks good overall, but I think it would be great if we can be more consistent with the package naming and directory structure.

Would it be possible to merge the packages into one package and maybe make the cpp build optional? Happy to discuss if you have any opinions.

We can also just merge this PR in and improve incrementally if you prefer it this way

offboard_cpp/src/offboard_cpp_node.cpp Outdated Show resolved Hide resolved
@Ecuashungo
Copy link
Author

@Jaeyoung-Lim thanks for the review, glad I can contribute to the PX4 community.

What do you suggest with respect to the naming and directory structure?

What would be the advantage of merging both the python node and the c++ node into the same package? With the packages separated, a user could add a COLCON_IGNORE file to one package to just use the other one, which is why I have implemented it in this way.

One other thought:
One thing that would be nice to add at some point is the same RVIZ functionality that you have for the python code.

@TheotimeBalaguer
Copy link

TheotimeBalaguer commented Dec 6, 2022

Hello and thanks for your contribution,

I tested the cpp version and faced the issue you mentioned in the Troubleshooting part of the ReadMe : the offboard control mode won't be applied to the SITL version of PX4. If I use the commander mode offboard to switch to offboard control mode, it works very fine. Do you have any clue on what is the problem with the VEHICLE_CMD_DO_SET_MODE ?

@Jaeyoung-Lim
Copy link
Owner

Do you have any clue on what is the problem with the VEHICLE_CMD_DO_SET_MODE ?

@TheotimeBalaguer There should be no problem with sending a switch mode command with mavlink. How are you sending this mavlink command?

@TheotimeBalaguer
Copy link

TheotimeBalaguer commented Dec 6, 2022

How are you sending this mavlink command?

I am using this function, which send an OffboardControlMode with only the position flag on true.

@Jaeyoung-Lim
Copy link
Owner

@TheotimeBalaguer That is not related to VEHICLE_CMD_DO_SET_MODE. VEHICLE_CMD_DO_SET_MODE is a mavlink message

@TheotimeBalaguer
Copy link

You are right my bad ! I guess it has something to do with px4_msgs::msg::OffboardControlModenot working as expected. I'll have a look today.

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