avishorp / TM1637

Arduino library for TM1637 (LED Driver)
GNU Lesser General Public License v3.0
427 stars 218 forks source link

pinMode instead digitalWrite #79

Open luwiack opened 3 years ago

luwiack commented 3 years ago

Hello, there were often strange wrong characters on the display. I compared it with the 6 digit version and I found a bug. Instead of digitalWrite, there's pinMode everywhere. I changed it. Now it is stably displayed, which I also noticed in the clock signal on the oscilloscope.

trr commented 3 years ago

The TM1637 is intended for use with pullups on the clock and data pins in open drain configuration, hence changing pinmode instead of digitalwrite. This is standard I2C practice. For low, master sets pin to output and pulls pin low. For high, MCU sets pin to input. This is how two way interaction (including ACK and clock stretching) can work on one wire.

It may well work with digitalwrite instead (eg, driving high for high states) but only if you don't rely on checking ACK (most of this library doesn't) and only if you don't mind your MCU periodically driving a pin high while the TM1637 is simultaneously driving the same pin to ground - probably not a great idea.

The garbled display is probably due to running too fast (delays too short) for the line capacitance and the pullup values, as open drain will have a slower rise time. If you can't correct this by running it slower then time to break out the multimeter or scope to check what's going wrong as it may be something electronic.

trr commented 3 years ago

Hmmm, just saw the other issues related to ESP32 implemention of pinmode. If you're on ESP32 I guess that could be issue. Could be an issue with pinmode function itself on that platform.

luwiack commented 3 years ago

Thank you for your reply. Now, please, can you imagine that someone for example a high school student, bought a 4-digit display and Arduino? Now he connected this and upload some of these examples and it does not work stable? What he can conclude? He bought a broken display or that he must calculate the capacitance of the lines?

PierreRossouw commented 3 years ago

I can confirm what luwiack is experiencing here. The current 'pinmode' code requires a bitdelay of at least 65 microseconds. In the .h file the default is set to be 100 microseconds. The TM1637 is capable of much better than that, and indeed it appears that digitalWrite is almost two orders of magnitude faster.

jernejp21 commented 2 years ago

Why don't use "wire.h" library and internal pull-ups? Or just internal pull-up and digital write. That way you don't have to worry if resistors are connected to circuit or not.

trr commented 2 years ago

The internal pull-ups are much weaker than the pull-ups that I2C recommends and may mean that time to go high is much longer, making communication are typical I2C speeds unreliable. Also, since we are discussing the TM1637, in my experience this has pull-ups at the device end anyway, and those are fine and suitable at least for 100kHz. The issue is not that pull ups are needed it's that if you are driving the data pin high at the MCU end you may occasionally do so while the device has it low.

PierreRossouw commented 2 years ago

What is the reason for choosing 100 microseconds as the default bitDelay value?

josefwegner commented 2 years ago

It seems that there is also an issue with ArduinoCore-mbed 2.6.1. With this latest version the output on a 4-digit TM1637 is garbled. Using an older version like 2.5.2 works fine.

I tested this on an Arduino Nano Connect and Raspberry Pi Pico.

josefwegner commented 2 years ago

This seems to be the commit that breaks the library on Arduino-mbed: https://github.com/arduino/ArduinoCore-mbed/commit/3f19f4906b53e2a3df03a142c03a6fb9d4f7e6d0 Not sure what the exact issue with the change is. My guess is that destroying the gpio objects takes too long and messes up the timing.