FlashIAP: Add timing test #7383

davidsaada · 2018-07-01T12:00:13Z

Description

Add a timing test for FlashIAP erase and write operations.

Pull request type

[ ] Fix
[ ] Refactor
[ ] New target
[x] Feature
[ ] Breaking change

0xc0170 · 2018-07-02T06:59:40Z

How is this testing timing ? there are few asserts that check return type. The timing is only informative via printf ?

davidsaada · 2018-07-02T09:04:34Z

How is this testing timing ? there are few asserts that check return type. The timing is only informative via printf ?

The output of this test is indeed the information given to us via prints. Information we now gather, which can be a base for an extension of this test, in case we see outstanding results.

0xc0170 · 2018-07-02T10:25:45Z

The output of this test is indeed the information given to us via prints. Information we now gather, which can be a base for an extension of this test, in case we see outstanding results.

Is it that we would run CI, you would gather results and update this PR to actually set some acceptable timing range ?

@ARMmbed/mbed-os-test Please review

davidsaada · 2018-07-02T12:01:56Z

Is it that we would run CI, you would gather results and update this PR to actually set some acceptable timing range ?

Indeed so. May add other tests, but this is the most reasonable outcome.

0xc0170 · 2018-07-02T12:15:04Z

/morph build

mbed-ci · 2018-07-02T14:18:59Z

Build : SUCCESS

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

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

mbed-ci · 2018-07-02T17:20:16Z

Exporter Build : FAILURE

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

mbed-ci · 2018-07-02T18:55:38Z

Test : SUCCESS

Build number : 2272
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7383/2272

davidsaada · 2018-07-03T10:58:31Z

Fixed a few bugs in calculations and pushed. @0xc0170 please rerun morph build.

cmonr · 2018-07-05T16:03:36Z

@davidsaada Need someone from @ARMmbed/mbed-os-storage to OK this before we can start CI.

0xc0170 · 2018-07-10T11:07:16Z

Running CI to gather test data as previously requested

/morph build

mbed-ci · 2018-07-10T12:03:42Z

Build : SUCCESS

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

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

mbed-ci · 2018-07-10T13:29:17Z

Test : SUCCESS

Build number : 2314
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7383/2314

cmonr · 2018-07-11T03:20:01Z

/morph export-build

cmonr · 2018-07-11T03:22:06Z

Pinging @ARMmbed/mbed-os-storage again

mbed-ci · 2018-07-11T06:28:25Z

Exporter Build : SUCCESS

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

cmonr · 2018-07-11T13:29:18Z

Is it that we would run CI, you would gather results and update this PR to actually set some acceptable timing range?

On hold, awaiting an update.

davidsaada · 2018-07-17T16:59:45Z

Added timing acceptance checks.

jamesbeyond · 2018-07-19T18:27:01Z

General speaking, we do not recommend the use of printf() inside GreenTea tests.
Since it is through mbed retargeting instead of GreanTea framework, the integrity of the GreenTea communication messages is not guaranteed.

We suggest using utest_printf() or greentea_send_kv() to send messages to the serial output, which will achieve the same results.
Also, those messages can be handled by host side test scripts if necessary.
@cmonr @davidsaada

studavekar · 2018-07-19T18:46:12Z

In addition to @jamesbeyond comment, we should try to avoid greentea_send_kv() if KV pair is not consumed by green tea.

@davidsaada is there a way to reduce the traffic on uart as we have seen test failure because of it ref - IOTMORF-1861

davidsaada · 2018-07-19T18:53:10Z

@davidsaada is there a way to reduce the traffic on uart as we have seen test failure because of it ref - IOTMORF-1861

Yes, Added a flash map print. Will remove it as it's not necessary for the test.

studavekar · 2018-07-19T19:18:25Z

TESTS/mbed_drivers/flashiap/main.cpp

+    printf("Flash layout:\n");
+    while(address < end_address) {
+        sector_size = flash_device.get_sector_size(address);
+        printf("Address 0x%08x, size %6d\n", address, sector_size);


can we reduce the number of print for the test, we may hit similar issue to IOTMORF-1861

0xc0170 · 2018-07-20T12:09:21Z

@jamesbeyond @studavekar Is this printf vs greentea kv / utest printf documented anywhere ? If not, can we add it. I've noticed this in some other tests also - often debug messages are left in tests.

davidsaada · 2018-07-22T11:08:59Z

Removed flash map prints.

0xc0170 · 2018-07-23T09:49:24Z

TESTS/mbed_drivers/flashiap/main.cpp

+
+    if (num_write_sizes) {
+        avg_erase_time /= num_write_sizes;
+        printf("\nErase size %6u bytes: avg %10u, min %10u, max %10u (usec)\n\n",


still there are 3 prints that do not have any value until the test fails (if it fails, then printing erase size makes sense but only if it fails).

Can we remove all 3 printf from from this test case (move them to assert msg rather)

@0xc0170 I would like to keep these prints to monitor these times also on success tests. We suspect these times can change with wear of the flash. They will also help us to understand for every device the level of intrusiveness of NVStore and SOTP

Understood.

We suggest using utest_printf() or greentea_send_kv() to send messages to the serial output, which will achieve the same results.

@jamesbeyond Should these be updated to utest_printf() ?

Should these be updated to utest_printf() ?

We'd suggest to utest_printf() over printf(), unless they are not interchangeable. In your case, I think the outcome would be the same.
Because it is less risk of disturbing the greentea communication strings, especially greentea is busy polling on serial.

Is this printf vs greentea kv / utest printf documented anywhere ? If not, can we add it. I've noticed this in some other tests also - often debug messages are left in tests.

Sorry, we don't have a formal document about that. We will add one in.

@0xc0170 @davidsaada @dannybenor

cmonr · 2018-08-02T15:26:35Z

@davidsaada Thoughts on @jamesbeyond' comment?

davidsaada · 2018-08-02T15:39:38Z

Thoughts on @jamesbeyond' comment?

Sorry, this slipped me. Updated all the code to use utest_printf.

cmonr · 2018-08-02T16:13:19Z

@davidsaada No worries, happens to all of us :)

cmonr · 2018-08-02T16:13:45Z

@0xc0170 @jamesbeyond @studavekar Mind re-reviewing?

studavekar

Looks good to me.

cmonr · 2018-08-02T19:39:35Z

/morph build

mbed-ci · 2018-08-02T20:29:22Z

Build : SUCCESS

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

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

mbed-ci · 2018-08-02T22:07:37Z

Exporter Build : SUCCESS

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

mbed-ci · 2018-08-02T22:12:24Z

Test : SUCCESS

Build number : 2458
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7383/2458

…_test FlashIAP: Add timing test

0xc0170 requested a review from a team July 2, 2018 06:57

0xc0170 added the needs: review label Jul 2, 2018

0xc0170 added needs: work and removed needs: review labels Jul 2, 2018

davidsaada force-pushed the david_flashiap_timing_test branch from 9118dae to 66152e6 Compare July 3, 2018 10:57

cmonr added needs: review and removed needs: work labels Jul 5, 2018

dannybenor approved these changes Jul 11, 2018

View reviewed changes

0xc0170 added needs: work do not merge and removed needs: review labels Jul 11, 2018

davidsaada force-pushed the david_flashiap_timing_test branch from 66152e6 to f9db073 Compare July 17, 2018 16:57

studavekar reviewed Jul 19, 2018

View reviewed changes

0xc0170 added needs: work and removed needs: review labels Jul 20, 2018

davidsaada force-pushed the david_flashiap_timing_test branch from f9db073 to 57f4d05 Compare July 22, 2018 11:05

0xc0170 reviewed Jul 23, 2018

View reviewed changes

FlashIAP: Add timing test

7cf5b3a

davidsaada force-pushed the david_flashiap_timing_test branch from 57f4d05 to 7cf5b3a Compare August 2, 2018 15:39

cmonr added needs: review and removed needs: work labels Aug 2, 2018

studavekar approved these changes Aug 2, 2018

View reviewed changes

cmonr added needs: CI and removed needs: review labels Aug 2, 2018

cmonr added the release-version: 5.9.5 label Aug 2, 2018

cmonr merged commit 8f7024a into ARMmbed:master Aug 3, 2018

0xc0170 removed the needs: CI label Aug 3, 2018

davidsaada deleted the david_flashiap_timing_test branch August 22, 2018 09:09

pan- pushed a commit to pan-/mbed that referenced this pull request Aug 22, 2018

Merge pull request ARMmbed#7383 from davidsaada/david_flashiap_timing…

fd21674

…_test FlashIAP: Add timing test

FlashIAP: Add timing test #7383

FlashIAP: Add timing test #7383

Uh oh!

Conversation

davidsaada commented Jul 1, 2018

Description

Pull request type

Uh oh!

0xc0170 commented Jul 2, 2018

Uh oh!

davidsaada commented Jul 2, 2018

Uh oh!

0xc0170 commented Jul 2, 2018

Uh oh!

davidsaada commented Jul 2, 2018

Uh oh!

0xc0170 commented Jul 2, 2018

Uh oh!

mbed-ci commented Jul 2, 2018

Triggering tests

Uh oh!

mbed-ci commented Jul 2, 2018

Uh oh!

mbed-ci commented Jul 2, 2018

Uh oh!

davidsaada commented Jul 3, 2018

Uh oh!

cmonr commented Jul 5, 2018

Uh oh!

0xc0170 commented Jul 10, 2018

Uh oh!

mbed-ci commented Jul 10, 2018

Triggering tests

Uh oh!

mbed-ci commented Jul 10, 2018

Uh oh!

cmonr commented Jul 11, 2018

Uh oh!

cmonr commented Jul 11, 2018

Uh oh!

mbed-ci commented Jul 11, 2018

Uh oh!

cmonr commented Jul 11, 2018

Uh oh!

davidsaada commented Jul 17, 2018

Uh oh!

jamesbeyond commented Jul 19, 2018

Uh oh!

studavekar commented Jul 19, 2018

Uh oh!

davidsaada commented Jul 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Jul 20, 2018

Uh oh!

davidsaada commented Jul 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cmonr commented Aug 2, 2018

Uh oh!

davidsaada commented Aug 2, 2018

Uh oh!

cmonr commented Aug 2, 2018

Uh oh!

cmonr commented Aug 2, 2018

Uh oh!

studavekar left a comment

Choose a reason for hiding this comment

Uh oh!

cmonr commented Aug 2, 2018

Uh oh!

mbed-ci commented Aug 2, 2018