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.

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

Add Servo #248

Closed
bennuttall opened this Issue Apr 4, 2016 · 97 comments

Comments

Projects
None yet
7 participants
@bennuttall
Member

bennuttall commented Apr 4, 2016

We should add a Servo class, or more likely, a series of Servo classes (e.g. Servo180, Servo360...). I'm not sure what the variety of different servos is like, but perhaps there needs to be much configuration ability on init.

The way servos work is with PWM, setting a specific duty cycle to turn the servo to a specific angle. The one I have, a Tower Pro SG90, moves proportionately between 0 degrees and about 170 degrees when the value is set between 0.033 and 0.253 at a frequency of 100, but any value outside that does nothing.

I started playing with one using PWMOutputDevice and got it turning to certain angles by sending various signals via the value property.

When thinking about implementing a class for this, I realised I probably didn't want to extend PWMOutputDevice as is, because I didn't need on(), off(), etc. so I guess we should introduce a DigitalPWMOutputDevice class for this, and all existing classes which currently extend PWMOutputDevice would switch to DigitalPWMOutputDevice, and Servo would extend the new PWMOutputDevice. Perhaps "digital" is the wrong word in this case...

I'm not certain what the servo API should look like, but I guess ideally, you could set it by angle, and maybe setting value between 0 and 1 would move proportionally between the minimum and maximum positions.

@bennuttall

This comment has been minimized.

Show comment
Hide comment
@bennuttall

bennuttall Apr 4, 2016

Member

A possible API:

>>> servo = Servo(17, angle=180)
>>> servo.angle = 0
>>> servo.value
0.0
>>> servo.angle = 180
>>> servo.value
1.0
>>> servo.value = 0.5
>>> servo.angle
90.0

So that value and angle are coupled between min and max angles according to the rotation potential of the servo (in this case 180 degrees).

However, this means that a servo's value does not correspond directly to its duty cycle, like other PWM objects. This would be mapped between the duty cycle of the 0 degree position and that of the maximum angle (e.g. 180 degree position).

We could provide a generic Servo class which takes parameters for max_angle, min_duty_cycle, max_duty_cycle, or something similar. Alternatively we could provide Servo180, Servo360 and so on. I'm not sure how much servos tend to vary. If they're all over the place we'll have to provide lots of configuration. If there's a decent set of standard ones then we should be able to just use those.

Any thoughts @waveform80?

Member

bennuttall commented Apr 4, 2016

A possible API:

>>> servo = Servo(17, angle=180)
>>> servo.angle = 0
>>> servo.value
0.0
>>> servo.angle = 180
>>> servo.value
1.0
>>> servo.value = 0.5
>>> servo.angle
90.0

So that value and angle are coupled between min and max angles according to the rotation potential of the servo (in this case 180 degrees).

However, this means that a servo's value does not correspond directly to its duty cycle, like other PWM objects. This would be mapped between the duty cycle of the 0 degree position and that of the maximum angle (e.g. 180 degree position).

We could provide a generic Servo class which takes parameters for max_angle, min_duty_cycle, max_duty_cycle, or something similar. Alternatively we could provide Servo180, Servo360 and so on. I'm not sure how much servos tend to vary. If they're all over the place we'll have to provide lots of configuration. If there's a decent set of standard ones then we should be able to just use those.

Any thoughts @waveform80?

@bennuttall bennuttall added this to the v1.3 milestone Apr 5, 2016

@WayneKeenan

This comment has been minimized.

Show comment
Hide comment
@WayneKeenan

WayneKeenan Apr 5, 2016

Please keep/expose on() and off() ( synonymous to the Arduino servo attach() and detach() functions)

The attach/detach allows saving on power when the servos don't need to move, and also help keep some robots still when not in use. As many servos oscillate slightly at any set angle and this can become more pronounced on something like a robot arm (e.g. MeArm), performing an off (or detach) after a few seconds, if there is no required movement, minimises this.

A min_angle property, and bounding to and above it, would be useful for when the servos are installed into a device that physically restrict their movement, e.g. the joints of a robot arm.

Having a setPosition(float) that takes a range 0.0 ..1.0 and maps it to min_angle .. max_angle could be handy.

Being able to set a servo.frequency for the underlying PWM frequency may be needed by some servos.

Also, continuous rotation servos use the 'angle' as a speed, so perhaps consider this type of Servo in the new class?

WayneKeenan commented Apr 5, 2016

Please keep/expose on() and off() ( synonymous to the Arduino servo attach() and detach() functions)

The attach/detach allows saving on power when the servos don't need to move, and also help keep some robots still when not in use. As many servos oscillate slightly at any set angle and this can become more pronounced on something like a robot arm (e.g. MeArm), performing an off (or detach) after a few seconds, if there is no required movement, minimises this.

A min_angle property, and bounding to and above it, would be useful for when the servos are installed into a device that physically restrict their movement, e.g. the joints of a robot arm.

Having a setPosition(float) that takes a range 0.0 ..1.0 and maps it to min_angle .. max_angle could be handy.

Being able to set a servo.frequency for the underlying PWM frequency may be needed by some servos.

Also, continuous rotation servos use the 'angle' as a speed, so perhaps consider this type of Servo in the new class?

@sebleedelisle

This comment has been minimized.

Show comment
Hide comment
@sebleedelisle

sebleedelisle Apr 5, 2016

This is great! As Wayne also mentions, I would take a look at the Arduino Servo object for inspiration : https://www.arduino.cc/en/Reference/Servo

It feels a bit bulky to have different objects for a 180 and 360, I would default to 180 as this seems by far the most common. I like the max_angle, min_duty_cycle, max_duty_cycle options as long as most of the time the defaults were the most common. Again, see the default set up for the Arduino Servo object.

