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

Cmakelists: Fix install directories #258

Merged
merged 4 commits into from
Nov 19, 2023

Conversation

ovaag
Copy link
Contributor

@ovaag ovaag commented Nov 15, 2023

This fixes issues of including headers from ouster_client or nonstd when using install space.

Folder layout on current master:
image

Note e.g. os_point.h being located in install/include/ouster_ros/include/ouster_ros/os_point.h
instead of just install/include/ouster_ros/os_point.h where I would expect it to be.

How install/include looks after this PR:
image
image
and so on..

This means that if you are using e.g. stagger function from lidar_scan.h in you own ros_pkg, you can now:

#include <ouster/lidar_scan.h>
ouster::stagger(my_img,pixel_shift_vec)

Related Issues & PRs

Summary of Changes

Update install path in CmakeLists.txt

Validation

Included header from ouster_client and compiled with install space. It works!

@Samahu Samahu self-requested a review November 16, 2023 00:46
@Samahu
Copy link
Contributor

Samahu commented Nov 16, 2023

This makes sense but only concern is there were different dependencies (ROS packages) using the nonstd::optional library and they by default did the same thing, wouldn't they collide when installing to the same folder.

CMakeLists.txt Outdated Show resolved Hide resolved
Copy link
Contributor

@Samahu Samahu left a comment

Choose a reason for hiding this comment

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

Besides changing the path for nonstd/optional.hpp as noted previously we would need the following to have this merged:

  • increase package minor version (0.11.0 to 0.12.0)
  • add a comment to the change logs in the unreleased section about this change

CMakeLists.txt Outdated Show resolved Hide resolved
@ovaag
Copy link
Contributor Author

ovaag commented Nov 19, 2023

Rebased on latest master, increased package.xml minor version and added comment in changelog now.

@ovaag ovaag requested a review from Samahu November 19, 2023 12:22
@Samahu Samahu merged commit 9bb8e3f into ouster-lidar:master Nov 19, 2023
1 check passed
@Samahu Samahu added the enhancement New feature or request label Nov 19, 2023
@ovaag ovaag deleted the fix-install-dirs branch November 21, 2023 18:29
tobii-ho pushed a commit to StarkStrom-Driverless/ouster-ros that referenced this pull request Dec 27, 2023
* Add support for automatic UDP destination in simple_viz and ROS (ouster-lidar#255)
* Add a read timeout for TCP sockets (ouster-lidar#258)
* Fall back to ipv4 when ipv6 is disabled via kernel parameters (ouster-lidar#261)
* Fix open3d example crash on macos (ouster-lidar#267)
tobii-ho pushed a commit to StarkStrom-Driverless/ouster-ros that referenced this pull request Jan 23, 2024
* Add support for automatic UDP destination in simple_viz and ROS (ouster-lidar#255)
* Add a read timeout for TCP sockets (ouster-lidar#258)
* Fall back to ipv4 when ipv6 is disabled via kernel parameters (ouster-lidar#261)
* Fix open3d example crash on macos (ouster-lidar#267)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants