Espeasy: Adding a new device does affect a next device's settings

[ghtester]

Summarize of the problem/feature request

When you add a new device above existing one, the name of already existing device is cleared (maybe some other settings could be affected as well)

Expected behavior

Adding a new device shouldn't affect other devices at all.

Actual behavior

Next device's name is cleared when a new device is added above.

Steps to reproduce

Add a Display - LCD2004 device at position 12, name it LCD.
Add a Communication - IR Transmit device at position 11 and name it IR.
The name of Display - LCD2004 device at position 12 disappears.
Add a Communication - TSOP4838 device at position 10.
The name of Communication - IR Transmit device at position 11 disappears.

Yes the problem persists after powering off and on - the settings was overwritten.

The same issue after restart.

System configuration

Hardware: NodeMCU Amica

BUILDS! --->
20102 - Mega;
official mega-20180503, mega-20180502 (but it may be an old bug)
4096 dev
ESP Easy version:
Plugins | 72 [Normal] [Testing] [Development]
ESP82xx Core 2_4_1, NONOS SDK 2.2.1(cfd48f3), LWIP: 1.4.0-RC2

[giig1967g]

I can confirm this behaviour

[TD-er]

I have seen something similar a while ago, but could not reproduce it then.
Does it also happen when you add some plugin that does not have a lot of settings?

[susisstrolch]

If you add device 12 the Controller 1 Settings are overwritten too.
Doesn‘t depend on Device size.
Steps on fresh ESP:
Add Controller 1 - FHEM
Add Device 12 - Dummy

FHEM Settings are gone.

[giig1967g]

I can reproduce it very easily:
create a device on task=2 (dummy device) and save
then go back to the main devices page and press "edit" for task 1.
Then select any device (in my test I chose Sysinfo), at this moment (even before saving the changes) the task name and value names of task number 2 are deleted (to check this open two browser windows and check)

[TD-er]

I will add some overlap checks to the data store to see if some offsets are defined too small. Maybe the settings struct has become too big.
That would explain corrupted settings we see happening lately.

[s0170071]

JSON settings... Told ya

[TD-er]

And there will be....

[TD-er]

@Oxyandy Did you test the wifi issues with something set as 12th task/device?

As a test, I added a 12th device and now it cannot make connection anymore.

[TD-er]

I was just thinking about this issue.
The settings are saved via partial updates.
Maybe such a partial update is writing more than requested and writes zeroes to the trailing bits to write a complete sector.

[Grovkillen]

Now that would explain a lot @TD-er !

[Grovkillen]

Could we make a debug version that dumps the binary for us to look at?

[TD-er]

I was thinking about copy file first, then update and see what area of the file has changed.
Or maybe store the settings in separate files. That would make changing the settings a lot more flexible.
However, I do not know what the limitations are of the SPIFF. Maybe there are limits on number of files, or is the overhead per file quite big.

Storing in Json is a bit too much I guess, but import/export via JSON is one thing we really must start looking into and start looking into the requirements for Cereal

[ghost]

This feature introduced a new bug:

String ClearCustomTaskSettings(int TaskIndex)
..
return(ClearInFile((char*)FILE_CONFIG, DAT_OFFSET_TASKS + (TaskIndex * DAT_TASKS_SIZE) + DAT_TASKS_CUSTOM_OFFSET, DAT_TASKS_CUSTOM_SIZE));

It clears an incorrect size of 2048 bytes, overwriting the next slot data (that would be the next task extrasettings, or the controller settings in case of the last task.

It's fixed now

[TD-er]

Great find. I thought it was somewhat static code, which was not touched for ages, so never thought of comparing that.

[ghtester]

Great work, thanks a lot! :-)