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

Fix plots of array items #71

Merged
merged 5 commits into from
Mar 5, 2021
Merged

Conversation

ivanpauno
Copy link
Contributor

Fixes #64.

I rewrote it completely to avoid rqt_py_common.message_helpers/rqt_py_common.topic_helpers.
Those modules should never have existed really, they provide badly maintained implementations of utilities that already exist in rosidl_runtime_py or rosidl_parser.

…osidl methods instead of random adhoc utilities

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno ivanpauno requested a review from jacobperron March 4, 2021 19:49
@ivanpauno ivanpauno self-assigned this Mar 4, 2021
@ivanpauno
Copy link
Contributor Author

This probably should target a new foxy-devel branch, as this is likely not backwards compatible with crystal.
I could try to check if this is backwards compatible with dashing, and in that case create a dashing-devel branch.

@mabelzhang @cottsay what do you think?

@ivanpauno
Copy link
Contributor Author

Those modules should never have existed really, they provide badly maintained implementations of utilities that already exist in rosidl_runtime_py or rosidl_parser.

Actually rosidl_runtime_py wasn't available in foxy/dashing the methods were available in rosidl_generator_py at that time.
So we should start by targeting a new galactic-devel branch and then work on a backport for previous releases (which should be easy to do).

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@jacobperron
Copy link
Contributor

Actually rosidl_runtime_py wasn't available in foxy/dashing the methods were available in rosidl_generator_py at that time.

I think rosidl_runtime_py was added in Eloquent. Or do you mean the functions you're using aren't available until Galactic?

src/rqt_plot/plot_widget.py Outdated Show resolved Hide resolved
src/rqt_plot/plot_widget.py Outdated Show resolved Hide resolved
src/rqt_plot/plot_widget.py Outdated Show resolved Hide resolved
src/rqt_plot/plot_widget.py Outdated Show resolved Hide resolved
src/rqt_plot/plot_widget.py Outdated Show resolved Hide resolved
@mabelzhang
Copy link
Contributor

mabelzhang commented Mar 5, 2021

New branch sounds fine to me, whichever distro you guys figure out is the right one.
If any of you want to do the release, that's fine. Otherwise I can do it too, just let me know which branch is getting released into which distro, and if we're doing any forward or back porting.
I haven't really touched the ROS 2 part of this repo too much yet.

ivanpauno and others added 3 commits March 5, 2021 09:32
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
@ivanpauno
Copy link
Contributor Author

I think rosidl_runtime_py was added in Eloquent. Or do you mean the functions you're using aren't available until Galactic?

Oh thanks! I don't know what I checked before 😅 .

@ivanpauno ivanpauno changed the base branch from crystal-devel to foxy-devel March 5, 2021 12:40
@ivanpauno
Copy link
Contributor Author

This is targeting now a new foxy-devel branch, I have double checked that all rosidl_parsed and rosidl_runtime_py were already available there.

I will open a PRs to ros2/repos after merging this

@jacobperron
Copy link
Contributor

Similar to ros-visualization/rqt_msg#11 (comment), we should also update the release and rosdistro repos as well.

Copy link
Contributor

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for fixing this!

@ivanpauno ivanpauno merged commit 9c56eab into foxy-devel Mar 5, 2021
@ivanpauno ivanpauno deleted the ivanpauno/fix-plots-of-array-items branch March 5, 2021 17:41
@ivanpauno
Copy link
Contributor Author

Similar to ros-visualization/rqt_msg#11 (comment), we should also update the release and rosdistro repos as well.

ros2/ros2#1097, ros2/ros2#1098, ros/rosdistro#28490, ros/rosdistro#28491 should solve everything

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

Successfully merging this pull request may close these issues.

3 participants