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 galactic branch #202

Merged
merged 10 commits into from
Sep 13, 2022
Merged

Update galactic branch #202

merged 10 commits into from
Sep 13, 2022

Conversation

Yadunund
Copy link
Member

This PR

  1. Merges in latest main into galactic
  2. Updates the rmf.repos file to checkout galactic-devel branches of the Open-RMF repos which has recently been created.

Moving forward main will be supported on humble. See #165
We can add instructions to that PR to inform users to use the galactic version of the rmf.repos file if the would like to build Open-RMF from source + galactic

xiyuoh and others added 4 commits August 25, 2022 18:01
Signed-off-by: Xi Yu Oh <xiyu@openrobotics.org>
Signed-off-by: Yadunund <yadunund@gmail.com>
Signed-off-by: Dev Manek <manekdev2001@gmail.com>

Co-authored-by: Grey <grey@openrobotics.org>
Co-authored-by: Aaron Chong <aaronchongth@gmail.com>
Co-authored-by: Luca Della Vedova <luca@openrobotics.org>
Co-authored-by: Yadu <yadunund@gmail.com>
Co-authored-by: youliang <tan_you_liang@hotmail.com>
Co-authored-by: Xiyu <ohxiyu@gmail.com>
Co-authored-by: Marco A. Gutiérrez <marco@openrobotics.org>
Signed-off-by: Yadunund <yadunund@gmail.com>
Signed-off-by: Yadunund <yadunund@gmail.com>
Signed-off-by: Yadunund <yadunund@gmail.com>
Yadunund and others added 3 commits August 26, 2022 01:55
Signed-off-by: Yadunund <yadunund@gmail.com>
Signed-off-by: Yadunund <yadunund@gmail.com>
Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
Copy link
Member

@luca-della-vedova luca-della-vedova left a comment

Choose a reason for hiding this comment

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

Nightly fails for some issue with Python3 requests, unrelated to this PR, stack trace here for reference:

