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

nvs: support iteration over namespace by index (IDFGH-9782) #11118

Closed
wants to merge 1 commit into from

Conversation

codysch
Copy link
Contributor

@codysch codysch commented Apr 3, 2023

Users of the nvs API are likely to have nvs_handle_t in all cases, but not all of them carry around the partition name and namespace name (as they aren't needed after creating an nvs_handle_t).

Allow this common case to use nvs iteration without them tracking additional data by using the nvs_handle_t to locate the namespace index and the partition name by introducing an alternative to nvs_entry_find called nvs_entry_find_in_handle, which operates similarly except that it is given a nvs_handle_t instead of partition and namespace strings.

This is somewhat more limited than the nvs_entry_find API as one cannot examine all keys in a given partition.

@CLAassistant
Copy link

CLAassistant commented Apr 3, 2023

CLA assistant check
All committers have signed the CLA.

@espressif-bot espressif-bot added the Status: Opened Issue is new label Apr 3, 2023
@github-actions github-actions bot changed the title nvs: support iteration over namespace by index nvs: support iteration over namespace by index (IDFGH-9782) Apr 3, 2023
@Alvin1Zhang
Copy link
Collaborator

Thanks for your contribution.

Users of the nvs API are likely to have `nvs_handle_t` in all cases, but
not all of them carry around the partition name and namespace name (as
they aren't needed after creating an `nvs_handle_t`).

Allow this common case to use nvs iteration without them tracking
additional data by using the `nvs_handle_t` to locate the namespace
index and the partition name by introducing an alterate to
`nvs_entry_find` called `nvs_entry_find_in_handle`, which operates
similarly except that it is given a `nvs_handle_t` instead of partition
and namespace strings.

This is somewhat more limited than the `nvs_entry_find` API as one
cannot examine all keys in a given partition.
@igrr igrr added the PR-Sync-Merge Pull request sync as merge commit label Apr 12, 2023
@igrr
Copy link
Member

igrr commented Apr 12, 2023

sha=2850cc67b8ace5051b7c4b7f0a255c30eb6e8eb5

@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Apr 12, 2023
* nvs_release_iterator(it);
* \endcode
*
* @param[in] handle Handle obtained from nvs_open function.
Copy link
Member

Choose a reason for hiding this comment

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

Over here Doxygen complains that the argument name (handle) doesn't match the argument name in function declaration (c_handle). I'll fix this to use handle name in the declaration, consistent with the rest of the functions.

Copy link

Choose a reason for hiding this comment

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

Ah, whoops, ya. I originally wrote this change for the esp-idf 4.x and then ported it to 5.x. Seems I didn't fully fix the doxygen comments after that.

@espressif-bot espressif-bot added Status: Reviewing Issue is being reviewed and removed Status: In Progress Work is in progress labels Apr 17, 2023
@espressif-bot espressif-bot assigned rrtandler and unassigned igrr Aug 29, 2023
@espressif-bot espressif-bot added Status: Done Issue is done internally Resolution: NA Issue resolution is unavailable and removed Status: Reviewing Issue is being reviewed labels Aug 31, 2023
@igrr
Copy link
Member

igrr commented Oct 2, 2023

Cherry-picked in 352e759, thanks for the contribution @codysch!

@igrr igrr closed this Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR-Sync-Merge Pull request sync as merge commit Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants