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

Improve ESP32 implementation #1777

Open
MaBecker opened this issue Mar 15, 2020 · 53 comments
Open

Improve ESP32 implementation #1777

MaBecker opened this issue Mar 15, 2020 · 53 comments
Labels

Comments

@MaBecker
Copy link
Contributor

@MaBecker MaBecker commented Mar 15, 2020

ESP-IDF is 3.1.3

Collection of observations

functional:

  • check wifi.save()
  • neopixel
  • add SDCard support by removing FlashFS pr 1780 if you like SDCard support build your own firmware.
  • source code contains jsWarn() , 65 matches across 8 files pr 1880
  • hardware SPI is slower than software
  • https see issue and forum, check this possible Solution
  • add touch sensor
  • update partitions_espruino.csv pr #1905
  • add jsuGetFreeStack() pr 1882
  • add exception if a i2c device is not available pr 1880
  • add FLASH_BAUD to make process, because different boards need different rates pr 1880
  • add jshSPISend16() removed as there is now jshSPISendMany()
  • add BLE-UART functionality
  • Wifi.getHostByName() is broken details
  • add jshSPISendMany() pr 1886 & pr 1888
  • improve shiftOut, check issue 1915 for details
  • I2C access on wrong address, check issue 1480 for details

documentation:

  • add list of pins that can be use with setWatch() pr 543

  • update SPI section

    • name hardware pins
    • update number of SPIs
  • add I2C section

    • name hardware pins
    • add i2c sample to left structure
  • remove SDCard function from ESP32 page

  • add neopixel

  • update flash-map-and-access

  • add sample and link to https

Last Update: 08/29/2020

@MaBecker MaBecker added the ESP32 label Mar 15, 2020
@MaBecker
Copy link
Contributor Author

@MaBecker MaBecker commented Mar 16, 2020

#1751 (comment)

wifi.save() does not work:

=function () { [native code] }
>wifi.connect('......', {password: '......'}, function() {
:    console.log('Connected to Wifi.  IP address is:', wifi.getIP().ip);
:    wifi.save(); // Next reboot will auto-connect
:});
ERROR: Wifi: event_handler STA_START: esp_wifi_connect: 12298(SSID is invalid)
=undefined
WARNING: Wifi:startMDNS - espressif
Connected to Wifi.  IP address is: 192.168.1.219
Uncaught Error: File already written with different data
 at line 2 col 15
    wifi.save();
              ^
in function called from system
>wifi.save();
Uncaught Error: File already written with different data
 at line 1 col 11
wifi.save();
          ^
>

Solution: Update to 2v04.401 or newer, go for lastest travis build and require('Storage').eraseAll()

@HyGy
Copy link

@HyGy HyGy commented Mar 16, 2020

Cannot use sd card. (I'm not sure this is not my fault.) I get:

>SPI1.setup({mosi: SD_CMD, miso:SD_DATA0, sck:SD_CLK});
=undefined
>E.connectSDCard(SPI1, SD_DATA3 );
Uncaught Error: Unimplemented on Linux
 at line 1 col 32
E.connectSDCard(SPI1, SD_DATA3 );
                               ^

Here is the pinout:
i'm using esp32 audio kit, from here: http://wiki.ai-thinker.com/_media/esp32-audio-kit_v2.2_sch.pdf

(sd detect is working)

SD_DETECT=D34;

SD_CLK=D14;
SD_DATA2=D12;
SD_DATA3=D13;
SD_CMD=D15;
SD_DATA0=D2;
SD_DATA1=D4;

Solution: check #1778 for details

@HyGy
Copy link

@HyGy HyGy commented Mar 16, 2020

save(); does not work

If call save(); ESP32 resets and then I cannot access the uploaded functions...

 2v04.401 (c) 2019 G.Williams
Espruino is Open Source. Our work is supported
only by sales of official boards and donations:
http://espruino.com/Donate
>Initializing
initializing sd watcher
>typeof watchSDCard
="function"
>save()
=undefined

After that esp32 resets:

Guru Meditation Error: Core  0 panic'ed (LoadProhibited). Excepti.
Core 0 register dump:
PC      : 0x400dd282  PS      : 0x00060130  A0      : 0x800e306f  
A2      : 0x00000065  A3      : 0x3f4064ac  A4      : 0x00000001  
A6      : 0xb33fffff  A7      : 0x00000000  A8      : 0x00000034  
A10     : 0x00000000  A11     : 0x00060023  A12     : 0x00060021  
A14     : 0xb33fffff  A15     : 0x0000000c  SAR     : 0x00000008  
EXCVADDR: 0x00000008  LBEG    : 0x4009b234  LEND    : 0x4009b262  

Backtrace: 0x400dd282:0x3ffde410 0x400e306c:0x3ffde440 0x400e32f60

Rebooting...
ets Jun  8 2016 00:22:57

rst:0xc (SW_CPU_RESET),boot:0x1f (SPI_FAST_FLASH_BOOT)
configsip: 0, SPIWP:0xee
clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_dr0
mode:DIO, clock div:2
load:0x3fff0018,len:4
load:0x3fff001c,len:2668
load:0x40078000,len:7304
load:0x40080000,len:5312
entry 0x40080274
E (38) boot: ota data partition invalid, falling back to factory
E (567) phy_init: store_cal_data_to_nvs_handle: store calibration)


 ____                 _ 
|  __|___ ___ ___ _ _|_|___ ___ 
|  __|_ -| . |  _| | | |   | . |
|____|___|  _|_| |___|_|_|_|___|
         |_| espruino.com
 2v04.401 (c) 2019 G.Williams

Espruino is Open Source. Our work is supported
only by sales of official boards and donations:
http://espruino.com/Donate

E (1177) phy_init: store_cal_data_to_nvs_handle: store calibratio)

>typeof watchSDCard
="undefined"

MaBecker: run require('Storage').eraseAll() to fix this

@HyGy
Copy link

@HyGy HyGy commented Mar 16, 2020

After some times switching the switch connected to D39 (headphone sense), ESP32 resets.

Here is the code:

DEBUG=true;
HP_DETECT=D39;

var watchHP = function()
{
  if (DEBUG) { console.log('initializing headphone watcher'); }
  setWatch(function(e) {
    if (HP_DETECT.read()===false) 
    {
      if (DEBUG) console.log('headphone plugged');
      HP_PRESENT=true;
    }
    else 
    {
      if (DEBUG) console.log('headphone unplugged');
      HP_PRESENT=false;
    }
  }, HP_DETECT, { repeat: true, debounce: 500 });
};
var init = function() {
  console.log("Initializing");

  watchHP();
};

E.on('init', function() {  init(); } );

init();

Here is the serial coredump:

-> Telnet
Guru Meditation Error: Core  0 panic'ed (Unhandled debug exception)
Debug exception reason: BREAK instr 
Core 0 register dump:
PC      : 0x400803c0  PS      : 0x00060536  A0      : 0x40095224  A1      : 0x3ffe2010  
A2      : 0x8d004136  A3      : 0x00000000  A4      : 0x00000000  A5      : 0x00000000  
A6      : 0x00000000  A7      : 0x00000000  A8      : 0x80093948  A9      : 0x40095224  
A10     : 0x00000000  A11     : 0xffffffff  A12     : 0x400837b8  A13     : 0x00000000  
A14     : 0x3ffc1150  A15     : 0x00000001  SAR     : 0x00000011  EXCCAUSE: 0x00000001  
EXCVADDR: 0x00000000  LBEG    : 0x4009b234  LEND    : 0x4009b262  LCOUNT  : 0xffffffff  

Backtrace: 0x400803c0:0x3ffe2010 0x40095221:0x4011936a

Rebooting...
ets Jun  8 2016 00:22:57

rst:0xc (SW_CPU_RESET),boot:0x1f (SPI_FAST_FLASH_BOOT)
configsip: 0, SPIWP:0xee
clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00
mode:DIO, clock div:2
load:0x3fff0018,len:4
@MaBecker
Copy link
Contributor Author

@MaBecker MaBecker commented Mar 18, 2020

Started running device testing from Device Test Harness

@MaBecker
Copy link
Contributor Author

@MaBecker MaBecker commented Mar 21, 2020

run test for https

reduced vars down to 1700

>process.memory()
={ free: 1675, usage: 25, total: 1700, history: 0,
  gc: 0, gctime: 1.269, blocksize: 16 }
>ESP32.getState()
={
  sdkVersion: "v3.1.3-dirty",
  freeHeap: 15904, BLE: true, Wifi: true, minHeap: 13972 }

trying some simple code

var http = require("http");
http.get("https://www.google.com", function(res) {
  res.on('data', function(data) {
    console.log("got data");
  });
});
>Uncaught InternalError: Failed! mbedtls_ssl_setup: Not enough memory
 at line 6 col 2
});

@jumjum123

  • What is the reason for using the mbed lib from espressif?
  • Can we switch back to the mbed version distributed with Espruino?
@MaBecker
Copy link
Contributor Author

@MaBecker MaBecker commented Mar 22, 2020

slow spi see forum and issues #695 #1212

@rgomezwap
Copy link

@rgomezwap rgomezwap commented Mar 22, 2020

I summarize the list of hardware functions that I use successfully:

  • I use SPI to communicate with SDCard and TFT ILIST7735 screen at the same time
  • I use PWM to move SG90 servo
  • I use DAC on pin D26 for 8KHz sound from samples in Storage area. It works regular. I am working to make it read from SDCard.
  • I use Neopixel with 16x16 matrix, quite well.
  • I use I2C with IMU9250 works very well
  • SetWatch works well, but it is not well documented that PINs are valid.
  • BLE I don't use it yet, it also takes up a lot of JSVars space
  • WIFI works very well for me, I use GET requests and receive javascript code via POST
  • I work via Telnet, instead of Serial, to assess the stability of the connection.

I am ready for any contribution or details that allow us to advance and improve the ESP32 version.

@rgomezwap
Copy link

@rgomezwap rgomezwap commented Mar 23, 2020

All these functions have been tested with firmware v2.0.130.

It seems that the latest firmware has problems, and perhaps some functions that I say work, have stopped working.

I have to check with the latest firmware.

@rgomezwap
Copy link

