feat(ros2): [6/8] add ROS2TopicVisibility default-startup flag - #9756
Conversation
|
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. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR refactors and expands CARLA’s ROS 2 “native” integration by introducing shared publisher/subscriber plumbing, adding new math/encoding seams with unit tests, and wiring a new ROS 2 topic-visibility default into Unreal startup.
Changes:
- Add ROS2 topic-visibility startup setting and propagate it to the streaming server’s default behavior.
- Add Ackermann control support (callback variant + Unreal handler + subscriber scaffolding) and refactor ROS2 pub/sub into Base* + *Impl templates.
- Introduce/test ROS 2 math + encoding seams (TF quaternion conversion, IMU compass, optical flow/DVS encoding, point-cloud field layouts) and expose new geometry types (Quaternion/Velocity/Acceleration/AngularVelocity) to the Python API.
Reviewed changes
Copilot reviewed 113 out of 113 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Unreal/CarlaUnreal/Plugins/Carla/Source/Carla/Settings/CarlaSettings.h | Adds a config setting for ROS2 default topic visibility on startup. |
| Unreal/CarlaUnreal/Plugins/Carla/Source/Carla/Game/CarlaEngine.cpp | Applies ROS2 topic-visibility default during engine init. |
| Unreal/CarlaUnreal/Plugins/Carla/Source/Carla/Actor/ActorROS2Handler.h | Adds AckermannControl visitor overload. |
| Unreal/CarlaUnreal/Plugins/Carla/Source/Carla/Actor/ActorROS2Handler.cpp | Implements Ackermann control -> vehicle control application. |
| Unreal/CarlaUnreal/Plugins/Carla/Source/Carla/Actor/ActorDispatcher.cpp | Switches ROS2 registration/unregistration wiring for actors and vehicles. |
| PythonAPI/examples/ros2/stack.json | Updates example sensor configuration (resolution) and adds GNSS/IMU sensors. |
| PythonAPI/examples/ros2/rviz/ros2_native.rviz | Updates RViz config (frames, lidar topic path) and adds GNSS/IMU visibility. |
| PythonAPI/carla/src/Geometry.cpp | Exposes Quaternion/Velocity/AngularVelocity/Acceleration to Python. |
| PythonAPI/carla/include/PythonAPI.h | Adds ostream formatting for new geometry types (incl. Quaternion). |
| LibCarla/source/test/common/test_transform_quaternion.cpp | Adds unit tests for TF quaternion/translation conversion seam. |
| LibCarla/source/test/common/test_streaming.cpp | Adds unit tests for ROS topic visibility default and toggles in streaming server. |
| LibCarla/source/test/common/test_radar_polar_to_cartesian.cpp | Adds unit tests for radar polar->Cartesian conversion seam. |
| LibCarla/source/test/common/test_point_cloud_fields.cpp | Adds unit tests for PointCloud2 field layouts/strides. |
| LibCarla/source/test/common/test_optical_flow_encoding.cpp | Adds unit tests for optical flow HSV->BGRA encoding seam. |
| LibCarla/source/test/common/test_imu_compass.cpp | Adds unit tests for compass->REP103 yaw quaternion seam. |
| LibCarla/source/test/common/test_dvs_event_encoding.cpp | Adds unit tests for DVS event encoding to BGR image. |
| LibCarla/source/test/common/test_camera_intrinsics.cpp | Adds unit tests for pinhole intrinsics computation seam. |
| LibCarla/source/test/common/test_ackermann_conversion.cpp | Adds unit tests for AckermannDrive->AckermannControl mapping and variant visitation. |
| LibCarla/source/carla/trafficmanager/MotionPlanStage.cpp | Updates to new vector normalization API. |
| LibCarla/source/carla/trafficmanager/LocalizationUtils.cpp | Updates to new vector normalization API. |
| LibCarla/source/carla/trafficmanager/CollisionStage.cpp | Updates to new vector normalization API. |
| LibCarla/source/carla/streaming/low_level/Server.h | Adds forwarding API to set ROS2 topic-visibility default. |
| LibCarla/source/carla/streaming/detail/Dispatcher.h | Adds ROS2 topic-visibility default flag + setter. |
| LibCarla/source/carla/streaming/detail/Dispatcher.cpp | Applies topic-visibility default to newly created streams. |
| LibCarla/source/carla/streaming/Server.h | Adds public API to set ROS2 topic-visibility default. |
| LibCarla/source/carla/ros2/types/CarlaLineInvasionPubSubTypes.h | Repurposed to AckermannDriveStamped PubSubTypes declarations (potentially inconsistent). |
| LibCarla/source/carla/ros2/types/CarlaLineInvasionPubSubTypes.cpp | Repurposed to AckermannDrive PubSubTypes implementation (potentially inconsistent). |
| LibCarla/source/carla/ros2/types/CarlaLineInvasion.h | Repurposed to AckermannDriveStamped type declaration (potentially inconsistent). |
| LibCarla/source/carla/ros2/types/CarlaLineInvasion.cpp | Removes old LaneInvasionEvent generated implementation. |
| LibCarla/source/carla/ros2/types/AckermannDriveStampedPubSubTypes.cpp | Adds generated PubSubTypes implementation for AckermannDriveStamped. |
| LibCarla/source/carla/ros2/types/AckermannDriveStamped.cpp | Adds generated type implementation for AckermannDriveStamped. |
| LibCarla/source/carla/ros2/types/AckermannDrivePubSubTypes.h | Adds generated PubSubTypes header for AckermannDrive. |
| LibCarla/source/carla/ros2/subscribers/SubscriberImpl.h | Adds shared FastDDS subscriber plumbing template. |
| LibCarla/source/carla/ros2/subscribers/CarlaEgoVehicleControlSubscriber.h | Refactors subscriber to BaseSubscriber + SubscriberImpl pattern. |
| LibCarla/source/carla/ros2/subscribers/BaseSubscriber.h | Introduces common subscriber base API. |
| LibCarla/source/carla/ros2/subscribers/AckermannControlSubscriber.h | Adds Ackermann control subscriber interface. |
| LibCarla/source/carla/ros2/subscribers/AckermannControlSubscriber.cpp | Implements Ackermann control subscriber using SubscriberImpl + conversion seam. |
| LibCarla/source/carla/ros2/subscribers/AckermannControlConversion.h | Adds FastDDS-free Ackermann conversion helper for unit tests. |
| LibCarla/source/carla/ros2/publishers/TransformQuaternion.h | Adds FastDDS-free TF quaternion conversion seam. |
| LibCarla/source/carla/ros2/publishers/RadarPolarToCartesian.h | Adds FastDDS-free radar polar->Cartesian seam. |
| LibCarla/source/carla/ros2/publishers/PublisherImpl.h | Adds shared FastDDS publisher plumbing template. |
| LibCarla/source/carla/ros2/publishers/PointCloudFieldsLayout.h | Adds FastDDS-free point-cloud field layout descriptors. |
| LibCarla/source/carla/ros2/publishers/OpticalFlowEncoding.h | Adds FastDDS-free optical-flow encoding seam. |
| LibCarla/source/carla/ros2/publishers/ImuMath.h | Adds FastDDS-free compass->orientation seam. |
| LibCarla/source/carla/ros2/publishers/DvsEventEncoding.h | Adds FastDDS-free DVS event->BGR encoding seam. |
| LibCarla/source/carla/ros2/publishers/CarlaTransformPublisher.h | Refactors TF publisher interface to BasePublisher + caching. |
| LibCarla/source/carla/ros2/publishers/CarlaSpeedometerSensor.h | Removes legacy per-type publisher header (refactor cleanup). |
| LibCarla/source/carla/ros2/publishers/CarlaSpeedometerSensor.cpp | Removes legacy per-type publisher impl (refactor cleanup). |
| LibCarla/source/carla/ros2/publishers/CarlaSemanticLidarPublisher.h | Refactors semantic lidar to CarlaPointCloudPublisher base. |
| LibCarla/source/carla/ros2/publishers/CarlaSSCameraPublisher.h | Aliases SS camera publisher to common RGB camera publisher. |
| LibCarla/source/carla/ros2/publishers/CarlaRadarPublisher.h | Refactors radar to CarlaPointCloudPublisher base. |
| LibCarla/source/carla/ros2/publishers/CarlaRGBCameraPublisher.h | Refactors RGB camera to CarlaCameraPublisher base. |
| LibCarla/source/carla/ros2/publishers/CarlaPublisher.h | Removes legacy publisher base. |
| LibCarla/source/carla/ros2/publishers/CarlaPointCloudPublisher.h | Introduces shared PointCloud2 publisher base. |
| LibCarla/source/carla/ros2/publishers/CarlaPointCloudPublisher.cpp | Implements shared PointCloud2 publisher base. |
| LibCarla/source/carla/ros2/publishers/CarlaOpticalFlowCameraPublisher.h | Refactors optical flow camera, adds encoding seam hook. |
| LibCarla/source/carla/ros2/publishers/CarlaNormalsCameraPublisher.h | Aliases normals camera publisher to common RGB camera publisher. |
| LibCarla/source/carla/ros2/publishers/CarlaMapSensorPublisher.h | Removes legacy per-type publisher header (refactor cleanup). |
| LibCarla/source/carla/ros2/publishers/CarlaMapSensorPublisher.cpp | Removes legacy per-type publisher impl (refactor cleanup). |
| LibCarla/source/carla/ros2/publishers/CarlaLineInvasionPublisher.h | Removes legacy line invasion publisher header (refactor cleanup). |
| LibCarla/source/carla/ros2/publishers/CarlaLineInvasionPublisher.cpp | Removes legacy line invasion publisher impl (refactor cleanup). |
| LibCarla/source/carla/ros2/publishers/CarlaLidarPublisher.h | Refactors lidar to CarlaPointCloudPublisher base. |
| LibCarla/source/carla/ros2/publishers/CarlaISCameraPublisher.h | Aliases IS camera publisher to common RGB camera publisher. |
| LibCarla/source/carla/ros2/publishers/CarlaIMUPublisher.h | Refactors IMU publisher to BasePublisher + PublisherImpl pattern. |
| LibCarla/source/carla/ros2/publishers/CarlaGNSSPublisher.h | Refactors GNSS publisher to BasePublisher + PublisherImpl pattern. |
| LibCarla/source/carla/ros2/publishers/CarlaDepthCameraPublisher.h | Aliases depth camera publisher to common RGB camera publisher. |
| LibCarla/source/carla/ros2/publishers/CarlaDVSCameraPublisher.h | Refactors DVS camera into image + pointcloud companion publishers + composite. |
| LibCarla/source/carla/ros2/publishers/CarlaCollisionPublisher.h | Refactors collision publisher to BasePublisher + PublisherImpl pattern. |
| LibCarla/source/carla/ros2/publishers/CarlaClockPublisher.h | Refactors clock publisher to BasePublisher + PublisherImpl pattern. |
| LibCarla/source/carla/ros2/publishers/CarlaCameraPublisher.h | Introduces shared camera publisher base (Image + CameraInfo writers). |
| LibCarla/source/carla/ros2/publishers/CarlaCameraPublisher.cpp | Implements shared camera publisher base. |
| LibCarla/source/carla/ros2/publishers/CameraIntrinsics.h | Adds intrinsics computation seam for testing. |
| LibCarla/source/carla/ros2/publishers/BasicPublisher.h | Refactors demo BasicPublisher to BasePublisher + PublisherImpl pattern. |
| LibCarla/source/carla/ros2/publishers/BasePublisher.h | Introduces common publisher base API. |
| LibCarla/source/carla/ros2/listeners/CarlaSubscriberListener.h | Removes legacy subscriber listener scaffolding (refactor cleanup). |
| LibCarla/source/carla/ros2/listeners/CarlaSubscriberListener.cpp | Removes legacy subscriber listener scaffolding (refactor cleanup). |
| LibCarla/source/carla/ros2/listeners/CarlaListener.h | Removes legacy publisher listener scaffolding (refactor cleanup). |
| LibCarla/source/carla/ros2/listeners/CarlaListener.cpp | Removes legacy publisher listener scaffolding (refactor cleanup). |
| LibCarla/source/carla/ros2/ROS2CallbackData.h | Adds AckermannControl type and extends callback variant. |
| LibCarla/source/carla/ros2/ROS2.h | Refactors ROS2 actor registration and publisher/subscriber ownership model. |
| LibCarla/source/carla/road/element/LaneCrossingCalculator.cpp | Updates to new vector normalization API. |
| LibCarla/source/carla/geom/Velocity.h | Adds Velocity geom wrapper type. |
| LibCarla/source/carla/geom/Vector3D.h | Changes normalization API (MakeUnitVector epsilon) and removes MakeSafeUnitVector. |
| LibCarla/source/carla/geom/Vector2D.h | Changes normalization to return unchanged for small vectors (removes assert). |
| LibCarla/source/carla/geom/Transform.h | Fixes rotation matrix sign convention to match corrected rotation math. |
| LibCarla/source/carla/geom/Rotation.h | Fixes RotateVector/InverseRotateVector sign convention. |
| LibCarla/source/carla/geom/RightHandedVector3D.h | Adds explicit handedness adapter helper. |
| LibCarla/source/carla/geom/Quaternion.h | Adds Quaternion type with Rotation conversion + vector basis helpers. |
| LibCarla/source/carla/geom/Math.cpp | Fixes GetForward/Right/Up vector sign convention. |
| LibCarla/source/carla/geom/AngularVelocity.h | Adds AngularVelocity geom wrapper type. |
| LibCarla/source/carla/geom/Acceleration.h | Adds Acceleration geom wrapper type. |
| LibCarla/source/carla/client/detail/Client.cpp | Updates to new vector normalization API. |
Comments suppressed due to low confidence (7)
Unreal/CarlaUnreal/Plugins/Carla/Source/Carla/Actor/ActorDispatcher.cpp:1
- On actor destruction, this always calls
UnregisterVehicleregardless of whether the actor was registered viaRegisterSensorvsRegisterVehicle. SinceROS2.hexposes bothUnregisterSensorandUnregisterVehicle, relying onUnregisterVehiclefor sensors risks leaking sensor registrations/publishers if the implementation does not remove sensor state. A concrete fix is to call bothUnregisterSensor(ActorKey)andUnregisterVehicle(ActorKey)(both should be idempotent), or introduce a singleUnregisterActorAPI that clears both code paths.
Unreal/CarlaUnreal/Plugins/Carla/Source/Carla/Actor/ActorDispatcher.cpp:1 - On actor destruction, this always calls
UnregisterVehicleregardless of whether the actor was registered viaRegisterSensorvsRegisterVehicle. SinceROS2.hexposes bothUnregisterSensorandUnregisterVehicle, relying onUnregisterVehiclefor sensors risks leaking sensor registrations/publishers if the implementation does not remove sensor state. A concrete fix is to call bothUnregisterSensor(ActorKey)andUnregisterVehicle(ActorKey)(both should be idempotent), or introduce a singleUnregisterActorAPI that clears both code paths.
Unreal/CarlaUnreal/Plugins/Carla/Source/Carla/Settings/CarlaSettings.h:1 - The new setting defaults to
true, which (together with the engine-init call) will make newly created streams ROS-visible by default whenever ROS2 is enabled. However, the newly added streaming tests and comments describe 'default off' as the out-of-the-box behavior. If out-of-the-box should remain 'off', change this default tofalse(and keep UI text consistent).
Unreal/CarlaUnreal/Plugins/Carla/Source/Carla/Settings/CarlaSettings.h:1 - The new setting defaults to
true, which (together with the engine-init call) will make newly created streams ROS-visible by default whenever ROS2 is enabled. However, the newly added streaming tests and comments describe 'default off' as the out-of-the-box behavior. If out-of-the-box should remain 'off', change this default tofalse(and keep UI text consistent).
LibCarla/source/carla/ros2/subscribers/SubscriberImpl.h:1 on_data_availableonly performs a singletake_next_sampleper callback invocation. FastDDS can deliver multiple samples before invoking the listener again, which can leave a backlog of unread samples and cause lag. Consider draining in a loop (taking until RETCODE_NO_DATA), and then only storing the most recentvalid_datasample into_messagebefore setting_new_message.
LibCarla/source/carla/ros2/subscribers/SubscriberImpl.h:1on_data_availableonly performs a singletake_next_sampleper callback invocation. FastDDS can deliver multiple samples before invoking the listener again, which can leave a backlog of unread samples and cause lag. Consider draining in a loop (taking until RETCODE_NO_DATA), and then only storing the most recentvalid_datasample into_messagebefore setting_new_message.
LibCarla/source/carla/ros2/publishers/DvsEventEncoding.h:1- This encoder relies on
bool polbeing exactly 1 byte to match the 13-byte packed wire contract.boolsize is implementation-defined, so the layout is less robust than using an explicit 1-byte type. To make the wire format unambiguous, usestd::uint8_t pol(or similar) and treat nonzero as true.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
67e5d0d to
23aaa0c
Compare
f0fd6d4 to
149c317
Compare
youtalk
left a comment
There was a problem hiding this comment.
Reviewed locally on a fresh worktree (LibCarla built with ENABLE_ROS2=ON; the new streaming tests and the full suite pass; carla-unreal-editor compiles+links clean). LGTM.
The flag plumbing is correct end-to-end — CarlaSettings::ROS2TopicVisibility → FCarlaEngine::NotifyInitGame → Server → low_level::Server → Dispatcher — and the boolean logic is not inverted (flag on ⇒ new streams auto-EnableForROS; off ⇒ legacy explicit-enable behavior). It's correctly gated on Settings.ROS2 and #if WITH_ROS2, and the example/rviz additions (GNSS + IMU) use attribute names and frame_ids that match the publishers.
149c317 to
2533bf8
Compare
Adds a ROS2TopicVisibility startup setting that selects whether sensor topics are visible to ROS 2 from the moment their stream is created. When true (the default), every topic offered by the implementation publishes from startup without an explicit enable_for_ros call; when false, only the sensors explicitly enabled via enable_for_ros() are visible. The setting is applied to the streaming server only when ROS 2 is enabled, so non-ROS-2 runs keep the previous behavior. Ported slice: only the topic-visibility-default portion of the upstream commit is brought to ue5-dev, adapted to its existing stream-level visibility model. The multistream/per-actor scaffolding (RpcServerInterface, the Message.h relocation, the Session abstraction, the Dispatcher shared_ptr migration, per-actor visibility) is intentionally omitted: it is groundwork for out-of-scope work and has no consumer on ue5-dev. Upstream commit message: Extend ROS2 support Step 1: Prepare for mulistream (carla-simulator#9431) Prepare server for multistream support and ROS2 client calls - Moved Blueprint handling sources to carla/actor from carla/client to allow using of that functionalilty within ROS2 server calls - Moved streaming/detail/tcp/Message.h -> streaming/detail/Message.h to be deployed for non-tcp communication streams like ROS2 - Moved general (tcp-independent parts) from tcp/ServerSession to streaming/detail/Session.h to be used for ROS2 streams. Had to rename a Write() function to WriteMessage because of virtual function overloaded with template function not possible. - Make Dispatcher a shared_pointer to be able to access from ROS2 - CarlaServer uses rpc::RpcServerInterface to communicate with TCP-Clients and upcoming DDS-Client - Dynamic switching on ROS visibility is possible for all actors now - Default startup behavior is selectable by ROS2TopicVisibility parameter in DefaultGame.ini: If true, then all and every topic that is currently offered by the implementation is visible from the beginning. If false, then only the sensors/actors created via the Client- or ROS-Interface are visible by defaults. Others can be activated via EnableForRos() calls (this allows for disabling interfaces e.g. for leaderboard) (adapted from ue4-dev 3b938d7) Co-Authored-By: berndgassmann <bernd.gassmann@motor-ai.com>
…nfig Add gnss and imu sensors to the native ROS 2 example stack alongside the existing rgb camera and lidar, and surface them in the rviz config as TF frames under the hero frame so their poses render next to the camera and point-cloud displays.
2533bf8 to
a7aee27
Compare
|
This PR is ready for code review |
Description
Adds a
ROS2TopicVisibilitystartup setting that controls whether sensor topics are visible to ROS 2from the moment their stream is created. When
true(the default), every topic offered by theimplementation publishes from startup without an explicit
enable_for_roscall; whenfalse, only thesensors explicitly enabled through
Sensor.enable_for_ros()are visible (useful for disabling interfaces,for example on the leaderboard).
On ue5-dev a sensor only produces and publishes data while
AreClientsListening()is true, which holdswhen a TCP client is subscribed, the stream is force-active, or the stream is marked enabled-for-ros.
That ROS flag defaults to off per stream and today flips on only through the per-sensor
enable_sensor_for_rosRPC, so a freshly spawned sensor stays silent on ROS 2 until a client enables it.This setting lets the server flip the default for every newly created stream at startup.
Lands the topic-visibility-default slice of upstream
3b938d718(#9431), adapted to ue5-dev's existingstream-level visibility model. The multistream and per-actor scaffolding from that commit
(
RpcServerInterface, theMessage.hrelocation, theSessionabstraction, the Dispatcher sharedpointer migration, per-actor visibility) is not included: it is groundwork for the out-of-scope DDS
middleware series and the ROS-2-as-RPC-client work, and has no consumer on ue5-dev.
Closes: #9627
Changes
Settings/CarlaSettings.h: newconfigUPROPERTYROS2TopicVisibility(defaulttrue), bound toDefaultGame.inilike the existingROS2flag.streaming/detail/Dispatcher.{h,cpp}: a_topic_visibility_default_enabledflag with aSetROS2TopicVisibilityDefaultEnabled(bool)setter; when set, every stream created throughMakeStream()and through theGetToken()create-on-miss path starts enabled for ROS 2.streaming/Server.handstreaming/low_level/Server.h: thin pass-throughs for the new setter. Theexisting stream-id based
EnableForROS/DisableForROS/IsEnabledForROSsurface is unchanged.Game/CarlaEngine.cpp: inside the existingif (Settings.ROS2)block ofNotifyInitGame, apply theconfigured default to the streaming server before any sensor stream is created. Gating on
Settings.ROS2keeps non-ROS-2 runs from forcing every sensor active each tick.test/common/test_streaming.cpp: four new GTest cases covering the default-off behavior, thedefault-on behavior through both the
GetTokenandMakeStreamstream-birth paths, and the manualenable/disable round-trip.
Where has this been tested?
libcarla_test_serverandlibcarla_test_clientboth pass the fullstreaming.*group (9/9: the 5pre-existing cases plus the 4 new
ros_topic_visibility_*cases). The Releasepackagetarget buildsgreen with no new warnings on the files this PR touches.
Possible Drawbacks
ROS2TopicVisibility = true(the default) and ROS 2 enabled, every sensor stream is active fromcreation, so sensors produce and publish data even when no client has subscribed and no explicit
enable_for_roswas called. Set it tofalseto restore the opt-in behavior. Non-ROS-2 runs areunaffected, since the default is applied only when ROS 2 is enabled.
the primary streaming server. Per-sensor
enable_for_roscontinues to work across the multigpu pathas before.
Series status
Each sub-PR's branch is cut from the previous sub-PR's tip; the GitHub PR opens against the predecessor's
branch and is auto-retargeted to
ue5-devonce the predecessor merges, at which point the diff narrows tothis PR's own commit.
Related
Part of the 8-PR port of
ue4-devROS 2 enhancements toue5-dev(issue #9627, discussion #9581Category 2).
This change is