Refactor to include CLUE #23

kattni · 2020-02-19T16:13:52Z

THIS IS A BREAKING CHANGE. Import is no longer the same and the library no longer requires instantiation. It is now:

from adafruit_pybadger import pyadger

All code beyond import and instantiation should function the same if library was instantiated as pybadger. To use features, simply include them as pybadger.foo, e.g.:

print(pybadger.acceleration)
pybadger.show_qr_code()

Updates:

No new features have been added.
Board type is automatically detected and applicable features are imported.
Features that are not supported on a given board throw a clear error.

Tested on PyBadge/LC, PyGamer and CLUE.

@nnja Feel free to test this with your code.

merge badge changes into clue adaptation

ladyada

lgtm but did not test on hardware!

caternuson · 2020-02-19T16:28:01Z

Want to update the README.rst also as part of this PR?

kattni · 2020-02-19T16:28:59Z

@caternuson Yes. I missed that. The example? Or is there more that needs to be updated.

FoamyGuy · 2020-02-19T16:31:54Z

@kattni the sentence in the introduction section could mention CLUE along with pybadge and pygamers as devices the library is made to be used with.

kattni · 2020-02-19T16:33:08Z

@FoamyGuy Already fixed, thanks!

caternuson · 2020-02-19T17:14:11Z

EDIT yah like that ⬆️

@kattni what @FoamyGuy said.

And also the example :)

makermelissa · 2020-02-19T17:48:05Z

I've been trying out the customizations to make sure they're looking good and so far they are.

kattni · 2020-02-19T17:50:01Z

@makermelissa Thank you so much for thorough testing!

makermelissa · 2020-02-19T17:55:42Z

You're welcome. I know it didn't get tested as thoroughly the first time we did PyBadger.

makermelissa

Code looks good to me. Tested on PyBadge, EdgeBadge, CLUE, and PyGamer.

kattni · 2020-02-19T18:04:49Z

@FoamyGuy Made a valid point regarding leaving in the d-pad treatment of the joystick on PyGamer. He's going to test this refactor tonight with a game example he wrote for PyBadge/PyGamer and let me know if it's worth keeping in. I removed it as it seemed redundant, but it allows for programs using the d-pad functionality to work on PyBadge and PyGamer with no changes, something I had not considered. He will reply here to let me know, and I may add that back in, meaning it will need to be tested again on PyGamer.

@makermelissa I will ping you as well if I make the change.

makermelissa · 2020-02-19T18:24:57Z

@kattni, are you saying we would add the joystick d-pad functionality back in, and retain the analog joystick function? I hope that's what you're saying, as I think having both would be very useful.

tannewt

Looks great! Nice job!

kattni · 2020-02-19T18:50:01Z

@makermelissa Yes, both would remain in.

FoamyGuy · 2020-02-20T00:10:20Z

adafruit_pybadger/pygamer.py

+
+          from adafruit_pybadger import PyBadger
+
+          pybadger = PyBadger()


I think some of the comments still have the old style example usage instead of the new way:
from adafruit_pybadger import pybadger

pybadge and clue files have some as well this way.

Good catch! Thank you!

FoamyGuy · 2020-02-20T00:43:32Z

I do think it's worth having the joystick to D-pad buttons treatment.

I tested this out with my game example on EdgeBadge and PyGamer, and it does break with the current code on the PyGamer. Users could implement this logic in the application layer like inside my game, but I do think it's nicer to have one simple interface across the PyGamer and PyBadge devices so potential app and game developers don't have to worry about the difference if they aren't using the joystick in a more analog way.

kattni · 2020-02-20T00:48:47Z

@FoamyGuy Please test the changes I pushed. Thanks!

FoamyGuy · 2020-02-20T00:52:34Z

@kattni I'm getting this error on PyGamer now:

Traceback (most recent call last):
  File "code.py", line 504, in <module>
  File "code.py", line 372, in <module>
  File "/lib/adafruit_pybadger/pygamer.py", line 111, in button
NameError: name 'x' is not defined

kattni · 2020-02-20T00:53:33Z

@FoamyGuy I'll look into it.

FoamyGuy · 2020-02-20T00:57:19Z

@kattni took a peek myself as well. Looks like it needs this:

x, y = self.joystick

I added that back in and it's working now.

kattni · 2020-02-20T00:57:51Z

@FoamyGuy Already pushed :)

FoamyGuy · 2020-02-20T00:58:01Z

testing the new one now

FoamyGuy · 2020-02-20T00:59:51Z

@kattni working perfectly now :) thank you!

kattni · 2020-02-20T01:00:11Z

@makermelissa If you could test the update as well, I'd appreciate it. Thanks!

makermelissa · 2020-02-20T01:03:59Z

Sure thing @kattni

makermelissa · 2020-02-20T01:16:12Z

It seems to be working great.

kattni · 2020-02-20T01:23:15Z

@makermelissa Thank you very much!

nnja

One question and one minor nit, otherwise LGTM 👍

nnja · 2020-02-20T07:19:10Z

adafruit_pybadger/pybadge.py

+                                     digitalio.DigitalInOut(board.BUTTON_OUT),
+                                     digitalio.DigitalInOut(board.BUTTON_LATCH))
+
+        self._light_sensor = analogio.AnalogIn(board.A7)


Is there a way to check if the light sensor isn't present, like for the pybadge LC?

