search

ros-drivers / ros2_ouster_drivers

ROS2 Drivers for the Ouster OS-0, OS-1, and OS-2 Lidars
https://ouster.com/
Apache License 2.0
134 stars 79 forks source link

Add support for rev7 fw v3.0.1 ouster sensors #137

Closed Imaniac230 closed 2 months ago

Imaniac230 commented 1 year ago

We recently bought the a new rev7 OS-0-128 sensor with firmware v3.0.1. Using the available ros2 drivers form ouster currently still seems suboptimal even when increasing the qos buffer sizes.

I think it would be useful to have support for the newer sensors here as well, so i started an attempt to port the new additions from ouster's sdk to here. This relates to https://github.com/ros-drivers/ros2_ouster_drivers/issues/133.

This PR is currently a heavy work in progress and the code will probably still change. I'll also probably clean up the commits and make a new PR once it is fully review ready.

At this point, however, the updated driver with all 4 processors is functional with our fw3 ouster using all of its lidar resolution and frequency modes. Unfortunately, we do not posses any older sensors, so I'm not able to verify breaking compatibility with older versions. Since this involved quite substantial changes in the client as well as the parsing code, I would find helpful any feedback to push this further to completion correctly with respect to the design goals.

Used platform:

What has been changed:

  1. The client code was almost completely refactored according to the ouster-sdk implementation, which should be backwards compatible down to fw 2.0 (compatibility has not been tested).
  2. Most refactors were required in the LidarScan, ScanBatcher, and packet_format structures, and the XYZLut construction.
  3. The processors were updated to support these changes, however their internal function should remain unchanged.
  4. Similar additions have been made to the sensor and ouster_driver code.

Has been added and is working:

  1. The http client used to communicate with the fw3 sensor was ported as is from the ouster-sdk and works.
  2. Support for the different lidar packet profile types is functional.
  3. All four processors produce correctly parsed data using the packet profiles.
  4. The additional 4096x5 lidar mode is working.
  5. Apart from maintaining backwards compatibility, all previous functionality should have remained unobstructed.

What currently still isn't implemented or doesn't work and is in progress: 1. There seems to be a problem with some map field accesses when using the reduced data rate profile RNG15_RFL8_NIR8, which currently throws out of bounds exceptions. Something in the parsing code was probably not ported fully correctly. https://github.com/ros-drivers/ros2_ouster_drivers/pull/137#issuecomment-1583142419

  1. Parsing packets with the RNG19_RFL8_SIG16_NIR16_DUAL profile works, however the additional publishers on the driver side have not yet been implemented. This shouldn't be much of a problem to implement.
  2. The http interfaces for fw versions 2.1 and 2.2 as they are in the ouster-sdk are not ported yet. This should also be quite straightforward, however I will not be able to test it out.
  3. The tcp client implementation has also been ported as is from the ouster-sdk, however it has not been tested. This should be testable on our sensor since the new rev should still support the tcp commands.
  4. The reset and reactivate sensor semantics using ros2 lifecycle state transitions have not been enabled. I haven't yet verified whether the lifecycle transitions as they are used by ouster are compatible with this implementation.
  5. The private helper methods added to sensor and full_rotation_accumulator may require more sensible placement.
  6. The reflectivity image output has not yet been added and the range image output doesn't seem to be quite right. The used _range_multiplier value is probably not compatible with the changes.

Finally, is there any clang-format config that I could use to keep the formatting consistent with the current code base? I haven't quite found any suitable settings yet.

Imaniac230 commented 1 year ago

Problem 1. was fixed in commit https://github.com/ros-drivers/ros2_ouster_drivers/pull/137/commits/f0ccedd92ff25c872dad20c5c7bcbb0631f1ad6d.

Parsing should now be correct for all combinations of packet profiles and lidar resolutions in all processors.

SteveMacenski commented 1 year ago

There are some discussions with ouster right now about deprecating this version and having them support their own version from now on. So, its not likely that anything of this nature will be merged here because it'll be entirely rewritten / superseded in the coming weeks to month(s).

Imaniac230 commented 1 year ago

Interesting, thanks for the info. Are they planning to merge the current concepts from this one or do they have a completely new version in the works?

I'll continue to keep playing with this a bit in the mean time, if it's no problem, since it behaves more reliably for us than the current ouster version.

SteveMacenski commented 1 year ago

Are they planning to merge the current concepts from this one or do they have a completely new version in the works?

Completely new. It is their current version in the github under the ROS 2 branch. There's an open PR with a number of changes and improvements (https://github.com/ouster-lidar/ouster-ros/pull/146) that you should check out and give feedback on it ASAP since that will be effectively what replaces this (minut any issues brought up by me or the community during reintregration)

Samahu commented 1 year ago

Thanks Steve for linking the PR: https://github.com/ouster-lidar/ouster-ros/pull/146. Yes the plan is to fully combine the two drivers into one implementation moving forward. The linked PR bridges the gap between the two drivers and and improves many aspects on both sides.

Imaniac230 commented 2 months ago

Closing this, as it is not valid anymore.