RoboStack / ros-noetic

vinca configuration files for ros-noetic
https://robostack.github.io
450 stars 70 forks source link

Add foxglove_bridge package #345

Closed defunctzombie closed 1 year ago

defunctzombie commented 1 year ago

The foxglove bridge package is a c++ websocket bridge for connecting ROS systems to Foxglove Studio. It is similar in spirit to rosbridge but written in c++ to overcome some performance problems we experienced with rosbridge under large message or high rate message workloads.

https://foxglove.dev/blog/announcing-the-new-foxglove-bridge-for-live-ros-data https://github.com/foxglove/ros-foxglove-bridge/

Humble PR: https://github.com/RoboStack/ros-humble/pull/49

Tobias-Fischer commented 1 year ago

Thanks for the contribution @defunctzombie!

Unfortunately our CI seems to have some problems unrelated to this package. Any idea what's going on here @wolfv? Seems like some boa issue:

Recipe validation error
[124](https://github.com/RoboStack/ros-noetic/actions/runs/4247326654/jobs/7385241857#step:15:125)

[125](https://github.com/RoboStack/ros-noetic/actions/runs/4247326654/jobs/7385241857#step:15:126)
Additional properties are not allowed ('outputs' was unexpected)
[126](https://github.com/RoboStack/ros-noetic/actions/runs/4247326654/jobs/7385241857#step:15:127)
Tobias-Fischer commented 1 year ago

Ah, actually it might be related to the package after all. Has this been released in the ROS build farm yet?

defunctzombie commented 1 year ago

@Tobias-Fischer I got the name wrong. I used the repo name rather than the package name. I've updated the PR with the correct name.

defunctzombie commented 1 year ago

I don't expect this PR and the humble one will pass the builds on the first try. We might need to adjust some aspects of our packaging or build.

Tobias-Fischer commented 1 year ago

Ok cool - now fails with a less cryptic error:

$PREFIX/include/actionlib/managed_list.h:221:73: error: '_1' was not declared in this scope
[2020](https://github.com/RoboStack/ros-noetic/actions/runs/4247483049/jobs/7385580850#step:14:2021)
  221 |     return add(elem, boost::bind(&ManagedList<T>::defaultDeleter, this, _1) );
[2021](https://github.com/RoboStack/ros-noetic/actions/runs/4247483049/jobs/7385580850#step:14:2022)
      |                                                                         ^~
[2022](https://github.com/RoboStack/ros-noetic/actions/runs/4247483049/jobs/7385580850#step:14:2023)

This needs std::place_holders or sth like that. Let me know if you run into any issues that you can't seem to solve.

achim-k commented 1 year ago

The error stems from actionlib:

$PREFIX/include/actionlib/managed_list.h: In member function 'actionlib::ManagedList<T>::Handle actionlib::ManagedList<T>::add(const T&)':
$PREFIX/include/actionlib/managed_list.h:221:73: error: '_1' was not declared in this scope
  221 |     return add(elem, boost::bind(&ManagedList<T>::defaultDeleter, this, _1) );
      |                                                                         ^~

A few lines above it says that declaring boost placeholders in the global namespace is deprecated:

$PREFIX/include/boost/bind.hpp:36:1: note: '#pragma message: The practice of declaring the Bind placeholders (_1, _2, ...) in the global namespace is deprecated. Please use <boost/bind/bind.hpp> + using namespace boost::placeholders, or define BOOST_BIND_GLOBAL_PLACEHOLDERS to retain the current behavior.'

Is foxglove_bridge the first package that (transitively) depends on actionlib? Should we propose a patch for actionlib? I guess we could also add a #define BOOST_BIND_GLOBAL_PLACEHOLDERS in foxglove_bridge

Tobias-Fischer commented 1 year ago

Whatever is easier for you :). You can easily patch packages - see https://github.com/RoboStack/ros-noetic/tree/master/patch

defunctzombie commented 1 year ago

@achim-k Seems like there are some more instances of this issue in the ros-noetic-ros-babel-fish/ package.

achim-k commented 1 year ago

@achim-k Seems like there are some more instances of this issue in the ros-noetic-ros-babel-fish/ package.

No, it's still from actionlib. Apparently adding the patch in https://github.com/RoboStack/ros-noetic/pull/345/commits/9bb189938c51afb0e8aea9f8aebb00c5319c2557 didn't work

Edit: You are right, depending on which build you have checked

defunctzombie commented 1 year ago

@achim-k I was looking at the linux-64 build. I figure until we get that one fixed the others have less hope.

Tobias-Fischer commented 1 year ago

See my PR to your repo which will build all selected packages (so you can rebuild actionlib etc. if needed). Many thanks again for your contributions!

defunctzombie commented 1 year ago

Woo all the builds passed! 👏

defunctzombie commented 1 year ago

@Tobias-Fischer FYI - this change also commented out all the other packages since we were iterating on this and didn't need to keep building those. Probably you didn't mean to also merge those changes?

Tobias-Fischer commented 1 year ago

No, this was deliberate, all good :)

Thanks again for the contribution!