Improved WS2812B support, fixed library import from 2 different trees, added demo to control neopixels with on/off and brightness control from tux touch pads by marcmerlin · Pull Request #32 · geekscape/aiko_engine_mp

marcmerlin · 2021-01-24T07:19:43Z

No description provided.

led.py: - had outdated import code, fixed comments - import configuration.main or File "<stdin>", line 1, in <module> File "examples/badge_np.py", line 41, in run File "/lib/aiko/led.py", line 142, in initialise AttributeError: 'module' object has no attribute 'main' - added length_get needed by demoe code - added write needed by demo code

marcmerlin · 2021-01-24T08:38:58Z

demo: https://youtu.be/hJYvMYOp11o

garthk

Yay demos!

garthk · 2021-01-27T08:22:41Z

lib/aiko/led.py

-# import aiko.led
-# aiko.led.initialise()
-# aiko.led.pixel(aiko.led.red, 0, True)
+# from lib.aiko import led
+# led.initialise()
+# led.pixel(aiko.led.red, 0, True)


Kinda fascinated… aiko.led is imported and initialised by main.py, so we could kinda eliminate those lines; what do we get from the switch from aiko.led.pixel to led.pixel?

yes and no, I ran this code from repl where it was not visible. There are some weird visibility issues between what's visible from repl and from code run by main.py.
I've had a lot of issues with led.dim being a different value when set from MQTT, or from code. I'm still trying to figure out why, it's very perplexing.
If you are experienced with micropython namespace issues, let me know and I'll share more details.

I'm not entirely clear on it either, it strikes me. I'd swear aiko is always imported on my REPL. Perhaps that's more to do the with limits of the soft reset inflicted by Control-D, though. (I've also noticed poor or zero reliability of MQTT hook-up after ^D, which I need to nut out.)

Ok, clear now: we've got the double import problem (see PR-level convo here) and when main.py runs it's doing so with the same globals as the REPL. So, straight after booting you can see aiko, application, common, configuration, plugins, and a bunch of others even though you didn't import them. ^D kicks it all off again.

garthk · 2021-01-27T08:24:26Z

lib/aiko/led.py

aiko.led.length works… sure, it's not at all encapsulated, but bytes are scarce, no?

yes, this was also to try namespace stuff, but this one is entirely useless, I removed it in my version.

marcmerlin · 2021-01-27T17:36:54Z

I'll push my updated version. If you have this working, please let me know if you can figure out why setting up dim values from MQTT, affects the brightness of leds also toggled via MQTT, but not when the LEDs are set by led.pixel(color, i, True)

led_py: Allow setting dimming from code and reading it back fill by default updates the strip, that's likely what's expected length_get was a test, removed added led:debug to see why dim has a different value from MQTT than when set by code badge_np: - added threading support so that it can be run from the main loop as an application - added color chase - added touch pad support (right tux) - Added toggling left tux LED form code

marcmerlin · 2021-01-27T18:01:33Z

@garthk please have a look at the latest version I just pushed.
I need to find out why set title isn't working (thankfully print over serial does), and more surprisingly why I get a different value for dim whether I set it from

I was discussing this with @geekscape : my current code outputs this:
loop3: 0.1
Dim: 0.1
Dim: 0.6
Dim: 0.1
loop1: 0.1

the middle 'dim 0.6' happens because I sent this MQTT:
sauron:# mosquitto_pub -h 101.181.46.180 -t public/esp32_X/0/in -m "(led:dim 0.6)"
sauron:# mosquitto_pub -h 101.181.46.180 -t public/esp32_X/0/in -m "(led:debug)"

so, dim has 2 values depending on where it's set from.

Interesting repl sees the version that is set by MQTT:

from lib.aiko import led; locals()["aiko"].led.dim;globals()["aiko"].led.dim;led.print_dim()
0.6
0.6
Dim: 0.1

Does this make sense to you Garth?

dim global variable in led.py was configured to have a separate value in the aiko net MQTT thread, and the main thread. This means that example code calling lib/led.py to set pixels, got a different dimming value than the same pixels set by the same led.py when they were set from the MQTT thread. Workaround is to store the dim value in configuration/main.py which is imported in memory before the threads happen, and that namespace is effectively shared between the different threads, so it can be used to share data. Using configuration/led.py would not work as it is not imported in memeory before aiko forks off the net/MQTT thread.

marcmerlin · 2021-01-28T19:22:14Z

ok, I managed to fix the namespace problem I could tell was there the entire time
@geekscape like I was smelling early on, the MQTT (aiko/net) thread imports aiko/lib/led.py and gets a different version than my code which also imports aiko/lib/net.py from the main thread.
As a result, both threads have a different version of the module with a different value of the dim global variable.
It is a micropython bug? I don't know. I'm assuming that threads that import modules after the thread was created, do not share the same memory and therefore values than if the import had been done before the thread (like is done for configuration/main.py)
See also the comment in c7ccdc0

marcmerlin · 2021-01-28T20:02:26Z

OMG, I couldn't get setting the title working either and I found out that

from lib.aiko import led,oled

is not the same as

from aiko import led,oled

They both work, but the first one creates a different namespace for all global variables, while he 2nd one does not and the global variables are shared between threads.
Is that an obscure bug, or WAI?

@geekscape I should add that the default files use both interchangeably, when mixing them, in fact introduces this very unclear bug I hit:
lib/aiko/led.py says: # from lib.aiko import led
lib/aiko/oled.py says: # import aiko.oled

Mixing both causes 2 different namespaces and the global variables aren't shared between access methods and background methods that actually update the hardware, causing massive confusion. Now that the very weird bug is found, no need to store 'dim' in configuration.main.dim Also update oled.py to fix the import and usage comments.

