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

Message._fill_message_args() fails on messages with fields of type 'time[]' #13

Closed
astaa-nrec opened this issue Jul 10, 2013 · 7 comments
Assignees

Comments

@astaa-nrec
Copy link

(on genpy version 0.4.10, ros_comm version 1.9.44)

To reproduce: define a message "rosbugs/msg/TimeArray.msg" with a single field:

time[] stamps

Build it and then try:

rostopic pub /test ros_bugs/TimeArray "{stamps: []}"

This fails with the error:

Traceback (most recent call last):
  File "/opt/ros/groovy/bin/rostopic", line 35, in <module>
    rostopic.rostopicmain()
  File "/opt/ros/groovy/lib/python2.7/dist-packages/rostopic/__init__.py", line 1654, in rostopicmain
    _rostopic_cmd_pub(argv)
  File "/opt/ros/groovy/lib/python2.7/dist-packages/rostopic/__init__.py", line 1330, in _rostopic_cmd_pub
    argv_publish(pub, msg_class, pub_args, rate, options.once, options.verbose)
  File "/opt/ros/groovy/lib/python2.7/dist-packages/rostopic/__init__.py", line 1354, in argv_publish
    publish_message(pub, msg_class, pub_args, rate, once, verbose=verbose)
  File "/opt/ros/groovy/lib/python2.7/dist-packages/rostopic/__init__.py", line 1224, in publish_message
    genpy.message.fill_message_args(msg, pub_args, keys=keys)
  File "/opt/ros/groovy/lib/python2.7/dist-packages/genpy/message.py", line 513, in fill_message_args
    _fill_message_args(msg, msg_args[0], keys, '')
  File "/opt/ros/groovy/lib/python2.7/dist-packages/genpy/message.py", line 461, in _fill_message_args
    _fill_val(msg, f, v, keys, prefix)
  File "/opt/ros/groovy/lib/python2.7/dist-packages/genpy/message.py", line 425, in _fill_val
    list_msg_class = get_message_class(base_type)
  File "/opt/ros/groovy/lib/python2.7/dist-packages/genpy/message.py", line 570, in get_message_class
    cls = _get_message_or_service_class('msg', message_type, reload_on_error=reload_on_error)
  File "/opt/ros/groovy/lib/python2.7/dist-packages/genpy/message.py", line 532, in _get_message_or_service_class
    raise ValueError("message type is missing package name: %s"%str(message_type))
ValueError: message type is missing package name: time

It looks like this could be fixed by inserting a new case around line 420 of message.py, handling the case when base_type is a time type.

@dirk-thomas
Copy link
Member

Can you please retry it with a current checkout of rospy? The commit e8e0f3e might already solve your use case.

@astaa-nrec
Copy link
Author

Thank you. That solves our most important use cases.

It does not, however, handle the case when a list of ints or longs is supplied.

So this works fine:

>>> timeArray = TimeArray()
>>> genpy.message.fill_message_args(timeArray, [{'stamps':[{'secs':1}, {'secs':1, 'nsecs':1}, {}]}])
>>> timeArray
stamps: 
  - 
    secs: 1
    nsecs: 0
  - 
    secs: 1
    nsecs: 1
  - 
    secs: 0
    nsecs: 0

But this is still broken:

>>> timeArray = TimeArray()
>>> genpy.message.fill_message_args(timeArray, [{'stamps':[1,2,3]}])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "genpy/message.py", line 513, in fill_message_args
    _fill_message_args(msg, msg_args[0], keys, '')
  File "genpy/message.py", line 461, in _fill_message_args
    _fill_val(msg, f, v, keys, prefix)
  File "genpy/message.py", line 431, in _fill_val
    _fill_message_args(inner_msg, el, prefix)
  File "genpy/message.py", line 475, in _fill_message_args
    raise ValueError("invalid msg_args type: %s"%str(msg_args))
ValueError: invalid msg_args type: 1

@dirk-thomas
Copy link
Member

I am not sure if [{'stamps':[1,2,3]}] is even supposed to work. Can you point to any doc where this is mentioned?

@astaa-nrec
Copy link
Author

I don't know whether it's documented, but it works for non-array fields:

>>> h = Header()
>>> genpy.message.fill_message_args(h, [{'stamp' : 1}])
>>> h
seq: 0
stamp: 
  secs: 0
  nsecs: 1
frame_id: ''

The logic that handles it is on line 394 of message.py :

       elif isinstance(def_val, TVal) and type(v) in (int, long):
            #special case to handle time value represented as a single number
            #TODO: this is a lossy conversion
            if isinstance(def_val, Time):
                setattr(msg, f, Time.from_sec(v/1e9))
            elif isinstance(def_val, Duration):                    
                setattr(msg, f, Duration.from_sec(v/1e9))
            else:
                raise MessageException("Cannot create time values of type [%s]"%(type(def_val)))

@dirk-thomas
Copy link
Member

Can you please check if pull request #19 fixes your use case?

@ghost ghost assigned dirk-thomas Aug 15, 2013
@astaa-nrec
Copy link
Author

Yes, that fixes it.

dirk-thomas added a commit that referenced this issue Aug 16, 2013
enable int/long values for list of time/duration (fix #13)
@dirk-thomas
Copy link
Member

Thanks for confirmation. Patch is merged and will be release with the next version of genpy.

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

No branches or pull requests

2 participants