rosidl icon indicating copy to clipboard operation
rosidl copied to clipboard

rosidl_cli is broken because of missing type_description support

Open marcoesposito1988 opened this issue 1 year ago • 4 comments

Bug report

Required Info:

  • Operating System:
    • Ubuntu 24.04
  • Installation type:
    • binaries from APT
  • Version or commit hash:
    • 4.6.2-1noble.20240513.233918
  • DDS implementation:
    • Fast-RTPS (not relevant)

Steps to reproduce issue

Use rosidl_cli to generate C/C++ code from a (very simple) test message

generate -o /tmp/test my_msgs /home/myuser/my_msgs:msg/MyTestMessage.msg

Expected behavior

Code generated correctly (as it happened with ROS2 humble)

Actual behavior

Fails with:

TypeError when expanding 'idl__description.c.em' into '.../MyTestMessage__description.c': 'NoneType' object is not subscriptable

If I get it correctly, this is due to type_description_info being None here, because it is never added here (see here)

Am I missing anything? Was rosidl_cli just not updated recently to cope with changes in the other generators? Or is a fix already underway?

I guess that fixing this would require hardcoding the type_description generator to be run before any other, and then update the legacy_generator_arguments_file function to be non-legacy anymore. Can anyone confirm this? If positive, I could attempt a fix

marcoesposito1988 avatar May 26 '24 21:05 marcoesposito1988

@sloretz , sorry to bug you. Just curious if you have spare cycle to peek at this issue. Thanks!

nkoenig avatar May 31 '24 18:05 nkoenig

I found a (inelegant) solution for my problem: https://github.com/marcoesposito1988/rosidl/tree/add-type-description-to-rosidl-cli

On this branch I added:

  • the generation of rosidl_generator_cpp__visibility_control.hpp. This should be trivial enough for me not having messed up anything.
  • a cli entry point for the type_description generator. This should also be fine.
  • the missing arguments to legacy_generator_arguments_file(). This function was already being used for all generators, which is fine because extra arguments would just be ignored. So I guess that I didn't make the situation worse than before.
  • forcing type_description to run before the c and cpp generators: this is what I am unsatisfied with. The cmake scripts ensure topological order of the generators, which I don't have the time to implement myself right now.

If anyone can confirm that I am down the right/desired path, I will try to implement the generator topology in python in rosidl-cli as soon as possible and open a PR

marcoesposito1988 avatar Jun 04 '24 23:06 marcoesposito1988

@marcoesposito1988 Thanks a lot for your help, I have encountered the same issue with rosidl under Jazzy and your patch has been really helpful, this would be great if it could be integrated into the upstream repository and ultimately make its way back to Jazzy.

If I may, I have a couple of extra notes.

First, type_description generation is required for other generators, so rosidl generate -t cpp -o generated my_msgs my_folder:msg/MyMsg will fail, one should either add -t type_description to the command line, run type_description generation in the same output folder before or patch rosidl to always insert the type_description generator in that case (however, the later changes where the files are output when you were specifying a single type so it's a minor annoyance)

Second, the type_description generator is also more sensitive on having the correct -I/--include-path option, if your message description includes anything besides builtin types or types inside your own package, it will fail if you don't specify the include path properly; that's because this generator needs to have the JSON of your dependencies whereas the other generators only generate the appropriate include/import and don't care too much that the messages exist at all.

gergondet-woven avatar Sep 23 '24 12:09 gergondet-woven

@marcoesposito1988 thank you for finding the solution. could you consider to open the PR, we can start there.

fujitatomoya avatar Oct 04 '24 04:10 fujitatomoya