@rgomezwap rgomezwap commented Mar 23, 2020

This is my actual enviroment :

>process.env;
={
  VERSION: "2v00.130",
  GIT_COMMIT: "9aba654",
  BOARD: "ESP32",
  FLASH: 0, RAM: 524288,
  SERIAL: "30aea432-34b4",
  CONSOLE: "Telnet",
  MODULES: "Flash,Storage,hea" ... "r,crypto,neopixel",
  EXPTR: 1073484860 }
> 

and API

>print(ESP32.getState());
{
  "sdkVersion": "v3.1",
  "freeHeap": 17024, "BLE": false, "Wifi": true, "minHeap": 13848 }
=undefined
> 

@rgomezwap
Copy link

@rgomezwap rgomezwap commented Mar 23, 2020

I just generated a new Build environment. I have downloaded the latest github and the firmware generated is the following version:

espruino_2v04.406_esp32.elf
esptool.py v2.6
** espruino_2v04.406_esp32.bin uses 1395712 bytes of 1572864 available

I'll do the tests from here ...

@rgomezwap
Copy link

@rgomezwap rgomezwap commented Mar 23, 2020

So my situation for testing is:

my previous environment where everything works well is:
2v00.130 and ESP-IDF 3.1 dated October 2018

and the new environment is
2v04.406 and ESP-IDF 3.1.3 dated March 2020

The objective is :
2v0x and ESP-IDF 4.x

@rgomezwap
Copy link

@rgomezwap rgomezwap commented Mar 23, 2020

I publish here the little code that I have tried to use the Touch functionality ...

It is pending to implement it correctly, since it is necessary to define the state of a PIN as TOUCH.

file jswrap_io.c ->

/*JSON{
  "type"     : "function",
  "name"     : "touchRead",
  "generate" : "jswrap_io_touchRead",
  "params"   : [
    ["pin","JsVar","The pin to use"]
  ],
  "return"   : ["int","The touch Value of the Pin"]
}
Get the touch value of the given pin.

*/
JsVarInt jswrap_io_touchRead(JsVar *pinVar)
{
	
	// Handle the case where it is a single pin.
    Pin pin=jshGetPinFromVar(pinVar);
	
	if (jshIsPinValid(pin))
		{
		if (!jshGetPinStateIsManual(pin))
			{
			// RIC Pendiente !!!!!
			// RIC de momento no tenemos estado para definir los Touch PINs
			}
    		
		return jshPinGetTouchValue(pin);
		}
	else
		{
		// Handle pin being invalid.
		jsExceptionHere(JSET_ERROR,"Invalid pin!");
		}
		
}

file in jshardware.c ->

/**
 * Get the touch value of the corresponding pin.
 * \return The current touch value of the pin.
 */
JsVarInt CALLED_FROM_INTERRUPT jshPinGetTouchValue(Pin pin 			//!< The pin to have its value read.
												  ) 				// can be called at interrupt time 
{
		
	// recojemos el numero de PIN en nomenclatura ESP32
	gpio_num_t gpioNum=pinToESP32Pin(pin);
	
	// ahora convertimos el PIN de ESP32 a su numero en el driver de TouchPin
	if (gpioNum == 4)
		{
		gpioNum=0;
		}
	else if (gpioNum == 0)
		{
		gpioNum=1;
		}
	else if (gpioNum == 2)
		{
		gpioNum=2;
		}
	else if (gpioNum == 15)
		{
		gpioNum=3;
		}
	else if (gpioNum == 13)
		{
		gpioNum=4;
		}
	else if (gpioNum == 12)
		{
		gpioNum=5;
		}
	else if (gpioNum == 14)
		{
		gpioNum=6;
		}
	else if (gpioNum == 27)
		{
		gpioNum=7;
		}
	else if (gpioNum == 33)
		{
		gpioNum=8;
		}
	else if (gpioNum == 32)
		{
		gpioNum=9;
		}
			
	uint16_t touch_value=8888;
	esp_err_t estado;
	estado=touch_pad_read(gpioNum,&touch_value);
	if (estado != ESP_OK)								
		{
		// configuramos el PIN, OJO de solo hacerlo una vez !!!
		#define TOUCH_THRESH_NO_USE   (0)
		touch_pad_config(gpioNum,TOUCH_THRESH_NO_USE);
		
		touch_pad_read(gpioNum,&touch_value);
		}
		
	return (JsVarInt)touch_value;
	
}

In case the code is useful for someone to start the implementation

@rgomezwap
Copy link

@rgomezwap rgomezwap commented Mar 23, 2020

The partition file (BIN) has been modified according to
#1776

but the CSV file detailing the partitions I don't know if it is still correct now.
It should be updated right?

https://github.com/espruino/Espruino/blob/master/targets/esp32/Changes_V3.1/partitions_espruino.csv

@rgomezwap
Copy link

@rgomezwap rgomezwap commented Mar 23, 2020

I have verified that the original js_code partition started at 0x2C0000

now it has been changed to 0x320000 to leave more room for the firmware.

Doing tests to add Tensorflow I had to increase the space more and I have it at 0x380000 which leaves me 1728Kbytes.

