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 reference modification in generate_dynamic #40

Closed
wants to merge 2 commits into from

Conversation

jpetereit
Copy link
Contributor

Reference modification in generate_dynamic can fail in certain cases.

Consider a custom message my_geometry_msgs/Vector3WithCovariance consisting of

geometry_msgs/Vector3 vector
float64[9]            covariance

When modifying the references in generate_dynamic, there is an erroneous match for my_geometry_msgs.msg.Vector3WithCovariance, which results in a wrong symbol after replacement (my__geometry_msgs__Vector3WithCovariance instead of _my_geometry_msgs__Vector3WithCovariance).

This happens because the currently used string replacement strategy is too simplistic. It does not check if it is actually replacing a full symbol.

@dirk-thomas
Copy link
Member

I tried to reproduce the issue you describe but was not successful. Can you please post a complete example with step-by-step instructions?

Are you trying this with the latest sources of genpy since very recently #37 has been addressed?

@jpetereit
Copy link
Contributor Author

I've checked out the latest version and the error does still occur.

I hope the following steps can help you to reproduce the bug:

  1. Check out my test workspace.

  2. catkin_make

  3. source devel/setup.bash

  4. This step is optional. You may just use the bag file dump.bag that is contained in my test case. It was created by

    $ rosbag record -O dump.bag /mytopic
    

    and then publishing some iosb_sensor_msgs/GpsWithVelocity messages with rqt_publisher.

    Optional: $ rosbag info dump.bag

    path:        dump.bag
    version:     2.0
    duration:    3.0s
    start:       Oct 05 2015 13:09:11.58 (1444043351.58)
    end:         Oct 05 2015 13:09:14.58 (1444043354.58)
    size:        13.5 KB
    messages:    4
    compression: none [1/1 chunks]
    types:       iosb_sensor_msgs/GpsWithVelocity [bfe1b7f396cf11c6c14531104407ff0c]
    topics:      /mytopic   4 msgs    : iosb_sensor_msgs/GpsWithVelocity
    
  5. Finally, run

    $ rostopic echo -b dump.bag /mytopic
    

    which results in the following error:

    Traceback (most recent call last):
    File "/opt/ros/jade/bin/rostopic", line 35, in <module>
     rostopic.rostopicmain()
    File "/opt/ros/jade/lib/python2.7/dist-packages/rostopic/__init__.py", line 1765, in rostopicmain
     _rostopic_cmd_echo(argv)
    File "/opt/ros/jade/lib/python2.7/dist-packages/rostopic/__init__.py", line 1136, in _rostopic_cmd_echo
     _rostopic_echo(topic, callback_echo, bag_file=options.bag)
    File "/opt/ros/jade/lib/python2.7/dist-packages/rostopic/__init__.py", line 767, in _rostopic_echo
     _rostopic_echo_bag(callback_echo, bag_file)
    File "/opt/ros/jade/lib/python2.7/dist-packages/rostopic/__init__.py", line 745, in _rostopic_echo_bag
     for t, msg, timestamp in b.read_messages():
    File "/opt/ros/jade/lib/python2.7/dist-packages/rosbag/bag.py", line 2306, in read_messages
     yield self.seek_and_read_message_data_record((entry.chunk_pos, entry.offset), raw)
    File "/opt/ros/jade/lib/python2.7/dist-packages/rosbag/bag.py", line 2455, in seek_and_read_message_data_record
     msg = msg_type()
    File "/tmp/genpy_uyHqoq/tmpWAffIp.py", line 157, in __init__
     self.velocity = iosb__geometry_msgs__Vector3WithCovariance()
    NameError: global name 'iosb__geometry_msgs__Vector3WithCovariance' is not defined
    

Please let me know if you need any further help to reproduce the issue.

@dirk-thomas
Copy link
Member

Thank you for the detailed example as well as the patch. I have squashed both commits and only changed the import order (to follow PEP8): 06cc06b

@dirk-thomas dirk-thomas closed this Oct 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants