Rework ros2 support

[berndgassmann]

Description

Fixes #

Where has this been tested?

• Platform(s): ...
• Python version(s): ...
• Unreal Engine version(s): ...

Possible Drawbacks

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would update our CHANGELOG.md based on your changes.

[berndgassmann]

CARLA-ROS2-Integration.pptx

I've created a basic PowerPoint describing a bit what is implemented...

[berndgassmann]

I got it compiled and linked under windows now; had to rename actor -> actors as windows file system is ... after that long time still not able to distiguish between LARGE letters and small letters ... incredible. I also cannot test if ROS2 works on windows... because I had to find out that: oh windows 11 is not supported by ROS2, yet. Theoretically, it should work though given that not some firewall stuff is interfering with the ports required by ROS2/DDS. But crashing when starting up on creating DdsDomainParticipantImpl() with access violation is pointing so some stuff like that. Maybe we should leave the debugging of this under Windows to someone who actually needs that there and has experience with DDS/ROS under Windows.

[berndgassmann]

Maybe some comment here on what's actually missing on this now is mainly:

• transfer the synchonization from ros-bridge to here in the sense that we don't just tick by some single instance, but rather every vehicle can decide it it wants to participate in "ticking"; but the idea would be rather a windowing (every client tells what future timestamp he will accept) than a simple tick -- that is what a simulation of multiple vehicles stacks actually require. A vehicle stack knows that while it is evaluating the sensor data there is multiple sensor data upcoming; therefore a let's day 40ms delta time into the future is for most controllers rather usual. Like this the overall simulation speed-up is massive, because the simulator can run at the maximum speed, the hardware is able to provide mostly. Even with massiv multi-vehicle setup.
  At some point the synchoniszation will come, but that portion can be merged and used before.
  The bridging parts of the old ROS bridge can then be removed.
  The supporting/scripting/controlling parts though will NOT BE REPLACED by this. The internal CARLA ROS support will not refine the topic data to shiny perfectly data. That has to be performed on ROS side in each case.
  One open of this ROS2 support -- which has to be clarified over time, I guess -- is the threading: do we need to call the publisher in an async way to not block the server, or is DDS already taking care for such. If DDS is able to block on the publish() call, then a std::async() must be placed to not block the carla server from continuing.
  I know: quite some things to be taken into account, in addition. But I won't be the one to look into these details. I implemented the initial version of the "new" ROS bridge in a C++ like python; now I had the chance to remove the loose python contracts by strong C++ implementation. The rest is up to the community. Thanks.

[Blyron]

Hey Bernd,

We are sorry from being late on your PR. We really like your PR and we think it takes very interesting things about ROS2 functionality, but we think we need to focus the PR on ROS2. We are aware that you have introduced quite useful functionality in CARLA, but in order to be tested properly. In order to provide better feedback, first we would like to focus on ROS2 to test it and then after the merge of ROS2 on the introduction of new classes or other stuff. This other functionality could go into another PR so we can test it properly too.

Would be possible to have a PR focused in ROS2?

Apart from that we had some questions and comments about your code;

Questions:

• Why are you using git submodules?
• Why have you changed gcc-7? Do you think changing gcc version can be risky?
• Are we mixing coordinate systems with Quaternions class?
• Why have you renamed CarlaTopics from Carla...Publisher to Ue...Publisher?

Comments:

• We are not gonna use a single clang format for the whole proyect, LibCarla is using google coding standard, but in Unreal we are gonna use Unreal's coding standard. So we think that if we want to include clang formatters they should go into a different PR.
• We would like to know the reason of changing namespaces and some files' path.
• The changes from MakeSafeUnitVector to MakeUnitVector needs to be reverted, we always want to make sure the unit vector is safely normalized as the location can be (0,0,0)
• What is the purpouse of RightHandedVector?.
• In build.cs we think we need to take care of Mac by adding IsLinux()
• What is the purpouse in ASyncDataStreamImpl Transform RelativeTransform and Rotation?
• Why are we making FCarlaServer inherit from ros2 interface? Does it work if we do not want ROS2 in the build? Or Are we making ROS2 mandatory dependency? We want to keep ROS2 as optional

Kind regards;,

CARLA team

[berndgassmann]

Hi CARLA team,

indeed this is focussing mainly required ROS2 changes. Let's see what is obsolete and can be moved out.
Most of the build system changes are just the merging of #7151 into this branch otherwise one is NOT able to compile successfully under Ubuntu 22.04 / ROS humble.
On the build system:

• gcc-7 was changed to gcc since that one is quite old and not available on ubuntu 22.04 system, why not just using the default gcc that is installed in the system? In Ubuntu 22.04 it's then gcc-11. That works perfectly fine.

• clang-format file I'll remove

Then on the ROS2 changes:

• git submodules allow to fix a specific branch/commit without the need of editing some setup files; especially since the carla-ros-msgs repository is tightly connected to the ros2 extension. It is not required at all for building, but might be useful to checkout as a reference to peek into the msg files. So checking out the msg files is optional; but the developer that wants to start some changes there is happy to have a tight connection of his CARLA PR to the carla-ros-msg PR.

