PelionIoT / mbed-cloud-client-example

Reference example application using Izuma Device Management Client library
https://izumanetworks.com
Apache License 2.0
30 stars 97 forks source link

Fix compile error for Mbed TF-M V8M target #81

Open ccli8 opened 3 years ago

ccli8 commented 3 years ago

[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 commented 3 years ago

Have a look?

marcuschangarm commented 3 years ago

Thank you, I'll take a look.

marcuschangarm commented 3 years ago

@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

ccli8 commented 3 years ago

@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

marcuschangarm commented 3 years ago

@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.

ccli8 commented 3 years ago

@marcuschangarm The above patch can work for me.

ccli8 commented 3 years ago

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

marcuschangarm commented 3 years ago

@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.