-
Notifications
You must be signed in to change notification settings - Fork 38
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
Camera bridge bindings #695
Conversation
ac95b14
to
81455fc
Compare
81455fc
to
ecd8db6
Compare
Let's wait for the CI then I will merge and release the software |
c24f770
to
2c4b53a
Compare
2c4b53a
to
33fb790
Compare
Given this error https://github.com/ami-iit/bipedal-locomotion-framework/actions/runs/5463371035/jobs/9946694347#step:22:2419 this PR is not compatible with pybind11 installed by apt ubuntu 20.04 :( |
The feature we need in pybind has been introduced by this commit pybind/pybind11@74a767d pybind/pybind11#2864 |
As mentioned in person, I think we can avoid compiling that parts of the bindings if pybind11 < 2.7.0, in this way we still support 20.04 for almost all the library, while avoiding being blocked by Ubuntu 20.04 quirks for adding new features. |
…_bipedal_locomotion_python_module cmake function
Hi @traversaro I added the logic to check if pybind11 is at least v2.7.0 can you review the PR again? |
bae99cd
to
ac6c830
Compare
ac6c830
to
9b94d4a
Compare
Wow thank you! |
I managed to reproduce the issue on my Ubuntu 20.04 system.
it crashes without errors |
It seems to crash at this line bipedal-locomotion-framework/bindings/python/Planners/tests/test_time_varying_dcm_planner.py Line 105 in 4851900
|
I had similar errors when I was debugging the Windows problems. |
I followed the operations with the debugger. It crashes here:
|
Maybe I was using an old version of |
As suggested by @traversaro, I commented out the modifications to |
Here the compilation output in case of failure and success: |
Under another @traversaro suggestion, I first removed |
Here the compilation output when removing PCL |
I did not see anything strange and also the avx/sse options that were set for PCL on Windows were are not set in PCL for Ubuntu 20.04 . To debug even further, we can remove the PCL compilation, but add the compilation flags it adds with target_compile_flags . If that continues to fail, them we remove and/or add the flags one by one until we find me the problematic one |
Beside a different order of some include directories, that (but it is just a guess) should not matter, the define that are in falure and not in
However, I could not find any use of this defines in commonly used libraries on GitHub. |
Actually, thinking about it, that could be a source of failures: if there there are two different headers that define the same structure with two different ABIs with the same name in two different include directoreies, changing the order of include directories could change which one is included. That could be a source of problems. Anyhow, if we do not find the culprit we could disable this feature for Ubuntu 22.04 . |
I created a docker image with ubuntu 20.04 I discovered that if I commented: bipedal-locomotion-framework/bindings/python/Planners/tests/test_time_varying_dcm_planner.py Line 105 in ec549f6
the tests pass without problems. That function uses qhull to find the convex hull |
If I recall correctly, in the robotology-superbuild qhull is compiled from source only on Ubuntu 20.04 . On the other hand, we use PCL compiled via apt that (if I recall correctly) also links qhull installed via apt. So it is possible that loading pcl compiled via apt loads in memory the qhull of apt, that is not ABI compatible with manually compiled qhull. See robotology/robotology-superbuild#517, robotology/robotology-superbuild#1350 and robotology/robotology-superbuild#1351 for more context. |
In particular, see robotology/robotology-superbuild#1350 (comment) that explains why we can't use apt-installed qhull on Ubuntu 20.04 . If this is confirmed to be the problem, a small brainstorming of possible solutions (each solution is alternative to each other solution):
|
Indeed:
in particular
|
We may switch to https://github.com/akuukka/quickhull |
Just to double check, you can force the loading of the robotology-superbuild's qhull via LD_PRELOAD, just to ensure that like that the test pass. |
Yes, it works! (I have set |
Another really ugly workaround could be to build in CI and robotology-superbuild a version of qhull ABI-compatible with the version installed by apt in Ubuntu 20.04 . |
Other idea: use qhull if PCL is not enabled, and PCL if both qhull and PCL are enabled. |
From #713, I started to use qhull c and not qhull cpp. In theory this should fix all the issue we have |
The CI passes without issues 🚀 #713 fixed all the problems related to qhull and pcl |
No description provided.