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

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

Closed
osrf-migration opened this issue Feb 8, 2018 · 6 comments
Closed

Comments

@osrf-migration
Copy link

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


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>
@osrf-migration
Copy link
Author

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.

@osrf-migration
Copy link
Author

Original comment by Shane Loretz (Bitbucket: Shane Loretz, GitHub: sloretz).


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.

@osrf-migration
Copy link
Author

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.

@osrf-migration
Copy link
Author

Original comment by Shane Loretz (Bitbucket: Shane Loretz, GitHub: sloretz).


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

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


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>

@azeey
Copy link
Collaborator

azeey commented Oct 17, 2024

type="custom" is supported as of #652. Following the discussion above, it seems that we want to keep the type attribute and since this is an old issue, I'll go ahead and close it.

@azeey azeey closed this as completed Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants