test: add us ticker test #4690

0xc0170 · 2017-07-03T11:45:05Z

This test exercises us ticker API

init should be executed just once
read timestamp
setting interrupts

Disable interrupt shall be also tested, should be proparly in its own test,
as it is the very last function to be executed (we dont have API to reenable interrupts), thus cant be in the same test unit.

Need to add documentation to this test.

This is very basic test, init version that can expand. I can add disable also but it is own file, was thinking something like this:

void test_us_ticker_disable_interrupt(void)
{
    // we use here ticker API to set properly callback to set our complete flag
    const ticker_data_t *us_ticker = get_us_ticker_data();
    ticker_event_t event;
    const uint32_t future_margin = 1000; // 1ms margin for interrupt to be fired 

    complete = false;
    ticker_set_handler(us_ticker, us_ticker_handler_test);
    timestamp_t future_timestamp = us_ticker_read() + 1000UL;
    ticker_insert_event_us(us_ticker, &event, future_timestamp, 1);
    us_ticker_disable_interrupt();
    while (us_ticker_read() <= (future_timestamp + future_margin));
    TEST_ASSERT_TRUE_MESSAGE(complete == false, "Interrupt was disabled and still fired");
}

I can refactor this to be us ticker and lp ticker tests (via that ticker data API), would our test allow it? This would be the idea:

ticker_lpus/ticker_header_test.h - would contain definitions for both
ticker_lpus/lp_ticker/main.cpp - this would use lp ticker data
ticker_lp_us/us_ticker/main.cpp - this would use us ticker data

Tested with ARM on K64F and NCS36510 so far, more testing required.

cc @pan- @c1728p9 Anyone else interested in tickers?

theotherjimmy · 2017-07-10T16:04:36Z

@pan- @c1728p9 Could you review?

c1728p9

This test seems like it would apply just as well to the lp_ticker. Maybe it could be combined with #4628 and applied to both the us_ticker and the lp_ticker?

c1728p9 · 2017-07-11T23:05:49Z

TESTS/mbed_hal/us_ticker/main.cpp

+Case cases[] = {
+    Case("us ticker - Init", test_us_ticker_init, greentea_failure_handler),
+    Case("us ticker - Read", test_us_ticker_read, greentea_failure_handler),
+    Case("us ticker - Read wiht loop", test_us_ticker_read_loop, greentea_failure_handler),


type - should be "with"

c1728p9 · 2017-07-11T23:12:50Z

TESTS/mbed_hal/us_ticker/main.cpp

+    complete = false;
+    ticker_set_handler(us_ticker, us_ticker_handler_test);
+    timestamp_t future_timestamp = us_ticker_read() + 5000UL;
+    ticker_insert_event_us(us_ticker, &event, future_timestamp, 1);


Using the ticker_* layer here adds extra code that could interfere with testing of the hal. You might consider using the us_ticker_* API here directly, so changes in ticker_* don't effect hal testing.

That was the intention but current us ticker API does not provide this, the handler calls directly upper layer, and there is not API to redirect this. Only the upper layer provides that.

We can set interrupt, but handling ISR is in the handler that calls the upper layer. We can remove this test, and the upper layer then exercises set_interrupt() ?

It could be via weak, but then we would not exercise IRQ handler. Therefore I can just remove this code from here completely, and set interrupt needs to be tested via upper layer tests

0xc0170 · 2017-07-12T09:28:48Z

This test seems like it would apply just as well to the lp_ticker. Maybe it could be combined with #4628 and applied to both the us_ticker and the lp_ticker?

As stated above, yes this could be refactored, I'll do that, rebase soon

0xc0170 · 2017-07-25T12:15:35Z

I pushed an update. @c1728p9 now exercises HAL ticker API.

+--------------+---------------+------------------------------+----------------------------------+--------+--------+--------+--------------------+
| target       | platform_name | test suite                   | test case                        | passed | failed | result | elapsed_time (sec) |
+--------------+---------------+------------------------------+----------------------------------+--------+--------+--------+--------------------+
| K64F-GCC_ARM | K64F          | tests-mbed_hal-lp_us_tickers | hal tickers - Init               | 1      | 0      | OK
    | 0.05               |
| K64F-GCC_ARM | K64F          | tests-mbed_hal-lp_us_tickers | hal tickers - Read               | 1      | 0      | OK
    | 0.06               |
| K64F-GCC_ARM | K64F          | tests-mbed_hal-lp_us_tickers | hal tickers - Read with the loop | 1      | 0      | OK
    | 0.07               |
+--------------+---------------+------------------------------+----------------------------------+--------+--------+--------+--------------------+

This test exercises ticker API - init should be executed just once - read timestamp Set interrupt should be tested via upper layer, as it does not provide API to change handler that is invoked in the ISR ticker handler.

0xc0170 · 2017-07-26T16:07:23Z

Rebased, now using setup handler to setup interface (lp vs us ticker), as @c1728p9 proposed. Tested with K64F, same result as previously, all OK

0xc0170 · 2017-07-26T16:08:20Z

/morph test

theotherjimmy · 2017-07-31T15:17:54Z

@studavekar It looks like this did not report to github properly.

studavekar · 2017-07-31T20:36:33Z

This is due to java process crash , re-triggering request

/morph test

mbed-bot · 2017-07-31T23:41:41Z

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 919

Example Build failed!

0xc0170 · 2017-08-03T08:55:50Z

/morph test

mbed-bot · 2017-08-03T16:08:38Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 928

All builds and test passed!

0xc0170 · 2017-08-03T16:42:39Z

/morph test-nightly

mbed-bot · 2017-08-04T08:10:20Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 935

All builds and test passed!

0xc0170 added the needs: review label Jul 3, 2017

0xc0170 mentioned this pull request Jul 3, 2017

ncs36510: us ticker improvements #4679

Merged

c1728p9 reviewed Jul 11, 2017

View reviewed changes

0xc0170 added needs: work and removed needs: review labels Jul 24, 2017

0xc0170 force-pushed the us_ticker_test branch from 129f882 to 856c486 Compare July 25, 2017 12:14

0xc0170 added needs: review and removed needs: work labels Jul 26, 2017

test: add hal ticker test (us + lp tickers)

ae51f18

This test exercises ticker API - init should be executed just once - read timestamp Set interrupt should be tested via upper layer, as it does not provide API to change handler that is invoked in the ISR ticker handler.

0xc0170 force-pushed the us_ticker_test branch from 856c486 to ae51f18 Compare July 26, 2017 16:06

c1728p9 approved these changes Jul 26, 2017

View reviewed changes

theotherjimmy added needs: CI and removed needs: review labels Jul 27, 2017

0xc0170 added ready for merge and removed needs: CI labels Aug 3, 2017

0xc0170 added needs: CI and removed ready for merge labels Aug 3, 2017

0xc0170 added the ready for merge label Aug 7, 2017

0xc0170 removed the needs: CI label Aug 7, 2017

theotherjimmy merged commit 294d7e7 into ARMmbed:master Aug 7, 2017

theotherjimmy added release-version: 5.5.5 and removed ready for merge labels Aug 7, 2017

test: add us ticker test #4690

test: add us ticker test #4690

Uh oh!

Conversation

0xc0170 commented Jul 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theotherjimmy commented Jul 10, 2017

Uh oh!

c1728p9 left a comment

Choose a reason for hiding this comment

Uh oh!

c1728p9 Jul 11, 2017

Choose a reason for hiding this comment

Uh oh!

c1728p9 Jul 11, 2017

Choose a reason for hiding this comment

Uh oh!

0xc0170 Jul 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Jul 12, 2017

Uh oh!

0xc0170 commented Jul 25, 2017

Uh oh!

0xc0170 commented Jul 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xc0170 commented Jul 26, 2017

Uh oh!

theotherjimmy commented Jul 31, 2017

Uh oh!

studavekar commented Jul 31, 2017

Uh oh!

mbed-bot commented Jul 31, 2017

Result: FAILURE

Output

Uh oh!

0xc0170 commented Aug 3, 2017

Uh oh!

mbed-bot commented Aug 3, 2017

Result: SUCCESS

Output

Uh oh!

0xc0170 commented Aug 3, 2017

Uh oh!

mbed-bot commented Aug 4, 2017

Result: SUCCESS

Output

Uh oh!

Uh oh!

0xc0170 commented Jul 3, 2017 •

edited

Loading

0xc0170 Jul 12, 2017 •

edited

Loading

0xc0170 commented Jul 26, 2017 •

edited

Loading