There is currently two way to set limits to a servo

  • with servo.map(minInput, maxInput, minOutput, maxOutput)
  • with servo.setMinMax(minOutput, maxOutput)

Both method are doing a good job at keeping the servo inside the limits. However, they behave differently and that dual behavior is causing problem to services using Servo. Tracking service is an example of service that suffer from it

the servo.map method takes an input range and convert it to an output range, while setMinMax limits the output range by clipping value not in his range.

I understand that the setMinMax method is probably there that way from the begining of the time, but I think with the evolution of the Servo class make it inadapted for interraction with services that want to use the Servo class. 

The main problem is that with two different way of using limits, it need two different way to remember and know where the servo have been and the two way can easily be mix. It should only have one way to set the servo limits

My view of the Servo class should that it is responsible to manage the servo interaction, including keeping it's position inside the "good" range. But with the current setMinMax implementation, other service constantly need to access the min/max output so it can send valid position. In my view, min/max output value sould be set for a servo, but never used outside of the Servo class.

My proposal is to keep the method setMinMax(minOutput, minInput) but make it behave the same way as using using the map() method. So

servo.setMinMax(0,90) = servo.map(0,90,0,90).

This will simplify the code by a lot and make thing easier to use

 

By example, here the current Tracking code that move the servo

    double currentXServoPos = x.getPos();
    double currentYServoPos = y.getPos();

    // TODO - work on removing currentX/YServoPos - and use the servo's
    // directly ???
    // if I'm at my min & and the target is further min - don't compute
    // pid
    if ((currentXServoPos <= Math.min(x.getMin(), x.getMax()) && xSetpoint - targetPoint.x < 0) || (currentXServoPos >= Math.max(x.getMin(), x.getMax()) && xSetpoint - targetPoint.x > 0)) {
      error(String.format("%f x limit out of range", currentXServoPos));
    } else {

      if (pid.compute("x")) {
        if(x.isInverted()) {
          currentXServoPos -= pid.getOutput("x");
        }
        else{
          currentXServoPos += pid.getOutput("x");
        }
        if (currentXServoPos != lastXServoPos) {
          // calamity- fix          
          x.moveTo(currentXServoPos);
          currentXServoPos = x.getPos();
          lastXServoPos = currentXServoPos;
        }
        // TODO - canidate for "move(int)" ?

      } else {
        log.warn("x data under-run");
      }
    }
in that bit of code, what I put in red is Servo Input value and what is in Yellow is servo output value. So mixing Input/Output will make that code work only with a default map setting.  
 
All that is in yellow should also been take care by the Servo class, not the by a service using Servo
 
We can simplify this code like this
 
if (pid.compute("x")){
   currentXServoPos += pid.getOutput("x");
   x.moveTo(currentXServoPos);
   currentXServoPos = x.getPos();
}
else {
   log.warn("x data under-run");
}
 
All the rest is take care by the Servo and Mapper class. 
 
But this wont work with how the setMinMax() is setting parameters. That's why I want both way to do the same thing
 
 
 

GroG

7 years 7 months ago

"setMinMax method is probably there that way from the begining of the time"

That is very true..  You can understand why things are, by why they were in the past .. 

When setMinMax was around there was NO concept of input & output and mapping .. mapping came later, and was more powerful.

Now is the time that getPos get what we expect !

My vote is AYE !

Imho the setOutputRange of the PID should be used to set the min and max values, so that the servo never will get values that makes it go out of range. That will make the PID work better too.

The log.warn("x data under-run") is really not an error. That will occur if the PID loop runs faster than the Tracking loop, and is not an error. It's how the PID compuitation takes care of the timing.

 

juerg

7 years 7 months ago

would welcome the intended change by Calamity to make things easier. I find mixing setMinMax and map is confusing. And if it makes the program simpler it's even better.

Maybe we should have somebody giving an example where the setMinMax as currently defined is in fact a necessity for the servo control.

calamity

7 years 7 months ago

Thanks Mats for the headups. I know nothing of the PID service and did not look further than the code in the Tracking service.

If it make the PID work better, then the setOutputRange should be set. 

That bit of code was to show that if we send a moveTo() outside the input range, the Servo class will adjust the input value to be inside it's input range, similar to what the the setMinMax() was doing. The difference is on what servo.getPos() return. With the map() method, it will return the adjusted input value that is inside the input range. The setMinMax() method will have servo.getPos return a value outside the min/max value

Another advantage of the map() method is that it can use any range of data as input in the Tracking service. The setMinMax method restricted the input value of the servo to be a valid output range for the servo (0-180)

GroG

7 years 7 months ago

Thanks Calamity for taking the lead on this.

I'd recommend summarizing the current proposed changes, and what the "user" will expect.

:)

the current proposed change is about normalizing the limitation setting for the Servo. It will make servo.setMinMax(min,max) set the Input and Output range at the same time with the same value. So

servo.setMinMax(min,max) do the same thing as servo.map(min,max,min,max). 

This change will allow the servo class to compute all the commands with input value and become the unique unit that manage the actual output value to the servo hardware.

For the "user" perspective, I don't think it will change anything. The only thing they may notice is that the slider in the GUI will go from min to max value (instead of the default 0-180) and there will be no "dead" zone in the slider when using setMinMax to set servo limits. That's already the behavior happening when using the map method to set servo limits.

for the "developer", the change will allow to make services that use servo without actually care about the servo output as everything will be manage by the servo class

More than 2 years old but Soooo useful.  Thanks Calamity !
This last few months I've been working on refactoring webgui and the new servo.  The webgui called AbstractServo.setMinMax(min, max) => mapper.setMinMaxInput(min, max); - which behaved strangely ...
I looked back on previous posts and the source-code of the LegacyServo was changed to :
setMinMax(min, max) => mapper.map(min, max, min, max).
Reading through post I updated the new servo to do the "map" ... 

And Yay ! ... it behaves as I would expect now .

This is on get_servos_worky branch.
The position slider end-points do not change, but I don't think I'd expect that as a user.  What I also did not expect is the strange behavior of the (old?) setMinMaxInput(min, max); when it just set minX and maxX ... I thought input would be clipped, but there is no clipping of those values in calc :P.

Now, with the replacement of AbstractServo.setMinMax(min, max) => mapper.map(min, max, min, max) there are dead zones (in this case from 0-82 and 134-180), but that is what I would expect when dealing with this gui.