Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build.extra_flags breaks nested include in a .h file #2041

Closed
3 tasks done
DaleSchultz opened this issue Jan 23, 2023 · 3 comments
Closed
3 tasks done

build.extra_flags breaks nested include in a .h file #2041

DaleSchultz opened this issue Jan 23, 2023 · 3 comments
Assignees
Labels
conclusion: invalid Issue/PR not valid topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project

Comments

@DaleSchultz
Copy link

DaleSchultz commented Jan 23, 2023

Describe the problem

I have a file called utils.h which I include from my main .ino sketch

utils.h has:
#include <WiFiManager.h>

The app builds fine in Arduino IDE and also with arduino-cli with this command:

arduino-cli compile -b esp8266:esp8266:d1_mini include.ino -e

However if I add a --build-property like this:

arduino-cli compile -b esp8266:esp8266:d1_mini include.ino -e --build-property "build.extra_flags=\"-DSCREEN=64\""

The build fails claiming WifiManager has not been declared:

C:\ard\common\utils.h:887:4: error: 'WiFiManager' was not declared in this scope; did you mean 'WiFiManager_h'?
 11 |    WiFiManager wifiManager;
      |    ^~~~~~~~~~~

I accept that perhaps my utils should be a pair of .h and .cpp files (I have code in the .h file) but it is the case that it all works perfectly without the --build-properties flag.

Here is the minimized utils.h file:

//utils
#ifndef utils_h
#define utils_h
#include "ESP8266WiFi.h"
#include <WiFiManager.h>          //https://github.com/tzapu/WiFiManager
void WiFiMan(char const* APName, int timeout) { // use wifi manager to ask for credentails
	WiFiManager wifiManager;	
} // WiFiMan
#endif  // #ifndef utils

and the include.ino file:

#include "utils.h"  // includes Wifimanager
void setup(){}
void loop(){}

To reproduce

  1. Ensure you have a library such as <WiFiManager.h> installed
  2. Place include.ino and utils.h in a folder
  3. From Windows command prompt, cd to the folder
  4. Issue the command:
    arduino-cli compile -b esp8266:esp8266:d1_mini include.ino -e
    
  5. Note that the sketch compiles OK
  6. Issue the command:
    arduino-cli compile -b esp8266:esp8266:d1_mini include.ino -e --build-property "build.extra_flags=\"-DSCREEN=64\""
    
  7. Note that the build no longer compiles

Expected behavior

I expect the build to succeed with both compile commands.

Arduino CLI version

nightly-20230122 Commit: a58d5ad Date: 2023-01-22T01:33:46Z

Operating system

Windows

Operating system version

11

Additional context

No response

Issue checklist

  • I searched for previous reports in the issue tracker
  • I verified the problem still occurs when using the nightly build
  • My report contains all necessary details
@DaleSchultz DaleSchultz added the type: imperfection Perceived defect in any part of project label Jan 23, 2023
@per1234
Copy link
Contributor

per1234 commented Jan 23, 2023

Hi @DaleSchultz. Thanks for taking the time to submit an issue.

I see you are using an older version of the ESP8266 boards platform. The --build-properties flag you added to your arduino-cli compile command is overriding the property value set in the ESP8266 boards platform:

https://github.com/esp8266/Arduino/blob/3.0.2/platform.txt#L86

build.extra_flags=-DESP8266

This means the ESP8266 is no longer defined when compiling your sketch. This macro is used by the "WiFiManager" library:

https://github.com/tzapu/WiFiManager/blob/v2.0.15-rc.1/WiFiManager.h#L17

#if defined(ESP8266) || defined(ESP32)

[...]

class WiFiManager

[...]

#endif

So you can see now why your sketch no longer compiles.

The --build-properties flag is very powerful, but if you are going to use it you must first carefully study the platform configuration to understand the effects of setting a property because you will override any value that is set in the platform configuration.

In this case, you would want to include the original value in your new customized value for the property:

arduino-cli compile -b esp8266:esp8266:d1_mini include.ino -e --build-property "build.extra_flags=\"-DESP8266 -DSCREEN=64\""

There is a proposal about making platforms more friendly to customization by the user via the --build-property flag here: #846

@per1234 per1234 closed this as not planned Won't fix, can't repro, duplicate, stale Jan 23, 2023
@per1234 per1234 self-assigned this Jan 23, 2023
@per1234 per1234 added conclusion: invalid Issue/PR not valid topic: code Related to content of the project itself labels Jan 23, 2023
@DaleSchultz
Copy link
Author

Thanks for the explanation of what was happening.

Is my understanding correct that all the existing board build properties are replaced with whatever build property is defined on the compile command?

If so perhaps the documentation for the compile should be updated to indicate that. At present it states:

Override a build property with a custom value. Can be used multiple times for multiple properties.

which makes it sound as if a single property can be customized. Perhaps that should read:

Override all build properties with a set of custom values. Can be used multiple times for multiple properties.

I was very much looking for a 'user' side ability to set a define at compile time, so I will add my use case to that discussion.

@per1234
Copy link
Contributor

per1234 commented Jan 24, 2023

Is my understanding correct that all the existing board build properties are replaced with whatever build property is defined on the compile command?

No. It only overrides the build property specified via the flag. So in your command, the value specified via the --build-property flag overrides the platform's definition of build.extra_flags and only that definition. Your command does not have any effect on other properties of the platform. For example the platform's build.lwip_flags property definition is not affected in any way by your command.

which makes it sound as if a single property can be customized.

That is exactly what it does.

I was very much looking for a 'user' side ability to set a define at compile time

You have one already. You just need to make sure your use of a property doesn't conflict with the use of the property in the platform. The build.extra_flags property you selected is not intended to be set by the user. Arduino CLI's powerful --build-property flag allows the user to this, but you must use it responsibly. As I explain in #846, there is already a set of flags which are intended to be reserved for the user to modify the compilation commands:

https://github.com/esp8266/Arduino/blob/3.0.2/platform.txt#L88-L95

In a platform that is respecting the intended use of those properties, as is the case with the ESP8266 platform, the user can set them via --build-property without fear of interfering with existing use of the properties by the platform

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conclusion: invalid Issue/PR not valid topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
Development

No branches or pull requests

2 participants