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

ERR_EMPTY_RESPONSE on submit of device form #352

Closed
krikk opened this issue Jun 10, 2017 · 27 comments
Closed

ERR_EMPTY_RESPONSE on submit of device form #352

krikk opened this issue Jun 10, 2017 · 27 comments
Labels
Category: Core related Related to the (external) core libraries Type: Bug Considered a bug
Milestone

Comments

@krikk
Copy link
Contributor

krikk commented Jun 10, 2017

Steps to reproduce

flash current git version of dev_4096, (reset settings with reset command)
connect to wlan
go to devices tab, select ina219 plugin, give it a name and click submit

-> i get ERR_EMPTY_RESPONSE in Google Chrome, same in Microsoft Edge

with the developer tools in chrome i see that i get no contend, if click refresh it loads find and the changed settings are remembered

Does the problem presist after powering off and on? (just resetting isnt enough sometimes)
yes

Expected behavior

i should directly get a page, not only after reloading

Actual behavior

ERR_EMPTY_RESPONSE

System configuration

Hardware: Wemos D1 R2

Software or git version: latest git

have this problem also with the oled plugin...

i suspect that this happens since the changes in the gui where done, but i am not sure about that...

@JK-de
Copy link
Contributor

JK-de commented Jun 11, 2017

Please check the following:

  • Do you see a reboot on serial terminal when accessing the web page
  • Is the problem als with compiling TEST_4096 or NORMAL_4096
  • Delete about 10 Plugins (you dont need now), compile and flash. Try again...

The memory usage in device settings page is hard at the limit. I got a reboot if free RAM is below 20kB and page size is over 8kB. The main problem was the select-box with the plugin names

@krikk
Copy link
Contributor Author

krikk commented Jun 11, 2017

always have the serial console open... no not a simple reboot, but free ram is below 20 kb, when testing with the oled plugin sometimes around 17-18 kb, but still no reboot
... will try if it also happens when there is more free ram...

@psy0rz
Copy link
Member

psy0rz commented Jun 12, 2017

krikk can you send mail an email so that i have your mailadres? mine is edwin@datux.nl

@krikk
Copy link
Contributor Author

krikk commented Jun 12, 2017

@jkDesignDE you where right, it is a memory issue, removed a few plugins:
INIT : Free RAM:24848
while running: SYS : 20560
...everything works...

than reflashed the full dev_4096 firmware:
INIT : Free RAM:22960
while running: SYS : 18896.00

...i can only load "small" device pages like systeminfo, but not "big" devices like oled or ina219...

a have adruino-OTA active in my full test-build, which probably makes the issus worse, due to consuming more ram, i think...

outcome: we should not release firmware to endusers which has less than about 22kb free after init... ..we are getting to the limits of this platform... :(

@JK-de
Copy link
Contributor

JK-de commented Jun 12, 2017

The only solutions I see:

Short term: Identify memory hungry plugins without being initialized.

Long term: Split the device-selector from the device-config page.
Like: Task-list with add-button. On click the add-button show a device-selector-page. After selection go to the device-config-page without a selector for changing devices. The device-selector produces itself about 4kb HTML code.

Also helping: Shorten device names a bit like "Environment - XXX" -> "Env. - XXX" or "Ambient - XXX"

Should be discussed with @psy0rz and others

@tedenda
Copy link
Contributor

tedenda commented Jun 12, 2017 via email

@psy0rz
Copy link
Member

psy0rz commented Jun 12, 2017

i'll have a look at it right now

@psy0rz
Copy link
Member

psy0rz commented Jun 12, 2017

i'm able to reproduce the problem, but only if FEATURE_OTA is enabled. so it probably doesnt affect other users (yet). however i'll take a look and see if we can safe some ram in the handle_devices() function.

@JK-de
Copy link
Contributor

JK-de commented Jun 12, 2017

The problem depends how fragmented the RAM is at this time. For the device page you need 8kB and for delivering additional ?kB in ONE block.
Another problem: there is no plugin destructor. Playing around with selecting different plugins at the same task allocates RAM but do not free it.
Other effects: staying on the device page eats 2kB more than switching to main page

Normal users uses normal-build with 10 plugins less.

@psy0rz
Copy link
Member

psy0rz commented Jun 12, 2017

ok there's a lot of temporary variables in handle_devices() that can be eliminated. working on that right now.

if after optimizing this function, memory still is short, i will have a look at the templating functions: this one duplicates the whole html-page in memory.

@psy0rz
Copy link
Member

psy0rz commented Jun 12, 2017

ah right, is that 8kb also used again (temporary) in the processwebpagetemplate function?

do you have an example of where RAM is not freed? we probably should fix that if its easy.

@psy0rz
Copy link
Member

psy0rz commented Jun 12, 2017

if we really want to save ram, we should do all the templating on the client side via javascript. this way we can let the server just return simple static html pages and short json strings for the actual data. some javascript in the browser will handle the rest.

i have a lot of experience with such things, but thats major change we should perhaps plan for 2.1.x .

(but there are other easier ways to save ram as well, we might be able to get rid of the Device[] struct fairly easy)

@JK-de
Copy link
Contributor

JK-de commented Jun 12, 2017

Quick fix: remove lines 1615 to 1623 in webserver.ino -> 400+ bytes

Also use external css-file. Maybe if there is no file on SPIFFS generate one at boot time once - then the <style> in the HTML can be removed

In the templating functions I try to shrink the one string while growing the other...

psy0rz added a commit that referenced this issue Jun 12, 2017
…ommented for reference, in case i created bugs. will remove later
@psy0rz
Copy link
Member

psy0rz commented Jun 12, 2017

that last commit solved the blank-page issue in my particular case. will see if i can find some more easy fixes while not removing features. :)

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

