Change #ifdef to #if to allow external configuration of the library

[dzindra]

This change allows user to customize library settings without changing the source code of the library itself. This is useful for example in PlatformIO IDE when using this library as a dependency.

You can place following snippet into your platformio.ini do disable/enable certain parts of the library:

build_flags=
  -DM5EZ_WPS=0
  -DM5EZ_WIFI=0
  -DM5EZ_BATTERY=1
  -DM5EZ_BLE=0
  -DM5EZ_WIFI_DEBUG=0
  -DM5EZ_BACKLIGHT=1
  -DM5EZ_CLOCK=0
  -DM5EZ_FACES=0

[paultech]

This is a breaking change and I don't believe it hits the intended goal.

Currently define guards are set with no value.
#define M5EZ_WIFI

#if requires a value to pass, otherwise, both #if and #ifdef are functional the same.

What is most likely happening is PlatformIO/Arduino IDE is not including the library as a compilation unit so no changes at your projects compile-time affect the already compiled binaries.

The solution most Arduino libraries use is to have a config.h and/or user_config.h file that users can modify to change preprocessor defines that forces a recompilation.

You can confirm there is no functional change, with no edits to M5Ez to force a recompilation; changes to your build_flags won't have any effect.

[dzindra]

Currently define guards are set with no value.
#define M5EZ_WIFI

This is exactly the problem - once the M5EZ_ setting is defined, there is no way how to change it/undefine it without editing the source code of the library itself when using #ifdef checks, because #ifdef evaluates to true even if macro has no value. On the other hand #if evaluates to true only if macro has nonzero value.

My PR checks if M5EZ_ macros are defined before setting them to their default values. If they are already defined it doesn't touch them anymore so you can easily enable/disable library features. You can of course change default values by editing the M5ez.h file directly, but if you are using PlatformIO and including the library via lib_deps directive in platformio.ini, it is installed automatically into local hidden directory and can be overwritten by library manager (so if you change the M5ez.h file, your changes are lost and project build fails). And #define used in build_flags will also affect compilation of M5ez.cpp file. So this is the indended goal of the PR.

Regarding the possible compilation issues - PlatformIO recompiles whole project including all the libraries if you change build_flags settings so the issue with out of date binaries doesn't exist here.

In Arduino IDE there might be possible issues if you #define the M5EZ_ macros in your sketch before including M5ez.h file, but M5ez.cpp will see different set of settings because these are not defined globally (as in PlatformIO build system) and the build will fail. However since the settings could be changed only by editing the source of the library, I don't think this will dramatically change the way of changing it in Arduino.

[paultech]

This is exactly the problem - once the M5EZ_ setting is defined, there is no way how to change it/undefine it without editing the source code of the library itself when using #ifdef checks, because #ifdef evaluates to true even if macro has no value. On the other hand #if evaluates to true only if macro has nonzero value.

Use -U to undef via build_flags, same way you use -D.

If PlatFormIO includes it as a compilation unit, that should be sufficient to alter compile time options for you.

[dzindra]

This is exactly the problem - once the M5EZ_ setting is defined, there is no way how to change it/undefine it without editing the source code of the library itself when using #ifdef checks, because #ifdef evaluates to true even if macro has no value. On the other hand #if evaluates to true only if macro has nonzero value.

Use -U to undef via build_flags, same way you use -D.

If PlatFormIO includes it as a compilation unit, that should be sufficient to alter compile time options for you.

Unfortunately -U works only for builtin macros or macros defined via -D option, not for code-defined macros. I tried it first before sending this PR (or at least i couldn't get it to work properly)

[paultech]

Was unaware of that, my mistake.

I think the desire for easier compile-time options is valid, I don't believe targeting a method that works best for PlatformIO is ideal.

Simplest/most flexible method I feel is:

• Provide m5ez_config.h file that makes for easier reading of available toggles/options.
• Use M5EZ_USER_CONFIG guard to #include <m5ez_user.h>
• Change feature-defines (ex: M5EZ_WIFI) to toggles with values and incorporate #if value checks.

This allows for PlatformIO dependency system to work as designed via build_flags, also allowing to change defines prior to including M5ez.h.
For Arduino, a sketch-local m5ez_user.h provides an optional location to change defines while providing a convenient location for theme defines also.
Limits users making changes to the library files directly that get overwritten by updates.

Thoughts?

[dzindra]

I think this concept will work well and will improve readability as you mentioned. Good idea with themes in user config file. I’ll implement this in next few days (also with example code).

[dzindra]

I have done some research and tried to implement the m5ez_user.h config file feature but it seems Arduino IDE doesn't support including sketch headers when it builds libraries, I always end up with file not found error.

After checking Arduino features it seems that idea of user library configuration is not supported in Arduino so far. See following issues from Arduino repo: arduino/arduino-builder#29 and arduino/arduino-builder#15 - instead they recommend changing the library API, but I think this would be even bigger breaking change.

However I can stil move configuration to M5ez_config.h file so it can be easily edited and possibly overriden with PlatformIO build flags.

[ropg]

I think having all the defines etc in a M5ez_config.h file is a good idea. One could try to think of ways for such config to survive library upgrades, but that would be a hack.

(My most useable idea so far: create a library called M5ez_config that we depend on, and never change its version number. But as clever as one can get: we should probably stick to simple and just warn people in a big comment at the top of the config file that they need to store a copy.)

[dzindra]

Added separate config file and merged upstream changes, what do you think?