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

Add static to NRF5 SDK11 macro definition #4353

Merged
merged 1 commit into from May 19, 2017

Conversation

Projects
None yet
6 participants
@theotherjimmy
Contributor

theotherjimmy commented May 19, 2017

This is blocking CI

Description

After ARMmbed/mbed-os-example-ble#80 was merged, the Austin Example build CI correctly started failing for NRF51_DK and NRF52_DK with IAR with a mulitple symbol definition:

Error[Li006]: duplicate definitions for "fs_config"; in "Z:\home\jimbri01\src\a
          rmmbed\mbed-os-example-ble\BLE_EddystoneService\BUILD\NRF51_DK\iar\mb
          ed-os\targets\TARGET_NORDIC\TARGET_NRF5\TARGET_SDK11\libraries\fds\fd
          s.o"
          , and "Z:\home\jimbri01\src\armmbed\mbed-os-example-ble\BLE_Eddystone
          Service\BUILD\NRF51_DK\iar\source\PersistentStorageHelper\nrfPersiste
          ntStorageHelper\nrfConfigParamsPersistence.o"
[ERROR] Error[Li006]: duplicate definitions for "fs_config"; in "Z:\home\jimbri01\src\a
          rmmbed\mbed-os-example-ble\BLE_EddystoneService\BUILD\NRF51_DK\iar\mb
          ed-os\targets\TARGET_NORDIC\TARGET_NRF5\TARGET_SDK11\libraries\fds\fd
          s.o"
          , and "Z:\home\jimbri01\src\armmbed\mbed-os-example-ble\BLE_Eddystone
          Service\BUILD\NRF51_DK\iar\source\PersistentStorageHelper\nrfPersiste
          ntStorageHelper\nrfConfigParamsPersistence.o"

This PR adds static to the NRF_SECTION_VARS_ADD macro for IAR because it's present on the other 2 compilers.

Status

READY

Todos

Steps to test or reproduce

Try building the BLE_EddystoneService with the latest mbed-os master.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented May 19, 2017

@studavekar

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented May 19, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 280

All builds and test passed!

@theotherjimmy theotherjimmy merged commit 0ca9a6b into ARMmbed:master May 19, 2017

4 checks passed

Cam-CI uvisor Build & Test Success
Details
ci/morph-test Job has completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@sg- sg- removed the needs: CI label May 19, 2017

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented May 19, 2017

@sg- @0xc0170 @adbridge Sorry for merging my own PR. It was a small fix that is blocking all test commands.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented May 19, 2017

@adbridge You will want to re-create the Release with this patch included to pass nightlys

@pan-

This comment has been minimized.

Member

pan- commented May 22, 2017

@theotherjimmy Could you detail the failure, compilation of all examples for this PR succeeded with master prior to your patch: https://jenkins-internal.mbed.com/job/BLE/view/TRIGGER/job/ble-examples-trigger/4/

It was unnoticed by the CI job running BLE because IAR are deactivated atm for PRs due to an internal issue. Apologies for the inconvenient time, fortunately the CI issue will be resolve soon and IAR support will be covered in PR builds, not just nightly builds.

@pan-

This comment has been minimized.

Member

pan- commented May 22, 2017

@nvlsianpu NRF_SECTION_VARS_REGISTER_VAR is supposed to declare and attach a variable in a section. Is there any reason to force the storage class specifier to static on GCC and ARMCC and not on IAR ?

@frkv

This comment has been minimized.

frkv commented May 22, 2017

The IAR version should also have the static storage class specifier. This will be fixed in upcoming versions of the nRF5 SDKs.

@nvlsianpu

This comment has been minimized.

Contributor

nvlsianpu commented May 23, 2017

As Frank respond: looks fine for use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment