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 support for non standard point step sizes #199

Merged
merged 4 commits into from
Aug 15, 2022
Merged

Conversation

Flova
Copy link
Contributor

@Flova Flova commented Jul 29, 2022

Fixes #197

Signed-off-by: Florian Vahl <florian@flova.de>
Signed-off-by: Florian Vahl <florian@flova.de>
Signed-off-by: Florian Vahl <florian@flova.de>
@gbiggs
Copy link
Member

gbiggs commented Jul 31, 2022

Thanks for the fix! Could you please add a test to the tests file that verifies this does fix the issue?

Signed-off-by: Florian <florian@flova.de>
@Flova
Copy link
Contributor Author

Flova commented Aug 1, 2022

Thanks for your quick response @gbiggs! I added the test and it seems to solve the issue.

@gbiggs
Copy link
Member

gbiggs commented Aug 1, 2022

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@Flova
Copy link
Contributor Author

Flova commented Aug 2, 2022

Do you have an idea what's causing the tests to fail? The option manager traceback seems unrelated to the changes from this or.

@clalancette
Copy link
Contributor

Do you have an idea what's causing the tests to fail? The option manager traceback seems unrelated to the changes from this or.

That's a separate issue that we are addressing in ros2/ci#675

@Flova
Copy link
Contributor Author

Flova commented Aug 2, 2022

Okay 👍

@gbiggs
Copy link
Member

gbiggs commented Aug 15, 2022

Another CI run:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@gbiggs gbiggs merged commit 57f1567 into ros2:rolling Aug 15, 2022
@gbiggs
Copy link
Member

gbiggs commented Aug 15, 2022

Thanks for the contribution!

@Flova Flova deleted the fix/197 branch August 16, 2022 08:09
gbiggs pushed a commit that referenced this pull request Nov 24, 2022
* Remove reference to old impementation

Signed-off-by: Florian Vahl <florian@flova.de>

* Add support for non standard point step sizes

Signed-off-by: Florian Vahl <florian@flova.de>

* Fix code style

Signed-off-by: Florian Vahl <florian@flova.de>

* Add test

Signed-off-by: Florian <florian@flova.de>

Signed-off-by: Florian Vahl <florian@flova.de>
Signed-off-by: Florian <florian@flova.de>
gbiggs added a commit that referenced this pull request Nov 25, 2022
* Remove reference to old impementation

Signed-off-by: Florian Vahl <florian@flova.de>

* Add support for non standard point step sizes

Signed-off-by: Florian Vahl <florian@flova.de>

* Fix code style

Signed-off-by: Florian Vahl <florian@flova.de>

* Add test

Signed-off-by: Florian <florian@flova.de>

Signed-off-by: Florian Vahl <florian@flova.de>
Signed-off-by: Florian <florian@flova.de>

Signed-off-by: Florian Vahl <florian@flova.de>
Signed-off-by: Florian <florian@flova.de>
Co-authored-by: Florian Vahl <florian@flova.de>
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.

[sensor_msgs_py] Missing handling of point_step value in PointCloud2 message in read_points() fcn
3 participants