Skip to content

Make mbed-trace available to bare metal #13649

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

Merged
merged 6 commits into from
Sep 22, 2020

Conversation

LDong-Arm
Copy link
Contributor

@LDong-Arm LDong-Arm commented Sep 21, 2020

Summary of changes

(Replaces #13617)

As discussed in #13617, we decide that mbed-trace is a basic feature and should be always available, even in bare metal builds. This is required by some targets and drivers code.

Note: Previously IPv6 tracing is enabled by default, which has dependency on nanostack-libservice (unavailable to bare metal). Rather than simply disabling IPv6 tracing which would break existing applications, we add a flag nanostack-libservice.present (which translates to MBED_CONF_NANOSTACK_LIBSERVICE_PRESENT) and set IPv6 tracing accordingly.

Impact of changes

mbed-trace is now available to all bare metal builds, without having to manually enable it.

Migration actions required

It's crucial that nanostack-libservice contains the nanostack-libservice.present change, otherwise mbed-trace will not enable IPv6 tracing.

The changes are upstreamed to PelionIoT/nanostack-libservice#92 and PelionIoT/mbed-trace#99

Documentation

None.


Pull request type

[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

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

Reviewers

@ARMmbed/mbed-os-core @artokin @jeromecoutant @kjbracey-arm


@ciarmcom
Copy link
Member

@LDong-Arm, thank you for your changes.
@artokin @jeromecoutant @ARMmbed/mbed-os-core @ARMmbed/mbed-os-maintainers please review.

Copy link
Contributor

@evedon evedon left a comment

Choose a reason for hiding this comment

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

That's a good solution!

Some code in "drivers" and "targets" use mbed-trace, which when
disabled has zero overhead as trace function calls are masks
by dummy macros.
This reverts commit 183ca77.

Now the "mbed_trace.h" header is always available and maps
trace functions to dummy macros when tracing is disabled.
@LDong-Arm
Copy link
Contributor Author

@evedon Thanks for the review, I've addressed your comment.

@LDong-Arm LDong-Arm requested a review from evedon September 21, 2020 14:46
@LDong-Arm
Copy link
Contributor Author

Hi @artokin, for reasons described above (Mbed OS bare metal), we need an extra check for IPv6 availability. This means updated mbed-trace will only work with updated nanoclient-libservice.
I've created PelionIoT/nanostack-libservice#92 and PelionIoT/mbed-trace#99 to upstream those changes, I hope they won't cause too much trouble on your side?
Thanks in advance.

@mergify mergify bot added needs: CI and removed needs: review labels Sep 21, 2020
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've just hit this problem with CMake, good to have it fixed !

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 22, 2020

CI started

@ladislas
Copy link
Contributor

I've just hit this problem with CMake, good to have it fixed !

Same here!

@mbed-ci
Copy link

mbed-ci commented Sep 22, 2020

Jenkins CI Test : ✔️ SUCCESS

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_build-ARM ✔️
jenkins-ci/mbed-os-ci_build-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_greentea-test ✔️
jenkins-ci/mbed-os-ci_dynamic-memory-usage ✔️
jenkins-ci/mbed-os-ci_cloud-client-pytest ✔️
jenkins-ci/mbed-os-ci_wisun-mesh-test ✔️

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.

8 participants