scpeters-test / sdformat

Simulation Description Format (SDF) parser and description files.
http://sdformat.org
Other
1 stars 0 forks source link

Remove attribute `type` from `<sensor>` and infer type from child tags #175

Open scpeters-test opened 6 years ago

scpeters-test commented 6 years ago

Original report (archived issue) by Shane Loretz (Bitbucket: Shane Loretz).


Issue

The type attribute duplicates information that can be inferred from the child tags. It also gets in the way of custom sensors that don't fit one of the predefined types.

Proposal

This proposes to remove the attribute type from the tag <sensor>. This sensor is a camera sensor because it has parameters for a camera.

<sensor name="some_camera">
    <pose>...</pose>
    <camera>...</camera>
</sensor>

This sensor doesn't have a type known by SDFormat, but that's OK because a plugin is responsible for it.

<sensor name="some_camera">
    <pose>...</pose>
    <plugin name="microphone_sensor_plugin">...</plugin>
</sensor>
scpeters-test commented 6 years ago

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


The type is used to distinguish between different implementation types. Take the gpuray and ray sensor types as an example. Both of these sensors use the <ray> child element.

scpeters-test commented 6 years ago

Original comment by Shane Loretz (Bitbucket: Shane Loretz).


Hmm. Is there a way to capture gpu_ray vs ray and allow custom sensors?

Looking at the gazebo implementations gpu_ray sees the geometry on <visual> elements while ray sees the geometry on<collision>. If that distiction became part of sdformat by splitting <ray> into <visual_ray> and <collision_ray> then this proposal would work for that case.

scpeters-test commented 6 years ago

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


Right, we can still remove the type attribute, but we'd have to expand the set of child elements.

Type is useful in cases where child data would be duplicated. I think there are a few sensors that use the <camera> child elements with different type attributes.

We could also make type="custom" as a valid sensor type.

scpeters-test commented 6 years ago

Original comment by Shane Loretz (Bitbucket: Shane Loretz).


While de-duplicating the type info would be nice, I have to admit adding type="custom" is a small change that solves the issue.

scpeters-test commented 6 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters).


The multi camera has multiple child <camera> elements:

        <sensor name="cam2" type="multicamera">
          <update_rate>20</update_rate>
          <camera name="left">
            <horizontal_fov>1.3962634</horizontal_fov>
            <image>
              <width>1024</width>
              <height>544</height>
              <format>L8</format>
            </image>
            <clip>
              <near>0.02</near>
              <far>100</far>
            </clip>
          </camera>
          <camera name="right">
            <pose>0 -0.1 0 0 0 0</pose>
            <horizontal_fov>1.3962634</horizontal_fov>
            <image>
              <width>1024</width>
              <height>544</height>
              <format>L8</format>
            </image>
            <clip>
              <near>0.02</near>
              <far>100</far>
            </clip>
          </camera>
        </sensor>