**Avoid** pinMode() and digitalWrite() in constructor()

[mgesteiro]

Hi there,

It seems to work but, are you aware that you should NOT use pinMode() or any other hardware related functions before the main setup() method is invoked (i.e. after the hidden main() is executed)? If you do so in the constructor (like in the examples) you are risking a malfunction depending on how the code is linked:

https://forum.arduino.cc/t/pinmode-in-class-constructor-seem-not-to-work/370583

A begin() or init() method called during setup() is the recommended way.

If you want, I can provide a PR for this: just let me know.

Best regards

[jasonacox]

Hi @mgesteiro ! Thanks for opening this issue. I understand what you are saying, but I believe that constraint was due to an older version of Arduino.

If you follow the guide to create a module for Arduino, you will end up with the constructor calling pinMode() before setup(), which is the recommended solution from Arduino now:

https://docs.arduino.cc/learn/contributions/arduino-creating-library-guide

Example in the Arduino guide:

Morse::Morse(int pin)
{
  pinMode(pin, OUTPUT);
  _pin = pin;
}

#include <Morse.h>

Morse morse(13);

void setup()
{
}

void loop()
{
  morse.dot(); morse.dot(); morse.dot();
  morse.dash(); morse.dash(); morse.dash();
  morse.dot(); morse.dot(); morse.dot();
  delay(3000);
}

I did a quick check this morning in case I'm missing something. Our TM1637TinyDisplay module seems to follow this guidance and is similar (and identical in the constructor example) to all of the other TM1637 modules out there.

Thanks for raising this. Please, let me know if I'm missing something.

[mgesteiro]

Thank you for taking the time to discuss this:

I tested your library before opening the issue and, as I commented, it seems to work as expected. But while reading the code (by pure chance!), I found the same problem that I encountered myself quite recently when calling hardware functions (analogRead() in my code) in a constructor, so I decided to report it to you.

In my case, although the code compiled perfectly, it wasn't working, and I was quite lost until I finally found this thread:

https://forum.arduino.cc/t/analogread-seems-not-working-into-a-class-constructor/109081/8

I know pretty well the guide you mention: I was following it while creating a library that took me precisely through this rabbit hole, but in my case, I was forced to move the code to an init() method, even though I was using the last version of Arduino tools.

Arduino documentation is not perfect (I mean, no documentation is...), and at this depth/level, I always take it with a pinch of salt: this is a very specific and obscure low level situation that may have passed under the radar for many reasons during a long time (years!). Maybe pinMode() works and only writing or reading from pins fail... I haven't dived so deep (yet).

I did a small experiment to prove my point: if you comment-out this four lines in the constructor()

...
  //pinMode(m_pinClk, INPUT);
  //pinMode(m_pinDIO, INPUT);
  //digitalWrite(m_pinClk, LOW);
  //digitalWrite(m_pinDIO, LOW);
...

the library still works! so ... is that code valid? is it [really] necessary? 🤷🏽‍♂️

About what other modules do (a.k.a. code reuse), well, I can't tell: I can only point what I discovered myself.

It's OK if you decide to leave the library as it is: if it works, don't touch it, right?. In any case, I was only trying to help, so feel free to close this issue if you want.

Best regards!

[jasonacox]

Thanks @mgesteiro . All that you say makes sense but isn't intuitive. I suspect most libraries have followed what the Arduino guide to creating a library outlines, and what you and I have done. To be fair, that approach is the most intuitive.

The beauty of Arduino development is the abstraction that handles the hardware. I'm wondering, instead of changing all of the libraries to accommodate, could we instead address how Arduino supports global declarations that initialize hardware? Calls made in a constructor that invoke hardware calls that need the main init() could trigger init() themselves since that is the first call in the main() function.

For our TM1637TinyDisplay project here, your "comment out" test is actually quite brilliant! Memory is a big issue for some of my projects so eliminating some of that code and those calls would be welcome. I did some quick research and the INPUT mode is the default so that is why that wouldn't be needed. The digitalWrite() calls may not be needed either. I'm going to research.

Thanks again for opening this. I also responded to the forum thread you mention to poke at this a bit more.

[mgesteiro]

Thanks @mgesteiro . All that you say makes sense but isn't intuitive. I suspect most libraries have followed what the Arduino guide to creating a library outlines, and what you and I have done. To be fair, that approach is the most intuitive.

Agree: that's how I got into the mess in the first place. Then it became worse: have you tried to debug code in a constructor using Serial? it's a nightmare and doesn't work most of the times. But makes sense because ... they're also hardware functions!

The beauty of Arduino development is the abstraction that handles the hardware. I'm wondering, instead of changing all of the libraries to accommodate, could we instead address how Arduino supports global declarations that initialize hardware? Calls made in a constructor that invoke hardware calls that need the main init() could trigger init() themselves since that is the first call in the main() function.

I don't expect such a big change happening anytime soon in the core: big impact with little benefit as, actually, already many many libraries have a begin(), init() or similar start-up function, i.e., the issue is well known (but poorly/not documented, at least that I could find).

For our TM1637TinyDisplay project here, your "comment out" test is actually quite brilliant! Memory is a big issue for some of my projects so eliminating some of that code and those calls would be welcome. I did some quick research and the INPUT mode is the default so that is why that wouldn't be needed. The digitalWrite() calls may not be needed either. I'm going to research.

Hope to see a new release soon then 🙂

Thanks again for opening this. I also responded to the forum thread you mention to poke at this a bit more.

Let's see what do we learn from this.

Best regards!

[jasonacox]

The more I study this issue, the worse it gets. 😲 There are classes in the core that are setting pinMode() in the constructor just like we are doing. Of course, others are setting it in begin() as we are discussing here.

• SoftwareSerial() runs pinMode() in the constructor (here)
• Serial.begin() doesn't set pinMode() (here)
• SPI.begin() sets pinMode() (here) - at least providing one good example

I wouldn't use those as examples (way too complex!!!) but I think it highlights the lack of consistency in the approach. At a minimum, it underscores how it isn't intuitive and if it is required, there is some cleanup needed, even in the core.

For reference for anyone else finding this thread, here is the ArduinoCore-avr init() and main()

@mgesteiro would you be willing to help me? I plan to update my libraries to honor the begin() approach (but want to do that without making any breaking changes). I think we can do that by just adding a begin() to the class and update the examples. Would you want to do that and submit a PR? I would like for you to get credit for discovering this. If not, I'll still reference you in the release notes. 😉

[mgesteiro]

The more I study this issue, the worse it gets. 😲 There are classes in the core that are setting pinMode() in the constructor just like we are doing. Of course, others are setting it in begin() as we are discussing here.

Fun, right!?!? 😅

I wouldn't use those as examples (way too complex!!!) but I think it highlights the lack of consistency in the approach. At a minimum, it underscores how it isn't intuitive and if it is required, there is some cleanup needed, even in the core.

Inconsistencies are all over the place, but it's natural in this kind of big/complex ecosystems. Honestly, I only got here because my code didn't work, if not, I would've missed it too.

For reference for anyone else finding this thread, here is the ArduinoCore-avr init() and main()

Nice references

@mgesteiro would you be willing to help me? I plan to update my libraries to honor the begin() approach (but want to do that without making any breaking changes). I think we can do that by just adding a begin() to the class and update the examples. Would you want to do that and submit a PR? I would like for you to get credit for discovering this. If not, I'll still reference you in the release notes. 😉

Sure! I'll send a PR.

Thanks for the productive discussion.