PyBadge LC has a light sensor. It's at the top, next to the reset button, with a hole over it to pass the front of the sensor through to the front of the board. From the back, it is white with a green symbol printed on it.

The lack of a light sensor would only be detectable in very specific situations (depending on whether the pin is connected to something else, has pull up or down, etc), and is a complicated process to do so. Knowing that the CLUE doesn't have the same light sensor, I included it in the unsupported list within the CLUE module. In the case of PyBadge vs. PyBadge LC, there's no way to tell the difference as they use the same CircuitPython build - there is no separate pin definition for PyBadge LC.

D'oh, I meant the accelerometer, not the light sensor.

If it's possible to auto detect that, then this line from the example could be modified or removed to make the experience on a PyBadge LC a little nicer.

Adafruit_CircuitPython_PyBadger/examples/pybadger_simpletest.py

Line 6 in bf25d8b

pybadger.auto_dim_display(delay=10) # Remove or comment out this line if you have the PyBadge LC

Try checking what pybadger.acceleration returns on the LC. I think it might end up as None on there but I don't have one to check with.

Along the same idea of improving the auto_dim: what if pressing any button would reset the dim timer. That would help all of the boards and allow for at least one type of auto_dim on the LC device.

adafruit_pybadger/pybadger_base.py

Updating https://github.com/adafruit/Adafruit_CircuitPython_HT16K33 to 3.2.2 from 3.2.1: > Merge pull request adafruit/Adafruit_CircuitPython_HT16K33#60 from geekguy-wy/adding_demos_and_tests Updating https://github.com/adafruit/Adafruit_CircuitPython_PyBadger to 2.0.0 from 1.1.1: > Merge pull request adafruit/Adafruit_CircuitPython_PyBadger#23 from kattni/clue-adaptation

kattni and others added 11 commits February 14, 2020 15:09

Badge and business card work on CLUE.

10f29fc

Basically working on CLUE and PyBadge LC.

d5cdc1e

Linting, comments.

928e009

Update example to use new import.

9d3d819

Appease Sphinx.

a6273ff

Appease Sphinx.

d426bd3

Automock all the things.

c5bf2a6

Still Sphinxing.

b78db48

SPHINX FINALLY BUILDS. Locally...

0aada9e

Merge remote-tracking branch 'ada/master' into clue-adaptation

07f6d9f

Merge pull request #1 from rhooper/clue-adaptation

bca9d76

merge badge changes into clue adaptation

kattni requested review from ladyada and makermelissa February 19, 2020 16:13

ladyada approved these changes Feb 19, 2020

View reviewed changes

Update README.

391f5d1

makermelissa approved these changes Feb 19, 2020

View reviewed changes

tannewt approved these changes Feb 19, 2020

View reviewed changes

FoamyGuy reviewed Feb 20, 2020

View reviewed changes

Add 'd-pad' to PyGamer, fix docs.

18489e7

PyGamer joystick fix.

778828f

nnja reviewed Feb 20, 2020

View reviewed changes

Create enable_speaker method.

ee0b46a

makermelissa merged commit bf25d8b into adafruit:master Feb 20, 2020

Refactor to include CLUE #23

Refactor to include CLUE #23

Uh oh!

Conversation

kattni commented Feb 19, 2020

Uh oh!

ladyada left a comment

Choose a reason for hiding this comment

Uh oh!

caternuson commented Feb 19, 2020

Uh oh!

kattni commented Feb 19, 2020

Uh oh!

FoamyGuy commented Feb 19, 2020

Uh oh!

kattni commented Feb 19, 2020

Uh oh!

caternuson commented Feb 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

makermelissa commented Feb 19, 2020

Uh oh!

kattni commented Feb 19, 2020

Uh oh!

makermelissa commented Feb 19, 2020

Uh oh!

makermelissa left a comment

Choose a reason for hiding this comment

Uh oh!

kattni commented Feb 19, 2020

Uh oh!

makermelissa commented Feb 19, 2020

Uh oh!

tannewt left a comment

Choose a reason for hiding this comment

Uh oh!

kattni commented Feb 19, 2020

Uh oh!

FoamyGuy Feb 20, 2020

Choose a reason for hiding this comment

Uh oh!

makermelissa Feb 20, 2020

Choose a reason for hiding this comment

Uh oh!

kattni Feb 20, 2020

Choose a reason for hiding this comment

Uh oh!

FoamyGuy commented Feb 20, 2020

Uh oh!

kattni commented Feb 20, 2020

Uh oh!

FoamyGuy commented Feb 20, 2020

Uh oh!

kattni commented Feb 20, 2020

Uh oh!

FoamyGuy commented Feb 20, 2020

Uh oh!

kattni commented Feb 20, 2020

Uh oh!

FoamyGuy commented Feb 20, 2020

Uh oh!

FoamyGuy commented Feb 20, 2020

Uh oh!

kattni commented Feb 20, 2020

Uh oh!

makermelissa commented Feb 20, 2020

Uh oh!

makermelissa commented Feb 20, 2020

Uh oh!

kattni commented Feb 20, 2020

Uh oh!

nnja left a comment

Choose a reason for hiding this comment

Uh oh!

nnja Feb 20, 2020

Choose a reason for hiding this comment

Uh oh!

kattni Feb 20, 2020

Choose a reason for hiding this comment

Uh oh!

caternuson commented Feb 19, 2020 •

edited

Loading

FoamyGuy Feb 22, 2020 •

edited

Loading