garthk · 2021-01-28T21:19:18Z

Oh, of course. sys.path is ['', '/lib']. You're getting literally two copies of the same module: one loaded via 'lib', the other via ''. For any module with code, it's being run twice.

garthk · 2021-01-28T21:32:49Z

Ha! Saved myself a bunch of time explaining it: it's already described as the double import trap in an article explaining how Python module resolution works.

If we remove '' from sys.path, we won't be able to reach applications, configuration, examples, plugins et al nor the built in or frozen modules. If we remove 'lib' it'll be messy… I think it's going to be easiest just not ever importing beginning with lib.

marcmerlin · 2021-01-29T03:06:54Z

Yes, thanks for confirming what too me way too long to find out. I can see sys.path being helpful, but here is made things much much worse. Even Andy used both forms, causing an unsafe mix, and ultimately bugs like the ones I ended up hitting.
Oh well, despite the price, I learned something about python :)

Front pad: brighter Rear pad: dimmer both pads: toggle on/off led.py: added reset_dim and change_dim.

marcmerlin · 2021-01-29T06:03:40Z

@garthk I think I fixed all the issues, it works as intended now.
Demo; https://www.youtube.com/watch?v=sNQQ8SSOUU0

Would you mind trying it out to see if it works for you?

@geekscape the code should be good for review and merge if it looks good to you

garthk

Haven't gone over the OLED stuff yet.

configuration/led.py

lib/aiko/led.py

Co-authored-by: Garth Kidd <garth@garthk.com>

lib/aiko/led.py

Co-authored-by: Garth Kidd <garth@garthk.com>

garthk

I'm going to stop bike-shedding, now. :)

marcmerlin · 2021-01-30T14:39:19Z

Thanks for the review. Let's now see what @geekscape says since I think he wants to personally review stuff like that before it goes in :)

marcmerlin · 2021-02-06T17:13:31Z

Hi @geekscape , how's life? :)

marcmerlin added 2 commits January 23, 2021 23:18

Allow pushing config file changes easily.

b9f4315

marcmerlin changed the title ~~Allow pushing config file changes easily.~~ Improved WS2812B support and added demo. Jan 24, 2021

marcmerlin added 2 commits January 24, 2021 00:41

Change neopixel pin to SAO 1, bottom pin.

6dfaa53

Merge branch 'master' of https://github.com/geekscape/aiko_engine_mp

495f3ee

marcmerlin changed the title ~~Improved WS2812B support and added demo.~~ WIP: Improved WS2812B support and added demo. Jan 25, 2021

garthk reviewed Jan 27, 2021

View reviewed changes

marcmerlin mentioned this pull request Jan 27, 2021

Move led module to ws2812 module #7

Open

marcmerlin added 2 commits January 27, 2021 20:54

Fixed python names for toggle_left_tux_led

8b8787e

Allow turning neopixels on and off using tux touchpads.

1956377

Front pad: brighter Rear pad: dimmer both pads: toggle on/off led.py: added reset_dim and change_dim.

marcmerlin changed the title ~~WIP: Improved WS2812B support and added demo.~~ Improved WS2812B support, fixed library import from 2 different trees, added demo to control neopixels with on/off and brightness control from tux touch pads Jan 29, 2021

garthk reviewed Jan 30, 2021

View reviewed changes

configuration/led.py Outdated Show resolved Hide resolved

lib/aiko/led.py Show resolved Hide resolved

lib/aiko/led.py Show resolved Hide resolved

lib/aiko/led.py Show resolved Hide resolved

lib/aiko/led.py Show resolved Hide resolved

Apply suggestions from code review

4fc9f46

Co-authored-by: Garth Kidd <garth@garthk.com>

garthk reviewed Jan 30, 2021

View reviewed changes

lib/aiko/led.py Outdated Show resolved Hide resolved

marcmerlin and others added 2 commits January 29, 2021 19:17

Update lib/aiko/led.py: shorten change_dim

3dbc34d

Co-authored-by: Garth Kidd <garth@garthk.com>

simplify set_dim/change_dim.

528bfa1

garthk approved these changes Jan 30, 2021

View reviewed changes

Conversation

marcmerlin commented Jan 24, 2021

Uh oh!

marcmerlin commented Jan 24, 2021

Uh oh!

garthk left a comment

Choose a reason for hiding this comment

Uh oh!

garthk Jan 27, 2021

Choose a reason for hiding this comment

Uh oh!

marcmerlin Jan 27, 2021

Choose a reason for hiding this comment

Uh oh!

garthk Jan 27, 2021

Choose a reason for hiding this comment

Uh oh!

garthk Jan 28, 2021

Choose a reason for hiding this comment

Uh oh!

garthk Jan 27, 2021

Choose a reason for hiding this comment

Uh oh!

marcmerlin Jan 27, 2021

Choose a reason for hiding this comment

Uh oh!

marcmerlin commented Jan 27, 2021

Uh oh!

marcmerlin commented Jan 27, 2021

Uh oh!

marcmerlin commented Jan 28, 2021

Uh oh!

marcmerlin commented Jan 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

garthk commented Jan 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

garthk commented Jan 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marcmerlin commented Jan 29, 2021

Uh oh!

marcmerlin commented Jan 29, 2021

Uh oh!

garthk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

garthk left a comment

Choose a reason for hiding this comment

Uh oh!

marcmerlin commented Jan 30, 2021

Uh oh!

marcmerlin commented Feb 6, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

marcmerlin commented Jan 28, 2021 •

edited

Loading

garthk commented Jan 28, 2021 •

edited

Loading

garthk commented Jan 28, 2021 •

edited

Loading