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 RealTek WiFi test configuration #5291

Merged
merged 1 commit into from Nov 6, 2017

Conversation

Projects
None yet
10 participants
@SenRamakri
Contributor

SenRamakri commented Oct 10, 2017

This change adds necessary configuration files to configure Wifi tests for Realtek Ameba board.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 11, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 11, 2017

@SenRamakri Can you provide test results for this patch?

@sarahmarshy

This comment has been minimized.

Contributor

sarahmarshy commented Oct 11, 2017

The only problem is that this will fail with the way the tests are invoked in CI. CI will need to be setup to pass the SSID and password.

@mbed-ci

This comment has been minimized.

mbed-ci commented Oct 11, 2017

Build : SUCCESS

Build number : 117
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5291/

Triggering tests

/test mbed-os

@SenRamakri

This comment has been minimized.

Contributor

SenRamakri commented Oct 11, 2017

TestResults.txt

See the attached test results for RealTek Ameba board executed using the test configuration in this PR. As you can see all the network tests passed except for one, which was failing even before these changes were added and has nothing to do with test configuration.

@mbed-ci

This comment has been minimized.

@tung7970

This comment has been minimized.

Contributor

tung7970 commented Oct 12, 2017

Will look into this particular ticker failed case.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 12, 2017

See the attached test results for RealTek Ameba board executed using the test configuration in this PR. As you can see all the network tests passed except for one, which was failing even before these changes were added and has nothing to do with test configuration.

It might be worth creating an issue to provide more details, and then fix will reference it

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 12, 2017

@SeppoTakalo Please review

@tung7970

This comment has been minimized.

Contributor

tung7970 commented Oct 12, 2017

@SenRamakri how to trigger this 'tests-mbed_drivers-ticker' test ? It seems different from local greentea test.

@SenRamakri

This comment has been minimized.

Contributor

SenRamakri commented Oct 12, 2017

@tung7970 - You can run the tests as
mbed test --run -m -t -n tests-mbed_drivers-ticker -v

@tung7970

This comment has been minimized.

Contributor

tung7970 commented Oct 12, 2017

The test result I saw is 1x ticker and 2x ticker, which is different from your test results.

mbedgt: test case report:
+---------------------------+-------------------+-----------------------------------+----------------------+--------+--------+--------+--------------------+
| target | platform_name | test suite | test case | passed | failed | result | elapsed_time (sec) |
+---------------------------+-------------------+-----------------------------------+----------------------+--------+--------+--------+--------------------+
| REALTEK_RTL8195AM-GCC_ARM | REALTEK_RTL8195AM | mbed-os-tests-mbed_drivers-ticker | Timers: 1x ticker | 1 | 0 | OK | 7.8 |
| REALTEK_RTL8195AM-GCC_ARM | REALTEK_RTL8195AM | mbed-os-tests-mbed_drivers-ticker | Timers: 2x callbacks | 1 | 0 | OK | 6.94 |
+---------------------------+-------------------+-----------------------------------+----------------------+--------+--------+--------+--------------------+
mbedgt: test case results: 2 OK
mbedgt: completed in 37.42 sec

@SenRamakri

This comment has been minimized.

Contributor

SenRamakri commented Oct 12, 2017

It appears the following tests got removed from "ticker" test suite recently and I ran my tests before they got removed on 10/3. That's why you are not seeing those tests in your test run. And when a test fail, following tests in that suite is skipped. That's why in my test runs, the first test in "ticker" test suite failed and rest of the tests are "SKIPPED". I'll have to find out why those tests got removed. As of now, you can go ahead make your decisions/conclusions based on your tests results.

| Test attach for 0.001s and time measure
| Test attach for 0.01s and time measure
| Test attach for 0.1s and time measure
| Test attach for 0.5s and time measure
| Test attach_us for 100ms and time measure
| Test attach_us for 10ms and time measure
| Test attach_us for 1ms and time measure
| Test attach_us for 500ms and time measure
| Test detach
| Test multi call and time measure
| Test multi ticker

@SenRamakri

This comment has been minimized.

Contributor

SenRamakri commented Oct 12, 2017

@tung7970 - I found that those tests were removed temporarily to address some issues, but they will be added back soon. At that point you may see the failure again. So I would suggest you to revert your mbed-os revision to some date before 10/3 and you should see the same results.

@tung7970

This comment has been minimized.

Contributor

tung7970 commented Oct 13, 2017

@SenRamakri It seems new tests were removed because they failed on several targets.
I may be wrong, but I checked the source, found that the tolerance factor might be incorrectly set. The test uses SystemCoreClock to calculate tolerance factor, but the SystemCoreClock may not be the same as real clock source. For example, for RTL8195AM, SystemCoreClock is roughly 166MHz, but the real clock source is 32K, so there's basically zero tolerance for RTL8195AM. If I change tolerance factor accordingly, all tests can pass except the multi-ticker one. Will check that one further.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 13, 2017

The test uses SystemCoreClock to calculate tolerance factor, but the SystemCoreClock may not be the same as real clock source. For example, for RTL8195AM, SystemCoreClock is roughly 166MHz, but the real clock source is 32K, so there's basically zero tolerance for RTL8195AM. If I change tolerance factor accordingly, all tests can pass except the multi-ticker one. Will check that one further.

cc @maciejbocianski @fkjagodzinski - I dont see this as a tracking issue where would could continue this discussion ! As this patch is for Wifi

@SeppoTakalo review pls

@maciejbocianski

This comment has been minimized.

Member

maciejbocianski commented Oct 13, 2017

@SenRamakri can we get full logs form tests or link to CI from failed test ?

@SenRamakri

This comment has been minimized.

Contributor

SenRamakri commented Oct 13, 2017

@maciejbocianski - The attached "TestResults.txt" is from test execution on my development pc and we don't have a CI link. You should be able to re-create the failure in your machine using the following steps.

Running mbed-os tests – Refer https://github.com/ARMmbed/mbed-os

  1. Checkout mbed-os from 10/3
  2. Apply the attached patch in zip – RealTek_patch_mbedos.diff
  3. Update tools\test_configs\RealTekInterface.json with the right values for ssid(WIFI_SSID), passphrase(WIFI_PASSWORD), security option(WIFI_SECURITY) and wifi channel(WIFI_CHANNEL).
  4. Run the tests as below for each compiler/toolchain.
    a. mbed test -m REALTEK_RTL8195AM -t arm –test-config Realtek_wifi
    b. mbed test -m REALTEK_RTL8195AM -t gcc_arm –test-config Realtek_wifi
    c. mbed test -m REALTEK_RTL8195AM -t iar –test-config Realtek_wifi

Running ci-test-shield tests – Refer https://github.com/ARMmbed/ci-test-shield

  1. Checkout ci-test-shield repo
  2. cd ci-test-shield
  3. mbed target auto
  4. mbed toolchain GCC_ARM ( Use IAR for IAR and ARM for ARM compiler )
  5. cd mbed-os
  6. Update mbed-os from 10/3
  7. Apply the attached patch in zip – RealTek_patch_citestshield.diff
  8. Cd .. ( change dir to ci-test-shield )
  9. mbed test -n tests-* --app-config .\mbed_app.json – This should start running the tests.

Let me know if you need more info on this.

RealTekTesting.zip

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 19, 2017

@tung7970 @SeppoTakalo happy with this patch?

The failure that @SenRamakri reported, has it been reported anywhere - tracking issue referenced here?

@maciejbocianski you referenced 5261, hwo is it related ?

@maciejbocianski

This comment has been minimized.

Member

maciejbocianski commented Oct 19, 2017

@0xc0170 5261 will be modified to make it working on REALTEK_RTL8195AM platform

@adbridge

This comment has been minimized.

Contributor

adbridge commented Oct 24, 2017

@tung7970 @SeppoTakalo happy with this patch?

BUMP

@tung7970

This comment has been minimized.

Contributor

tung7970 commented Oct 24, 2017

@adbridge It looks good to me.

@adbridge

This comment has been minimized.

Contributor

adbridge commented Oct 26, 2017

The only problem is that this will fail with the way the tests are invoked in CI. CI will need to be setup to pass the SSID and password.

Doesn't look like this comment has been addressed ?
Moving this back to needs review until this has been considered.

@theotherjimmy theotherjimmy changed the title from RealTek WiFi test configuration to Add RealTek WiFi test configuration Nov 2, 2017

@tung7970

This comment has been minimized.

Contributor

tung7970 commented Nov 3, 2017

Is this PR pending for change from the CI side or from the Realtek side ?

There is a way to store WiFi credentials in non-volatile storage and remove the need of passing additional parameters during connect. @Archcady Please kindly check if this is feasible.

@Archcady

This comment has been minimized.

Contributor

Archcady commented Nov 3, 2017

@tung7970 it looks good to me, plus I'm using the same way to config tests for REALTEK_RTL8195AM, need to define wifi credentials as micros in RealtekInterface.json in order to compile and run.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Nov 3, 2017

The only problem is that this will fail with the way the tests are invoked in CI. CI will need to be setup to pass the SSID and password.

Doesn't look like this comment has been addressed ?
Moving this back to needs review until this has been considered.

@SeppoTakalo @studavekar Is this blocking this PR, do we have a way to configure this, and test in CI? Let us know please

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented Nov 3, 2017

I'm unsure whether we run any WiFi tests in the PR verification phase.
I have no knowledge of such system.

@theotherjimmy theotherjimmy merged commit 863b3fd into ARMmbed:master Nov 6, 2017

7 checks passed

AWS-CI uVisor Build & Test Success
Details
AWS-CI uvisor Test DIDNT RUN UVISOR TESTS
Details
Cam-CI uvisor Build & Test DIDNT RUN UVISOR TESTS
Details
ci-morph-build
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@adbridge

This comment has been minimized.

Contributor

adbridge commented Nov 7, 2017

Looks like this PR broke master. How did it get through the ci testing ?

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Nov 7, 2017

How did it break master?

@adbridge

This comment has been minimized.

Contributor

adbridge commented Nov 7, 2017

From what @0xc0170 was saying, I believe it enables a test which wasn't being run at the time the CI ran for this. But the test was subsequently added in a separate PR and is now active.

@adbridge

This comment has been minimized.

Contributor

adbridge commented Nov 7, 2017

The CI run on this was 27 days ago, so we should have re-run the CI before marking as ready for merge and merging

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