Level testing #47

mray190 · 2017-03-27T23:03:08Z

Full refactor of how testing is done with the CI test shield. Details of each level found below:

Bare Implementation - First stage of development.

Constructors / destructors
Assumptions
At least 1 of every peripheral possible implimented

Basic Compliance - things actually start working.

Basic Read / Write operations for each API implimented
Single instance of each peripheral works
Timing of API measured to 10%

mbed Enabled Compliance - required to be mbed enabled, this level must pass for it to be marked public

At least one of each peripheral is broken out to Arduino Hardware spec header
Full API implimented
Every pin available is tested with single instance
Multiple instances of each peripheral can be implimented at same time
peripherals work in interrupt context
Timing of API measured to 5%

Corner Case Compliance - Goal level, not everyone will get here, sets goals for the best platforms to achieve

Edge / Corner case tests
Timing of API measured to 1%
Check for corner cases (really beat on the API hard)
Instantiate everything and test all at once

Full documentation can be found here: https://gist.github.com/BlackstoneEngineering/e3db036272daa2e5424076fbca8b9d43

jeromecoutant · 2017-03-28T12:33:48Z

TestFramework.cpp

+	pinout[CITS_AnalogInput].push_back(construct_pinmap(MBED_CONF_APP_AIN_4));
+	pinout[CITS_AnalogInput].push_back(construct_pinmap(MBED_CONF_APP_AIN_5));
+	pinout[CITS_AnalogOutput].push_back(construct_pinmap(MBED_CONF_APP_AOUT));
+	pinout[CITS_DigitalIO].push_back(construct_pinmap(MBED_CONF_APP_DIO_0));


Hi
Note that for some platform, D0 and D1 are used for the debug serial channel...
So tests will be failed?

Valid point. The D0/D1 lines of code don't actually do anything anyway because when I dynamically find digital pins to test, the D0/D1 pins aren't pulled into the list because they are associated with UART and not Digital or PWM pinmaps. Therefore when the dynamic pin list is cross referenced with the CI Test Shield pins, the MBED_CONF_APP_DIO0/1 pins won't be used. I will remove lines 64 and 65 anyway to remove confusion

jeromecoutant · 2017-03-28T12:40:16Z

Hi
It seems that level0 tests are not related to ci-shield ?
So this part should/can be moved to mbed-os level ?

jeromecoutant · 2017-03-28T13:01:42Z

TESTS/Level2/Pwm/Pwm.cpp

