Pass in filesystem rather than requiring SPIFFS (Fixes #74)

[thorrak]

This PR is a breaking change to the library, introducing a new filesystem variable in the esp32FOTA constructor. This is intended to be used similar to the following:

Spiffs Example:

#include <FS.h>
#include <SPIFFS.h>
#include <esp32fota.h>

esp32FOTA esp32FOTA("esp32-fota-http", 1, SPIFFS, false);

LittleFS Example (Arduino Core 1):

#include <FS.h>
#include <LITTLEFS.h>
#include <esp32fota.h>

esp32FOTA esp32FOTA("esp32-fota-http", 1, LITTLEFS, false);

Users can update their code to use this version of the library by simply inserting their filesystem after the version number (SPIFFS or LIITTLEFS in the above)

This filesystem variable allows for the user to pass in the actual filesystem that he/she is using rather than having it assumed that he/she is using SPIFFS. This format ensures that the user can use SPIFFS, LittleFS (Arduino Core 2), or LITTLEFS (Arduino Core 1) without additional changes, and also potentially enables the use of SD cards/FATfs if additional changes were made to allow selecting the storage location.

Unfortunately, the way that ESP32 implements the filesystem drivers mean that attempting to use an incompatible driver can result in the filesystem being wiped out. This change ensures that this will never occur by having the user select the filesystem rather than defaulting to SPIFFS.

This implementation follows the way that similar features are implemented in other core libraries which are filesystem-agnostic such as ESPAsyncWebServer.

[chrisjoyce911]

I think this could be the change that set the version to 1.0.0 ?

Did you want to include this in patch .

[thorrak]

Sure thing. I think I caught them all in the commit I just pushed.

[chrisjoyce911]

Looks like a small conflict !

[thorrak]

Sorry about that - should be fine now!

[thorrak]

Since this was introducing one breaking change, I just changed the constructors slightly in order to introduce another.

Previously, the most basic form of the constructor allowed for defaulting of the validate (and from my earlier PR, allow_insecure_https) flags: esp32FOTA(String firwmareType, int firwmareVersion, const fs::FS& fs, boolean validate = false, boolean allow_insecure_https = false );

This change removes the defaulting of those flags, requiring the user to specify their value. This change also adds an optional last characteristic url which prevents the need to set checkURL separately later: esp32FOTA(String firwmareType, int firwmareVersion, const fs::FS& fs, boolean validate, boolean allow_insecure_https, String url);

If you'd prefer to not merge these changes, let me know and I can strip them out -- just figured it was a good opportunity to simplify things a bit!

[thinksilicon]

Do you think it woiuld make more sense not to pass the Filesystem as variable but rather use a Macro?

#define ESPFS SPIFFS

So in your code you'd reference ESPFS and then you can define whichever FS you'd use?

Could even do a switch in your header file:

#ifdef ESP32
    #define ESPFS SPIFFS
    #include <SPIFFS.h>
#elif ESP8266
    #define ESPFS LittleFS
    #include <LittleFS.h>
#endif

And then just use like

File f = ESPFS.open( "file", "r" );

[thorrak]

Do you think it woiuld make more sense not to pass the Filesystem as variable but rather use a Macro?

That's how I've done it in other projects, but then you end up with a million libraries all competing to get different macros set, and it is still a breaking change - unless you default the SPIFFS.

The problem with defaulting to SPIFFS is that it will wipe your filesystem if you happen to be using LittleFS.

This solution is the same as used by other filesystem-agnostic libraries such as ESPAsyncWebServer.

[oseiler2]

Hi @chrisjoyce911, it would be nice to get this PR merged so that we can use LittleFS. Can I help with this?

[tobozo]

bump

Please have a look at #92, it implements agnostic filesystem handling along with progmem for crypto assets storage while freeing the main class from any hardcoded values.

This fork also implements the update capability for spiffs/littlefs partitions.
However, unlike what OTA0/OTA1 offer, no rollback is possible for spiffs/littlefs partition types, so make sure any deploy scenario using this feature does not use spiffs/littlefs to store certificates or signatures as it can eventually get corrupted.

The recommended usage when updating both firmware and spiffs/littlefs partition is to store the certificate in progmem or SD.

There's an open thread if any of you's want to discuss this suggest new constructors, create a config struct, etc.

[chrisjoyce911]

I've added you as a Collaborator to allow for faster merges , at the moment I'm unable to do any affective testing due to my current physical location.

Looking over the code it seems ok .

[tobozo]

@chrisjoyce911 thanks!

it's a bit soon to merge as the signature part is still untested and some new logical situations need to be solved

1. When using JSON manifest to list several versions of the same firmware type:

• Current behaviour is that esp32fota flashes the "next" version as listed in the JSON file.
• Possible optional behaviour: esp32fota jumps to the "highest" version listed in the JSON file

2. Decision tree when spiffs/littlefs signature validation fails (happens after the partition is flashed):

• Leave as is (creates a "dirty" situation where security is compromised)
• Erase partition (will need formatting upon next reboot)

3. Test suite strategy:

Although it's been convenient to host the test-suite binaries in the source tree while implementing new features, it's obviously an unnecessary weight for the package.

However the process of building all binaries and the json manifest for the test suite still needs to exist for the maintainers, and it would be much better automated than manually set in a trial/error fashion :-)

I'm not sure yet how, but a workflow can probably solve that.

[chrisjoyce911]

On 13 Sep 2022, at 9:13 pm, tobozo ***@***.******@***.***>> wrote: @chrisjoyce911<https://github.com/chrisjoyce911> thanks! it's a bit soon to merge as the signature part is still untested and some new logical situations need to be solved 1. When using JSON manifest to list several versions of the same firmware type: * Current behaviour is that esp32fota flashes the "next" version as listed in the JSON file. * Possible optional behaviour: esp32fota jumps to the "highest" version listed in the JSON file I’m a fan of stepping forward one version at a time , but have a flag to use latest is also a nice option. Just need to have a rollback option .. or ’safe’ version as fallback 1. Decision tree when spiffs/littlefs signature validation fails (happens after the partition is flashed): * Leave as is (creates a "dirty" situation where security is compromised) * Erase partition (will need formatting upon next reboot) 1. Test suite strategy: Although it's been convenient to host the test-suite binaries in the source tree while implementing new features, it's obviously an unnecessary weight for the package. However the process of building all binaries and the json manifest for the test suite still needs to exist for the maintainers, and it would be much better automated than manually set in a trial/error fashion :-) I'm not sure yet how, but a workflow can probably solve that. Having a way to test is important, I’m not sure of the best way at the moment. Its interesting how a small project to allow me to update a swimming pool water temp monitor has grown so much ! [image]<https://user-images.githubusercontent.com/1893754/189886553-3aa02fe6-8ad1-4e83-b8e2-6655c311f670.png> — Reply to this email directly, view it on GitHub<#79 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABHPDCS33LGW2M4FIKVQAF3V6BOVVANCNFSM5M3GLWPQ>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[tobozo]

Closing this as implemented in 0.2.0.