openrr / urdf-rs

URDF parser for Rust
Apache License 2.0
30 stars 11 forks source link

Bad namespace #87

Closed dsandber closed 9 months ago

dsandber commented 9 months ago

I'm trying to view a urdf file made by the OnShape importer, and I'm getting this:

Error: Urdf(UrdfError(Other("bad namespace for robot, found http://www.ros.org")))

What should the namespace be and can the importer be changed to issue a warning instead?

taiki-e commented 9 months ago

Thanks for the report! xmlns="http://www.ros.org" should be correct here, but it does not seem to be handled properly at this time.

@luca-della-vedova: Is there a way to get yaserde to handle this properly? I tried adding #[yaserde(namespace = "http://www.ros.org")] and got the same error.

luca-della-vedova commented 9 months ago

Very interesting, there is a unit test that shows that namespaces should work here. I tried to reproduce the issue in the latest version and it doesn't happen, it seems the PR that introduced the test https://github.com/media-io/yaserde/pull/124 was merged after the 0.7.1 version was released, hence it didn't make it into the version we use. I guess the solution is either to ping the maintainer to make a patch release with that fix (imho not very likely to happen), or resurrect the yaserde bump https://github.com/openrr/urdf-rs/pull/67. Last time there was a concern about using a private API, maybe we can work around them by pinning yaserde to an exact version so we are safe from potential breakage in patch versions?

taiki-e commented 9 months ago

Thanks for the investigation! I opened #88 to update yaserde to 0.8 and add #[yaserde(namespace = ...)].

Last time there was a concern about using a private API, maybe we can work around them by pinning yaserde to an exact version so we are safe from potential breakage in patch versions?

Well, even if they made the re-exports not public APIs, we can still depend on the same version of xml-rs because xml-rs is a public dependency of their APIs anyway.

(Ideally I would like them to make re-export a public API again, though.)