mbed-cloud-client-example icon indicating copy to clipboard operation
mbed-cloud-client-example copied to clipboard

Fix compile error for Mbed TF-M V8M target

Open ccli8 opened this issue 3 years ago • 8 comments

[x] I confirm this contribution is my own and I agree to license it with Apache 2.0. [x] I confirm the moderators may change the PR before merging it in. [x] I understand the release model prohibits detailed Git history and my contribution will be recorded to the list at the bottom of CONTRIBUTING.md.

Summary of changes

Use DEVICE_FLASH to exclude KVStore/FLASHIAP code for Mbed TF-M V8M target, usually without FLASHIAP.

ccli8 avatar Jul 08 '21 01:07 ccli8

Have a look?

ccli8 avatar Jul 22 '21 06:07 ccli8

Thank you, I'll take a look.

marcuschangarm avatar Jul 23 '21 15:07 marcuschangarm

@ccli8 Can you share the compile errors you are seeing, please? Even without FlashIAP, the function should still be available: https://github.com/ARMmbed/mbed-os/blob/master/storage/kvstore/kv_config/source/kv_config.cpp#L1002-L1004

marcuschangarm avatar Jul 28 '21 19:07 marcuschangarm

@marcuschangarm Oh, it is runtime error, not compile error. As you pointed out, kv_init_storage_config() will return MBED_ERROR_UNSUPPORTED when FLASHIAP is not supported.

https://github.com/PelionIoT/mbed-cloud-client-example/blob/3d8d83b876786ee00f083aa9097f167cf328bf3b/source/platform/mbed-os/mcc_common_setup.cpp#L415-L419

ccli8 avatar Jul 29 '21 01:07 ccli8

@ccli8 Would this work for you instead?

    int status = kv_init_storage_config();
    if (status != MBED_SUCCESS) {
#ifdef MBED_CONF_MBED_CLOUD_CLIENT_PSA_SUPPORT
        if (status == MBED_ERROR_UNSUPPORTED) {
            printf("PSA enabled, ignore kv_init_storage_config() is not supported\n");
        } else
#endif
        {
            printf("kv_init_storage_config() - failed, status %d\n", status);
            return status;
        }
    }

We support KV store on other mediums than internal flash, so using DEVICE_FLASH is too restrictive.

marcuschangarm avatar Jul 30 '21 04:07 marcuschangarm

@marcuschangarm The above patch can work for me.

ccli8 avatar Jul 30 '21 06:07 ccli8

@marcuschangarm Need I update this PR to include above modification, or you'll merge it separately?

ccli8 avatar Aug 05 '21 06:08 ccli8

@ccli8 Sorry, I can see my message was ambiguous. I've made the changes to our internal repository and it should be part of one of the next releases. I'll close this PR once everything is ready to go.

marcuschangarm avatar Aug 05 '21 17:08 marcuschangarm