adafruit / Adafruit_TLC5947

Adafruit's Arduino driver for the TLC5947
http://www.adafruit.com/products/1429
Other
40 stars 22 forks source link

Constant NUM_TLC5974 is wrong #12

Open AlanGitMan opened 1 year ago

AlanGitMan commented 1 year ago

Picky bug, but nonetheless here we go! In the example code the device name is wrong in the NUM_TLC5974 declaration and usages throughout. It should of course be NUM_TLC5947

Also, I haven't been able to find any info in the code or the docs about when and for what you would use the OE input to the board - or not.

caternuson commented 1 year ago

Yah, looks like a typo. Luckily just cosmetic, since it's a constant and the code at least is consistent with the usage. Would you be willing to make a PR that fixes the typo?

The pin labeled OE is tied to the BLANK pin on the TLC5947. Here's the description from the datasheet: image That just needs to be attached to any available GPIO pin, and can then be controlled in user code with digitalWrite

AlanGitMan commented 1 year ago

Hi Carter,

That’s fine. I’ll do a pull request on this evening or tomorrow morning.

The OE pin I think needs more explanation. Essentially, if as suggested in the library code you leave it floating and don’t define an Arduino code to control it then during Arduino resets and updates the LEDs you have connected to the TLC5947 will blink and flash randomly cos the Arduino pins have all gone tri-state temporarily. This is not acceptable in some applications (mine is one) and so I think we need to document that:

It is not essential, but if you need to avoid LEDs randomly blinking during Arduino reset or update you need to connect the TLC5947 board’s OE (Outputs Enable) pin to a spare pin on your Arduino. That Arduino pin also needs to be connected to the +5V pin on the Arduino. This will actively pull OE high during Arduino update or reset, which will ensure that all LEDs remain off during those times. You need to change the OE pin designation in the supplied library example code from “-1” to whatever Arduino pin you decide to use (e.g. “11”).

With those changes made, the OE pin on the board stays at HIGH when the Arduino is not running the code, which keeps all LEDs dark. However, when the code runs, it sets the pin LOW which overcomes the resistor and once again allows the TLC5947 outputs to turn on LEDs under control of the program.

Best regards

Alan T

From: Carter Nelson @.> Sent: 15 January 2023 16:18 To: adafruit/Adafruit_TLC5947 @.> Cc: AlanGitMan @.>; Author @.> Subject: Re: [adafruit/Adafruit_TLC5947] Constant NUM_TLC5974 is wrong (Issue #12)

Yah, looks like a typo. Luckily just cosmetic, since it's a constant and the code at least is consistent with the usage. Would you be willing to make a PR that fixes the typo?

The pin labeled OE is tied to the BLANK pin on the TLC5974. Here's the description from the datasheet: https://user-images.githubusercontent.com/8755041/212552556-f08b0e1b-b61e-472c-a95a-e4b47195a751.png That just needs to be attached to any available GPIO pin, and can then be controlled in user code with digitalWrite

— Reply to this email directly, view it on GitHub https://github.com/adafruit/Adafruit_TLC5947/issues/12#issuecomment-1383191209 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ACISC7NYGBTNB7VTLQQ7MKDWSQPKTANCNFSM6AAAAAAT35MUOE . You are receiving this because you authored the thread.Message ID: @.***>

caternuson commented 1 year ago

There's nothing currently in the library that deals with the OE pin in any way. It's only shown in the example sketch - that's not library code. To make things generally easier to use, the TLC9547 has a pull down resistor on the BLANK (OE) pin so the default is always on. That way, using the OE pin is optional.

However, that does not fix the general issue you are describing, since it requires actively driving whatever pin on the microcontroller the OE pin is attached to. That's trivial code - but the code must be running. So during resets, the behavior can be application dependent. It sounds like you'd prefer to have the pull resistor be up instead of down so that the TLC5947 is default off. And thus requires user code to drive the OE pin to enable.

AlanGitMan commented 1 year ago

Hi Carter,

Okay done that now (I think!) pull request complete.

Your summation of the OE pin issue is correct – the on-board pull down resistor makes it default to Enable-On – even when the MCU is not running code. That is not the behaviour I need for my project and my proposed fix (i.e. the resistor) can fix that. I suspect that there would be other users of the board who’d prefer it defaulted to Enable-Off in the absence of an MCU signal – for example if I had a chain of boards during no-MCU times the LEDs attached to them all could possibly all come on at full and (depending on what power supply I was using) overload the power bus. Yes, I know the PSU should have enough headroom to drive them all, but in the absence of any useful documentation about this, I expect someone with no experience might miss that and provide just enough for what their code lights up.

I’m happy to let the OE thing go, it’s kind of spilt milk really as this is not a new Ada product – but if you guys are planning any updates or new versions of the board maybe a jumper arrangement allowing a choice of pull-up or pull-down might be a good option to suggest? Plus of course some guidance in any updated docs on why to use one or other.

One other thing. I have been trying to use the TLC library with an ESP32-WROOM but no joy. I found the Ada forum discussion on this (now locked) but – although ESP32 gets a mention on there – the people making replies all seem to be using 8266 and not ESP32. I tried using GPIO pins 2,4 and 5 for the SPI pins as suggested there but no luck and I also tried GPIO 25-27 – same thing. In both cases the debug stream out through the USB is completely garbled so I am guessing maybe the library is using a timer which ESP32 (but not 8266) uses for baud rate? I tried the ESP32 with just a simple sketch to loop around doing a “Serial.println(x++)” where x is a globally declared int and that works fine, so the hardware side is not the problem. Any ideas?

Thanks for your help. Best regards

Alan T

From: Carter Nelson @.> Sent: 16 January 2023 15:53 To: adafruit/Adafruit_TLC5947 @.> Cc: AlanGitMan @.>; Author @.> Subject: Re: [adafruit/Adafruit_TLC5947] Constant NUM_TLC5974 is wrong (Issue #12)

There's nothing currently in the library that deals with the OE pin in any way. It's only shown in the example sketch - that's not library code. To make things generally easier to use, the TLC9547 has a pull down resistor on the BLANK (OE) pin so the default is always on. That way, using the OE pin is optional.

However, that does not fix the general issue you are describing, since it requires actively driving whatever pin on the microcontroller the OE pin is attached to. That's trivial code - but the code must be running. So during resets, the behavior can be application dependent. It sounds like you'd prefer to have the pull resistor be up instead of down so that the TLC5947 is default off. And thus requires user code to drive the OE pin to enable.

— Reply to this email directly, view it on GitHub https://github.com/adafruit/Adafruit_TLC5947/issues/12#issuecomment-1384241906 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ACISC7KY75NTNHJVSDLHHS3WSVVFNANCNFSM6AAAAAAT35MUOE . You are receiving this because you authored the thread.Message ID: @.***>

caternuson commented 1 year ago

The OE thing is a design trade off, and more of a hardware thing. You could open an issue in the PCB repo: https://github.com/adafruit/Adafruit-TLC5947-PCB that brings this up. That way it might be considered for a future rev of the board. Like maybe solder pad jumpers that would allow configuring the pull resistor to go either way?

The ESP32 issue should be dealt with in a new issue or forum post. A forum post would be preferred as a starting point, along with photos of the setup and a simple example sketch that can demonstrate the issue.