Maybe we could leave a big size thinking about the future.

@rgomezwap
Copy link

@rgomezwap rgomezwap commented Mar 23, 2020

Screenshot_286

This is a partition proposal, which I don't know if it supports ESP-IDF 4.x.

An issue that remains in the air is the storage area. Currently if we use SDCard, Storage cannot be used. And since Storage and js_code are similar (do not support rewrite), perhaps they could be merged into one.

@MaBecker
Copy link
Contributor Author

@MaBecker MaBecker commented Mar 23, 2020

Doing tests to add Tensorflow I had to increase the space more and I have it at 0x380000 which leaves me 1728Kbytes.

There is one bitter site on adding function. It reduces the heap and that reduces memory.
These are the two command that will give you information about what is left process.memory() and ESP32.getState()

@rgomezwap
Copy link

@rgomezwap rgomezwap commented Mar 23, 2020

There is one bitter site on adding function. It reduces the heap and that reduces memory.
These are the two command that will give you information about what is left process.memory() and ESP32.getState()

Yes it's correct. I have not calculated how much heap and JSvar spends. But if it could work as a BLE that can be disabled on reset it is a very good solution.
There are applications that can use it and others that cannot, but better if the possibility exists in the firmware. In ESP32 there are currently no firmware space problems, another is runtime space.

@MaBecker MaBecker changed the title Improve EP32 implementation Improve ESP32 implementation Apr 1, 2020
@MaBecker
Copy link
Contributor Author

@MaBecker MaBecker commented Apr 23, 2020

ESP32 and https is all about memory .....

The kicker to get heap memory is to remove this build option

 'DEFINES+=-DJSVAR_MALLOC', # Allocate space for variables at jsvInit time 

and set 'variables' to a lower value like 1800, having compact vars would be helpfull.

test code:

var http = require("http");

http.get("https://httpbin.org/ip", function(res) {
  console.log(res.headers);
  res.on('data', function(data) {
    console.log("got data",data);
  });
});

output:

{
  "Date": "Thu, 23 Apr 2020 05:47:06 GMT",
  "Content-Type": "application/json",
  "Content-Length": "30",
  "Connection": "close",
  "Server": "gunicorn/19.9.0",
  "Access-Control-Allow-Origin": "*",
  "Access-Control-Allow-Credentials": "true"
 }
got data {
  "origin": a.b.c.d"   <- replaced my IP 
}

@jumjum123: What do you think about removing this option?

# before https call
>ESP32.getState()
={
  "sdkVersion": "v3.1.3-dirty",
  "freeHeap": 68796, "BLE": false, "Wifi": true, "minHeap": 62448 }

# after https call
>ESP32.getState()
={
  sdkVersion: "v3.1.3-dirty",
  freeHeap: 62920, BLE: false, Wifi: true, minHeap: 14136 }

@gfwilliams
Copy link
Member

@gfwilliams gfwilliams commented Apr 23, 2020

JSVAR_MALLOC is all about allowing memory to be allocated as needed - so theoretically it'd be possible to set it up to allocate less memory if you wanted HTTPS, even without a reboot. Maybe via whatever flags are currently used to toggle BLE state?

@MaBecker
Copy link
Contributor Author

@MaBecker MaBecker commented Apr 23, 2020

Toggle BLE cause a reboot.

So let me look into JSVAR_MALLOC and try to make it dynamical. Maybe @jumjum123 can support too.

Does JSVAR_MALLOC is using the conventional allocation or does it use something like compact vars?

EDIT: Ok, so it is not ESP32 specific as it lifes in src/jsvar.c - wrong!

It is used in main.c for esp32 with a heap limitation of 40000.
So let’s increase this value to 60000 and than integreate it into the esp32 board file.

Edit: added test results for different values

// branch: master

 heapVars = (esp_get_free_heap_size() - X) / 16; 

 X = 60000 - https works 

    process.memory():
     { "free": 2738, "usage": 62, "total": 2800, "history": 30,
      "gc": 0, "gctime": 2.208, "blocksize": 16 }
    ESP32.getState():
     {
      "sdkVersion": "v3.1.3-dirty",
      "freeHeap": 51000, "BLE": false, "Wifi": true, "minHeap": 48124 }

 X = 55000 - https fails 

    process.memory():
     { "free": 3038, "usage": 62, "total": 3100, "history": 30,
      "gc": 0, "gctime": 2.427, "blocksize": 16 }
    ESP32.getState():
     {
      "sdkVersion": "v3.1.3-dirty",
      "freeHeap": 47684, "BLE": false, "Wifi": true, "minHeap": 43004 }

// branch: experimental_compact_vars

  X = 50000

    process.memory():
     { "free": 3334, "usage": 66, "total": 3400, "history": 38,
      "gc": 0, "gctime": 3.216, "blocksize": 13 }
    ESP32.getState():
     {
      "sdkVersion": "v3.1.3-dirty",
      "freeHeap": 53388, "BLE": false, "Wifi": true, "minHeap": 47044 }

Summary: Need about 47 KB free heap to run https.

@Zibri
Copy link

@Zibri Zibri commented May 29, 2020

also add these to the list: #1832

@GaryOtt
Copy link
Contributor

@GaryOtt GaryOtt commented Jul 5, 2020

Using Storage, it does not take many write/erase cycles to cause an error that results in a reboot.

Test code:

var s    = require("Storage"),

	       //256 Char string:
	temp = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" +
	       "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" +
	       "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" +
	       "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",

	writeFile = (id, len) => {
		var a = 0,
			chunkSize = 0;

		console.log("Erasing " + id);
		s.erase(id);

		console.log("Writing " + id);
		while (a < len) {
			chunkSize = Math.min(256, len - a);

			s.write(id, temp.substr(0, chunkSize), a, (a === 0 ? len : undefined));

			a += chunkSize;
		}
	};


writeFile("web_svg", 1131);
writeFile("web_css", 6405);
writeFile("web_htm", 11249);
writeFile("web_js", 26127);

writeFile("web_svg", 1131);

writeFile("fileserv", 3106);
writeFile("jq_small", 859);
writeFile("parsemql", 1103);
writeFile("varint64", 2230);
writeFile("flex_es6", 5590);

while (true) {
	writeFile(".bootcde", 38693);
}
@MaBecker
Copy link
Contributor Author

@MaBecker MaBecker commented Jul 5, 2020

Using Storage, it does not take many write/erase cycles to cause an error that results in a reboot.

This code causes a stack overflow

***ERROR*** A stack overflow in task espruinoTask has been detected.
abort() was called at PC 0x40096ce4 on core 0

If it runs again it also produces

ERROR: >> jshKickWatchDog Not implemented,using taskwatchdog from RTOS

@GaryOtt
Copy link
Contributor

@GaryOtt GaryOtt commented Jul 8, 2020

I have spent a lot of time on this stack overflow bug and I think I have exhausted what I can do without help. Here is what I have found so far:

My investigation is focused on jsfCompactInternal() and jsfGetFileHeader() in src/jsflash.c and jsFlashRead() in targets/esp32/jshardware.c.

The stack overflow exclusively occurs when jsfCompactInternal() is called. jsfCompactInternal() calls jsfGetFileHeader() which in turn calls jshFlashRead() and that is when the overflow occurs. There are many other calls to jsfGetFileHeader() and jshFlashRead() that do not result in any problems.

Using uxTaskGetStackHighWaterMark shows that the call stack always has over 21000 words (CORRECTION: bytes) free when jshFlashRead() is called. On occasions when jshFlashRead() is called from any other code, the high water mark is not raised by the call and is largely stable.

My attention is drawn to the input parameters to jshFlashRead(). jshGetFileHeader() passes the arguments addr and header (a pointer to destination) straight through to jshFlashRead() without modification. That makes some sense of why jshFlashRead() only fails when called (indirectly) from jsfCompactInternal() and leads me to try to spot the differences between the calls to jshGetFileHeader(). That's where I am up to because I have failed to spot any difference that I could prove significant.

Anything from a nudge in the right direction from a more experienced Espruino maintainer to a solution would be greatly appreciated at this point.

@gfwilliams
Copy link
Member

@gfwilliams gfwilliams commented Jul 9, 2020

Thanks for looking into this.

Have you been able to add a jsiConsolePrintf to jshFlashRead to dump the arguments? It may shed some light on what's causing the issue?

It's very odd about there being enough stack free. If not, I'd have put it down to maybe jsuGetFreeStack() at https://github.com/espruino/Espruino/blob/master/src/jsflash.c#L349 not returning the correct value?

In that case I can totally see how it would overwrite the available stack.

@GaryOtt
Copy link
Contributor

@GaryOtt GaryOtt commented Jul 9, 2020

Hi Gordon,

By creating duplicate versions of jsfGetFileHeader() and jshFlashRead(), I have been able to log the parameters only at the point immediately prior to the stack overflow.

I'm including my code so my methods can be checked. I never spent enough time programming in C.

Using this:

void jshFlashRead2(
    void *buf,     //!< buffer to read into
    uint32_t addr, //!< Flash address to read from
    uint32_t len   //!< Length of data to read
  ) {

  jsiConsolePrintf("jshFlashRead2\n");
  jsiConsolePrintf("Source Address: 0x%08x\n", addr);
  jsiConsolePrintf("Length: %d\n", len);
  
  //I got caught out by %p being for pin, not pointer in jsiConsolePrintf()
  char dest[20];
  sprintf(dest, "%p", buf);
  jsiConsolePrintf("Destination: %s", dest);
  
  jsiConsolePrintf("                \n");
  jsiConsolePrintf("                \n");
  jsiConsolePrintf("                \n");
  jsiConsolePrintf("                \n");
       
  if(len == 1){ // Can't read a single byte using the API, so read 4 and select the byte requested
    uint word;
    spi_flash_read(addr & 0xfffffffc,&word,4);
    *(uint8_t *)buf = (word >> ((addr & 3) << 3 )) & 255;
  }
  else spi_flash_read(addr, buf, len);
}

I captured this:

