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

sysparam: split the key into a name-space and name. #638

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ourairquality
Copy link
Contributor

This is largely to aid compatibility with other non-volatile stores that have limited sized names but support a name space. This might open a path to more compact storage of the key names although that is not implemented here.

Have been exploring how to also target code the esp-idf, and that requires a compatibility layer that works with the esp-idf nvs. The nvs has a default key names size of 15 characters, but supports name spaces so in total 30 characters. I found it convenient to keep longer key names in my app, so that the names are more obviously meaningful, and moving common prefixes to namespaces helped. For example all the wificfg keys are now in the 'wificfg' namespace, and the prefix has been removed from the name.

People might not like this PR, might argue that it was simpler the way it was. The use of the namespace is optional, it can be passed in as NULL, and existing names still read back.

One positive might be that it sets the stage for storing the name space names only once in the store, and referencing them by a shorter index. I did explore this, and made some progress, but decide it was not worth the added complexity.

If someone wanted to write a lighter weight 'nvs' for the esp8266 then that might just be adopted instead, but I am not holding my breath!

This is largely to aid compatibility with other non-volatile stores
that have limited sized names but support a name space. This might
open a path to more compact storage of the key names although that is
not implemented here.
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.

1 participant