Skip to content

Unacceptable defaults that destroy servos #7023

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
Nov 14, 2020
Merged

Unacceptable defaults that destroy servos #7023

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
0827917
The default timings were chosen vastly outside specification. On COTS…
dok-net Jan 5, 2020
a2a1e0d
Change timing limits to safe values. With previous default timings an…
dok-net Jan 7, 2020
86caa07
Review action: revert changes to hard boundary for min/max duty cycle…
dok-net Jan 20, 2020
cc26161
A Servo on each available ESP8266 GPIO (D0-D8), no observed interfere…
dok-net Jan 20, 2020
e7695a9
Remove possible jerk due to force-cancelling duty cycle.
dok-net Feb 20, 2020
4c4906f
Overload attach: can specify initial angle for servo, too.
dok-net Feb 20, 2020
c551dc2
Stricter checks for configured, and default, bounded timings.
dok-net Mar 8, 2020
ac0f239
Missed the min/max in a comment.
dok-net Mar 8, 2020
ef33c5f
Default microsecond lower bound of 1000 causes confusing behavior - 2…
dok-net Mar 8, 2020
66933c1
Comment rationale for changed defaults of min/max pulse widths.
dok-net Oct 29, 2020
2f1e390
Merge branch 'master' into servo
earlephilhower Nov 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 20 additions & 15 deletions libraries/Servo/src/Servo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
uint32_t Servo::_servoMap = 0;

// similiar to map but will have increased accuracy that provides a more
// symetric api (call it and use result to reverse will provide the original value)
// symmetrical api (call it and use result to reverse will provide the original value)
int improved_map(int value, int minIn, int maxIn, int minOut, int maxOut)
{
const int rangeIn = maxIn - minIn;
const int rangeOut = maxOut - minOut;
const int deltaIn = value - minIn;
// fixed point math constants to improve accurancy of divide and rounding
const int fixedHalfDecimal = 1;
const int fixedDecimal = fixedHalfDecimal * 2;
constexpr int fixedHalfDecimal = 1;
constexpr int fixedDecimal = fixedHalfDecimal * 2;

return ((deltaIn * rangeOut * fixedDecimal) / (rangeIn) + fixedHalfDecimal) / fixedDecimal + minOut;
}
Expand All @@ -46,9 +46,9 @@ int improved_map(int value, int minIn, int maxIn, int minOut, int maxOut)
Servo::Servo()
{
_attached = false;
_valueUs = DEFAULT_PULSE_WIDTH;
_minUs = MIN_PULSE_WIDTH;
_maxUs = MAX_PULSE_WIDTH;
_valueUs = DEFAULT_NEUTRAL_PULSE_WIDTH;
_minUs = DEFAULT_MIN_PULSE_WIDTH;
_maxUs = DEFAULT_MAX_PULSE_WIDTH;
}

