Commit 23b3e21
feat(ros2): [3/8] unify camera publishers under CarlaCameraPublisher (carla-simulator#9746)
* feat(ros2): unify camera publishers under CarlaCameraPublisher
Lands the camera-image slice of upstream ROS2 Refactor cd3d640 on
ue5-dev, plus the K-matrix correctness fix from 86ebadc and the
<cmath> include from 02a83ef:
* New CarlaCameraPublisher.{h,cpp} base inherits BasePublisher (from
PR-2) and owns the sensor_msgs/Image + sensor_msgs/CameraInfo
DataWriter pair via PublisherImpl<>. CameraIntrinsics.h hosts the
pinhole-camera intrinsics math (fx = width / (2 * tan(fov * pi /
360))) in a header-only seam so the K matrix can be exercised by a
GTest without linking FastDDS. The PublisherImpl<> forward
declarations in the header keep <fastdds/...> includes out of
carla-server's ROS2.cpp compile unit, same pattern PR-2 uses for
AckermannControlSubscriber.
* CarlaRGBCameraPublisher collapses to a 24-LOC header subclass with
GetChannels() = 4 and GetEncoding() = "bgra8". CarlaDepth, CarlaSS,
CarlaIS, CarlaNormals camera publishers become `using` aliases of
CarlaRGBCameraPublisher (their wire format is uniform 4-channel
BGRA uint8 on ue5-dev today: encoding "bgra8", step = width * 4).
Their per-sensor .cpp files are deleted.
* CarlaOpticalFlowCameraPublisher stays as a non-alias subclass that
overrides ComputeImage to reinterpret the post-header buffer as
const float * and run the HSV colour-wheel encoding. The encoding
math is lifted out of the publisher into a header-only seam
OpticalFlowEncoding.h so the GTest can exercise it without
FastDDS. This intentionally diverges from ue4-dev (which aliased
OpticalFlow to RGB and shipped raw float bytes as BGRA);
preserving the encoded form keeps the topic visually useful for
ROS 2 subscribers consuming ue5-dev today.
* ROS2.cpp gains a private GetOrCreateCameraSensor<CameraT> template
that replaces the six per-camera switch cases in GetOrCreateSensor.
The five BGRA8 camera dispatches in ProcessDataFromCamera collapse
to one shared block; OpticalFlow keeps its own dispatch because
the buffer cast differs. A parallel _camera_publishers map tracks
CarlaCameraPublisher-derived publishers (which do not inherit the
legacy CarlaPublisher base); RemoveActorRosName clears it alongside
_publishers and _transforms.
* CarlaLineInvasionPublisher.{h,cpp} and the four CarlaLineInvasion
types files are deleted along with their three references in
ROS2.cpp (include, make_shared, dynamic_pointer_cast). The
publisher was wired in but had no Unreal-plugin call sites on
ue5-dev.
* DVS publisher migration is intentionally deferred to PR-4. The
unified CarlaDVSPublisher composite needs a CarlaPointCloudPublisher
base for its event-cloud half, and that base is introduced in PR-4
alongside the lidar/radar/semantic-lidar migration.
Test coverage:
* LibCarla/source/test/common/test_camera_intrinsics.cpp exercises
ComputeIntrinsics against known FOV/resolution combinations and
guards against the pre-86ebadcee operator-precedence regression
(the old spelling applied M_PI / 360 to tan(fov) instead of to
fov, returning ~0.06 for a 90deg FOV instead of the correct 960).
* LibCarla/source/test/common/test_optical_flow_encoding.cpp
exercises EncodeFlowPixelToBgra and EncodeFlowImageToBgra with
7 cases covering zero flow, hue hemispheres, brightness clamp,
alpha-byte invariant, and buffer length.
(adapted from ue4-dev cd3d640)
(adapted from ue4-dev 86ebadc)
Co-Authored-By: joel-mb <joel.moriana@gmail.com>
Co-Authored-By: csonthomisi <48515100+csonthomisi@users.noreply.github.com>
* fix(ros2): address Copilot review on PR carla-simulator#9746
Round-1 follow-up on top of 30b5c8e addressing the four inline comments
left by GitHub Copilot:
* test_optical_flow_encoding.cpp: add an explicit <algorithm> include so
std::max({...}) does not rely on a transitive pull from the production
header.
* publishers/OpticalFlowEncoding.h: rewrite the saturation-curve comment.
With shift = 0.999 and a = 1 / log(0.1 + shift) the intensity reaches 1
at magnitude ~= 0.1 (not at unit magnitude as the prior comment claimed).
Math is unchanged; ue5-dev's existing wire format is preserved.
* publishers/PublisherImpl.h: null-guard Publish(). If Init() failed (or
was never called), the previous code dereferenced _datawriter and
crashed; now it logs once and returns false.
* publishers/CarlaCameraPublisher.cpp: log if either PublisherImpl<>::Init
fails inside the constructor instead of silently dropping the return
value. Pairs with the PublisherImpl guard above.
* Actor/ActorDispatcher.cpp: drop the redundant RemoveActorRosName call
after RemoveActorCallback. RemoveActorCallback already routes to
UnregisterVehicle which clears the ros_name maps, so the explicit
second call was a no-op. Comment updated to reflect the actual cleanup
chain.
No CHANGELOG entry (consolidated in PR-12).
* fix(ros2): address Copilot round-2 review on PR carla-simulator#9746
Round-2 follow-up on top of 0dbc1ef addressing the three new inline
comments from the post-round-1 Copilot pass:
* publishers/CarlaCameraPublisher.cpp: Publish() now calls the image and
camera_info writers unconditionally and combines the results. The
previous "_impl_camera_info->Publish() && _impl_image->Publish()" form
short-circuited the image publish whenever the camera_info publish
failed; the new form preserves the per-sensor publishers' behaviour
and ships the image regardless of header-writer state.
* ros2/ROS2CallbackData.h: explicit #include <cstdint>. VehicleControl
declares an int32_t gear field but the header only pulled <variant>
and <functional>. Avoids relying on transitive includes.
* ros2/ROS2.h: explicit #include <string>. Many public APIs take
std::string arguments; the header previously pulled it transitively
through <unordered_map> / <vector>.
No CHANGELOG entry (consolidated in PR-12).
* fix(ros2): enforce camera publisher type on cache hit (PR carla-simulator#9746 review)
GetOrCreateCameraSensor cached the camera publisher by actor only, so a
cache hit returned the existing publisher regardless of the requested
CameraT. Under the current one-actor-one-camera-type invariant this is
safe, but if an actor were ever dispatched as two camera types the
first-created type would silently win and, for example, route optical
flow float bytes through an RGB publisher.
The cache-hit path now dynamic_pointer_cast<CameraT> the stored
publisher. RGB/Depth/SS/IS/Normals all alias CarlaRGBCameraPublisher so
a hit across them casts cleanly and is expected; only an RGB to
OpticalFlow mismatch fails the cast, in which case the sample is logged
and skipped (callers already guard on a null publisher) instead of
corrupting the published image.
Add test_camera_publisher_types.cpp to lock the type partitioning the
guard relies on (the BGRA cameras collapse to one type, optical flow
stays a distinct polymorphic sibling). The publisher constructors live
in the FastDDS-linked native build and are not linked into the test
binaries, so the test asserts the relationships at compile time via type
traits.
Add stack_cameras.json under PythonAPI/examples/ros2 spawning all six
unified camera types for manual ROS 2 validation of the publisher path.
* fix(ros2): rate-limit publisher Init-failure log to one-shot
A failed Init() leaves the publisher cached, so Publish() runs once per
frame for the sensor's lifetime. Log the uninitialized-DataWriter guard
only once instead of every frame to avoid flooding the server log.
---------
Co-authored-by: joel-mb <joel.moriana@gmail.com>
Co-authored-by: csonthomisi <48515100+csonthomisi@users.noreply.github.com>1 parent 0056362 commit 23b3e21
31 files changed
Lines changed: 948 additions & 4058 deletions
File tree
- LibCarla/source
- carla/ros2
- publishers
- types
- test/common
- PythonAPI/examples/ros2
- Unreal/CarlaUnreal/Plugins/Carla/Source/Carla/Actor
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
15 | | - | |
16 | | - | |
17 | 15 | | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
18 | 19 | | |
19 | 20 | | |
20 | 21 | | |
| |||
38 | 39 | | |
39 | 40 | | |
40 | 41 | | |
| 42 | + | |
41 | 43 | | |
42 | 44 | | |
43 | 45 | | |
| |||
162 | 164 | | |
163 | 165 | | |
164 | 166 | | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
165 | 178 | | |
166 | 179 | | |
167 | 180 | | |
| |||
175 | 188 | | |
176 | 189 | | |
177 | 190 | | |
| 191 | + | |
178 | 192 | | |
179 | 193 | | |
180 | 194 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
9 | | - | |
| 9 | + | |
10 | 10 | | |
| 11 | + | |
11 | 12 | | |
12 | 13 | | |
13 | 14 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
Lines changed: 130 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
Lines changed: 76 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
0 commit comments