Skip to content

adapted esp-idf spiffs initialization#218

Merged
matth-x merged 3 commits intomatth-x:masterfrom
OlliLenz:master
Oct 19, 2023
Merged

adapted esp-idf spiffs initialization#218
matth-x merged 3 commits intomatth-x:masterfrom
OlliLenz:master

Conversation

@OlliLenz
Copy link
Contributor

Hi @matth-x,

If you use more than one spiffs partition in your project, it makes sense to adjust the .base_path of the partitions so that it is not just "/".
In the current version, however, this unfortunately leads to problems when using the esp-idf:

if you set the base_path to "/mo/", for example, you get an invalid argument error when initialising the spiffs partition, as the base path must not contain a slash at the end.
if you set the base_path to "/mo", the initialisation works, but the file names are no longer correct because they do not begin with a "/". An attempt is then made to save /mobootstats.jsn, for example, which does not work.
For this reason, a check has been introduced which, when setting the base_path, checks whether a "/" is present at the end of the define MOCPP_FILENAME_PREFIX. If this is the case, it is only removed for the base_path. Thus, the file names in the further course are also correct.

In addition, the define MOCPP_PARTITION_LABEL was introduced. The change from the former "ao" to "mo" could lead to problems in a production environment if devices are in circulation that have partitions with the name "ao", as these cannot or should not be changed OTA without further ado.

…fs, so we can actually use a base path like "/ao/" without breaking initialization
…leared by the logic and an empty string would be used for initialization
@matth-x matth-x merged commit 47c50dc into matth-x:master Oct 19, 2023
@matth-x
Copy link
Owner

matth-x commented Oct 19, 2023

Hi @OlliLenz, thanks for this contribution and getting more involved!

True, this is a severe defect. On the develop branch I already made a possible fix, but it doesn't allow "/" as the filename prefix, so still a drawback.

In the next few days I will merge the develop branch (and make this the v1.0 release finally), but likely will adopt a third approach of trimming the trailing / which is already used here to get the directory name from the filename prefix:

char dname [MOCPP_MAX_PATH_SIZE];
auto dlen = snprintf(dname, MOCPP_MAX_PATH_SIZE, "%s", MOCPP_FILENAME_PREFIX);
if (dlen < 0 || dlen >= MOCPP_MAX_PATH_SIZE) {
MOCPP_DBG_ERR("fn error: %i", dlen);
return -1;
}
// trim trailing '/' if not root directory
if (dlen >= 2 && dname[dlen - 1] == '/') {
dname[dlen - 1] = '\0';
}

The problem of strdup here is that it allocates a string on the heap which won't be released later. Anyway the SPIFFS integration becomes functional with this fix and the MOCPP_PARTITION_LABEL is a good idea. Thanks!

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.

2 participants