Go Down

Topic: Wish list for Servo library enhancements (Read 2 times) previous topic - next topic

mem

A couple of recent threads have discussed servo enhancements such as setting the rate of servo movement or automatically sweeping at user provided speeds.
I would be interested to hear if these or other features would be a popular addition to the Servo library. No promises that suggestions will be quickly implemented but if a number if people want enhancements that fit within the spirit of the current library then it could be considered for a future release.

see: http://arduino.cc/forum/index.php/topic,61586.0.html
and: http://arduino.cc/forum/index.php/topic,68305.msg504226.html

vinceherman

Trim and Reversing are features in most RC transmitters.
I added those features to the servo library (a few versions old) for my own use.

mem


Trim and Reversing are features in most RC transmitters.
I added those features to the servo library (a few versions old) for my own use.


Yes, they are standard features on an RC transmitter, but I wonder if the ease of adding a map function to get this capability into a sketch is one reason why more people don't request this feature. How many bytes of RAM did you use for the trim functions.

One way of implementing trim without using and RAM is to modify the min and max values that can be set in the attach method. The resolution of these values is currently 4 microseconds, I wonder if that would be fine enough for trim settings?

robtillaart

Quick look in the lib:

Servo::write
1) negative values make no sense , replace  by unsigned int types ?
2) I don't like it that the param value in write() can have two distinct meanings.
Code: [Select]
void Servo::write(int value)  <<<<<<<<< make this unsigned int ??

  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;     <<<<<<<<<<<<<< that line not needed if value is unsigned int
    if(value > 180) value = 180;
    value = map(value, 0, 180, SERVO_MIN(),  SERVO_MAX());     
  }
  this->writeMicroseconds(value);
}
==>

// only supports angles between 0..180
// use writeMicroseconds for pwm
void Servo::write(uint8_t value)

  if(value > 180) value = 180;
  this->writeMicroseconds(map(value, 0, 180, SERVO_MIN(),  SERVO_MAX()));     
}


Servo::writeMicroseconds
1) negative value make no sense , replace by unsigned int?
Code: [Select]
void Servo::writeMicroseconds(int value) 
==>
void Servo::writeMicroseconds(unsigned int value)[/code 


[b]Servo::attach[/b]
1) negative value make no sense , replace by unsigned int?
2) test MIN_PULSE_WIDTH <= min <= max <= MAX_PULSE_WIDTH?
uint8_t Servo::attach(int pin, int min, int max) 
==>
uint8_t Servo::attach(uint8_t pin, unsigned int min, unsigned int max)


Servo::read
1) strange values if ( this->servoIndex == INVALID_SERVO ); readMicroseconds returns value 0 << SERVO_MIN.
Code: [Select]
int Servo::read() // return the value as degrees
{
  return  map( this->readMicroseconds()+1, SERVO_MIN(), SERVO_MAX(), 0, 180);     
}
==>
int Servo::read() // return the value as degrees  && -1 if invalid.
{
  if ( this->servoIndex != INVALID_SERVO )
     return map( this->readMicroseconds()+1, SERVO_MIN(), SERVO_MAX(), 0, 180);
  return -1;   
}



Servo::readMicroseconds
1) return type of function differs from internal var pulsewidth; which is stricly speaking not needed
2) using -1 for invalid values..
Code: [Select]
int Servo::readMicroseconds()
{
  unsigned int pulsewidth;
  if( this->servoIndex != INVALID_SERVO )
    pulsewidth = ticksToUs(servos[this->servoIndex].ticks)  + TRIM_DURATION ;   // 12 aug 2009
  else
    pulsewidth  = 0;  //

  return pulsewidth;   
}
==>
int Servo::readMicroseconds()
{
  if( this->servoIndex != INVALID_SERVO )
    return ticksToUs(servos[this->servoIndex].ticks)  + TRIM_DURATION ;   // 12 aug 2009
  return -1;   
}


my 2 cents
Rob

Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

bill2009

Not a concrete wish list item but i find myself struggling with the mismatch between real servos and the model. E.G. Many servos can't really rotate 180 and can't take pulses from 644 to 2400. If i set the limits to protect the servo i have to map the angle for Accuracy.

retrolefty


Not a concrete wish list item but i find myself struggling with the mismatch between real servos and the model. E.G. Many servos can't really rotate 180 and can't take pulses from 644 to 2400. If i set the limits to protect the servo i have to map the angle for Accuracy.


Unfortunately that's a nature of the beast, the hobby servo industry never agreed on extending the 'standard' PPM range (1 to 2 milliseconds, and 1.5 millisecond for midpoint) for the 'standard' end of range values, and there was never a 'standard' mechanical travel range. So the software library has to adjust to the servo brand/model you are using.

Once you do know your servo's actual PPM travel range there is the optional "servo.attach(pin, min, max)" where min and max are the actual end of travel pulse widths for your specific servo. Personally I never liked the arduino abstraction of servo 'degrees' and have always used the "myservo.writeMicroseconds(1500);  // set servo to mid-point" command and any mapping I use is typically expressed in 'percentage' of travel 0-100% mapped to min to max pulse width.

Lefty


vinceherman

Personally I never liked the arduino abstraction of servo 'degrees' and have always used the "myservo.writeMicroseconds(1500);  // set servo to mid-point" command


Me too!


Yes, they are standard features on an RC transmitter, but I wonder if the ease of adding a map function to get this capability into a sketch is one reason why more people don't request this feature. How many bytes of RAM did you use for the trim functions.


My recollection is that the trim added 1 byte.  I also used the 4microsecond accuracy to trade some resolution for range.  The reverse flag was in the pinout attribute so used an otherwise unused bit.


One way of implementing trim without using and RAM is to modify the min and max values that can be set in the attach method. The resolution of these values is currently 4 microseconds, I wonder if that would be fine enough for trim settings?


