ros2 / rosidl

Packages which provide the ROS IDL (.msg) definition and code generation.
Apache License 2.0
76 stars 125 forks source link

Support empy3 and empy4 #821

Closed ahcorde closed 2 months ago

clalancette commented 2 months ago

I took this for a spin locally. I have good news and bad news.

The good news is that this works, and seems to keep compatibility with both Empy 3 and Empy 4.

The bad news is that with Empy 4, there is at least one bug. In particular, every time a rosidl_generator_py template file is expanded, it is printed to the console. This makes the output when building with it extremely noisy. I haven't dug into what is causing this, but I think we should find a way to fix/suppress this before we merge this in.

ahcorde commented 2 months ago

I took this for a spin locally. I have good news and bad news.

The good news is that this works, and seems to keep compatibility with both Empy 3 and Empy 4.

The bad news is that with Empy 4, there is at least one bug. In particular, every time a rosidl_generator_py template file is expanded, it is printed to the console. This makes the output when building with it extremely noisy. I haven't dug into what is causing this, but I think we should find a way to fix/suppress this before we merge this in.

I noticed this, That's why is still a draft

ahcorde commented 2 months ago

@clalancette should be fixed now

ahcorde commented 2 months ago
ahcorde commented 2 months ago
yashi commented 1 month ago

should we backport this to Jazzy, Iron, and Humble? Or they should use empy==3.3.4?

fujitatomoya commented 1 month ago

AFAIK, ubuntu nobel and jammy both use 3.3.4 so basically we do not use empy4. besides https://github.com/ros2/rosidl/pull/821#issuecomment-2271098700, we should wait until we make sure rolling works with em.Configuration without any problems. and then, we can consider backporting?

or do you have any specific requirement for this backporting?

yashi commented 1 month ago

No I don't. We can wait.