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

NVStore: key management enhancements #6388

Merged
merged 1 commit into from Mar 23, 2018

Conversation

Projects
None yet
6 participants
@davidsaada
Contributor

davidsaada commented Mar 19, 2018

Description

This PR makes the following changes in NVStore:

  • Define the nvstore_predefined_keys_e enum for predefined keys (later filled by internal users of NVStore)
  • Add the set_alloc_key API, allocating a free key from the non predefined keys

Both changes are required for better handling of the NVStore key management.

Pull request type

  • Feature
NVStore: key management enhancements
- Define an enum for predefined keys (later filled by internal users of NVStore)
- Add the set_alloc_key API, allocating a free key from the non predefined keys

@davidsaada davidsaada force-pushed the davidsaada:david_nvstore_set_alloc_key branch from d8ff3d9 to b7bb29a Mar 19, 2018

@davidsaada

This comment has been minimized.

Contributor

davidsaada commented Mar 19, 2018

Seems like Jenkins failure is not related to the PR, but more to Ethernet issues. Setup problem? Known issue?

@geky

This comment has been minimized.

Member

geky commented Mar 19, 2018

Unfortunately the "continuous-integration/jenkins/pr-head" CI is unreliable. Usually it's not the PR's fault.

@geky

geky approved these changes Mar 19, 2018

Looks good to me 👍

* @param[in] buf_size Buffer size (bytes).
* @param[in] buf Input Buffer.
* @param[in] flags Record flags.
*
* @returns 0 for success, nonzero for failure.
*/
int do_set(uint16_t key, uint16_t buf_size, const void *buf, uint16_t flags);
int do_set(uint16_t &key, uint16_t buf_size, const void *buf, uint16_t flags);

This comment has been minimized.

@geky

geky Mar 19, 2018

Member

Have you considered exposing do_set as a public overload of the set function? Letting the user chose flags would gives them more options.

This comment has been minimized.

@davidsaada

davidsaada Mar 19, 2018

Contributor

Don't think it's the way to go here. Aim is to keep the API as simple as possible (as well as keeping the user on a short leash).

@cmonr

This comment has been minimized.

Contributor

cmonr commented Mar 21, 2018

@geky Looks like it passed this time.

/morph build

@cmonr cmonr added needs: CI and removed needs: review labels Mar 21, 2018

@mbed-ci

This comment has been minimized.

mbed-ci commented Mar 21, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Mar 21, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Mar 21, 2018

Build : SUCCESS

Build number : 1508
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6388/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@0xc0170 0xc0170 added needs: review and removed needs: CI labels Mar 21, 2018

@0xc0170 0xc0170 requested a review from SenRamakri Mar 21, 2018

@cmonr cmonr added ready for merge and removed needs: review labels Mar 22, 2018

@cmonr cmonr merged commit 6dc0c9d into ARMmbed:master Mar 23, 2018

11 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Local events testing has passed
Details
travis-ci/littlefs Passed, code size is 10060B
Details
travis-ci/tools Local tools testing has passed
Details

@cmonr cmonr removed the ready for merge label Mar 23, 2018

@adbridge adbridge referenced this pull request Apr 20, 2018

Merged

Fix typo with NVStore #6595

@davidsaada davidsaada deleted the davidsaada:david_nvstore_set_alloc_key branch Jul 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment