RotaryEncoder based on gpiozero #763

veloxidSchweiz · 2020-01-31T11:09:43Z

Hey,
As already mentioned once, I think it would be great to build up a configurable gpio_control using configfiles.
After some different ways of implementation, here is a rough idea how this could look like later.
What I am still want to improve, is the way how the functionCalls are connected with the buttons, especially when one wants to use a oled display as well. Here I am thinking about a way that one can register functioncalls and then can be used later on. This will be one of the next steps.
At the moment I do not want to have this branch merged.
I just want to have a Feedback what you think about this structure....
In order to simplify testing I am using a different implemntation of the RotaryEncoder using gpiozero. Using only one gpio interface is in my opinion prefarable. (I actually hope/think that this implementation will be integrated in the gpiozero class sooner or later)

WOuld be glad about any kind of feedback

new gpio_control add gpio_settings.ini in settings

s-martin · 2020-01-31T11:53:54Z

I like the clear separation, this could make maintenance easier in the future.

General question: should the Python tests be located in ./scripts/tests, because there's already a directory ./test (which contains only some shell tests)?

Additionally the defined Python tests could be used in GitHub Actions, so tests are executed at every commit. See https://github.com/MiczFlor/RPi-Jukebox-RFID/blob/develop/.github/workflows/pythonpackage.yml

s-martin · 2020-01-31T11:55:49Z

With the separation of the settings from the py file, could renaming of gpio-buttons py.sample to py during installation also be dropped?

s-martin · 2020-01-31T12:08:27Z

This PR should probably be synchronized with PR #387

MiczFlor · 2020-01-31T14:09:36Z

Hi @veloxidSchweiz @s-martin @ZyanKLee
I am not sure what the best way is to move forward with the Phoniebox code base. @ZyanKLee in some thread suggested to build precompiled images :) also an approach.
There seem to be two possible routes and I would like your feedback

a) the Apple way

make two or three version with hardware specs (let's say: 1. USB only, 2. Common soldering RFID reader and a few arcade buttons, 3. rotary encoder, arcade buttons, display, the works)
provide rock solid installs for these three versions and possibly each with and without Spotify
everything else is not "supported"

b) the Android way

build a configurable core code base
open up for different configurations and create a community space (like LineageOS) for all kinds of derivates of Phonieboxes.

I think (b) is kind of happening elsewere, but with no config / API standards. @veloxidSchweiz suggested something here for GPIO in yaml style.
Having been around the block a few years in web development, I remember co-working spaces which collected old mobile phones to have a testing lab for mobile sites on all possible devices. It was hell :)
This makes me weary of the (b) option, because it will require the API / config concept to be tested against all kind of hardware fruit salads

Option (a) might be an easier way to separate the entry level Phonieboxers from the experts.

Am I missing more options? Please share.

veloxidSchweiz · 2020-01-31T16:11:59Z

Hi @MiczFlor, @s-martin
Thank you for your comments.
I am personally a fan of high test coverage with automated tests, i.e. ensuring that the stuff you wrote is properly tested and you have a high confidence that nothing will break.
With having everywhere the same code and using only different configuration files, the code is used more, there is no code duplication.
I don't think you need to do system tests for all every hardware of the fruit salad. But testing each functionality and the deployment would be enough.
At the end you have two to three different config files there which are there and installed automatically. i.e. this configurations are prepared and are distributed/installed automatically, when you choose one of the 'standard' versions. As soon as you want to have something special you can point to a single location.
So i actually (a) and (b) are not directly disagreeing with each other and we can benefit from a configurable system..

P.S.: What definitely makes sense is to check how good the different modules are actually tested. if one for examples looks in the code of gpiozero, one sees that this has many unittests which also gives a certain confidence that one can work with that module.

martinclausen2 · 2020-01-31T19:59:33Z

Hi @veloxidSchweiz a year ago I removed carefully gpio.zero for performance reasons:
#387 (comment)
You might also notice the some what unusual encoder decoding table I used: It actually prevents to cause many +1 / -1 actions to be triggered, if the encodes is in-stable between two states. Since we have this long python shell script mpc mpd pipeline, to me it seems important.
I like the configuration a lot! Great!

s-martin · 2020-01-31T20:15:29Z

I think the strength of this project is that is highly configurable and limiting it to only a few configurations would limit the potential.

I think, if we get some automated testing done, then stability would improve.

MiczFlor · 2020-01-31T23:07:34Z

If we think bigger, not just GPIO, then a configuration would also need to take into account different soundcards (which in the past have often proven to be the reason for sudden GPIO problems, since they take up GPIO pins at times used in some of the existing scripts).
With the soundcards, also the possibility to limit maximum volume came into play. Some sound cards would blast the windows out of their frames in the kids room, if given the chance :)
with this, also the steps by which the web app and/or RFID cards can up or down the volume would change.
I am not thinking this through right now, just pointing towards the fact that: once we enter the road of config files, additional question would come up, related to the hardware line-up. I did plan on doing something like this. If you run the install script, it will write the config to a file, as you might have noticed. My thoughts at the time: having different configs for different devices.
Given the current state of the project, the mess is at a point where everybody upgrading at the moment will end up with a dead box :) it seems that way, at least.
I think for a configurable solution we should start a new branch, call it version 3.x or the like.
First and foremost, we should fix version 2.x
And I will create a tarball for a working 1.9.x version installer - which then would not work with Spotify, obviously.

veloxidSchweiz · 2020-02-02T10:59:42Z

I agree on that, and by having standard configurations prepared we actually have the

Hi @veloxidSchweiz a year ago I removed carefully gpio.zero for performance reasons:
#387 (comment)
You might also notice the some what unusual encoder decoding table I used: It actually prevents to cause many +1 / -1 actions to be triggered, if the encodes is in-stable between two states. Since we have this long python shell script mpc mpd pipeline, to me it seems important.
I like the configuration a lot! Great!

Hi @martinclausen2 ,
I am trying to understand your decoding table of the rotary encoder. could you explain it a little bit more in detail? Do you only count full turns and look at the speed you turned it?

start pytests for RotaryEncoder

s-martin

I

s-martin · 2020-02-02T12:08:22Z

scripts/gpio_control.py

+import configparser
+import logging
+
+import mock


Is this import necessary?

s-martin · 2020-02-02T12:11:17Z

scripts/RotaryEncoder.py

+#!/usr/bin/python3
+# rotary volume knob
+# these files belong all together:
+# RPi-Jukebox-RFID/scripts/rotary-encoder.py


Having files RotaryEncoder.py and rotary-encoder.py may be confusing or do you intend to merge these files?

veloxidSchweiz · 2020-02-02T13:43:27Z

Hi @s-martin,
My intention of this PR was more to discuss the basic ideas, and not yet all details of the implementation. I would definietly cleanup the encoderfiles, as this concept would end up in having only one gpio_control.py .
Before beeing changed for a 'real' review, I would like to do a review for my self.

new gpio_control add gpio_settings.ini in settings

start pytests for RotaryEncoder

…/veloxidSchweiz/RPi-Jukebox-RFID into feature/configureableGPIOButtons

martinclausen2 · 2020-02-03T21:08:34Z

I agree on that, and by having standard configurations prepared we actually have the

Hi @veloxidSchweiz a year ago I removed carefully gpio.zero for performance reasons:
#387 (comment)
You might also notice the some what unusual encoder decoding table I used: It actually prevents to cause many +1 / -1 actions to be triggered, if the encodes is in-stable between two states. Since we have this long python shell script mpc mpd pipeline, to me it seems important.
I like the configuration a lot! Great!

Hi @martinclausen2 ,
I am trying to understand your decoding table of the rotary encoder. could you explain it a little bit more in detail? Do you only count full turns and look at the speed you turned it?

The state machine tracks more than just the current state (=>more than one transition of the A and B output) and can thereby avoid to issue commands if the encoder just flickers back an forth.
So it still can get the full resolution of the encoder. If you have a encoder with dents, the table is only correct for encoders that have the same number of dents and pulses. If you have more dents then pulses, you would need a different table or go more then one dent per e.g. track.

The code also uses the speed for a feature that I call acceleration: The faster you turn the more the output variable is changed per pulse from the encoder.
This can be set more aggressive for volume than for tracks.

…s' into feature/configureableGPIOButtons # Conflicts: # components/gpio_control/gpio_control.py

…r rotary_encoders

components/gpio_control/example_configs/phoniebox_gpio_control.service

MiczFlor · 2020-04-14T09:36:31Z

Hi @veloxidSchweiz @ZyanKLee @s-martin @martinclausen2
I will merge this to the develop branch. So people can play with it. Does that make sense?

RotaryEncoder based on gpiozero

6814a82

new gpio_control add gpio_settings.ini in settings

martinclausen2 mentioned this pull request Jan 31, 2020

Simple powered up led #387

Merged

removing gpiozero dependencies and switching to GPIO.RPIO,

f18d7e8

start pytests for RotaryEncoder

s-martin reviewed Feb 2, 2020

View reviewed changes

felb-dectris and others added 2 commits February 2, 2020 14:45

RotaryEncoder based on gpiozero

b59b8a7

new gpio_control add gpio_settings.ini in settings

removing gpiozero dependencies and switching to GPIO.RPIO,

b10d64f

start pytests for RotaryEncoder

veloxidSchweiz force-pushed the feature/configureableGPIOButtons branch from f18d7e8 to b10d64f Compare February 2, 2020 14:21

veloxid added 2 commits February 2, 2020 15:22

Merge branch 'feature/configureableGPIOButtons' of https://github.com…

432ddb0

…/veloxidSchweiz/RPi-Jukebox-RFID into feature/configureableGPIOButtons

refactoring

4181137

ZyanKLee mentioned this pull request Feb 2, 2020

Discussion: Future of Phoniebox #774

Closed

veloxid and others added 6 commits February 2, 2020 19:20

aded SimpleButton as a replacement of the gpiozero functionality

7ab54da

separate TwoButtonControl to separate file

ce5509b

refactoring

a04194e

bugfix: add_event_detection does not have argument channel but gpio

ace098e

set GPIO mode to BCM

ff35752

Merge branch 'feature/configureableGPIOButtons' of https://github.com…

3a1eb4b

…/veloxidSchweiz/RPi-Jukebox-RFID into feature/configureableGPIOButtons

veloxidSchweiz added 3 commits February 3, 2020 22:09

minor adjustments after testing with actual buttons

3240b59

bugfix

116b639

ifixing test

36dfa8c

veloxidSchweiz and others added 16 commits February 23, 2020 10:42

remove print statement replace by logger.debug

3914bf5

add del function, changed logging

bf975dc

change config file location, updated logging

51e51b7

logging output

1cfd5db

argument in check_call

539179b

fixing tests

60c0a9f

fixing tests

f491138

initial commit of README.md

3c3c5d6

add gpio_control.service

2dbbb7a

Move devices in subfolder

7c04481

Merge remote-tracking branch 'veloxid/feature/configureableGPIOButton…

44c34fb

…s' into feature/configureableGPIOButtons # Conflicts: # components/gpio_control/gpio_control.py

add __init__.py for GPIODevices to simplify the GPIODevice imports

02a270d

making two_buttoncontrol work

1b6f8ce

fixed name of gpio_control.py

75e233b

change check_call command to Popen, as this allows better reaction fo…

6240cf7

…r rotary_encoders

working on install script

5036995

s-martin reviewed Mar 4, 2020

View reviewed changes

components/gpio_control/example_configs/phoniebox_gpio_control.service Show resolved Hide resolved

add shebang for gpio_control.py

b882479

s-martin added the enhancement label Mar 8, 2020

s-martin mentioned this pull request Apr 12, 2020

Configuration via one settings file #589

Closed

MiczFlor marked this pull request as ready for review April 14, 2020 09:36

MiczFlor merged commit 64a42f1 into MiczFlor:develop Apr 14, 2020

s-martin mentioned this pull request Apr 16, 2020

Add Testing article to the wiki #904

Closed

3 tasks

s-martin modified the milestones: 3.0, 2.0 May 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RotaryEncoder based on gpiozero #763

RotaryEncoder based on gpiozero #763

veloxidSchweiz commented Jan 31, 2020

s-martin commented Jan 31, 2020

s-martin commented Jan 31, 2020

s-martin commented Jan 31, 2020

MiczFlor commented Jan 31, 2020 •

edited

Loading

veloxidSchweiz commented Jan 31, 2020 •

edited

Loading

martinclausen2 commented Jan 31, 2020 •

edited

Loading

s-martin commented Jan 31, 2020

MiczFlor commented Jan 31, 2020

veloxidSchweiz commented Feb 2, 2020

s-martin left a comment

s-martin Feb 2, 2020

s-martin Feb 2, 2020

veloxidSchweiz commented Feb 2, 2020

martinclausen2 commented Feb 3, 2020

MiczFlor commented Apr 14, 2020

RotaryEncoder based on gpiozero #763

RotaryEncoder based on gpiozero #763

Conversation

veloxidSchweiz commented Jan 31, 2020

s-martin commented Jan 31, 2020

s-martin commented Jan 31, 2020

s-martin commented Jan 31, 2020

MiczFlor commented Jan 31, 2020 • edited Loading

veloxidSchweiz commented Jan 31, 2020 • edited Loading

martinclausen2 commented Jan 31, 2020 • edited Loading

s-martin commented Jan 31, 2020

MiczFlor commented Jan 31, 2020

veloxidSchweiz commented Feb 2, 2020

s-martin left a comment

Choose a reason for hiding this comment

s-martin Feb 2, 2020

Choose a reason for hiding this comment

s-martin Feb 2, 2020

Choose a reason for hiding this comment

veloxidSchweiz commented Feb 2, 2020

martinclausen2 commented Feb 3, 2020

MiczFlor commented Apr 14, 2020

MiczFlor commented Jan 31, 2020 •

edited

Loading

veloxidSchweiz commented Jan 31, 2020 •

edited

Loading

martinclausen2 commented Jan 31, 2020 •

edited

Loading