Nice to have : a moveTo or similar that interpolates from the current position to the target position with a specified duration, and ease in and ease out (either a linear ramp up and down or an exponential ease-in/out if you're feeling fancy). Otherwise you're relying on the in-built circuitry in a server to manage that, and it's not often very pleasant. (I have some experience in this area so can likely help if required).

sebleedelisle commented Apr 5, 2016

This is great! As Wayne also mentions, I would take a look at the Arduino Servo object for inspiration : https://www.arduino.cc/en/Reference/Servo

It feels a bit bulky to have different objects for a 180 and 360, I would default to 180 as this seems by far the most common. I like the max_angle, min_duty_cycle, max_duty_cycle options as long as most of the time the defaults were the most common. Again, see the default set up for the Arduino Servo object.

Nice to have : a moveTo or similar that interpolates from the current position to the target position with a specified duration, and ease in and ease out (either a linear ramp up and down or an exponential ease-in/out if you're feeling fancy). Otherwise you're relying on the in-built circuitry in a server to manage that, and it's not often very pleasant. (I have some experience in this area so can likely help if required).

@bennuttall

This comment has been minimized.

Show comment
Hide comment
@bennuttall

bennuttall Apr 5, 2016

Member

on/off or equivalent makes sense actually, to get it to disengage.

Member

bennuttall commented Apr 5, 2016

on/off or equivalent makes sense actually, to get it to disengage.

@lurch

This comment has been minimized.

Show comment
Hide comment
@lurch

lurch Apr 5, 2016

Contributor

Is using PWM necessarily the best idea for this? It's something I've not (yet) experimented with myself, but I recall reading in the past that trying to use PWM (especially from Python - see http://raspi.tv/2013/how-to-use-soft-pwm-in-rpi-gpio-pt-2-led-dimming-and-motor-speed-control) is too jittery on a RaspberryPi for accurate servo control (although I guess this may be less of an issue on the Pi2 and Pi3 than it was on the Pi1), and that's why things like ServoBlaster was developed, to allow much more accurate servo control?

And having Servos as an "object in their own right" and not inherited from PWMOutputDevice also allays Ben's class-inheritance (and appropriate-method-overloading) concerns.

Contributor

lurch commented Apr 5, 2016

Is using PWM necessarily the best idea for this? It's something I've not (yet) experimented with myself, but I recall reading in the past that trying to use PWM (especially from Python - see http://raspi.tv/2013/how-to-use-soft-pwm-in-rpi-gpio-pt-2-led-dimming-and-motor-speed-control) is too jittery on a RaspberryPi for accurate servo control (although I guess this may be less of an issue on the Pi2 and Pi3 than it was on the Pi1), and that's why things like ServoBlaster was developed, to allow much more accurate servo control?

And having Servos as an "object in their own right" and not inherited from PWMOutputDevice also allays Ben's class-inheritance (and appropriate-method-overloading) concerns.

@carlosperate

This comment has been minimized.

Show comment
Hide comment
@carlosperate

carlosperate Apr 5, 2016

I agree as well that maybe having different classes for 180 or 360 is a bit too much, sounds better to have one with sane defaults that can be edited in the constructor. The only thing I would maybe separate would be servos from "continuous servos", have you guys given any though to that?

carlosperate commented Apr 5, 2016

I agree as well that maybe having different classes for 180 or 360 is a bit too much, sounds better to have one with sane defaults that can be edited in the constructor. The only thing I would maybe separate would be servos from "continuous servos", have you guys given any though to that?

@WayneKeenan

This comment has been minimized.

Show comment
Hide comment
@WayneKeenan

WayneKeenan Apr 5, 2016

@lurch I understand that by now* it might be possible to load Videocore's VPU with a small chunk of user defined code to drive PWM on the GPIO pins.

*at a CamJam about/over a year ago I queried using VideoCore todo PWM in a conversation with Eben and he said that the VPU can access the GPIO. He also mentioned the functions the VPU were currently (at the time) performing were moving off the VPU, so it might become available for such 'user' programs. (I think there's a documented interface and a VPU compiler now too (?) )

This is perhaps a far more intricate system level project todo and better kept separate from the gpiozero repository... :)

WayneKeenan commented Apr 5, 2016

@lurch I understand that by now* it might be possible to load Videocore's VPU with a small chunk of user defined code to drive PWM on the GPIO pins.

*at a CamJam about/over a year ago I queried using VideoCore todo PWM in a conversation with Eben and he said that the VPU can access the GPIO. He also mentioned the functions the VPU were currently (at the time) performing were moving off the VPU, so it might become available for such 'user' programs. (I think there's a documented interface and a VPU compiler now too (?) )

This is perhaps a far more intricate system level project todo and better kept separate from the gpiozero repository... :)

@bennuttall

This comment has been minimized.

Show comment
Hide comment
@bennuttall

bennuttall Apr 5, 2016

Member

sounds better to have one with sane defaults that can be edited in the constructor

That's pretty much the philosophy I aim for. As long as there are reasonable standards we can adhere to, that makes sense.

The only thing I would maybe separate would be servos from "continuous servos", have you guys given any though to that?

That makes sense. Will bear in mind.

Member

bennuttall commented Apr 5, 2016

sounds better to have one with sane defaults that can be edited in the constructor

That's pretty much the philosophy I aim for. As long as there are reasonable standards we can adhere to, that makes sense.

The only thing I would maybe separate would be servos from "continuous servos", have you guys given any though to that?

That makes sense. Will bear in mind.

@carlosperate

This comment has been minimized.

Show comment
Hide comment
@carlosperate

carlosperate Apr 5, 2016

I haven't use the PWM in the Pis that much to be honest, but are we saying that the hardware peripheral is not reliable enough to control a servo? Or is the general conversation about alternative PWMs as a software implementation on other GPIOs?

carlosperate commented Apr 5, 2016

I haven't use the PWM in the Pis that much to be honest, but are we saying that the hardware peripheral is not reliable enough to control a servo? Or is the general conversation about alternative PWMs as a software implementation on other GPIOs?

@WayneKeenan

This comment has been minimized.

Show comment
Hide comment
@WayneKeenan

WayneKeenan Apr 5, 2016

@carlosperate Just options... The Pi's PWM for servo control is 'good enough' to get demos and fun things up and running, it's probably 'not good enough' for controlling a quad-copter though, but what people do with the Pi is always enlightening.

This is a demo (not mine) of using a Python wrapper around the ServoBlaster (software + DMA driven) PWM, which would seem ok for at least 4 PWM's, ServoBlaster says it supports up to at least 8. http://fortoffee.org.uk/2015/02/me-arm-and-me-raspberry-pi/

Although ServoBlaster introduces another dependancy. If I get a chance, I might plug in my MeArm and try controlling it using RPi.GPIO PWM support.

WayneKeenan commented Apr 5, 2016

@carlosperate Just options... The Pi's PWM for servo control is 'good enough' to get demos and fun things up and running, it's probably 'not good enough' for controlling a quad-copter though, but what people do with the Pi is always enlightening.

This is a demo (not mine) of using a Python wrapper around the ServoBlaster (software + DMA driven) PWM, which would seem ok for at least 4 PWM's, ServoBlaster says it supports up to at least 8. http://fortoffee.org.uk/2015/02/me-arm-and-me-raspberry-pi/

Although ServoBlaster introduces another dependancy. If I get a chance, I might plug in my MeArm and try controlling it using RPi.GPIO PWM support.

@sebleedelisle

This comment has been minimized.

Show comment
Hide comment
@sebleedelisle

sebleedelisle Apr 5, 2016

Re continuous servos, I think most people these days equate a "servo" to a "hobby servo" or "RC servo" ie, the cheap plastic things with built-in controllers that use the pulse system for positional information.

As someone who uses industrial servos with separate controllers it can get confusing but I'm not really sure if we can fix that at this stage!

Of course the other type of continuous servo is a hobby servo that has had the positional feedback system pulled out (usually a pot), I'm not really sure how to cater for that but I think the basic servo API would suffice.

As for using PWM, it is basically PWM but with short pulse between 1 and 2ms followed by a wide gap. If the pulse is 1ms then the motor goes to 0º, if it's 2ms then it goes to 180º. If the servo doesn't get regular pulses or the timing isn't accurate enough then you get the horrible jitter effect. The pulse frequency should be once every 20ms or 50hz which isn't actually that fast.

So it's a slightly different use case for PWM with slightly different requirements. But I'm thinking that the PWM libs I've seen should be able to handle it as long as the timing is accurate enough. (I always thought it was a weird protocol!)

cheers!

Seb

PS A good diagram on the wiki page https://en.wikipedia.org/wiki/Servo_control#/media/File:Sinais_controle_servomotor.JPG

sebleedelisle commented Apr 5, 2016

Re continuous servos, I think most people these days equate a "servo" to a "hobby servo" or "RC servo" ie, the cheap plastic things with built-in controllers that use the pulse system for positional information.

As someone who uses industrial servos with separate controllers it can get confusing but I'm not really sure if we can fix that at this stage!

Of course the other type of continuous servo is a hobby servo that has had the positional feedback system pulled out (usually a pot), I'm not really sure how to cater for that but I think the basic servo API would suffice.

As for using PWM, it is basically PWM but with short pulse between 1 and 2ms followed by a wide gap. If the pulse is 1ms then the motor goes to 0º, if it's 2ms then it goes to 180º. If the servo doesn't get regular pulses or the timing isn't accurate enough then you get the horrible jitter effect. The pulse frequency should be once every 20ms or 50hz which isn't actually that fast.

So it's a slightly different use case for PWM with slightly different requirements. But I'm thinking that the PWM libs I've seen should be able to handle it as long as the timing is accurate enough. (I always thought it was a weird protocol!)

cheers!

Seb

PS A good diagram on the wiki page https://en.wikipedia.org/wiki/Servo_control#/media/File:Sinais_controle_servomotor.JPG

@bennuttall

This comment has been minimized.

Show comment
Hide comment
@bennuttall

bennuttall Apr 5, 2016

Member

Although ServoBlaster introduces another dependancy. If I get a chance, I might plug in my MeArm and try controlling it using RPi.GPIO PWM support.

Or GPIO Zero PWM support? :P

Member

bennuttall commented Apr 5, 2016

Although ServoBlaster introduces another dependancy. If I get a chance, I might plug in my MeArm and try controlling it using RPi.GPIO PWM support.

Or GPIO Zero PWM support? :P

@WayneKeenan

This comment has been minimized.

Show comment
Hide comment
@WayneKeenan

WayneKeenan Apr 5, 2016

Or GPIO Zero PWM support? :P

same difference isn't it? :)

WayneKeenan commented Apr 5, 2016

Or GPIO Zero PWM support? :P

same difference isn't it? :)

@carlosperate

This comment has been minimized.

Show comment
Hide comment
@carlosperate

carlosperate Apr 5, 2016

As far as continuous rotation "hobby" servos go, I've seen them being commercially available for robotic kits and similar things. I've perhaps wrongly inferred from @sebleedelisle comment that most continuous servos are manually modified, but I just wanted to clarify that it seems (at least to me) like a potential use case for this library as well.

The signal control basics are very much the same as with a normal servo, but the end result can be considered different enough where the API design would most likely change. So maybe being able to derive a “servo” and “continuous servo” class from the same parent could be a good aim to have in order to clearly separate the signal control from the end device implementation, as it has been done with other components in this package.

carlosperate commented Apr 5, 2016

As far as continuous rotation "hobby" servos go, I've seen them being commercially available for robotic kits and similar things. I've perhaps wrongly inferred from @sebleedelisle comment that most continuous servos are manually modified, but I just wanted to clarify that it seems (at least to me) like a potential use case for this library as well.

The signal control basics are very much the same as with a normal servo, but the end result can be considered different enough where the API design would most likely change. So maybe being able to derive a “servo” and “continuous servo” class from the same parent could be a good aim to have in order to clearly separate the signal control from the end device implementation, as it has been done with other components in this package.

@waveform80

This comment has been minimized.

Show comment
Hide comment
@waveform80

waveform80 Apr 5, 2016

Member

Cripes this thread has grown.. Okay, I'll throw some thoughts into the wind:

  1. Different classes for different angles: no. That should be a constructor arg just like distances and thresholds are for the DistanceSensor.
  2. I think PWM's probably the way to go (the only way?), but @lurch has a point that software PWM is (as I understand it) too jittery for servos on the Pi. That said, the Pi does support hardware PWM and not just on one pin! I know the "real" PWM is only on one pin, or two on later models, but you can do extremely accurate PWM (good enough for servos) on any pin via the DMA controller - both RPIO and pigpiod implement this. The major downside is that it requires root (because the DMA controller isn't exposed to non-root users, for good reasons) although that's a relatively minor downside in the case of pigpiod where only the daemon needs to run as root and the script can simply request DMA'd PWM via a socket. RPi.GPIO's PWM is entirely software (so probably not good enough for Servo control on slower / loaded Pi's but it doesn't require root).
  3. The angle limits are an interesting idea - I'll bear that in mind.

Sorry, I've just skimmed all the messages here - head buried in unit-testing at the moment...

Member

waveform80 commented Apr 5, 2016

Cripes this thread has grown.. Okay, I'll throw some thoughts into the wind:

  1. Different classes for different angles: no. That should be a constructor arg just like distances and thresholds are for the DistanceSensor.
  2. I think PWM's probably the way to go (the only way?), but @lurch has a point that software PWM is (as I understand it) too jittery for servos on the Pi. That said, the Pi does support hardware PWM and not just on one pin! I know the "real" PWM is only on one pin, or two on later models, but you can do extremely accurate PWM (good enough for servos) on any pin via the DMA controller - both RPIO and pigpiod implement this. The major downside is that it requires root (because the DMA controller isn't exposed to non-root users, for good reasons) although that's a relatively minor downside in the case of pigpiod where only the daemon needs to run as root and the script can simply request DMA'd PWM via a socket. RPi.GPIO's PWM is entirely software (so probably not good enough for Servo control on slower / loaded Pi's but it doesn't require root).
  3. The angle limits are an interesting idea - I'll bear that in mind.

Sorry, I've just skimmed all the messages here - head buried in unit-testing at the moment...

@waveform80

This comment has been minimized.

Show comment
Hide comment
@waveform80

waveform80 Apr 5, 2016

Member

Oh, and @WayneKeenan - not exactly the same difference - RPi.GPIO is indeed the default pin implementation but since 1.1 you can use RPIO or pigpiod as the backend instead for "proper" hardware PWM (although the setup's more involved - haven't written any serious docs about that yet largely because pigpiod isn't properly packaged for Raspbian (yet)). Oh, and you can even use a pure Python implementation but that doesn't support PWM so it's a bit of a moot point for this thread.

Member

waveform80 commented Apr 5, 2016

Oh, and @WayneKeenan - not exactly the same difference - RPi.GPIO is indeed the default pin implementation but since 1.1 you can use RPIO or pigpiod as the backend instead for "proper" hardware PWM (although the setup's more involved - haven't written any serious docs about that yet largely because pigpiod isn't properly packaged for Raspbian (yet)). Oh, and you can even use a pure Python implementation but that doesn't support PWM so it's a bit of a moot point for this thread.

@lurch

This comment has been minimized.

Show comment
Hide comment
@lurch

lurch Apr 5, 2016

Contributor

So, if you try to create a Servo object using a non-hardware-DMA-able pin (i.e. a NativePin or RPiGPIOPin), should Servo's __init__ method raise an Exception?

Contributor

lurch commented Apr 5, 2016

So, if you try to create a Servo object using a non-hardware-DMA-able pin (i.e. a NativePin or RPiGPIOPin), should Servo's __init__ method raise an Exception?

@bennuttall

This comment has been minimized.

Show comment
Hide comment
@bennuttall

bennuttall Apr 5, 2016

Member

Different classes for different angles: no.

Ok sure. Updated my example above.

I think there needs to be a base PWM class, which this and PWMOutputDevice should extend. Any thoughts?

I've got an idea how the Servo class might work, so I'll try to throw something together. Could do with some more servos to test though.

Member

bennuttall commented Apr 5, 2016

Different classes for different angles: no.

Ok sure. Updated my example above.

I think there needs to be a base PWM class, which this and PWMOutputDevice should extend. Any thoughts?

I've got an idea how the Servo class might work, so I'll try to throw something together. Could do with some more servos to test though.

@sebleedelisle

This comment has been minimized.

Show comment
Hide comment
@sebleedelisle

sebleedelisle Apr 5, 2016

I guess a Servo could be the same as an ordinary PWM output as long as the
frequency was 50hz and the pulse width could be accurately adjusted between
5% and 10% (which would equate to the 1-2ms pulse mentioned earlier). If
I've done my maths right! :)

On 5 April 2016 at 17:32, Ben Nuttall notifications@github.com wrote:

Different classes for different angles: no.

Ok sure. Updated my example above.

I think there needs to be a base PWM class, which this and PWMOutputDevice
should extend. Any thoughts?

I've got an idea how the Servo class might work, so I'll try to throw
something together. Could do with some more servos to test though.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#248 (comment)

sebleedelisle commented Apr 5, 2016

I guess a Servo could be the same as an ordinary PWM output as long as the
frequency was 50hz and the pulse width could be accurately adjusted between
5% and 10% (which would equate to the 1-2ms pulse mentioned earlier). If
I've done my maths right! :)

On 5 April 2016 at 17:32, Ben Nuttall notifications@github.com wrote:

Different classes for different angles: no.

Ok sure. Updated my example above.

I think there needs to be a base PWM class, which this and PWMOutputDevice
should extend. Any thoughts?

I've got an idea how the Servo class might work, so I'll try to throw
something together. Could do with some more servos to test though.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#248 (comment)

@waveform80

This comment has been minimized.

Show comment
Hide comment
@waveform80

waveform80 Apr 5, 2016

Member

You can easily do 50Hz PWM with DMA on the Pi (in fact, it's damn near millisecond accurate looking at RPIO's oscilloscope pics) but with software PWM (i.e. straight-forward bit-banging from a userspace process like that implemented by RPi.GPIO) you can't guarantee anything. While it might be fine most of the time, you can't be certain that something isn't going steal all your process' clock cycles for the next second or two (even with a fully preemptable kernel - can't remember exactly what Raspbian's defaults are in this area though).

In other words, if you're okay with your PWM occasionally going out to lunch and letting your servo go wild, then software PWM might be acceptable. But if you need to ensure your servo does exactly what you request, then you have to use some form of hardware assistance (whether that's a proper PWM implementation, which the Pi does have but only on a very limited set of pins, or programming the DMA controller to do bit-banging on the GPIO registers for you).

Member

waveform80 commented Apr 5, 2016

You can easily do 50Hz PWM with DMA on the Pi (in fact, it's damn near millisecond accurate looking at RPIO's oscilloscope pics) but with software PWM (i.e. straight-forward bit-banging from a userspace process like that implemented by RPi.GPIO) you can't guarantee anything. While it might be fine most of the time, you can't be certain that something isn't going steal all your process' clock cycles for the next second or two (even with a fully preemptable kernel - can't remember exactly what Raspbian's defaults are in this area though).

In other words, if you're okay with your PWM occasionally going out to lunch and letting your servo go wild, then software PWM might be acceptable. But if you need to ensure your servo does exactly what you request, then you have to use some form of hardware assistance (whether that's a proper PWM implementation, which the Pi does have but only on a very limited set of pins, or programming the DMA controller to do bit-banging on the GPIO registers for you).

@bennuttall

This comment has been minimized.

Show comment
Hide comment
@bennuttall

bennuttall Apr 5, 2016

Member

It works well enough to send a servo to the right position. However, I noticed when setting mine to 90 degrees, it would twitch. This could be resolved by disengaging it once it's moved (I'm not sure how we'd time this if we did it automatically, but that could be down to the user).

Member

bennuttall commented Apr 5, 2016

It works well enough to send a servo to the right position. However, I noticed when setting mine to 90 degrees, it would twitch. This could be resolved by disengaging it once it's moved (I'm not sure how we'd time this if we did it automatically, but that could be down to the user).

@sebleedelisle

This comment has been minimized.

Show comment
Hide comment
@sebleedelisle

sebleedelisle Apr 5, 2016

Yeah the twitching is almost definitely to do with poor timing on the
pulse, which happens often with software PWM. Or else it could be just a
really bad servo :)

On 5 April 2016 at 19:39, Ben Nuttall notifications@github.com wrote:

It works well enough to send a servo to the right position. However, I
noticed when setting mine to 90 degrees, it would twitch. This could be
resolved by disengaging it once it's moved (I'm not sure how we'd time this
if we did it automatically, but that could be down to the user).


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#248 (comment)

