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

fuse -> ROS 2 fuse_graphs : Port fuse_graphs #289

Merged
merged 4 commits into from
Dec 8, 2022

Conversation

methylDragon
Copy link
Collaborator

@methylDragon methylDragon commented Nov 18, 2022

See: #276

Description

This PR ports the entirety of fuse_graphs, including:

  • CMakeLists and packages
  • Source code
  • Tests

Other stuff like docs are in other PRs.
Some method signatures were altered in fuse_core to support the PR.

Notes

Pinging @svwilliams for visibility.

fuse_core/include/fuse_core/graph.h Outdated Show resolved Hide resolved
fuse_graphs/CMakeLists.txt Outdated Show resolved Hide resolved
fuse_graphs/CMakeLists.txt Outdated Show resolved Hide resolved
@methylDragon methylDragon force-pushed the rolling-graphs branch 2 times, most recently from 02d05fa to cb7ea7d Compare December 8, 2022 04:38
Signed-off-by: methylDragon <methylDragon@gmail.com>
Signed-off-by: methylDragon <methylDragon@gmail.com>
Signed-off-by: methylDragon <methylDragon@gmail.com>
@@ -1,4 +1,4 @@
cmake_minimum_required(VERSION 3.0.2)
cmake_minimum_required(VERSION 3.16)
project(fuse_graphs)

set(build_depends
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit, unused?

<test_depend condition="$ROS_DISTRO >= melodic">benchmark</test_depend>
<test_depend>roslint</test_depend>
<test_depend>rostest</test_depend>
<buildtool_depend>ament_cmake</buildtool_depend>
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit, ament_cmake_ros

* ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*/
#ifndef FUSE_GRAPHS_TEST_EXAMPLE_VARIABLE_H // NOLINT{build/header_guard}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Accidental copy of fuse_graphs/tests/example_variable.h?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How did that happen 🤔
I hope it wasn't my script doing it :/

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah looks like it was being relied on by the benchmark. Adding the test directory as an include path should fix it

14:36:52 /tmp/ws/src/fuse/fuse_graphs/benchmark/benchmark_create_problem.cpp:39:10: fatal error: example_variable.h: No such file or directory
14:36:52    39 | #include "example_variable.h"
target_include_directories(benchmark_create_problem PRIVATE "test/")

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, you're right. I duplicated it because of that, but making it rely on test's version works too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@methylDragon
Copy link
Collaborator Author

@ros-pull-request-builder retest this please!

Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Copy link
Collaborator Author

methylDragon commented Dec 8, 2022

@ros-pull-request-builder retest this please!

Build Status

Copy link
Collaborator

@sloretz sloretz left a comment

Choose a reason for hiding this comment

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

LGTM with RPr job where only linters for fuse_graphs are failing

@sloretz
Copy link
Collaborator

sloretz commented Dec 8, 2022

Ah I see, https://build.ros2.org/job/Rpr__fuse__ubuntu_jammy_amd64/23/#showFailuresLink is for this PR (I see 38006f3 being checked out in the console text). Looks ready to merge to me!

@methylDragon methylDragon merged commit 10a0438 into locusrobotics:rolling Dec 8, 2022
@methylDragon methylDragon deleted the rolling-graphs branch December 8, 2022 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants