iotappstory / ESP-Library

Software Distribution and Management Over the Air
GNU Lesser General Public License v2.1
124 stars 35 forks source link

Update config.h to allow overriding values #104

Closed abaskin closed 4 years ago

abaskin commented 5 years ago

Update config.h to allow configuration values to be overridden by defining the value before including IOTAppStory.h.

Onno-Dirkzwager commented 5 years ago

Hi @abaskin, thanks for the request! Which compiler are you using? We agree that this is the way to go. We have tried this before for Arduino IDE but because of the strange compiling order it did not work. (maby we missed something or things have changed...)

An local config.h for each project would be great but we have only had succes in platformio.

abaskin commented 5 years ago

I’m using version 1.8.8 of the Arduino IDE on OSX. However I do understand the problem with the Arduino IDE you are speaking of. As I understand it the Arduino IDE compiles the libraries used by the sketch once and uses these compiled versions in subsequent compiles for the session. As a result if you make a change the a value using #define that effects the compilation of the library this change will not have any effect since the library has already been compiled and you are using the compiled copy. There are a few ways to force a the libraries to be recompiled.

From: Onno-Dirkzwager notifications@github.com notifications@github.com Reply: iotappstory/ESP-Library reply@reply.github.com reply@reply.github.com Date: February 25, 2019 at 1:27:39 AM To: iotappstory/ESP-Library esp-library@noreply.github.com esp-library@noreply.github.com Cc: abaskin andre@telatomica.com andre@telatomica.com, Mention mention@noreply.github.com mention@noreply.github.com Subject: Re: [iotappstory/ESP-Library] Update config.h to allow overriding values (#104)

Hi @abaskin https://github.com/abaskin, thanks for the request! Which compiler are you using? We agree that this is the way to go. We have tried this before for Arduino IDE but because of the strange compiling order it did not work. (maby we missed something or things have changed...)

An local config.h for each project would be great but we have only had succes in platformio.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/iotappstory/ESP-Library/pull/104#issuecomment-466802553, or mute the thread https://github.com/notifications/unsubscribe-auth/AMkMw-IviSl1nwkIASXqqoG0vKWGgLtZks5vQtmagaJpZM4bOdSv .

abaskin commented 5 years ago

(sorry hit send before I was finished) The most straight forward way to force a recompile is to close and reopen the IDE. You can also temporarily change a board setting such as Flash Size to Debug Port.

Onno-Dirkzwager commented 5 years ago

@abaskin thanks for the feedback! Ill try to confirm. If I can. We will accept!

Any chance you could resend your pull request to the develop branche?

Onno-Dirkzwager commented 5 years ago

Edit: fixed typo!

Hi @abaskin

unfortunately I could NOT replicate your outcome. This is wat I did:

Sketch:

include "config.h"

include

IOTAppStory IAS(COMPDATE, MODEBUTTON);

Local config.h

define DEBUG_LVL 1

define SERIAL_SPEED 115200

define BOOTSTATISTICS false

Library config.h

ifndef DEBUG_LVL

#define DEBUG_LVL               2       // Debug level: 0 - 3 | none - max

endif

ifndef SERIAL_SPEED

#define SERIAL_SPEED            115200

endif

ifndef BOOTSTATISTICS

#define BOOTSTATISTICS          true

endif

Serial ouput ------------------------------------------------------------------------- Start esp32 blink v4.0.0 ------------------------------------------------------------------------- Mode select button: GPIO0 Boardname: iasblink Update on boot: 1 ------------------------------------------------------------------------- bootTimes since last update: 2 boardMode: N

So my results are DEBUG_LVL 2 & BOOTSTATISTICS true

QuantusHoka commented 5 years ago

Similarly, I suggest adding

**Library config.h**
#ifndef MAX_WIFI_RETRIES
#define MAX_WIFI_RETRIES                20      // sets the maximum number of retries when trying to connect to the wifi
#endif

I edited config.h accordingly, and compiled it successfully (without config.h overriding my MAX_WIFI_RETRIES setting) under PlatformIO and the Arduino IDE.

Onno-Dirkzwager commented 5 years ago

@QuantusHoka,

thanks for joining in. What do you suggest adding? I just used DEBUG_LVL <> BOOTSTATISTICS as an example to test the theory(*again) And did not get it to work with Arduino IDE.

Edit: Btw I have extensively used #ifndef between files and libraries without problems. Just never got it to work from Arduino IDE overiding deeper library defines.

* This topic has passed multiple times in last 2 years. And nobody was able to send us an working example for Arduino IDE in the past. We really hope we are missing something.... because this strategy would greatly benefit all using this library.

QuantusHoka commented 5 years ago

I understand. I'll play with it some more, because I thought that I had it working in the Arduino IDE (and definitely in PlatformIO) but, frankly, y'all are far more skilled than I, so I expect that it is I who am missing something, rather than all of you.

QuantusHoka commented 5 years ago

Weird... In PlatformIO the position of the local/overriding #define relative to "#include " determines whether the override works. I.e., the overriding #define must be placed before the #include, or the compiler will throw a warning, and the override will not work, regardless whether #ifndef instructions are used in config.h.

In Arduino IDE 1.8.8 the relative positions has not mattered. I edited the IAS config.h to use #ifndef, then varied the relative location in my sketch of the overriding #define and the #include (changing the board and recompiling between each change, and also strategically commenting-out my overriding #define), and confirmed that the Arduino toolchain (esp32 1.0.1, Lolin D32) seems to accept the override.

Perhaps I am not understanding the issues.

Onno-Dirkzwager commented 5 years ago

@abaskin @QuantusHoka Its clear this works for you guys. It does not for me.

My changes can be found below. In between changes I restarted the IDE, changed board settings. And even rebooted.

Any ideas? btw which OS are you guys running? I tested with Arduino IDE 1.8.6 & 1.8.8 on 2 different win7 machines.

PlatformIO no prob. btw.

Sketch skech

Local config local config

Library config library config file

Onno-Dirkzwager commented 5 years ago

I gave it an other try by adding the defines directly in the main sketch.

defines in sketch

Of course still keeping the #ifndef statements in the library config.h library config file

I compiled multiple times restarted the IDE & changed boards in between. Unfortunately still without results.

QuantusHoka commented 5 years ago

I have always included the defines in the main sketch (I'll consider a separate header file in future projects - that looks like it would make for cleaner code).

I'm using: Arduino 1.8.8 IAS 2.0.1 - I edited the config.h file to include the #ifndef statements VSCode 1.31.1 + PlatformIO 2.0.0 (Core 3.6.4) All on Windows 10 with all current updates applied.

It is interesting that you seem to be getting precisely the opposite result that I see - won't work on Arduino, but will in PlatformIO. If it helps, I have included my complete sketch code below (don't judge too hard, I haven't written code since turbo Pascal in 1992). Well... I was going to attach the full sketch, but it is over 700 lines, and that seems excessive The full code is here. The attached code compiles on both PIO and Arduino. Compiling works on Arduino (for me) and throws a warning if I move the #include in with the rest of the includes.

src\main.cpp:14:0: warning: "MAX_WIFI_RETRIES" redefined

define MAX_WIFI_RETRIES 10

^ In file included from C:/users/markc/.platformio/lib/IOTAppStory-ESP_ID1704/src/IOTAppStory.h:10:0, from src\main.cpp:8: C:/users/markc/.platformio/lib/IOTAppStory-ESP_ID1704/src/config.h:22:0: note: this is the location of the previous definition

define MAX_WIFI_RETRIES 20 // sets the maximum number of retries when trying to connect to the wifi<

My IAS config.h (relevant section)

#include "serialFeedback_EN.h"              // language file for serial feedback

    #ifndef DEBUG_LVL
        #define DEBUG_LVL                           2           // Debug level: 0 - 3 | none - max
    #endif
    #define SERIAL_SPEED                        115200
    #define BOOTSTATISTICS                  true

    // config 
    #define INC_CONFIG                          true  // include Config mode (Wifimanager!!!)
    #define CFG_AUTHENTICATE                false   // Set authentication | Default: admin - admin | Password can be changed when in config mode | max 16 char
    #define CFG_PAGE_INFO                       true    // include the info page in Config mode
    #define CFG_PAGE_IAS                        true  // include the IAS page in Config mode

    // Wifi defines
    #define SMARTCONFIG                         false   // Set to true to enable smartconfig by smartphone app "ESP Smart Config" or "ESP8266 SmartConfig" | This will add (2%) of program storage space and 1%) of dynamic memory
    #define WIFI_MULTI                          true    // false: only 1 ssid & pass will be used | true: 3 sets of ssid & pass will be used
    #ifndef MAX_WIFI_RETRIES
        #define MAX_WIFI_RETRIES                20      // sets the maximum number of retries when trying to connect to the wifi
    #endif
    #define USEMDNS                                 true  // include MDNS responder http://yourboard.local

Functional Sketch

#include <Arduino.h>
#include <ArduinoJson.h>
#include <OneWire.h>
#include <DallasTemperature.h>
#include <WiFi.h>
#include <Nextion.h>
#include <AsyncMqttClient.h>

/*-------------------  IOTAppStory basic stuff----------------*/
#define COMPDATE __DATE__ __TIME__
#define MODEBUTTON 0
#define DEBUG_LVL         2
#define MAX_WIFI_RETRIES  10
#define IAS_BOOT_UPDATE false
#define IAS_AUTO_CONFIG false
#define IAS_DEVICE_NAME "RAMControl"
#define IAS_ERASE_EEPROM 'L' //'F" = full, 'P' = Partial, 'L' = Leave Intact
//#define IAS_CALLHOME_INTERVAL   7200 //seconds = 2h * 60min/h * 60s/min
#include <IOTAppStory.h>
IOTAppStory IAS(COMPDATE, MODEBUTTON);

/*-----------------------------------MQTT Basic Stuff-------------------------*/
AsyncMqttClient mqttClient;
#define MQTT_HOST "my MQTT host"
#define MQTT_PORT 9999
#define MQTT_RAM_SETTINGS_TOPIC "ram/settings/#"

Failing Sketch (PIO only)

#include <Arduino.h>
#include <ArduinoJson.h>
#include <OneWire.h>
#include <DallasTemperature.h>
#include <WiFi.h>
#include <Nextion.h>
#include <AsyncMqttClient.h>
#include <IOTAppStory.h>

/*-------------------  IOTAppStory basic stuff----------------*/
#define COMPDATE __DATE__ __TIME__
#define MODEBUTTON 0
#define DEBUG_LVL         2
#define MAX_WIFI_RETRIES  10
#define IAS_BOOT_UPDATE false
#define IAS_AUTO_CONFIG false
#define IAS_DEVICE_NAME "RAMControl"
#define IAS_ERASE_EEPROM 'L' //'F" = full, 'P' = Partial, 'L' = Leave Intact
//#define IAS_CALLHOME_INTERVAL   7200 //seconds = 2h * 60min/h * 60s/min
IOTAppStory IAS(COMPDATE, MODEBUTTON);
Onno-Dirkzwager commented 5 years ago

I redid my tests on 1.8.8 with a ESP32 just to be sure this was not a platform thing.... Still no succes. This is becomming annoying.

Anyway forget about me. If we can find 2 or more others who can confirm this works for them. Ill accept the PR. Invite anyone you would like to....

@CwlBroeders @Bolukan ?

Onno-Dirkzwager commented 5 years ago

Hi @abaskin A lot of things have changed since the first 2.x.x Maby you can have a look at the develop branch?

I would like to give you the credits for proposed changes. It would be a shame if you update files that are getting removed in the next release.

abaskin commented 5 years ago

Yes, let me have a look at the develop branch and see how it goes.

Onno-Dirkzwager commented 5 years ago

@abaskin You are probably still working on the PR......

It might be wise to start with a fresh fork. Looking at the latest changes ( Change to New HTTPClient ... )

It looks like Github lost track and is mixing up results