Bug in Joints Limits: Fix and Potential Backport to ODE Repository

Issue #84 closed
Yannick Goumaz created an issue

While developing Webots, we encountered a bug related to hinge joints. The bug caused the angle limits (min and max) to be violated when the position of the joint moved out of the range [-PI;+PI], particularly when the limits were set close to -PI or +PI. We have addressed this issue with a fix that has been submitted in this PR. You can also find links to relevant issues.

We are considering backporting this fix to the main ODE repository. Would you be interested in this backport?

Comments (5)

  1. Oleh Derevenko

    I regret but your code change is invalid.

    First of all, dxJointLimitMotor::testRotationalLimit()'s purpose is to set ‘limt’ and ‘limit_err’ fields according to the value of ‘angle’ parameter which, obviously, has to be the current angle value for the joint. The function is not producing the angle but, rather, is getting it from outside. Therefore, it may not modify the angle value for its logic. Any modifications in this method may mismatch logic in places whete the angle is actually generated and therefore may lead to incorrect behavior.
    To put it short: a function should do the job it is intended and designed to and not something else.

    Even though the first objection is pretty sufficient, here are some more.

    I do not see a reason why the angle would have to be not able to change from 0 to something like 6*M_PI/5 if limits permit motion from 0 to 4*M_PI/3, for example. Changing the angle into negative values will move it below its low limit in this case.

    Also, I don’t see any solid reasons for subtracting 2*M_PI if delta_angle is 3.1*M_PI or 4*M_PI, or 5.2*M_PI, or similar.

  2. Yannick Goumaz reporter

    You are correct that the code location is incorrect, and it should not be contained within the testRotationalLimit function. I will fix this.

    However, regarding the behavior, I strongly believe that the angle should not be clamped between -M_PI and +M_PI when determining if a limit is violated and which one it is. To illustrate this, let's consider a simple example:

    1. Suppose you set a lower limit of -3 radians and an upper limit of +0.5 radians on a hinge joint.
    2. Next, you apply a torque that rotates your object from position 0 radian to negative angle values. As the values (depending on torque intensity and timestep magnitude) go from 0 to -0.1, -0.4, -0.9, and so on, at some point, the angle value will transition from, say, -2.85 radians to +3.04 radians (instead of -3.24 radians due to the range [-M_PI, M_PI]).
    3. In this case, testRotationalLimit will return that there is a limit violation on the upper limit, which is fundamentally incorrect because the lower limit has just been violated.
    4. This erroneous limit detection will cause the joint to rotate fully until it reaches the upper limit, resulting in a trajectory of the joint from the lower to upper limit in the forbidden zone, which is the opposite of the intended purpose of these limits.
    5. Combined with gravity, you can obtain some infinitely rotating objects even if the limit are set to values given in the example.
    6. This user's video in this issue demonstrates this example.

  3. Oleh Derevenko

    To fix this you would have to change angle generation so that it kept joint state in form of rotation value and accounted that context for tracking motion range and correcting angle values to avoid these discontinuities.

    Also,clamping rotation to M_PI alone is insufficient. There should be a way to know the applied rotation direction (plus or minus) and allowing motion for more than M_PI if the overall range limits permit.

    And also, this would affect several joints and would need to be inplemented for them all and then be possible to be tested. That is, existing ODE demos may need to be changed to demonstrate that the new feature works correctly.

  4. Log in to comment