point_cloud2 read_message returns 2 values when reading field

[sebaleme]

Hello,

I think I found a bug in the file point_cloud2.py

I wrote a python ros node for reading a bag with a pointcloud2 format (and 4 fields, x,y,z,intensity). When I read the 3 first fields, it works fine, but when I read "intensity", it returns 2 values, "y" and "intensity". After some research, I found out that the issue occurs in _get_struct_fmt(), which generates the read mask for the Struct.unpack function.

In the line:
for field in (f for f in sorted(fields, key=lambda f: f.offset) if field_names is None or f.name in field_names):
The for loop occurs twice in the case of intensity, probably because when doing if f.name in field_names , the interpreter is able to find a "y" in the string "intensity". Now, the real question is, why does it parse character wise is the key. Either this is a bug happening for all bags, or somehow the input parameter fields doesn t come with the right type. I would lean towards the first hypothesis, since the resulting field variable is interpreted as a structure, from where later the members name or offset are extracted.

In the ZIP, you can see 4 tests I did:

• Bag Unchanged ==> field intensity is named intensity
• Bag Changed ==> field intensity is named intensit
• python Unchanged ==> dictionaries fields_PI6 and fields_PI6_invers are using intensity
• python Changed ==> dictionaries fields_PI6 and fields_PI6_invers are using intensit

Results:
BagU PythonU ==> read_message returns 2 values for intensity
BagU PythonC ==> read_message returns 0 values for intensity
BagC PythonU ==> read_message returns 2 values for intensity
BagC PythonC ==> read_message returns 1 value for intensity
pc2_tests.zip

The fact that looking for intensit when the field is named intensity doesn t work (0 values) shows that in this case, the field_name is not considered as a charater list, but as a string. But why then the parsing intensity for the character y? Maybe this is the result of having python not strong typed?

Configuration:

• Ubuntu 18.04
• Ros Melodic
• Python 2.7

I had some difficulties to understand the file hierarchy. On my machine, sensor_message is installed directly under:
/opt/ros/melodic/lib/python2.7/dist-packages/sensor_msgs
And not under common_msgs as here in this repo. Maybe you reorganized the files? And also, there is no file provided or any XML giving me the package version.

Sebastien

[sebaleme]

As noted by SoftwareApe, if I force the type of the inputs, I dont get this error anymore:

fields_PI6 = {
0 : ['x'] ,
1 : ['y'] ,
2 : ['z'] ,
3 : ['intensity'] ,
}

Now, _get_struct_fmt() gets a list of string, and not a list of character list. However, I think it would be great to test the inputs in the function, and return at least a warning in case of wrong input type. Maybe something like:

    # Wrong input type could lead to unexpected behaviour
    if field_names is not None:
        for input_field_name in field_names:
            if isinstance(input_field_name, basestring):
                warnings.warn("Input should be a list of string")

However, this check doesn t work, because it always send out the warning, even if the input has the right format.

Below the callback code which gets the fields name from the bag topic. It works, I read the intensity properly, but I also get the warning.

def callback(data):
    print("pointcloud size is %d bytes" % len(data.data))

    fields_PI6 = []
    for field in data.fields:
        # the received names have the type "character list", but read_points() needs a "string"
        fields_PI6.append([field.name])

    pointCloudArray_a = []
    for indoux in range(len(fields_PI6)):
        pointCloudArray_a.append(np.array(list((read_points2(data, field_names=(fields_PI6[indoux]))))))
        print(fields_PI6[indoux], pointCloudArray_a[indoux].shape)

[peci1]

I think your usage is the problem:

read_points2(data, field_names=(fields_PI6[indoux]))

You pass a single string to field_names parameter. The docs @type says it expects an Iterable. Now, yes, a string is also an Iterable. But the docs in @param say "The names of fields" which clearly points at a list-like structure and not a single string.

If you'd like to read the points field-by-field, I think you're missing the comma that allows making single-element tuples in Python

read_points2(data, field_names=(fields_PI6[indoux],))

If you'd like to add a sanity check to _get_struct_fmt(), I think the correct thing to do would be:

if isinstance(field_names, basestring):
    field_names = (field_names,)

You can try sending in a pull request with this change (and ideally with a test case). The file you're looking for is https://github.com/ros/common_msgs/blob/noetic-devel/sensor_msgs/src/sensor_msgs/point_cloud2.py .