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

Add check for float32 and float64 to check_type #131

Merged
merged 3 commits into from
Aug 31, 2020

Conversation

mintar
Copy link
Contributor

@mintar mintar commented Aug 20, 2020

Fixes #130.

Specifically, this PR improves the error messages during serialization. Consider the following script:

from io import BytesIO
from geometry_msgs.msg import PoseStamped

msg = PoseStamped()
msg.pose.position.x = '0.0'
buff = BytesIO()
msg.serialize(buff)

Old behavior:

Traceback (most recent call last):
  File "/opt/ros/noetic/lib/python3/dist-packages/geometry_msgs/msg/_PoseStamped.py", line 106, in serialize
    buff.write(_get_struct_7d().pack(_x.pose.position.x, _x.pose.position.y, _x.pose.position.z, _x.pose.orientation.x, _x.pose.orientation.y, _x.pose.orientation.z, _x.pose.orientation.w))
struct.error: required argument is not a float

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "src/genpy/test.py", line 9, in <module>
    msg.serialize(buff)
  File "/opt/ros/noetic/lib/python3/dist-packages/geometry_msgs/msg/_PoseStamped.py", line 107, in serialize
    except struct.error as se: self._check_types(struct.error("%s: '%s' when writing '%s'" % (type(se), str(se), str(locals().get('_x', self)))))
  File "/opt/ros/noetic/lib/python3/dist-packages/genpy/message.py", line 364, in _check_types
    raise SerializationError(str(exc))
genpy.message.SerializationError: <class 'struct.error'>: 'required argument is not a float' when writing 'header: 
  seq: 0
  stamp: 
    secs: 0
    nsecs:         0
  frame_id: ''
pose: 
  position: 
    x: "0.0"
    y: 0.0
    z: 0.0
  orientation: 
    x: 0.0
    y: 0.0
    z: 0.0
    w: 0.0'

New behavior:

Traceback (most recent call last):
  File "/opt/ros/noetic/lib/python3/dist-packages/geometry_msgs/msg/_PoseStamped.py", line 106, in serialize
    buff.write(_get_struct_7d().pack(_x.pose.position.x, _x.pose.position.y, _x.pose.position.z, _x.pose.orientation.x, _x.pose.orientation.y, _x.pose.orientation.z, _x.pose.orientation.w))
struct.error: required argument is not a float

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "src/genpy/test.py", line 9, in <module>
    msg.serialize(buff)
  File "/opt/ros/noetic/lib/python3/dist-packages/geometry_msgs/msg/_PoseStamped.py", line 107, in serialize
    except struct.error as se: self._check_types(struct.error("%s: '%s' when writing '%s'" % (type(se), str(se), str(locals().get('_x', self)))))
  File "/ws/src/genpy/src/genpy/message.py", line 392, in _check_types
    check_type(n, t, getattr(self, n))
  File "/ws/src/genpy/src/genpy/message.py", line 323, in check_type
    check_type('%s.%s' % (field_name, n), t, getattr(field_val, n))
  File "/ws/src/genpy/src/genpy/message.py", line 323, in check_type
    check_type('%s.%s' % (field_name, n), t, getattr(field_val, n))
  File "/ws/src/genpy/src/genpy/message.py", line 272, in check_type
    raise SerializationError('field %s must be float type' % field_name)
genpy.message.SerializationError: field pose.position.x must be float type

@mintar
Copy link
Contributor Author

mintar commented Aug 20, 2020

@dirk-thomas : If you don't like the added numpy dependency, we could avoid it like this (at the cost of doing not-so-nice string comparisons instead of type comparisons):

click to see outdated code
diff --git i/package.xml w/package.xml
index 0f09e82..5af3210 100644
--- i/package.xml
+++ w/package.xml
@@ -24,8 +24,8 @@
   <buildtool_depend condition="$ROS_PYTHON_VERSION == 2">python-setuptools</buildtool_depend>
   <buildtool_depend condition="$ROS_PYTHON_VERSION == 3">python3-setuptools</buildtool_depend>
 
-  <exec_depend condition="$ROS_PYTHON_VERSION == 2">python-numpy</exec_depend>
-  <exec_depend condition="$ROS_PYTHON_VERSION == 3">python3-numpy</exec_depend>
+  <test_depend condition="$ROS_PYTHON_VERSION == 2">python-numpy</test_depend>
+  <test_depend condition="$ROS_PYTHON_VERSION == 3">python3-numpy</test_depend>
 
   <exec_depend condition="$ROS_PYTHON_VERSION == 2">python-yaml</exec_depend>
   <exec_depend condition="$ROS_PYTHON_VERSION == 3">python3-yaml</exec_depend>
diff --git i/src/genpy/message.py w/src/genpy/message.py
index ad9f229..8e57c3b 100644
--- i/src/genpy/message.py
+++ w/src/genpy/message.py
@@ -43,8 +43,6 @@ import math
 import struct
 import sys
 
-import numpy as np
-
 import genmsg
 
 import yaml
@@ -264,7 +262,8 @@ def check_type(field_name, field_type, field_val):
             if field_val >= maxval:
                 raise SerializationError('field %s exceeds specified width [%s]' % (field_name, field_type))
         elif field_type in ['float32', 'float64']:
-            if type(field_val) not in [float, int, long, np.float32, np.float64, np.int8, np.int16, np.int32, np.int64, np.uint8, np.uint16, np.uint32, np.uint64]:
+            if type(field_val) not in [float, int, long] and not (type(field_val).__module__ == 'numpy' and type(field_val).__name__ in
+                    ['float32', 'float64', 'int8', 'int16', 'int32', 'int64', 'uint8', 'uint16', 'uint32', 'uint64']):
                 raise SerializationError('field %s must be float type' % field_name)
         elif field_type == 'bool':
             if field_val not in [True, False, 0, 1]:

Edit: Never mind, I've just pushed a commit (0f37e30) that avoids the numpy dependency this in a better way.

@dirk-thomas
Copy link
Member

Thanks for the improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

check_type does not check float
2 participants