WIP: Towards adding an SDFormat parser to this repo.#32
Conversation
d35bf5c to
c2fea06
Compare
OTL
left a comment
There was a problem hiding this comment.
I added some comments. Basically, it seems that good direction for me.
(I wrote comments before knowing that WIP: in the title.)
|
|
||
| ```rust | ||
| let urdf_robo = urdf_rs::read_file("sample.urdf").unwrap(); | ||
| let urdf_robo = urdf_rs::urdf::read_file("samples/sample.urdf").unwrap(); |
There was a problem hiding this comment.
I want to keep urdf_rs::read_file() interface.
I think it's acceptable to move to urdf_rs::urdf::read_file, but urdf_rs::read_file should be kept.
How about reading sdf file if the extension is .sdf in urdf_rs::read_file ?
There was a problem hiding this comment.
Would have to manage an enum return type then but we could add that.
Another option might be urdf_rs::read_urdf_file but IDK if that's any better really.
There was a problem hiding this comment.
Ah, I understand about the type. I prefer to keep the current API.
I mean adding urdf_rs::read_stf_file()
| @@ -0,0 +1,5 @@ | |||
| mod elements; | |||
There was a problem hiding this comment.
I don't want to use mod.rs, I think it's an old style.
I think it should be src/common.rs in modern style.
There was a problem hiding this comment.
Hm, is there another way to split up into multiple files? IDK how much this module will grow
There was a problem hiding this comment.
Oh I see - this style is new to me, didn't realise common.rs can have the same contents of common/mod.rs. I can see how that might be clearer in tabbed editors. I'll switch.
There was a problem hiding this comment.
We want to use rust2021 edition style if possible.
There was a problem hiding this comment.
FYI, there are clippy lints to force such module styles.
https://rust-lang.github.io/rust-clippy/master/index.html#module_files
| @@ -1,6 +1,9 @@ | |||
| use crate::deserialize::Robot; | |||
| //! ROS-specific functions that make use of `rosrun` and `rospack` as | |||
There was a problem hiding this comment.
This rename also changes the interface, but it might be accepted. Because it's reasonable.
| use crate::sdf::deserialize::*; | ||
|
|
||
| use std::path::Path; | ||
|
|
There was a problem hiding this comment.
At least, bellow sort_link_joint() seems to be a duplication of urdf/funcs:sort_link_joint(). It can be avoided.
There was a problem hiding this comment.
Yeah this file in particular I haven't really looked at at all.
|
Any updates on this? |
|
No updates, we're currently using sdformat python bindings, and it's working acceptably. I'm not likely to put any time into this soon, but would use it if it existed 🤷 |
Refactor to move urdf functions and types under
urdf_rs::urdf, in order to start implementing SDF parsing underurdf_rs::sdf.I haven't actually started writing the SDF code yet.
N.B. My interest here is still experimental - I'm not on the road to using urdf_rs in a product and as such may not be very responsive. That being said, I'm very excited about Rust's possibilities in Robotics.