[Flash] jsfWriteFile create file
[Flash] CreateFile (11249 bytes)
[Flash] Compacting
[Flash] Compacting - all data fits in RAM for 0x00320000 (46820 bytes)
[Flash] Compacting from 0x00320000
[Flash] Compacting - copy data to swap
jsfGetFileHeader2
jshFlashRead2
Source Address: 0x00320000
Length: 32
Destination: 0x3ffd47f0                
                
                
                
***ERROR*** A stack overflow in task espruinoTask has been detected.
abort() was called at PC 0x40091810 on core 0

Backtrace: 0x4009169f:0x3ffc8db0 0x400917f7:0x3ffc8dd0 0x40091810:0x3ffc8df0 0x4

Rebooting...

I have tried calling jshFlashRead() by adding this code to top of jsfDebugFiles():

  char          *swapBuffer    = alloca(sizeof(JsfFileHeader)),    //sizeof(JsfFileHeader) = 32
                *swapBufferPtr = swapBuffer;

  JsfFileHeader header;

  //These two methods of obtaining pointers to pass to jshFlashRead() are found in jsflash.c
  jsiConsolePrintf("ONE\n");
  jshFlashRead(swapBufferPtr, (uint32_t)0x320000, sizeof(JsfFileHeader)); 
  
  jsiConsolePrintf("TWO\n");
  jshFlashRead(&header,       (uint32_t)0x320000, sizeof(JsfFileHeader));

This code did not cause a crash. I left an ESP32 calling this code in a loop for an hour last night.

jsuGetFreeStack() does not contain an ESP32 implementation and instead always returns 1000000. Reading 32 bytes of data does not explain the apparent consumption of over 21kb of stack space. I'm trawling the esp-idf API documentation to look for a way to enhance jsuGetFreeStack(). Maybe I'll get results that disagree with uxTaskGetStackHighWaterMark().

@gfwilliams
Copy link
Member

@gfwilliams gfwilliams commented Jul 9, 2020

jsuGetFreeStack() does not contain an ESP32 implementation and instead always returns 1000000

Wow, ok. So yeah, I'd say that would be a really good place to start.

There are so many ways to make Espruino trash the stack without this it's untrue. eg: function a() { a() }; a();

@gfwilliams
Copy link
Member

@gfwilliams gfwilliams commented Jul 9, 2020

Just to add, I guess uxTaskGetStackHighWaterMark probably just shows you how high the stack has been at some point in the past, not what it is right now.

It looks from https://www.freertos.org/FreeRTOS_Support_Forum_Archive/November_2015/freertos_Measuring_Stack_Usage_b5706bb4j.html like if you can get the stack pointer, you could compare it to pxCurrentTCB->pxStack and the allocated stack size to see how much is free

@gfwilliams
Copy link
Member

@gfwilliams gfwilliams commented Jul 9, 2020

or... you can define a variable on the stack at the task's entrypoint:

char *stackLowPtr;
void taskEntrypoint() {
  int stackLow;
  stackLowPtr = &stackLow;
  // calling into Espruino...
}

int jsuGetFreeStack() {
   int stackHi;
   int usage = stackLowPtr - &stackHi;
   return STACK_ALLOCATED_FOR_TASK = usage;
}

@GaryOtt
Copy link
Contributor

@GaryOtt GaryOtt commented Jul 9, 2020

uxTaskGetStackHighWaterMark probably just shows you how high the stack has been at some point in the past

Yes, exactly. That's why we need another way to improve jsuGetFreeStack() but think I know the stack space isn't the cause of the stack overflow in this particular case.

The jsuGetFreeStack() function only includes code for Linux and ARM platforms so it is probably possible to enhance the stability of a lot of boards by enhancing this one function.

I'm was trying to find out what the eTaskGetState API returns but I like the simplicity of your suggestion so I'll see if I can use that instead.

@MaBecker
Copy link
Contributor Author

@MaBecker MaBecker commented Jul 10, 2020

As @GaryOtt describes it, this not only a ESP32 issue. ESP8266 is effected too.

Espruino/src/jsutils.c

Lines 857 to 876 in e78f33f

size_t jsuGetFreeStack() {
#ifdef ARM
void *frame = __builtin_frame_address(0);
size_t stackPos = (size_t)((char*)frame);
size_t stackEnd = (size_t)((char*)&LINKER_END_VAR);
if (stackPos < stackEnd) return 0; // should never happen, but just in case of overflow!
return stackPos - stackEnd;
#elif defined(LINUX)
// On linux, we set STACK_BASE from `main`.
char ptr; // this is on the stack
extern void *STACK_BASE;
uint32_t count = (uint32_t)((size_t)STACK_BASE - (size_t)&ptr);
const uint32_t max_stack = 1000000; // give it 1 megabyte of stack
if (count>max_stack) return 0;
return max_stack - count;
#else
// stack depth seems pretty platform-specific :( Default to a value that disables it
return 1000000; // no stack depth check on this platform
#endif
}

On ESP32 and ESP8266 there is getState() function to get free heap.

jsvObjectSetChildAndUnLock(esp32State, "freeHeap", jsvNewFromInteger(esp_get_free_heap_size()));

jsvObjectSetChildAndUnLock(esp8266State, "freeHeap", jsvNewFromInteger(system_get_free_heap_size()));

So I guess this can be used, add some ifdef board in the else section and then call board specific free_heap_size function.

@gfwilliams
Copy link
Member

@gfwilliams gfwilliams commented Jul 10, 2020

I'm not 100% sure how FreeRTOS works, but the heap may not be in the same area as the stack - so get_free_heap_size may not help you...

@GaryOtt
Copy link
Contributor

@GaryOtt GaryOtt commented Jul 10, 2020

Quick update: I was called away to other things yesterday but now it's a new day and I'm at my computers with bleary eyes and a cup of coffee, hoping to make some progress.

I've looked through the FreeRTOS API documentation. There isn't a function to get the live stack level (heap, yes but not stack). I'm working towards using the pointer arithmetic method and I'm now trawling through code to find where the stack size is set.

I found a #define called configMINIMAL_STACK_SIZE but I didn't find it where it is used. It's set at 2048 which is interesting because that's no where the numbers reported by uxTaskGetStackHighWaterMark() and the name suggests FreeRTOS somehow dynamically assigns memory to stacks for each task.

@MaBecker
Copy link
Contributor Author

@MaBecker MaBecker commented Jul 10, 2020

Hmm, what about calculation free stack size based on process.memory()?

>process.memory()
={ free: 3294, usage: 106, total: 3400, history: 16,
  gc: 0, gctime: 3.282, blocksize: 13 }
@gfwilliams
Copy link
Member

@gfwilliams gfwilliams commented Jul 10, 2020

I don't think process.memory knows about the stack size either?

@MaBecker
Copy link
Contributor Author

@MaBecker MaBecker commented Jul 10, 2020

I don't think process.memory knows about the stack size either?

Wow, so we are really blind for now on stack and free stack size. for ESP32 and ESP8266 ......

@GaryOtt
Copy link
Contributor

@GaryOtt GaryOtt commented Jul 10, 2020

JSVars are assigned using malloc() which uses heap memory:

Espruino/src/jsvar.c

Lines 265 to 280 in e78f33f

void jsvInit(unsigned int size) {
#ifdef RESIZABLE_JSVARS
assert(size==0);
jsVarsSize = JSVAR_BLOCK_SIZE;
jsVarBlocks = malloc(sizeof(JsVar*)); // just 1
jsVarBlocks[0] = malloc(sizeof(JsVar) * JSVAR_BLOCK_SIZE);
#elif defined(JSVAR_MALLOC)
if (size) jsVarsSize = size;
if(!jsVars) jsVars = (JsVar *)malloc(sizeof(JsVar) * jsVarsSize);
#else
assert(size==0);
#endif
jsVarFirstEmpty = jsvInitJsVars(1/*first*/, jsVarsSize);
jsvSoftInit();
}

We're a little blind but I have more coffee and I REALLY need to stop my Espruino based project from crashing.

@GaryOtt
Copy link
Contributor

@GaryOtt GaryOtt commented Jul 10, 2020

The Espruino stack allocation for the ESP32 is defined by literals of 25000 bytes at lines 104 and 107 of main.c:

#ifdef RTOS
queues_init();
tasks_init();
task_init(espruinoTask,"EspruinoTask",25000,5,0);
task_init(uartTask,"ConsoleTask",2200,20,0);
#else
xTaskCreatePinnedToCore(&espruinoTask, "espruinoTask", 25000, NULL, 5, NULL, 0);
xTaskCreatePinnedToCore(&uartTask,"uartTask",2200,NULL,20,NULL,0);
#endif

I'm working on an update to jsuGetFreeStack(). I'll change those literals to constants, check my code works and begin reading how to go about making my first commit to Espruino. It might take me some time to make sure I don't do anything silly. Links to any current guides would be quite helpful.

@MaBecker
Copy link
Contributor Author

@MaBecker MaBecker commented Jul 12, 2020

Finding: I2C setup write messages

WARNING: jshI2CSetup: driver installed, sda: 25 scl: 21 freq: 100000

Update: ifndef jsWarn() depending on

@MaBecker
Copy link
Contributor Author

@MaBecker MaBecker commented Jul 12, 2020

Finding: I2C read writes messages if device is not attachhed

ERROR: jshI2CRead:, slave doesn't ACK the transfer.

Update: change error message to exception to make a scanner work

function isDeviceOnBus(i2c,id) {
      try {
        return i2c.readFrom(id,1);
     }
      catch(err) {
        return -1;
      }
}
@gfwilliams
Copy link
Member

@gfwilliams gfwilliams commented Jul 13, 2020

I guess a simple solution would actually be to define the stack size inside the BOARD.py file: https://github.com/espruino/Espruino/blob/master/boards/ESP32.py#L39

so 'DEFINES+=-DESP_STACK_SIZE=25000' - then you could use it anywhere in the code, and also it'd be possible to change it on a per-board basis which could be really handy

@jumjum123
Copy link
Contributor

@jumjum123 jumjum123 commented Jul 13, 2020

@GaryOtt
Copy link
Contributor

@GaryOtt GaryOtt commented Jul 13, 2020

I have a working (hopefully) version of jsuGetFreeStack() for the ESP32. The numbers returned appear sensible and I haven't seen any new problems in my application that uses WiFi, HTTP and Serial so it's a promising start.

Going back to the original problem with jsfCompactInternal(), I'm now getting an error message due to insufficient stack space instead of an out right crash. jsfCompactInternal() attempts to place a whole file onto the stack but some of my files are considerably larger than the entire stack allocation. I'm working on improving jsfCompactInternal() so that it will move files in multiple writes when the file size exceeds the available stack space. I'm expecting this to reveal any short comings in jsuGetFreeStack().

I'll submit these changes as two different commits when ready.

Regards guidance for new contributors, unless anyone tells me anything has changed, I'll follow this: https://github.com/espruino/Espruino/tree/master/targets/esp32

@MaBecker
Copy link
Contributor Author

@MaBecker MaBecker commented Jul 13, 2020

@GaryOtt have you thought about using other available flash areas?

>require('Flash').getFree()
=[
  { addr: 57344, length: 8192 },
  { addr: 3211264, length: 65536 },
  { addr: 3538944, length: 655360 }
 ]
@gfwilliams
Copy link
Member

@gfwilliams gfwilliams commented Jul 13, 2020

Great - thanks! Honestly, I wouldn't put too much effort into jsfCompactInternal right now unless you have the time for it - there are a bunch of issues with it and a rewrite is very high up my to-do list. Giving you're having issues too I'll try and get something done this week.

Just a bit of background - the Storage was designed initially to handle a big chunk of program code plus a few extra bits of config info that changed. It had to work on STM32F411 (where there is only a single 64k page, but 128k RAM), and then nRF52 where we had ~10x4k pages. Obviously ESPxx and Bangle.js now have shedloads of pages, and the quick and dirty compaction just doesn't work in those cases.

Regards guidance

Yes, totally. Just follow what's there - and try not to change a bunch of whitespace for existing code (at least not in the same commit as the fix). In terms of code style, please try and use the sort of style from http://www.espruino.com/Code+Style (eg what you'll find in the code in src). Some of the ESPxx code doesn't follow the style used for the rest of Espruino.

But the fact you're even asking the question means I'm pretty confident what you commit will be fine :)

@GaryOtt
Copy link
Contributor

@GaryOtt GaryOtt commented Jul 13, 2020

@MaBecker
No, I haven't tried to use the Flash module because of what I'm trying to do with the files. Although I could move some files out of Storage, I would still encounter problems with those that remain. My biggest file is .bootcde which I believe has to be in Storage. During development of my Javascript application, I'm repeatedly loading new versions of files into storage. When it gets to the point it crashes, I have been reflashing with Espruino and starting from scratch.

@gfwilliams
I'd be very appreciative if you rewrite the Storage module. From my point of view, that would be a right result!

I'll continue doing what I'm doing until I've at least proven jsuGetFreeStack() to my own satisfaction.

Thanks for the back story. I've been trying to reverse engineer all the design considerations from the existing code but I was always going to miss something.

@gfwilliams
Copy link
Member

@gfwilliams gfwilliams commented Jul 14, 2020

@GaryOtt ok, could you try now? I think the recent storage compaction changes will probably sort you out

@GaryOtt
Copy link
Contributor

@GaryOtt GaryOtt commented Jul 14, 2020

Great, thank you. I will report back as soon as I can which will probably be this afternoon.

@GaryOtt
Copy link
Contributor

@GaryOtt GaryOtt commented Jul 14, 2020

@gfwilliams Good and not so good. It's passes the test posted above which is a big leap forward but I do have another error.

I get Guru Meditation Error: Core 0 panic'ed (LoadStoreError). Exception was unhandled when my project attempts to rewrite configuration files (it works on the first write). I haven't yet dug in to find exactly where the error is triggered so that will be my next job.

I'm trying to push the update to jsuGetFreeStack() but have a problem. Following the instructions, after providing my GitHub credentials for git push -u origin --all, I get an error 403. If I deliberately put in a wrong password I am told my username or password were wrong. So it looks like I need to be approved before I can commit. Is that right?

@MaBecker
Copy link
Contributor Author

@MaBecker MaBecker commented Jul 14, 2020

@GaryOtt there are some nice helper for working with git like GitHup Desktop or SourcetTree.
Using a tool like that will make many things much simpler and easier.

@parasquid
Copy link

@parasquid parasquid commented Jul 14, 2020

I'm trying to push the update to jsuGetFreeStack() but have a problem. Following the instructions, after providing my GitHub credentials for git push -u origin --all, I get an error 403. If I deliberately put in a wrong password I am told my username or password were wrong. So it looks like I need to be approved before I can commit. Is that right?

If you cloned the Espruino repo (we can call this upstream) then yes, you'd need commit privileges to push changes to it. The usual way would be to fork the upstream repo into a repo that you would own (so something like https://github.com/GaryOtt/Espruino.git ) and then you can push your changes that way. When you're ready to get your work reviewed and merged, you can then send a Pull Request against upstream.

@GaryOtt
Copy link
Contributor

@GaryOtt GaryOtt commented Jul 14, 2020

@parasquid Thank you for the help earlier.

@gfwilliams The storage module is so much better now. I made a separate issue for the LoadStoreError here: #1883

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants
You can’t perform that action at this time.