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

Update comments in VehicleCommands #25

Open
audrow opened this issue Mar 31, 2023 · 3 comments
Open

Update comments in VehicleCommands #25

audrow opened this issue Mar 31, 2023 · 3 comments

Comments

@audrow
Copy link
Contributor

audrow commented Mar 31, 2023

It seems that the comments in https://github.com/PX4/px4_msgs/blob/main/msg/VehicleCommand.msg have fallen out of sync with the messages being used. Maybe the comments could be updated, or if not deleted (especially the incorrect information).

For example, the comments with VEHICLE_CMD_DO_SET_MODE say that it takes only one argument,
https://github.com/PX4/px4_msgs/blob/main/msg/VehicleCommand.msg#L32

But the command on Mavlink takes up to three parameters.

https://mavlink.io/en/messages/common.html#MAV_CMD_NAV_LAND

Also, some comments list the inputs for the command. In the case that no inputs are listed, I assume that the command takes no parameters, but this is not true in every case. For example,

https://github.com/PX4/px4_msgs/blob/main/msg/VehicleCommand.msg#L32

Has two arguments documented here:

https://mavlink.io/en/messages/common.html#MAV_CMD_DO_VTOL_TRANSITION

I assume that these examples are not the only errors or possibly confusing comments. It may be good to direct users to the documentation or source code, instead of keeping the comments up-to-date.

@audrow
Copy link
Contributor Author

audrow commented Mar 31, 2023

As a side note, it is not clear how the https://github.com/PX4/px4_msgs/tree/ros2 branch is different from the main branch. They both seem to be ROS 2, but the ros2 branch doesn't seem to have been updated since 2019.

@kjrusscher
Copy link

Thanks for pointing out that the ros2 branch is a stale branch; that saved me a lot of time. The file generated form https://github.com/PX4/px4_msgs/blob/main/msg/VehicleCommand.msg contain some errors that give compile errors. The main branch does not give any compile errors. So I guess that we have to use the main branch for ROS2.

@beniaminopozzan
Copy link
Member

@TSC21 , can we safely remove the ros2 branch?

Thanks @audrow !
BTW, all the .msg files in this repo are directly copied form the ones in the main firmware, there is a 1 : 1 correspondence, so this issue should be opened in https://github.com/PX4/PX4-Autopilot

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

No branches or pull requests

3 participants