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

KVStore: Support external storage out of mbed-os tree #10355

Merged
merged 1 commit into from May 7, 2019

Conversation

Projects
None yet
8 participants
@ccli8
Copy link
Contributor

commented Apr 9, 2019

Description

This PR tries to support external storage out of mbed-os tree in KVStore. To enable it, user needs to:

  1. Configure blockdevice to other.
  2. Override get_other_blockdevice() to provide block device out of mbed-os tree.

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[X] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Release issue

#10300

Release Notes

Support external storage out of mbed-os tree in KVStore. To enable it, user needs to:

  1. Configure KVStore configuration parameter blockdevice to other.
  2. Override get_other_blockdevice() to provide block device out of mbed-os tree.
    BlockDevice *get_other_blockdevice();

@ciarmcom ciarmcom requested review from ARMmbed/mbed-os-maintainers Apr 9, 2019

@ciarmcom

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

@ccli8, thank you for your changes.
@ARMmbed/mbed-os-storage @ARMmbed/mbed-os-maintainers please review.

@bulislaw

This comment has been minimized.

Copy link
Member

commented Apr 24, 2019

@davidsaada please review. Are there any docs that need to be updated?

@davidsaada
Copy link
Contributor

left a comment

Looks OK all in all. Just got one comment regarding the classification of the device as a flash one.

*
* @returns true for flash type or false for non-flash type.
*/
bool is_blockdevice_flash_type(BlockDevice *bd);

This comment has been minimized.

Copy link
@davidsaada

davidsaada Apr 24, 2019

Contributor

There's no need for this function. One can call the BD's get_erase_value method. A non flash device would return a -1 value, telling that that this block device is incapable of erasing.

This comment has been minimized.

Copy link
@ccli8

ccli8 Apr 25, 2019

Author Contributor

@davidsaada Yes. BD's get_erase_value method can help check flash type or not. I've updated it and related config description.

KVStore: Support block device out of mbed-os tree
To support block device out of mbed-os tree in KVStore, user needs to:
1. Configure blockdevice to "other".
2. Override get_other_blockdevice() to provide block device out of mbed-os tree.

@ccli8 ccli8 force-pushed the OpenNuvoton:nuvoton_kvstore_other_bd branch from 8b885d3 to 63d9cde Apr 25, 2019

@ccli8 ccli8 changed the title Support external storage out of Mbed OS tree in KVStore KVStore: Support external storage out of mbed-os tree Apr 25, 2019

@adbridge

This comment has been minimized.

Copy link
Contributor

commented Apr 25, 2019

@davidsaada could you approve now please if you are happy with the changes ?

@davidsaada
Copy link
Contributor

left a comment

Looks good now. Thanks.

@adbridge adbridge added needs: CI and removed needs: review labels May 1, 2019

@adbridge

This comment has been minimized.

Copy link
Contributor

commented May 1, 2019

CI started

@mbed-ci

This comment has been minimized.

Copy link

commented May 1, 2019

Test run: FAILED

Summary: 2 of 11 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_exporter
  • jenkins-ci/mbed-os-ci_greentea-test
@0xc0170

This comment has been minimized.

Copy link
Member

commented May 2, 2019

CI restarted (internal license server fault)

@0xc0170

This comment has been minimized.

Copy link
Member

commented May 2, 2019

@davidsaada please review. Are there any docs that need to be updated?

@davidsaada
Is there any docs for these change that needs updating? Should this have release notes included? To illustrate how to add this to own project (using own BD implementation).

@mbed-ci

This comment has been minimized.

Copy link

commented May 2, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 3
Build artifacts

@0xc0170 0xc0170 added needs: review and removed needs: CI labels May 2, 2019

@davidsaada

This comment has been minimized.

Copy link
Contributor

commented May 2, 2019

@davidsaada please review. Are there any docs that need to be updated?

@davidsaada
Is there any docs for these change that needs updating? Should this have release notes included? To illustrate how to add this to own project (using own BD implementation).

Our KVStore documentation doesn't go deep as in specifying how to implement block devices. I think that adding a description in the release notes is the way to go.

@0xc0170

This comment has been minimized.

Copy link
Member

commented May 2, 2019

@ccli8 Set to 5.13 as functionality change (PR type should be fixed above). If that is correct, release notes section should be filled.

@ccli8

This comment has been minimized.

Copy link
Contributor Author

commented May 3, 2019

@0xc0170 Changed PR type to Functionality change and added Release Notes section.

@adbridge adbridge merged commit 0ac1c97 into ARMmbed:master May 7, 2019

26 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/build-ARM Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage Success
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARM Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Success!
Details
travis-ci/docs Success!
Details
travis-ci/doxy-spellcheck Success!
Details
travis-ci/events Success! Runtime is 8530 cycles.
Details
travis-ci/gitattributestest Success!
Details
travis-ci/include_check Success!
Details
travis-ci/licence_check Success!
Details
travis-ci/littlefs Success! Code size is 8448B.
Details
travis-ci/psa-autogen Success!
Details
travis-ci/tools-py2.7 Success!
Details
travis-ci/tools-py3.5 Success!
Details
travis-ci/tools-py3.6 Success!
Details
travis-ci/tools-py3.7 Success!
Details

@ccli8 ccli8 deleted the OpenNuvoton:nuvoton_kvstore_other_bd branch May 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.