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

Allow FS (fatfs) API to work on ESP8266's flash #1116

Closed
gfwilliams opened this issue Mar 14, 2017 · 71 comments
Closed

Allow FS (fatfs) API to work on ESP8266's flash #1116

gfwilliams opened this issue Mar 14, 2017 · 71 comments
Labels
ESP32 This is only a problem on ESP32-based devices ESP8266 This is only a problem on ESP8266 devices implemented

Comments

@gfwilliams
Copy link
Member

This would make a lot of sense for the 4M parts (#1110)

I think this got shot down a while ago with concerns about repeated writes to flash, but I think realistically the use it would be to everyone would far outweigh any problems.

So either the fatfs library could be tweaked such that the block read/write functionality wrote flash blocks (which I imagine would be easy), or SPIFFS could be ported - but that'd mean re-writing every file IO function to use SPIFFS :(

@gfwilliams gfwilliams added the ESP8266 This is only a problem on ESP8266 devices label Mar 14, 2017
@wilberforce
Copy link
Member

wilberforce commented Mar 14, 2017

Hi Gordon,
If I recall when @tve looked at this it was to due with the 4096 byte pages vs 512 bytes sector size.

I have been thinking about storage recently and looking at options.

The esp32 has a number of options that could be of use...

http://esp-idf.readthedocs.io/en/latest/api/storage/index.html

  1. There is a nvs storage -
    Non volatile storage that uses a key and stores a value. All sorts of types are supported, however I thought the best thing to use would Ben the binary blob...

So the idea is to take a js object - json stringy, compress with heat shrink, and then store the blob against the key.
For retrieval, it would be lookup, decompress, and restore into a js object. This way we can support, all sorts of complex js types - nested structures etc, and also functions and modules....

  1. The esp32 also supports vfs. A virtual file system. This is cunning in that if links into newlib, so that the standard posix fopen hook into this transparently.

They have an sd card fat example that uses this.

This means that the Lunix implemention of the fs Library could be made to use this, and the layer of mapping to sd card and fat done via vfs. There is some talk of a spiffs implementation, however have not seen that yet. Cesanta have a spiffs library on github for the esp32/8266. The lua fold have added too.

  1. There is a read only file system https://github.com/Spritetm/libesphttpd/tree/master/espfs
    This uses a tools to make an gpio file and copy to to flash. The API then provides read only access.

  2. what every file system is implemented, and easy way of updating would be required. As we already have a webserver implementation, I think a http PUT method would be a good way of getting files onto the device, or even a page that files could be dargged and dropped onto..

@wilberforce
Copy link
Member

I was looking fs libs last night and the implemention is #ifdef Linux.... #else

If this was abstracted the same way as the network library, with a function table it would be more extensible and easier to add new layers

@wilberforce
Copy link
Member

Here is the nvs stuff on the esp32

https://gitter.im/espruino/esp32?at=58c66411dd08b4b859c2c560

At this stage just working with i32

@gfwilliams
Copy link
Member Author

Yes, some kind of function table could work. I guess it depends whether we have to support different filesystem types with the same build? If not we could do like jsHardware - create a common API and then just compile in the implementation that's needed? Swapping to the function table from that could be easy to do later though.

Personally, I don't think adding ESP32-specific stuff is a good idea at the moment - 99% of people with this problem right now are using ESP8266, so ideally we want some solution which handles both.

It's fine just to set the block size to 4096b in FatFS - which would solve this? http://elm-chan.org/fsw/ff/en/appnote.html

It would require 4k of RAM temporarily to access files, but it's not the end of the world. Or there's so much free flash available it wouldn't be the end of the world to just ignore the other 3500 bytes.

I kind of like that approach, because we can do it using the existing Espruino flash APIs. The filesystem could then be supported on any device that Espruino runs on if it has spare pages of flash - not just ESP8266/32.

For writing, a simple JS webserver that handles posting data should be fine - maybe as a library. I'd posted some example code on the forum a year or so ago.

Also at some point I'll be looking at adding a built-in server to allow console over websocket, and I guess at that point we could look at adding a way to upload files with it.

@wilberforce
Copy link
Member

wilberforce commented Mar 14, 2017

Here is a js post server that emulates the esp8266 ota server that is in firmware.

https://github.com/wilberforce/Espruino/blob/ota/targets/esp32/tests/ota-server.js

I was thinking something like this could be extended, so not only handle ota updates, but update files in a storage system.

@wilberforce
Copy link
Member

So it looks like these set of functions would need to be set up to use the inbuild flash libs:

https://github.com/espruino/Espruino/blob/master/libs/filesystem/fat_sd/diskio.h#L34-L38

@gfwilliams
Copy link
Member Author

I think so, yes. It seems like they'd all be pretty simple. Looks like you specify the sector size with ioctl as well: http://elm-chan.org/fsw/ff/en/dioctl.html

Part of me wonders whether that could be exposed to JS. It'd be very cool to be able to put a FAT filesystem on external SPI flash.

@wilberforce
Copy link
Member

wilberforce commented Mar 15, 2017

http://elm-chan.org/fsw/ff/en/appnote.html#port

Function Required

  • get_fattime
  • disk_status
  • disk_initialize
  • disk_read
  • disk_write
  • disk_ioctl (CTRL_SYNC)
  • disk_ioctl (GET_SECTOR_COUNT)
  • disk_ioctl (GET_BLOCK_SIZE)
  • disk_ioctl (GET_SECTOR_SIZE)
DSTATUS disk_initialize (BYTE pdrv);
DSTATUS disk_status (BYTE pdrv);
DRESULT disk_read (BYTE pdrv, BYTE* buff, DWORD sector, UINT count);
DRESULT disk_write (BYTE pdrv, const BYTE* buff, DWORD sector, UINT count);
DRESULT disk_ioctl (BYTE pdrv, BYTE cmd, void* buff);

Use as template for starting point
https://github.com/espruino/Espruino/blob/master/libs/filesystem/fat_sd/sdio_diskio.c

gotchas:

  • Write will need to pad to 4 bytes?

@wilberforce
Copy link
Member

wilberforce commented Mar 19, 2017

It doesn't look like much, however have made a start here:
master...wilberforce:flash_fs

It would be great if we can select as using an SD card or flash as some people are already using the SD card on the ESP32.

Perhaps with some sort of initialiser like: E.connectSDCard(s,D16);

perhaps `E.flashFS(0x300000); or do you think the address for the flash should be defaulted?

The other possible params would be fs type - fat/fat32 or size of the block of flash to use?

@gfwilliams
Copy link
Member Author

Nice - does it work?

I didn't realise there was a mkfs command - I guess we could expose that as a special format function?

Probably for the final version I'd add a new flash_diskio.c file, since on the majority of platforms it's actually spi_diskio.c that gets included, not the spi one (iirc).

It sounds like a good idea to force people to initialise it, yes. To stick with E.connectSDCard maybe do E.connectFlashFS? I guess we could make it check what areas of memory are free (jshardware.c has jshFlashGetFree or something similar) if nothing is specified? It'd have to know how much memory it could use as well :)

@wilberforce
Copy link
Member

wilberforce commented Mar 20, 2017

No - it doesn't work ;-(
I'm getting crashes at the moment when I try to mount the fs fat system.

I have set up a test harness that attempts to make the file system. It's going to be pretty slow as all of the pages get set to zero as part of the format - only to clear them back to FF before the next write..

It is spi_diskio.c I'm changing at the moment - Ideally want to make it switchable so depending on what call you make, you can connect to flash or an SD card.

BTW - what command do you use to format the SD card in linux? I've been trying to make an img, and then flash that and then try to read from it..

``
dd if=/dev/zero of=fat.fs bs=1024 count=1024
mkfs.vfat -v -F 16 -S 4096 -s 1 fat.fs.img


This spits out errors:

WARNING: Not enough clusters for a 16 bit FAT! The filesystem will be
misinterpreted as having a 12 bit FAT without mount option "fat=16".
fat.fs.img has 64 heads and 32 sectors per track,
hidden sectors 0x0000;
logical sector size is 4096,
using 0xf8 media descriptor, with 256 sectors;
drive number 0x80;
filesystem has 2 16-bit FATs and 1 sector per cluster.
FAT size is 1 sector, and provides 249 clusters.
There is 1 reserved sector.
Root directory contains 512 slots and uses 4 sectors.
Volume ID is a849e3a6, no volume label.


So I really want to start from a known state.



@gfwilliams
Copy link
Member Author

That command looks ok to me as far as I know (however I guess fat.fs and fat.fs.img should be the same?).

I guess another option would be to implement jshFlash* on Linux (just writing to a file) and then you could run and debug much more easily?

@wilberforce
Copy link
Member

wilberforce commented Mar 20, 2017

Yay - success (of sorts).

Created - libs/filesystem/fat_sd/flash_diskio.c
https://github.com/espruino/Espruino/compare/master...wilberforce:flash_fs?expand=1

The files system can be formatted, and files written.

>var files = require("fs").readdirSync();
WARNING: Flash Init - disk_initialize 0
WARNING: Flash disk_read sector: 0, buff: 1073526768 len: 4096
WARNING: jsfsInit - appears to f_mount!
WARNING: Flash Init - disk_status 0
WARNING: Flash Init - disk_status 0
WARNING: Flash disk_read sector: 2, buff: 1073526768 len: 4096
WARNING: Flash Init - disk_status 0
WARNING: Flash Init - disk_status 0
WARNING: Flash Init - disk_status 0

=[
  "hello.txt",
  "hello2.txt",
  "data.csv"
 ]

however, it is crashing on file read - which is weird since the folder structures etc can be read fine.

make clean && BOARD=ESP32 USE_FLASH_FILESYSTEM=1 make

@gfwilliams
Copy link
Member Author

I don't know - maybe print out exactly what addresses and sizes you're reading and see if anything looks odd about the one that crashes.

Good sign though!

@wilberforce
Copy link
Member

I've spent quite a bit of time on this today - it's very frustrating.

Reading back the flash:

f=require("Flash");
b=0x300000;
function rSect(s){
uintArray=f.read(64,b+s*4096);
+console.log(String.fromCharCode.apply(null, uintArray));
}
rSect(34);

It looks like all the bits are getting put in the write places... however still crashes on file reads on the ESP32 -so it could be be some other issue?

I guess I could see it compiles on the esp8266 and if the same problem .

Downloading the 1M block and trying to read the img file does not work either - so it could be some sort of corruption with what is written.

Anyway, it's a starting point if anyone is keen for the challenge.

@gfwilliams
Copy link
Member Author

I just committed some flash emulation functions to the Linux port - so if you're able to compile your code for linux then you should be able to see how it works on the PC (might need some bodging in jswrap_fs to force it to use the SPI FS functions).

It'll be much easier to debug and it should even trap segfaults/etc :)

@wilberforce
Copy link
Member

wilberforce commented Mar 21, 2017

It's interesting how comments trigger thought processes... following what you said about fs_wrap lead to here... I think this read is the issue...

JsVar *buffer = jswrap_file_read(f, 0x7FFFFFFF);

JsVar *buffer = jswrap_file_read(f, 0x7FFFFFFF);

Is this meant to be -1 ?

@gfwilliams
Copy link
Member Author

gfwilliams commented Mar 21, 2017

That read itself should be safe since you don't want the user to be able to segfault by calling File.read(0x7fffffff) - but it could still be causing you some trouble for another reason I guess :) Maybe something like ftell is broken?

@wilberforce
Copy link
Member

Found it. argg,h. It was a buffer overflow and no safety checks in the ff.c

#define	_MIN_SS		512
#define	_MAX_SS		512

to

#define	_MIN_SS		512
#define	_MAX_SS		4096

https://github.com/espruino/Espruino/blob/master/libs/filesystem/fat_sd/ffconf.h#L174-L181

According to the comment - get sector size is implemented so we should be able to leave _MAX_SS 4096 - unless you want it in an ifdef?

here is some output anyway...

>E.flashFatFs(0x300000,2);
WARNING: E.flashFatFs addr: 3145728 format: 2
WARNING: Flash Init - disk_initialize 0
WARNING: Flash disk_ioctl 2
WARNING: disk_ioctl FS_SECTOR_SIZE 4096
WARNING: Flash disk_read sector: 0, buff: mem: 3145728 buff: 1073527048 len: 4096
WARNING: after read

ERROR: Unable to mount SD card : NO_FILESYSTEM
WARNING: jsfsInit: 0
=undefined
>E.flashFatFs(0x200000,1);
WARNING: E.flashFatFs addr: 2097152 format: 1
ERROR: E.flashFatFs formatting...
WARNING: Flash Init - disk_initialize 0
WARNING: Flash disk_ioctl 2
WARNING: disk_ioctl FS_SECTOR_SIZE 4096
WARNING: Flash disk_ioctl 1
WARNING: disk_ioctl FS_SECTOR_COUNT 256
WARNING: Flash disk_ioctl 3
WARNING: disk_ioctl FS_BLOCK_SIZE 1
WARNING: Flash disk_write sector:  0, buff: mem: 2097152 buff: 1073527048 len: 4096
WARNING: Flash disk_write sector:  1, buff: mem: 2101248 buff: 1073527048 len: 4096
WARNING: Flash disk_write sector:  2, buff: mem: 2105344 buff: 1073527048 len: 4096
WARNING: Flash disk_write sector:  3, buff: mem: 2109440 buff: 1073527048 len: 4096
WARNING: Flash disk_write sector:  4, buff: mem: 2113536 buff: 1073527048 len: 4096
WARNING: Flash disk_write sector:  5, buff: mem: 2117632 buff: 1073527048 len: 4096
WARNING: Flash disk_ioctl 0
WARNING: Flash disk_ioctl CTRL_SYNC
=undefined
>require("fs").writeFileSync("hello.txt", "Hello World\nHello World\nNew line\n");
WARNING: Flash Init - disk_initialize 0
WARNING: Flash disk_ioctl 2
WARNING: disk_ioctl FS_SECTOR_SIZE 4096
WARNING: Flash disk_read sector: 0, buff: mem: 2097152 buff: 1073527048 len: 4096
WARNING: after read

WARNING: jsfsInit - appears to f_mount!
WARNING: Flash Init - disk_status 0
WARNING: Flash disk_read sector: 2, buff: mem: 2105344 buff: 1073527048 len: 4096
WARNING: after read

WARNING: Flash Init - disk_status 0
WARNING: Flash disk_write sector:  2, buff: mem: 2105344 buff: 1073527048 len: 4096
WARNING: Flash disk_read sector: 1, buff: mem: 2101248 buff: 1073527048 len: 4096
WARNING: after read

WARNING: Flash Init - disk_status 0
WARNING: Flash Init - disk_status 0
WARNING: Flash disk_write sector:  6, buff: mem: 2121728 buff: 1073550728 len: 4096
WARNING: Flash disk_write sector:  1, buff: mem: 2101248 buff: 1073527048 len: 4096
WARNING: Flash disk_read sector: 2, buff: mem: 2105344 buff: 1073527048 len: 4096
WARNING: after read

WARNING: Flash disk_write sector:  2, buff: mem: 2105344 buff: 1073527048 len: 4096
WARNING: Flash disk_ioctl 0
WARNING: Flash disk_ioctl CTRL_SYNC
WARNING: Flash Init - disk_status 0
WARNING: Flash Init - disk_status 0
=true
>var files = require("fs").readdirSync();
WARNING: Flash Init - disk_status 0
WARNING: Flash Init - disk_status 0
WARNING: Flash Init - disk_status 0
=[
  "hello.txt"
 ]
>console.log(require("fs").readFileSync("hello.txt")); // prints "Hello World"
WARNING: Flash Init - disk_status 0
WARNING: Flash Init - disk_status 0
WARNING: Flash disk_read sector: 6, buff: mem: 2121728 buff: 1073550284 len: 4096
WARNING: after read

WARNING: Flash Init - disk_status 0
WARNING: Flash Init - disk_status 0
Hello World
Hello World
New line

=undefined

@wilberforce
Copy link
Member

wilberforce commented Mar 21, 2017

further good news....

read the flash to fs.img:

..\esp-idf\components\esptool_py\esptool\esptool.py --chip esp32 --port COM3 --baud 921600  read_flash 0x200000 0x100000 fs.img

And on windows opened with OSFMount. This mounts it as a drive... copied the README.md

..\esp-idf\components\esptool_py\esptool\esptool.py  --chip esp32 --port COM3 --baud 921600 --before esp32r0 --after hard_reset write_flash -z --flash_mode dio --flash_freq 40m --flash_size detect  0x200000 fs.img
>var files = require("fs").readdirSync();

=[
  "hello.txt",
  "README.md"
 ]
>console.log(require("fs").readFileSync("README.md"));
Espruino JavaScript for Microcontrollers
========================================
<pre>
 _____                 _
|   __|___ ___ ___ _ _|_|___ ___
|   __|_ -| . |  _| | | |   | . |
|_____|___|  _|_| |___|_|_|_|___|
          |_|
</pre>
http://www.espruino.com &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; )

@wilberforce
Copy link
Member

I copied a module onto the SDcard:

>console.log(require("fs").readFileSync("/DS18B20.js"));
var C={CONVERT_T:0x44,COPY:0x48,READ:0xBE,WRITE:0x4E};function DS18B20(oneWire,}
if(!this.sCode)throw new Error("No DS18B20 found");this.type=parseInt(this.sCod}
DS18B20.prototype._r=function(){var b=this.bus;b.select(this.sCode);b.write(C.R)
c=(c>>1)^0x8C*(c&1);}
var temp=null;if(c==s[8]){temp=s[0]+(s[1]<<8);if(temp&32768)temp-=65536;temp=te}
if(callback)callback(temp);return temp;}
this.bus.select(this.sCode);this.bus.write(C.CONVERT_T,true);if(!callback)retur}
DS18B20.prototype.searchAlarm=function(){return this.bus.search(0xEC);};DS18B20;
=undefined

>d=require("DS18B20");
WARNING: Flash Init - disk_status 0
WARNING: Flash disk_read sector: 2, buff: mem: 2105344 buff: 1073527048 len: 4096
WARNING: after read

ERROR: Could not open file : NO_PATH
WARNING: Module "DS18B20" not found
=undefined

No sure what the path or syntax is meant to be for this?

@wilberforce
Copy link
Member

@MaBecker
I'll clean this up and then you can try it out on the ESP8266!

@gfwilliams
Copy link
Member Author

I think it looks in a 'node_modules' folder?

That's great news though!
_MAX_SS would definitely need ifdefing - it's used here afaik: https://github.com/espruino/Espruino/blob/master/libs/filesystem/fat_sd/ff.h#L105

Which is statically allocated in jswrap_file.c - so by increasing it you're using 3.5kB of RAM all the time.

I guess potentially:

  • We could look at only allocating that buffer out of jsVars when needed, which could be good for all platforms
  • Maybe on devices where you care about the RAM (everywhere apart from ESP32?) you could just use 512 byte blocks and waste 3.5k of each flash block
  • If we know we'll always have 4k of free RAM on the stack then we could still write 512 byte blocks but could 'emulate' block erase by reading 4k, erasing, then writing a full 4k again

@wilberforce
Copy link
Member

wilberforce commented Mar 22, 2017

I have used #ifdef for the sector side, so the other builds are not affected.

https://github.com/espruino/Espruino/compare/master...wilberforce:flash_fs?expand=1

The code is getting close to ready to merge.

  • move the flash*.bat to scripts - these are windows flashing scripts - move to /scripts/eps32/ ?

E.flashFatFs( { addr: 0x300000, sectors : 100, readonly: true } );

  • the flash fs is auto mounted on demand and formatted on first use, at the default address. This can be overidden using E.flashFatFs
  • readonly not yet implemented.

The Fatfs also supports disks.... 0:disk0_file.txt 1:disk0_file.txt so we have the potential of using system and user volumes - like modules on 0: and user files on another....

@gfwilliams
Copy link
Member Author

Nice, thanks! Looks to me like disk_ioctl might need to be aware of the different sector sizes too?

It might be an idea to just disable it by default or at least make it read-only? as-is, I imagine just trying to use a module that doesn't exist would cause 1MB of flash memory to get formatted?

I'd steer clear of multiple disks - that just sounds like pain (i mean, that's what directories are for?). If we did do that, IMO we should focus on allowing normal SD cards and the flash memory to coexist on separate volumes first as that could be really useful.

Please could we move jsDebug to just commented out jsWarns for now? seems like overkill to add a new global debug macro just for this?

@wilberforce
Copy link
Member

I made the jsDebug macro global, so it can be used anywhere - not just with this code.
It is useful when you need to see the details, and is removed when RELEASE=1

I don't have any gdb debugging, so was using this. I can make it local to these files if required.

The fat_initialised flag is off at startup, so a missing module should not trigger a format. I'll check this.

I just wanted to make it easy to use, by turning on if you used fs or file...

@wilberforce
Copy link
Member

wilberforce commented Mar 23, 2017

To Do:

  • Fix create fs if missing module, init call required
  • readonly mode - seems to be compile only option?
  • mount command - format param
  • turn off other E.SD stuff ifndef E.connectSDCard E.unmountSD
  • new File("hello world.txt").pipe(res); fails http://www.espruino.com/File+IO#line=34,49,59 No File.pipe method
  • add mkdir
  • test mkdir on linux

Pull request #1119

@wilberforce
Copy link
Member

wilberforce commented Mar 23, 2017

@MaBecker - Working on the ESP8266!

For this 4Mb build:
#1110

Add to the BOARD.py:
'NEOPIXEL',
'FILESYSTEM',
'FLASHFS'
]

http://espruino.com/Donate
Flash map 4MB:512/512, manuf 0xe0 chip 0x4016
>fs=require("fs");
=function () { [native code] }
=undefined
>if ( typeof(fs.readdirSync())==="undefined" ) {
:  console.log("Formatting FS");
:  E.flashFatFS({format:true});
:}
=undefined
=undefined
>fs.readdirSync();
=[
  "hello world.txt",
  "node_modules"
 ]
>require("fs").writeFileSync("hello world.txt", "This is the way the world ends\nHello World\nnot with a bang but a whimper.\n");
=true
=undefined
>fs.readdirSync();
=[
  "hello world.txt",
  "node_modules"
 ]

// This is with 'GRAPHICS', and  'CRYPTO' as wont fit on 512 image
>require("ESP8266").getState().freeHeap;
=6312

@wilberforce wilberforce added the ESP32 This is only a problem on ESP32-based devices label Mar 23, 2017
@hungryforcodes
Copy link

Does it work for you on the ESP32, btw?

@wilberforce
Copy link
Member

wilberforce commented Apr 11, 2017

Pipe read from file piping to http res does not work on esp32 however everything in the comment above works:

I modified you write test to be less extreme.....

With the esp8266 you are battling with such little memory, and there might be other things going on.

The post above with pipe=0 and if you set the read buffer to 32, it should work for you.

@hungryforcodes
Copy link

Right -- we both agree on that. And its great work! :) Aside from the "closing" bug we also both agree on. It already makes things more usable even just for basic file I/O, and this is amazing, and honestly I'm very thankful for it. :)

However now I'm trying to write using pipe (or even fs.write()) from a browser. So I was curious if you had tried writing (say a large file from a browser) to the ESP32. Anyways its not so important I'll probably find a way around it sometime. I'll continue testing and see if I can find anything more concrete for you to work with.

@wilberforce
Copy link
Member

This should work for you on the ESP8266:

if ( pipe === 0 ) {
do{
data = f.read(32);
if (data) res.write(data);
} while(data);
res.end();
f.close();
}

Please try that.

@hungryforcodes
Copy link

Yes I'm sorry I wasn't clear -- I agree some variation of that will work on the ESP8266. I tried that extensively on the weekend -- the do / while construct will fail with out of memory on very large files like in my original example. Your modified example of mine is not large enough to trigger this. Its only a few K not a few 100K. However, this WILL work generally:

 readInterval=setInterval(function() {
      data = f.read(32);
      if (data) {res.write(data);}
      if (!data) {
        clearInterval(readInterval);
        f.close();
        res.end(page);
        console.log("\/\/\/ ::: READ ENDING ::: \\\\\\");
        return;
        }
  },100);

though occasionally will not detect the end of data and will start outputting again. Since Gordon's pipe() works more or less fine, I didn't pursue it too much further. It would seem that buffers are not being cleared fast enough for the do / while construct. If I reduce the setTimeout below 100ms, or increase the f.read() to 64 or higher and keep the setTimeout at 100ms, it eventually does run out of memory about halfway through my example (the 10000 iteration one).

But we agree that pipe works for sending data to the browser :)

What I am trying to do is WRITE to the file system, using pipe(). My understanding is that if I want to upload stuff to the ESP8266, this would be the best way if the file is large.

Again I wasn't clear -- I believe we are confusing "res.write()" with a "write to the file system". Its the second I would like to do. So again my question would be have you tried writing to the file system on the ESP32 this way, using pipe()? If you can, then its clearly a problem on the ESP8266 side, is more or less what I'm trying to figure out. :)

Thanks!
hfc

@wilberforce
Copy link
Member

ok. I Found the issue for reading a file wth File.pipe. It is the read:

JsVar *jswrap_file_read(JsVar* parent, int length) {
  jsWarn( ">> jswrap_file_read len %d",length);
  if (length<0) length=0;
  JsVar *buffer = 0;
  JsvStringIterator it;
  FRESULT res = 0;
  size_t bytesRead = 0;
  if (jsfsInit()) {
    JsFile file;
    if (fileGetFromVar(&file, parent)) {
      if(file.data.mode == FM_READ || file.data.mode == FM_READ_WRITE) {
        size_t actual = 0;
#ifndef LINUX
        // if we're able to load this into a flat string, do it!
        size_t len = f_size(&file.data.handle)-f_tell(&file.data.handle);
        
        jsWarn( ">> jswrap_file_read  flat string %d",len);
        if (len>(size_t)length) len=(size_t)length;
        if ( len == 0 ) {
          jsError( ">> jswrap_file_read returning empty string");
          buffer = jsvNewFromEmptyString();
          return buffer;
        }
        buffer = jsvNewFlatStringOfLength(len);
        jsWarn( ">> jswrap_file_read  buffer is now  %d",buffer);
        if (buffer) {
          res = f_read(&file.data.handle, jsvGetFlatStringPointer(buffer), len, &actual);
          if (res) jsfsReportError("Unable to read file", res);
          fileSetVar(&file);
           jsWarn( ">> jswrap_file_read  returning flat string buffer %d",res );
          return buffer;
        }
        jsError( ">> jswrap_file_read  flat string fall through ... %d",len);
#endif
        char buf[32];

        while (bytesRead < (size_t)length) {
          size_t requested = (size_t)length - bytesRead;
          jsWarn( ">> jswrap_file_read requested %d",requested);
          if (requested > sizeof( buf ))
            requested = sizeof( buf );
          actual = 0;
          jsWarn( ">> jswrap_file_read requested now %d",requested);
    #ifndef LINUX
          res = f_read(&file.data.handle, buf, requested, &actual);
          if(res) break;
    #else
          actual = fread(buf, 1, requested, file.data.handle);
          jsWarn( ">> jswrap_file_read actual %d, got %s",requested, buf);
    #endif
          if (actual>0) {
            if (!buffer) {
              buffer = jsvNewFromEmptyString();
              if (!buffer) return 0; // out of memory
              jsvStringIteratorNew(&it, buffer, 0);
            }
            size_t i;
            for (i=0;i<actual;i++)
              jsvStringIteratorAppend(&it, buf[i]);
          }
          bytesRead += actual;
          if(actual != requested) break;
        }
        fileSetVar(&file);
      }
    }
  }
  if (res) jsfsReportError("Unable to read file", res);

  if (buffer)
    jsvStringIteratorFree(&it);

  return buffer;
}

This goes into a loop:

WARNING: >> jswrap_file_read len 64
WARNING: >> jswrap_file_read  flat string 0
ERROR: >> jswrap_file_read returning empty string
WARNING: >> jswrap_file_read len 64
WARNING: >> jswrap_file_read  flat string 0
ERROR: >> jswrap_file_read returning empty string
WARNING: >> jswrap_file_read len 64
WARNING: >> jswrap_file_read  flat string 0
ERROR: >> jswrap_file_read returning empty string
WARNING: >> jswrap_file_read len 64
WARNING: >> jswrap_file_read  flat string 0
ERROR: >> jswrap_file_read returning empty string
WARNING: >> jswrap_file_read len 64
WARNING: >> jswrap_file_read  flat string 0
ERROR: >> jswrap_file_read returning empty string
WARNING: >> jswrap_file_read len 64
WARNING: >> jswrap_file_read  flat string 0
ERROR: >> jswrap_file_read returning empty string
WARNING: >> jswrap_file_read len 64
WARNING: >> jswrap_file_read  flat string 0
ERROR: >> jswrap_file_read returning empty string
WARNING: >> jswrap_file_read len 64

If this part is changed #ifndef LINUX to #ifdef XXXLINUX to comment out the block, and the fall through code is called, it works...

So it seems the code that optimisation for a flat string returns something different so the receiving pipe does does recognise this as the end of file.

@hungryforcodes
Copy link

hungryforcodes commented Apr 11, 2017

OK great! I think there has been progress then on both sides. I don't know quite enough about the Espruino code to fully get whats going on there, but I get the gist of it. :)

On my side I rethought about the problem and realized that the setTimeout was really awkward, and so after going through the docs thought to redo our snippet ("pipe==0"), with

res.on("drain", function () {....});

which really seems like the correct way to do it. And sure enough it works, even detecting the end of data and does not leave the browser hanging. Here is the snippet I ended up with:

f = E.openFile("data.csv","r");

    res.on("drain", function () {

      data = f.read(32);
          if (data) {res.write(data);}
          if (!data) {
              f.close();
              res.end(page);
              console.log("\/\/\/ ::: READ ENDING (DRAIN) ::: \\\\\\");
              return;
            }
        });

This of course is not the actual pipe() function, but is functionally identical (as far as I can tell), though a bit slower by maybe 20 seconds for my "large file" example.

Very minimal overhead in terms of JsVars too, which is great (maybe 100).

I would imagine this would easily work on the ESP32 as well. :)

Cheers,
hfc

@hungryforcodes
Copy link

Yes so the above snippet seems to work after over 20 requests, without any problems, and drops the JsVars by only about only 90 vars while transferring, and then it returns to baseline. Which on the ESP8266 is pretty good! My results from using pipe() dropped available memory by almost 500. Not sure why pipe() would do that, but if you're working on an extremely limited board like mine, the code above seems more efficient.

@hungryforcodes
Copy link

hungryforcodes commented Apr 11, 2017

Oh and finally I'm happy to announce that the reset after save (see my original post), seems to have totally disappeared since I changed boards. So I'd say its not an issue at this point. This I feel is why its important to always test on several of the same type of board, to see if you can replicate the problem.

@wilberforce
Copy link
Member

wilberforce commented Apr 11, 2017

@gfwilliams
I found the cause issue of the pipe not closing after a File.read

https://github.com/espruino/Espruino/blob/master/src/jswrap_pipe.c#L17-L18

  • Piping works as follows:
  • On idle, data is read from the source stream
    • If this returns undefined, the stream is considered finished and is closed
    • If this returns "" we just assume that it's waiting for more data

https://github.com/espruino/Espruino/blob/master/libs/filesystem/jswrap_file.c#L462-L472

 // if we're able to load this into a flat string, do it!
        size_t len = f_size(&file.data.handle)-f_tell(&file.data.handle);
        if (len>(size_t)length) len=(size_t)length;
        buffer = jsvNewFlatStringOfLength(len);
        if (buffer) {
          res = f_read(&file.data.handle, jsvGetFlatStringPointer(buffer), len, &actual);
          if (res) jsfsReportError("Unable to read file", res);
          fileSetVar(&file);
          return buffer;
        }

So when the len is 0, a buffer of "" is returned, so it waits for more data and then re-reads, resulting in a loop.

Here is the fix, to make it behave like the non flat string code:

        // if we're able to load this into a flat string, do it!
        size_t len = f_size(&file.data.handle)-f_tell(&file.data.handle);
        if ( len == 0 ) { // nothing left
          return 0; // if called from a pipe single end callback
        }

@gfwilliams
Copy link
Member Author

Great, thanks for finding that! Looks like it might have been a regression due to my addition of flat strings?

Do you want to issue a PR for it? Sorry for the lack of replies but I'm supposed to be on holiday this week :)

@wilberforce
Copy link
Member

wilberforce commented Apr 11, 2017

@gfwilliams

I can add to the existing PR I have in the ESP32 branch - Are you happy for me to apply that?
5f08a95

I noticed your absence - though you might have been away!

@gfwilliams
Copy link
Member Author

Yes, that's fine. On the ESP32 PR the romdata_jscodestuff looks a bit dodgy though - it's still executing all the code before and is then just overwriting the result of it

@wilberforce
Copy link
Member

Ah - you mean here:

Espruino/src/jswrap_flash.c

Lines 555 to 569 in d87fe18

code = (char *)(FLASH_DATA_LOCATION);
#ifdef ESP8266
// the flash address is just the offset into the flash chip, but to evaluate the code
// below we need to jump to the memory-mapped window onto flash, so adjust here
code += 0x40200000;
#endif
#ifdef ESP32
// romdata_jscode is memory mapped address from the js_code partition in rom - targets/esp32/main.c
extern char* romdata_jscode;
if (romdata_jscode==0) {
jsError("Couldn't find js_code partition - update with partition_espruino.bin\n");
return 0;
}
code = &romdata_jscode[FLASH_DATA_LOCATION-FLASH_SAVED_CODE_START];
#endif

It's executing this line which is not necessary:
code = (char *)(FLASH_DATA_LOCATION);

@wilberforce
Copy link
Member

Ok - tidied that up:
1c793f2

But now it is showing as a conflict? I think you might have changed permissions?

I know see:
You're not authorized to merge this pull request.

@MaBecker
Copy link
Contributor

Started to work with this on the ESP8266_4MB board

What am I missing, after a reboot the file is gone?

// test fs and E.flashFatFS

fs=require("fs");
freeFlash=require("ESP8266").getFreeFlash();
index=0;
fname="Hello.txt";

content="Hello Espruino\n";
if ( typeof(fs.readdirSync())==="undefined" ) {
  console.log("Formatting FS");
  E.flashFatFS({ addr:freeFlash[index].addr,
                 sectors:freeFlash[index].length/4096,
                 readonly:false,
                 format:true });
  require("fs").writeFileSync(fname,content );
}
else {
  require("fs").readFileSync(fname);
}

@MaBecker
Copy link
Contributor

Found the missing part, works as designed, great way to use 1MB.

var fs = require("fs");
var ESP8266 = require('ESP8266');
var freeFlash = ESP8266.getFreeFlash();
var index = 0;  // 1MB  flash area

var fname = "Hello.txt";
var content = "Hello Espruino";
var rc;

// attach flash as Fat FS
rc = E.flashFatFS({ addr:freeFlash[index].addr,
                    sectors:freeFlash[index].length/4096,
                    readonly:false,
                    format:false });

// format and write sample file 
if ( typeof(fs.readdirSync())==="undefined" ) {
  console.log("I: attach and format FS, rc="+
    E.flashFatFS({ addr:freeFlash[index].addr,
                   sectors:freeFlash[index].length/4096,
                   readonly:false,
                   format:true }));
  // write sample file
  fs.writeFileSync(fname,content);
}

// read sample file 
console.log("I: Content of File "+fname+":["+fs.readFileSync(fname)+"]");

@MaBecker
Copy link
Contributor

Adding 'FILESYSTEM' and 'FLASHFS' to ESP8266 is using to much heap space:

'libraries' : ['NET','TELNET','GRAPHICS','CRYPTO','NEOPIXEL', 'FILESYSTEM','FLASHFS']

    "cpuFrequency": 160, "freeHeap": 5040, "maxCon": 10,

I think a combination of Flash storage Module, any js type , web pages, css, js and images and module FlashEEPROM would be nice to have for systems with poor resources like the ESP8266.

I am pretty sure this works nicely for ESP32.

@wilberforce
Copy link
Member

I had a looks this again recently and the module nesting a sub classing no longer works and I'm not sure why.

I fixed the compiler warnings in the fatfs code, and made sure on the esp8266 that the 4 pages of flash are not used.

@MaBecker
Copy link
Contributor

I started to work on module I name "simpleFlashStore", hope to share soon.

Concept: Use some pages as directory and the other to store data.

@gfwilliams
Copy link
Member Author

A JS module? Obviously there's FlashEEPROM already to do very similar stuff?

@MaBecker
Copy link
Contributor

MaBecker commented Apr 22, 2017

Yes as JS module and using parts of FlashEEPROM as well plus a 4 byte fix for ESPs

So I am not reventing the wheel, but going for cherry picking.

This is what I am working on:

/* simpleFlashStore

This is a mix of

	Flash storage Module, any js type , web pages, css, js and images, by Wilberforce
	http://forum.espruino.com/conversations/283045/ 

	and

	EEPROM on Flash, by Gordon Williams
	http://www.espruino.com/FlashEEPROM

Load Module

	sfs = new (require('simpleFlashStore')) ({addr:a, length:l , pageSize:c,rwsize:d})

		addr:  		address of flash page to use
	
		length: 	number of bytes as a multible of 4096
	
		pageSize:	4096, 2048, 1024 bytes

		rwsize: 	number of bytes to read/write with a singel call
				every call will be adjusted to this size 
				eg: want to 
			            write 3 bytes -> writes 4 bytes
			            read 10 bytes -> read 12 bytes

        sfs.write(item,data)   write item to  item section and data to data section
        sfs.read(item)         find item and return
        sfs.list()             list all items
        sfs.clear()            clear whole flash area
    .....

Item section:

    first page used for identifier and item directory
        16  bytes   0 - 15    "simpleFlashStore"
        52 bytes   16 - 67  "{name:'01234567890123456789',length:99,type:'json','text'}
        next items ...

Data section:

    all other pages for data 

*/

@hungryforcodes
Copy link

So with the 4 pages of flash disabled does that mean the FatFS stuff is effectively disabled on the ESP8266?

@wilberforce
Copy link
Member

wilberforce commented Apr 23, 2017

So with the 4 pages of flash disabled does that mean the FatFS stuff is effectively disabled on the ESP8266?

The last 4 pages of flash 4x4096 are used by the esp no-os to store wifi data. If This was used for the flashfs, it would wipe over this area and there would be issues for both wifi and the data! So This was protection to ensure if the dfualts are used the areas did not overlap.

FlashFs is not enabled on any of the default builds, as it look about 4.5K of heap, of a device that already had low heap so @MaBecker though it was better to leave this off the new 4Mb build.

You can still compile it in yourself as you have been and it will work fine.

@hungryforcodes
Copy link

@wilberforce OK thanks for clarifying about the flash pages! :) Yes I understand about the heap space, too bad -- but I've been using it successfully in my builds -- sometimes with as little as 2.5K of heap space. I'll continue to use it as I have been then. :)

@wilberforce
Copy link
Member

Each time a network buffer is required on the esp8266 I think it allocates 620 bytes on the heap - so you might need more headroom

@hungryforcodes
Copy link

@wilberforce Thats funny -- I started noticing on my builds that wifi.save() was not saving. So that makes sense now. I'll update my local copy from the latest on github. I see your point about the network buffer, 620 bytes is alot (I assume per connection). Thanks for the heads up, I'll play maybe with the JsVars to see what I can do.

@wilberforce
Copy link
Member

I think we close now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ESP32 This is only a problem on ESP32-based devices ESP8266 This is only a problem on ESP8266 devices implemented
Projects
None yet
Development

No branches or pull requests

4 participants