2022-09-08T02:25:54.4520766Z Traceback (most recent call last):
2022-09-08T02:25:54.4521136Z   File "/__w/rmf/rmf/ros_ws/install/rmf_building_map_tools/lib/rmf_building_map_tools/building_map_generator", line 33, in <module>
2022-09-08T02:25:54.4521718Z     sys.exit(load_entry_point('rmf-building-map-tools', 'console_scripts', 'building_map_generator')())
2022-09-08T02:25:54.4522202Z   File "/__w/rmf/rmf/ros_ws/install/rmf_building_map_tools/lib/rmf_building_map_tools/building_map_generator", line 25, in importlib_load_entry_point
2022-09-08T02:25:54.4522566Z     return next(matches).load()
2022-09-08T02:25:54.4522862Z   File "/usr/lib/python3.8/importlib/metadata.py", line 77, in load
2022-09-08T02:25:54.4523232Z     module = import_module(match.group('module'))
2022-09-08T02:25:54.4523549Z   File "/usr/lib/python3.8/importlib/__init__.py", line 127, in import_module
2022-09-08T02:25:54.4523951Z     return _bootstrap._gcd_import(name[level:], package, level)
2022-09-08T02:25:54.4524259Z   File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
2022-09-08T02:25:54.4524580Z   File "<frozen importlib._bootstrap>", line 991, in _find_and_load
2022-09-08T02:25:54.4524912Z   File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
2022-09-08T02:25:54.4525336Z   File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
2022-09-08T02:25:54.4525656Z   File "<frozen importlib._bootstrap_external>", line 848, in exec_module
2022-09-08T02:25:54.4526003Z   File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
2022-09-08T02:25:54.4526417Z   File "/__w/rmf/rmf/ros_ws/build/rmf_building_map_tools/building_map_generator/building_map_generator.py", line 3, in <module>
2022-09-08T02:25:54.4526780Z     from building_map.generator import Generator
2022-09-08T02:25:54.4527141Z   File "/__w/rmf/rmf/ros_ws/build/rmf_building_map_tools/building_map/generator.py", line 4, in <module>
2022-09-08T02:25:54.4527463Z     from .building import Building
2022-09-08T02:25:54.4527799Z   File "/__w/rmf/rmf/ros_ws/build/rmf_building_map_tools/building_map/building.py", line 20, in <module>
2022-09-08T02:25:54.4528099Z     from .level import Level
2022-09-08T02:25:54.4528429Z   File "/__w/rmf/rmf/ros_ws/build/rmf_building_map_tools/building_map/level.py", line 15, in <module>
2022-09-08T02:25:54.4528736Z     from .floor import Floor
2022-09-08T02:25:54.4529036Z   File "/__w/rmf/rmf/ros_ws/build/rmf_building_map_tools/building_map/floor.py", line 9, in <module>
2022-09-08T02:25:54.4529401Z     from .material_utils import (add_pbr_material, get_pbr_textures,
2022-09-08T02:25:54.4529782Z   File "/__w/rmf/rmf/ros_ws/build/rmf_building_map_tools/building_map/material_utils.py", line 2, in <module>
2022-09-08T02:25:54.4530071Z     import requests
2022-09-08T02:25:54.4530471Z   File "/usr/lib/python3/dist-packages/requests/__init__.py", line 95, in <module>
2022-09-08T02:25:54.4530795Z     from urllib3.contrib import pyopenssl
2022-09-08T02:25:54.4531233Z   File "/usr/lib/python3/dist-packages/urllib3/contrib/pyopenssl.py", line 46, in <module>
2022-09-08T02:25:54.4531519Z     import OpenSSL.SSL
2022-09-08T02:25:54.4531907Z   File "/usr/lib/python3/dist-packages/OpenSSL/__init__.py", line 8, in <module>
2022-09-08T02:25:54.4532209Z     from OpenSSL import crypto, SSL
2022-09-08T02:25:54.4532606Z   File "/usr/lib/python3/dist-packages/OpenSSL/crypto.py", line 1553, in <module>
2022-09-08T02:25:54.4532905Z     class X509StoreFlags(object):
2022-09-08T02:25:54.4533323Z   File "/usr/lib/python3/dist-packages/OpenSSL/crypto.py", line 1573, in X509StoreFlags
2022-09-08T02:25:54.4533645Z     CB_ISSUER_CHECK = _lib.X509_V_FLAG_CB_ISSUER_CHECK
2022-09-08T02:25:54.4534060Z AttributeError: module 'lib' has no attribute 'X509_V_FLAG_CB_ISSUER_CHECK'

@orensbruli
Copy link
Contributor

With the new commits, I think we have progressed a little bit.
@Yadunund can you double check that the packages removed in c0769bf are not needed in galactic.

https://github.com/open-rmf/rmf/runs/8254192916?check_suite_focus=true#step:7:43856

Can some of you take a look into those failing tests or ping someone who can know about it?

@luca-della-vedova
Copy link
Member

With the new commits, I think we have progressed a little bit. @Yadunund can you double check that the packages removed in c0769bf are not needed in galactic.

I think the simulation plugins listed there would indeed be needed in galactic, otherwise simulation wouldn't work.

https://github.com/open-rmf/rmf/runs/8254192916?check_suite_focus=true#step:7:43856

Can some of you take a look into those failing tests or ping someone who can know about it?

I believe the rmf_utils issue is due to the exclusion for catch.hpp not working in the ament_uncrustify. That file is so long that uncrustify times out. I guess I would suggest going in that direction to investigate it.
For rmf_traffic it seems a bit more sketchy, I can reproduce locally failing tests that seem due to an fcl unit test, specifically with this failing testcase:

     <testcase classname="test_rmf_traffic.global" name="Verify that FCL can handle continuous collisions" time="0.004" status="run">
       <failure message="result.is_collide" type="CHECK">
 FAILED:
   CHECK( result.is_collide )
 with expansion:
   false
 at /home/luca/rmf_ws/src/rmf/rmf_traffic/rmf_traffic/test/unit/fcl_test.cpp:98
       </failure>
     </testcase>

I'm not sure what we can do about it, since it seems to be due to a test on a third party library

Signed-off-by: Esteban Martinena <orensbruli@gmail.com>
@orensbruli orensbruli force-pushed the update_galactic branch 3 times, most recently from bc406a2 to 02e2681 Compare September 9, 2022 06:45
Signed-off-by: Esteban Martinena <orensbruli@gmail.com>
This reverts commit 3f1021b.
Signed-off-by: Esteban Martinena <orensbruli@gmail.com>
@orensbruli
Copy link
Contributor

I think the simulation plugins listed there would indeed be needed in galactic, otherwise simulation wouldn't work.

https://github.com/open-rmf/rmf/runs/8254192916?check_suite_focus=true#step:7:43856
Can some of you take a look into those failing tests or ping someone who can know about it?

You are right. My fault, thank you.

I believe the rmf_utils issue is due to the exclusion for catch.hpp not working in the ament_uncrustify. That file is so long that uncrustify times out. I guess I would suggest going in that direction to investigate it.

I'm gonna try looking into this.

For rmf_traffic it seems a bit more sketchy, I can reproduce locally failing tests that seem due to an fcl unit test, specifically with this failing testcase:

     <testcase classname="test_rmf_traffic.global" name="Verify that FCL can handle continuous collisions" time="0.004" status="run">
       <failure message="result.is_collide" type="CHECK">
 FAILED:
   CHECK( result.is_collide )
 with expansion:
   false
 at /home/luca/rmf_ws/src/rmf/rmf_traffic/rmf_traffic/test/unit/fcl_test.cpp:98
       </failure>
     </testcase>

I'm not sure what we can do about it, since it seems to be due to a test on a third party library

It's a third-party library but I think the test that is failing is ours, right? https://github.com/open-rmf/rmf_traffic/blame/main/rmf_traffic/test/unit/fcl_test.cpp
I think @mxgrey created this test, may be you can take a look into this?

@orensbruli
Copy link
Contributor

orensbruli commented Sep 9, 2022

I believe the rmf_utils issue is due to the exclusion for catch.hpp not working in the ament_uncrustify. That file is so long that uncrustify times out. I guess I would suggest going in that direction to investigate it.

Investigating this. Notes for myself and anyone who want to follow this.

This is the call that @luca-della-vedova suggested to be failing https://github.com/open-rmf/rmf_utils/blob/main/rmf_utils/CMakeLists.txt#L95 and the EXCLUDE argument that is probably not working https://github.com/open-rmf/rmf_utils/blob/main/rmf_utils/CMakeLists.txt#L99

The code of ament_uncrustify command itself:
https://github.com/ament/ament_lint/blob/galactic/ament_uncrustify/ament_uncrustify/main.py

OPTION 1:
About the exclude, it looks like it's different between galactic version and rolling:
Rolling https://github.com/ament/ament_lint/blob/rolling/ament_uncrustify/ament_uncrustify/main.py#L222
Galactic https://github.com/ament/ament_lint/blob/galactic/ament_uncrustify/ament_uncrustify/main.py#L234

As we are working on Galactic I'm debugging and I see that it only looks to exclude names of subdirectories in the current directory.
We are passing include/rmf_utils/catch.hpp and this does not match the name of any of the subdirectories.
It looks like this behavior is fixed in rolling with this commit: ament/ament_lint@0a7c40f
Made a PR to backport this fix: ament/ament_lint#409

Any ideas on how to get through this as it is released?

OPTION 2:
I have also found this pull request related to ament_add_test and TIMEOUT. I think we could add TIMEOUT argument to the ament_uncrustify call to ament_add_test (https://github.com/ament/ament_lint/blame/rolling/ament_cmake_uncrustify/cmake/ament_uncrustify.cmake#L70)
Added a PR for this: https://github.com/ament/ament_lint/pull/408/commits

Copy link
Member

@luca-della-vedova luca-della-vedova left a comment

Choose a reason for hiding this comment

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

Thanks @orensbruli for the investigation!
With Galactic going EOL in November I don't know how likely it is that upstream fixes will be given a high priority. It seems that both fixes (timeout and exclude logic) are upstream so there isn't much we can do about them other than possibly commenting failing tests.

I think we can go ahead and merge this in and accept that galactic nightlies will only become green if the upstream PR is merged (and the rmf_traffic failure investigated).

@luca-della-vedova luca-della-vedova merged commit 540fbb8 into galactic Sep 13, 2022
@luca-della-vedova luca-della-vedova deleted the update_galactic branch September 13, 2022 06:11
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.

None yet

5 participants