Add support for multiple chained boards #7

caternuson · 2018-11-15T22:39:27Z

PRing on behalf of forum poster:
https://forums.adafruit.com/viewtopic.php?f=60&t=140130&p=709433#p709428
Their code simply copy-pasted here.

brennen

I haven't tested on hardware, but code looks good and it's well-documented.

ladyada · 2018-11-16T17:11:20Z

hmm i would require a hardware test - even if its just 2 boards - since there's a bit of math going on here and its non-trivial changes... @brennen @kattni wanna check it next week sometime?

brennen · 2018-11-16T17:40:51Z

Yep, will do. Pretty sure I've got two boards here.

brennen

Something's off here, at least on CPython / the Pi. Doesn't seem to control pins 21-23 on the second board.

brennen · 2018-11-20T05:24:59Z

adafruit_tlc5947.py

@@ -50,6 +50,8 @@
 # access is by design for the internal class.
 #pylint: disable=protected-access

+_CHANNELS = 24
+_STOREBYTES = _CHANNELS + _CHANNELS/2


Suggested change

_STOREBYTES = _CHANNELS + _CHANNELS/2

_STOREBYTES = int(_CHANNELS + _CHANNELS/2)

...otherwise we wind up with a float here, at least on CPython.

Thank you for finding this one! I hoped to avoid this pitfall by rephrasing _STOREBYTES = _CHANNELS * 1.5 but forgot about Python 3 division rules. So here is my fix:

Suggested change

_STOREBYTES = _CHANNELS + _CHANNELS/2

_STOREBYTES = _CHANNELS + _CHANNELS//2

which I find less bulky, but also a bit less clear.

ArthurDent62

Had time to test my own code now. Sincere apologies for being so boastful to think I could do without.

Improvements to the library to allow for Python's negative index feature on the board. Changed example file because the last LED was on after each cycle.

ArthurDent62 · 2018-11-24T10:56:48Z

adafruit_tlc5947.py

        """
-        assert 0 <= key <= 23
+        assert 0 <= key < _CHANNELS * self._n


To allow for Python's negative indices (adressing elements from the rear end of an array), the suggested change is needed. Otherwise the negative index adressing near the end of tlc5947_chain.py would fail.

Suggested change

assert 0 <= key < _CHANNELS * self._n

if key < 0: # allow reverse adressing with negative index

key = key + _CHANNELS * self._n

assert 0 <= key < _CHANNELS * self._n

ArthurDent62 · 2018-11-24T11:00:40Z

adafruit_tlc5947.py

        If auto_write is enabled (the default) then the chip PWM state will
        immediately be updated too, otherwise you must call write to update
        the chip with the new PWM state.
        """
-        assert 0 <= key <= 23
+        assert 0 <= key < _CHANNELS * self._n


Same here - enable Python's negative array index feature:

Suggested change

assert 0 <= key < _CHANNELS * self._n

if key < 0: # allow reverse adressing with negative index

key = key + _CHANNELS * self._n

assert 0 <= key < _CHANNELS * self._n

ArthurDent62 · 2018-11-24T11:23:15Z

examples/tlc5947_chain.py

+# (note this is NOT the 12-bit value supported by the chip natively) and the
+# PWM channel will be updated.
+
+# With one RGB LED hooked up to pins 0, 1, and 2, AND


Simplify comment.

Suggested change

# With one RGB LED hooked up to pins 0, 1, and 2, AND

# Hook up one RGB LED to pins 0, 1, and 2, AND connect another RGB LED

# to pins 21, 22 and 23 of the last chained driver.

ArthurDent62 · 2018-11-24T11:23:54Z

examples/tlc5947_chain.py

+# PWM channel will be updated.
+
+# With one RGB LED hooked up to pins 0, 1, and 2, AND
+# with another RGB LED hooked up to pins 21, 22 and 23 of the last chained driver,


remove.

Suggested change

# with another RGB LED hooked up to pins 21, 22 and 23 of the last chained driver,

ArthurDent62 · 2018-11-24T11:25:08Z

examples/tlc5947_chain.py

+
+# With one RGB LED hooked up to pins 0, 1, and 2, AND
+# with another RGB LED hooked up to pins 21, 22 and 23 of the last chained driver,
+# cycle the red, green, and blue pins of LED one up and down, and the other vice versa:


move to below, and change to reflect code changes.

Suggested change

# cycle the red, green, and blue pins of LED one up and down, and the other vice versa:

ArthurDent62 · 2018-11-24T11:37:01Z

examples/tlc5947_chain.py

+while True:
+    for (pinA, pinZ) in ((redA, redZ), (greenA, greenZ), (blueA, blueZ)):
+        # Brighten:
+        print("LED A up, LED Z down")


With current code the second LED will stay lit after the cycle. Proposed change will brighten the LEDs one after the other, then dim them again, repeating for each channel:

Suggested change

print("LED A up, LED Z down")

print("LED A up")

ArthurDent62 · 2018-11-24T11:38:00Z

examples/tlc5947_chain.py

+        print("LED A up, LED Z down")
+        for pwm in range(start_pwm, end_pwm, step):
+            pinA.duty_cycle = pwm
+            pinZ.duty_cycle = end_pwm - pwm


remove.

Suggested change

pinZ.duty_cycle = end_pwm - pwm

ArthurDent62 · 2018-11-24T11:40:58Z

examples/tlc5947_chain.py

+            pinA.duty_cycle = pwm
+            pinZ.duty_cycle = end_pwm - pwm
+#            tlc5947.write()        # see NOTE below
+


now brighten the LED on the last pins of the last driver board:

Suggested change

print("LED Z up")

for pwm in range(start_pwm, end_pwm, step):

pinZ.duty_cycle = pwm

# tlc5947.write() # see NOTE below

ArthurDent62 · 2018-11-24T11:42:02Z

examples/tlc5947_chain.py

+#            tlc5947.write()        # see NOTE below
+
+        # Dim:
+        print("LED A down, LED Z up")


dimming both LEDs now:

Suggested change

print("LED A down, LED Z up")

print("LED A and LED Z down")

ArthurDent62 · 2018-11-24T11:43:37Z

examples/tlc5947_chain.py

+        print("LED A down, LED Z up")
+        for pwm in range(end_pwm, start_pwm, 0 - step):
+            pinA.duty_cycle = pwm
+            pinZ.duty_cycle = end_pwm - pwm


just do the same with both LEDs:

Suggested change

pinZ.duty_cycle = end_pwm - pwm

pinZ.duty_cycle = pwm

caternuson · 2018-11-25T01:15:54Z

This is where me in the middle kind of falls apart.

@ArthurDent62 Do you feel like you could PR this yourself at this point?

ArthurDent62 · 2018-11-25T20:27:24Z

@caternuson Well, see me convinced. The multiple edit sure was over the top. I have opened a new PR with all the related changes included. I've also extended the chain example to test all the available channels one by one. So you could delete this PR when you find mine. Thanks!

kattni · 2018-11-25T20:55:07Z

Closing in favor of #8

add support for multiple chained boards

465fb2b

caternuson requested a review from a team November 15, 2018 22:39

a few lint fixes

7ca3e00

brennen approved these changes Nov 16, 2018

View reviewed changes

brennen self-assigned this Nov 16, 2018

kattni mentioned this pull request Nov 20, 2018

CircuitPython Libraries! adafruit/circuitpython#1246

Open

brennen suggested changes Nov 20, 2018

View reviewed changes

ArthurDent62 suggested changes Nov 24, 2018

View reviewed changes

kattni closed this Nov 25, 2018

	_STOREBYTES = _CHANNELS + _CHANNELS/2
	_STOREBYTES = int(_CHANNELS + _CHANNELS/2)

	_STOREBYTES = _CHANNELS + _CHANNELS/2
	_STOREBYTES = _CHANNELS + _CHANNELS//2

	# With one RGB LED hooked up to pins 0, 1, and 2, AND
	# Hook up one RGB LED to pins 0, 1, and 2, AND connect another RGB LED
	# to pins 21, 22 and 23 of the last chained driver.

Add support for multiple chained boards #7

Add support for multiple chained boards #7

Uh oh!

Conversation

caternuson commented Nov 15, 2018

Uh oh!

brennen left a comment

Choose a reason for hiding this comment

Uh oh!

ladyada commented Nov 16, 2018

Uh oh!

brennen commented Nov 16, 2018

Uh oh!

brennen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArthurDent62 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

caternuson commented Nov 25, 2018

Uh oh!

ArthurDent62 commented Nov 25, 2018

Uh oh!

kattni commented Nov 25, 2018

Uh oh!

Uh oh!