Espeasy: Discussion on how to define 24 task settings

[TD-er]

There has been a lengthy discussion already about how to implement the patch to use 24 tasks on ESP8266 ESPEasy.
However this was posted below a pull request, which makes it hard to follow and rather invisible to others unless they track all changes and updates on this repo.

So let's continue here.

The most useful parts of the discussion:

• Arduino IDE compilation was (again) broken by my attempt to clean up the code.
• Splitting defines into separate files made it hard to apply a user defined patch for 24 tasks which had been working for a while.
• How to add/suggest some kind of universal define to make switching to a different set of tasks more reliable and reproducible
• Other defines a user may want to set using a simple header file for creating a personalized build.

@uzi18 has some set to define 24 tasks: https://github.com/letscontrolit/ESPEasy/commit/f7e0d7cfdde65d9503ff08fd92d48681dc8da702#commitcomment-35122941

#define TASKS_MAX                          24
#define DAT_OFFSET_CONTROLLER            4096    // 0x1000 each controller = 1k, 4 max
#define DAT_OFFSET_CUSTOM_CONTROLLER     8192    // 0x2000 each custom controller config = 1k, 4 max.
#define DAT_OFFSET_TASKS                16384    // 0x4000 each task = 2k, (1024 basic + 1024 bytes custom), 12 max

@clumsy-stefan has another (incompatible) one: https://github.com/letscontrolit/ESPEasy/commit/f7e0d7cfdde65d9503ff08fd92d48681dc8da702#commitcomment-35126444

#define TASKS_MAX                          24 // max 12! 
#define DAT_OFFSET_CONTROLLER            DAT_OFFSET_TASKS + (DAT_TASKS_DISTANCE * TASKS_MAX)                        // each controller = 1k, 4 max 
#define DAT_OFFSET_CUSTOM_CONTROLLER     DAT_OFFSET_CONTROLLER + (DAT_CUSTOM_CONTROLLER_SIZE * CONTROLLER_MAX)  // each custom controller config = 1k, 4 max 

[clumsy-stefan]

One thing I'd like to add to your list is the possibility to export/import settings in some structured way (json, etc.), this would make changes in the config structure much less of an issue for users when they can somehow easily backup/restore a config across versions!

[TD-er]

That's another issue, which has been a feature request for a long time already.
The problem with serializing/de-serializing structs is that you also need to include their variable names as text in the binary. Since we're already struggling with the binary size, I don't think that's a good idea right now.

[clumsy-stefan]

Not sure if I understand, but the variable-name is in the settings file not the binary? Basically a json export of the config page would be enough?
Ialready thought about hacking something together which exports the config-pages in some kind of HTML and then does a series of post-request to put in the values again...
I think even if it's an external script (python, perl, shell, etc.) that fetches the configs in some text format and pushes it again if needed, would already help!
IMHO...

[TD-er]

For example, if I want to export this struct into JSON:

struct Foo {
int bar;
};

Then I need to create a JSON with the name "bar" in it.
Also when reading the JSON, I need to look for a name "bar". So this means the string "bar" must be included in the binary.
The same for all other variables in the struct.
Also the string name for these variables may not change, so you cannot use the "display name" string for it, since someone may want to change that if there is a typo in it for example.

This means you will have some "internal string" and a "display string" to represent the same variable.

[TD-er]

The so called "new GUI" feature actually does interpret the binary file in JavaScript, thus in the browser.

[TD-er]

About the defines you use. They depend on other defines.
Is it possible those defines are not yet known when your define is being initiated?

[clumsy-stefan]

Yes, I also realised thet, that's why I also included in my Custom.h files all #define's that I use in the new definitions further up above... It's quite likely however that I missed something there, even after double-checking... or that one of these fixed defines had been changed before somewhere dynamically... I'm also still digging there...

[TD-er]

Maybe just define these values with an integer value instead of computing it.?

[clumsy-stefan]

I hate fixed-values for things that can be computed 😄 I think that's error prone and double work... :grin . And as it worked before, there has to be a mistake somewhere (which probably also has an influence to other things)...

[TD-er]

Well, before you were using it in the globals header file, but now the order in which things may be defined has changed.

And I do agree it is error prone, that's why we should handle it in the core with a single define.
I think @uzi18 has the more useful layout then, since he has the tasks at the end.

[clumsy-stefan]

but this would mean to manually reconfigure some 50 nodes currently, when I change the config-file structure... :(

[TD-er]

Well, it is not being said we should only use one ;)
There may be others using those settings if you gave them via a forum post or something?
The same applies for @uzi18 's layout parameters.

[clumsy-stefan]

There may be others using those settings if you gave them via a forum post or something?

I got mine from a forum posting actually...

[clumsy-stefan]

Tried PR #2617 as suggested by @TD-er and included my changes to have 24 Tasks as I had them before the cleanup merge. Unfortunately I still experience the same issues (runtime) again, which I did not have before the cleanup merge.

One of them is, that task nr. 13 (and it seems only that one) "looses" task- and variable-names after pushin "submit". However they are still present and when closing the config page they show up again.

The worse thing is, that there seem to be an issue with (some) I2C (probably due to config-corruptions). Even though in "scan I2C bus" the sensor shows up, the task that reads from the sensor (an SHT30) is eitehr never started or does not provide a result. In the debug log it does not create an event. some screenshots attached.

So it seems, that moving around the defines had other impacts on Custom.h defines. I n my Custom.h I use (as before the cleanup merge):

  #define DAT_TASKS_DISTANCE               2048  // DAT_TASKS_SIZE + DAT_TASKS_CUSTOM_SIZE
  #define DAT_TASKS_SIZE                   1024
  #define DAT_TASKS_CUSTOM_OFFSET          1024  // Equal to DAT_TASKS_SIZE
  #define DAT_TASKS_CUSTOM_SIZE            1024
  #define DAT_CUSTOM_CONTROLLER_SIZE       1024
  #define DAT_CONTROLLER_SIZE              1024
  #define DAT_NOTIFICATION_SIZE            1024

  #define DAT_BASIC_SETTINGS_SIZE          4096

  #if defined(ESP8266)
    #define DAT_OFFSET_TASKS                 4096  // each task = 2k, (1024 basic + 1024 bytes custom), 12 max
    #define DAT_OFFSET_CONTROLLER           28672  // each controller = 1k, 4 max
    #define DAT_OFFSET_CUSTOM_CONTROLLER    32768  // each custom controller config = 1k, 4 max.
    #define CONFIG_FILE_SIZE                65536
  #endif
  #if defined(ESP32)
    #define DAT_OFFSET_CONTROLLER            8192  // each controller = 1k, 4 max
    #define DAT_OFFSET_CUSTOM_CONTROLLER    12288  // each custom controller config = 1k, 4 max.
    #define DAT_OFFSET_TASKS                32768  // each task = 2k, (1024 basic + 1024 bytes custom), 32 max
    #define CONFIG_FILE_SIZE               131072
  #endif

  #define CONTROLLER_MAX                      3 // max 4!


#ifdef TASKS_MAX
#undef TASKS_MAX
#endif
#define TASKS_MAX                          24 // max 12! 

#ifdef DAT_OFFSET_CONTROLLER
#undef DAT_OFFSET_CONTROLLER
#endif
#define DAT_OFFSET_CONTROLLER            DAT_OFFSET_TASKS + (DAT_TASKS_DISTANCE * TASKS_MAX)                        // each controller = 1k, 4 max 

#ifdef DAT_OFFSET_CUSTOM_CONTROLLER
#undef DAT_OFFSET_CUSTOM_CONTROLLER
#endif
#define DAT_OFFSET_CUSTOM_CONTROLLER     DAT_OFFSET_CONTROLLER + (DAT_CUSTOM_CONTROLLER_SIZE * CONTROLLER_MAX)  // each custom controller config = 1k, 4 max 

[TD-er]

Hmm, this is probably not a good idea:

#ifdef TASKS_MAX
#undef TASKS_MAX
#endif
#define TASKS_MAX                          24 // max 12! 

Then you have no idea what value for TASKS_MAX is being used.
In PlatformIO, you can define such things on the "command line" to make sure they are defined before any other define is being called.
Not sure if something like that can be done in Arduino IDE?

[uzi18]

#ifndef TASKS_MAX
#ifndef TASKS_24
#define TASKS_MAX                          12 // max 12!
#else
#define TASKS_MAX                          24 // max 12!
#endif
#endif

will read all discussion later ...

[clumsy-stefan]

Why not? Like this it's made sure, that TASKS_MAX is always 24, independent if it's already defined before or not...

[TD-er]

Why not? Like this it's made sure, that TASKS_MAX is always 24, independent if it's already defined before or not...

Nope, for the simple reason that we don't really know when that change of you is being interpreted.
It is becoming more clear to me these global defines are really hard to predict when they will be evaluated.

I have to think about what to do here, since it does seem these will be evaluated at some time when the structures depending on them may already being handled by the pre-processor.

[clumsy-stefan]

ok, understand.. but it still seems related to moving the things around, as it worked before...

one other thing I just found, I flashed another node with my 24-task test build (with your PR) that has multiple I2C sensors. It seems, that only the SHT30 has an issue (see screenshots). Probably something directly in that Plugin (it uses direct I2C access and not only the ESPEasy functions). However this did not happen before the cleanup-merger... so this could likely be another sepparate issue...

[TD-er]

ok, understand.. but it still seems related to moving the things around, as it worked before...

It IS related, although I am now wondering to WHY it is related.

[clumsy-stefan]

did you test a SHT30 with your "standard" build? I can't check if it works with teh standard 12Task build see if it's related to the cleanup or to the 24tasks....

[uzi18]

@clumsy-stefan @TD-er about why my patch is different:

• if use original offsets with 24 tasks some data overlap another from tasks on controllers data, as we use max 4 so it could be safely moved just after general settings, this provide incompatibility
• it does not change config file size

@clumsy-stefan idea is far more clean maybe to move controller settings just after tasks in structure, but config file may grow a little

[uzi18]

about configuration file, with python script it should be possible to convert one to another

[TD-er]

about configuration file, with python script it should be possible to convert one to another

Sure, but then we should write one to interpret the binary blob.
Almost all of it has already been done by @ppisljar but then in JavaScript, which is then run in the browser.
We will need the browser part in the future I guess, so I am a bit reluctant in having 2 interpreters for the data, since we then have to maintain 2 versions (and I already don't understand the JavaScript version, or at least how to maintain it)

[uzi18]

I think more about Stefans automation for configuration conversion =) just in case

[clumsy-stefan]

it's actually not a "conversion" issue but a general "backup-restore" in binary independant format... eg. if you want to move from a ESP8266 to a ESP32 with the same sensors attached... it would be cool to just dump and restore the config... or even modify the baclkup file and upload it again... or for fully automated configurations etc...

[TD-er]

True, but where should this be executed?
The vagrant environment could do it I guess...

[clumsy-stefan]

P068_SHT3x plugin seems to fail here:

      SHT3X* sht3x = static_cast<SHT3X*>(getPluginTaskData(event->TaskIndex));
            if (nullptr == sht3x) {
        addLog(LOG_LEVEL_ERROR, F("SHT3x: not initialised!"));
              return success;
            }

sht3x always returns a nullptr, not sure yet why...

[TD-er]

It could be this object is considered not to have changed so the linker may include one already built into an .o file.
Can you make a clean build?

[clumsy-stefan]

ArduinoIDE always does clean builds, as I think, could not find any .o files anywhere... reverting git to before this merger and compiling it again (in the same tree) makes it work again...

[ppisljar]

adding to the discussion, the new GUI (as TD-er mentioned) parses the configuration from espeasy into json. we can store this json and load it on another ESPeasy (maybe running on esp32) and save it back into binary format. We could even add managing capabilities to the ui which would allow you to have more than allowed tasks defined in your browser and then store just the ones you want to ESP.

i plan to spend some time on moving the new UI forward in the following month.

[TD-er]

I think I have an idea what's happening here.
Not sure yet if it is due to the way how Arduino is handling these files or maybe it is a very standard C++ thing.

What we're seeing here is this:

• Some code operates like TASKS_MAX is set to 12.
• Some other code uses TASKS_MAX = 24.

I am not entirely sure yet if the preprocessor is working in 1 run or multiple runs.
It could be the preprocessor is just filling in the blanks it already knows and later evaluated the left over #ifdef parts.
We already know that (at least PlatformIO) is not always handling the existing #ifdef wrappers the way we expect when generating forward declarations.
Maybe there is also some issue here where defines set in other header files only will be evaluated by the preprocessor when they are being processed.
For example:

Globals.h:

#include "limits.h"  <-- Defines TASKS_MAX
#include "custom.h" <-- Updates TASKS_MAX

byte array[TASKS_MAX];  <-- What size is it?

byte getValue(byte index);

In globals.cpp:

byte getValue(byte index) {
  if (index < TASKS_MAX) {return array[index];}  <--- What is TASKS_MAX here?
  return 0;
}

[clumsy-stefan]

ok, I thought that's handled serially.... at least it seemd to have before...

[TD-er]

ok, I thought that's handled serially.... at least it seemd to have before...

I think that has to do with the fact it used to be all in a single .h file.

[clumsy-stefan]

Ah, understand... now it could be that src/Datastruct/* could be precompiled before the Custom.h is actually included... (or so..)?

[TD-er]

It does seem indeed that some of these values may change after some parts of the code are already being dealt with by the preprocessor.

[TD-er]

So either we must make sure a define is only defined once and not changed, or we must use get functions on runtime parts.
I think we may also have similar issues with the selection of which plugin to include, since the build size does increase a lot more than I would expect it to be.

[clumsy-stefan]

I guess that's an issue with all #define's (especially in Custom.h) that are specified in different places (centrally and lcoally), and could lead to all kind of strange effects..

[TD-er]

Well the difference is that they should be defined first and if not defined then set to the defaults.

[clumsy-stefan]

So you would need to make sure, that the Custom.h (or wherever the global defines are) are always and everywhere included somehow.... (can be casted if already included somewhere else) eg. some kind of an universal #include at the start of every file....

[TD-er]

Or we should stop using custom.h as something optional, but something that has to contain all defines or otherwise use the defaults.
Or go the PlatformIO way used in the custom build by using Python to define all from the build command line.

[TD-er]

What if you move this part into the globals.h file

/*
    To modify the stock configuration without changing this repo file :
    - define USE_CUSTOM_H as a build flags. ie : export PLATFORMIO_BUILD_FLAGS="'-DUSE_CUSTOM_H'"
    - add a "Custom.h" file in this folder.

*/
#ifdef USE_CUSTOM_H
#include "../../Custom.h"
#endif

[TD-er]

I've thought about it and to me it does look the best if we make all user configurable stuff be handled in the Custom.h and make it mutual exclusive.
So either you use the Custom.h and have to define all, or you use the values from ESPEasy.
If you're missing a value in the Custom.h, you will have a failing build.

Or should all defines be made using #ifndef statements?
The include of the Custom.h should then be one of the first lines in the code compilation.

Edit:
I think including it here in ESPEasy-Globals.h would be enough:

#ifndef ESPEASY_GLOBALS_H_
#define ESPEASY_GLOBALS_H_

#ifndef CORE_POST_2_5_0
  #define STR_HELPER(x) #x
  #define STR(x) STR_HELPER(x)
#endif

#ifdef __GCC__
#pragma GCC system_header
#endif

/*
    To modify the stock configuration without changing this repo file :
    - define USE_CUSTOM_H as a build flags. ie : export PLATFORMIO_BUILD_FLAGS="'-DUSE_CUSTOM_H'"
    - add a "Custom.h" file in this folder.

*/
#ifdef USE_CUSTOM_H
#include "Custom.h"
#endif



#include "ESPEasy_common.h"
....

[uzi18]

should be like this:

#include "custom.h" <-- Defines TASKS_MAX
#include "limits.h"  <-- Updates if not definied out of limit

[TD-er]

I think it should be like you suggested earlier and also discussed before.
We must have one to define the 24 tasks parameter, which does set them all.

[clumsy-stefan]

Or should all defines be made using #ifndef statements?
The include of the Custom.h should then be one of the first lines in the code compilation.

I'd vote in favor of this one, but I'm not a really structured coder ;)

I think including it here in ESPEasy-Globals.h would be enough:

Not sure if that's enough, as this file does not get included everywhere I think...

[TD-er]

See latest commit and try building using the USE_NON_STANDARD_24_TASKS flag.

@uzi18 Should we also make a USE_NON_STANDARD_24_TASKS_alt version for you?

[clumsy-stefan]

See latest commit and try building using the USE_NON_STANDARD_24_TASKS flag.

this works as good (or bad) as with what I had in my Custom.h previously... Meaning that task 13 still shows empty names after pushing "submit" and SHT30 not working...

but at least someone else should verify, if its because of this the 24 tasks or if there are other issues in my environment (which I tried to rule out as much as possible)...

[TD-er]

I'm going to test myself too, but that's using PlatformIO.

You can also use some tests like these at the places where you suspect some issues:

  #if defined(USE_NON_STANDARD_24_TASKS) && defined(ESP8266)
    static_assert(TASKS_MAX == 24, "TASKS_MAX invalid size");
  #endif

These static_asserts will fail the build if they are not passing the test.

[clumsy-stefan]

I'll try that... I just put it in every file at the beginning then 😁

[TD-er]

I was more thinking of places where it is actually being used.
So either in .cpp files, or in .ino files.

[clumsy-stefan]

sure, sorry, wasn't meant tooo serious.....

[uzi18]

@TD-er

#define DAT_OFFSET_CONTROLLER            4096    // 0x1000 each controller = 1k, 4 max
#define DAT_OFFSET_CUSTOM_CONTROLLER     DAT_OFFSET_CONTROLLER + (DAT_CONTROLLER_SIZE * CONTROLLER_MAX)  // each custom controller config = 1k, 4 max
// note not DAT_CUSTOM_CONTROLLER_SIZE
#define DAT_OFFSET_TASKS                16384    // 0x4000 each task = 2k, (1024 basic + 1024 bytes custom), 12 max

so we only change DAT_OFFSET_CONTROLLER and DAT_OFFSET_TASKS
and DAT_OFFSET_CUSTOM_CONTROLLER is not correct in @clumsy-stefan - to be updated like above

maybe we can use 3rd option like just tasks 12+ store at the end of the file so it will be more/less backword compatible but need some logic load/save routines (incompatibility only in basic settings)

with https://github.com/letscontrolit/ESPEasy/commit/6277a8fad249cefc6ed8860b70f0f32b528603a1 you can't build 24 tasks version for esp32

[TD-er]

I had the issues described by @clumsy-stefan when trying his offsets.
With the 'fixed' settings of @uzi18 I had crashes, when accessing something related to the tasks > 12

With these offsets used by @uzi18 it all seems to work just fine

#define TASKS_MAX                          24
#define DAT_OFFSET_CONTROLLER            4096    // 0x1000 each controller = 1k, 4 max
#define DAT_OFFSET_CUSTOM_CONTROLLER     8192    // 0x2000 each custom controller config = 1k, 4 max.
#define DAT_OFFSET_TASKS                16384    // 0x4000 each task = 2k, (1024 basic + 1024 bytes custom), 12 max

These settings look like this:

And the settings used by @clumsy-stefan (his screenshot):

[TD-er]

Please note, in the last commit, I have used the format @uzi18 uses for 24 tasks layout.

After dinner I can have another look at it.

[TD-er]

Hmm, just an idea here...
What if we place the offsets inside the settings?
Then reading the settings file will determine the position of the offsets.
This way we can make the settings interoperable between nodes without having to change the build itself.
At reset to factory default we can make a selection to change the nr. of tasks.

It will break backwards compatibility if you change the nr. of tasks.

[uzi18]

we can but we tried to avoid it, we have also in basic settings cfg file version so we can upgrade it in the fly but it is one way ticket, we can also warn user to do backup of old settings before upgrade or copy file and use new one

[TD-er]

Yep, setting it to a different value will break compatibility.
Maybe we could also have it changed back into the old version:

• loose all tasks > 12
• provided enough free space on SPIFFS.
• Conversion can only be done at boot, right before the settings will be made active.

Converting the settings is just a matter of moving to another offset of a block of data.
So we can split it and copy to a new file and rename the files.

This can also be used to convert from/to ESP32 settings.

I think we can then also change some sizes to make room for future upgrades?

This is the layout used on ESP32:

Edit: Hmm the settings struct on ESP32 is not the same size, so 1-to-1 conversion is not possible.
But if the settings file has the offset, we could extend the "Settings Archive" feature to copy task definitions from it instead of the entire file.

[clumsy-stefan]

What if we place the offsets inside the settings?

That would be a cool thing with the caveats that @uzi18 wrote...

Please note, in the last commit, I have used the format @uzi18 uses for 24 tasks layout.

So this would break backwards compatibility?

I had the issues described by @clumsy-stefan when trying his offsets.

I still don'tunsderstand why these things worked before the splitting of the files... this is what concerns me most. It did work without issues before, where else are such issues introduced with splitting the files that we don't captured (yet)?

For example the SHT30 problem, why it only happens after the cleanup commit? or why it only happens with task nr. 13?

[TD-er]

What if we place the offsets inside the settings?

That would be a cool thing with the caveats that @uzi18 wrote...

I don't think the caveats are that much of an issue, with proper warning and a "convert to 12 tasks" as fall back.

Please note, in the last commit, I have used the format @uzi18 uses for 24 tasks layout.

So this would break backwards compatibility?

With _your_ 24 tasks settings. So it was more of a warning to you.

I had the issues described by @clumsy-stefan when trying his offsets.

I still don'tunsderstand why these things worked before the splitting of the files... this is what concerns me most. It did work without issues before, where else are such issues introduced with splitting the files that we don't captured (yet)?

That's the whole reason I've been working on this the whole day, since I have the same feeling.
I had entirely different plans for today.

For example the SHT30 problem, why it only happens after the cleanup commit? or why it only happens with task nr. 13?

Can you test on a node where starting with clean settings isn't really an issue, to see if it still is an issue with the current parameters (as in my last commit)

[clumsy-stefan]

Can you test on a node where starting with clean settings isn't really an issue, to see if it still is an issue with the current parameters (as in my last commit)

tried this, completely factory resetted node with just an SHT30 connected...

same issues as before: task nr. 13 goes blank when pressing "submit" and SHT30 can't be read...

[uzi18]

@TD-er @clumsy-stefan isn't same issue as with dummy plugin?

[clumsy-stefan]

@uzi18 not sure if I understand? what you mean?

I just use the dummy plugin for demonstration, it does not happen with the dummy plugin on any other task, only if it's on task nr. 13

the same happens eg. with system plugin:

it does not happen on any other task, eg. 17:

[uzi18]

dummy has got one bug with settings saveing (this should be fixed with my PR) but we found also another issues with settings after digging in code (only possible to use same output data type on all).

[clumsy-stefan]

as you can see it also happens with the system plugin... and it ONLY happens on task nr. 13 and no other one... also it does not explain the issue with SHT30... all of these things that have not been there before the cleanup merge..

[uzi18]

dummy issues are before these changes, did not pull from git tree for some days, it is affected also on 12task version.

[clumsy-stefan]

did you read above? It's not (only) the dummy plugin and it happens only on task nr 13!!!! It's any plugin (in above screenshot the system plugin) and the SHT30.... however, let's see what @TD-er finds..

[uzi18]

@clumsy-stefan no dummy have this problem on every task nr., try with my change only one line of code:
https://github.com/letscontrolit/ESPEasy/pull/2601/commits/359fecff112669f1a15bf57969940a76e2052e6b

[clumsy-stefan]

@uzi18 sorry, but I think you don't understand... in my test it happens only on task nr 13 but with all different kind of plugins...otherwise it works fine, except for example the sht30 plugin....

[TD-er]

That's strange. I have it with the standard Custom.h settings and set the flag for 24 tasks.

With the settings suggested by @clumsy-stefan I got exactly as he described.
Are you sure you don't have your settings also in the Custom.h file?

[clumsy-stefan]

excerpt from my Config.h:

#define USE_NON_STANDARD_24_TASKS
/*
#ifdef TASKS_MAX
#undef TASKS_MAX
#endif
#define TASKS_MAX                          24 // max 12! 

#ifdef DAT_OFFSET_CONTROLLER
#undef DAT_OFFSET_CONTROLLER
#endif
#define DAT_OFFSET_CONTROLLER            DAT_OFFSET_TASKS + (DAT_TASKS_DISTANCE * TASKS_MAX)                        // each controller = 1k, 4 max 

#ifdef DAT_OFFSET_CUSTOM_CONTROLLER
#undef DAT_OFFSET_CUSTOM_CONTROLLER
#endif
#define DAT_OFFSET_CUSTOM_CONTROLLER     DAT_OFFSET_CONTROLLER + (DAT_CUSTOM_CONTROLLER_SIZE * CONTROLLER_MAX)  // each custom controller config = 1k, 4 max 
*/

#endif

as you see, it's commented out... except the new define...

[TD-er]

Hmm I made a clean build here and now I do have the same issues as you describe.
I will try to replicate what I did before dinner.

N.B. it also happens with the Switch plugin, so it is not related to the DHT3x plugin.

If I remember correctly it may also have been a define of the 24 task parameter at PIO level.

[clumsy-stefan]

Hmm I made a clean build here and now I do have the same issues as you describe.

at least no strange magic going on on my nodes 😄

I think you need to find what esentially changed before and after the cleanup merge...

thanks a lot for your help and efforts!!! really appreciated!!!

[TD-er]

I really think we're running into another bug regarding Arduino compilations.
I have now set all defines in the code to set the TASKS_MAX to 24 and it is working perfectly fine.
So #ifdef stuff to toggle defines is just not working like it should be.

The value of TASKS_MAX is clearly not used consistently throughout the compilation process (very likely the preprocessor)

So that's another day gone looking at something that's clearly a bug.

This makes you wonder what else is there in the code which is causing issues and is actually a compiler bug here.

I don't yet know what is actually going wrong, so I don't know how or where to report it.
But if this is the fix:

  #ifndef TASKS_MAX
    #ifdef USE_NON_STANDARD_24_TASKS
     #define TASKS_MAX                          24
    #else
     #define TASKS_MAX                          24
    #endif
  #endif

Then there is clearly something wrong here.
Apparently it is not (only) a bug in PlatformIO here, but clearly something is terribly wrong here when combining generated .cpp code from Arduino stuff, combined with defines in .h files.

I will now take a shower and go to bed and sleep on it what to do here.
I think the safest way is to move all defines a user wishes to make to a single file.
So either include that Custom.h file or use the ESPeasy files and no #ifndef....#def stuff, just have to make sure it is defined in the Custom.h or else it will not build.

Other way is to do it in PlatformIO and let it generate from a Python script, but then Arduino IDE support is gone.

But this way is just not working. It just is buggy as hell.

[clumsy-stefan]

first of all, thanks a lot for all your help, debugg and time you're investing. this is really much appreciated (by everyone Iguess)!!!

now for the issue, the (temporary) fix seems rather simple, so I?ll try that. Now for the last changes, could you revert back to the original laysout/offsets? I think there is currently no need to completely change the config file (offsets) if it's working with the original ones and the bug is somewhere completely different. also because we probably shouldn't change in multiple places at the same time!

have a good day!!!

[clumsy-stefan]

quick update: I can confirm @TD-er 's findings, changing the TASKS_MAX define to 24 in the above definitions (and revrting to the old offset's) makes everything working again, except that murphy decided to kill my SHT30 on my test node (still t.b.c.)...

However the uncertainity still remians in what other places where we have conditional defines the same problem occurs (but did not yet surface)... this could explain the larger-than-expected sizes for example...

[uzi18]

@TD-er should move all files to h/cpp, and check durring build if values of TASK_MAX changed somewhere

[TD-er]

@uzi18 Well that's probably what _should_ be done, but it's not what I'm going to do.
Problem is that we simply don't know yet what exactly causes these issues and I already lost a lot of time in this, which is very likely a bug in how Arduino converts its code to .cpp.

I will collect all defines which may be user defined and determine structs and put them in ESPeasy.ino. (not in .h file)
So then we will be sure they are being dealt with in a predictable manner and later I will decide on how to use the other core related stuff.

Something similar is also causing issues with forward declarations and global defined variables.
So it is good we now have somewhat clear what is happening here so we can make sure to use a work around for this.
But believe me that I have spent a lot of thinking about these issues last night and it feels a bit frustrated to realize we probably have a lot more hard to reproduce issues which are very likely related to these issues.

[uzi18]

If you move all other ino files to h/cpp/c you probably will resolve all these problems related to conversion and Arduino builder support this, just one more step left.
It is good part of work and your cplugin queue macros will work one more time as expected :)

[TD-er]

Thing is you're also converting all plugins and controllers then. That's kind of breaking with the origin of the project.
Not sure yet if that's wise to do.
The project is setup to have the plugins part completely separated.
Nothing defined in a plugin will be used elsewhere. So I think the plugins can remain .ino files.

[clumsy-stefan]

Nothing defined in a plugin will be used elsewhere. So I think the plugins can remain .ino files.

Are no crossreferences at all in Plugins? Especially MQTT stuff that is all over the project (or is that only in controllers)? Serial or GPIO stuff (Switch)?

Even if you would convert everything. can you be sure that these issues are gone then?

[TD-er]

Those parts use external files, some of which are already in .h/.cpp files and some still in .ino files.
The only tricky part may be the global defined variables.
Thing is with C++, if you define global variables (not members of a class/struct) in a .h file and you include that file more than once (even with #ifdef ... #define wrappers), you will create multiple instances of that same object.
So then you have two objects with the same name. That may give strange results.
So for example the ESPEasy-glbals.h file should really only be included once or else you will break everything.
Or you declare them in a .ino file, but then you cannot have an easy access to them from within .h/.cpp files.

So we do keep having possible pitfalls for issues like these as long as we keep merging .ino and .h/.cpp files in the same project.

As some kind of temp work around I created the fwdecls.h file to have some functions forward declared in a .h file so we can call them from within .h/.cpp files while the functions are still declared in .ino files.
That's the only way I could think of to not converting them all and still have a working project.
Well that was working fine, so it seems, until you showed the effects of the Arduino conversion + C++ preprocessor.

So for now I really don't know what will be the best here to do.

[uzi18]

@TD-er another solution - before build (with python) create new dir src_effective copy all from src dir there
all *.ino plugins copy to *.h files, next generate one file all_plugins.h with one line "#include "plugin001.h" per plugin, next at EspEasy.h add before first real function #include "all_plugins.h"

or just have all ino plugins in one known dir to convert

and we need to change build dir to another or change src dir name ;)

[TD-er]

@uzi18 That may work, but that's not how I would like it to work.
If something breaks in that process, then it is a hell to debug.
I'm not masochist enough for it I guess ;)

I was also thinking about another work-around, since the issue we're seeing appears to be related to multi-step interpretation of defines (actually replacing the defines with text strings by the preprocessor).
My idea is to have the 24 or 12 first set into another define itself and only using that define to initialize the TASKS_MAX value.

[uzi18]

one big advantage of all plugins/functions included in one file is better optimisation by compiler

this idea is only for ino plugins, controllers can be moved to h/cpp and I take here into account all core is h/cpp already - take it only as compatibility layer with plugins

[TD-er]

one big advantage of all plugins/functions included in one file is better optimisation by compiler

Not entirely sure, since the Arduino environment is also trying to do that in their build process.
They put it in .cpp file, which ofcourse need forward declarations like it was in a .h file.
Problem with functions defined in a .h file is that those functions are always compiled as if they were declared inline.
Which effectively means the function is not wrapped in a function call, but its internal is just copied into the function that calls it.

That's fine for functions which are really small like just returning a member value or something like that.
But for larger functions it does mean the executable will grow with every other call to that function from another location in the code.

A bit like template functions. For every new template argument there will be a new instance of the compiled code in the binary.

[uzi18]

even simpler just include every pl ugin without conversion, with
'''

ifdefs USES_Pxxx

include "pluginxxx.ino"

endif

'''

[clumsy-stefan]

before build (with python) create new dir src_effective copy all from src dir there
all *.ino plugins copy to *.h files, next generate one file all_plugins.h with one line "#include "plugin001.h" per plugin, next at EspEasy.h add before first real function #include "all_plugins.h"

this sounds really ugly (sorry not to take it personally) and for sure breaks building in every other environment than PIO. I guess....

what about putting the user-changable defines in one file and then just check if it's defined at all or throew error otherwise at build time (I think @TD-er already suggested something like this...

[uzi18]

already wrote it can be more simple and Arduino builder compatible

ifdefs USES_Pxxx

include ".. /plugins_ino/pluginxxx.ino"

endif

here: https://github.com/letscontrolit/ESPEasy/issues/2621#issuecomment-532694666

[uzi18]

@clumsy-stefan not a problem, just trying to put some ideas, @TD-er did lot's of work with code and testing with you and it is better to win this battle now than try another (random) time with wasting resources.

[clumsy-stefan]

already wrote it can be more simple and Arduino builder compatible

Hmm... but this doesn't affect on how the whole storage part is handled, where the issues with (wrongly defined) TASKS_MAX occurs... The Plugins are not the issue... or probably I misunderstand that comment...

[TD-er]

I think we're also slightly diverting in what each of us is trying to say :)

I think (hope?) keeping the plugins in .ino will not cause any of these issues.
If that appears to be true, then it should not be needed (yet?) to convert those to .h/.cpp too.
At compile time converting to some .h or .cpp is not the way to go, since that's also what Arduino is doing and they have (hopefully) far more knowledge about this than we have and their method is also not working like it should. (or my knowledge of C++ has some serious short comings)

If in the end we must convert the plugins to .h/.cpp too, then so be it, but then I will do it manually. This conversion will be slightly automated, but not automated as in a full automated conversion during each build.
Like I said that will then cause major headaches when something unexpected is happening.

What really bothers me by the way, is that I have the feeling I'm tracking bugs in a lot of 3rd party code for the last 2 years which is holding this project's progress back for a long time.

• Build issues (PlatformIO and now Arduino IDE so it seems)
• WiFi issues.
• LWIP issues.
• esp8266/Arduino issues.

These are happening at way too many levels, so new features, bugfixes and code optimizations in our own code are pushed back over and over again.

[clumsy-stefan]

@TD-er couldn't agree more... but I guess that's the problem using open source community code and libraries... you never now what's happening at other places...

still I think you (we) did make quite some advance over the last month's (years) probably not as fast as expected, but quality wise it has improved a lot. Eg. my nodes are running for weeks now (rather than hours or days) as some months ago!!! so just keep up the good work!! 😄

[TD-er]

but I guess that's the problem using open source community code and libraries... you never now what's happening at other places...

Well, at least with open source you can see what's going on.
I've also had to work around bugs in proprietary code and that's debugging with a black box.

[clumsy-stefan]

offtopic... but yes, that's why I also use more or less exclusively opensource and try to contribute if possible...

[TD-er]

Just an update, will commit my updated files soon.

I'm still not convinced that the Arduino preprocessor is off the hook here, since the hiding of compiler errors is caused by the way the Arduino code conversion operates.

On the other hand, the way we use the .h/.cpp files along with Arduino .ino files is also incorrect.

So, what is happening here?

The scope of a #define is only local to the file in which it is defined.
This means that if it is defined in a .cpp file, it is only present in that file, from the point where it was defined, until it is undefined, or the end of the file has been reached.

If you define something in a .h file, you cannot expect it to be present in other files, unless you include that .h file.

So by including the Custom.h file, the content of that file is copied by the preprocessor into the .cpp file that's being compiled.
This is something you really should understand to see all the implications of it.
It also has to do with variables defined in that file, not being in the scope of a class/struct.
For example, if you include file Foo.h which has variable int bar; declared to be globally defined, then every .cpp file where this Foo.h is included will have an instance of that same variable bar. So there are several versions of the variable with the same name.

Now back to the #define issues, combined with working in Arduino environment.
When compiling code in the Arduino environment, the .ino files are being concatenated one after eachother to a single .cpp file and the function declarations are being forward declared at the start of the file.
So at some time, the .h files are included and thus copied into the generated .cpp file by the preprocessor.
Meaning, the define will occur at some somewhat random position in the code.
All code following the initial define will have the initial value of the define replaced in the .cpp file.
Then the preprocessor will start over and tries to replace the other occurrences of the defined macro with the value of that macro, known at that time. (!!).
So what if we have some defined depending on USE_NON_STANDARD_24_TASKS, which is only known when we include the Custom.h?
Then the preprocessor may have replaced occurrences of your macro without knowing USE_NON_STANDARD_24_TASKS was defined.
So essentially the value for TASKS_MAX, which is depending on USE_NON_STANDARD_24_TASKS may have had the value 12 at first and 24 later on during the replacement actions of the preprocessor.

So what do we need to do here?

• include "ESPEasy_common.h" whenever you define something that may be defined also in Custom.h (which is included in ESPEasy_common.h ) (Do not include it in Custom.h)
• include the .h file which has the define you need, in any file you need it, right at the start of the file.
• Try to include as few .h files in a .h file as possible. Forward declare where possible and include .h files needed in .cpp files.

I will think about what to do with the global defined variables in ESPEasy-Globals.h.
C++17 has the option to define these using inline, which does help the linker to link those "global" variables. which does allow to include the ESPEasy-Globals.h multiple times.
But I'm not sure how many compilers already support this and it is still not really an elegant solution.
The best way would be to have a single (singleton?) object which has get/set functions to allow access to these variables, but then I think any file using them may need to be updated.
Or maybe I just declare them as extern and add a .cpp file for it to actually declare them.

Anyway, I am now more confident we're back on track here and maybe we also do fix some stuff without knowing it ;)
It does explain very well why sometimes a build is working fine and sometimes it isn't.

Edit:
Almost forgot to mention why I don't think Arduino's process is off the hook here.
The way how Arduino environments are being compiled does allow for very strange situations and since eventually the defines are made clear, they do not end up as being unknown. Thus effectively suppressing compiler errors.

One way this could be prevented is by not only collecting all function declarations and echo them as forward declarations in the generated .cpp file.
But the same could be done by all #include statements in .ino files.
If those are put in the beginning of the generated .cpp file, then defines will be known to the preprocessor right at the beginning

[clumsy-stefan]

A quick first check with a couple of nodes show that the new commits seem to work fine. Currently I can't see any strange things going on... will leave it running on these nodes and do some checking during the day.

@TD-er great work!!!

[clumsy-stefan]

ESP32 won't compile with USE_NON_STANDARD_24_TASKS due to line 66 in src/DataStructs/SettingsStruct.h. I guess the line should check for #if defined(USE_NON_STANDARD_24_TASKS) && defined(ESP8266).

[TD-er]

Will fix it, but why would you build ESP32 with USE_NON_STANDARD_24_TASKS?
To make settings exchangeable?
If so then it should not be limited to ESP32 I guess.

[clumsy-stefan]

hmm.. right, I guess after changing all the defines in my Config.h probably this is was defined even though it shouldn't... however the assert fails, as TASKS_MAX for ESP build is set to 32 regardless of USE_NON_STANDARD_24_TASK (which is ok), but then you should check everywhere for ESP8266 everywhere.

but the exchangable thing is a good point, why not...

[TD-er]

but the exchangable thing is a good point, why not...

Will do :)
Only thing is, it does then make the config file smaller (or at least it doesn't need 128k it is right now)

Maybe we should use different names for the files to indicate the nr of tasks?

• config.dat for 12 tasks
• config24.dat for 24 tasks
• config32.dat for 32 tasks

This will make it clear what file is compatible and which isn't.
We can also make conversions between them (in binary) and it makes detection of a config a lot easier and simpler to grasp.
So then I can make it dynamic at load which version should be used and we can do a manual convert to another version if required.
The newer firmware should be able to handle all and I think it can be done without much resources being used.

By the way, I managed to get rid of a few kB RAM being wasted, since some code was including the ESPEasy-Globals.h while it shouldn't.
This also may have caused some bugs in handling commands from rules when they depend on settings and also loading of ExtraSettings may have been buggy when called from commands (not sure if they did)

[clumsy-stefan]

Sounds great! Still lots of ideas to integrate though... Priorisation is everything 😄

I updated to the latest esp8266 core this morning, which unfortunately increased bin. size some 20k... My 1M builds are by now some 40k too big for 2step OTA... so saving binary size is still an important thing... on the other hand, I started to throw out 1M units anyways, the hazzle with OTA updates is just too much and all newer units have anyways much more mem...

[TD-er]

If the chips can be reached, you can also replace the flash chips.

Still size reduction is important, since also the dev and test build approach the limits.

[clumsy-stefan]

Did that with the sonoff-basics... but with the 4ch. and dual's I couldn't find the mem chips... I think it uses only the internal mem... but even with the basics it's much more complicated to exchange the mem chip than buying the pow R2 with is a few dollars more but therefore gives power measurments (ans has 4M mem)

[TD-er]

The ESP8285 has 1M flash internal.

[clumsy-stefan]

the ESP8266 also, not? if not then it's probably a 8285..... but I'll throw them out anyways... there are enough 4/8/16M bpards out there...

[TD-er]

Can you test what I have committed so far?

[clumsy-stefan]

I flashed all my units with the commit from this morning (I'm currently away, so I can't say which one exactly that is).

Currently all units are running fine except the two deep sleep units that don't deliver the analog value when walking up.

I'm back tomorrow an can report more then.

[clumsy-stefan]

All my (40) nodes are now running for some 24h with up to commit #213fae7718dd614e2ded0feebe3015c45b8be42e without issues. also included PR #2605 for the dallas improvments.

I'll add the new commits now since then and update some nodes...

[TD-er]

Things you need to check are:

• Consistent behavior when using rules to run the same commands via web page (or http requests)
• Save/update settings (also with 24 tasks)
• Commands via controllers (only a few are capable of handling commands)
• Check memory usage for rules execution (also longer lines with comments or lots of indents at the beginning of the line)
• Check handling of nested ifs in rules (and memory usage)
• Check new "int" variables. They use the same data as the var ones, but then formatted to int when used.
• Check the formatting of variables (which is finally documented in the docs in this PR) and their memory usage.

[clumsy-stefan]

Ok, I see how far I get... I'll try as much as possible.

one thing I can already say (I'm still compiling) is, that all the includes need to be changed to relative paths if it should work out of the box with arduino. Eg. all "src//.h" need to be "..//.h" and stuff that is in the top directory need to have "../../fwdecl.h" or similar... I think I found all of them now and it's compiling..

[TD-er]

OK, good to know.
Arduino IDE is a strange tool.

[clumsy-stefan]

I think that's just gcc/C... includes with " are alwys relative to the directory where it's included or you have to specify with -I additional directories to search for includes..

[clumsy-stefan]

after a few tests with my test nodes everything seem to be fine..

Problem is, that I don't use MQTT at all, so I can't really say anything about that. But at least I use some 20 different plugins/sensors and remote commands, rules etc. so I think it's quite a good test. Also I have 4 different type of nodes and running 40nodes in parallel...

I'm going to update the rest of them (so rules and external commands are executed and testted) and will report back.

[clumsy-stefan]

A few quick remarks after updating:

• espota only seems to work ond ESP32 units not on ESP8266 (already before this build), on the ESP8266 you need to update via /upgrade weblink.
• crc.py does not seem to work on ESP32 builds (also already before these changes)
• with these commits included deepsleep units (which wake for 15sec at a time) either only sned the first task if multiple tasks have the same name or does not send some tasks at all (still try to verify). Interestingly this happens only on Wemos D1 Mini nodes not on the Wemos D1 Pro nodes. where the only difference is the available memory (4M/16M).

I'll graph the memory, rssi and CPU usage and will report on this after they have run for a day or so.

[clumsy-stefan]

@TD-er I found at least one issue which is different from before. On deepsleep units it seems that the tasks are (sometimes) executed before the wifi connection is establieshed (especially when it takses a litte longer for the connection), therefore not all values are sent to the controller (see log, events occur before connection). Also the analog value is not available or so at that time..

I think this should be adjusted somehow.

INIT : Booting version: SMY_5.11DP Sep 22 2019 10:54:49 (ESP82xx Core 5920ce61, NONOS SDK 2.2.2-dev(38a443e), LWIP: 2.1.2)
70 : INIT : Free RAM:32248
71 : INIT : Rebooted from deepsleep #8 Last Task: Background Task - Restart Reason: Deep-Sleep Wake
73 : FS   : Mounting...
95 : FS   : Mount successful, used 75802 bytes of 957314
167 : FS   : Success garbage collection
242 : FS   : Success garbage collection
318 : FS   : Success garbage collection
657 : CRC  : program checksum       ...OK
688 : CRC  : SecuritySettings CRC   ...OK 
786 : INIT : Free RAM:28880
788 : INIT : I2C
790 : INIT : I2C custom clockstretchlimit:1000
791 : INIT : SPI not enabled
793 : EVENT: System#NoSleep=15
803 : INFO : Plugins: 26 [Normal] [4M OTA SMY] 24tasks (ESP82xx Core 5920ce61, NONOS SDK 2.2.2-dev(38a443e), LWIP: 2.1.2)
805 : EVENT: System#Wake
912 : WIFI : Set WiFi to STA
944 : WIFI : Connecting clumsy_ap2 attempt #0
948 : OTA  : Arduino OTA enabled on port 18266
963 : EVENT: System#Boot
1070 : WIFI : Connecting clumsy_ap2 attempt #1
1211 : SYS  : 31.00,26408.00,100.00,0.00
1217 : EVENT: sysinfo#rssi=31.00
1245 : EVENT: sysinfo#freeheap=26408.00
1250 : EVENT: sysinfo#load=100.00
1254 : EVENT: sysinfo#uptime=0.00
1404 : SYS  : 1.00,3376.00,0.00,0.00
1407 : EVENT: sysinfo#web=1.00
1433 : EVENT: sysinfo#freestack=3376.00
1437 : EVENT: sysinfo#ip3=0.00
1440 : EVENT: sysinfo#ip4=0.00
1586 : ADC  : Analog value: 0 = 0.000
1589 : EVENT: sysinfo#lipo=0.00
3110 : WD   : Uptime 0 ConnectFailures 0 FreeMem 25112 WiFiStatus 6
5029 : WIFI : Connected! AP: clumsy_ap2 (E4:8D:8C:49:4F:BA) Ch: 5 Duration: 3756 ms
5030 : EVENT: WiFi#ChangedAccesspoint
5055 : EVENT: WiFi#ChangedWiFichannel
5057 : WIFI : DHCP IP: 10.0.10.116 (wemos-mini-16-16) GW: 10.0.0.2 SN: 255.255.0.0   duration: 125 ms
5094 : NTP  : NTP replied: delay 20 mSec Accuracy increased by 0.628 seconds
5095 : Time adjusted by -1569162449568.58 msec. Wander: -435878458.21 msec/second
5097 : Current Time Zone:  DST time start: 2019-03-31 02:00:00 offset: 120 minSTD time start: 2019-10-27 03:00:00 offset: 60 min
5100 : EVENT: Time#Initialized
5104 : EVENT: WiFi#Connected
5110 : Webserver: start
5111 : firstLoopConnectionsEstablished
5113 : EVENT: System#NoSleep=15
5118 : WD   : Uptime 1 ConnectFailures 0 FreeMem 19368 WiFiStatus 3
5121 : EVENT: Clock#Time=Sun,16:27
20116 : EVENT: System#Sleep
20141 : SLEEP: Powering down to deepsleep...

[TD-er]

One thing that has been changed is when tasks have the same name.
I'm now using a cache to keep track of what task index has what name.
So it will only refer to the first one to match a name.
Not sure if it should match only enabled ones, or just looking at the name.
But you're being warned when trying to save a task with a name that's not unique, so I guess it has been made clear those should be unique.

[clumsy-stefan]

hmm.. yes, but it worked before ;) the problem is, when I use multiple names for the same "thing" (eg. system values) I get 2 different "devices" on my server and need to do silly workarounds there... (each task name refers to a single device in fhem)... so, when I need more than 4 system values I get them in different devices serverside..

however what's stange is, that I have the same things defined in my wemos-pro deepsleep node, where it still works as before!

Also if you look at the serial log above, the ADC is read and creating an event, but it's always 0! It seems the ADC is taking longer to initialize. Obvisously the same with the IP-Adress, as it's run before the connection is set up it always returns zero.

On deep-sleep units, probably the tasks should not run immediately, but som time later, like after 2 sec. or even something like awake-time/2 so all the sensors get time to power-up and initialize (and WLAN gets time to connect). just as a proposal...

EDIT: obviously there is a "workaround" for this. If you set the "send to controller" value small enough (eg. 2sec. when staying 15sec. awake) then it will send the values multiple times, and at some point, the connection is established and the sensors alive....

[TD-er]

When waking from deepsleep, all tasks are executed at least once, so that's not addressed by "calling" the values.
ADC is working when powered on long enough?
One of the things I also changed (not sure if it is in this PR, or changed before) is that the ADC is not read when WiFi connection is being executed. (connect to AP)
This because the WiFi RF calibration is also using the same ADC, so a read value can be off quite a bit or the ESP may crash.

[clumsy-stefan]

ADC is working when powered on long enough?

yes, or probably as you write, after WiFi is established (RF Calibration?)...

When waking from deepsleep, all tasks are executed at least once, so that's not addressed by "calling" the values.

Not sure if I understand correctly, but yes, the tasks are called, but even before the WiFi connection is established (see log above), therefore some values (like RSSI, IP, etc.) are not yet initialized. That's why I think it should get more time... Only for "standalone" units which do not make any (wifi) connection, it doesn't care if already conencted or not...

but as said, as a workaround, I set the "send to controlelr" values to 3 sec. and the awake time to 15, so the tasks get executed at least once, after the wifi has been initialized.

[TD-er]

Yep, a task should always be executed, or else no measurement will be taken if the WiFi cannot connect.
That's a bit nasty when the same ESP node should switch on/off something based on measurements taken by a sensor on the same node.

There is a wifi connected event, which you can use in the rules.
Oh and the wake time is "reset" when the connection is made, so you can also play with that setting.

[clumsy-stefan]

That's a bit nasty when the same ESP node should switch on/off something based on measurements taken by a sensor on the same node.

or if you power-on with a GPIO set an external sensor, and only want to take the reading after some init-time..

therefore the tasks in the "single-run" should not be started as first thing... probably even just before going to sleep...

[clumsy-stefan]

Memory seems ok with the latest commits:

the update was this morning at 11... no visible increas or similar on any node.

[TD-er]

Hmm, I would expect an increase of free RAM by about 2 kB at least.
Your charts don't show that.

[clumsy-stefan]

probably because I use a different set of plugins and no MQTT... depends where you would expect the increase... the graphs shows the free heap!

but still, after 12 hours running, I think that's a very good PR and a lot of improvments...

you would (from my pov) just need to make sure that the includes are all right (I had to chang about 25 files... other thatn that I think it could be merged, if someone els tests the MQTT part...

[TD-er]

Well, problem was that the ESPEasy-Globals.h was included from more than 1 file.
And since it had the global variables, it means variables declared in there would be allocated more than once.
One of the variables is the Settings struct, which alone is over 2 kB.

I am now (in this PR) declaring the global variables in the header files using extern and then construct them in a .cpp file. This means you can then include such a header file more than once and it will be the same instance of the object you will be referring to.

Edit:
Hmm... Just at the moment I press the submit button of my reply I realize what's going on.
Those global variables are allocated on the stack, not the heap.
And not just any stack, but the sys stack (Core library territory)
We use the cont stack, which is just 4 kB.
N.B. This reduction may also prevent issues when the sys stack reaches stack overflow.

[uzi18]

nice to see you found solution for header files and objects so now road is more straightforward then before =)

[clumsy-stefan]

I had not a single unit rebooting (due to HW-WD or similar) since updating yesterday, so that's already an improvment!
As soon as you have pushed this new commit I can recompile and try again.

[TD-er]

I added a new commit which has even more stuff stripped from the ESPEasy-Globals.h into separate file.

@clumsy-stefan Would be nice if you could also so your tests with the last one, mainly to see what has to be changed in the #include paths.
If you then could make a PR based on the last commit, then I can merge it this evening.
This PR is already getting quite big and more changes could end up making it very hard for PRs to be merged.

If the fixes of include paths is as simple as renaming everything in a subdir from #include src/ to something like #include ../ then it is just a matter of some very basic query replace.

Edit:
I made a new commit to make the include paths relative to the file location.

[clumsy-stefan]

Your're latest commit is more or less what I had in my stash before. Basically replacing all src/ with ../ as you wrote..

I have only one more change in SettingsStruct.hline 66 changed from
#ifdef USE_NON_STANDARD_24_TASKS
to
#if defined(USE_NON_STANDARD_24_TASKS) && defined(ESP8266)

and I added an error line in _P068_SHT3x.inoline 192:
addLog(LOG_LEVEL_ERROR, F("SHT3x: not initialised!"));

I'm going to compile, install and test it now...

[clumsy-stefan]

@TD-er commit 0b24c7179b637cfeb62720b0e02877bef32ac52d has src/ again in the includes....

EDIT: also src/Globals/MQTT.h and src/Globals/Plugins.cpp are missing a ../../ in the include for define_plugin_sets.h and ESPEasy_plugindefs.h

[TD-er]

@TD-er commit 0b24c71 has src/ again in the includes....

That's in files which are in ESPEasy/src. so the file they include actually has src/.... as prefix.

[clumsy-stefan]

That's in files which are in ESPEasy/src. so the file they include actually has src/.... as prefix.

hmm... No, it's in files that are in src/Globals/ (see above)

[TD-er]

Then it must be a different commit, the one you link only has .ino files in it.

[clumsy-stefan]

Sorry, you're right, these ones are ok.. It's the two files mentioned above (from a different commit) which are wrong... sorry..

[TD-er]

Ah you had edited the comment...
Will add them in a new commit

[clumsy-stefan]

yes... also if you could change the define in SettingsStruct.hon line 66... to make it compile on esp32...

[TD-er]

Done in https://github.com/letscontrolit/ESPEasy/pull/2617/commits/75f24711be05e92a770dc5e2221a582e651c37ca

[clumsy-stefan]

wow, you're quicker commiting source changes, than me writing comments 😀

Commit 75f24711be05e92a770dc5e2221a582e651c37ca makes it compile again for ESP32 and ESP8266.. So I'm going to flash some units now, do some tests and tehn flash the rest of them to see how they run with commands, rules, sensors, etc...

PS: crc32.py does not seem to work on esp32 builds, if I'm correct?

[TD-er]

I have never tested that Python script on ESP32, or at least never checked it.

[clumsy-stefan]

FIrst tests show, that all my unit's seem to run fine... commands via controller (fhem) and via web-intrerface seem to work, also the basic rules I have... Memory graph shows more or less flat...

Only issue with the deep-sleep units as descibed. tasks are run before the connection is established therefore no ADC, RSSI, IP, etc. values!

I'll keep it running and can report tmorrow if there were any reboots or anything...

Great work!!!

[TD-er]

Not bad for a pull request containing:

• Commits 47
• Files changed 179
• About 200 comments

:)

[clumsy-stefan]

did you test the MQTT part? I can't do that one...

[TD-er]

Did you also test changing settings on ESP32?
I'm changing the controller and then the settings of a plugin get mangled.
Also the other way around, if I edit settings of a plugin, the controller settings get corrupted.

[TD-er]

Ah, bug found....

Thats not good.....

Found it where it was mangled.
Apparently the labels and values got mixed up in the StorageLayout.h
Will test it again, but this should now work fine again.
Will commit in a minute, but I guess it is best not yet to merge this evening.

[clumsy-stefan]

Oh, as I only have one esp32 I did not check this... but good you found it ;) I'll test later on.

Generally I'm still a bit undecided if I should move to esp32 or stay with esp8266.

Regarding esp32 there are a couple of things that I found:

• crc32.py does not work, so you can't sign the binaries
• arduinoOTA only works on esp32 not on esp8266 (there it says it uploaded and reboots, but comes up with the old version)
• I have boards that use multiple I2C buses (eg. one for sensors and another one for the OLED). Generally it would be cool to be able to select "which" I2C should be used per plugin instead of centrally.. But I guess that would make the binary bigger, which is not that much of a problem for esp32 as it normally has much more memory.

[TD-er]

Arduino OTA can be enabled on the ESP8266, but isn't by default.
It does take quite a bit of resources.

I'm also doing some tests here on ESP32 and the builds are quite stable there.
Only thing is the WiFi response time. The ESP32 does not send Gratuitous ARP packets, so it may act slow sometimes.
But it is really stable in my tests.
Also nice that it has way more memory, GPIO pins and 3 HW uarts.
Also proper (de)bounce detection in hardware.
It also has Bluetooth :)
I do not yet use the 2nd core, since we don't have proper locking in our data structures.

And some boards have PSRAM, which allows to have 4 MB of heap or even more. I already saw some boards with 8 MB PSRAM or even more.

I have one board here running GPS on ESP32 and it is really really stable.

[clumsy-stefan]

Arduino OTA can be enabled on the ESP8266, but isn't by default.

I have it enabled, and it works (espota.py tells ans shows me it uploads succesfully) after that it reboots, but stimm with the old version active... I have no clue why it doesn't work..

[TD-er]

The default port was 8266, if you also use that port for ESPeasy p2p, you have a conflict :)

I just tried ArduinoOTA on my ESP32 unit and it does work, but had to try it at least 3 times to complete.
The last time I did send pings during the update and it did succeed. (not sure if that was the reason it worked)

[clumsy-stefan]

I use non-standard port and upload succeeds on both, esp32 and esp8266, but only on esp32 it gets activated, on esp8266 it just loads the old version....

[TD-er]

The big PR has been merged.

[clumsy-stefan]

Just compiled all versions I need and updating all units currently (after doing some basic tests on a couple of them).

The provious version (with these commits) has been running on all nodes without issues, reboots or similar for the last 2 days, so this (for me) really seems to be an improvemnt!

[TD-er]

Good to hear :)
I really hope we can now leave those strange reboots behind and move forward.

[clumsy-stefan]

No reboots of any unit for the last 3 days, uptimes >7'000min.... these changes definitely improved the build a lot!! thanks again

[ps915]

I searched the whole repo for #define TASKS_MAX 12 to set it to 24, but i cant find the file where i must be changed.

Where can i find "#define TASKS_MAX"?

[TD-er]

@ps915 See my reply to your question on the forum

[ps915]

@TD-er thanks for your reply. I set up my Arduino IDE and compiled the original version successfully. But how do i use those buildflags to get 24 devices supported!

[TD-er]

@ps915 I have not tried it in ArduinoIDE, but my guess would be to have this as the first lines of the ESPEasy.ino file:

#define USE_NON_STANDARD_24_TASKS
#define TASKS_MAX  24

[clumsy-stefan]

Or you can add -DUSE_CUSTOM_H to the ArduinoIDE Build Flags (eg. make your own platform.local.txt) and then add the following line in Custom.h:
#define USE_NON_STANDARD_24_TASKS

