Before the year is out, I vote we update the motor, servo, & position interfaces from int to double.

Double allows us better resolution for the future, and ints can change into doubles, but not the other way around.

And if there are questions to use a Double the object vs double the primitive :

  • Double can be null - so if you want the potential state of being null you have to use the object (e.g. sometimes this is used for the concept of "not being set", which can sometimes be useful)
  • double the primitive - "always" has a value, even if you don't initialize it, it will be initialized to 0.0
  • Assigning a Double which is null to a primitive double will result in a Null Pointer Exception. Usually, you want to avoid this.
  • If your method uses the Double object you will need to add a decimal in Java-land - I think Python doesn't require this .. For example if you make a method
    moveTo(Double position)
    In Java-land you have to servo.moveTo(30.0)
    however if a primitive is used
    servo.moveTo(30) will work
  • Invoking and remote method calls will automatically change into Objects - this is called "Boxing"
  • I think now, our invoking mechanism is smarter than before, and it can push Double object parameters into double primitive parameters ... previously it could not, or it would complain about upcasting..

I think usually the easy rule of thumb is (use the primitive double, unless you really want the concept of null supported)

 

calamity

7 years 3 months ago

I agree with that, I see no need to limits the positions to integers.

And that change will fit well with well with the change I just made. Servo can now take the range value in microsecond (544-2400), wich gives 1856 possible position instead of the 180 we used in the past.

You can use servo.map(0,180, 544, 2400) (or map(544,2400,544,2400) to use the send the microsecond value to the servo. No other change are required 

I'm all for this. Using a double is better than an Integer. I think the preference is always to use primitives when possible.

My only comment is the double should represent the angle. End users should never specify microseconds. If we want to do this change, the moveTo(double angle). Should take the angle and NOT the micro seconds.

the way moveTo is implement, it take an input value between a range of value and convert it into an output range usable by the servo

by default, the input range is set to 0-180, but it can be any range, from a degree position, a percentage (0-100) or a microsecond range (544-2400). All depend of the usage you want to do with the servo. The only important thing is that it must define a minimum and a maximum. 

By default it will always be 0-180. But I think it's a good thing to be able to change it with the map method

That's great, cause I'm also against breaking backward compatibility

Nothing was change in the map behaviour.

The only thing that was add is the servo can understand value between 544 and 2400 (microsecond range) in addition to the range 0-180 (wich is convert into the 544-2400 range by the arduino servo library).

Nothing is wrong in using the 0-180 value, but now we have a way to use the double value as input (like 67.445 degree)  and map to microsecond and have a much precise commande to send to servo instead of have to value cast to integer in the way

No. I disagree. Currently people use the map function to convert a set of angles such as 90 to 180 degrees to a different set of angles such as 0 to 90.

If you change the behavior so the 3rd and 4th args to the map function represent microseconds, then you will be breaking backwards compatablility..

no no no, you misunderstand me.

what you describe won`t change and will still work

Let me show you the Servo::write method of the arduino servo library

void Servo::write(int value)
{
  if(value < MIN_PULSE_WIDTH)
  {  // treat values less than 544 as angles in degrees (valid values in microseconds are handled as microseconds)
    if(value < 0) value = 0;
    if(value > 180) value = 180;
    value = map(value, 0, 180, SERVO_MIN(),  SERVO_MAX());
  }
  this->writeMicroseconds(value);
}
 
As you can see, the method accept an int with value between 0-180 (degree) or a value greater than MIN_PULSE_WIDTH (544, value in microsecond). The value in degree will be converted into microsecond value. and writeMicroseconds will be called.
 
What I did is to propagate this dual behavior to MRL.
 
In your example, you mean doing map(90,180,0,90) wich mean if you input the value 90 (moveTo(90)) the map function will convert it to a value of 0 (output) that will be send to the arduino and do servo.write(0).
That value will then be remap to 544 (by the map function in the arduino servo library. and will use writeMicroseconds(544)
 
Now we want to increase the precision so we can do servo.moveTo(double).
with the same map as you use as example (90,180, 0,90), we do servo.moveTo(90.5). That value will be map to 0.5 then send to the arduino, We need to cast that value to an int to use Servo.write() and lose the precision.
 
now using the following map that do exactly the same thing, but use microsecond value
map(90,180,544,1500)
 
the value 90.5 will be map to 549 and we don't lose the precision as we don't have the cast the double to the int.
 
 
 
 

GroG

7 years 3 months ago

writeMicroseconds and using micros() to calculate delta is completely in the implementation details of MrlServo.  There should be no changes to the interface except people can "have the option" of providing sub-degree amounts, and the movement of servos at subspeeds will be smoother.

So backward compatibility for these changes should be completely supported.

The one area were I cannot see a way to provide backward support is the setSpeed/setVelocity.
But I would suggest the following.
setSpeed use the new linear implementation by default.
we create a Servo.setLegacySpeedStyle(true/false) .. and set it false by default ...  people will complain and we can show them how to use the new linear implementation or tell them Servo.setLegacySpeedStyle(true)

 

Ah. I was confused thinking that you were taking about Servo.map(x,y,w,z).

So long as we maintain consistency in the Java/python layer then it should all be good.

We are free to change out anything at lower levels without breaking anything.

All good stuff. More resolution is better... Pretty much always.

Goodtimes

GroG

7 years 3 months ago

As a general rule a Service should use common units within ...

For example I believe Servo service should "always" be a double now and the unit should "always" be degrees (fractional degrees now allowed with the double)

There should be no 'mix' ... in fact targetOutput should be degrees - the conversion from degrees to more precise firmware unit of microseconds should be done in the Controller not ServoControl

all methods from ServoControll should return degrees as doubles.... and all input of values should be considered degrees as doubles...

If there is any method which uses a different value it should be painfully obvious ...
.e.g. Servo.writeMicroseconds ..

 

I agree with you except for one point

the input of the servo should not be considered as degrees, but should reflect what is set with the map function. If we expect value to be in degree, we are limiting the usage of the servo.

By example Gael use frequently the map method to set his servo. He use something like (0,180, 45,135) In that case the range 0-180 is not a value in degree, but he use it as a range where 0 = minOutput = 45 and 180 = maxOutput = 135.

I think the input should accept any value the user want to use. It could be value in degree (default), a range expressing MIN/MAX, a value in radian, or in percentage of effective movement

So what is important is that the servoControl work with INPUT value and try not to mix with OUTPUT value

I agree completely ...

The only thing I would have to add, is that without mapping, min max clipping, or setting of other parameters.. the Servo should have a simple and well defined 'default'

The "default" should be considered degrees - it makes things easy to start with a well defined default.