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

STM32 : enable MBED trace for QSPI #12451

Merged
merged 1 commit into from
Feb 27, 2020
Merged

Conversation

jeromecoutant
Copy link
Collaborator

Summary of changes

Goal is to enable mbed trace within QSPI STM32 file.

    "target_overrides": {
        "*": {
            "mbed-trace.enable": "1",

Impact of changes

Migration actions required

Documentation


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


@0xc0170
Copy link
Contributor

0xc0170 commented Feb 17, 2020

A check - Isn't this similar to the commit that was part of another PR and broke mbed 2 build?

@jeromecoutant
Copy link
Collaborator Author

A check - Isn't this similar to the commit that was part of another PR and broke mbed 2 build?

Yes, same (except one word in a comment I think)

Let's see

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 17, 2020

As result, this fail mbed 2 tests in PRs , won't it ? The best is to wait until we complete baremetal feature, there are still couple of things to finish there - this will allow us to remove mbed 2 CI check. Until then, we should keep the code compatible.

cc @bulislaw

#include "mbed_trace.h"

#if defined(OCTOSPI1)
#define TRACE_GROUP "ST_OSPI"
Copy link
Contributor

Choose a reason for hiding this comment

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

"The group name length is limited to four characters." - https://github.com/ARMmbed/mbed-trace#compromises

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh...
I will correct.

PS: I suggest you a "git grep TRACE_GROUP".... :-(

@ciarmcom ciarmcom requested a review from a team February 17, 2020 14:00
@ciarmcom
Copy link
Member

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

@bulislaw
Copy link
Member

Mbed 2 is gone (even if not yet removed) on master (feeding into 6) so we shouldn't test it.

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 20, 2020

CI started

@mbed-ci
Copy link

mbed-ci commented Feb 20, 2020

Test run: FAILED

Summary: 3 of 11 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_mbed2-build-ARM
  • jenkins-ci/mbed-os-ci_mbed2-build-GCC_ARM
  • jenkins-ci/mbed-os-ci_mbed2-build-IAR

@jeromecoutant
Copy link
Collaborator Author

CI started

Could you try again ?

Thx

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 25, 2020

CI restarted

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 27, 2020

Ci did not report back, restarted

@mbed-ci
Copy link

mbed-ci commented Feb 27, 2020

Test run: SUCCESS

Summary: 8 of 8 test jobs passed
Build number : 3
Build artifacts

@0xc0170 0xc0170 merged commit 2d93a45 into ARMmbed:master Feb 27, 2020
@jeromecoutant
Copy link
Collaborator Author

NB: difference between "test failed" commit, and "test success" commit:

#include "mbed_trace.h"
=> MBED2 build failed

#include "mbed-trace/mbed_trace.h"
=> all OK

@jeromecoutant jeromecoutant deleted the PR_QSPI_TRACE branch March 2, 2020 09:06
@0xc0170
Copy link
Contributor

0xc0170 commented Mar 2, 2020

I don't understand this then, looks like a bug in tools

@VeijoPesonen
Copy link
Contributor

NB: difference between "test failed" commit, and "test success" commit:

#include "mbed_trace.h"
=> MBED2 build failed

#include "mbed-trace/mbed_trace.h"
=> all OK

I have stumbled on this same issue myself - #12270 (comment)

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.

6 participants