Fix function not in scope

[jbaudoux]

Your IDE is using a pre-processor that declares all your functions. Platformio is not using such and fails to compile. I fixed the scope issue by moving a few functions and mainly pre-declaring functions that are used before their definition (what your pre-processor is doing to your code before compiling) to keep the diff small. Also moved your recent startLoggingDevice function between #if defined LOGGER - #endif statements

[fredlcore]

I'm surprised you encouter these issues as we're not only using a PlatformIO workflow, but I also use PlatformIO and VS Code myself for development and don't run into these issues. It's also the reason we have already forward declarations where necessary. Attached is the PlatformIO configuration I'm using: platformio.txt

But since you are using the same environment as I do, I hope you could help me with this problem: I have introduced a function for the ESP32 called netEvent that is called by WiFi.onEvent(netEvent). All the relevant code is guarded by #if defined (ESP32), but still I get this error message when compiling for the Arduino Due:

Building in release mode
Compiling .pio/build/due-PPS/src/BSB_LAN.ino.cpp.o
Compiling .pio/build/due-PPS/src/src/SdFat/SpiDriver/SdSpiSTM32.cpp.o
Compiling .pio/build/due-PPS/src/src/SdFat/SpiDriver/SdSpiSTM32Core.cpp.o
Compiling .pio/build/due-PPS/src/src/SdFat/SpiDriver/SdSpiTeensy3.cpp.o
Compiling .pio/build/due-PPS/src/src/SdFat/common/FmtNumber.cpp.o
Compiling .pio/build/due-PPS/src/src/SdFat/common/FsCache.cpp.o
Compiling .pio/build/due-PPS/src/src/SdFat/common/FsDateTime.cpp.o
Compiling .pio/build/due-PPS/src/src/SdFat/common/FsName.cpp.o
Compiling .pio/build/due-PPS/src/src/SdFat/common/FsStructs.cpp.o
/Users/frederik/Documents/PlatformIO/Projects/BSB-LAN/src/BSB_LAN/BSB_LAN.ino:1008:15: error: variable or field 'netEvent' declared void
     return input - 'a' + 10;
               ^~~~~~~~~~~
/Users/frederik/Documents/PlatformIO/Projects/BSB-LAN/src/BSB_LAN/BSB_LAN.ino:1008:15: error: 'WiFiEvent_t' was not declared in this scope
Compiling .pio/build/due-PPS/src/src/SdFat/common/FsUtf.cpp.o
Compiling .pio/build/due-PPS/src/src/SdFat/common/PrintBasic.cpp.o
Compiling .pio/build/due-PPS/src/src/SdFat/common/upcase.cpp.o
Compiling .pio/build/due-PPS/src/src/SdFat/iostream/StdioStream.cpp.o
Compiling .pio/build/due-PPS/src/src/SdFat/iostream/StreamBaseClass.cpp.o
Compiling .pio/build/due-PPS/src/src/SdFat/iostream/istream.cpp.o
Compiling .pio/build/due-PPS/src/src/SdFat/iostream/ostream.cpp.o
Compiling .pio/build/due-PPS/src/src/Time/DateStrings.cpp.o
*** [.pio/build/due-PPS/src/BSB_LAN.ino.cpp.o] Error 1

The weird thing is that the quoted line (which is indeed at 1008) has nothing to do with the added code which is actually in the 7700++ range. The error disappears when I remove the void netEvent function which is, as I said, completely guarded by #ifdef. The strange thing is that the same error with the same lines come up when the GitHub workflow runs. I have no idea how this can happen, do you have an idea or solution for this?

[fredlcore]

Ok, I managed to solve my problem by adding a #define WiFiEvent_t int in the non-ESP32 definitions, but still have no clue why this would even be necessary in a completely guarded function :(...

[fredlcore]

Some things before considering merging: We already have a forward declaration section in the beginning of BSB_LAN.ino (for example for loop() and set()), so why do you not declare all problematic functions there? This also makes code review easier for me as I don't have to check severl files. But as I said, I would like to figure out first why you encounter this problem and it doesn't happen for me or the GitHub workflow...

Secondly, please do not add preprocessor directives such as in the case of DHT_PINS. Nowadays these directives are just a leftover from earlier times where we used these directives as configuration options. Eventually, I'm trying to get rid of these and move everything to configuration using variables.

[jbaudoux]

I have the same platformio.ini except that I'm using platform_packages = framework-arduinoespressif32 @ https://github.com/espressif/arduino-esp32.git#2.0.14 I'm compiling on Ubuntu 20.04

For example I get: BSB-LAN/BSB_LAN/include/get_cmdtbl_values.h:53:10: error: 'recognizeVirtualFunctionGroup' was not declared in this scope

I don't see the forward declaration at the beginning of BSB_LAN.ino. Can you point me to the line? I'll do the change. Can you enable the github workflow run on this PR?

[fredlcore]

#if defined(ESP32)
void loop();
int set(float line, const char *val, bool setcmd);
#else
#define WiFiEvent_t int         // Very strange that the netEvent() function which is completely guarded by preprocessor directives throws an error on Arduino Due, complaining that WiFiEvent_t is not defined. This workaround fixes this.
#endif

[fredlcore]

So please run it with my platformio.ini. If neither the GitHub workflow (which runs under Linux) nor my MacOS build process exhibits this error, then the problem is more likely to be on your side...

[fredlcore]

If the error is not reproducible here or elsewhere, you can also just add the required forward-declarations in you BSB_LAN_custom_global.h.

[fredlcore]

I'm most likely not going to accept this because it will significantly increase completion of the workflow. One run usually takes around 40 seconds to one minute. This change would result in an approx. 20 minute run - per architecture.

[fredlcore]

Sorry, commented in wrong PR...

[fredlcore]

Just one more general comment on what kind of PRs help me (and thus the project) and which one not so much: I really appreciate it if people contact me first before they start working on something they want to contribute to the project. If people just submit a PR and I have to figure out or ask about the reason or idea behind it, it costs me quite some time which I'd rather want to spend on other project-related matters. The same goes for code reviews that come with vetting a PR, these often span several files. All this takes time and work from both sides that can be saved if an idea is put forward first in the discussion section. I can then explain why an idea has been rejected in the past or why I would like something to be in some way or another. Generally, I'm open to ideas that improve the project for the average user. Special interest wishes need to be argued well to be accepted, simply because we have explicitly created the BSB_LAN_custom.h and related files for that. For the current PR, this means that if there is no problem with the GitHub workflow (which mirrors an average installation), it's not likely that I'm going to accept it because there is no general problem in the first place. I'd still be interested to know why you have this problem, but I cannot invest any time in this. So unless you figure out why it's not working for you, but it does for the GitHub installation (which runs "ubuntu-latest"), and that this would be a problem for other users, I'd suggest you use the BSB_LAN_custom_global.h file to fix the issue for your installation.

[jbaudoux]

I think this deserves a note in the config file. I thought I had to configure the options I was interested in and I run into several compilation issue. If I don't touch them, it works perfectly, the same way it works on github actions. Currently LOGGER and BUTTONS cannot be disabled without causing scope issues. I wanted to help you understand the issues by improving the test coverage (and also because it's useless to fix something that could break again at any time without figuring out). I don't think there is a need for that many combinations and once the first compilation is done, the other tests are very quick. I'm also fine with keeping some options I don't need enabled and concentrate on more valuable topics.

[fredlcore]

The config file is not a manual. In the manual, we explain how and what to configure. If it is still mentioned there that you can disable these functions, then that is the place to correct that. I appreciate your desire to help, but as I mentioned before, I prefer to discuss any changes with me first before submitting PRs that may become obsolete in the forseeable future. And as you can see in the ChangeLog, removing the need for preprocessor directives has been something I've been working on in the last couple of weeks... If you say that compilation works fine with you if you follow the basic configuration, then I'll close this matter for now.

[fredlcore]

I started now to move the BUTTONS code from main code (which I had planned to anyway because it no longer works with the default installation) to the custom code library and the result is as bizarre as the fact that I needed to define WiFiEvent_t for the Arduino code, despite it should only be active in ESP32 code segments. Why simply removing a few lines of code results in dozens of functions required to be forward-declared without these functions even being used in these lines of code, is really beyond me. Yes, it makes sense that these forward declarations are necessary, but why the compiler didn't complain before but does now doesn't. Anyway, thanks for highlighting this issue. If I had started to remove these lines of code without this background information, I would have probably pulled my hair as to why these errors are related to the BUTTONS code.

[fredlcore]

Ok, most configuration definements are now removed and almost all configuration options can now be done without reflashing.