PedalPi / Raspberry-Physical

Raspberry physical components support (RotaryEncoder, knobs, display)
Apache License 2.0
2 stars 0 forks source link

Add RotaryEncoder support #1

Open SrMouraSilva opened 7 years ago

SrMouraSilva commented 7 years ago

Description

Add RotaryEncoder support

Rotary Encoder

Steps

SrMouraSilva commented 7 years ago

@PaulStroffregen implementation

https://github.com/PaulStoffregen/Encoder

Copyright (c) 2011,2013 PJRC.COM, LLC - Paul Stoffregen <paul@pjrc.com>

//                           _______         _______       
//               Pin1 ______|       |_______|       |______ Pin1
// negative <---         _______         _______         __      --> positive
//               Pin2 __|       |_______|       |_______|   Pin2

        //  new new old old
        //  pin2    pin1    pin2    pin1    Result
        //  ----    ----    ----    ----    ------
        //  0   0   0   0   no movement
        //  0   0   0   1   +1
        //  0   0   1   0   -1
        //  0   0   1   1   +2  (assume pin1 edges only)
        //  0   1   0   0   -1
        //  0   1   0   1   no movement
        //  0   1   1   0   -2  (assume pin1 edges only)
        //  0   1   1   1   +1
        //  1   0   0   0   +1
        //  1   0   0   1   -2  (assume pin1 edges only)
        //  1   0   1   0   no movement
        //  1   0   1   1   -1
        //  1   1   0   0   +2  (assume pin1 edges only)
        //  1   1   0   1   -1
        //  1   1   1   0   +1
        //  1   1   1   1   no movement