sebleedelisle commented Apr 5, 2016

Yeah the twitching is almost definitely to do with poor timing on the
pulse, which happens often with software PWM. Or else it could be just a
really bad servo :)

On 5 April 2016 at 19:39, Ben Nuttall notifications@github.com wrote:

It works well enough to send a servo to the right position. However, I
noticed when setting mine to 90 degrees, it would twitch. This could be
resolved by disengaging it once it's moved (I'm not sure how we'd time this
if we did it automatically, but that could be down to the user).


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#248 (comment)

@bennuttall

This comment has been minimized.

Show comment
Hide comment
@bennuttall

bennuttall Apr 5, 2016

Member

Likely both :)

Member

bennuttall commented Apr 5, 2016

Likely both :)

@sebleedelisle

This comment has been minimized.

Show comment
Hide comment
@sebleedelisle

sebleedelisle Apr 5, 2016

Do you have decent PWM on gpiozero? Or is it all done in python?

On 5 April 2016 at 19:45, Ben Nuttall notifications@github.com wrote:

Likely both :)


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#248 (comment)

sebleedelisle commented Apr 5, 2016

Do you have decent PWM on gpiozero? Or is it all done in python?

On 5 April 2016 at 19:45, Ben Nuttall notifications@github.com wrote:

Likely both :)


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#248 (comment)

@waveform80

This comment has been minimized.

Show comment
Hide comment
@waveform80

waveform80 Apr 5, 2016

Member

@bennuttall now try it on a slower Pi (like a model 1) and fire up some simultaneous threads - it'll only get worse :)

@sebleedelisle here's a rough overview of the way things currently stand:

gpiozero (which is pure python) relies on "pin libraries" to handle controlling GPIO pins (for everything from simple high/low settings up to PWM). Currently four "pin libraries" are supported and gpiozero tries to import them in the following preference order:

  1. RPi.GPIO - this is the default library largely because it's extremely popular in the community, it's properly packaged on Raspbian, it's well maintained and is kept up to date with new model releases. It's written entirely in C and supports most operations by bit-banging on an mmap of the GPIO registers (edge detection is handled by epoll on sysfs files). It supports rootless control of the GPIO pins by mapping /dev/gpiomem in preference to /dev/mem. However, its PWM implementation is pure software: threads bit-banging on the GPIO registers with nanosleep for timing (in other words it's really not good enough for servo control).
  2. RPIO - this is the initial fallback library largely because it's a drop-in replacement for RPi.GPIO (mostly) so it was the second one developed. It's packaged on PyPI (pip), but not on Raspbian and unfortunately at the moment it's fallen behind a bit in as much as it doesn't support the Pi 2 or 3, just the Pi 1 (and zero?). It's also root only (doesn't know about /dev/gpiomem). However, its PWM implementation uses the DMA controller to bit-bang on the GPIO registers so it's very accurate (see the oscilloscope images linked to earlier) and certainly good enough for servos.
  3. pigpiod - this is the next fallback library. I'd love it to be the primary library (for reasons we'll get to), but there's a few obstacles to overcome first. Firstly it's not packaged at all (no PyPI, no .deb) so installation is below par. Its architecture is unusual: it uses a root daemon (written entirely in C) to control the GPIO pins and the Python library just talks to the daemon over a socket. On the downside this means you're subject to network latency, but over localhost this is minimal to the point it really doesn't matter (in fact, even remotely over Ethernet it's still fast enough to accurately do things like time a capacitor charging with an LDR, i.e. LightSensor). On the upside, this gives all scripts reasonably safe, rootless control of everything the daemon implements. And the daemon implements everything and the kitchen sink: straight GPIO input/output, high/low, PWM via DMA, SPI, I2C, you name it. It does have a few flaws: the daemon always forks (no foreground mode which makes testing a pain), I've managed to get it into unusual states which required a restart on a couple of occasions, and the Python module looks like copy'n'pasted C (urgh) but forget all that: it's a great architecture and it's something I'd love to see given some attention.
  4. native - this is the final "everything else has failed" fallback library. It's a brutally simple pure Python library which I whipped up in a couple of days after reading the source of all the aforementioned libraries. It's similar in architecture to RPi.GPIO and RPIO (bit-banging on an mmap), it's /dev/gpiomem aware (so it supports rootless control), but it's got some pretty big drawbacks at the moment. The primary one for this thread is that there's no PWM support (pure Python's way too slow for PWM via bit-banging and I'm not terribly interested in a root-only DMA implementation). The other is that edge detection is via the GPIO registers which causes a kernel module distress (although it works nicely otherwise). Still, it works even when nothing else is installed and even works happily under things like pypy so it's a good enough backstop for now.

See the pins documentation for a rough overview of all the above, along with detail of the pin interface gpiozero expects (and source links to all the above libraries), and instructions on overriding the default pin implementation.

So, that's how things stand right now. For the near future, RPi.GPIO is almost certain to remain the default library (especially as it's installed by default on Raspbian). For the future, I'd love to see pigpiod given some attention as I think it's the most promising architecture (the fact it exposes nearly all GPIO functionality over sockets is also a potential boon to other languages) but that's largely dependent on how much free time people have (I really ought to spend some more time on picamera which is long overdue a release, then there's all the picademy worksheets that need some work, etc. etc.)

Member

waveform80 commented Apr 5, 2016

@bennuttall now try it on a slower Pi (like a model 1) and fire up some simultaneous threads - it'll only get worse :)

@sebleedelisle here's a rough overview of the way things currently stand:

gpiozero (which is pure python) relies on "pin libraries" to handle controlling GPIO pins (for everything from simple high/low settings up to PWM). Currently four "pin libraries" are supported and gpiozero tries to import them in the following preference order:

  1. RPi.GPIO - this is the default library largely because it's extremely popular in the community, it's properly packaged on Raspbian, it's well maintained and is kept up to date with new model releases. It's written entirely in C and supports most operations by bit-banging on an mmap of the GPIO registers (edge detection is handled by epoll on sysfs files). It supports rootless control of the GPIO pins by mapping /dev/gpiomem in preference to /dev/mem. However, its PWM implementation is pure software: threads bit-banging on the GPIO registers with nanosleep for timing (in other words it's really not good enough for servo control).
  2. RPIO - this is the initial fallback library largely because it's a drop-in replacement for RPi.GPIO (mostly) so it was the second one developed. It's packaged on PyPI (pip), but not on Raspbian and unfortunately at the moment it's fallen behind a bit in as much as it doesn't support the Pi 2 or 3, just the Pi 1 (and zero?). It's also root only (doesn't know about /dev/gpiomem). However, its PWM implementation uses the DMA controller to bit-bang on the GPIO registers so it's very accurate (see the oscilloscope images linked to earlier) and certainly good enough for servos.
  3. pigpiod - this is the next fallback library. I'd love it to be the primary library (for reasons we'll get to), but there's a few obstacles to overcome first. Firstly it's not packaged at all (no PyPI, no .deb) so installation is below par. Its architecture is unusual: it uses a root daemon (written entirely in C) to control the GPIO pins and the Python library just talks to the daemon over a socket. On the downside this means you're subject to network latency, but over localhost this is minimal to the point it really doesn't matter (in fact, even remotely over Ethernet it's still fast enough to accurately do things like time a capacitor charging with an LDR, i.e. LightSensor). On the upside, this gives all scripts reasonably safe, rootless control of everything the daemon implements. And the daemon implements everything and the kitchen sink: straight GPIO input/output, high/low, PWM via DMA, SPI, I2C, you name it. It does have a few flaws: the daemon always forks (no foreground mode which makes testing a pain), I've managed to get it into unusual states which required a restart on a couple of occasions, and the Python module looks like copy'n'pasted C (urgh) but forget all that: it's a great architecture and it's something I'd love to see given some attention.
  4. native - this is the final "everything else has failed" fallback library. It's a brutally simple pure Python library which I whipped up in a couple of days after reading the source of all the aforementioned libraries. It's similar in architecture to RPi.GPIO and RPIO (bit-banging on an mmap), it's /dev/gpiomem aware (so it supports rootless control), but it's got some pretty big drawbacks at the moment. The primary one for this thread is that there's no PWM support (pure Python's way too slow for PWM via bit-banging and I'm not terribly interested in a root-only DMA implementation). The other is that edge detection is via the GPIO registers which causes a kernel module distress (although it works nicely otherwise). Still, it works even when nothing else is installed and even works happily under things like pypy so it's a good enough backstop for now.

See the pins documentation for a rough overview of all the above, along with detail of the pin interface gpiozero expects (and source links to all the above libraries), and instructions on overriding the default pin implementation.

So, that's how things stand right now. For the near future, RPi.GPIO is almost certain to remain the default library (especially as it's installed by default on Raspbian). For the future, I'd love to see pigpiod given some attention as I think it's the most promising architecture (the fact it exposes nearly all GPIO functionality over sockets is also a potential boon to other languages) but that's largely dependent on how much free time people have (I really ought to spend some more time on picamera which is long overdue a release, then there's all the picademy worksheets that need some work, etc. etc.)

@sebleedelisle

This comment has been minimized.

Show comment
Hide comment
@sebleedelisle

sebleedelisle Apr 6, 2016

@waveform80 thanks for the info! I've used similar things in node, including the node wrapper for pigpio but very little experience with Python on the RPi so this was really helpful.

sebleedelisle commented Apr 6, 2016

@waveform80 thanks for the info! I've used similar things in node, including the node wrapper for pigpio but very little experience with Python on the RPi so this was really helpful.

@bennuttall

This comment has been minimized.

Show comment
Hide comment
@bennuttall

bennuttall Apr 6, 2016

Member

Ok here's some code that works but isn't perfect:

class Servo(PWMOutputDevice):
    def __init__(self, pin=None, active_high=True, initial_value=0, frequency=100, max_angle=180, minimum=0.033, maximum=0.254):
        super(Servo, self).__init__(pin, active_high, initial_value, frequency)
        self._max_angle = max_angle
        self._minimum = minimum
        self._maximum = maximum
        self._angle = None  # don't know what it is on init

    @property
    def angle(self):
        return self._angle

    @angle.setter
    def angle(self, value):
        if 0 > value > self._max_angle:
            raise OutputDeviceBadValue("angle must be between 0 and %s" % self._max_angle)
        self._angle = value
        self.value = ((self._maximum - self._minimum) * value / self._max_angle) + self._minimum

I want to create a new base class, rather than letting this extend PWMOutputDevice, and make off() disengage the servo. I don't think on() is necessary. I want value to contain a value 0 to 1 mapped to the angle potential, rather than the duty cycle.

Member

bennuttall commented Apr 6, 2016

Ok here's some code that works but isn't perfect:

class Servo(PWMOutputDevice):
    def __init__(self, pin=None, active_high=True, initial_value=0, frequency=100, max_angle=180, minimum=0.033, maximum=0.254):
        super(Servo, self).__init__(pin, active_high, initial_value, frequency)
        self._max_angle = max_angle
        self._minimum = minimum
        self._maximum = maximum
        self._angle = None  # don't know what it is on init

    @property
    def angle(self):
        return self._angle

    @angle.setter
    def angle(self, value):
        if 0 > value > self._max_angle:
            raise OutputDeviceBadValue("angle must be between 0 and %s" % self._max_angle)
        self._angle = value
        self.value = ((self._maximum - self._minimum) * value / self._max_angle) + self._minimum

I want to create a new base class, rather than letting this extend PWMOutputDevice, and make off() disengage the servo. I don't think on() is necessary. I want value to contain a value 0 to 1 mapped to the angle potential, rather than the duty cycle.

@waveform80

This comment has been minimized.

Show comment
Hide comment
@waveform80

waveform80 Apr 6, 2016

Member

Any particular reason this shouldn't extend PWMOutputDevice? You could simply make on a method equivalent to setting the maximum angle (which would be consistent with the fact value will be 1 which is the result of calling on in other PWM devices). If you really want to get rid of on, Python is unusual in that you can remove methods from sub-classes with a little magic, but it's unusual enough that I'd generally discourage it (simply because people won't expect it).

Member

waveform80 commented Apr 6, 2016

Any particular reason this shouldn't extend PWMOutputDevice? You could simply make on a method equivalent to setting the maximum angle (which would be consistent with the fact value will be 1 which is the result of calling on in other PWM devices). If you really want to get rid of on, Python is unusual in that you can remove methods from sub-classes with a little magic, but it's unusual enough that I'd generally discourage it (simply because people won't expect it).

@waveform80

This comment has been minimized.

Show comment
Hide comment
@waveform80

waveform80 Jun 15, 2016

Member

First impression I don't like this...

Do you think ms would be a better unit? Part of me thinks so, and I'm sure for people experienced with servos it probably would be. But I think for complete newbies (which is the major focus of the library), consistency with everything else might be easier (even if it means writing 2e-3, or 2/1000, or 0.002). Happy to be persuaded otherwise though!

What's the use case? I thought it made sense for stopping jitter, but we won't have that with pigpio.

When the servo is actively controlled you can't manually adjust the servo's position. There may be some minor use cases for manual adjustment but a more important point (particularly for robotics) is that the servo's constantly drawing power when actively controlled. When "free" it doesn't draw power but there's usually enough resistance in the servo to reasonably assume that a steering wheel on a robot wouldn't veer off.

Member

waveform80 commented Jun 15, 2016

First impression I don't like this...

Do you think ms would be a better unit? Part of me thinks so, and I'm sure for people experienced with servos it probably would be. But I think for complete newbies (which is the major focus of the library), consistency with everything else might be easier (even if it means writing 2e-3, or 2/1000, or 0.002). Happy to be persuaded otherwise though!

What's the use case? I thought it made sense for stopping jitter, but we won't have that with pigpio.

When the servo is actively controlled you can't manually adjust the servo's position. There may be some minor use cases for manual adjustment but a more important point (particularly for robotics) is that the servo's constantly drawing power when actively controlled. When "free" it doesn't draw power but there's usually enough resistance in the servo to reasonably assume that a steering wheel on a robot wouldn't veer off.

@waveform80

This comment has been minimized.

Show comment
Hide comment
@waveform80

waveform80 Jun 15, 2016

Member

All sounds good. What about people using fully rotational servos as robot motors? Would a Motor style interface make sense? forward(), backward(), etc.

Still researching these, but yes I'm thinking another class similar to Motor for them (this was part of the reason I suggested AngularServo earlier ... as I'm not happy with RotationalServo vs FullyRotationalServo ... far too confusing :)

Member

waveform80 commented Jun 15, 2016

All sounds good. What about people using fully rotational servos as robot motors? Would a Motor style interface make sense? forward(), backward(), etc.

Still researching these, but yes I'm thinking another class similar to Motor for them (this was part of the reason I suggested AngularServo earlier ... as I'm not happy with RotationalServo vs FullyRotationalServo ... far too confusing :)

@bennuttall

This comment has been minimized.

Show comment
Hide comment
@bennuttall

bennuttall Jun 15, 2016

Member

Do you think ms would be a better unit?

Maybe. Can you give examples of the full constructor? i.e. Servo(4, ...)

When the servo is actively controlled you can't manually adjust the servo's position

Fair enough

Member

bennuttall commented Jun 15, 2016

Do you think ms would be a better unit?

Maybe. Can you give examples of the full constructor? i.e. Servo(4, ...)

When the servo is actively controlled you can't manually adjust the servo's position

Fair enough

@waveform80

This comment has been minimized.

Show comment
Hide comment
@waveform80

waveform80 Jun 15, 2016

Member

Maybe. Can you give examples of the full constructor? i.e. Servo(4, ...)

Minimal example (assuming a servo on GPIO17 with a 20ms overall pulse width, minimum position 1ms pulse, maximum position 2ms pulse):

servo = Servo(17)

Alternatively, with all parameters specified:

servo = Servo(17,
    frame_width=20/1000, min_pulse_width=1/1000,
    max_pulse_width=2/1000)

Adding in angles, the minimum case (assuming angles 0-180):

servo = RotationalServo(17)

Alternatively, with all parameters specified:

servo = RotationalServo(17,
    frame_width=20/1000, min_pulse_width=1/1000, max_pulse_width=2/1000,
    min_angle=0, max_angle=180)

Definitely thinking AngularServo might be better - hints at the fact there's an angle property...

Member

waveform80 commented Jun 15, 2016

Maybe. Can you give examples of the full constructor? i.e. Servo(4, ...)

Minimal example (assuming a servo on GPIO17 with a 20ms overall pulse width, minimum position 1ms pulse, maximum position 2ms pulse):

servo = Servo(17)

Alternatively, with all parameters specified:

servo = Servo(17,
    frame_width=20/1000, min_pulse_width=1/1000,
    max_pulse_width=2/1000)

Adding in angles, the minimum case (assuming angles 0-180):

servo = RotationalServo(17)

Alternatively, with all parameters specified:

servo = RotationalServo(17,
    frame_width=20/1000, min_pulse_width=1/1000, max_pulse_width=2/1000,
    min_angle=0, max_angle=180)

Definitely thinking AngularServo might be better - hints at the fact there's an angle property...

@waveform80

This comment has been minimized.

Show comment
Hide comment
@waveform80

waveform80 Jun 15, 2016

Member

