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

yarpmotorgui allows selecting some iCub_SIM joint values out of bounds. #2402

Open
gccunha1 opened this issue Nov 9, 2020 · 6 comments
Open
Assignees

Comments

@gccunha1
Copy link

gccunha1 commented Nov 9, 2020

Describe the bug
A clear and concise description of what the bug is.
yarpmotorgui allows selecting values for joint 3 of the iCub_SIM's arms out of their bounds.

To Reproduce
Steps to reproduce the behavior:

  1. Open iCub_SIM
  2. Open yarpmotorgui
  3. Try to move joint 3 of any arm to the minimum value allowed
  4. The arm will not move

Expected behavior
The minimum value allowed for joint 3 of the arms should be 15.5 and not 15.385 degrees.

Configuration (please complete the following information):

  • OS: Ubuntu 18.04.4 LTS
  • yarp version: 3.3.103+20200515.2+gitd74a0636e
  • compiler: gcc

Additional context
A video of what happens is attached in this issue robotology/community#442, as well as what are the bounds of the iCub robot given by IControlLimits::getLimits().

I will also attach the zip file with the video here.

Qt Robot Motor GUI V2.0 2020-11-05 16-06-23.zip

@drdanz
Copy link
Member

drdanz commented Nov 9, 2020

Sounds like a bug in iCub_SIM to me... @randaz81 what do you think?

@pattacini
Copy link
Member

I've tested iCub_SIM along w/ simpleClient in robotology/community#442 (comment) and the behavior seems correct to me.

The quirk seems to be related to the fact that yarpmotorgui allows the user to specify 15.385 degs as a possible target, which instead is out of bounds, given that the lower limit is actually 15.5 degs.

Perhaps, some subtle approximation is taking place under the hood there.

@randaz81
Copy link
Member

randaz81 commented Nov 9, 2020

TY. I'm checking it

@randaz81 randaz81 self-assigned this Nov 10, 2020
@randaz81
Copy link
Member

Hi, the issue is actually involving some reasoning on various components, which I'm detailing below.

  1. First of all iCub_SIM. The motion controller implemented in the simulator does not perform the correct check on the joint limits when a position command is received (only the arms are bugged!). Indeed, it's responsibility of the motion controller (or the control board firmware) to clamp an invalid command. Yarp interfaces are transparent in this sense. All the other motion controllers (canMotionControl, embObjMotionControl for the real robot, controlboard for gazebo-yarp-plugins) perform this check correctly.
    For this reason I opened this PR (Fix on icubSIM joint limits icub-main#690) which fixes the incorrect behavior of iCub-Sim (it's an authentic bug, because the if condition is missing its else statement)

  2. YarpMotorgui. The slider widget available in QT is implemented with an integer logic, which inevitably generates some inaccuracy near the boundaries. Improving this behavior is certainly possible, but requires some not-obvious development of a custom widget to implement a floating point logic on the boundaries. Additionally, Yarp is unaware of the measurement units used to control the joint. They could be degrees, radians, millimeters. For this reason, the definition of a safety threshold near the boundary is difficult and depends from case to case. The same consideration applies also to the minimum step of the slider.

BUT:

  1. even if it is difficult to define an universal accuracy step for slider, Yarpmotorgui already implements the possibility to define a custom value! Just open the sliders option window and define a custom step size. For the specific case described in this issue, defining a step of 0.1degrees is enough to prevent the problem.

@pattacini
Copy link
Member

Thanks @randaz81, PR integrated ✔

IMHO yarpmotorgui shouldn't allow for specifying out-of-range references anyway.
I would limit for example the effort to the handling of the bounds and perhaps keep the logic for the step size unchanged.

@randaz81
Copy link
Member

randaz81 commented Nov 11, 2020

Indeed the issue is not closed yet, but some longer reasoning/testing is required before taking an action.
e.g. What happens if I clamp the value to the exact limit before calling positionMove() ?
Some safety threshold (epsilon) is anyway needed because the double value can be approximated by the machine. This may cause similar issues anyway (15.5 could be later approximated to 15.500000001 or 14.499999999. The latter will trigger an incorrect behavior in a test >= 15.5) so we must be careful when checking the limits.

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

4 participants