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

adapted esp-idf spiffs initialization #218

Merged
merged 3 commits into from
Oct 19, 2023
Merged

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
5 checks passed
@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