/*
    // Simple, easy-to-read "documentation" version :-)
    //
    void update(void) {
        uint8_t s = state & 3;
        if (digitalRead(pin1)) s |= 4;
        if (digitalRead(pin2)) s |= 8;
        switch (s) {
            case 0: case 5: case 10: case 15:
                break;
            case 1: case 7: case 8: case 14:
                position++; break;
            case 2: case 4: case 11: case 13:
                position--; break;
            case 3: case 12:
                position += 2; break;
            default:
                position -= 2; break;
        }
        state = (s >> 2);
    }
id new pin2 new pin1 old pin2 old pin1 Result
0 0 0 0 0 no movement
1 0 0 0 1 +1
2 0 0 1 0 -1
3 0 0 1 1 +2 (assume pin1 edges only)
4 0 1 0 0 -1
5 0 1 0 1 no movement
6 0 1 1 0 -2 (assume pin1 edges only)
7 0 1 1 1 +1
8 1 0 0 0 +1
9 1 0 0 1 -2 (assume pin1 edges only)
10 1 0 1 0 no movement
11 1 0 1 1 -1
12 1 1 0 0 +2 (assume pin1 edges only)
13 1 1 0 1 -1
14 1 1 1 0 +1
15 1 1 1 1 no movement
SrMouraSilva commented 7 years ago

Please, view https://github.com/RPi-Distro/python-gpiozero/issues/392

SrMouraSilva commented 7 years ago
# -*- coding: utf-8 -*-
from gpiozero import DigitalInputDevice

class RotaryEncoder(object):
    """
    Decode mechanical rotary encoder pulses.

    The following example will print a Rotary Encoder change direction::

        from gpiozero import RotaryEncoder

        def change(value):
            if value > 0:
                print("clockwise")

            else:
                print("counterclockwise")

        rotary = RotaryEncoder(13, 19)
        rotary.when_rotated = change

    Based in http://abyz.co.uk/rpi/pigpio/examples.html#Python_rotary_encoder_py
    """
    gpio_a = None
    gpio_b = None

    high_a = False
    high_b = True

    when_rotated = lambda *args : None

    def __init__(self, pin_a, pin_b, pull_up=False):
        """
        Uses for dettect rotary encoder changes (set when_rotated attribute)
        It takes one parameter which is +1 for clockwise and -1 for counterclockwise.

        :param int pin_a:
            Pin number of first (left) pin.
        :param int pin_b:
            Pin number of last (right) pin.
        :param bool pull_up:
            The common contact (middle) should be NOT connected to ground?
        """
        self.gpio_a = DigitalInputDevice(pin=pin_a, pull_up=pull_up)
        self.gpio_b = DigitalInputDevice(pin=pin_b, pull_up=pull_up)

        self.high_a = self.gpio_a.value
        self.high_b = self.gpio_b.value

        high = True

        self.gpio_a.when_activated = lambda *args : self.pulse(True, high)
        self.gpio_a.when_deactivated = lambda *args : self.pulse(True, not high)

        self.gpio_b.when_activated = lambda *args : self.pulse(False, high)
        self.gpio_b.when_deactivated = lambda *args : self.pulse(False, not high)

    def pulse(self, is_pin_a, level):
        """
        Decode the rotary encoder pulse.

                   +---------+         +---------+      1
                   |         |         |         |
         A         |         |         |         |
                   |         |         |         |
         +---------+         +---------+         +----- 0

             +---------+         +---------+            1
             |         |         |         |
         B   |         |         |         |
             |         |         |         |
         ----+         +---------+         +---------+  0

        :param bool is_pin_a:
            The pulse origin came from pin_a?
        :param bool level:
            The level is high?
        """
        value = 0

        plus_a = not self.high_a and self.gpio_a.is_active
        plus_b = not self.high_b and self.gpio_b.is_active

        minus_a = self.high_a and not self.gpio_a.is_active
        minus_b = self.high_b and not self.gpio_b.is_active

        self.update_values(is_pin_a, level)

        high_a = self.high_a
        high_b = self.high_b

        low_a = not self.high_a
        low_b = not self.high_b

        # Commented: middle click interval
        # Uncommented: end click interval

        # Up transitions (0 → 1)
        #if plus_a and low_b:
        #    value = -1
        if plus_a and high_b:
            value = 1
        #elif low_a and plus_b:
        #    value = 1
        elif high_a and plus_b:
            value = -1

        # Botton transitions (1 → 0)
        elif minus_a and low_b:
            value = 1
        #elif minus_a and high_b:
        #    value = -1
        elif low_a and minus_b:
            value = -1
        #elif high_a and minus_b:
        #    value = 1

        # Other cases
        elif plus_a and plus_b:
            value = +2
        elif plus_a and minus_b:
            value = -2
        elif minus_a and plus_b:
            value = -2
        elif minus_a and minus_b:
            value = +2

        # else is no movement
        else:
            return

        self.when_rotated(value)

    def update_values(self, is_pin_a, level):
        """
        :param bool is_pin_a:
            The pulse origin came from pin_a?
        :param bool level:
            The level is high?
        """
        if is_pin_a:
            self.high_a = level
        else:
            self.high_b = level
SrMouraSilva commented 7 years ago
No movement
Old pin a New pina a Old pin b New pin b Result
0 0 0 0 no movement
0 0 1 1 no movement
1 1 0 0 no movement
1 1 1 1 no movement
Up transitions (0 → 1)
Old pin a New pina a Old pin b New pin b Result Click interval
0 1 0 0 -1 Middle click interval
0 1 1 1 1 End click interval
0 0 0 1 1 Middle click interval
1 1 0 1 -1 End click interval
Botton transitions (1 → 0)
Old pin a New pina a Old pin b New pin b Result Click interval
1 0 0 0 1 End click interval
1 0 1 1 -1 Middle click interval
0 0 1 0 -1 End click interval
1 1 1 0 1 Middle click interval
Other cases
Old pin a New pina a Old pin b New pin b Result
0 1 0 1 +2 (assume pin1 edges only)
0 1 1 0 -2 (assume pin1 edges only)
1 0 0 1 -2 (assume pin1 edges only)
1 0 1 0 +2 (assume pin1 edges only)
lurch commented 7 years ago

I haven't tested this, but from a quick glance you really ought to be using DigitalinputDevice (and the EventsMixin callback names), rather than Button.

SrMouraSilva commented 7 years ago

@lurch, thanks for your review. I will study your requests for implement this. Seizing the moment, I appreciate the effort of you in helping the open source community! :smile:

Actually I have an small problems when the encoder has very speed rotated.

When I can make corrections and suggestions, will pull request!

One question, some encoders have a button as well. I implement one RotaryEncoderWithButton extending RotaryEncoder or a single class?

lurch commented 7 years ago

Seizing the moment, I appreciate the effort of you in helping the open source community!

I do my best ;-D

the encoder has very speed rotated.

I guess it's possible that at high speeds, python isn't "fast enough" to keep up with all the switching events?

One question, some encoders have a button as well. I implement one RotaryEncoderWithButton extending RotaryEncoder or a single class?

I'd have a RotaryEncoder class, and if you really wanted to you could create a RotaryEncoderWithButton as a CompositeDevice consisting of a RotaryEncoder object and a Button object. The docs for CompositeDevice aren't very clear, but have a look at https://github.com/RPi-Distro/python-gpiozero/issues/391#issuecomment-231354973 for the sort of thing I mean.

SrMouraSilva commented 7 years ago

I guess it's possible that at high speeds, python isn't "fast enough" to keep up with all the switching events?

  1. Or my RotaryEncoder has a physical problem;
  2. Or the (button) interruptions (when_pressed, when_released) aren't fast. But I realy presumed Python / RPi is fast for this;
  3. Or my implementation simply don't work. However, I struggled to implement a readable code AND works if the rotation aren't fast.

I need to find a development process that gives to test it.

I'd have a RotaryEncoder class, and if you really wanted to you could create a RotaryEncoderWithButton as a CompositeDevice consisting of a RotaryEncoder object and a Button object. The docs for CompositeDevice aren't very clear, but have a look at https://github.com/RPi-Distro/python-gpiozero/issues/391#issuecomment-231354973 for the sort of thing I mean.

Good to know the Composite Device. I think I'll do this later. ^^

SrMouraSilva commented 7 years ago
# -*- coding: utf-8 -*-
from gpiozero import DigitalInputDevice

class RotaryEncoder(object):
    """
    Decode mechanical rotary encoder pulses.

    The following example will print a Rotary Encoder change direction::

        from gpiozero import RotaryEncoder

        def change(value):
            if value > 0:
                print("clockwise")

            else:
                print("counterclockwise")

        rotary = RotaryEncoder(13, 19)
        rotary.when_rotated = change

    Based in http://abyz.co.uk/rpi/pigpio/examples.html#Python_rotary_encoder_py
    """
    gpio_a = None
    gpio_b = None

    high_a = False
    high_b = False

    when_rotated = lambda *args : None

    def __init__(self, pin_a, pin_b, pull_up=False):
        """
        Uses for dettect rotary encoder changes (set when_rotated attribute)
        It takes one parameter which is +1 for clockwise and -1 for counterclockwise.

        :param int pin_a:
            Pin number of first (left) pin.
        :param int pin_b:
            Pin number of last (right) pin.
        :param bool pull_up:
            The common contact (middle) should be NOT connected to ground?
        """
        self.gpio_a = DigitalInputDevice(pin=pin_a, pull_up=pull_up)
        self.gpio_b = DigitalInputDevice(pin=pin_b, pull_up=pull_up)

        self.gpio_a.when_activated = lambda *args : self.pulse()
        self.gpio_a.when_deactivated = lambda *args : self.pulse()

        self.gpio_b.when_activated = lambda *args : self.pulse()
        self.gpio_b.when_deactivated = lambda *args : self.pulse()

        self.high_a = self.gpio_a.is_active
        self.high_b = self.gpio_b.is_active

        self.table_values = TableValues()

    def pulse(self):
        """
        Calls when_rotated callback if dettected changes
        """

        new_pin2 = self.gpio_b.is_active
        new_pin1 = self.gpio_a.is_active

        old_pin2 = self.high_b
        old_pin1 = self.high_a

        value = self.table_values.value(new_pin2, new_pin1, old_pin2, old_pin1)

        self.high_b = new_pin2
        self.high_a = new_pin1

        if value != 0:
            self.when_rotated(value)

class TableValues:
    """
    Decode the rotary encoder pulse.

               +---------+         +---------+      1
               |         |         |         |
     A         |         |         |         |
               |         |         |         |
     +---------+         +---------+         +----- 0

         +---------+         +---------+            1
         |         |         |         |
     B   |         |         |         |
         |         |         |         |
     ----+         +---------+         +---------+  0

    Based in table:

    https://github.com/PedalPi/Physical/issues/1#issuecomment-248977908
    """

    def __init__(self):
        values = {
            0: +0,
            1: +1,
            2: -1,
            3: +2
            4: -1,
            5: +0,
            6: -2,
            7: +1,
            8: +1,
            9: -2
            10: +0,
            11: -1,
            12: +2
            13: -1
            14: +1,
            15: +0
        }

    def calcule_index(self, new_pin2, new_pin1, old_pin2, old_pin1):
        value = 8 if new_pin2 else 0
        value += 4 if new_pin1 else 0
        value += 2 if old_pin2 else 0
        value += 1 if old_pin1 else 0

        return value

    def value(self, new_pin2, new_pin1, old_pin2, old_pin1):
        index = self.calcule_index(new_pin2, new_pin1, old_pin2, old_pin1)
        return self.values[index]
lurch commented 7 years ago

Or the (button) interruptions (when_pressed, when_released) aren't fast. But I realy presumed Python / RPi is fast for this;

I wonder if you're also being affected by the same issue as https://github.com/RPi-Distro/python-gpiozero/issues/385 , which suggests that the way GpioZero is implemented (using Python Events) is slower than 'just' using RPi.GPIO ?

I need to find a development process that gives to test it.

GpioZero has quite a comprehensive test suite (with MockPins that you can use to simulate GPIO pins), but I dunno if it's able to generate pin-change-events quicker than it's able to read them? The other approach (if you want reproducible test timings) would be to use something like an Arduino, which is definitely able to generate pin-changes faster than the Pi can read them (but don't forget to use a level-converter to shift the 5V Arduino signals down to 3.3V for the Pi). The Arduino is 'bare metal' so has no OS getting in the way ;)

Couple of little code tips - feel free to ignore these if you wish:

    def calcule_index(self, new_pin2, new_pin1, old_pin2, old_pin1):
        value = 0
        if new_pin2:
            value += 8
        if new_pin1:
            value += 4
        if old_pin2:
            value += 2
        if old_pin1:
            value += 1

        return value

But this is obviously all your own code, and I'm guessing it's still a work in progress, so feel free to do whatever you want with it :-)

SrMouraSilva commented 7 years ago

Slow?

I wonder if you're also being affected by the same issue as RPi-Distro/python-gpiozero#385 , which suggests that the way GpioZero is implemented (using Python Events) is slower than 'just' using RPi.GPIO ?

I had not thought that might be it. I will test it and will inform you. However, I can say that the native implementation of version 1.2.0 (which I think is the RPi.GPIO) is slow.

I implemented a "driver" for PCD8544 for Pi4j (https://github.com/Pi4J/pi4j/issues/84, https://github.com/Pi4J/pi4j/pull/215). The Pi4J implementation in the version 1.0 has slow, but 1.1 has be speeded. (I do not remember where I saw it on their website, then Citation Needed)

I try port it for gpiozero 3 months ago (https://github.com/PedalPi/Physical/tree/master/component/display), but it's verry slow. I was recently discovered that the pin api in gpiozero. If you can tell me which is the fastest to work, I can resume the adaptation work (#2).

Anyway, I will test it.

Tests

GpioZero has quite a comprehensive test suite (with MockPins that you can use to simulate GPIO pins), but I dunno if it's able to generate pin-change-events quicker than it's able to read them? The other approach (if you want reproducible test timings) would be to use something like an Arduino, which is definitely able to generate pin-changes faster than the Pi can read them (but don't forget to use a level-converter to shift the 5V Arduino signals down to 3.3V for the Pi). The Arduino is 'bare metal' so has no OS getting in the way ;)

I do not understand what's going on, so I do not know what to simulate! The current refactoring is for me to make sure that is not a problem in the implementation. That is, let aside the refactorings and I am testing the 16 possible cases

Code review

Thanks for the suggestions! I'm Java developer, so pythonic patterns (like pep 8) sometimes escape from my fingers.

self.gpio_a.when_activated = lambda *args : self.pulse() could be replaced with self.gpio_a.when_activated = self.pulse

:+1: In my old version, it's neccessary send parameters in the pulse calling. Now not.

No need to pre-declare gpio_a etc. outside of the init method

Using the PyCharm, I understood that. But for me, the code is cleaner. But if the pattern is this, then I have to do

I guess old_pin2 doesn't need to be a separate variable inside your pulse function - you could just pass self.high_b directly into your table_values.value function?

Readability. I have to update the commentary documentation to make more explicit the algorithm. Something like "The current change value is based in the old state of the pins and your new state."

What makes me realize is that high_a is that it is not clear. Will fix it.

I'm not sure TableValues warrants a whole class of its own? (rather than just merging the data and functions into the RotaryEncoder class)

I do not know for sure. It has to do with the responsibilities... I'll still change something, so then decide.

I'd be tempted to rewrite your calcule_index function as:

So for your consideration I'll update! :smile:

As calcule_index is only ever used in one place, it probably doesn't need to be a separate function

I believe that improvements in this sense who have to make is the compiler. Robert "Uncle Bob" Martin, in Clean Code, convinced me to make small methods. (Ok, I try, right! :laughing:)

Observations

SrMouraSilva commented 7 years ago

Do not judge me from my version control be comments. haha

from gpiozero import DigitalInputDevice

class RotaryEncoder(object):
    """
    Decode mechanical rotary encoder pulses.

    The following example will print a Rotary Encoder change direction::

        from gpiozero import RotaryEncoder

        def change(value):
            if value > 0:
                print("clockwise")

            else:
                print("counterclockwise")

        rotary = RotaryEncoder(13, 19)
        rotary.when_rotated = change

    Based in http://abyz.co.uk/rpi/pigpio/examples.html#Python_rotary_encoder_py
    """
    gpio_a = None
    gpio_b = None

    old_a_value = False
    old_b_value = False

    when_rotated = lambda *args : None

    def __init__(self, pin_a, pin_b, pull_up=False):
        """
        Uses for dettect rotary encoder changes (set when_rotated attribute)
        It takes one parameter which is +1 for clockwise and -1 for counterclockwise.

        :param int pin_a:
            Pin number of first (left) pin.
        :param int pin_b:
            Pin number of last (right) pin.
        :param bool pull_up:
            The common contact (middle) should be NOT connected to ground?
        """
        self.gpio_a = DigitalInputDevice(pin=pin_a, pull_up=pull_up)
        self.gpio_b = DigitalInputDevice(pin=pin_b, pull_up=pull_up)

        self.gpio_a.when_activated = self.pulse
        self.gpio_a.when_deactivated = self.pulse

        self.gpio_b.when_activated = self.pulse
        self.gpio_b.when_deactivated = self.pulse

        self.old_a_value = self.gpio_a.is_active
        self.old_b_value = self.gpio_b.is_active

        self.table_values = TableValues()

    def pulse(self):
        """
        Calls when_rotated callback if dettected changes
        """

        new_b_value = self.gpio_b.is_active
        new_a_value = self.gpio_a.is_active

        value = self.table_values.value(new_b_value, new_a_value, old_b_value, old_a_value)

        self.old_b_value = new_b_value
        self.old_a_value = new_a_value

        if value != 0:
            self.when_rotated(value)

class TableValues:
    """
    Decode the rotary encoder pulse.

               +---------+         +---------+      1
               |         |         |         |
     A         |         |         |         |
               |         |         |         |
     +---------+         +---------+         +----- 0

         +---------+         +---------+            1
         |         |         |         |
     B   |         |         |         |
         |         |         |         |
     ----+         +---------+         +---------+  0

    Based in table:

    https://github.com/PedalPi/Physical/issues/1#issuecomment-248977908
    """

    def __init__(self):
        self.values = {
            0: +0,
            1: +1,
            2: -1,
        #    3: +2,
        #    4: -1,
            5: +0,
            6: -2,
        #    7: +1,
        #    8: +1,
            9: -2,
            10: +0,
        #    11: -1,
        #    12: +2,
            13: -1,
            14: +1,
            15: +0
        }

    def calcule_index(self, new_b_value, new_a_value, old_b_value, old_a_value):
        value = 0
        if new_b_value:
            value += 8
        if new_a_value:
            value += 4
        if old_b_value:
            value += 2
        if old_a_value:
            value += 1

        return value

    def value(self, new_b_value, new_a_value, old_b_value, old_a_value):
        index = self.calcule_index(new_b_value, new_a_value, old_b_value, old_a_value)

        try:
            return self.values[index]
        except:
            return 0

current = 0
def change(value):
    global current
    current += value
    print(current, value)

rotary = RotaryEncoder(19, 13, True)
rotary.when_rotated = change

while True:
    continue
SrMouraSilva commented 7 years ago

Pins test

Commentary

Dummy

In my firsts tests I do it in the end:

while True:
    continue

This makes my processor to operate in> 25% unnecessarily.

So so

When I use the below, was a significant improvement

while True:
    time.sleep(5)
    continue

Correct way http://gpiozero.readthedocs.io/en/v1.3.1/notes.html#keep-your-script-running

Usign signal.pause, the best!

Analisys

RPIOPin

> sudo python3 rotaryEncoder.py
Traceback (most recent call last):
  File "encoder2.py", line 3, in <module>
    from gpiozero.pins.rpio import RPIOPin
  File "/usr/lib/python3/dist-packages/gpiozero/pins/rpio.py", line 11, in <module>
    import RPIO
ImportError: No module named 'RPIO'

> sudo python rotaryEncoder.py
Traceback (most recent call last):
  File "encoder2.py", line 3, in <module>
    from gpiozero.pins.rpio import RPIOPin
  File "/usr/lib/python2.7/dist-packages/gpiozero/pins/rpio.py", line 11, in <module>
    import RPIO
  File "build/bdist.linux-armv7l/egg/RPIO/__init__.py", line 115, in <module>
  File "build/bdist.linux-armv7l/egg/RPIO/_GPIO.py", line 7, in <module>
  File "build/bdist.linux-armv7l/egg/RPIO/_GPIO.py", line 6, in __bootstrap__
SystemError: This module can only be run on a Raspberry Pi!

Please note that at the time of writing, RPIO is only compatible with Pi 1’s. Ok, I'm usign RPi2... :disappointed:

PiGPIOPin

NativePin

In general, the best results.

RPiGPIOPin

After I install the others (RPIOPin, PiGPIOPin) and slowed. After the reboot, IF I execute first RPi.GPIO, works very well, but, if tests after the NativePin use, slow. It's neccessary reboot to correctly speed.

Conclusion

It does not work for a while: continue. But I found a bug in NativePin use (1.2.0 and 1.3.0)! I will test in RPi3

lurch commented 7 years ago

However, I can say that the native implementation of version 1.2.0 (which I think is the RPi.GPIO) is slow.

Yes, GpioZero v1.3.1 still uses RPi.GPIO as the default Pin backend. But it's not the Pin backend that's making things slow, but the Event framework that @waveform80 has layered on top of it that is slowing down the detection of input-pin-changes. (which is what https://github.com/RPi-Distro/python-gpiozero/issues/385 is looking at)

I implemented a "driver" for PCD8544 for Pi4j ... I try port it for gpiozero 3 months ago, but it's verry slow.

Looks like it should be possible to drive that display using SPI, and GpioZero has support for the Pi's hardware SPI interface (which is much faster than bit-banging) https://gpiozero.readthedocs.io/en/latest/api_spi.html

I was recently discovered that the pin api in gpiozero. If you can tell me which is the fastest to work

The Pin API is currently going through another re-working https://github.com/RPi-Distro/python-gpiozero/issues/459 But you shouldn't modify your code to use a specific Pin backend, but instead let the user of your script choose which backend to use by setting the GPIOZERO_PIN_FACTORY environment-variable.

I do not understand what's going on, so I do not know what to simulate!

You mentioned that you weren't sure if it was your code that was going wrong, or your rotary encoder that was going wrong, so I was suggesting using an Arduino to "simulate" a rotary encoder (i.e. output the same pattern on the pins), so that you at least know the pin-changes are working reliably, to eliminate the possibly-dodgy rotary encoder ;-) (and by altering the timings, you can simulate a rotary encoder turning at different speeds)

I'm Java developer, so pythonic patterns (like pep 8) sometimes escape from my fingers.

Yup, Python is definitely very different to Java! ;-)

I believe that improvements in this sense who have to make is the compiler.

Python is an interpreted language, not compiled. So creating unnecessary classes, having more function calls than are strictly needed, etc. can sometimes slow down the code.

Please note that at the time of writing, RPIO is only compatible with Pi 1’s. Ok, I'm usign RPi2...

Yeah. The author started updating it, but then stopped again https://github.com/metachris/RPIO/issues/77#issuecomment-223403985 :-(

PiGPIOPin The problem occurs in worse shape than in the original case

Yeah, PiGPIO works by talking over a socket to the pigpiod daemon, which can sometimes slow things down.

NativePin In general, the best results.

Really? I'd expect it to be slower than RPi.GPIO shrug

After the reboot, IF I execute first RPi.GPIO, works very well, but, if tests after the NativePin use, slow. It's neccessary reboot to correctly speed.

Interesting... @waveform80 I wonder if this is related to the bcm2835_gpio_irq_handler stuff you mention in https://github.com/RPi-Distro/python-gpiozero/pull/141#issuecomment-181532090 ?

SrMouraSilva commented 7 years ago

However, I can say that the native implementation of version 1.2.0 (which I think is the RPi.GPIO) is slow.

Yes, GpioZero v1.3.1 still uses RPi.GPIO as the default Pin backend. But it's not the Pin backend that's making things slow, but the Event framework that @waveform80 has layered on top of it that is slowing down the detection of input-pin-changes. (which is what RPi-Distro/python-gpiozero#385 is looking at)

I implemented a "driver" for PCD8544 for Pi4j ... I try port it for gpiozero 3 months ago, but it's verry slow.

Looks like it should be possible to drive that display using SPI, and GpioZero has support for the Pi's hardware SPI interface (which is much faster than bit-banging) https://gpiozero.readthedocs.io/en/latest/api_spi.html

I will analyze this! Is a good idea!

I was recently discovered that the pin api in gpiozero. If you can tell me which is the fastest to work

The Pin API is currently going through another re-working RPi-Distro/python-gpiozero#459 But you shouldn't modify your code to use a specific Pin backend, but instead let the user of your script choose which backend to use by setting the GPIOZERO_PIN_FACTORY environment-variable.

You're right. I will have this care.

I do not understand what's going on, so I do not know what to simulate!

You mentioned that you weren't sure if it was your code that was going wrong, or your rotary encoder that was going wrong, so I was suggesting using an Arduino to "simulate" a rotary encoder (i.e. output the same pattern on the pins), so that you at least know the pin-changes are working reliably, to eliminate the possibly-dodgy rotary encoder ;-) (and by altering the timings, you can simulate a rotary encoder turning at different speeds)

That was a good plan! But as I discovered that the problem was while True: continue, I think it is not necessary to test.

I will add this tip list checks on bugs in adverse situations :)

Python is an interpreted language, not compiled. So creating unnecessary classes, having more function calls than are strictly needed, etc. can sometimes slow down the code.

Macho, isso é paia, ó.

I know it is interpreted and there are problems as well. Only he had tried to speak briefly. But it is at least strange language that trapped both readability "suggest" things like this (less functions).

As is gained on the one hand, it loses the other.

Please note that at the time of writing, RPIO is only compatible with Pi 1’s. Ok, I'm usign RPi2...

Yeah. The author started updating it, but then stopped again metachris/RPIO#77 (comment) :-(

A curiosity, you think of some way to support the wiring pi?

PiGPIOPin The problem occurs in worse shape than in the original case

Yeah, PiGPIO works by talking over a socket to the pigpiod daemon, which can sometimes slow things down.

True... but you can control other pins remotely is very cool! I think I was wrong, the gpiozero supports remote pinout as (almost) a whole! https://github.com/RPi-Distro/python-gpiozero/issues/434 :tada:

NativePin In general, the best results.

Really? I'd expect it to be slower than RPi.GPIO shrug

In fact, it works better with while True: continue with pause, my measurement criteria were guessing. Then I'll fix while there is time, I did not notice a difference. I'm sorry.

After the reboot, IF I execute first RPi.GPIO, works very well, but, if tests after the NativePin use, slow. It's neccessary reboot to correctly speed.

Interesting... @waveform80 I wonder if this is related to the bcm2835_gpio_irq_handler stuff you mention in RPi-Distro/python-gpiozero#141 (comment) ?

I do not understand you're asking me something or asking @waveform80. Anyway, I will test the RPi 3 to bring more details.

SrMouraSilva commented 7 years ago

I do not understand you're asking me something or asking @waveform80. Anyway, I will test the RPi 3 to bring more details.

In RPi 3 works - the problem was not detected. Maybe it's something to do with bcm2835_gpio_irq_handler since the RPi 3 is bcm2836.

lurch commented 7 years ago

I discovered that the problem was while True: continue

Yeah, that's just using a busy-loop to suck up all the CPU, so it's not surprising other things slowed down ;-) As you've already discovered, using signal.pause is the 'proper' way to do this. And if you really did want to busy-loop (?!?), it'd probably be 'better' to do while True: pass

But it is at least strange language that trapped both readability "suggest" things like this (less functions).

Well, it depends if you want fast code, or readable code. Sometimes both things aren't possible at the same time.

you think of some way to support the wiring pi?

I did already suggest that ( https://github.com/RPi-Distro/python-gpiozero/issues/180 ) but I guess @waveform80 decided it wasn't worth adding.

the gpiozero supports remote pinout as (almost) a whole!

That's only possible if you choose to use PiGPIIOPin as the backend. But yeah, it's great that the flexibility in GpioZero means that it's possible to switch out the Pin backend, and all the higher-level classes (LED, Button, Motor, etc). all work exactly the same!

I do not understand you're asking me something or asking @waveform80.

Sorry for the confusion, that was a question aimed at @waveform80 ;-)

the RPi 3 is bcm2836.

RPi3 is actually bcm2837 :)

BTW I've just looked at the table at the top of this page - and I'm not convinced by the +2 / -2 stuff. If both input pins have changed, then you know that the encoder has moved, but AFAICT it's impossible to tell in which direction it moved.

SrMouraSilva commented 7 years ago

Yeah, that's just using a busy-loop to suck up all the CPU, so it's not surprising other things slowed down ;-) As you've already discovered, using signal.pause is the 'proper' way to do this. And if you really did want to busy-loop (?!?), it'd probably be 'better' to do while True: pass

Thanks for your tip!

RPi3 is actually bcm2837 :)

Oops!

I did already suggest that ( RPi-Distro/python-gpiozero#180 ) but I guess @waveform80 decided it wasn't worth adding.

ok :confused:

BTW I've just looked at the table at the top of this page - and I'm not convinced by the +2 / -2 stuff. If both input pins have changed, then you know that the encoder has moved, but AFAICT it's impossible to tell in which direction it moved.

I have relied on https://github.com/PaulStoffregen/Encoder/blob/master/Encoder.h. I do not understand either. Maybe @PaulStoffregen can enlighten us.


Do you think Rotary Encoder would be a nice increase for gpiozero? If so, I will do a pull request, and you (gpiozero team) - when you have time - could inform what dislike. OK?

PaulStoffregen commented 7 years ago

Sorry, I can't help with Raspberry Pi.

lurch commented 7 years ago

Do you think Rotary Encoder would be a nice increase for gpiozero?

Definitely :-)

@PaulStoffregen We're just wondering how / why your code assumes a change in value of +2 or -2 when both input pins have changed. Surely if both pins have changed, the encoder could have moved in either direction?

waveform80 commented 7 years ago

First off - sorry I haven't looked at the rotary encoder stuff yet. I've noticed it on my notifications list over on the gpio-zero repo, but I just haven't got time to look into right now (I am definitely interested in supporting it, but right now I'm concentrating on getting the foundations right!)

I did already suggest that ( RPi-Distro/python-gpiozero#180 ) but I guess @waveform80 decided it wasn't worth adding.

I'm afraid it fell a bit off the radar while I was investigating pigpio. I do still think I should add it as a backend at some point, simply for the sake of those who want it for whatever reason. However, in terms of capability it's not going to give us much more than RPi.GPIO does already: as I understand it, wiringPi's PWM is also software driven (or more precisely: it can do hardware PWM, but it only does "real" hardware PWM, i.e. on one or two pins only which I don't think is as useful as the DMA-based PWM that RPIO and pigpio provide).

On the subject of using SPI for this (sorry if I've mis-understood, I'm just skimming the thread) "proper" support for remote SPI which alleviates the socket delays should be landing "real soon now".

And finally ...

NativePin

In general, the best results.

Huh?! That's utterly bizarre. NativePin is just some rubbish I threw together as a fallback for when nothing else was importable. It really is terrible, and I wouldn't seriously recommend people use it :)

After the reboot, IF I execute first RPi.GPIO, works very well, but, if tests after the NativePin use, slow. It's neccessary reboot to correctly speed.

Interesting... @waveform80 I wonder if this is related to the bcm2835_gpio_irq_handler stuff you mention in RPi-Distro/python-gpiozero#141 (comment) ?

Quite likely; I vaguely remember seeing similar effects when I was playing with NativePin. Causing that kernel mod distress seemed to affect several things including edge detection via epoll.

SrMouraSilva commented 7 years ago

@waveform80, a question.

NativePin In general, the best results. Really? I'd expect it to be slower than RPi.GPIO shrug

Huh?! That's utterly bizarre. NativePin is just some rubbish I threw together as a fallback for when nothing else was importable. It really is terrible, and I wouldn't seriously recommend people use it :)

In Internet, Joonas Pihlajamaa compare the gpio speed:

In your analysis, C (Native library) is the fastest and Shell /proc/mem access is the slowest penultimate.

The native implementation (https://github.com/RPi-Distro/python-gpiozero/blob/master/gpiozero/pins/native.py) uses what?

I'm needing to do SPI for 3 devices, via software is slow and via hardware could only use channel 1 with access to 2 devices.

I plan on using WiringPi only to bitbang (https://github.com/PedalPi/Physical/blob/276f3f3519db5c3b8c03fa354db7b722b7ba5ca9/component/pcd8544/wiring_bitbang.c)...

(I have no idea of the consequences of using together)

tarababa commented 7 years ago

Hi SrMouraSilva

As the rotary encoder classes are not yet available in a current release of GPIOZERO could I use them as they are in a project and if so under which license?

SrMouraSilva commented 7 years ago

@tarababa

Thank you for the contact! It really was not chosen a license for the project.

You can use the will, either commercially or for open-source purposes. The only thing I ask is not to use it for evil and, if possible, reference this project and GPIOZero - since the progress of inclusion in GPIOZero is a bit slow.

If a problem occurs in the use of the library, please open an issue!

SrMouraSilva commented 6 years ago

~Will be add in GPIOZero 1.5 https://github.com/RPi-Distro/python-gpiozero/pull/482~

EDIT: Support for the rotary encoder will be added in 1.5, though it does not mean that this implementation will be.

waveform80 commented 6 years ago

In Internet, Joonas Pihlajamaa compare the gpio speed:

http://codeandlife.com/2012/07/03/benchmarking-raspberry-pi-gpio-speed/
http://codeandlife.com/2015/03/25/raspberry-pi-2-vs-1-gpio-benchmark/

In your analysis, C (Native library) is the fastest and Shell /proc/mem access is the slowest penultimate.

The native implementation (https://github.com/RPi-Distro/python-gpiozero/blob/master/gpiozero/pins/native.py) uses what?

The native implementation bangs on the GPIO registers (in an mmap) like several of the C-based GPIO libraries. Because it avoids the file-system it's therefore "fast" but not as fast a C doing the same thing (i.e. the speed difference between it and C-based GPIO libraries is simply that native is pure python; the GPIO access technique is basically equivalent, but for edge detection).

I'll certainly look to get rotary encoder into 1.5. Hopefully we'll be investigating the schedule for that shortly but there's no dates just yet.