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

Automatic stack location selection (SYS or HEAP), enable per library AR-chive in arduino build system #5018

Merged
merged 20 commits into from
Aug 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions cores/esp8266/cont.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@
#define CONT_STACKSIZE 4096
#endif

#ifdef __cplusplus
extern "C" {
#endif

typedef struct cont_ {
void (*pc_ret)(void);
unsigned* sp_ret;
Expand All @@ -45,6 +49,8 @@ typedef struct cont_ {
unsigned* struct_start;
} cont_t;

extern cont_t* g_pcont;

// Initialize the cont_t structure before calling cont_run
void cont_init(cont_t*);

Expand All @@ -68,4 +74,8 @@ int cont_get_free_stack(cont_t* cont);
// continuation stack
bool cont_can_yield(cont_t* cont);

#ifdef __cplusplus
}
#endif

#endif /* CONT_H_ */
36 changes: 36 additions & 0 deletions cores/esp8266/core_esp8266_app_entry_noextra4k.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* This is the original app_entry() not providing extra 4K heap, but allowing
* the use of WPS.
*
* see comments in core_esp8266_main.cpp's app_entry()
*
*/

#include <c_types.h>
#include "cont.h"
#include "coredecls.h"

void disable_extra4k_at_link_time (void)
{
/*
* does nothing
* allows overriding the core_esp8266_main.cpp's app_entry()
* by this one below, at link time
*
*/
}

/* the following code is linked only if a call to the above function is made somewhere */

extern "C" void call_user_start();

/* this is the default NONOS-SDK user's heap location */
static cont_t g_cont __attribute__ ((aligned (16)));

extern "C" void ICACHE_RAM_ATTR app_entry_redefinable(void)
{
g_pcont = &g_cont;

/* Call the entry point of the SDK code. */
call_user_start();
}
39 changes: 19 additions & 20 deletions cores/esp8266/core_esp8266_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ extern void (*__init_array_end)(void);
/* Not static, used in Esp.cpp */
struct rst_info resetInfo;

/* Not static, used in core_esp8266_postmortem.c.
/* Not static, used in core_esp8266_postmortem.c and other places.
* Placed into noinit section because we assign value to this variable
* before .bss is zero-filled, and need to preserve the value.
*/
Expand Down Expand Up @@ -175,10 +175,15 @@ void init_done() {

WPS beeing flawed by its poor security, or not beeing used by lots of
users, it has been decided that we are still going to use that memory for
user's stack and disable the use of WPS, with an option to revert that
back at the user's discretion. This selection can be done with the
global define NO_EXTRA_4K_HEAP. An option has been added to the board
generator script.
user's stack and disable the use of WPS.

app_entry() jumps to app_entry_custom() defined as "weakref" calling
itself a weak customizable function, allowing to use another one when
this is required (see core_esp8266_app_entry_noextra4k.cpp, used by WPS).

(note: setting app_entry() itself as "weak" is not sufficient and always
ends up with the other "noextra4k" one linked, maybe because it has a
default ENTRY(app_entry) value in linker scripts).

References:
https://github.com/esp8266/Arduino/pull/4553
Expand All @@ -188,31 +193,25 @@ void init_done() {

*/

#ifdef NO_EXTRA_4K_HEAP
/* this is the default NONOS-SDK user's heap location */
cont_t g_cont __attribute__ ((aligned (16)));
#endif

extern "C" void ICACHE_RAM_ATTR app_entry(void)
extern "C" void ICACHE_RAM_ATTR app_entry_redefinable(void) __attribute__((weak));
extern "C" void ICACHE_RAM_ATTR app_entry_redefinable(void)
{
#ifdef NO_EXTRA_4K_HEAP

/* this is the default NONOS-SDK user's heap location */
g_pcont = &g_cont;

#else

/* Allocate continuation context on this SYS stack,
and save pointer to it. */
cont_t s_cont __attribute__((aligned(16)));
g_pcont = &s_cont;

#endif

/* Call the entry point of the SDK code. */
call_user_start();
}

static void ICACHE_RAM_ATTR app_entry_custom (void) __attribute__((weakref("app_entry_redefinable")));

extern "C" void ICACHE_RAM_ATTR app_entry (void)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we drop this extra call level?

In this file, you define:

extern "C" void ICACHE_RAM_ATTR app_entry_sysstack(void)
{
    cont_t s_cont __attribute__((aligned(16)));
    g_pcont = &s_cont;
    call_user_start();
}
extern "C" void app_entry(void) __attribute__((weak, alias("app_entry_sysstack")));

In the other file, you define app_entry as a strong symbol:

extern "C" void ICACHE_RAM_ATTR app_entry(void)
{
    g_pcont = &g_cont;
    call_user_start();
}

I think it should work...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that, and just re-tried your way (weak+alias instead of weakref) and it still does not work, the strong one is always used. I don't know why for sure.

I had put that comment just above about that.

(note: setting app_entry() itself as "weak" is not sufficient and always ends up with the other "noextra4k" one linked, maybe because it has a default value in linker scripts).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right. That's a side effect of specifying something as ENTRY, it also makes the symbol undefined, and the linker picks it up from core_esp8266_app_entry_noextra4k.cpp before it sees core_esp8266_main.cpp (probably due to alphabetical ordering when creating the archive file)...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried again (after dot_a_linkage, just to be sure), still the same. The indirection is still needed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem, that only cost us 16 bytes of the stack, which we can later remove by re-writing app_entry as a one-line assembly function which call0-s app_entry_redefinable.

{
return app_entry_custom();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we call app_entry_redefinable() directly here? or is that what is meant about the extra level of indirection in the comments above?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment above: tried that 3 times with different context with no luck. It is because (maybe) app_entry is defined as an ENTRY in ldscripts.

tools/sdk/ld/eagle.app.v6.common.ld.h:ENTRY(app_entry)

}

extern "C" void user_init(void) {
struct rst_info *rtc_info_ptr = system_get_rst_info();
memcpy((void *) &resetInfo, (void *) rtc_info_ptr, sizeof(resetInfo));
Expand Down
2 changes: 0 additions & 2 deletions cores/esp8266/core_esp8266_postmortem.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@

extern void __real_system_restart_local();

extern cont_t* g_pcont;

// These will be pointers to PROGMEM const strings
static const char* s_panic_file = 0;
static int s_panic_line = 0;
Expand Down
5 changes: 5 additions & 0 deletions cores/esp8266/coredecls.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,15 @@ extern "C" {

// TODO: put declarations here, get rid of -Wno-implicit-function-declaration

#include <cont.h> // g_pcont declaration

extern bool timeshift64_is_set;

void esp_yield();
void esp_schedule();
void tune_timeshift64 (uint64_t now_us);
void settimeofday_cb (void (*cb)(void));
void disable_extra4k_at_link_time (void) __attribute__((noinline));

#ifdef __cplusplus
}
Expand Down
2 changes: 0 additions & 2 deletions doc/faq/a05-board-generator.rst
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ As of today you can:

* increase available flash space by disabling floats in ``*printf`` functions

* enable WPS which is now disabled by default (at the cost of a smaller heap by ~4KB)

* change led pin ``LED_BUILTIN`` for the two generic boards

* change the default lwIP version (1.4 or 2)
Expand Down
19 changes: 13 additions & 6 deletions doc/faq/readme.rst
Original file line number Diff line number Diff line change
Expand Up @@ -46,22 +46,29 @@ How can I get some extra KBs in flash ?
* Using ``*printf()`` with floats is enabled by default. Some KBs of flash can
be saved by using the option ``--nofloat`` with the boards generator:

``./tools/boards.txt.py --nofloat --allgen``
``./tools/boards.txt.py --nofloat --boardsgen``

* Use the debug level option ``NoAssert-NDEBUG`` (in the Tools menu)

`Read more <a05-board-generator.rst>`__.

Why can't I use WPS ?
~~~~~~~~~~~~~~~~~~~~~
About WPS
~~~~~~~~~

WPS is disabled by default, this offers an extra 4KB in ram/heap. To enable
WPS (and lose 4KB of useable ram), use this boards generator option:
From release 2.4.2 and ahead, not using WPS will give an exra ~4.5KB in
heap.

``./tools/boards.txt.py --allowWPS --allgen``
In release 2.4.2 only, WPS is disabled by default and the board generator is
required to enable it:

``./tools/boards.txt.py --allowWPS --boardsgen``

`Read more <a05-board-generator.rst>`__.

This manual selection is not needed starting from 2.5.0 (and in git
version). WPS is always available, and not using it will give an extra
~4.5KB compared to releases until 2.4.1 included.

This Arduino library doesn't work on ESP. How do I make it work?
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
1 change: 1 addition & 0 deletions libraries/ArduinoOTA/library.properties
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ paragraph=With this library you can enable your sketch to be upgraded over netwo
category=Communication
url=
architectures=esp8266
dot_a_linkage=true
1 change: 1 addition & 0 deletions libraries/DNSServer/library.properties
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ paragraph=This library implements a simple DNS server.
category=Communication
url=
architectures=esp8266
dot_a_linkage=true
1 change: 1 addition & 0 deletions libraries/EEPROM/library.properties
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ paragraph=
category=Data Storage
url=http://arduino.cc/en/Reference/EEPROM
architectures=esp8266
dot_a_linkage=true
1 change: 1 addition & 0 deletions libraries/ESP8266AVRISP/library.properties
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ paragraph=This library allows programming 8-bit AVR ICSP targets via TCP over Wi
category=Communication
url=
architectures=esp8266
dot_a_linkage=true
1 change: 1 addition & 0 deletions libraries/ESP8266HTTPClient/library.properties
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ paragraph=
category=Communication
url=https://github.com/esp8266/Arduino/blob/master/libraries/ESP8266HTTPClient
architectures=esp8266
dot_a_linkage=true
1 change: 1 addition & 0 deletions libraries/ESP8266HTTPUpdateServer/library.properties
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ paragraph=The library accepts HTTP post requests to the /update url, and updates
category=Communication
url=
architectures=esp8266
dot_a_linkage=true
1 change: 1 addition & 0 deletions libraries/ESP8266NetBIOS/library.properties
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ paragraph=With this library you can connect to your ESP from Windows using a sho
category=Communication
url=http://www.xpablo.cz/?p=751#more-751
architectures=esp8266
dot_a_linkage=true
1 change: 1 addition & 0 deletions libraries/ESP8266WebServer/library.properties
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ paragraph=The library supports HTTP GET and POST requests, provides argument par
category=Communication
url=
architectures=esp8266
dot_a_linkage=true
1 change: 1 addition & 0 deletions libraries/ESP8266WiFi/library.properties
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ paragraph=With this library you can instantiate Servers, Clients and send/receiv
category=Communication
url=
architectures=esp8266
dot_a_linkage=true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why these are all being converted to .a. Does this mean anyone providing a library for the 8266 now needs to modify their lib.properties?

Copy link
Collaborator Author

@d-a-v d-a-v Aug 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not mandatory (except for the ESP8266WiFi library). It is not needed by anyone. Such library's object files are stored in an intermediate libraryname.a (the regular way after all) and this one is used instead of all library's object files in the linker command line.
Other libraries not using "dot_a_linkage" are still usable.
I checked with ESP8266Audio (local branch with this PR merged with latest master) and it's OK. I also tried with "dot_a_linkage" in your lib and the final sketch was reduced by 25KB (!).
Not to mention the linker command looking like a real one (with some .a instead of a huge bunch of .o - well, this is minor cosmetic effect, but still :) ).
What I do not understand is why this was not done by default from the beginning in Arduino.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's awesome news! Glad to hear about it, I'll throw it in my own lib as well now...

111 changes: 111 additions & 0 deletions libraries/ESP8266WiFi/src/ESP8266WiFiSTA-WPS.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/*
ESP8266WiFiSTA-WPS.cpp - WiFi library for esp8266

Copyright (c) 2014 Ivan Grokhotkov. All rights reserved.
This file is part of the esp8266 core for Arduino environment.

This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public
License as published by the Free Software Foundation; either
version 2.1 of the License, or (at your option) any later version.

This library is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
Lesser General Public License for more details.

You should have received a copy of the GNU Lesser General Public
License along with this library; if not, write to the Free Software
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA

Reworked on 28 Dec 2015 by Markus Sattler

*/


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGPL license missing

#include "ESP8266WiFi.h"
#include "ESP8266WiFiGeneric.h"
#include "ESP8266WiFiSTA.h"
#include "coredecls.h" // disable_extra4k_at_link_time()

static void wifi_wps_status_cb(wps_cb_status status);

/**
* WPS config
* so far only WPS_TYPE_PBC is supported (SDK 1.2.0)
* @return ok
*/
bool ESP8266WiFiSTAClass::beginWPSConfig(void) {

// SYS ram is used by WPS, let's configure user stack inside user's HEAP
disable_extra4k_at_link_time();

if(!WiFi.enableSTA(true)) {
// enable STA failed
return false;
}

disconnect();

DEBUGV("wps begin\n");

if(!wifi_wps_disable()) {
DEBUGV("wps disable failed\n");
return false;
}

// so far only WPS_TYPE_PBC is supported (SDK 1.2.0)
if(!wifi_wps_enable(WPS_TYPE_PBC)) {
DEBUGV("wps enable failed\n");
return false;
}

if(!wifi_set_wps_cb((wps_st_cb_t) &wifi_wps_status_cb)) {
DEBUGV("wps cb failed\n");
return false;
}

if(!wifi_wps_start()) {
DEBUGV("wps start failed\n");
return false;
}

esp_yield();
// will return here when wifi_wps_status_cb fires

return true;
}

/**
* WPS callback
* @param status wps_cb_status
*/
void wifi_wps_status_cb(wps_cb_status status) {
DEBUGV("wps cb status: %d\r\n", status);
switch(status) {
case WPS_CB_ST_SUCCESS:
if(!wifi_wps_disable()) {
DEBUGV("wps disable failed\n");
}
wifi_station_connect();
break;
case WPS_CB_ST_FAILED:
DEBUGV("wps FAILED\n");
break;
case WPS_CB_ST_TIMEOUT:
DEBUGV("wps TIMEOUT\n");
break;
case WPS_CB_ST_WEP:
DEBUGV("wps WEP\n");
break;
case WPS_CB_ST_UNK:
DEBUGV("wps UNKNOWN\n");
if(!wifi_wps_disable()) {
DEBUGV("wps disable failed\n");
}
break;
}
// TODO user function to get status

esp_schedule(); // resume the beginWPSConfig function
}
Loading