Refactoring and fixing some issues in KVStore configuration. #9237
Conversation
…efault values are use remove of rbp_number_of_entries from the kvstore configuration. Adding default option for storage_type allowing the system to choose TDB_INTERNAl, TDB_EXTERNAL or FILESYSTEM base on the blockdevice component set in the target board. Adding remarks to kv_config.cpp and break simplify the _get_blockdevice_FLASHIAP function
|
@yossi2le, thank you for your changes. |
|
@ChiefBureaucraticOfficer as this is requesting a breaking change for a patch release, it needs your specific approval |
|
In Mbed OS we have policy of not breaking compatibility. Why does it need to break it? Is it to fix critical bug? |
|
@bulislaw The breaking change is in the configuration. The original setup included too much freedom and complexity, in a way that the PDMC and bootloader were not able to cope with. |
There was a problem hiding this comment.
I don't actually think it's a breaking change. Looks good, except the docs should be reworded as they are confusing.
@dannybenor please approve as TL for storage.
| @@ -62,6 +62,8 @@ The following is a list of all storage parameters available and their descriptio | |||
| * `TDB_EXTERNAL_NO_RBP`. | |||
| * `FILESYSTEM`. | |||
| * `FILESYSTEM_NO_RBP`. | |||
| * `default` | |||
| If default is set, the system will choose the type of storage base on the block device component set in target.json. For QSPIF, SPIF and DATAFLASH block devices TDB_EXTERNAL will be used. If SD is set a FILESYSTEM storage is set and if only FLASHIAP exists as the only block device component, a TDB_INTERNAL will be used. | |||
There was a problem hiding this comment.
@AnotherButler this addition to docs is confusing, could you help us reword it a bit to make it clearer?
There was a problem hiding this comment.
As Amanda is not available this week, can this be addressed separately (@yossi2le can you make sure it will be updated )
There was a problem hiding this comment.
No problem, I will take for that next week,
|
CI started |
Test run: FAILEDSummary: 2 of 7 test jobs failed Failed test jobs:
|
|
We are experiencing CI node failures, few jobs were aborted. We are investigating, will restart the jobs once fixed. cc @ARMmbed/mbed-os-test |
|
Ci restarted |
Test run: FAILEDSummary: 1 of 7 test jobs failed Failed test jobs:
|
|
Please review the build failure (kv_config.cpp does not compile for one target) |
… component is defined.
|
Found the issue. a return statement was missing in _storage_config_default when no component is defined. |
|
CI restarted |
Test run: SUCCESSSummary: 11 of 11 test jobs passed |
Description
In this PR the kv_confoig is refactored by breaking some function, adding lots of remarks and fixing some bugs. Also, some changes in the config parameters are done. below is the list of changes:
Adding default option to storage_type parameter which enables the system to set the best kvstore storage_type which fits the type of default block device enabled in target.json.
The parameter of rbp_num_of_enteries has been remove from TDB_EXTERNAl and FILESYSTEM configuration files because its created confusion with the rbp_internal_size parameter and was not needed.
The function _get_blockdevice_FLASHIAP in kv_config.cpp has been refactored into 3 function. the first function will calculate the default start address and size for internal TDBStore in case of rbp when KVStore is configured as TDB_EXTERNAL or FILESYSTEM. a second function will calculate the default start address and size for the case we run in TDB_INTERNAL. and the last is _get_blockdevice_FLASHIAP which will check the validity of parameters and create the block device.
Fixing a bug in which FAT filesystem has been failed to mount after reset.
This PR is needed for mbed-os 5.11.2
Pull request type
Reviewers