(that's how I do it....)

[TD-er]

That's even better.
That means you don't need to change the code itself, which can be forgotten.

[clumsy-stefan]

Not quite... The platform.local.txt needs to be in the same directory as the esp8266 core resides (depending on the OS you use you have to look in the subfolders of the ArduinoIDE). Look for a file called platform.txt within ArduinoIDE directory and put the platform.local.txt in there. You'll find instructions on which variables you can alter in platform.txt. As a kickstart see my platform.local.txt below...

compiler.c.extra_flags=-DUSE_CUSTOM_H -DPUYA_SUPPORT=0 -DCONFIG_FREERTOS_ASSERT_DISABLE -DCONFIG_LWIP_ESP_GRATUITOUS_ARP -DCONFIG_LWIP_GARP_TMR_INTERVAL=30 -DCORE_POST_2_5_0 -DCORE_POST_2_6_0 -DNDEBUG -DICACHE_FLASH -DWEBSERVER_RULES_DEBUG=0 -DVTABLES_IN_FLASH -mtarget-align -Werror=unused-variable
compiler.c.elf.extra_flags=-DUSE_CUSTOM_H -DPUYA_SUPPORT=0 -DCONFIG_FREERTOS_ASSERT_DISABLE -DCONFIG_LWIP_ESP_GRATUITOUS_ARP -DCONFIG_LWIP_GARP_TMR_INTERVAL=30 -DCORE_POST_2_5_0 -DCORE_POST_2_6_0 -DNDEBUG -DICACHE_FLASH -DWEBSERVER_RULES_DEBUG=0 -DVTABLES_IN_FLASH -mtarget-align -Werror=unused-variable
compiler.S.extra_flags=-DUSE_CUSTOM_H -DPUYA_SUPPORT=0 -DCONFIG_FREERTOS_ASSERT_DISABLE -DCONFIG_LWIP_ESP_GRATUITOUS_ARP -DCONFIG_LWIP_GARP_TMR_INTERVAL=30 -DCORE_POST_2_5_0 -DCORE_POST_2_6_0 -DNDEBUG -DICACHE_FLASH -DWEBSERVER_RULES_DEBUG=0 -DVTABLES_IN_FLASH -mtarget-align -Werror=unused-variable
compiler.cpp.extra_flags=-DUSE_CUSTOM_H -DPUYA_SUPPORT=0 -DCONFIG_FREERTOS_ASSERT_DISABLE -DCONFIG_LWIP_ESP_GRATUITOUS_ARP -DCONFIG_LWIP_GARP_TMR_INTERVAL=30 -DCORE_POST_2_5_0 -DCORE_POST_2_6_0 -DNDEBUG -DICACHE_FLASH -DWEBSERVER_RULES_DEBUG=0 -DVTABLES_IN_FLASH -mtarget-align -Werror=unused-variable
compiler.ar.extra_flags=
compiler.objcopy.eep.extra_flags=
compiler.elf2hex.extra_flags=

Probably you need to adapt the defines to your likes...

[TD-er]

@clumsy-stefan Maybe you can make some tutorial for compiling in ArduinoIDE?
Then we can add it to the documentation.

[clumsy-stefan]

@TD-er sure, I can try... I'm just a bit short on time currently...

[ps915]

@clumsy-stefan That sounds like the perfect solution.

What i have done so far (maybe usefull for a possible wiki entry)

1. Set up ArduinoIDE like this: https://www.letscontrolit.com/wiki/index.php/Tutorial_Arduino_Firmware_Upload
   install ESP8266 2.5.2 or higher
2. create a new blank file called platform.local.txt in the folder:
   arduino-1.6.12P230 -> Portable -> packages -> esp8266 -> hardware -> esp8266 -> 2.5.2
3. added following lines to platform.local.txt
   _compiler.c.extra_flags=-DUSE_NON_STANDARD_24_TASKS -DTASKS_MAX=24
   compiler.c.elf.extra_flags=-DUSE_NON_STANDARD_24_TASKS -DTASKS_MAX=24
   compiler.S.extra_flags=-DUSE_NON_STANDARD_24_TASKS -DTASKS_MAX=24
   compiler.cpp.extra_flags=-DUSE_NON_STANDARD_24_TASKS -DTASKS_MAX=24
   compiler.ar.extra_flags=
   compiler.objcopy.eep.extra_flags=
   compiler.elf2hex.extra_flags=_
4. recompiled .bin file via ArduinoIDE: Sketch -> Export compiled Binary
5. copy the compiled Binary file (.bin) in the same folder where FlashESP8266.exe is located
6. Do a Factory Reset of the ESPeasy Node, if you have used it before (Tools -> Factory Reset -> Factory Reset)
7. Use FlashESP8266.exe to upload the Firmware

[clumsy-stefan]

Seems OK to me. DId you restart ArduinoIDE after creating platform.local.txt?

Also the directory for platform.lcoal.txt shoudl be something with esp8266 in it.. You'll find a platform.txt for every architecture you have installed... So in the hardware folder of your installation there should also be the esp8266 structure. you need to put the local platform file in there...

[TD-er]

Maybe you should also execute a 'factory reset' on the ESP node.
Not sure if the ESP will handle file version difference very well.
But as long as you don't see 24 tasks, it does not matter if you factory reset or not.
Just don't use old settings on a 24-tasks config.

[ps915]

Okay, i got it working. I edited my comment above, so other users can follow this mini guide.
By the way, if i declare "#define TASKS_MAX 24" in the custom.h i wasn't able to compile. Instead i put -DTASKS_MAX=24 as a buildflag!

I also wanted to preset my WiFi Settings (SSID & password) and also the DEFAULT_NAME in Custom.h.

Unfortunately the ESP always goes into configuration Mode and creates a hotspot where i have to configure the ssid and password. Also the name wasn't set! Any idea how i can actually use the Custom.h at its full potential? Before flashing, i alsways do a factory reset!

[TD-er]

Any idea how i can actually use the Custom.h

-DUSE_CUSTOM_H as @clumsy-stefan also mentioned.

[clumsy-stefan]

#define USE_NON_STANDARD_24_TASKS should be enough, you don't need to define TASKS_MAX in Custom.h , this gets automatically defined later on.

Use

#ifdef DEFAULT_SSID
#undef DEFAULT_SSID
#endif
#define DEFAULT_SSID        "my_SSID"    // Enter your Wifi network SSID 

etc. in Custom.h for all defaults you want to change.

[ps915]

I did, but that does not work. I just used the buildflag -DUSE_CUSTOM_H and edited "Custom.h" with
#define USE_NON_STANDARD_24_TASKS
I was able to compile it. Still 12 Sensors.

When adding #define TASKS_MAX 24 to "Custom.h" i could compile.

Thats because i now use these twi buildflags, which work:
compiler.c.extra_flags=-DUSE_CUSTOM_H -DTASKS_MAX=24

[TD-er]

If you add -DTASKS_MAX=24 to the extra flags, I would suggest also to have
-DUSE_NON_STANDARD_24_TASKS added there.
Just to keep them together.

[clumsy-stefan]

Hmm... you shouldn't need this... It should work by only adding
#define USE_NON_STANDARD_24_TASKS in Custom.h as then TASKS_MAX gets correctly defined in src/DataStructs/StorageLayout.h.

There has to be something else which is going wrong (which probably also explains why you're other defaults dont' get defined)!

EDIT: are you yure your Custom.h gets included?

[ps915]

Did a fresh reinstall and now everything works as expected! I updated the guide above just for the 24 Tasks.

[clumsy-stefan]

@ps915 good to see that it works, but the guide is not really how it should be done... The defines should only include the '-DDUSE_CUSTOM_H' and only 'USE_NON_STANDARD_24_TASKS' should be defined in 'Custom.h'. If you define "TASKS_MAX' yourself it could fail in a later stage when something changes (as it's set in 'src/DataStructs/StorageLayout.h' depending on other things. Eg. if you compile for ESP32 it will fail like this!

Also if you want to change other definitions (like SSID etc) you would always need to change the 'platform.local.txt' instead of using it with the provided examples for 'Custom.h' (which is why it was created and included in ESPEasy). There it's also visible which definitions can be changed and which shouldn't

Just IMHO....

[uzi18]

@TD-er

diff --git a/src/src/DataStructs/StorageLayout.h b/src/src/DataStructs/StorageLayout.h
index c2778912..368a202f 100644
--- a/src/src/DataStructs/StorageLayout.h
+++ b/src/src/DataStructs/StorageLayout.h
@@ -120,6 +120,21 @@
   #   define CONFIG_FILE_SIZE                65536
   #  endif // ifndef CONFIG_FILE_SIZE

+  # elif TASKS_MAX == 24 // ifdef USE_NON_STANDARD_24_TASKS
+
+  #  ifndef DAT_OFFSET_CONTROLLER
+  #   define DAT_OFFSET_CONTROLLER            4096
+  #  endif // ifndef DAT_OFFSET_CONTROLLER
+  #  ifndef DAT_OFFSET_CUSTOM_CONTROLLER
+  #   define DAT_OFFSET_CUSTOM_CONTROLLER     8192
+  #  endif // ifndef DAT_OFFSET_CUSTOM_CONTROLLER
+  #  ifndef DAT_OFFSET_TASKS
+  #   define DAT_OFFSET_TASKS                16384
+  #  endif // ifndef DAT_OFFSET_TASKS
+  #  ifndef CONFIG_FILE_SIZE
+  #   define CONFIG_FILE_SIZE                65536
+  #  endif // ifndef CONFIG_FILE_SIZE
+
   # else // ifdef USE_NON_STANDARD_24_TASKS

   #  ifndef DAT_OFFSET_TASKS

without it task data overlap controller data, will prepare PR with this if you want?

[TD-er]

Hmm, I only see + symbols in the diff output.
What makes it different from the current setup?

[uzi18]

this adds my layout as standard for 24 tasks on esp8266

[TD-er]

Ah, so you are still using your own layout ;)

[uzi18]

because actual standard is malformed, did you tried grapical representation of layout when tasks_max is 24?

[TD-er]

Yep, please make a PR for it, but also add a comment that it's the "Uzi18 layout" with TASKS_MAX set to 24.
But then you are not using the USE_NON_STANDARD_24_TASKS define, so don't you run into other issues then?

[TD-er]

The layout with USE_NON_STANDARD_24_TASKS should not overlap. Does it?

[uzi18]

no nothings wrong with this
we can use this layout for tasks_max > 12 it is fully extensible, not only for 24 tasks

we can also think about hybrid configuration where 13+ tasks data are stored at the end of file, so we will get backword compatibility, but this need some additional logic in save/load functions

the best should be to have same layout for esp8266 and esp32, than add upgrade button for configuration file, to store it with next free ID so we will know it is new layout incompatible with older if you know what i am talking about :)

[TD-er]

TASKS_MAX is also used in arrays in the SettingsStruct.
So there is no simple transition anyway between settings.

That's why I made the SettingsStruct templated based on the TASKS_MAX, so we can make a transition in software.

[uzi18]

@TD-er it depends if someone will try scenario like USE_NON_STANDARD_24_TASKS upgrade from 24 tasks to 36 configuration file will be malformed, because controller data are stored at the end of file

[TD-er]

And that's why I asked for a proper layout suggestion when we were working on the issue.
Now we're running into a lot of options and with more than "a few" options, it is next to impossible to make a conversion based on templated structs. Then we must make a very elaborate function to convert the settings, since otherwise all supported structs will take up space in the binary

[uzi18]

for now standard 24 task will be malformed

but we can try to rework transition to official and wide platform compatible solution

I'm open to your suggestions here

[TD-er]

What do you consider "standard 24 task" configuration?
I was under the impression we had made a "standard 24 task" config with the (total mismatch named) define USE_NON_STANDARD_24_TASKS ?

[uzi18]

exactly i say standard because name of define is USE_NON_STANDARD_24_TASKS

[TD-er]

Well that name was chosen to suggest: "It is possible, but we don't officially support it" :)

[uzi18]

ok so i missed this, so. maybe it is better to not PR my changes

[clumsy-stefan]

I also use the USE_NON_STANDARD_24_TASKS for nearly all my units without any issues (since years now). I don't see overlaps or any other strange issues!

So I suggest if you want to change the layout completely it should be either upgradable or backwards compatible!

Just IMHO...

[TD-er]

Well, I was thinking about making some upgrade path possible for the settings and maybe even allow to use 24 tasks as default, but then we may run into issues with 1M modules.
I think it would be a great feature if we can even switch between ESP8266 and ESP32.

But the conversion is something that's only needed for a few people and it may add a few kB to the size of the binary
I guess it would make sense to have the 12 task setup on 1M units, especially the "minimal OTA" ones and maybe allow the 24 or 32 task ones on units with more room.
Still the more tasks, the more RAM is needed for the settings struct.

[clumsy-stefan]

agree on that...

therefore coming back to some old discussion about settings import/export (eg. in JSON, XML, or some other readable format) which wouwld make migration easier as well as programatically configuring units!

[TD-er]

And JSON/XML export of the settings is also taking a lot of string storage in the binary.

[clumsy-stefan]

hmm.. can't we reuse the json status/readings that's already in there?

[TD-er]

Those are not complete.

[uzi18]

couldn't we put such scripts on github/wiki and fetch into browser page when needed?

[TD-er]

Yes we can, but I don't feel that comfortable in JavaScript.