Refactor `#ifdef`s in `Commander-API-Commands.cpp`

[ondras12345]

This is a followup to https://github.com/dani007200964/Commander-API/pull/20 with some more aggressive changes.

The changes I made to commander_analogRead_func make it less readable, so feel free to cherry-pick only the commits you want.

Testing

I was unable to test my changes directly due to the following error (not caused by this PR):

error: invalid conversion from 'void (*)(char*, Stream*)' to 'void (*)(char*, Stream*, void*)' [-fpermissive]

However, I was able to do it by using my new Commander-API-Commands.cpp with the rest of the repo from the master branch.

commander_reboot_func

I have added an error message to it that warns the user at runtime if reboot is not available on that platform (e.g. STM32). However, there are other options:

1. message printed at runtime (current implementation)
2. compile-time #warning
3. #ifdef around the whole function. This will cause a linker error if unavailable, similar to commander_analogRead_func (but unlike commander_configTime_func, which also has #ifdef guards in Commander-API-Commands.hpp that prevent it from even defining the corresponding API_ELEMENT_.
4. #ifdef around the whole function and the API_ELEMENT_ definition (similar to commandr_configTime_func)

Let me know which one you prefer, I will update this PR. Until then, I will make it a draft.

[dani007200964]

This is a big topic. I started working on this as well because the previous code of mine was pretty ugly. In 0759886af505555df8491471942c83e74b829b98 I made some modifications in Commander-API-Commands.hpp and I started to separate the source files for it because it will be a huge chunk of code.

There will be one header for it to make it easy to link, but the implementations will be categorized in separate source files. For now, only Commander-API-Digital-IO-Commands.cpp is implemented.

I also modified the arguments for the other built-in command functions to make the compile with the new version, but I need some time to make them better.

I also worked on the declaration structure of the internal commands. I think it will be easier to edit like this: https://github.com/dani007200964/Commander-API/blob/0759886af505555df8491471942c83e74b829b98/src/Commander-API-Commands.hpp#L114-L130

The function implementation behind these functions will be reworked as well. I created an Argument class to make argument parsing easier and more flexible. I will need some time to finish it and implement it in all of the internal commands. For example with the new version setting a pin direction looks like this: pinMode -p 13 -o or pinMode --pin 13 --output. I think it is more flexible now.

I was unable to test my changes directly due to the following error (not caused by this PR): This was a known issue. The parent argument would be implemented in the next release, but the internal commands only support it from this commit: 0759886af505555df8491471942c83e74b829b98

I think it would be a nice touch to create separate source files for the internal commands as I explained above. For example, create a source file called Commander-API-System-Commands.cpp and move the commander_reboot_func there. This way it is readable I think. I like your solution to make the architecture-dependent part safe without compile errors. I think a compiler warning would be nice, but Arduino IDE won't print them( as far as I know ). But advanced IDEs like STM32-Cube will show them, and I think it is the right way to tell if something compiles, but it is still broken or not functional.

I'm thinking of some kind of 'hook' solution to add custom interfaces to these functions in case of a custom environment. I tried to do some linker magic with attribute((weak)) to these functions, but I could not manage to make it compile for now.

In the STM32 Cube environment, there are a lot of bookable functions like this for event handling. I think the ultimate solution would be to make a week implementation for these kinds of functions to make them compile. Also, add a compiler warning to them. If the user needs the functionality for them, the weak function could be overridden from the user code space without modifying the library.

What do you think about this way?

[ondras12345]

I like your changes to Commander-API-Commands.hpp.

Some thoughts about weak:

• API_ELEMENT_*_DESCRIPTION defines should definitely be kept even if the matching func is not available for the target platform to make it easier for the user to implement their own version.
• I would prefer to get a warning at compile time if a function is not available on my platform. However, if that function is defined weak and overridden, the #warning in its body would still be generated (I think), which is not desirable.
• Perhaps wrapping the whole function with #ifdef (and not making it weak) is not that bad after all. There will be a linker error, which might not be particularly user-friendly, but I still think it is preferable to get an error (or at least a warning) at compile time as opposed to having a println at run time.
• Actually, if we also hide the prototype in Commander-API-Commands.hpp, we should get a compiler error, which is much better than a linker error.
• I am unsure if we want to keep the final API_ELEMENT* define for unavailable functions. If we keep it and hide the function prototype, using it will result in a compiler error. However, as soon as the user implements their own function, it will be useful to them.
• Another interesting point is examples. I assume you'll want to show off all available commands for each platform, but that would require cluttering the example with #ifdefs. However, it might still be better than having weak commands that only print an error message when the user tries to run them.

This PR is now obsolete, closing.