Hmm, maybe ContinuousServo for the fully rotational style servos - implies that they continue rotating like a Motor ... or maybe just ServoMotor (although that's a bit confusing because all servos are ultimately motors...)

Member

waveform80 commented Jun 15, 2016

Hmm, maybe ContinuousServo for the fully rotational style servos - implies that they continue rotating like a Motor ... or maybe just ServoMotor (although that's a bit confusing because all servos are ultimately motors...)

@bennuttall

This comment has been minimized.

Show comment
Hide comment
@bennuttall

bennuttall Jun 15, 2016

Member

I like that. 1/1000 is fine. Any issues with floating point precision?

@MarcScott any thoughts on the above usage?

Member

bennuttall commented Jun 15, 2016

I like that. 1/1000 is fine. Any issues with floating point precision?

@MarcScott any thoughts on the above usage?

@waveform80

This comment has been minimized.

Show comment
Hide comment
@waveform80

waveform80 Jun 15, 2016

Member

I'm happy with this decision. How thoroughly is it tested? Both in the test suite and in real use

Real use - how's it going in Scratch? That's probably its biggest exposure at the moment.

We need to improve our test suite for it though - we're doing horrible things to the daemon at the moment (killall because I quickly hacked it together); we should be using systemctl for it instead. Will have to see what I can do.

Member

waveform80 commented Jun 15, 2016

I'm happy with this decision. How thoroughly is it tested? Both in the test suite and in real use

Real use - how's it going in Scratch? That's probably its biggest exposure at the moment.

We need to improve our test suite for it though - we're doing horrible things to the daemon at the moment (killall because I quickly hacked it together); we should be using systemctl for it instead. Will have to see what I can do.

@bennuttall

This comment has been minimized.

Show comment
Hide comment
@bennuttall

bennuttall Jun 15, 2016

Member

Real use - how's it going in Scratch?

Good point. I'll ask around.

Member

bennuttall commented Jun 15, 2016

Real use - how's it going in Scratch?

Good point. I'll ask around.

@waveform80

This comment has been minimized.

Show comment
Hide comment
@waveform80

waveform80 Jun 15, 2016

Member

I like that. 1/1000 is fine. Any issues with floating point precision?

Not really (we're way below the precision provided by the 53bit mantissa) - there's more of an issue with what happens when frequencies not directly supportable by DMA are selected (for semi-HW PWM). I need to do a bit more testing of this but my preliminary results are that even my cheapy servos are quite tolerant of timing variances provided they're regular.

Member

waveform80 commented Jun 15, 2016

I like that. 1/1000 is fine. Any issues with floating point precision?

Not really (we're way below the precision provided by the 53bit mantissa) - there's more of an issue with what happens when frequencies not directly supportable by DMA are selected (for semi-HW PWM). I need to do a bit more testing of this but my preliminary results are that even my cheapy servos are quite tolerant of timing variances provided they're regular.

@SteveAmor

This comment has been minimized.

Show comment
Hide comment
@SteveAmor

SteveAmor Jun 16, 2016

Contributor

@waveform80 said:

Do you think ms would be a better unit? Part of me thinks so, and I'm sure for people experienced with servos it probably would be. But I think for complete newbies (which is the major focus of the library), consistency with everything else might be easier (even if it means writing 2e-3, or 2/1000, or 0.002). Happy to be persuaded otherwise though!

I am not in favour of using numbers less than 1 for the simple fact that it's even more for kids to get their head around. If you're going to use msec, you may just as well go with usec and use nice big whole numbers from 1000 to 1500 to 2000 for example. I know 10 year old kids will get that.

Of course, I agree, in engineering, that we should use consistent units but the whole thing about gpiozero is abstracting all this complexity away.

Just my 2c (or should that be pence - lol). I won't take it personally. Just some feedback from working with kids (think very small numbers used in sleep()).

Contributor

SteveAmor commented Jun 16, 2016

@waveform80 said:

Do you think ms would be a better unit? Part of me thinks so, and I'm sure for people experienced with servos it probably would be. But I think for complete newbies (which is the major focus of the library), consistency with everything else might be easier (even if it means writing 2e-3, or 2/1000, or 0.002). Happy to be persuaded otherwise though!

I am not in favour of using numbers less than 1 for the simple fact that it's even more for kids to get their head around. If you're going to use msec, you may just as well go with usec and use nice big whole numbers from 1000 to 1500 to 2000 for example. I know 10 year old kids will get that.

Of course, I agree, in engineering, that we should use consistent units but the whole thing about gpiozero is abstracting all this complexity away.

Just my 2c (or should that be pence - lol). I won't take it personally. Just some feedback from working with kids (think very small numbers used in sleep()).

@lurch

This comment has been minimized.

Show comment
Hide comment
@lurch

lurch Jun 16, 2016

Contributor

My comments...

So, for 1.3 we probably want to make pigpiod the primary implementation (especially now it's packaged and installed in raspbian by default) with RPIO and RPi.GPIO as fallback, then native as final fallback

I think there ought to be more comprehensive pin unit-tests before this switch is made, which you've already addressed with:

We need to improve our test suite for it though - we're doing horrible things to the daemon at the moment (killall because I quickly hacked it together); we should be using systemctl for it instead. Will have to see what I can do.

I guess if pigpiod is running before the unit-tests start, then it should still be left running after the unit-tests finish. But if it isn't running before the unit-tests start, then IMHO it should be automatically terminated when the unit-tests finish.
(i.e. unit-tests should have no side-effects)

How much effort would it be to get the python-wrapper part of pigpiod pip-installable, to make virtualenv and remote-gpio usage easier?

From reading lots of stuff today (a good half the day has been spent reading!) ... Given there's much more than just rotational servos out there

Please enlighten us... what kind of other 'servos' are there? Are any of them likely to be used by the same kind of people using GpioZero?
E.g. I'd expect GpioZero to only be used with hobby-type servos - so is there really any need to have the frame_width as an optional parameter, rather than hardcoding it to 20ms? (Or if it needs to remain as a parameter, maybe it should be moved to the end of the parameter-list, as it's the option least likely to need changing?

I agree with Ben that 1/1000 is much nicer notation (for kids) than 1e-3 or 0.001. Alternatively I guess the parameters could be named e.g. min_pulse_width_ms=1.0 to make it clear that they expect milliseconds rather than seconds.

initial_value should probably be 0.5 but I've left it at 0.0 for now because ... laziness.

I think this makes sense for the current API.
If you wanted to use 0.5 as the initial_value i.e. have the midpoint being the starting-point of the servo, then IMHO it'd be more consistent with the rest of GpioZero if the midpoint was value 0.0 and values either side of the midpoint went up to +1 / -1 for either direction. (although as I suggested in an earlier comment, perhaps we could have a CenteredAngularServo class which does exhibit this behaviour?)

The "detached" use-case is interesting and sounds useful. I'm not enamoured of the idea of off and on doing something completely different to the rest of the library, and they also don't correspond to what the device actually does. So, following Motor's example I've deviated completely and created the following methods:
...
off - switch PWM off (set value to None)
Tempted to add detach as an alias for off (for Arduino users) although that would mean I ought to add attach as well to set the last non-None value.

Why not just skip off and simply name the function detach? I think if you had an off method, people might expect a matching on method, and as discussed (to death?) earlier on is fairly ambiguous for servos. Whereas I think adding both detach and attach is fairly clear what they should do?
(although I guess having an off method would allow it to work with http://gpiozero.readthedocs.io/en/v1.2.0/api_boards.html#gpiozero.CompositeOutputDevice.off - so perhaps on and off should be aliases of attach and detach). It's easy to imagine a ServoBoard class (analogous to LEDBoard and ButtonBoard) being used to provide an interface to e.g. https://shop.pimoroni.com/products/adafruit-16-channel-pwm-servo-hat-for-raspberry-pi-mini-kit

And I guess perhaps is_active for a servo should return self.value is not None. I guess Servo should also be an OutputDevice, i.e. have source and values, so the sourcetools stuff will need modifying to be able to cope with None values.

min - switch PWM on (if it's not already) and set minimum pulse width (basically set value to 0)
max - switch PWM on (if it's not already) and set maximum pulse width (set value to 1)
mid - switch PWM on (if it's not already) and set mid-point pulse width (set value to 0.5)

I guess setting-value-to-0 and setting-value-to-1 makes min / max similar to on / off for a PWMLED. Less convinced about the need for mid though?
Given what @SteveAmor has said in previous comments, there probably ought to be a centrepoint (or something) function which outputs an exact 1.5ms pulse-width (as long as it falls between min_pulse_width and max_pulse_width), rather than simply being the average of min_pulse_width and max_pulse_width ? (Or instead of hardcoding 1.5ms, maybe we could have an optional centrepoint_pulse_width which defaults to 1.5/1000 ?)
And/or perhaps there could also be a property to get / set the currently-output pulse-width? (again, as long as it falls between min_pulse_width and max_pulse_width) I guess it'd be the equivalent of raw_value in other GpioZero classes.

I've derived RotationalServo (AngularServo?) from Servo

IMHO RotationalServo sounds too similar to continuous-rotation servo, so I think AngularServo would be better.

Thought about adding left, right, clockwise, etc. but there seems to be plenty of variation on this subject (i.e. it'll just vary servo to servo) and I think sticking with straight aliases of what we're actually doing (min, max, etc.) is probably simpler.

Agreed. Perhaps @bennuttall 's active_high idea could be incorporated, to 'invert' the meaning of the value? (i.e. if the user wants increasing values to rotate the servo in the opposite direction)

is just setting value, or angle in RotationalServo, enough?

Yeah, I think so. I guess the only reason that http://gpiozero.readthedocs.io/en/stable/api_output.html#motor has separate backward and forward methods is to hide the "complexity" of backward effectively setting negative values?

If you're going to use msec, you may just as well go with usec

I think the advantage to using seconds is that it 'matches' the rest of the GpioZero API, and the advantage to using milliseconds is that they're the units traditionally used to specify servo parameters. I don't think using microseconds gives any advantages at all?

Just some feedback from working with kids (think very small numbers used in sleep()).

Yeah, I was a little bit surprised when I noticed that http://microbit-micropython.readthedocs.io/en/latest/microbit.html#microbit.sleep takes values in milliseconds, in contrast to https://docs.python.org/2/library/time.html#time.sleep which takes values in seconds.

All sounds good. What about people using fully rotational servos as robot motors? Would a Motor style interface make sense? forward(), backward(), etc.

Slightly OffTopic, but I've been pondering 'inverting' the logic of the value property-setter and forward / backward methods in Motor, so that setting the value directly controls the forward_device and backward_device objects; and then we could have a MotorMixin class (see discussions elsewhere about varieties of Motors and Robots) that provided is_active, forward, backward, reverse and stop methods, all by manipulating self.value. So by simply providing a ContinuousRotationServo class with a value settable between -1 and +1, we could use the MotorMixin to automatically give it forward and backward methods, to allow it to be used anywhere in GpioZero that expects a Motor-like object :-)
(And there's other places in GpioZero where doing "everything" by manipulating self.value offers advantages too)

Contributor

lurch commented Jun 16, 2016

My comments...

So, for 1.3 we probably want to make pigpiod the primary implementation (especially now it's packaged and installed in raspbian by default) with RPIO and RPi.GPIO as fallback, then native as final fallback

I think there ought to be more comprehensive pin unit-tests before this switch is made, which you've already addressed with:

We need to improve our test suite for it though - we're doing horrible things to the daemon at the moment (killall because I quickly hacked it together); we should be using systemctl for it instead. Will have to see what I can do.

I guess if pigpiod is running before the unit-tests start, then it should still be left running after the unit-tests finish. But if it isn't running before the unit-tests start, then IMHO it should be automatically terminated when the unit-tests finish.
(i.e. unit-tests should have no side-effects)

How much effort would it be to get the python-wrapper part of pigpiod pip-installable, to make virtualenv and remote-gpio usage easier?

From reading lots of stuff today (a good half the day has been spent reading!) ... Given there's much more than just rotational servos out there

Please enlighten us... what kind of other 'servos' are there? Are any of them likely to be used by the same kind of people using GpioZero?
E.g. I'd expect GpioZero to only be used with hobby-type servos - so is there really any need to have the frame_width as an optional parameter, rather than hardcoding it to 20ms? (Or if it needs to remain as a parameter, maybe it should be moved to the end of the parameter-list, as it's the option least likely to need changing?

I agree with Ben that 1/1000 is much nicer notation (for kids) than 1e-3 or 0.001. Alternatively I guess the parameters could be named e.g. min_pulse_width_ms=1.0 to make it clear that they expect milliseconds rather than seconds.

initial_value should probably be 0.5 but I've left it at 0.0 for now because ... laziness.

I think this makes sense for the current API.
If you wanted to use 0.5 as the initial_value i.e. have the midpoint being the starting-point of the servo, then IMHO it'd be more consistent with the rest of GpioZero if the midpoint was value 0.0 and values either side of the midpoint went up to +1 / -1 for either direction. (although as I suggested in an earlier comment, perhaps we could have a CenteredAngularServo class which does exhibit this behaviour?)

The "detached" use-case is interesting and sounds useful. I'm not enamoured of the idea of off and on doing something completely different to the rest of the library, and they also don't correspond to what the device actually does. So, following Motor's example I've deviated completely and created the following methods:
...
off - switch PWM off (set value to None)
Tempted to add detach as an alias for off (for Arduino users) although that would mean I ought to add attach as well to set the last non-None value.

Why not just skip off and simply name the function detach? I think if you had an off method, people might expect a matching on method, and as discussed (to death?) earlier on is fairly ambiguous for servos. Whereas I think adding both detach and attach is fairly clear what they should do?
(although I guess having an off method would allow it to work with http://gpiozero.readthedocs.io/en/v1.2.0/api_boards.html#gpiozero.CompositeOutputDevice.off - so perhaps on and off should be aliases of attach and detach). It's easy to imagine a ServoBoard class (analogous to LEDBoard and ButtonBoard) being used to provide an interface to e.g. https://shop.pimoroni.com/products/adafruit-16-channel-pwm-servo-hat-for-raspberry-pi-mini-kit

And I guess perhaps is_active for a servo should return self.value is not None. I guess Servo should also be an OutputDevice, i.e. have source and values, so the sourcetools stuff will need modifying to be able to cope with None values.

min - switch PWM on (if it's not already) and set minimum pulse width (basically set value to 0)
max - switch PWM on (if it's not already) and set maximum pulse width (set value to 1)
mid - switch PWM on (if it's not already) and set mid-point pulse width (set value to 0.5)

I guess setting-value-to-0 and setting-value-to-1 makes min / max similar to on / off for a PWMLED. Less convinced about the need for mid though?
Given what @SteveAmor has said in previous comments, there probably ought to be a centrepoint (or something) function which outputs an exact 1.5ms pulse-width (as long as it falls between min_pulse_width and max_pulse_width), rather than simply being the average of min_pulse_width and max_pulse_width ? (Or instead of hardcoding 1.5ms, maybe we could have an optional centrepoint_pulse_width which defaults to 1.5/1000 ?)
And/or perhaps there could also be a property to get / set the currently-output pulse-width? (again, as long as it falls between min_pulse_width and max_pulse_width) I guess it'd be the equivalent of raw_value in other GpioZero classes.

I've derived RotationalServo (AngularServo?) from Servo

IMHO RotationalServo sounds too similar to continuous-rotation servo, so I think AngularServo would be better.

Thought about adding left, right, clockwise, etc. but there seems to be plenty of variation on this subject (i.e. it'll just vary servo to servo) and I think sticking with straight aliases of what we're actually doing (min, max, etc.) is probably simpler.

Agreed. Perhaps @bennuttall 's active_high idea could be incorporated, to 'invert' the meaning of the value? (i.e. if the user wants increasing values to rotate the servo in the opposite direction)

is just setting value, or angle in RotationalServo, enough?

Yeah, I think so. I guess the only reason that http://gpiozero.readthedocs.io/en/stable/api_output.html#motor has separate backward and forward methods is to hide the "complexity" of backward effectively setting negative values?

If you're going to use msec, you may just as well go with usec

I think the advantage to using seconds is that it 'matches' the rest of the GpioZero API, and the advantage to using milliseconds is that they're the units traditionally used to specify servo parameters. I don't think using microseconds gives any advantages at all?

Just some feedback from working with kids (think very small numbers used in sleep()).

Yeah, I was a little bit surprised when I noticed that http://microbit-micropython.readthedocs.io/en/latest/microbit.html#microbit.sleep takes values in milliseconds, in contrast to https://docs.python.org/2/library/time.html#time.sleep which takes values in seconds.

All sounds good. What about people using fully rotational servos as robot motors? Would a Motor style interface make sense? forward(), backward(), etc.

Slightly OffTopic, but I've been pondering 'inverting' the logic of the value property-setter and forward / backward methods in Motor, so that setting the value directly controls the forward_device and backward_device objects; and then we could have a MotorMixin class (see discussions elsewhere about varieties of Motors and Robots) that provided is_active, forward, backward, reverse and stop methods, all by manipulating self.value. So by simply providing a ContinuousRotationServo class with a value settable between -1 and +1, we could use the MotorMixin to automatically give it forward and backward methods, to allow it to be used anywhere in GpioZero that expects a Motor-like object :-)
(And there's other places in GpioZero where doing "everything" by manipulating self.value offers advantages too)

@lurch lurch referenced this issue Jun 16, 2016

Closed

Motor.active_high #380

@SteveAmor

This comment has been minimized.

Show comment
Hide comment
@SteveAmor

SteveAmor Jun 16, 2016

Contributor

We do need a frame_width parameter. Hobby servos come in two types. Analogue and digital. The analogue servos run at 50Hz max but the digital ones can run up to 400Hz.

Contributor

SteveAmor commented Jun 16, 2016

We do need a frame_width parameter. Hobby servos come in two types. Analogue and digital. The analogue servos run at 50Hz max but the digital ones can run up to 400Hz.

@lurch

This comment has been minimized.

Show comment
Hide comment
@lurch

lurch Jun 16, 2016

Contributor

The analogue servos run at 50Hz max but the digital ones can run up to 400Hz.

Ahh, thanks :) Do the digital servos use smaller pulse-widths too? (I'm wondering if there should be a DigitalAngularServo with different default init-params?)

BTW: You can use the greater-than symbol > at the start of a line to 'quote' previous comments.

Contributor

lurch commented Jun 16, 2016

The analogue servos run at 50Hz max but the digital ones can run up to 400Hz.

Ahh, thanks :) Do the digital servos use smaller pulse-widths too? (I'm wondering if there should be a DigitalAngularServo with different default init-params?)

BTW: You can use the greater-than symbol > at the start of a line to 'quote' previous comments.

@SteveAmor

This comment has been minimized.

Show comment
Hide comment
@SteveAmor

SteveAmor Jun 16, 2016

Contributor

Ahh, thanks :) Do the digital servos use smaller pulse-widths too?

Nope.

Would a servo ever be active_low??

The same make and model of a servo is unlikely to be but a mix of servos can rotate in opposite directions for a given pulse width due to the wires on the motor and the servo gearbox. Therefore, it would be good to have the active_low parameter as this would be the only solution for the user to solve this issue (unlike the motor in #380 which can be resolved by reversing the wires in hardware or pins in software).

Contributor

SteveAmor commented Jun 16, 2016

Ahh, thanks :) Do the digital servos use smaller pulse-widths too?

Nope.

Would a servo ever be active_low??

The same make and model of a servo is unlikely to be but a mix of servos can rotate in opposite directions for a given pulse width due to the wires on the motor and the servo gearbox. Therefore, it would be good to have the active_low parameter as this would be the only solution for the user to solve this issue (unlike the motor in #380 which can be resolved by reversing the wires in hardware or pins in software).

@lurch

This comment has been minimized.

Show comment
Hide comment
@lurch

lurch Jun 16, 2016

Contributor

#314 (comment) seems to reinforce the point that we should probably have an optional centrepoint_pulse_width init-param.

Contributor

lurch commented Jun 16, 2016

#314 (comment) seems to reinforce the point that we should probably have an optional centrepoint_pulse_width init-param.

@bennuttall

This comment has been minimized.

Show comment
Hide comment
@bennuttall

bennuttall Jun 16, 2016

Member

Apparently no reported issues with Scratch. However the reason the daemon isn't run by default is that it hogs a lot of CPU due to constant GPIO polling.

Member

bennuttall commented Jun 16, 2016

Apparently no reported issues with Scratch. However the reason the daemon isn't run by default is that it hogs a lot of CPU due to constant GPIO polling.

waveform80 added a commit that referenced this issue Aug 30, 2016

@waveform80

This comment has been minimized.

Show comment
Hide comment
@waveform80

waveform80 Aug 30, 2016

Member

The PR merged doesn't contain everything everyone wanted, but it's intended to be a start we can build on in 1.4. I'll be opening that milestone shortly as 1.3 should be released tonight (we've got a Raspbian freeze tomorrow so things are a bit rushed!)

Member

waveform80 commented Aug 30, 2016

The PR merged doesn't contain everything everyone wanted, but it's intended to be a start we can build on in 1.4. I'll be opening that milestone shortly as 1.3 should be released tonight (we've got a Raspbian freeze tomorrow so things are a bit rushed!)

@SteveAmor

This comment has been minimized.

Show comment
Hide comment
@SteveAmor

SteveAmor Aug 31, 2016

Contributor

Great work guys.

Contributor

SteveAmor commented Aug 31, 2016

Great work guys.

@waveform80

This comment has been minimized.

Show comment
Hide comment
@waveform80

waveform80 Aug 31, 2016

Member

Some more notes on the implementation I didn't have time for last night:

  1. I haven't switched the default pin backend to pigpiod in this release. As @lurch notes we need more tests first. However, it is much easier for users to use alternate backends in this release. So if you want to try it, just make sure pigpiod is running and run your script with GPIOZERO_PIN_FACTORY=PiGPIOPin python my_script.py - you can also specify remote hosts with PIGPIO_ADDR=host too.
  2. I went with AngularServo rather than RotationalServo.
  3. Haven't looked into continuous rotational servos yet, so there's nothing in there for that.
  4. There's a facility for querying the current pulse width but not for setting it explicitly; if there's enough demand we can add it easily enough.
  5. The default pulse width limits are set at 1ms and 2ms, and for consistency all values are measured in seconds (the defaults are specified with the notation 1/1000 and 2/1000 rather than exponents or decimals, as consensus seems to be that's the easiest to understand).
  6. I tested with four different servos (A0090, DS-929MG, S05NF, and what's probably an SG90 but it's unlabelled) of which three used those pulse width limits. The fourth used 0.5 and 2.5ms, but after jamming one of the other servos when testing expanded ranges (and getting horrible grinding sounds from the rest) I've left out any mention of expanded ranges in the docs :)
  7. Value ranges are currently fixed at -1 to +1 for Servo. Might look to make this configurable in the future (e.g. 0..1 as an option). Angle ranges on AngularServo are configurable and include being reversible (for servos that run counter to expectations). The docs include some brief calibration instructions.
Member

waveform80 commented Aug 31, 2016

Some more notes on the implementation I didn't have time for last night:

  1. I haven't switched the default pin backend to pigpiod in this release. As @lurch notes we need more tests first. However, it is much easier for users to use alternate backends in this release. So if you want to try it, just make sure pigpiod is running and run your script with GPIOZERO_PIN_FACTORY=PiGPIOPin python my_script.py - you can also specify remote hosts with PIGPIO_ADDR=host too.
  2. I went with AngularServo rather than RotationalServo.
  3. Haven't looked into continuous rotational servos yet, so there's nothing in there for that.
  4. There's a facility for querying the current pulse width but not for setting it explicitly; if there's enough demand we can add it easily enough.
  5. The default pulse width limits are set at 1ms and 2ms, and for consistency all values are measured in seconds (the defaults are specified with the notation 1/1000 and 2/1000 rather than exponents or decimals, as consensus seems to be that's the easiest to understand).
  6. I tested with four different servos (A0090, DS-929MG, S05NF, and what's probably an SG90 but it's unlabelled) of which three used those pulse width limits. The fourth used 0.5 and 2.5ms, but after jamming one of the other servos when testing expanded ranges (and getting horrible grinding sounds from the rest) I've left out any mention of expanded ranges in the docs :)
  7. Value ranges are currently fixed at -1 to +1 for Servo. Might look to make this configurable in the future (e.g. 0..1 as an option). Angle ranges on AngularServo are configurable and include being reversible (for servos that run counter to expectations). The docs include some brief calibration instructions.
@waveform80

This comment has been minimized.

Show comment
Hide comment
@waveform80

waveform80 Aug 31, 2016

Member

Oh, missed some (looking back through @lurch's posts):

  1. on and off aren't included - instead there's a detach method which stops driving the servo, and None can be assigned to value or angle for similar effect.
  2. There's currently no attach - you need to explicitly specify how you want to start driving the servo again (either by setting an explicit value or calling min, mid, or max. We might add one, but it means keeping state around which I'm never keen on ... so that's a maybe.
  3. is_active is indeed configured to be True when value is non-None.
Member

waveform80 commented Aug 31, 2016

Oh, missed some (looking back through @lurch's posts):

  1. on and off aren't included - instead there's a detach method which stops driving the servo, and None can be assigned to value or angle for similar effect.
  2. There's currently no attach - you need to explicitly specify how you want to start driving the servo again (either by setting an explicit value or calling min, mid, or max. We might add one, but it means keeping state around which I'm never keen on ... so that's a maybe.
  3. is_active is indeed configured to be True when value is non-None.
@lurch

This comment has been minimized.

Show comment
Hide comment
@lurch

lurch Aug 31, 2016

Contributor

Value ranges are currently fixed at -1 to +1 for Servo. Might look to make this configurable in the future (e.g. 0..1 as an option).

I think making it configurable might be too confusing - perhaps a better option would be a Servo subclass that treats value as 0..1
Although I guess AngularServo(pin, initial_angle=0.5, min_angle=0, max_angle=1) would effectively achieve the same thing?

Something else that's just occurred to me is that a servo.pulse() function might be useful for quick flag-waving type demos ;-) (although that may be possible already with the sourcetools stuff)

I tested with four different servos (A0090, DS-929MG, S05NF, and what's probably an SG90 but it's unlabelled)

I assume you tested with the different Pin backends too? How do they end up comparing in terms of jitteryness?

The PR merged doesn't contain everything everyone wanted

In that case perhaps this issue should be re-opened for continued discussion? (so that it doesn't get "lost").
Or given how many comments this issue already has, perhaps a new 'cleaner' issue should be opened?

Contributor

lurch commented Aug 31, 2016

Value ranges are currently fixed at -1 to +1 for Servo. Might look to make this configurable in the future (e.g. 0..1 as an option).

I think making it configurable might be too confusing - perhaps a better option would be a Servo subclass that treats value as 0..1
Although I guess AngularServo(pin, initial_angle=0.5, min_angle=0, max_angle=1) would effectively achieve the same thing?

Something else that's just occurred to me is that a servo.pulse() function might be useful for quick flag-waving type demos ;-) (although that may be possible already with the sourcetools stuff)

I tested with four different servos (A0090, DS-929MG, S05NF, and what's probably an SG90 but it's unlabelled)

I assume you tested with the different Pin backends too? How do they end up comparing in terms of jitteryness?

The PR merged doesn't contain everything everyone wanted

In that case perhaps this issue should be re-opened for continued discussion? (so that it doesn't get "lost").
Or given how many comments this issue already has, perhaps a new 'cleaner' issue should be opened?

@bennuttall

This comment has been minimized.

Show comment
Hide comment
@bennuttall

bennuttall Aug 31, 2016

Member

Or given how many comments this issue already has, perhaps a new 'cleaner' issue should be opened?

New issue please!!

Member

bennuttall commented Aug 31, 2016

Or given how many comments this issue already has, perhaps a new 'cleaner' issue should be opened?

New issue please!!

@waveform80

This comment has been minimized.

Show comment
Hide comment
@waveform80

waveform80 Aug 31, 2016

Member

Something else that's just occurred to me is that a servo.pulse() function might be useful for quick flag-waving type demos ;-) (although that may be possible already with the sourcetools stuff)

Heh - one of the first things I tried was:

$ GPIOZERO_PIN_FACTORY=PiGPIOPin python

from gpiozero import *
from gpiozero.tools import *
s = Servo(21)
s.source = sin_values()

Sit back and watch it wave!

I assume you tested with the different Pin backends too? How do they end up comparing in terms of jitteryness?

Yeah - no time to test RPIO, but tested RPiGPIOPin and PiGPIOPin. The latter is rock solid - just perfect. The former always jitters but how much depends on system load and other factors (and also on servo choice I think - one servo was really awful with - jumped around the desk!)

New issue please!!

Yeah - my e-mail notification thread for this one is just ridiculous ;)

Member

waveform80 commented Aug 31, 2016

Something else that's just occurred to me is that a servo.pulse() function might be useful for quick flag-waving type demos ;-) (although that may be possible already with the sourcetools stuff)

Heh - one of the first things I tried was:

$ GPIOZERO_PIN_FACTORY=PiGPIOPin python

from gpiozero import *
from gpiozero.tools import *
s = Servo(21)
s.source = sin_values()

Sit back and watch it wave!

I assume you tested with the different Pin backends too? How do they end up comparing in terms of jitteryness?

Yeah - no time to test RPIO, but tested RPiGPIOPin and PiGPIOPin. The latter is rock solid - just perfect. The former always jitters but how much depends on system load and other factors (and also on servo choice I think - one servo was really awful with - jumped around the desk!)

New issue please!!

Yeah - my e-mail notification thread for this one is just ridiculous ;)

@lurch

This comment has been minimized.

Show comment
Hide comment
@lurch

lurch Aug 31, 2016

Contributor

Created #419 :-)

Contributor

lurch commented Aug 31, 2016

Created #419 :-)

@RPi-Distro RPi-Distro locked and limited conversation to collaborators Aug 31, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.