psy0rz commented Jun 12, 2017

@jkDesignDE in processwebpagetemplate() we're doing a pageResult += pageContent;. at that point we're probably at "peak mem usage" :)

directly after that we clean it, but if then its too late so to speak.

@psy0rz
Copy link
Member

psy0rz commented Jun 12, 2017

perhaps we could save some space by making getWebPageTemplateDefault return a PSTR and work with that. (http://www.nongnu.org/avr-libc/user-manual/group__avr__pgmspace.html#ga9530b4f628b49635c4085c269da8867a)

but that breaks file-templating, but perhaps we should drop that? (or make that byte-for-byte processing instead of reading it to memory)

well i'm going to sleep now before i get even more far fetched ideas ;)

@JK-de
Copy link
Contributor

JK-de commented Jun 12, 2017

do you have an example of where RAM is not freed? we probably should fix that if its easy.

Every plugin with a new in the PLUGIN_INIT like:
Plugin_052_SoftSerial = new SoftwareSerial(...);
We should add a PLUGIN_EXIT which is called if the user changes the device

@psy0rz
Copy link
Member

psy0rz commented Jun 12, 2017

ah right, that always bothered me indeed. a PLUGIN_EXIT would solve that.

it would also make reproducing other issues that people might have more consistent. we definitely need that.

@JK-de
Copy link
Contributor

JK-de commented Jun 12, 2017

I take a look at pageResult += pageContent

The template itself are very small (compared to the content). I think there is nothing/less to optimize

@psy0rz
Copy link
Member

psy0rz commented Jun 12, 2017

maybe if we add to the result in blocks of 16 bytes, and shrinking pagecontent by 16 bytes everytime. but the shrinking would have to be done in a way that doesnt create a temporary string somewhere (under water perhaps).

@psy0rz
Copy link
Member

psy0rz commented Jun 12, 2017

added a new issue for the PLUGIN_EXIT stuff: #354

@JK-de
Copy link
Contributor

JK-de commented Jun 12, 2017

I had the same idea but substring makes a copy :(

What do you think about the css-file instead of <style> - it saves about 1kB !!!

@psy0rz
Copy link
Member

psy0rz commented Jun 12, 2017

you mean auto-write it to SPIFFS somewhere ( if it doesnt exist already) and then replace {{css}} with a static <link> to it?

thats a real good idea actually. should be in a way that works right after factory resetting, but also for existing configs.

@JK-de
Copy link
Contributor

JK-de commented Jun 12, 2017

The code in the {{css}} exist to link to an external css-file (line248).

We/you/I have to remove the style from {{css}} and at boot time create (if not exist) the css out of a F("XXX")

@JK-de
Copy link
Contributor

JK-de commented Jun 13, 2017

for better understanding I colored the parts: red=style/css green=menu yellow=device-selector white=device-itself
devicecolor

@psy0rz
Copy link
Member

psy0rz commented Jun 13, 2017

thanks, we can do that. :)

@psy0rz
Copy link
Member

psy0rz commented Jun 15, 2017

actually you did it already ;) i think this issue is fixed for now, until we need more ram in the future, but then we think of some bigger better changes to fix it. (after the 2.0.0 release)

@psy0rz psy0rz closed this as completed Jun 15, 2017
ghost pushed a commit that referenced this issue Jun 19, 2017
Send web page as junked data - issue #352
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

4 participants