I do not see how modifying the end points changes where the servo centers.  Servo horn attachment leaves the horn connected at a somewhat random angle (within a fraction of the spline angle)  Trim is used so that when everything else says for a servo to be centered, the horn is actually centered.  Adding the trim feature keeps the logic in the code clean.  Tell the servo to go straight, and the servo is straight.

I know that this results in telling the servo to go to position 1500, and the resulting servo.writemicroseconds sends a value different than 1500.  But this is a servo class, using methods that make working with servos convenient.
Trim and Reverse are features that are so convenient, they have been included in most systems using servos for ages.


Not a concrete wish list item but i find myself struggling with the mismatch between real servos and the model. E.G. Many servos can't really rotate 180 and can't take pulses from 644 to 2400. If i set the limits to protect the servo i have to map the angle for Accuracy.


This brings up another feature I added to my version of the library.  Scaling.  I have 30 of the same servos that I ordered at the same time.  I tested each, and there is deviation in the servo movement per uS.  I used a protractor to measure difference in servo movement with two pulses that differed by 1600uS.  I expected 160 degrees of servo movement.  What I got was 164 degrees through 174 degrees.  I am also using 6 higher torque servos.  They gave me 160 degrees.
Since I want a predictable amount of movement for a given command, I want to assign a scaling factor to each servo.  I can use the same map command I use to reverse the servo, but just modify the range by the scaling factor, and get the results I am looking for.  This feature adds another byte to the servo structure.

This particular scaling feature is less critical in my mind that trim and reversing.

mem


Quick look in the lib:

Servo::write
1) negative values make no sense , replace  by unsigned int types ?
2) I don't like it that the param value in write() can have two distinct meanings.

Servo::attach
1) negative value make no sense , replace by unsigned int?
2) test MIN_PULSE_WIDTH <= min <= max <= MAX_PULSE_WIDTH?

Servo::read
1) strange values if ( this->servoIndex == INVALID_SERVO ); readMicroseconds returns value 0 << SERVO_MIN.

Servo::readMicroseconds
1) return type of function differs from internal var pulsewidth; which is stricly speaking not needed
2) using -1 for invalid values..


Rob, thanks for your input.  I do agree that the argument types for Arduino libraries such as this appear less than ideal.  The current Servo library was  a drop in replacement for the one used prior to version 0017 and the argument and return types were deliberately kept the same.
ints are used throughout Arduino code to express values that cannot be negative, for example see analogRead and digitalRead.  If you can convince the Arduino team to change the rest of the API I will be more than happy to update the core Servo library accordingly.

Your point about the dual nature of write is well taken. It was included for backwards compatibility with an earlier library but  in hindsight I should have removed this capability from the initial release of this library when it went into the Arduino core. If its not too late I will see if I can make this change for the 1.0 release.

There may be a way to address all your points without changing the core, see later post regarding a superServo library.

Michael

mem

superServo

The current sevo library uses relatively little RAM and the very good suggestions from bill2009 and vinceherman would increase this even if the new features were not used.

What do you guys think about adding new features into a class built on top of Servo? This library would derive from Servo and contain additional fields as necessary for the new features. It could also have the API tweaks Rob suggested earlier in the thread.

robtillaart

@mem
(unsigned  vs signed etc)
Quote
If you can convince the Arduino team to change the rest of the API I will be more than happy to update the core Servo library accordingly.


The basic idea behind the arduino libs is to make working wit it as easy as possible. that include preventing bugs if possible by building smart libraries/classes functions etc.
(this is my assumption why the arduino is succesfull)

Defining values that can't be negative as unsigned int, helps to prevent bugs that can be caught at syntax level, aka compile time. This fits the Arduino philosophy!!

As far as I can see my proposed changes (wrt signed/unsigned)  will not lead to backwards trouble as negative values would lead to errors, e,g, what would a write(-45) mean?  The param represent an absolute angle (my proposal) not a relative one. Maybe that is a point to consider too, using more descriptive names for the params/vars so people understand better when diving into the code. 

my proposed  Servo::write() would become
Code: [Select]
void Servo::write(uint8_t absoluteAngle)

  if(absoluteAngle > 180) absoluteAngle= 180;
  this->writeMicroseconds(map(absoluteAngle, 0, 180, SERVO_MIN(),  SERVO_MAX()));     
}


(superServo)
Think the idea of a superServo class is  good practice it keeps footprint as small as possible if not needed. Base both a virtual base class?
Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

mem

#10
Aug 06, 2011, 07:05 pm Last Edit: Aug 06, 2011, 07:39 pm by mem Reason: 1

@mem
(unsigned  vs signed etc)

Quote
If you can convince the Arduino team to change the rest of the API I will be more than happy to update the core Servo library accordingly.


Defining values that can't be negative as unsigned int, helps to prevent bugs that can be caught at syntax level, aka compile time. This fits the Arduino philosophy!!

As far as I can see my proposed changes (wrt signed/unsigned)  will not lead to backwards trouble as negative values would lead to errors, e,g, what would a write(-45) mean?  The param represent an absolute angle (my proposal) not a relative one. Maybe that is a point to consider too, using more descriptive names for the params/vars so people understand better when diving into the code.  


I think you missed my point that the Arduino APIs for things like Servo and analogWrite were defined from the start as signed types. Perhaps its a question of style but changing some of these abstractions but not others does not seem a good idea to me. Anyway, the API for the core libraries are the prerogative of the Arduino team so further discussion on signed vs unsigned arguments should be addressed to the developers list if you feel strongly about this point.

However, a richer and more rigorous API can be provided with a superServo wrapper that would remain compatible with the documented Arduino Servo functionality.






robtillaart

Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Go Up