VEXU-GHOST / VEXU_GHOST

Advanced robot software framework for a Vex V5 + Nvidia Jetson stack
11 stars 5 forks source link

changes submodules to debians #76

Closed xwilson03 closed 6 months ago

xwilson03 commented 6 months ago

PR Summary

PR Link: Link

Issue Link: Link

Description

Switches compilation-heavy submodules for pre-compiled debians to assist CI pipeline and embedded devices.

Reviewers


Changelog

MaxxWilson commented 6 months ago

Looks pretty solid. Left relevant comments.

Another big question, do we know how this works for ROS Package dependencies? i.e. exporting the proper environment variables so that the ros packages are detected? None of these submodules are ros packages, and BehaviorTree related ones seem to take a bit.

I think should be good to merge AFTER you figure out the Jetson binaries or make an issue to do it later. Im approving for now because I dont have bandwidth to double check later, but I assume you will handle these before merging.

MaxxWilson commented 6 months ago

FOLLOWUP:

https://github.com/VEXU-GHOST/VEXU_GHOST/pull/77 This needs to get merged into this branch so we can debianize plotjuggler and plotjuggler_ros bc they are THICK and effect every build. We should also just do the cpp and lidar ones while we have this PR open, bc otherwise we never will lol.

xwilson03 commented 6 months ago

Looks pretty solid. Left relevant comments.

Another big question, do we know how this works for ROS Package dependencies? i.e. exporting the proper environment variables so that the ros packages are detected? None of these submodules are ros packages, and BehaviorTree related ones seem to take a bit.

I think should be good to merge AFTER you figure out the Jetson binaries or make an issue to do it later. Im approving for now because I dont have bandwidth to double check later, but I assume you will handle these before merging.

@MaxxWilson This should be possible through pre/post installation scripts. Since the submodule debians are somewhat hand-crafted, maintainers would need to ensure that the proper environment variables get set post-install.

EDIT: do not use post install scripts in the way that i did! use bloom to handle this instead of trying to do it yourself: https://docs.ros.org/en/rolling/How-To-Guides/Building-a-Custom-Debian-Package.html

xwilson03 commented 6 months ago

All x86 debians have been added, just not their arm equivalents. Moving onto a small testing phase using the pipeline to ensure their quality; the build script will need to omit these packages going forward.

xwilson03 commented 6 months ago

update on this; regular submodules continue to work flawlessly, but the mentioned approach to packaging ROS dependencies (postinst scripts that move files into the repository directory using $VEXU_HOME) was super hacky and now needs to be pretty much overhauled and replaced with bloom generated packages.

xwilson03 commented 6 months ago

tests for submodules got disabled since the submodules themselves werent technically "built"; some file location assertions were failing and necessitated this. either we need to skip submodule tests (current solution) or we figure out a way around this somehow.

other than that, though, pipeline is passing so this is ready for a merge. arm64 binaries are otw via the ghost_dependencies repo, but all the work here is done.