Fix and improve pulse width range #24
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
R/C servos have a standard pulse width range of 1000 to 2000µs<sup name="a1">[1](#f1)</sup>, with the zero point between the two at 1500µs. Currently, Arduino's Servo library sets: - [`#define MIN_PULSE_WIDTH 544`](https://github.com/arduino-libraries/Servo/blob/4970d615a13f4c0d08026ee361cc8a01974924a2/src/Servo.h#L80) - [`#define MAX_PULSE_WIDTH 2400`](https://github.com/arduino-libraries/Servo/blob/4970d615a13f4c0d08026ee361cc8a01974924a2/src/Servo.h#L81) - [`#define DEFAULT_PULSE_WIDTH 1500`](https://github.com/arduino-libraries/Servo/blob/4970d615a13f4c0d08026ee361cc8a01974924a2/src/Servo.h#L82) This causes a lot of confusion<sup name="a2">[2](#f2)</sup>, especially since [the docs say `write(90)` should correspond to the mid-point] (https://www.arduino.cc/en/Reference/ServoWrite); in actuality, it results in a call to `writeMicroseconds(1472)`<sup name="a3">[3](#f3)</sup>. This change adjusts the defaults to align with R/C standards. Specifically, - `write(0)` now corresponds to the standard min pulse width of 1000µs. - `write(90)` now corresponds to the standard zero point pulse width, and aligns with the library's `DEFAULT_PULSE_WIDTH` variable. - `write(180)` now corresponds to the standard max pulse width of 2000µs. Tested on an Arduino Uno with a [Tower Pro Micro Servo SG90](http://www.ee.ic.ac.uk/pcheung/teaching/DE1_EE/stores/sg90_datasheet.pdf), and a [Parallax Feedback 360° High-Speed Servo](https://parallax.com/sites/default/files/downloads/900-00360-Feedback-360-HS-Servo-v1.2.pdf). --- <a name="f1" href="#a1">1</a>: For example, http://www.ee.ic.ac.uk/pcheung/teaching/DE1_EE/stores/sg90_datasheet.pdf <a name="f2" href="#a2">2</a>: For instance: - julianduque/beaglebone-io#54 - arduino-libraries#3 - https://toolguyd.com/oscilloscope-arduino-servo-pwm-signal-mistakes/ - https://makezine.com/2014/04/23/arduinos-servo-library-angles-microseconds-and-optional-command-parameters/ I also see a _lot_ of posts on https://forum.arduino.cc about this. <a name="f3" href="#a3">3</a>: There is actually no way to set a standard servo to the zero-point using `write(angle)`; the closest you can get is `write(92)`, for a pulse of 1504µs.
Different servo models can accept a wide range of pulse widths. Even different servos of the same model might vary a bit. Currently, the Arduino Servo library has a severely restricted hard limit on the pulse widths that can be sent to servos. Specifically: - Minimum pulse width must be between [32, 1052]. - Maximum pulse width must be between [1888, 2908]. Many popular servos have min/max pulse widths that fall in that unavailable range between (1052, 1888). For instance, the [Parallax Feedback 360° High-Speed Servo](https://parallax.com/sites/default/files/downloads/900-00360-Feedback-360-HS-Servo-v1.2.pdf) operates between [1280, 1720]. Before this commit, each instance of `Servo` stored their `min` and `max` values as `int8_t`. Since that only leaves room for values in the range [-128, 127], it can't store meaningful servo pulse widths, which are typically in the ~[1000, 2000]µs range. To compensate, `min` and `max` store the distance from the default values, divided by 4… There are two problems with this: - The first, mentioned above, is that you can never stray more than 512µs from `MIN_PULSE_WIDTH` and `MAX_PULSE_WIDTH`. - The second is that unexpected and unnecessary rounding occurs. Simply storing `min` and `max` as `uint16_t` and using the values directly solves this problem, and reduces the complexity involved in working around it. This commit makes the library faster, and allows it to work with a wider range of servos. It also fixes some subtle bugs where the minimum value was hardcoded to `MIN_PULSE_WIDTH`. Tested on an Arduino Uno with a [Tower Pro Micro Servo SG90](http://www.ee.ic.ac.uk/pcheung/teaching/DE1_EE/stores/sg90_datasheet.pdf), and a [Parallax Feedback 360° High-Speed Servo](https://parallax.com/sites/default/files/downloads/900-00360-Feedback-360-HS-Servo-v1.2.pdf).
While working on 36f25f4 and 931445b, I encountered some typos, trailing whitespace, etc. I've moved the fixes into its own commit to reduce noise in the functional diffs. The only logic change here is removing some boundary checking redundancy. I removed a redundant bounds check from `write()`, since the value is `constrain`ed in `writeMicroseconds()` anyway. In `writeMicroseconds()`, I changed: ```.cpp if (value < this->min) // ensure pulse width is valid value = this->min; else if (value > this->max) value = this->max; ``` to: ```.cpp value = constrain(value, this->min, this->max); // ensure pulse width is valid ``` They are functionally equivalent.
|
@pnndra tagging you since you were active on this repository recently. I can update this to fix the conflicts, but I wonder if you have any initial thoughts are before I continue. |
|
@facchinm any chance you can take a look at this? |
This was referenced Sep 23, 2020
|
|
|
This seems like a reasonable and desired change to have. |
|
Thanks @uhertlein! @facchinm I’m still willing to fix the merge conflicts if you’re interested in the change. |
|
Does this change result in a change to public API, where existing projects with specific numbers to myServo.write() will have different behavior? |
It does, yes. Two options:
Also, Paul, I want to take this opportunity to say thanks for all you’ve done and continue to do for the community! Between Teensy, your shared libraries, and your inspiring and well-documented projects, you’ve made a huge impact. Thanks for all your hard work, and for sharing it so generously. |
per1234
added
type: enhancement
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This collects two functional commits (931445b, 36f25f4) which aim to simplify and improve behavior around the minimum/maximum pulse widths sent to the servo. Most notably, this brings Arduino's servo library in line with standard R/C pulse width expectations. It also allows for a wider range of servo motors to be used, by increasing the configurable pulse width range. Finally, it removes a few bugs and
todos, bringing it in line with how the docs say this library should work.Each commit has a detailed message about the changes. I can rebase these before merging if you'd prefer, but I thought they'd be easier to review broken up like this.
Cheers,