Servo::~Servo() {
Expand All @@ -58,10 +58,15 @@ Servo::~Servo() {

uint8_t Servo::attach(int pin)
{
return attach(pin, MIN_PULSE_WIDTH, MAX_PULSE_WIDTH);
return attach(pin, DEFAULT_MIN_PULSE_WIDTH, DEFAULT_MAX_PULSE_WIDTH);
}

uint8_t Servo::attach(int pin, uint16_t minUs, uint16_t maxUs)
{
return attach(pin, minUs, maxUs, _valueUs);
}

uint8_t Servo::attach(int pin, uint16_t minUs, uint16_t maxUs, int value)
{
if (!_attached) {
digitalWrite(pin, LOW);
Expand All @@ -76,7 +81,7 @@ uint8_t Servo::attach(int pin, uint16_t minUs, uint16_t maxUs)
_maxUs = max((uint16_t)250, min((uint16_t)3000, maxUs));
_minUs = max((uint16_t)200, min(_maxUs, minUs));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Tech-TX These are the hard-coded limits.


write(_valueUs);
write(value);

return pin;
}
Expand All @@ -85,27 +90,28 @@ void Servo::detach()
{
if (_attached) {
_servoMap &= ~(1 << _pin);
startWaveform(_pin, 0, REFRESH_INTERVAL, 1);
delay(REFRESH_INTERVAL / 1000); // long enough to complete active period under all circumstances.
stopWaveform(_pin);
_attached = false;
digitalWrite(_pin, LOW);
_valueUs = DEFAULT_NEUTRAL_PULSE_WIDTH;
}
}

void Servo::write(int value)
{
// treat values less than 544 as angles in degrees (valid values in microseconds are handled as microseconds)
if (value < _minUs) {
// treat any value less than 200 as angle in degrees (values equal or larger are handled as microseconds)
if (value < 200) {
// assumed to be 0-180 degrees servo
value = constrain(value, 0, 180);
// writeMicroseconds will contrain the calculated value for us
// for any user defined min and max, but we must use default min max
value = improved_map(value, 0, 180, _minUs, _maxUs);
}
writeMicroseconds(value);
}

void Servo::writeMicroseconds(int value)
{
value = constrain(value, _minUs, _maxUs);
_valueUs = value;
if (_attached) {
_servoMap &= ~(1 << _pin);
Expand All @@ -117,8 +123,7 @@ void Servo::writeMicroseconds(int value)

int Servo::read() // return the value as degrees
{
// read returns the angle for an assumed 0-180, so we calculate using
// the normal min/max constants and not user defined ones
// read returns the angle for an assumed 0-180
return improved_map(readMicroseconds(), _minUs, _maxUs, 0, 180);
}

Expand Down
36 changes: 24 additions & 12 deletions libraries/Servo/src/Servo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@
//
// Servo - Class for manipulating servo motors connected to Arduino pins.
//
// attach(pin ) - Attaches a servo motor to an i/o pin.
// attach(pin, min, max ) - Attaches to a pin setting min and max values in microseconds
// default min is 544, max is 2400
// attach(pin) - Attaches a servo motor to an i/o pin.
// attach(pin, min, max) - Attaches to a pin setting min and max values in microseconds
// default min is 1000, max is 2000
//
// write() - Sets the servo angle in degrees. (invalid angle that is valid as pulse in microseconds is treated as microseconds)
// writeMicroseconds() - Sets the servo pulse width in microseconds
Expand All @@ -44,13 +44,17 @@

#include <Arduino.h>

// the following are in us (microseconds)
//
#define MIN_PULSE_WIDTH 544 // the shortest pulse sent to a servo
#define MAX_PULSE_WIDTH 2400 // the longest pulse sent to a servo
#define DEFAULT_PULSE_WIDTH 1500 // default pulse width when servo is attached
#define REFRESH_INTERVAL 20000 // minumim time to refresh servos in microseconds
#define MAX_SERVOS 12
// The following values are in us (microseconds).
// Since the defaults can be overwritten in the new attach() member function,
// they were modified from the Arduino AVR defaults to be in the safe range
// of publically available specifications. While this implies that many 180°
// servos do not operate the full 0° to 180° sweep using these, it also prevents
// unsuspecting damage. For Arduino AVR, the same change is being discussed.
#define DEFAULT_MIN_PULSE_WIDTH 1000 // uncalibrated default, the shortest duty cycle sent to a servo
#define DEFAULT_MAX_PULSE_WIDTH 2000 // uncalibrated default, the longest duty cycle sent to a servo
#define DEFAULT_NEUTRAL_PULSE_WIDTH 1500 // default duty cycle when servo is attached
#define REFRESH_INTERVAL 20000 // classic default period to refresh servos in microseconds
#define MAX_SERVOS 9 // D0-D8

#if !defined(ESP8266)

Expand All @@ -63,8 +67,16 @@ class Servo
public:
Servo();
~Servo();
uint8_t attach(int pin); // attach the given pin to the next free channel, sets pinMode, returns channel number or 0 if failure
uint8_t attach(int pin, uint16_t min, uint16_t max); // as above but also sets min and max values for writes.
// attach the given pin to the next free channel, sets pinMode, returns channel number or 0 if failure.
// returns channel number or 0 if failure.
uint8_t attach(int pin);
// attach the given pin to the next free channel, sets pinMode, min, and max values for write().
// returns channel number or 0 if failure.
uint8_t attach(int pin, uint16_t min, uint16_t max);
// attach the given pin to the next free channel, sets pinMode, min, and max values for write(),
// and sets the initial value, the same as write().
// returns channel number or 0 if failure.
uint8_t attach(int pin, uint16_t min, uint16_t max, int value);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Tech-TX Have you tried either of these two overloads of attach()?

Copy link
Contributor

@Tech-TX Tech-TX Apr 18, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dok-net I pulled it this morning, Dirk, but I missed the attach() parameters. 😉

I really need to add a bunch of *.rst or *.md help files or additions for the stuff in the libraries, especially where things differ from the original Arduino libraries. That would help minimize confusion for our Gentle Users (and me!). The current info at ReadTheDocs is woefully incomplete for the servo library. Here's the full quote:

Servo
This library exposes the ability to control RC (hobby) servo motors. It will support up to 24 servos on any available output pin. By default the first 12 servos will use Timer0 and currently this will not interfere with any other support. Servo counts above 12 will use Timer1 and features that use it will be affected. While many RC servo motors will accept the 3.3V IO data pin from a ESP8266, most will not be able to run off 3.3v and will require another power source that matches their specifications. Make sure to connect the grounds between the ESP8266 and the servo motor power supply.

I didn't know we had 24 I/O pins that could drive servos. You learn something every day... and now I need to hunt down the 13 other (missing) GPIO pins. 😄

Here's the info on the Arduino library: https://www.arduino.cc/en/Reference/ServoAttach
so the 8266 library has an INT VALUE option for the initial starting degrees that the older library doesn't. Also, the max servos went from 12 to 9 with this PR, so we should mention that somewhere.

Your PR is working OK with my test bed and servos.

void detach();
void write(int value); // if value is < 200 its treated as an angle, otherwise as pulse width in microseconds
void writeMicroseconds(int value); // Write pulse width in microseconds
Expand Down