Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[memory usage] Device combobox takes enormous amount of memory #654

Closed
TD-er opened this issue Dec 23, 2017 · 16 comments
Closed

[memory usage] Device combobox takes enormous amount of memory #654

TD-er opened this issue Dec 23, 2017 · 16 comments
Labels
Category: Core related Related to the (external) core libraries Type: Bug Considered a bug
Milestone

Comments

@TD-er
Copy link
Member

TD-er commented Dec 23, 2017

The device combobox with the listed devices takes about 4kB of memory.
For some device pages with lots of information, it has become too much.
For instance the OLED framed plugin - when active- can hardly load anymore and may lead to several crashes. Especially when the scrolling on the OLED display is enabled and more than 4 lines of text are available. (N.B. mega branch) This page (OLED framed plugin) can become around 9.5 kB of HTML.

So perhaps we can generate the combobox with devices at compile time and store it in flash?
Or generate those things in real time when sending a HTTP chunk?
Or disable the plugin when settings are being altered?

@Grovkillen Grovkillen added Type: Bug Considered a bug Category: Core related Related to the (external) core libraries labels Dec 23, 2017
@Grovkillen Grovkillen added this to the 2.1.0 milestone Dec 23, 2017
@psy0rz
Copy link
Member

psy0rz commented Dec 28, 2017

perhaps, or even get the list with a seperate ajax call?

@TD-er
Copy link
Member Author

TD-er commented Dec 28, 2017

I was thinking about generating part of the combo box only once and store it in flash.
I am still thinking about it and have not yet a clear answer to it.

@psy0rz
Copy link
Member

psy0rz commented Jan 2, 2018

yeah ah you mean, generate it runtime and store it in spiffs?

@TD-er
Copy link
Member Author

TD-er commented Jan 2, 2018 via email

@psy0rz
Copy link
Member

psy0rz commented Jan 2, 2018

yep, but does that help? we still would have to load that file and that still takes 4kb of memory?

4k wouldn't be a problem if we create a extra GET handler for it that only generates and returns this list.

@TD-er
Copy link
Member Author

TD-er commented Jan 2, 2018

There is also the retrieving of the plugins, sorting them, converting to HTML (and adding the selected part), etc.
We could make a single function in the webserver class to stream it live without keeping it as a string in memory.
Just a single function that reads from flash, add wrapping HTML and get the selected item number as input and stream that directly to the web-client.

@psy0rz
Copy link
Member

psy0rz commented Jan 2, 2018

perhaps..still its pretty complex and i wonder how much it eventually saves.

@TD-er
Copy link
Member Author

TD-er commented Jan 2, 2018

I will try and test myself.
You don't see any issues I may not see yet?

@psy0rz
Copy link
Member

psy0rz commented Jan 2, 2018

ah the function would not cache it to flash? then i see no issue.

@TD-er
Copy link
Member Author

TD-er commented Jan 2, 2018

No, flash is for reading, not for writing (often) :)

@psy0rz
Copy link
Member

psy0rz commented Jan 2, 2018

ok, i'm setting up a build "bot" to trigger nightly builds for the mega-branch. should make testing easier for users. (will make a forum post about this as well)

@TD-er
Copy link
Member Author

TD-er commented Jan 3, 2018 via email

@psy0rz
Copy link
Member

psy0rz commented Jan 3, 2018

Ok i will.

@psy0rz
Copy link
Member

psy0rz commented Jan 10, 2018

i just though of a better solution: Just create a separate and simple "Add device" page for empty tasks. T

After that we wont need to show the combobox on the Edit page. We only need to add a delete button to delete a device.

@TD-er
Copy link
Member Author

TD-er commented Jan 10, 2018

Sounds like a plan :)

@psy0rz psy0rz modified the milestones: 2.1.0, 2.0.0 Jan 10, 2018
@psy0rz
Copy link
Member

psy0rz commented Jan 10, 2018

this was pretty easy to fix and we had a lot of memory issues, so moved to v2.0.

psy0rz added a commit that referenced this issue Jan 10, 2018
@psy0rz psy0rz closed this as completed Jan 10, 2018
TD-er pushed a commit to TD-er/ESPEasy that referenced this issue Jan 10, 2018
psy0rz pushed a commit that referenced this issue Jan 10, 2018
* [switch] Fixed switch behavior and default settings. (#675)

As described in #673 .
The problem was partly related to the default values stored in flash ("0"), which was not a valid value for the switch type.

When upgrading from an older version of ESPeasy, make sure to check the switch type (normal switch or dimmer) and save the settings for the switch device again, even when nothing was changed.
Default configuration and new added switches will now work like intended.

When a controller is enabled (e.g. Domoticz MQTT or -HTTP) and the button is pressed multiple times, the ESP may reboot. See issue #674.

* ABC calibration feature added (#606)

* [Flash info] Detailed flash information (#678)

Last few days a number of issues and forum topic was about the type of flash used on the ESP boards.

This is an extension of the detailed information page.

Perhaps also merge with the newer and more clear layout of pull request #624?
That pull request was only merged to the mega branch.
I kept the changes local, but perhaps they should be placed in the "Storage" section introduced with #624.
Maybe also that pull request should get merged into the v2.0 branch.

* Bugfix/v2.0 crash switch (#682)

* [crashes] Added constructors to initialize all members in structs

Numerous structs are defined, but none of them have default constructors and there is no guarantee the members will be set when used. 
With these default constructors, the parameters at least have an initialized value.

* [PubSubClient] Add bound checks on the internal buffer

Not sure if this was really causing an issue, but proper bound checks are always a good thing.

* [Crash Switch] Disabled delayBackground and added yield() calls

Something really fishy is going on with the delayBackground function, which will result in crashes when pressing the switch multiple times, with Domoticz MQTT enabled as first controller.
Disabled for now and delay(1) added to give background tasks a chance to do their work and make sure the watchdog doesn't perform a reset.

* [CI build errors] Commented out some unused variables

Travis considers them as error and fails the checks.

* [CI check] Out-of-bounds check fix

* actually ignore MQTT messages that are too big.

* moved mqtt stuff outside of backgroundtasks(). fixes #683 in my test scenario

* [Adafruit MPR121] Change deprecated name setThreshholds to setThresholds (#685)

See #684

* fixed plugin id of "Communication - Kamstrup Multical 401". (accidental octal notation)

* changed devicecombobox handling to save a lot of memory on device page. fixes #654 #676 and could be triggered by #683 in some cases.

* [CPPcheck] v2.0 ControllerSettingsStruct some variables not initialized (#692)

Fixing these cppcheck errors:
101.43s$ cppcheck --enable=warning src/*.ino -q --force -I src --include=src/ESPEasy.ino --error-exitcode=1
[src/ESPEasy.ino:500]: (warning) Member variable 'ControllerSettingsStruct::HostName' is not initialized in the constructor.
[src/ESPEasy.ino:500]: (warning) Member variable 'ControllerSettingsStruct::Publish' is not initialized in the constructor.
[src/ESPEasy.ino:500]: (warning) Member variable 'ControllerSettingsStruct::Subscribe' is not initialized in the constructor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Core related Related to the (external) core libraries Type: Bug Considered a bug
Projects
None yet
Development

No branches or pull requests

3 participants