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

[Feature Request] JointLimit should be inferred to be unlimited if a urdf has no explicitely defined JointLimit. #99

Open
rydb opened this issue Jan 29, 2024 · 4 comments

Comments

@rydb
Copy link

rydb commented Jan 29, 2024

Looking at deserialize.rs

#[derive(Debug, YaDeserialize, YaSerialize, Default, Clone)]
pub struct JointLimit {
    #[yaserde(attribute)]
    pub lower: f64,
    #[yaserde(attribute)]
    pub upper: f64,
    #[yaserde(attribute)]
    pub effort: f64,
    #[yaserde(attribute)]
    pub velocity: f64,
}

f64 defaults to 0.0, this means the max lower and upper bound is inferred to be 0.0. This means, if a urdf has no explicitely defined joint limit, urdf-rs defaults to the joint not being able to move at all

I think Default for JointLimit should be changed to the below to fix that.

impl Default for JointLimit {
    fn default() -> Self {
        Self {
            lower: f64::MIN,
            upper: f64::MAX,
            effort: f64::MAX,
            velocity: f64::MAX,
        }
    }
}
@luca-della-vedova
Copy link
Contributor

I'm not too sure about this default, it seems the docs specify that the default should be 0 if unspecified (here), This also seems to be the behavior for the ROS parser

@rydb
Copy link
Author

rydb commented Jan 30, 2024

I'm not too sure about this default, it seems the docs specify that the default should be 0 if unspecified (here), This also seems to be the behavior for the ROS parser

looking at the docs them selves:

<limit> (required only for revolute and prismatic joint)

    An element can contain the following attributes:

    lower (optional, defaults to 0)

        An attribute specifying the lower joint limit (in radians for revolute joints, in meters for prismatic joints). Omit if joint is continuous. 

    upper (optional, defaults to 0)

        An attribute specifying the upper joint limit (in radians for revolute joints, in meters for prismatic joints). Omit if joint is continuous. 
 Omit if joint is continuous. 

this implies there are instances where ROS assumes there are no limits like on continuous joints. I suppose it makes sense from a safety sense, but its still inconvenient.

Maybe there should be a JointLimit::unlimited() then?

@luca-della-vedova
Copy link
Contributor

Yap there are continuous joints, it's a separate type (not revolute), have a look here.
Now someone could make the case that we could exploit Rust's enum system better and make sure JointLimit is only contained if the joint is of a variant where it makes sense, but that would make serialization / deserialization a fair bit trickier

@taiki-e
Copy link
Contributor

taiki-e commented Feb 7, 2024

One option would be to add a method that returns Option<Range> based on JointType like this.

Given that it would complicate the parsing code, it would not necessarily be easy to embed the limit in the JointType. However, it would probably be the most type-safe way to do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants