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

[ros] Add mavros_msgs to build dependencies #2642

Merged
merged 1 commit into from
Jul 22, 2020

Conversation

jsharpe
Copy link
Contributor

@jsharpe jsharpe commented Apr 30, 2020

If mavros is being built in the same catkin workspace then airsim_ros_pkgs needs to depend upon mavros_msgs for the packages to be built in the correct order.

If mavros is being built in the same catkin workspace then airsim_ros_pkgs needs to depend upon mavros_msgs for the packages to be built in the correct order.
@madratman
Copy link
Contributor

This doesn't really fix the issue.
sudo apt-get install ros-melodic-mavros* can be added to the docs

@madratman
Copy link
Contributor

madratman commented Apr 30, 2020

But ok, if some one is compiling it from source, we should have that in list of deps.
Wouldn't it also be a runtime dep?

@jsharpe
Copy link
Contributor Author

jsharpe commented May 1, 2020

But ok, if some one is compiling it from source, we should have that in list of deps.
Wouldn't it also be a runtime dep?

RIght - it should just be a <depend> node, also a number of the other msgs dependencies could be converted to just a <depend> to remove duplication. Also now I look at it why is it depending upon message_generation at exec time? Seems like there is some clean up to be done in the packages.xml file

@madratman
Copy link
Contributor

ah yes, it should be

<build_depend>message_generation</build_depend> 
<exec_depend>message_runtime</exec_depend>

Yeah, I agree the <depend> tag can half the number of lines for sure, as it's kinda like a catch-all. Please feel free to add that on to this PR. If not, I can do that, and merge this one. Lemme know.

@madratman madratman merged commit 09bc774 into microsoft:master Jul 22, 2020
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.

2 participants