Security fixes, build-break fix, and best-practices fixes

[henrygab]

INCLUDES POTENTIAL SECURITY FIXES.

CHECKS IN A DEFAULT secrets.h FILE -- REVIEW REQUESTED. See comments.

I've not written any proof-of-concepts for the potential security issues. I've also not tested the resulting binary yet, so I'm marking as a "Draft" PR for now, to enable early review and comments.

• Fresh clone build break due to missing secrets.h file. Added the file; .gitignore already prevents changes from being tracked, and already makes it difficult to push this file. REVIEW OF LOCATION REQUESTED (currently sits next to .INO file).
• c_ExternalInput should be marked final, and its destructor marked non-virtual.
• Enum InputValue_t defines unused states. Remove shortOn and longOn.
• fsm_ExternalInput_state (FSM abstract class) has not declared a virtual destructor. Bad practice... Easy Fix.
• fsm_ExternalInput_*** (FSM implementation classes) should be marked final.
• fsm_ExternalInput_*** (FSM implementation classes) should declare functions with override.
• FSM abstract class virtual functions take a pointer (not a reference), but presume non-null values. Easy fix.
• Build Warnings: -Wformat-overflow=. Potential stack overflows. Fairly easy fix.
• c_ExternalInput should use reference (not pointer) for FSM member variable, as it is not valid for that to be a null pointer

[MartinMueller2003]

This change does not help the user. As with the secrets.h file, a user does not get a user.ini file by default and worse yet, there will be no compiler / linker error to tell them they forgot a step. At the moment creating a secrets.h file is an intentional requirement. Coding it this way makes adding this information optional, which it is not.

[henrygab]

I'm trying to understand your concerns.

There are no other references to a file similar to user.ini, and the readme.md doesn't give any other hints.

... a user does not get a user.ini file by default and worse yet, there will be no compiler / linker error to tell them they forgot a step.

Unlike what the sample file says, platformio_user.ini is actually optional for most users, who only have a single ESP device plugged in. Moreover, if a user actually does have multiple ESP devices, unless they have FTDI(*) chips, the COM ports can change with each boot, or even just by plugging them into the machine in a different order.

I also reviewed the code for any attempt to load a user.ini from flash, but came up empty. Can you help me understand what I missed about a user.ini file?

At the moment creating a secrets.h file is an intentional requirement. Coding it this way makes adding this information optional, which it is not.

ESPFlashTool.jar provides the supported method for setting WiFi username and password. This works for downloaded release builds, and also works for builds generated using github CI. Given it's the supported method, it would posit that using ESPFlashTool.jar should be used for at least a final test, if changes will be pushed to main (butterfly effect).

secrets.h is a separate mechanism, which allows overriding the username and password at build time. That's a great feature, and will save time for those using it.

Summarizing the disagreement:

• I believe a clean clone of the depot should build successfully, and preferably result in identical binaries to those that would be released from the same commit.
• You appear to believe a clean clone of the depot should fail to build, as a forcing function to require at least creation of a header file, to forcibly document how to hard-code the WiFi credentials.

For the above disagreement, I will defer to Shelby's decision, as we both appear to feel strongly.

(*) Not technically accurate, but close enough.

FTDI chips have serial numbers, and thus can be assigned a fixed COM port number.
This is not true for either the CH340 nor CP2104 chips ... which are much more common.
As for the ESP32-Cx and ESP32-Sx processors, they aren't supported in ESPixelStick yet.

Thus, while technically there are other times where a user could get a fixed COM port,
realistically most users won't, and will only have a single ESP device connected to their
machine at any given time. Thus, both upload and monitoring will just work, without the
need to specify any COM ports in a platformio_user.ini file.

Even then, I'd recommend the readme suggesting folks could just as easily use github
to build (via the CI checks), allowing them to upload using ESPFlashTool.