+int main() {
+	// Formulate a specification and run the tests based on the Case array
+	Specification specification(TestFramework::test_setup<200>, cases);
+    return !Harness::run(specification);


200 s seems not enough ?
For NUCLEO_L476RG, I need 300

I can move to 300s. I tested with the K64F and Nucleo_F429ZI in which 200s was enough. Forgot that additional targets will have more PWMs which will increase the time

mray190 · 2017-03-28T15:56:23Z

@jeromecoutant - there are still some things I have to review with @BlackstoneEngineering. This PR should stay in an open state until Austin has gone over it with me as he created the specifications

0xc0170 · 2017-03-29T08:23:04Z

LM75B.lib

@@ -1 +0,0 @@
-https://developer.mbed.org/users/neilt6/code/LM75B/#7ac462ba84ac


why is this being removed?

It is being replaced by the more generic I2CEeprom.lib file. This new library also future proofs us and enables use of the File System API's.

0xc0170 · 2017-03-29T08:30:33Z

TESTS/Arduino/AnalogOut/AnalogOut.cpp

+	Case("Arduino - Analog Output Constructor on AOUT_0", AnalogOutput_Test<MBED_CONF_APP_AOUT>, greentea_failure_handler),
+};
+
+int main() {


fix formatting, also indentation

0xc0170 · 2017-03-29T08:31:32Z

mbed_app.json

@@ -31,7 +31,7 @@
        "PWM_1": "D5",
        "PWM_2": "D6",
        "PWM_3": "D9",
-        "DEBUG_MSG": 0
+        "DEBUG_MSG": 1


Debug messages on by default? I would expect to set them to 1 once something is failing.

Correct, needs to have Debug Messages off by default.

0xc0170 · 2017-03-29T08:35:02Z

TestFramework.hpp

+
+	TestFramework();
+
+	static bool check_size(Type pintype);


Move doxy docs from .ccp file to .h file please

@0xc0170 Is this standard practice? What is the rationale behind this?

Yes, it is. The header file is for declarations, I believe it is stated in our coding standard. The header file provides a declaration, implementation follows it (not the other way around).

0xc0170 · 2017-03-29T08:35:41Z

TESTS/host_tests/rand_provider.py

@@ -0,0 +1,42 @@
+from mbed_host_tests import BaseHostTest


license header comment should be in each file

0xc0170 · 2017-03-29T08:40:03Z

TESTS/Level3/SPI/SPI.cpp

+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * @author Michael Ray


why are you putting this info in the files? we got version control (git in our case) to hold that information. these will be soon left behind (out-dated).

So it looks awesome on his resume. Also so we know who to ask / blame later? :-P

It will be outdated soon, please remove it. we got version control. No one will touch this ever after editing the file (we don't do it automatically).

0xc0170 · 2017-03-29T08:40:42Z

TESTS/Level3/Pwm/Pwm.cpp

+    TEST_ASSERT_MESSAGE( std::abs(rc-fc) <= calculated_percent, "There was more than a specific variance in number of rise vs fall cycles");
+    TEST_ASSERT_MESSAGE( std::abs(waveform_count - rc) <= calculated_percent, "There was more than a specific variance in number of rise cycles seen and number expected.");
+    TEST_ASSERT_MESSAGE( std::abs(waveform_count - fc) <= calculated_percent, "There was more than a specific variance in number of fall cycles seen and number expected.");
+    // TEST_ASSERT_MESSAGE( std::abs(expectedTime - avgTime) <= calculated_percent,"Greater than a specific variance between expected and measured duty cycle");


shall we remove all these dead code or provide a reason there why we are keeping it in the file?

0xc0170 · 2017-03-29T09:46:03Z

TestFramework.cpp

+	int i = 0;
+	while(pinmap[i].pin != (PinName)NC) {
+		bool alreadyExists = false;
+		// printf("Pin %d\n", pinmap[i].pin);


debug printf or removed?

0xc0170 · 2017-03-29T10:02:56Z

TestFramework.cpp

+			// State: End case
+			// Pin was found, but execution had already occurred so queue up another test case
+			if (tag) {
+				TEST_ASSERT(true);


whats the use for this assert?

0xc0170 · 2017-04-03T15:20:05Z

Any update?

mray190 · 2017-04-03T15:24:00Z

@0xc0170 Made those changes but still have to discuss everything with @BlackstoneEngineering. This PR should stay in a 'Changes requested' state until everything has been approved by Austin. I will push those changes regardless

BlackstoneEngineering · 2017-04-04T20:40:55Z

Need to run these tests on boards that are known to have corner cases to verify that we are not missing something.
I reccomend

ST 401RE (has UART issue on D0/D1)
OnSemi board (does not have A4/5)
Silicon Labs board (Does not have arduino headers)
NRF52 DK - no particular reason, just want to see what happends
NRF51 EK - does not have Arduino Headers

BlackstoneEngineering · 2017-04-04T20:41:45Z

@andcor02 Can you also please review / be familiar with the proposed changes as you help maintain the HW and any changes should have you looped in on

jeromecoutant · 2017-04-05T09:25:06Z

TestFramework.cpp

+**/
+TestFramework::TestFramework() {
+	map_pins(PinMap_ADC, AnalogInput);
+	map_pins(PinMap_DAC, AnalogOutput);


You should add some capability test like
#if !DEVICE_ANALOGOUT
for each map_pins call ?

jeromecoutant · 2017-04-05T09:37:36Z

Hi

What about small targets ?
I noticed, for example, that all Level0 tests need more than 200K of flash and 64K RAM

From ST point of view, check with NUCLEO_F070RB, NUCLEO_F072RB, NUCLEO_F103RB, NUCLEO_L073RZ, NUCLEO_F410RB

BlackstoneEngineering · 2017-04-08T20:47:24Z

@jeromecoutant Noted on the small target issue. I will make sure we address this before these changes to mainline.

jeromecoutant · 2017-06-21T14:41:18Z

Hi
Any updates ? :-)

mray190 · 2017-06-22T15:41:45Z

@jeromecoutant - A lot of the code presented in this PR is a 'hack' around the current HAL setup. We are working with the mbed-os team to integrate some of this code into a library in mbed-OS HAL in a more efficient/cleaner way.

Once the changes to mbed-os have been completed, I will refactor this PR to satisfy those changes

…necessary TEST_ASSERT(true) from all tests

…light formatting corrections to TestFramework.cpp

…ents

…ins throughout the different test iterations

…ble pin assignments that a user could configure. Fixed typo found in SPI.cpp, construct_i2c->construct_spi. Added wait(0.1) to SPI.cpp so that the print statement can finish printing in the event of an error

Formatted code, removed TEST_ASSERT(true), fixed bugs, added comments

…llow along.

… USBRX check in Level0/AnalogIn as it was failing when running on the K22F. It was failing because one of its analogIn in pins is tied and already assigned to USBTX

…etting reassigned

…ey are already assigned to the USB_UART. I check here now instead of the individual tests since we should never be reassigning the USB_UART pins in any test

… DebugPrintf to Level1-AnalogIn. Uncommented line in constructor so that more spi pins get added to DIO map.

Cleaned up code, added more checks, updated mbed_app.json...

jeromecoutant · 2017-07-17T08:29:57Z

Hi

We are working with the mbed-os team to integrate some of this code into a library in mbed-OS HAL in a more efficient/cleaner way.

I don't see any mbed-os change ? :-)

jeromecoutant · 2017-07-17T08:31:00Z

TESTS/Arduino/DigitalIO/DigitalIO.cpp

+
+Case cases[] = {
+  Case("Arduino - Digital IO Constructor on DIO_0", DigitalIO_Test<MBED_CONF_APP_DIO_0>, greentea_failure_handler),
+  Case("Arduino - Digital IO Constructor on DIO_1", DigitalIO_Test<MBED_CONF_APP_DIO_1>, greentea_failure_handler),


Are you sure you want to test D0 and D1 as they are often used for printf ?

jeromecoutant · 2017-07-17T08:33:05Z

TestFramework.cpp

+/////////////////////////////////////////////////////////////////////////////*/
+TestFramework::TestFramework() {
+	map_pins(PinMap_ADC, AnalogInput);
+	map_pins(PinMap_DAC, AnalogOutput);


For targets which doesn't support DAC, this file doesn't compile.
Maybe we should add some #if DEVICE_ANALOGOUT for each line ?

@brandonboesch

I will look into adding #ifdef guards for these calls.

jeromecoutant · 2017-07-17T08:49:02Z

TESTS/Level3/Pwm/Pwm.cpp

+	// Formulate a specification and run the tests based on the Case array
+	Specification specification(TestFramework::test_setup<200>, cases);
+    return !Harness::run(specification);
+}


I don't think that level3 PWM test is working?

mbedgt: test suite 'tests-level3-pwm' ................................................................ OK in 17.67 sec
test case: 'Level 3 - PWM randomized' ........................................................ ERROR in 0.00 sec

Will rereview again

jeromecoutant · 2017-07-17T11:57:18Z

TESTS/Arduino/AnalogIn/AnalogIn.cpp

+template <PinName ain_pin> 
+void AnalogInput_Test(){
+  AnalogIn ain(ain_pin);
+  DEBUG_PRINTF("Tested pin %d for Arduino analog in capabilities\n", ain_pin);


Global comment: from my side, I would prefer a debug printf as %#x instead of %d for pin number

jeromecoutant · 2017-07-17T12:00:45Z

@BlackstoneEngineering

Noted on the small target issue. I will make sure we address this before these changes to mainline.

Is it still valid ? (I can't test NUCLEO_F070RB for ex)

mray190 · 2017-07-17T13:32:32Z

@jeromecoutant - All of these comments are great comments and we will incorporate the suggested changes.

The functionality that this PR brings to the CI test shield won't be ready for a bit - the mbed-OS changes still need to occur, and some other changes need to happen before this can be mainlined.

Closing the PR until the branch is ready again.

mray190 and others added 15 commits February 16, 2017 10:02

Initial setup of mbed level testing

23db630

Working version of dynamic pin testing for L0

9c786c8

Working state machine for L1/L2 tests

80d2c56

Failing analog out

ca78d45

Working L0-L2 for AnalogIn/Out - intended as the starting framework

567634c

DigitalIO framework setup

3d4b143

Added PWM and DigitalIO tests

9be016f

Added I2C and SPI base tests

dac97ca

Finished Level0 tests

e49460f

Added arduino tests

a135774

Additional Level1 and Level2 tests added

401ea3f

Merged master into level-testing branch for I2C and SPI changes

e98b4c8

Halfway through porting over to final version

64e3bdc

Final commit for up to level 2

786ad08

Finished Level 3 tests

6f6a274

jeromecoutant reviewed Mar 28, 2017

View reviewed changes

0xc0170 reviewed Mar 29, 2017

View reviewed changes

0xc0170 previously requested changes Mar 29, 2017

View reviewed changes

mray190 mentioned this pull request Apr 3, 2017

PWM test : Make PWM pin configurable #44

Open

jeromecoutant reviewed Apr 5, 2017

View reviewed changes

mray190 added 2 commits April 5, 2017 19:04

Made changes outlined in pull request ARMmbed#47

62b43d9

Update mbed_app.json

8e1904d

BlackstoneEngineering mentioned this pull request Jun 28, 2017

MBED_CONF_APP_DIO_X not well named for PWM tests #55

Open

brandonboesch and others added 15 commits June 28, 2017 13:03

Formated code so that it is uniform between all the tests. Removed un…

6b956e6

…necessary TEST_ASSERT(true) from all tests

Removed unused ANSI_COLOR_<COLOR NAME> defs from TestFramework.hpp

4846b7b

Changed BusInOut so that it creates a bus of 16 pins instead of 15. S…

d093ddc

…light formatting corrections to TestFramework.cpp

Changed tag from unnecessary int to a bool instead. Updated some comm…

d15893d

…ents

Fixed some bugs in TestFramework::run_spi. SPI_CS was not switching p…

df47ace

…ins throughout the different test iterations

Added greater coverage for level0/SPI testing. It now tests all possi…

c9f75d4

…ble pin assignments that a user could configure. Fixed typo found in SPI.cpp, construct_i2c->construct_spi. Added wait(0.1) to SPI.cpp so that the print statement can finish printing in the event of an error

Merge pull request #1 from brandonboesch/level-testing

0015759

Formatted code, removed TEST_ASSERT(true), fixed bugs, added comments

Cleaned up SPI code in TestFramework to make it easier to read and fo…

169500e

…llow along.

Added K22F to mbed_app.json. Formated Level1/AnalogIn. Added USBTX &&…

c7ca382

… USBRX check in Level0/AnalogIn as it was failing when running on the K22F. It was failing because one of its analogIn in pins is tied and already assigned to USBTX

Added check to prevent pins already assigned to USBTX && USBRX from g…

f3c379a

…etting reassigned

Modified TestFramework::map_pins so that it no longer adds pins if th…

285d385

…ey are already assigned to the USB_UART. I check here now instead of the individual tests since we should never be reassigning the USB_UART pins in any test

Condensed code in TestFramework for level1 testing

3053b8d

Removed wait() from Level0-spi test, and changed timeout value. Added…

9919bb1

… DebugPrintf to Level1-AnalogIn. Uncommented line in constructor so that more spi pins get added to DIO map.

Merge pull request #2 from brandonboesch/level-testing

cdf9af1

Cleaned up code, added more checks, updated mbed_app.json...

Merge branch 'master' into level-testing

974b776

jeromecoutant reviewed Jul 17, 2017

View reviewed changes

mray190 closed this Jul 17, 2017

		@@ -1 +0,0 @@
		https://developer.mbed.org/users/neilt6/code/LM75B/#7ac462ba84ac

Level testing #47

Level testing #47

Conversation

mray190 commented Mar 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeromecoutant commented Mar 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mray190 commented Mar 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BlackstoneEngineering Apr 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BlackstoneEngineering Apr 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xc0170 commented Apr 3, 2017

mray190 commented Apr 3, 2017

BlackstoneEngineering commented Apr 4, 2017

BlackstoneEngineering commented Apr 4, 2017

Choose a reason for hiding this comment

jeromecoutant commented Apr 5, 2017

BlackstoneEngineering commented Apr 8, 2017

jeromecoutant commented Jun 21, 2017

mray190 commented Jun 22, 2017

jeromecoutant commented Jul 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeromecoutant commented Jul 17, 2017

mray190 commented Jul 17, 2017

BlackstoneEngineering Apr 4, 2017 •

edited

Loading

BlackstoneEngineering Apr 4, 2017 •

edited

Loading