Problem: HTTP module is now too large for 512k Flash devices

mickley commented 7 years ago

Expected behavior

Building a firmware with HTTP and only the bare minimum modules (file gpio net node tmr uart wifi) will run on a 512k flash ESP.

Actual behavior

The firmware clocks in at 582,544 bytes, and unsurprisingly doesn't fit/run. It also won't run on 16MB modules (Wemos D1 Mini Pro). The same commit built without the http module (7 modules) runs on both.

512k Modules Errors

Fatal exception (28): 
epc1=0x4000228b, epc2=0x00000000, epc3=0x00000000, excvaddr=0x000000b5, depc=0x00000000

WeMos D1 Mini Pro Errors

ets Jan  8 2013,rst cause:2, boot mode:(3,6)

load 0x40100000, len 27148, room 16 
tail 12
chksum 0xe6
ho 0 tail 12 room 4
load 0x3ffe8000, len 2424, room 12 
tail 12
chksum 0x06
ho 0 tail 12 room 4
load 0x3ffe8978, len 136, room 12 
tail 12
chksum 0x4c
csum 0x4c
system param error, use last saved param!
rf_cal[0] !=0x05,is 0xCE

NodeMCU version

Dev branch: c5c0143b2fa6402915beca2a50da7241247e1409

Hardware

ESP-03, ESP-12 (512k), WeMos D1 Mini Pro

Notes:

The addition of the http module adds a whopping 171176 bytes or nearly 30% of the entire firmware! That is kind of crazy!
Presumably the changes from sdk 2.0.0 caused the size of the http module to balloon. A 10-module build (including http and a few others) from the last master was only 457kb.
Besides the extreme case of including ONLY the http module, it's kind of important that at least 10-12 modules (eg I2C and one or two sensor modules) fit on 512k. Otherwise, these devices are more or less useless with the current firmware. I realize they're a bit outdated and could have their chips upgraded, but I still have quite a few and if they're shielded, the upgrade is a fair bit of work. For simple tasks, they've previously been fine.
Perhaps more importantly, this also effects the Wemos D1 Mini Pro (and probably other 16MB modules). These appear to have the same limitations on firmware size that a 512k module would have, at least for now. I haven't gotten a firmware build larger than 512k to work on them (but see [#1634] & [#1646]). So the end result is that http will not work on these at all.

djphoenix commented 7 years ago

Hmm... Interesting that in http module source no dependencies on CLIENT_SSL_ENABLE, so it have links to espconn_secure and so to mbedTLS. Maybe this chain make firmware growth.

Also maybe irom text location is not optimal, because text and data segments are not really big, but irom always located at 0x10000. The possible fix is to use bootloader (e.g. rBoot) that supports "boot format 2" that more compact and optimal for me.

I'll take some experiments on this cases.

djphoenix commented 7 years ago

So sizes is (for dev/integer/no-ssl clean build):

0x00000.bin    0x10000.bin   TLS links   modules
28544          330944        x           file gpio net node tmr uart wifi
29248          502480        √           file gpio http net node tmr uart wifi

-- After patch httpclient.c
28544          336112        x           file gpio http net node tmr uart wifi

I used this patch:

diff --git a/app/http/httpclient.c b/app/http/httpclient.c
index 3062a93..6b21134 100644
--- a/app/http/httpclient.c
+++ b/app/http/httpclient.c
@@ -140,9 +140,11 @@ static void ICACHE_FLASH_ATTR http_receive_callback( void * arg, char * buf, uns
    {
        HTTPCLIENT_ERR( "Response too long (%d)", new_size );
        req->buffer[0] = '\0';                                                                  /* Discard the buffer to avoid using an incomplete response. */
+#ifdef CLIENT_SSL_ENABLE
        if ( req->secure )
            espconn_secure_disconnect( conn );
        else
+#endif
            espconn_disconnect( conn );
        return;                                                                                 /* The disconnect callback will be called. */
    }
@@ -170,9 +172,11 @@ static void ICACHE_FLASH_ATTR http_send_callback( void * arg )
    {
        /* The headers were sent, now send the contents. */
        HTTPCLIENT_DEBUG( "Sending request body" );
+#ifdef CLIENT_SSL_ENABLE
        if ( req->secure )
            espconn_secure_send( conn, (uint8_t *) req->post_data, strlen( req->post_data ) );
        else
+#endif
            espconn_send( conn, (uint8_t *) req->post_data, strlen( req->post_data ) );
        os_free( req->post_data );
        req->post_data = NULL;
@@ -213,7 +217,11 @@ static void ICACHE_FLASH_ATTR http_connect_callback( void * arg )
     int host_len = 0;
     if ( os_strstr( req->headers, "Host:" ) == NULL && os_strstr( req->headers, "host:" ) == NULL)
     {
-        if ((req->port == 80) || ((req->port == 443) && ( req->secure )))
+        if ((req->port == 80)
+#ifdef CLIENT_SSL_ENABLE
+               || ((req->port == 443) && ( req->secure ))
+#endif
+           )
         {
             os_sprintf( host_header, "Host: %s\r\n", req->hostname );
         }
@@ -236,11 +244,13 @@ static void ICACHE_FLASH_ATTR http_connect_callback( void * arg )
             "\r\n",
             req->method, req->path, host_header, req->headers, ua_header, post_headers );

+#ifdef CLIENT_SSL_ENABLE
     if (req->secure)
     {
         espconn_secure_send( conn, (uint8_t *) buf, len );
     }
     else
+#endif
     {
         espconn_send( conn, (uint8_t *) buf, len );
     }
@@ -437,9 +447,11 @@ static void ICACHE_FLASH_ATTR http_timeout_callback( void *arg )
    }
    request_args_t * req = (request_args_t *) conn->reverse;
    /* Call disconnect */
+#ifdef CLIENT_SSL_ENABLE
    if ( req->secure )
        espconn_secure_disconnect( conn );
    else
+#endif
        espconn_disconnect( conn );
 }

@@ -487,11 +499,13 @@ static void ICACHE_FLASH_ATTR http_dns_callback( const char * hostname, ip_addr_
        os_timer_setfn( &(req->timeout_timer), (os_timer_func_t *) http_timeout_callback, conn );
        os_timer_arm( &(req->timeout_timer), req->timeout, false );

+#ifdef CLIENT_SSL_ENABLE
        if ( req->secure )
        {
            espconn_secure_connect( conn );
        } 
        else 
+#endif
        {
            espconn_connect( conn );
        }

djphoenix commented 7 years ago

/cc @marcelstoer FYI (you was asked this in this comment)

marcelstoer commented 7 years ago

Thanks for reporting this. The fact that your firmware doesn't run on the WeMos D1 Mini Pro is unrelated to the HTTP module.

rf_cal[0] !=0x05,is 0xCE

indicates corrupt init data: http://nodemcu.readthedocs.io/en/latest/en/flash/#sdk-init-data

mickley commented 7 years ago

@marcelstoer Yes, I'm aware of the init data being the cause for the D1 Mini Pro, but this seems to also be related to the size of the firmware.

If I flash the base 7 modules without http and the init data at 0x3fc000, the firmware runs. If I add the http module, the firmware runs into init data problems. Likewise, if I try any other combination of modules that builds to > 512k there are problems as well. I can do this without changing the rest of the firmware flashing setup or sequence at all.

I've tried various places to flash init data, including the address that would be predicted by the series for 16MB (0xFFC000). Only 0x3FC000 has worked.

My impression with the D1 Mini Pro has been that there are quite a number of bugs. In addition to firmware size/flashing problems, they are less stable (iirc wifi.getphymode() crashes them, some random panics too), and of course they suffer from the aforementioned bug of not saving wifi parameters. Unfortunately, I don't really have time to go bug hunting with them, and they are not close to being reliable enough to use, and mine sit collecting dust mostly.

I'd be cautious about writing these sorts of things off as unrelated. From what I understand reading on here, many of you do not have a 16MB module to work with. They are not well-tested, and more or less absent from the documentation (the init data link in particular).

marcelstoer commented 7 years ago

I'd be cautious about writing these sorts of things off as unrelated. From what I understand reading on here, many of you do not have a 16MB module to work with. They are not well-tested, and more or less absent from the documentation (the init data link in particular).

That observation is correct. However, the firmware reset due to rf_cal[0] !=0x05 is solely due to faulty init data. Can we ask you to help testing #1646? Do you know how to build a binary from devsaurus:beyond_4mb_flash?

mickley commented 7 years ago

Your observation that this reset is due solely to faulty init data does not explain the fact that a smaller firmware runs fine when flashed the same way. So I think there is more to it than that. The init data is certainly the proximate cause, but not necessarily the ultimate one.

I am not set up to build from source and that hasn't been high on my priority list due to the nice nodemcu-build.com. Otherwise, I'm willing to do some small tests.

mickley commented 7 years ago

@djphoenix

It looks like your patch has worked and cut 166k off the firmware size when built with the HTTP module and without SSL. Is that correct?

Does enabling the SSL support add this 166k back? If so, would that effectively make SSL incompatible with 512kb modules?

djphoenix commented 7 years ago

@mickley Yes and no.

mbedTLS adding huge block of code, and 166k is big chunk for 512k modules. So "full-featured" SSL lib may not work for small chips. But mbedtls used in nodeMCU is open-sourced and you may configure it in app/include/user_mbedtls.h. Maybe disable of some parts should help you.

Also I work now on rewrite espconn-based connections on top of plain LWIP and mbedTLS, maybe in future we can strip all of espconn layer and so keep some flash.

mickley commented 7 years ago

Well, at least the ability to use the HTTP module without SSL on 512k modules would be nice.

djphoenix commented 7 years ago

@marcelstoer will you make a PR for my patch (or parts of it), or delegate it to me? :)

marcelstoer commented 7 years ago

The current firmware team would be nothing w/o the support of passionate individuals like you to drive the project forward. We're always welcoming PRs 😄

devsaurus commented 7 years ago

@mickley please check current dev for compatibility with your setup (http with basic modules, SSL disabled). Wemos D1 mini pro should be no issue anymore since it is handled as a 1 MByte module per default now.

mickley commented 7 years ago

Thanks for your work @devsaurus. The current dev with the minimal 8-module setup (SSL disabled) referenced above now clocks in at 416,688 bytes, a savings of 165k. It fits on a 512k module with 62.5k left for the filesystem. There's now plenty of room to pack 15-20 modules on, which I think is enough to use 512k modules for simple tasks at least.

Everything seems to work, though I'll test more extensively when I have time.

nodemcu / nodemcu-firmware