ros2 / rosidl_python

rosidl support for Python
Apache License 2.0
20 stars 45 forks source link

Backport array optimization to Galactic and Foxy #135

Closed sloretz closed 2 years ago

sloretz commented 3 years ago

This is an issue to backport #129 to Galactic and Foxy. I think it would be a good idea to have it sit in Rolling a little bit to make sure it works since it changes something so low level.

pulkitgoyal1506 commented 3 years ago

@sloretz Can we merge this patch on foxy now? I am facing this issue on ros2::foxy, it would be great if this can be merged.

sloretz commented 3 years ago

Seems like it has worked fine in Rolling. I'll backport to Galactic first out of caution since it's a very low level change.

https://github.com/ros2/rosidl_python/pull/145

sloretz commented 2 years ago

Released to Galactic in 0.11.1 https://github.com/ros/rosdistro/pull/30955

hidmic commented 2 years ago

@sloretz have we backported this to Foxy already?

sloretz commented 2 years ago

Yeah it's been backported. It might need a foxy release

jacobperron commented 2 years ago

This has also been released into Foxy.

travipross commented 2 years ago

This has also been released into Foxy.

@jacobperron Did you mean this as in, fetching the latest container from Dockerhub for ros2:foxy should include this fix?

I see this issue is listed as "Needs release" in the Foxy Release Patch 8 project, so I wasn't sure whether to expect this to exist in the official Docker image.

jacobperron commented 2 years ago

@travipross The patch was backported to Foxy in https://github.com/ros2/rosidl_python/pull/146 and released in 0.9.5. It looks like 0.9.5 has also been sync'd to the main release repository: http://repo.ros2.org/status_page/ros_foxy_default.html?q=rosidl_python So, the binary package should be available in Ubuntu. I'm not involved in maintaining the Docker image for Foxy, so I'm not sure what is required for updating it.

jacobperron commented 2 years ago

And, the Foxy Patch release board is lagging behind a bit since I have to update it manually.