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

Add CI file and fix ros2 directory #359

Closed
wants to merge 6 commits into from

Conversation

pmo73
Copy link

@pmo73 pmo73 commented Jun 26, 2023

This pull request will fix the build process for the ros2 package, so that it can be built with clang and g++

@masskro0
Copy link

masskro0 commented Nov 24, 2023

Thanks a lot! Your changes work flawlessly for me in ROS Foxy. I can finally run FSDS with the ROS2 bridge instead of a separate ROS1 to ROS2 bridge.

@pmo73
Copy link
Author

pmo73 commented Nov 24, 2023

Thank you, glad to hear it

@wouter-heerwegh
Copy link
Member

Hi @pmo73,

I've been looking a bit at this, mainly for the workflow. I tried running the workflow locally with act, but for my tests it seems like colcon is not properly parsing the base-path entry in the colcon-defaults.

Regarding the other changes, I still need to run tests, to make sure it's compatible with all non-EOL ROS versions (both 1 and 2).

@pmo73
Copy link
Author

pmo73 commented Jan 13, 2024

Hi @wouter-heerwegh, you're absolutely right that the workflow doesn't work like this. Unfortunately, I have no idea why it doesn't work with the colcon default.yaml, but luckily you can do it manually. I have revised the workflow and now it should work. I also fixed the workflow for ros1, the workflow did also not work locally for me.

@wouter-heerwegh
Copy link
Member

Hi @pmo73,

Thanks, I'll have a look at it. I was also working on some changes for the workflow. One of them would be to drop support for melodic, as it is EOL as is Ubuntu 18.04. I'll probably merge my changes with yours.

Regarding the other changes, were they only to be able to build Airsim? Or did you also test if you could open the Unreal Project with it? I used to build on Ubuntu 18.04, which had no issues. These days I'm on 20.04, but I can't get the project packaged at all.

@pmo73
Copy link
Author

pmo73 commented Jan 13, 2024

Hi @wouter-heerwegh , since we changed the simulator just before Christmas, I have not installed Unreal Engine to be able to confirm this now. In this respect, it is currently only tested to be able to build AirSim at all. But since there are actually no special c++ features that have been changed that are used here, I would have assumed that the changes do not significantly affect loading in Unreal Engine. In addition, @masskro0 seems to have been able to load the project with Ubuntu20.04 and ROS Foxy.

@pmo73
Copy link
Author

pmo73 commented Jan 13, 2024

I just thought that the changes we made might help others. If the code doesn't work with Unreal Engine, feel free to close the PR

@pmo73
Copy link
Author

pmo73 commented Jan 13, 2024

What kind of error do you get on ubuntu20.04?

@wouter-heerwegh
Copy link
Member

wouter-heerwegh commented Jan 13, 2024

I need to build with clang 8, which was also confirmed by someone else in #365, otherwise librpc.a exposes symbols of pthread, pthread_cond_clockwait to be more specific. This function has been introduced in glibc 2.30. UE 4.27 uses a toolchain to compile on linux which is based on Centos 7, which has glibc 2.17.

I can open the Unreal project when using clang 8, but when packaging it complains about not knowing what uint64_t inside a header file of the Engine itself. I think I'll try to port the project to UE5.3 to see if this fixes all the issues I'm currently having.

I can still build binaries, but in Windows. For me this is no issue, but I can imagine that some people do not have their system dual booted, and they would probably just like to develop and build in linux.

@wouter-heerwegh
Copy link
Member

Superseded by #375

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.

3 participants