urdf-rs icon indicating copy to clipboard operation
urdf-rs copied to clipboard

[BUG] If <origin> is below <geometry> in a urdf file, then <origin> doesn't load.

Open rydb opened this issue 2 years ago • 7 comments

E.G: the below code will cause right_leg have its origin set to xyz = [0, 0, 0] and rpy = [0, 0, 0], even though it is a valid xml that does not error when loaded by urdf_rs

invalid

<?xml version="1.0"?>
<robot name="origins">
  <link name="base_link">
    <visual>
      <geometry>
        <cylinder length="0.6" radius="0.2"/>
      </geometry>
    </visual>
  </link>

  <link name="right_leg">
    <visual>
      <geometry>
        <box size="0.6 0.1 0.2"/>
      </geometry>
      <origin rpy="0 1.57075 0" xyz="0 0 -0.3"/>
    </visual>
  </link>

  <joint name="base_to_right_leg" type="fixed">
    <parent link="base_link"/>
    <child link="right_leg"/>
    <origin xyz="0 -0.22 0.25"/>
  </joint>

</robot>

valid

<?xml version="1.0"?>
<robot name="origins">
  <link name="base_link">
    <visual>
      <geometry>
        <cylinder length="0.6" radius="0.2"/>
      </geometry>
    </visual>
  </link>

  <link name="right_leg">
    <visual>
      <origin rpy="0 1.57075 0" xyz="0 0 -0.3"/>
      <geometry>
        <box size="0.6 0.1 0.2"/>
      </geometry>
    </visual>
  </link>

  <joint name="base_to_right_leg" type="fixed">
    <parent link="base_link"/>
    <child link="right_leg"/>
    <origin xyz="0 -0.22 0.25"/>
  </joint>

</robot>

rydb avatar Dec 29 '23 22:12 rydb

Thanks for the report! This is another regression introduced in https://github.com/openrr/urdf-rs/pull/64. I confirmed urdf-rs 0.6 (that uses serde-xml-rs) doesn't have this problem.

taiki-e avatar Jan 09 '24 10:01 taiki-e

Can you check if this branch fixes it?

luca-della-vedova avatar Jan 16 '24 03:01 luca-della-vedova

Can you check if this branch fixes it?

Can you make the version requirement of thiserror looser? I can't test urdf-rs due to the below cargo error.

cargo check failed to start: Cargo watcher failed, the command produced no valid metadata (exit code: ExitStatus(unix_wait_status(25856))):

Updating git repository `[https://github.com/luca-della-vedova/urdf-rs`](vscode-file://vscode-app/snap/code/148/usr/share/code/resources/app/out/vs/code/electron-sandbox/workbench/workbench.html)

Updating crates.io index

error: failed to select a version for thiserror.

... required by package `urdf-rs v0.8.0 ([https://github.com/luca-della-vedova/urdf-rs?rev=cb42f5a#cb42f5aa)`](vscode-file://vscode-app/snap/code/148/usr/share/code/resources/app/out/vs/code/electron-sandbox/workbench/workbench.html)

... which satisfies git dependency `urdf-rs` of package `bevy_serialization_extras v0.2.1 (/home/ry/Projects/bevy_serialization_extras)`

versions that meet the requirements ^1.0.56 are: 1.0.56

all possible versions conflict with previously selected packages.

previously selected package thiserror v1.0.50

... which satisfies dependency `thiserror = "^1.0"` (locked to 1.0.50) of package `bevy_obj v0.12.0`

... which satisfies dependency `bevy_obj = "^0.12"` (locked to 0.12.0) of package `bevy_serialization_extras v0.2.1 (/home/ry/Projects/bevy_serialization_extras)`

failed to select a version for thiserror which could resolve this conflict

Failed to load workspaces.

rydb avatar Jan 16 '24 19:01 rydb

Ah you are working with bevy! That's awesome, I worked with bevy_obj as well :)

Since bevy_obj just needs 1.0 I believe a simple cargo update should fix your build error.

Edit: Regarding the making the version looser, I noticed that urdf-rs also fixes a patch version so I just updated that, I'm not sure if it really needs 1.0.7 or if we can just fix it to 1.0

luca-della-vedova avatar Jan 17 '24 02:01 luca-della-vedova

I noticed that urdf-rs also fixes a patch version

No, "1.0.7" means ">= 1.0.7, < 2.0.0" (not "=1.0.7") because the cargo's default version requirement strategy is caret requirements.

https://github.com/openrr/urdf-rs/blob/e07b0ea013b47223213708a15430a2350c4ef02e/Cargo.toml#L16

I'm not sure if it really needs 1.0.7 or if we can just fix it to 1.0

1.0.7+ is needed because we use a feature only available since https://github.com/dtolnay/thiserror/releases/tag/1.0.7

https://github.com/openrr/urdf-rs/blob/e07b0ea013b47223213708a15430a2350c4ef02e/src/errors.rs#L7

taiki-e avatar Jan 17 '24 02:01 taiki-e

OK thanks! I reverted the update since it's not necessary.

luca-della-vedova avatar Jan 17 '24 03:01 luca-della-vedova

As for an error about thiserror: I think the real problem is that one of bevy_obj's dependencies (or itself) is pinning a proc-macro related dependency (I remember wgpu doing that before), but it's okay to remove the dependency on thiserror to avoid the problem. It would not be too complicated even if we wrote that manually.

taiki-e avatar Jan 17 '24 03:01 taiki-e

Fixed in 0.9.0 (#98, #107).

taiki-e avatar Sep 06 '24 07:09 taiki-e