Skip to content

fix(ros2): correct PointField descriptors in DVS camera publisher#9649

Open
youtalk wants to merge 1 commit intocarla-simulator:ue5-devfrom
youtalk:fix/dvs-pointfield-datatypes
Open

fix(ros2): correct PointField descriptors in DVS camera publisher#9649
youtalk wants to merge 1 commit intocarla-simulator:ue5-devfrom
youtalk:fix/dvs-pointfield-datatypes

Conversation

@youtalk
Copy link
Copy Markdown
Contributor

@youtalk youtalk commented Apr 7, 2026

Description

Fix a copy-paste bug in CarlaDVSCameraPublisher::SetPointCloudData where descriptor4 (for the pol field) was incorrectly assigned using descriptor3, causing:

  1. descriptor3 (t field) to be overwritten with pol field values
  2. descriptor4 to remain uninitialized

The PointCloud2 message published by the DVS camera sensor had incorrect field metadata, making it unusable by downstream ROS 2 consumers. I found it in the #9638 (comment).

Where has this been tested?

  • Platform(s): Ubuntu 24.04
  • Python version(s): N/A (C++ only)
  • Unreal Engine version(s): 5.5

Possible Drawbacks

None. This is a straightforward bug fix with no behavioral change beyond correcting the PointField metadata.


This change is Reviewable

Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
@update-docs
Copy link
Copy Markdown

update-docs bot commented Apr 7, 2026

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.

@youtalk youtalk marked this pull request as ready for review April 7, 2026 19:15
@youtalk youtalk requested a review from a team as a code owner April 7, 2026 19:15
Copilot AI review requested due to automatic review settings April 7, 2026 19:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes the ROS 2 DVS camera PointCloud2 field descriptors so the published message has correct metadata for downstream consumers.

Changes:

  • Fixes a copy/paste error so the pol PointField descriptor is configured on descriptor4 (instead of overwriting descriptor3).
  • Ensures the pol field descriptor is no longer left uninitialized.
Comments suppressed due to low confidence (1)

LibCarla/source/carla/ros2/publishers/CarlaDVSCameraPublisher.cpp:499

  • PointCloud2 dimensional fields and steps appear internally inconsistent: the function copies height * width bytes into data, sets point_step to sizeof(DVSEvent), but then sets width directly on the message and computes row_step as width * point_size. With the current call site passing width = elements * sizeof(DVSEvent), this makes row_step equal to elements * sizeof(DVSEvent)^2, which won’t match the actual data buffer size. Please set pc.width to the number of points (likely elements with height = 1), compute row_step = pc.width * point_step, and size/copy the data buffer as elements * point_step bytes.
    descriptor4.count(1);

    const size_t point_size = sizeof(carla::sensor::data::DVSEvent);
    _point_cloud->_pc.width(width);
    _point_cloud->_pc.height(height);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

@JesusAnaya JesusAnaya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed. The descriptor block is correct: all four fields (x, y, t, pol) are now independently initialized and the offsets/datatypes match the DVSEvent memory layout. Clean fix.

@youtalk
Copy link
Copy Markdown
Contributor Author

youtalk commented Apr 14, 2026

@Blyron @LuisPovedaCano This is a bug fix. Please review it.

@LuisPovedaCano LuisPovedaCano self-assigned this Apr 14, 2026
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.

4 participants