Mbed-os: FlashIAPBlockDevice init failing if using default constructor

[tommikas]

Description

Target: At least NRF52_DK
Toolchain: GCC_ARM
Build tools: mbed-cli
Mbed OS revision: 5.10.2 and newer

At least on NRF52_DK the default base address (defined here) causes init to fail.

Issue request type

[ ] Question
[ ] Enhancement
[x] Bug

[0xc0170]

cc @ARMmbed/mbed-os-storage

[dannybenor]

@tommikas I do not see any component defined in targets.json. Are you adding FlashIAP in your mbed_app.json?

[davidsaada]

@tommikas FlashIAP block device is a special case where no default constructor can work for all boards. Reason is that flash addresses are different, block device can overrun the code and/or internal storage, and all in all much care should be taken when initializing this block device. Thus calling the default constructor without considering all the implications can be destructive - one can erase the code, the storage, configuration etc. Therefore an init failure is probably the best scenario. Maybe a better documentation or a better explained error can make things clearer, but all in all - this is the expected outcome. In addition, default constructor was kept for backward compatibility sake, as changing it broke few example codes.

[dannybenor]

@tommikas why not using the KVStore static API that will provide an automatic address?

[tommikas]

@tommikas I do not see any component defined in targets.json. Are you adding FlashIAP in your mbed_app.json?

Yep.

@tommikas FlashIAP block device is a special case where no default constructor can work for all boards. Reason is that flash addresses are different, block device can overrun the code and/or internal storage, and all in all much care should be taken when initializing this block device. Thus calling the default constructor without considering all the implications can be destructive - one can erase the code, the storage, configuration etc. Therefore an init failure is probably the best scenario. Maybe a better documentation or a better explained error can make things clearer, but all in all - this is the expected outcome. In addition, default constructor was kept for backward compatibility sake, as changing it broke few example codes.

I'm fine with all that, as long as it is indeed documented clearly and the failure is explicit. Fixing it now that I know what the problem is is trivial, but figuring out what exactly was wrong took too much work.

In addition, default constructor was kept for backward compatibility sake, as changing it broke few example codes.

As a user I'd prefer it breaking at compile time rather than run time. I understand not wanting to break the API, but that doesn't help much if the functionality breaks.

@tommikas why not using the KVStore static API that will provide an automatic address?

Defining the flash address and size statically for the targets I want to support isn't a problem.

[ciarmcom]

Internal Jira reference: https://jira.arm.com/browse/MBOCUSTRIA-787

[dannybenor]

@tommikas thanks for pointing this. We will fix it by removing the default values from the json file. This will cause compilation error if you do not provide addresses to the constructor or add them to the json file

[davidsaada]

PR created to resolve this issue. Note that it can't be resolved in compilation time, as the compiler can't tell whether the default constructor is used or not. Added a run time assertion that points out the error - better than before, where it just didn't work.