Update Ring field data type
girvenavery2022 opened this issue · 5 comments
OS
- Ubuntu 20.04
Ros2 version
- Galactic
Branch
- Galactic
Information
As of now, the Ring field is defined in the point.h file as a uint8_t. While this doesn't cause any issues in the node so to speak, but it can cause issues down stream like decreasing the portability of the Ros2 Ouster driver along with the reusability of the users code for those who use ignition gazebo for simulation or also have other 3D Lidars like the Velodyne because they define their ring datatype as a uint16_t.
This can be solved by changing the ring data type from a std::uint8_t to a std::uint16_t. I implemented this fix on my local computer as well as tested it on an Ouster OS1 16 sensor to confirm it fixes the issue and it indeed does.
I can make a PR with this change so please let me know!
That sounds reasonable, but I think you should also file the ticket / PR with the ROS 1 drivers where this was taken from. We try to keep all of the "client" code the same between them so as Ouster makes updates we can ingest them easily here. If you can make them there, I'd be happy to pull them in here as well
ok cool. I'll fill out an issue/PR with the ros1 Ouster driver as well. I've only skimmed the ros1 code so I'll have to find where they define it at
There should be the exact same file elsewhere in the project
that is correct. I made the fix on the ros1 driver and created an issue for it!
Hi @SteveMacenski,
I wanted to give you an update on this issue. The maintainers of the Ros1 Repository just got back to me and it is considered a breaking change with the ros bags, So they said they will merge my PR in with the next Breaking change Update sometime this year.