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 leaking in as dependency #8815

Closed
LorenzMeier opened this issue Feb 3, 2018 · 10 comments
Closed

ROS leaking in as dependency #8815

LorenzMeier opened this issue Feb 3, 2018 · 10 comments
Assignees
Labels

Comments

@LorenzMeier
Copy link
Member

@dagar We need a CI system set up without any ROS to avoid ROS packages leaking in as unintended dependency:

Traceback (most recent call last):
  File "/Users/lorenzmeier/src/Firmware/Tools/sitl_gazebo/scripts/jinja_gen.py", line 9, in <module>
    import rospkg
ImportError: No module named rospkg

@TSC21 FYI

@PX4BuildBot
Copy link
Collaborator

GitMate.io thinks possibly related issues are #7335 (GPS coordinate in GPS-denied environments), #4764 (VTOL: Nasty Yaw problem after back trans. in ALT and POS), #6193 (GPS altitude spike feeding into altitude control loop despite GPS fusion disabled), #7675 (Some questions about “mc_att_control” hope to get help), and #7765 (Problem with update firmware pixhawk on aero drone).

@PX4BuildBot PX4BuildBot added the bug label Feb 3, 2018
@LorenzMeier
Copy link
Member Author

@dagar And Gitmate starts to fail us badly - can we disable related issues? It has no clue what it's doing.

@LorenzMeier
Copy link
Member Author

Just disabled duplicate checking.

@TSC21
Copy link
Member

TSC21 commented Feb 3, 2018 via email

@LorenzMeier
Copy link
Member Author

Fixed in df32297

@TSC21 This failed on my machine. So you don't need to fix it on the container. Instead we need a new container without ROS to make sure we're not picking up dependencies we don't want in Firmware.

@TSC21
Copy link
Member

TSC21 commented Feb 3, 2018

The question is that you can install rospkg without ROS. That'a why I added to the install instructions on sitl_gazebo side.

@LorenzMeier
Copy link
Member Author

That's not a good argument. It still becomes a dependency. If you start thinking that way you will have soon hundreds of packages as dependencies and the only working platform will be Ubuntu.

Instead, any package pulled into the system must serve a purpose. If it doesn't (there is no value for non-ROS enabled Gazebo SITL) it must not become a dependency in the first place.

And to enforce all this we need a minimal image running CI.

@TSC21
Copy link
Member

TSC21 commented Feb 3, 2018

I understand. I actually initially was for not adding any ROS dependency but having it available by pip did change my mind. But that makes sense

@dagar
Copy link
Member

dagar commented Feb 4, 2018

gitmate silenced (it's still going to be responsible for closing stale issues).

@dagar
Copy link
Member

dagar commented Feb 4, 2018

@LorenzMeier rospkg was allowed to slip through because it's available via pip. If you feel strongly about it not being a requirement I think we need to seriously reconsider what's going to be done in sitl_gazebo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants