Skip to content
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

Baremetal: Fix ARM toolchain Greentea test compilation for NUCLEO_F411RE #11875

Merged
merged 1 commit into from
Nov 21, 2019

Conversation

rajkan01
Copy link
Contributor

@rajkan01 rajkan01 commented Nov 15, 2019

Description (required)

Removed the nanostack lib service and in mbed-trace with fea-ipv6 to false to remove dependency issue on greentea test for bare metal with ARM toolchain.

Summary of change (What the change is for and why)

The Greentea tests were not compiling for the Nucleo F411RE target with the bare metal profile when using the ARM toolchain.
The build failure was due to a missing symbol from a library not included in the bare metal profile (nanostack-hal-mbed-cmsis-rtos). This only occurs for the ARM toolchain because it attempts to resolve all symbols even if not executed (the GCC_ARM toolchain does not attempt to resolve symbols not executed and therefore compiles fine). In order to solve the build issue, the dependency on the missing symbol is removed by not including the library (nanostack-libservice) that was bringing it in and compiling out sections of the code (mbed_trace) that was relying on it by setting a macro (MBED_CONF_MBED_TRACE_FEA_IPV6) to 0 via the configuration file.

Documentation (Details of any document updates required)

Pull request type (required)

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results (required)

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers (optional)

@evedon @hugueskamba @jamesbeyond


Release Notes (required for feature/major PRs)

Summary of changes
Impact of changes
Migration actions required

@rajkan01 rajkan01 force-pushed the feature_arm_greentea_test branch from 1f55ded to 1055894 Compare November 15, 2019 17:17
@rajkan01 rajkan01 changed the title Removed the nanostack lib service and mbed-trace fea ipv6 to false to Fixing the greentea test for baremetal with ARM toolchain Nov 15, 2019
… remove dependency issue on greentea test for bare metal with ARM toolchain
@rajkan01 rajkan01 force-pushed the feature_arm_greentea_test branch from 1055894 to 0e4d9d9 Compare November 15, 2019 17:19
@ciarmcom ciarmcom requested review from evedon, hugueskamba, jamesbeyond and a team November 15, 2019 18:00
@ciarmcom
Copy link
Member

@rajkan01, thank you for your changes.
@jamesbeyond @hugueskamba @evedon @ARMmbed/mbed-os-maintainers please review.

Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit does not state a reason why is this being removed/added, please add more vital information there

1.if we build with GCC_ARM toolchain and toolchain linker will not resolve all the unused symbols while linking.

  1. In ARM toolchain will try to resolve even all unused symbols which leads to building failure while building greentea test for bare metal with ARM toolchain.

I don't follow these 2. How is this related to nanostack removal or trace disable?

@rajkan01
Copy link
Contributor Author

Commit does not state a reason why is this being removed/added, please add more vital information there

1.if we build with GCC_ARM toolchain and toolchain linker will not resolve all the unused symbols while linking.

In ARM toolchain will try to resolve even all unused symbols which leads to building failure while building greentea test for bare metal with ARM toolchain.

I don't follow these 2. How is this related to nanostack removal or trace disable?

I have updated the "summary of change" with more details please refer.

@0xc0170 0xc0170 self-requested a review November 18, 2019 13:33
Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont understand the connection for fea-ipv6 and libservice. why fea-ipv6 is enabled by default and requires nanostack-libservice ? This might be rather another improvement - having tracing compatible with baremetal (so its configured the way it works without this ipv6) ?

@AnttiKauppila Is this good as it is in this PR?

@rajkan01 rajkan01 changed the title Fixing the greentea test for baremetal with ARM toolchain Bartemetal: Fix ARM toolchain Greentea test compilation for NUCLEO_F411RE Nov 19, 2019
@rajkan01 rajkan01 changed the title Bartemetal: Fix ARM toolchain Greentea test compilation for NUCLEO_F411RE Baremetal: Fix ARM toolchain Greentea test compilation for NUCLEO_F411RE Nov 19, 2019
@evedon
Copy link
Contributor

evedon commented Nov 19, 2019

I dont understand the connection for fea-ipv6 and libservice. why fea-ipv6 is enabled by default and requires nanostack-libservice ? This might be rather another improvement - having tracing compatible with baremetal (so its configured the way it works without this ipv6) ?

The issue is https://github.com/ARMmbed/mbed-os/blob/master/features/frameworks/mbed-trace/source/mbed_trace.c#L24-L32
I am more than happy for this to be fixed properly by @AnttiKauppila

The change in this PR is to allow bare metal greentea tests to pass. For info we do not have bare metal tests in CI currently (a nightly test should be added in this PI by the test team) - we currently run bare metal test manually.

@AnttiKauppila
Copy link

@mikter Please check this

@0xc0170 0xc0170 self-requested a review November 21, 2019 08:33
Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rajkan01 Please create an issue for what @evedon referenced above

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 21, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Nov 21, 2019

Test run: SUCCESS

Summary: 5 of 5 test jobs passed
Build number : 1
Build artifacts

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

Successfully merging this pull request may close these issues.

7 participants