ros-industrial / reach_ros2

ROS2 packages for REACH
Apache License 2.0
17 stars 4 forks source link

Add customizable color scale for visualization #18

Closed SammyRamone closed 1 year ago

SammyRamone commented 1 year ago

See https://github.com/ros-industrial/reach/pull/58

edit: building of CI fails as the mentioned PR is required as a basis

acbuynak commented 1 year ago

Is red to green a good choice? These colors are commonly difficult to distinguish for color blind people.

SammyRamone commented 1 year ago

You are right, but I don't want to change the default, just give another option. In my cultural setting (Germany) people typically just expect a range from bad->good being represented by red->green.

Anyway, this PR will be changed based on the discussion here https://github.com/ros-industrial/reach/pull/58

gavanderhoorn commented 1 year ago

Is red to green a good choice? These colors are commonly difficult to distinguish for color blind people.

I was going to post something similar.

Most flexible option would be to allow configuring colour scales. But that might require more work.

marip8 commented 1 year ago

/usr/bin/ld: /opt/ros/rolling/lib/libinteractive_markers.so: undefined reference to `rclcpp::TimerBase::TimerBase(std::shared_ptr, std::chrono::duration<long, std::ratio<1l, 1000000000l> >, std::shared_ptr, bool)'

Any ideas why this is happening on the rolling build? It seems to happen when we link the reach study node to the library being built by this repo. I'm not sure why some distributed library that is not being built in this repo is complaining about an undefined reference

marip8 commented 1 year ago

/usr/bin/ld: /opt/ros/rolling/lib/libinteractive_markers.so: undefined reference to `rclcpp::TimerBase::TimerBase(std::shared_ptrrclcpp::Clock, std::chrono::duration<long, std::ratio<1l, 1000000000l> >, std::shared_ptrrclcpp::Context, bool)'

Any ideas why this is happening on the rolling build? It seems to happen when we link the reach study node to the library being built by this repo. I'm not sure why some distributed library that is not being built in this repo is complaining about an undefined reference

Turns out the rolling docker image is not up to date. Running apt upgrade in the container before building solves this issue. Rather than running apt upgrade on the image in CI, I think we should wait for the image to be updated so we continue to build on a nominal image