Port FW 2.0 and 2.1 changes from ouster_examples

[MattSYoung]

ouster_client

• From ouster_example #239.
  • Ported Optional code:
  • Optional header and license file are now present in their own folder: include/ros2_ouster/client/optional-lite/
  • Ported some mode-getting functions that use Optional to types.cpp
  • Ported the sensor_config struct
  • Ported sensor_config struct to types.hpp
  • Ported the get/set_config functions for sensor_config to client.cpp/.hpp
  • Ported some documentation updates, and a != operator for LidarScan objects to lidar_scan.h.
  • Ported/expanded data objects added by ouster_example #239
  • Ported several additional enumerated types (e.g. OperatingMode and MultipurposeIOMode) to types.h
  • Ported parsing functions for additional enumerated types to types.cpp
  • Ported ColumnWindow definition and added it as a parameter to data_format struct, which is the only existing struct changed.
  • In lidar_scan.cpp, ported some changes to make_xyz_lut and cartesian functions
• From ouster_example/#246
  • Removed a json include incorrectly added to types.h
• From ouster_example #259
  • Increased default timeout time from 30 to 60 seconds for init_client and read_imu_packet
  • In netcompat.cpp, changed non_blocking_mode from off to on (0 to 1) if using Win32.
  • Ported signal_multiplier parameter
  • Added parameter to sensor_config in types.h
  • Added signal_multiplier parameter setting to some functions in client.cpp and types.cpp
• Updated changelog

General Changes

• Changed indenting in client/types/lidar_scan source header files to be more consistent with existing indenting style.

[SteveMacenski]

Added new functions to types/lidar/client source and header files

What "new" did you add? I think I was pretty clear that nothing in client/ will be merged if it diverges with the ouster_example repository at all

[SteveMacenski]

Make sure to run the ROS2 linter suite, it looks like you reformatted some stuff that I don't know if it'll fail, but looks like to me that you run that risk

[MattSYoung]

What's this new license in here?!

It is the license that was added to the client lib for the optional-lite library, here for reference: #239 - /ouster_client/include/optional-lite/LICENSE.txt. I copied it as-is because I assumed it was necessary for usage of the Optional code in this repository.

What "new" did you add? I think I was pretty clear that nothing in client/ will be merged if it diverges with the ouster_example repository at all

Sorry for not being clearer. I mean "new" relative to the previous version of ros2_ouster. I haven't added any brand-new functions, only ones that were ported from the ouster_example repo. I was referring specifically to some of the getter and setter functions that were added in ouster example PR#239

Make sure to run the ROS2 linter suite, it looks like you reformatted some stuff that I don't know if it'll fail, but looks like to me that you run that risk

Ah so there is a linter suite to be run? Do you have any links to that? Some of the formatting seemed inconsistent, so I tried to modify the code I was porting to match what seemed to be the most commonly used conventions with newlines and whitespace etc.

[SteveMacenski]

Got it, thanks for the explanation

Sorry for not being clearer. I mean "new" relative to the previous version of ros2_ouster. I haven't added any brand-new functions, only ones that were ported from the ouster_example repo. I was referring specifically to some of the getter and setter functions that were added in ouster example PR#239

Just to verify before I go deeper into this PR: These are all changes in ouster example ported over here without any additions / changes to the client/ code? Changes to the OusterDriver (e.g. non-client code) to be compatible is OK. Though I don't see any changed files outside of client/ so am I correct in saying you've added nothing and its simply a port over? This is a bit of code, so I'd like to review it with a complete understanding of what I'm looking at. If its just ported, I can shim and merge. If there are particular areas you changed, I need to know to look at those a bit more.

Ah so there is a linter suite to be run? Do you have any links to that? Some of the formatting seemed inconsistent, so I tried to modify the code I was porting to match what seemed to be the most commonly used conventions with newlines and whitespace etc.

Yes.... but it makes me wonder now if this is properly linted in its current form. Usually I'm pretty picky about that, but since we're pulling code from Ouster themselves, we don't have alot of control over that so I probably just :shrug: on this repo. But in general, ROS2 does have its own style guide and linters built in (nicely!). You can enable them in the cmakelists but you can run the CLI applications of them with ament_cpplint and ament_uncrustify (the 2 relevant ones for C++ code, another dozen of linters for python, cmake, xml, etc)

[MattSYoung]

I don't see any changed files outside of client/ so am I correct in saying you've added nothing and its simply a port over?

Yes that is correct. The changes mostly consist of porting the Optional header and license file, porting the sensor_config struct, and then modifications to the types / client / lidar_scan source and header files to accomodation this new struct. These changes are also direct ports.

Because the use of the structs in the client folder is different between the ROS1 / ROS2 drivers, I have not implemented any of the changes to the ROS1 driver. This is because those changes should happen in a separate PR, and because the sensor_config struct has a lot of overlap with your Metadata struct, and so I suspect that future changes might involve re-writing a lot of code to replace Metadata with sensor_config. I'll make the changelog and description for this PR more detailed so you have a better idea of what exactly has changed.

but it makes me wonder now if this is properly linted in its current form ... But in general, ROS2 does have its own style guide and linters built in (nicely!

It quite possibly isn't. Formatting in the client folder is definitely different in some ways (mostly indenting from what I've noticed) from the original source code in ouster_example. The few changes to formatting that I made were mostly to make the code a smidgen more readable. Now that I think about it more, if there is going to be a push for consistent formatting, that should probably also be its own PR.

Thanks for bringing the ROS2 linting stuff to my attention, I'll have a read of that shortly.

[MattSYoung]

Updated the readme. There's nothing else I think needs to be added. If you're happy with the changes and it doesn't look like I've missed anything, then it should be good to go.

[SteveMacenski]

Thanks!!