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

Homologation of ouster client for ROS / ROS 2 support #8

Closed
SteveMacenski opened this issue Jan 7, 2021 · 5 comments · Fixed by #170
Closed

Homologation of ouster client for ROS / ROS 2 support #8

SteveMacenski opened this issue Jan 7, 2021 · 5 comments · Fixed by #170
Assignees
Labels
enhancement New feature or request

Comments

@SteveMacenski
Copy link

How can we combine the changes that we've made in ROS2 to enable different time sources into the ouster_client so that we can make migration between firmware version easier

@SteveMacenski
Copy link
Author

SteveMacenski commented Jan 7, 2021

After, it would be good to have ouster_client in its own repository we can release as binaries so that both the ROS1 and ROS2 drivers can share them without copy-paste action. I can probably help with that.

@dmitrig
Copy link

dmitrig commented Jan 16, 2021

You mean making the timestamp mode configurable? If I understand the changes in the ROS2 repo correctly, we've already added that functionality here:

https://github.com/ouster-lidar/ouster_example/blob/5c00aa97ee8244072ed9bf14870facf57d4aab98/ouster_client/include/ouster/client.h#L51

The only part that's missing is the "fake" mode that sets timestamps based on the host clock. Happy to add that here if it's necessary.

Re: splitting out ouster_client: is that really necessary if it's possible to build just the client binaries from this repo?

@dmitrig dmitrig added the enhancement New feature or request label Jan 18, 2021
@SteveMacenski
Copy link
Author

You mean making the timestamp mode configurable? If I understand the changes in the ROS2 repo correctly, we've already added that functionality here:

Yes, but I think we added a couple of new options including using ROS time. If you've added the different modes, awesome! Just need to have all the options Tom added.

Re: splitting out ouster_client: is that really necessary if it's possible to build just the client binaries from this repo?

For versioning, probably it would be good software practice? For making the client code releasable for both the ROS 1 and ROS 2 drivers, yes. I can't have the ROS 2 drivers build on the build farm without it being released itself already on apt or as a separate repo I can myself release via the ROS build farm to then link against in the ROS 2 drivers - since we want to share it ideally.

@RFRIEDM-Trimble
Copy link

Can you use cmake FetchContent or git submodules instead of copy/paste?

@SteveMacenski
Copy link
Author

No, they do not work for the ros build farm to release binaries

@Samahu Samahu self-assigned this Sep 22, 2022
@Samahu Samahu transferred this issue from ouster-lidar/ouster-sdk Oct 12, 2022
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 a pull request may close this issue.

4 participants