• Carla...Publisher to Ue...Publisher: the Ue...Publisher classes are derived from UePublisherBaseSensor, the other publishers not without that prefix not. That makes it easier to understand which publisher interface the classes belong to. The topics are not named differently, only the classes.

• Are we mixing coordinate systems with Quaternions class?
  No, we don't. The quaternions are introduced because then the conversion
  A) quaternion (Unreal) -> rotation (Carla) -> quaternions (ROS)becomes
  B) quaternion (Unreal) -> quaternions (Carla) -> quaternions (ROS).
  The conversion B) is trivial. The conversion A) requires lots of sin/cos calls.
  I added the quationions in a way, that these are currently only used for ROS2. The rest of carla still uses the A) conversion. The only point is that the quaternion data is added to ActorDynamicState and like that also transferred to other clients

• namespaces/paths: let's see what had to be changed:
  A) Coexistence of TCP and ROS2
  Message is not only used within TCP, but also for ROS2. Since Message.h is independent from TCP, I moved it outside (ROS2 is not depending on TCP tough).
  In a similar way, Session has been split into the basic parts of the session (Session.h) which are independant from TCP and are used by TCP and ROS2 and kept the TPC specidif parts in tcp/ServerSession.h.
  B) Usage of ActorAttribute/ActorBlueprint/BlueprintLibrary from within server build, therefor had to be moved out from client subfolder; new "actors" namespace was the most matching one here as we also have a "sensor" namespace. "actor" namespace was not possible due to folder clash in windows builds where files named identical also resist in "Actor/..." folder.
  C) Did I forget some?

• The changes from MakeSafeUnitVector to MakeUnitVector needs to be reverted, we always want to make sure the unit vector is safely normalized as the location can be (0,0,0)
  -> Indeed I didn't remove the MakeSafeUnitVector functionality, I rather removed the MakeUnitVector() plain functionality since I saw in the develeopment build some usages of the MakeUnitVector() that lead to the Developement-Assert. Therefore, now ALWAYS safe unit vectors are calculated, either with the epsilon provided, or, if not with the default value of 2.0f * std::numeric_limits::epsilon(). Those happened I believe with the opendrive maps, wich also can have points that are exactly on the same position in the map or at least very very close to each other. Since there isn't the possibility of making unsafe unit vectors anymore, I decided to just rename it to MakeUnitVector(). Like this no one can accidently call the unsafe function anymore.

• RightHandedVector: this is a temporary class that isn't visible for the user. In the current CALRA code the rotations around the pitch and roll axes were wrong!
  RightHandedVector has autoconversion functions in both directions to flip the y-axis from UE coordinate system to a right handed coordinate system. The rotation matrixes as formulated are actually rotating in a right handed coordinate system. Therefor, the UE/Carla-Vector3D has to converted to a RightHandedVector before the rotation and converted back to UE/Carla-Vector3D after the rotation. Like this the rotation is performed properly. The same is true for rotations using the quaternion. I have extended the unit test to perform roations around all axis so the proper rotation calculation for Rotator as well for Quaternions is ensured. I also commented out the RotationMatrix retrieval by ifdef ALLOW_UNSAFE_GEOM_MATRIX_ACCESS, because users generally are not aware of that problem and will most proably misuse the matrices (as it was formerly done within CARLA itself).

• ASyncDataStreamImpl: Since ROS2 requires the relative transform to the parent (e.g. of sensors attached to the vehicle) to provide the correct TF trees, the relativeTransform + the relative.Rotation() which is the Quaternion Roation is added to the sensor header (LibCarla/source/carla/sensor/s11n/SensorHeaderSerializer.h). With some smaller changes in the interfaces now ROS2 Ue...Publishers are able to directly read the very same stream data that is already prepared for TCP transmission. That required also the switching of the move semantics on deserialization within the server build (LibCarla/source/carla/sensor/Deserializer.h). Because if the data is moved, the ROS2 publisher "eat" the data and the Buffers provided to the second reader (TCP in that case), get empty buffers.

• Why are we making FCarlaServer inherit from ros2 interface? Does it work if we do not want ROS2 in the build? Or Are we making ROS2 mandatory dependency? We want to keep ROS2 as optional
  -> ROS2 is totally optional. We could also move the interface somewhere else and rename it into e.g. carla::rpc::RpcServerInterface maybe that's then clearer. I did only define the server functions to the interface that are required for ROS2
  Cleanup and harmonize Carla server build to use Unreal sysroot #7151

In summary: You might want to look into the general server build fixes (#7151) that the carla-server and the libraries built for the server are compiled against the Unreal-Sysroot and don't use ANYTHING from the host (maybe someone has to adapt the same for the pytorch build afterwards to make that also clean). If that one is merged we can rebase this commit (or merge the dev branch into this one) to get rid of the general build system changes.
I will rename Ros2ServerInterface and move to rpc folder as suggested. I will remove the clang file. But there is not much more that can be moved out otherwhise the ROS2 build won't work. All other changes were required to get the interface to work, I guess.

[Blyron]

Hey Bernd, can you answer my message on discord so we can have a chat about the PRs?