jasonacox / TM1637TinyDisplay

Arduino library to display numbers and text on a 4 and 6 digit 7-segment TM1637 display modules.
GNU Lesser General Public License v3.0
69 stars 19 forks source link

Updated library operation with new begin() method #29

Closed mgesteiro closed 1 year ago

mgesteiro commented 1 year ago

Updated library operation to include an initializing method begin() to move outside the constructor hardware related calls, as reported in https://github.com/jasonacox/TM1637TinyDisplay/issues/28

Worth mentioning this Arduino document about digital pins, as it's relevant to understand how the library operates with the communication line using pinMode() instead of digitalWrite() to drive it.

As requested, the offending calls where moved to begin() and ALL the examples updated. Keywords, Library properties and RELEASE were also updated accordingly.

I just have one display available (a Robotdyn's) and only Arduino boards to test it, so my checks are limited, but I'm pretty sure (after reading the code) that the begin() method call is, in reality, completely optional, as I somehow mention in the method docstring. That's also why I removed the clear()call from the method and put it at the top user-level. The good news here are that preexisting code will still work anyway.

Please, feel free to amend anything you want: I took the liberty to upgrade the release number to 1.8.0 as it is a significant change, but you decide.

Best regards!

jasonacox commented 1 year ago

Thank you @mgesteiro !!! 🙇

release number to 1.8.0

I agree. Good call. I'll review now and run tests against AVR and ESP devices driving 4 & 6 digit displays. If that looks good, I'll push the release.

jasonacox commented 1 year ago

Merged - starting hardware tests.

mgesteiro commented 1 year ago

NOTE: now that you've added the clear() call to the begin() method, remember to update the examples accordingly!

jasonacox commented 1 year ago

Test Results: PASS

I'll push the release for Ardunio to pick up v1.8.0 tonight.

Once again, thank you @mgesteiro !! 🙏

jasonacox commented 1 year ago

https://github.com/jasonacox/TM1637TinyDisplay/releases/tag/v1.8.0

mgesteiro commented 1 year ago

welcome! thanks to you also: it's a very useful library 👌

mgesteiro commented 1 year ago

One question: by default (at least my units behave this way) the TM1637 IC has a brightness level of 0 so, at the end, all my programs have this code in the setup():

display.begin();
display.setBrightness(BRIGHT_7);

i.e., making always two consecutive calls ...

So, shouldn't we include this call directly in our new begin() method the same way (and for the same reasons) we call clear()?

What do you think?

jasonacox commented 1 year ago

I agree! I won't have time to work on it until the weekend, but feel free to submit a PR if you can.

Thanks for your help and great suggestion.