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

fix bug for RDA UNO_91H:calling us ticker functions without init. #9318

Merged
merged 1 commit into from Jan 24, 2019

Conversation

caixue1102
Copy link
Contributor

@caixue1102 caixue1102 commented Jan 10, 2019

Description

fix bug for RDA UNO_91H:calling us ticker functions without init.
solution: init us ticker when init TRNG, because we use us ticker read to delay and wait for TRNG seed ready.

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 10, 2019

Can you review travis build failure in docs:
$ find "(" -name "*.a" -or -name "*.ar" ")" -and -not -name "lib*" | tee BUILD/badlibs | sed -e "s/^/Bad library name found: /" && [ ! -s BUILD/badlibs ]

Another request is to use Description to describe your pull request. I'll paste here an example

Description

Add support wifi for RDA target

Tested with GCC ARM, IAR and ARMCC.

Test report here: `paste here test results

@SeppoTakalo
Copy link
Contributor

Please specify the target name in commit message and Pull-Request title. Otherwise this is a bit misleading.

Also, please submit a test results when running Socket, Wifi + EMAC tests against current Mbed OS master.
These are documented in:

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 11, 2019

@caixue1102 Let us know if you need any assistance

@caixue1102 caixue1102 changed the title Add WIFI support fix bug for RDA UNO_91H:init us ticker when init TRNG Jan 16, 2019
@kyliuxing
Copy link
Contributor

@0xc0170 We need fix a TRNG bug before support wifi for UNO_91H. Please help review it. Thanks.

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 17, 2019

Looks fine to me, one question: why are we initalizing us ticker in TRNG ?

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.

Simple fix but not clear to a reader how is TRNG related to us ticker - what bug is this fixing - can you add this to the commit msg?

Travis failure will be fixed via rebase, we can help there once the last fix is on master (today)

@caixue1102
Copy link
Contributor Author

We use us ticker delay to waiting for TRNG seed ready, and we do this to avoid calling us ticker functions without us_ticker_init.

@caixue1102 caixue1102 changed the title fix bug for RDA UNO_91H:init us ticker when init TRNG fix bug for RDA UNO_91H:calling us ticker functions without init. Jan 17, 2019
@0xc0170
Copy link
Contributor

0xc0170 commented Jan 17, 2019

We will rebase once travis is done and start CI

@cmonr
Copy link
Contributor

cmonr commented Jan 17, 2019

NOTE: This PR has now been rebased.

If this was made in error, feel free to force-push your local branch to restore the PR.

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 21, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Jan 21, 2019

Test run: FAILED

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

Failed test jobs:

  • jenkins-ci/mbed-os-ci_greentea-test

@caixue1102
Copy link
Contributor Author

Thank you for the test, but I can't find the failed test job in the link, so that I don't know the relationship with my commit. Could you please give me some pointers.

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 22, 2019

Build artifacts

You can find it linked above via Build artifacts.

I restarted teh test as this was an issue with CI job itself, restarted

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 22, 2019

CI restarted

@cmonr cmonr merged commit 899ea59 into ARMmbed:master Jan 24, 2019
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.

None yet

6 participants