Skip to content

[ros] Add mavros_msgs to build dependencies - #2642

Merged
Ratnesh Madaan (madratman) merged 1 commit into
microsoft:masterfrom
jsharpe:patch-1
Jul 22, 2020
Merged

[ros] Add mavros_msgs to build dependencies#2642
Ratnesh Madaan (madratman) merged 1 commit into
microsoft:masterfrom
jsharpe:patch-1

Conversation

@jsharpe

Copy link
Copy Markdown

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
Copy Markdown
Contributor

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

@madratman

Ratnesh Madaan (madratman) commented Apr 30, 2020

Copy link
Copy Markdown
Contributor

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

James Sharpe (jsharpe) commented May 1, 2020

Copy link
Copy Markdown
Author

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
Copy Markdown
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
Ratnesh Madaan (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