Skip to content

feat: add livox ros driver2 #53

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

Merged
merged 3 commits into from
May 14, 2025
Merged

Conversation

wep21
Copy link
Contributor

@wep21 wep21 commented Apr 22, 2025

livox ros driver is not on rosdistro, so I've added it as additional recipe.

@wep21
Copy link
Contributor Author

wep21 commented Apr 22, 2025

@traversaro @Tobias-Fischer How can I build additional recipe in ci? What do you think about adding a package which is not on rosdistro into robostack channel?

@traversaro
Copy link
Member

@traversaro @Tobias-Fischer How can I build additional recipe in ci?

I am not an expert of those (ideally it would be great if they were generated togetehr with other recipes, just from other data, not coming from rosdistro) but I guess the PR in https://github.com/RoboStack/ros-humble/pull/268/files#diff-069a2e75bc939d92ad32a50dada65893abaf603c1a586b97183ec1ca476bba23R148 could provide useful hints.

What do you think about adding a package which is not on rosdistro into robostack channel?

As long as it the motivation is quite clear (and in that case I see the reason) no problem for me.

@wep21
Copy link
Contributor Author

wep21 commented Apr 29, 2025

I am not an expert of those (ideally it would be great if they were generated togetehr with other recipes, just from other data, not coming from rosdistro) but I guess the PR in https://github.com/RoboStack/ros-humble/pull/268/files#diff-069a2e75bc939d92ad32a50dada65893abaf603c1a586b97183ec1ca476bba23R148 could provide useful hints.

I will try something.

As long as it the motivation is quite clear (and in that case I see the reason) no problem for me.

The motivations is that livox mid360 sensor is quite popular in slam realm while livox ros driver is not well maintained by DJI and seems to be never added into rosdistro unfortunately.

@wep21 wep21 force-pushed the livox-driver branch 2 times, most recently from 77b3a6f to a4448cd Compare April 29, 2025 15:02
@wep21 wep21 marked this pull request as draft April 29, 2025 16:41
@wep21 wep21 force-pushed the livox-driver branch 7 times, most recently from eebebb4 to e060d3c Compare May 4, 2025 23:33
@wep21 wep21 force-pushed the livox-driver branch 12 times, most recently from 717e637 to 830c6f9 Compare May 11, 2025 12:50
Signed-off-by: wep21 <[email protected]>
@wep21 wep21 marked this pull request as ready for review May 11, 2025 12:59
@wep21
Copy link
Contributor Author

wep21 commented May 11, 2025

@traversaro @Tobias-Fischer I appreciate it if you could review this PR now. The platforms where livox driver is built are only linux.

@Tobias-Fischer
Copy link
Contributor

What do you think about copying the selected additional recipes into the recipes folder? I think this would avoid some duplication. Otherwise this looks great thanks a lot!!

Signed-off-by: wep21 <[email protected]>
@wep21
Copy link
Contributor Author

wep21 commented May 13, 2025

@traversaro @Tobias-Fischer I've addressed the review. Please check the change again.

@wep21 wep21 requested a review from traversaro May 13, 2025 15:03
@Tobias-Fischer Tobias-Fischer merged commit 76c3659 into RoboStack:main May 14, 2025
5 checks passed
@Tobias-Fischer
Copy link
Contributor

Side note @traversaro - with now noetic, humble and jazzy our repositories somewhat diverge, we should think whether there is a way to better handle this.

@nmarticorena
Copy link

Hi, as an additional side note, this problem is quite similar to the issue we are currently having on ros-humble RoboStack/ros-humble#296 due to the emscripten recipes, as I can see here, the additional recipes still trigger the CI win example due to the vinca_gha only checks if the package is already available in the channel.

My two cents will be to solve this problem from Vinca by adding a parse skip, and we can ease up by adding a note in the docs on how to write the platforms to skip, so then we dont need to implement for all viable skip syntax allowed by rattler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants