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

Installable File System #1579

Merged
merged 7 commits into from
Feb 10, 2021
Merged

Installable File System #1579

merged 7 commits into from
Feb 10, 2021

Conversation

mikee47
Copy link
Contributor

@mikee47 mikee47 commented Jan 25, 2019

This is a work in progress. See Basic_IFS sample.

TODO:

  • Test/fix ESP32 build (WIP)
  • Add virtual IFileSystem::mkdir() and IFileSystem::makedirs()
  • Review framework and use host filesystem where appropriate
    * [ ] Implement fscopy utility
  • Add partition support
  • Review and tidy documentation
    * [ ] Tidy up test application

@mikee47 mikee47 changed the title Installable File System Installable File System (experimental) Jan 25, 2019
@mikee47
Copy link
Contributor Author

mikee47 commented Jan 26, 2019

@slaff This build fails because a number of submodules don't get pulled in. I've put the IFS code into a separate repo, pulled in as third-party code, which itself pulls in another submodule and that seems to be the reason it breaks - the build process stops pulling in further submodules. Any idea why this might be the case?

UPDATE: Nothing to do with submodules, bug in my make process somwhere...

Missed a '+' in Makefile. Doh!

@slaff
Copy link
Contributor

slaff commented Jan 26, 2019

UPDATE: Nothing to do with submodules, bug in my make process somwhere...

Ok, anyway I will take a look at the latest PRs later today.

@mikee47 mikee47 closed this Jan 26, 2019
@mikee47 mikee47 reopened this Jan 27, 2019
@mikee47 mikee47 force-pushed the ifs branch 2 times, most recently from 420829c to cf53191 Compare January 29, 2019 11:41
@mikee47
Copy link
Contributor Author

mikee47 commented Jan 29, 2019

@slaff travis is having a tantrum about coding style, seems there's a difference in output between clang-format versions 6.0.1 (my version) versus the travis build (version 5.0.0).

It looks to me like a bug in the earlier version. Here's an excerpt from the travis log:

!!! samples/HttpServer_IFS/app/application.cpp not compliant to coding style, here is the fix:
--- samples/HttpServer_IFS/app/application.cpp	2019-01-29 11:43:50.227521629 +0000
+++ /dev/fd/63	2019-01-29 11:43:54.935562811 +0000
@@ -185,7 +185,7 @@
 
 void init()
 {
-	// Configure serial port
+// Configure serial port
 #if DEBUG_BUILD
 	//	Serial.setPort(UART_ID_1);
 	Serial.setTxBufferSize(4096);

I could downgrade to version 5.0.0, but thought I'd check first in case you prefer to fix the travis build. I should add this is the first time this has happened - I've been using version 6.0.1 all along and never had a problem with it.

@slaff
Copy link
Contributor

slaff commented Jan 30, 2019

but thought I'd check first in case you prefer to fix the travis build

If clang-format 5 has an issue it is more logical to replace it with clang-format 6. So please submit a PR that updates to version 6 the clang-format version used in Travis.

@slaff
Copy link
Contributor

slaff commented Jan 30, 2019

So please submit a PR that updates to version 6 the clang-format version used in Travis.

@mikee47 Don't create such a PR. I have already prepared PR #1589 that is switching the version of clang-format to 6.0. Should be merged soon. Then you can rebase your PR on latest develop.

@slaff
Copy link
Contributor

slaff commented Jan 31, 2019

Should be merged soon.

Done. Finally found the time to do it. Pull the develop branch and then rebase your one on develop.

@slaff
Copy link
Contributor

slaff commented Feb 2, 2019

@mikee47 Make sure to rebase so that I can see only your changes related to this PR.

@mikee47 mikee47 force-pushed the ifs branch 2 times, most recently from 1b214c3 to 059d030 Compare February 7, 2019 22:56
@mikee47 mikee47 force-pushed the ifs branch 2 times, most recently from a9ec46c to 9243d45 Compare April 9, 2019 10:02
@slaff slaff added this to the 3.9.0 milestone Apr 9, 2019
@mikee47
Copy link
Contributor Author

mikee47 commented Apr 13, 2019

Just came across the esp VFS implementation https://github.com/espressif/esp-idf/tree/master/components/vfs

@slaff
Copy link
Contributor

slaff commented Apr 13, 2019

Just came across the esp VFS implementation https://github.com/espressif/esp-idf/tree/master/components/vfs

My dream is to make Sming run on multiple platforms. As a start (Sming version 4 of ) we could try to make it at least ESP32 compatible. Therefore it would be great if we can use similar/same VFS implementation so that the migration/adaptation effort to ESP32 is as little as possible.

@mikee47
Copy link
Contributor Author

mikee47 commented Apr 14, 2019

try to make it at least ESP32 compatible

Been thinking over this for months now. I'm aware of the attempt to migrate Sming to FreeRTOS, however I think we at least need to migrate to the RTOS SDK, though we wouldn't use any of the FreeRTOS code. It seems better maintained and its structure matches the ESP32 IDF.

We also need to sort out how we manage flash usage, making use of the flash partition management that the SDK provides.

At that point we should be in a far stronger position to look at migrating versions of components from ESP32 for the ESP8266, including VFS if appropriate.

What d'ya think?

@mikee47
Copy link
Contributor Author

mikee47 commented Apr 14, 2019

Maybe worth having a roadmap in the Wiki so we all know where things are headed?

@slaff
Copy link
Contributor

slaff commented Apr 14, 2019

however I think we at least need to migrate to the RTOS SDK, though we wouldn't use any of the FreeRTOS code.

IMHO it would be best to stay as close as possible to the current async execution model and use FreeRTOS only for ESP32 and only to run Sming applications as its main task.

We should start describing the code-restructuring needed to have multi-platform code. Ideally Sming v4 should be able to run on ESP8266, ESP32 and some portions of it should run on a Linux machine. The latter is for testing and development purposes. I already have big chunks of the network code compiling under Linux and there I can test it and analyze it better before running it on a real device. Therefore I would love to have that code in one repository instead of keeping the Linux repo in sync with the develop branch.

Maybe worth having a roadmap in the Wiki so we all know where things are headed?

Virtual file system and multi-platform support are the biggest tasks that come to my mind. A roadmap is something very nice to have once we have a rough estimation of the efforts needed to complete those tasks.

@mikee47 mikee47 changed the title [WIP] Installable File System Installable File System Feb 10, 2021
@mikee47
Copy link
Contributor Author

mikee47 commented Feb 10, 2021

@slaff Ready for review on this one.

@slaff
Copy link
Contributor

slaff commented Feb 10, 2021

Ready for review on this one.

Wonderful!

@mikee47
Copy link
Contributor Author

mikee47 commented Feb 10, 2021

Docs available shortly at https://smingdev.readthedocs.io/en/ifs/

Applications should not use SPIFFS functions directly.

SPIFFS is now built with ``SPIFFS_OBJ_META_LEN=16`` to store extended attribute information,
but existing volumes should still be readable.
Copy link
Contributor

Choose a reason for hiding this comment

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

Quote from https://github.com/pellepl/spiffs/blob/9d12e8f47b0e611f277a0de8d39dc9089971ce20/src/default/spiffs_config.h#L155

// Setting to non-zero value enables metadata-related API but also
// changes the on-disk format, so the change is not backward-compatible.

If I understand this correctly all existing spiffs that we have up to now will not be readable any more. @mikee47 Can you check if this is the case? If yes - we should update the documentation and also the upgrade document.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested this with the Basic_IFS sample for Host since it gives a detailed file listing, replacing the call to initFileSystem() with a spiffs_mount(). After a make flash I then built the HttpServer_ConfigNetwork sample and flashed the resulting spiff_rom.bin. This is what we get:

#     Name                             Modified               Size       Size (KB)  Orig Size  Attributes Access
----- -------------------------------- ---------------------- ---------- ---------- ---------- ---------- ----------
0     core.js.gz                       2021-02-10T14:58:26Z   30474      29.76      0          .....      aa
1     bootstrap-core.css.gz            2021-02-10T14:58:26Z   15861      15.49      0          .....      aa
2     wifi-sprites.png                 2021-02-10T14:58:26Z   1769       1.73       0          .....      aa
3     .lastModified                    2021-02-10T14:58:26Z   30         0.03       0          .....      aa
4     settings.html                    2021-02-10T14:58:26Z   2535       2.48       0          .....      aa
5     index.html                       2021-02-10T14:58:26Z   6030       5.89       0          .....      aa

I then build the same HttpServer_ConfigNetwork sample under develop branch and flash the resulting spiff_rom.bin:

#     Name                             Modified               Size       Size (KB)  Orig Size  Attributes Access
----- -------------------------------- ---------------------- ---------- ---------- ---------- ---------- ----------
0     core.js.gz                       1971-08-06T13:09:20Z   30474      29.76      0          .....      -x
1     bootstrap-core.css.gz            2040-03-06T05:54:08Z   15861      15.49      0          .....      -x
2     wifi-sprites.png                 2075-10-19T13:17:52Z   1769       1.73       0          .....      -x
3     .lastModified                    2105-07-27T16:51:12Z   30         0.03       0          CARDM      xx
4     settings.html                    2081-08-24T13:47:44Z   2535       2.48       0          .....      -x
5     index.html                       2088-01-10T18:42:08Z   6030       5.89       0          .....      -x

So the answer I think is YES, it still works, but room for improvement perhaps!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll investigate further to see if there's a reilable way to detect this. Logically I'd expect the unused area (now occupied by 16 bytes of meta) to be 0xFF, but it's not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For info, although file listing is correct file content is not. Fix coming shortly.

Copy link
Contributor

@slaff slaff left a comment

Choose a reason for hiding this comment

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

Looks good to me and I am happy that this PR is ready for prime time. But we should check if the old spiffs are still readable. If not - then we should explain this in detail before the users cry foul.

@slaff
Copy link
Contributor

slaff commented Feb 10, 2021

So the answer I think is YES, it still works, but room for improvement perhaps!

Let's merge it then :)

@slaff slaff merged commit 9b5143f into SmingHub:develop Feb 10, 2021
@slaff slaff removed the 3 - Review label Feb 10, 2021
@mikee47 mikee47 deleted the ifs branch February 10, 2021 15:05
@slaff slaff mentioned this pull request Feb 14, 2021
5 tasks
@slaff
Copy link
Contributor

slaff commented Feb 22, 2021

@mikee47 There are problems when compiling IFS for SMING_ARCH=HOST with GCC 9. The error that I get is:

Building /home/slavey/dev/esp8266.dev.box/dev/Sming/Sming/out/Host/debug/lib/clib-IFS-577940125a31051edc0fbae78576d71a.a
C+ /home/slavey/dev/esp8266.dev.box/dev/Sming/Sming/Components/IFS/src/FileDevice.cpp
C+ /home/slavey/dev/esp8266.dev.box/dev/Sming/Sming/Components/IFS/src/FileSystem.cpp
C+ /home/slavey/dev/esp8266.dev.box/dev/Sming/Sming/Components/IFS/src/Error.cpp
C+ /home/slavey/dev/esp8266.dev.box/dev/Sming/Sming/Components/IFS/src/GdbFileSystem.cpp
C+ /home/slavey/dev/esp8266.dev.box/dev/Sming/Sming/Components/IFS/src/HostUtil.cpp
In file included from /usr/include/c++/9/stdlib.h:36,
                 from /home/slavey/dev/esp8266.dev.box/dev/Sming/Sming/System/include/m_printf.h:11,
                 from /home/slavey/dev/esp8266.dev.box/dev/Sming/Sming/Wiring/FakePgmSpace.h:17,
                 from /home/slavey/dev/esp8266.dev.box/dev/Sming/Sming/System/include/debug_progmem.h:19,
                 from /home/slavey/dev/esp8266.dev.box/dev/Sming/Sming/Components/IFS/src/include/IFS/Host/../Types.h:14,
                 from /home/slavey/dev/esp8266.dev.box/dev/Sming/Sming/Components/IFS/src/include/IFS/Host/../Error.h:16,
                 from /home/slavey/dev/esp8266.dev.box/dev/Sming/Sming/Components/IFS/src/include/IFS/Host/Util.h:13,
                 from /home/slavey/dev/esp8266.dev.box/dev/Sming/Sming/Components/IFS/src/HostUtil.cpp:9:
/usr/include/c++/9/cstdlib:132:11: error: ‘::aligned_alloc’ has not been declared
  132 |   using ::aligned_alloc;
      |           ^~~~~~~~~~~~~
/home/slavey/dev/esp8266.dev.box/dev/Sming/Sming/component-wrapper.mk:169: recipe for target 'src/HostUtil.o' failed
make[2]: *** [src/HostUtil.o] Error 1
/home/slavey/dev/esp8266.dev.box/dev/Sming/Sming/project.mk:428: recipe for target 'IFS-build' failed
make[1]: *** [IFS-build] Error 2
make[1]: Leaving directory '/media/slavey/416431e8-fd46-4982-896d-5025a72dd361/slavey/dev/esp8266.dev.box/dev/Sming/samples/Basic_Blink'
/home/slavey/dev/esp8266.dev.box/dev/Sming/Sming/project.mk:22: recipe for target 'all' failed

You can reproduce the same issue also in https://www.katacoda.com/slaff/scenarios/sming-host-emulator. I guess it is related to the following undefines in HostUtil.cpp

#undef __STRICT_ANSI__
#undef _GNU_SOURCE

@mikee47
Copy link
Contributor Author

mikee47 commented Feb 22, 2021

@slaff strerror_r is a pain as it's not well standardised. It should be fine to use strerror here, which is standard. I've got some IFS changes to push so will include in that. NB. I'm using GCC 9.3 and no such issues.

@slaff
Copy link
Contributor

slaff commented Feb 22, 2021

I'm using GCC 9.3 and no such issues.

Here is my GCC. Also you can check the GCC on katacoda.

gcc -v

gcc -v 

Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/9/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none:hsa
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 9.3.0-10ubuntu2~16.04' --with-bugurl=file:///usr/share/doc/gcc-9/README.Bugs --enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++,gm2 --prefix=/usr --with-gcc-major-version-only --program-suffix=-9 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-plugin --with-system-zlib --with-target-system-zlib=auto --enable-objc-gc=auto --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none,hsa --without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 9.3.0 (Ubuntu 9.3.0-10ubuntu2~16.04) 

@mikee47
Copy link
Contributor Author

mikee47 commented Feb 22, 2021

I'm running Ubuntu 20.04 so gcc version 9.3.0 (Ubuntu 9.3.0-17ubuntu1~20.04). Clearly this is an issue with library versions so the safest solution is to get rid of those defines as you suggest in HostUtil.cpp.

@mikee47
Copy link
Contributor Author

mikee47 commented Feb 22, 2021

diff --git a/src/HostUtil.cpp b/src/HostUtil.cpp
index 6994aa0..b7a63d9 100644
--- a/src/HostUtil.cpp
+++ b/src/HostUtil.cpp
@@ -17,14 +17,9 @@
  *
  ****/
 
-#undef __STRICT_ANSI__
-#undef _GNU_SOURCE
-
 #include "include/IFS/Host/Util.h"
 #include <fcntl.h>
-
 #include <string.h>
-extern "C" int strerror_r(int, char*, size_t);
 
 #ifndef O_BINARY
 #define O_BINARY 0
@@ -61,11 +56,7 @@ int mapFlags(OpenFlags flags)
 String getErrorString(int err)
 {
 	if(Error::isSystem(err)) {
-		char buffer[256];
-		buffer[0] = '\0';
-		auto r = strerror_r(-Error::toSystem(err), buffer, sizeof(buffer));
-		(void)r;
-		return String(buffer);
+		return strerror(-Error::toSystem(err));
 	}
 
 	return IFS::Error::toString(err);

@mikee47
Copy link
Contributor Author

mikee47 commented Feb 22, 2021

Spoke too soon. Now getting strerror missing when compiling LiveDebug for esp8266. This seems to work OK on all the tool combinations I've tried. Probably missed some but time will tell:

diff --git a/src/HostUtil.cpp b/src/HostUtil.cpp
index 6994aa0..90f8867 100644
--- a/src/HostUtil.cpp
+++ b/src/HostUtil.cpp
@@ -17,14 +17,11 @@
  *
  ****/
 
-#undef __STRICT_ANSI__
-#undef _GNU_SOURCE
-
 #include "include/IFS/Host/Util.h"
 #include <fcntl.h>
 
 #include <string.h>
-extern "C" int strerror_r(int, char*, size_t);
+extern "C" char* strerror_r(int, char*, size_t);
 
 #ifndef O_BINARY
 #define O_BINARY 0

@kmihaylov
Copy link
Contributor

kmihaylov commented Apr 14, 2021

Does this mean that the gcc 7.5 compiler included in Ubuntu 18.04 will fail (as I just discovered) to compile HostUtil.cpp?

Building /opt/Sming/Sming/out/Esp8266/debug/lib/clib-IFS-577940125a31051edc0fbae78576d71a.a
C+ /opt/Sming/Sming/Components/IFS/src/HostUtil.cpp
/opt/Sming/Sming/Components/IFS/src/HostUtil.cpp:24:18: error: conflicting declaration of C function 'char* strerror_r(int, char*, size_t)'
   24 | extern "C" char* strerror_r(int, char*, size_t);
      |                  ^~~~~~~~~~
In file included from /home/opt/esp-open-sdk/xtensa-lx106-elf/xtensa-lx106-elf/include/assert.h:9,
                 from /opt/Sming/Sming/Arch/Esp8266/Components/libc/include/assert.h:1,
                 from /opt/Sming/Sming/Components/IFS/src/include/IFS/Host/../Types.h:25,
                 from /opt/Sming/Sming/Components/IFS/src/include/IFS/Host/../Error.h:31,
                 from /opt/Sming/Sming/Components/IFS/src/include/IFS/Host/Util.h:25,
                 from /opt/Sming/Sming/Components/IFS/src/HostUtil.cpp:20:
/home/opt/esp-open-sdk/xtensa-lx106-elf/xtensa-lx106-elf/include/string.h:106:5: note: previous declaration 'int strerror_r(int, char*, size_t)'
  106 | int _EXFUN(strerror_r,(int, char *, size_t))
      |     ^~~~~~
/opt/Sming/Sming/Components/IFS/src/HostUtil.cpp: In function 'String IFS::Host::getErrorString(int)':
/opt/Sming/Sming/Components/IFS/src/HostUtil.cpp:62:68: error: call of overloaded 'strerror_r(int, char [256], unsigned int)' is ambiguous
   62 |   auto r = strerror_r(-Error::toSystem(err), buffer, sizeof(buffer));
      |                                                                    ^
In file included from /home/opt/esp-open-sdk/xtensa-lx106-elf/xtensa-lx106-elf/include/assert.h:9,
                 from /opt/Sming/Sming/Arch/Esp8266/Components/libc/include/assert.h:1,
                 from /opt/Sming/Sming/Components/IFS/src/include/IFS/Host/../Types.h:25,
                 from /opt/Sming/Sming/Components/IFS/src/include/IFS/Host/../Error.h:31,
                 from /opt/Sming/Sming/Components/IFS/src/include/IFS/Host/Util.h:25,
                 from /opt/Sming/Sming/Components/IFS/src/HostUtil.cpp:20:
/home/opt/esp-open-sdk/xtensa-lx106-elf/xtensa-lx106-elf/include/string.h:106:5: note: candidate: 'int strerror_r(int, char*, size_t)'
  106 | int _EXFUN(strerror_r,(int, char *, size_t))
      |     ^~~~~~
/opt/Sming/Sming/Components/IFS/src/HostUtil.cpp:24:18: note: candidate: 'char* strerror_r(int, char*, size_t)'
   24 | extern "C" char* strerror_r(int, char*, size_t);
      |                  ^~~~~~~~~~
/opt/Sming/Sming/component-wrapper.mk:169: recipe for target 'src/HostUtil.o' failed
make[2]: *** [src/HostUtil.o] Error 1
/opt/Sming/Sming/project.mk:431: recipe for target 'IFS-build' failed
make[1]: *** [IFS-build] Error 2
make[1]: Leaving directory '/home/opt/Sming/samples/Basic_Blink'
/opt/Sming/Sming/project.mk:22: recipe for target 'all' failed
make: *** [all] Error 2

GCC9 isn't in the official Ubuntu 18.04 packages repository.

=============

Just did Sming through install.sh and it added some llvm package, now it builds successfully. But I wonder what makes it working now?

@mikee47
Copy link
Contributor Author

mikee47 commented Apr 14, 2021

@kmihaylov The strerror_r issues are largely to do with the ESP open SDK - the error messages in your log above are related to the esp-open-sdk usage, not the installed linux toolchains. The install.sh script installs the esp quick toolchain which you can see by inspecting ESP_HOME setting and doesn't have these issues.

@mikee47
Copy link
Contributor Author

mikee47 commented Apr 14, 2021

@kmihaylov Also Ubuntu 18.04 has GCC 8.4 which is fine. You can install it thus:

sudo apt install g++-8-multilib
sudo update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-8 60 --slave /usr/bin/g++ g++ /usr/bin/g++-8

@kmihaylov
Copy link
Contributor

@mikee47 I install manually esp-quick-toolchain, tried with gnu20 and gnu23. Probably I missed something in the paths, although I checked env.

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

Successfully merging this pull request may close these issues.

3 participants