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

Added conconcurrent testing #62

Merged
merged 8 commits into from
Aug 7, 2017
Merged

Conversation

brandonboesch
Copy link
Contributor

GPIO test runs DIO, AnalogIn, and InterruptIn sequentially in one test case.

@0xc0170
Copy link
Collaborator

0xc0170 commented Jul 21, 2017

This module (this patch) should follow the mbed style guideline. We should align it.

Plus where is this test described what it does, what's expected output?

@brandonboesch
Copy link
Contributor Author

brandonboesch commented Jul 21, 2017

@0xc0170 , I didn't know there was an mbed style guideline, can you point me to that? Also, where are we supposed to be documenting what the test is doing, and the expected output? Thanks.

@0xc0170
Copy link
Collaborator

0xc0170 commented Jul 21, 2017

I didn't know there was an mbed style guideline, can you point me to that?

https://docs.mbed.com/docs/mbed-os-handbook/en/latest/cont/code_style/

Also, where are we supposed to be documenting what the test is doing, and the expected output?

I believe for this one we will need to start doing it. We got few cases in mbed OS where above the test case, we describe the test (what is testing) in the comments

@mray190 mray190 self-requested a review July 21, 2017 14:49
Copy link
Contributor

@mray190 mray190 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@0xc0170 - this feature was requested by @screamerbg in issue #57

@brandonboesch - Tests need to be concurrent instead of sequential in the same test case as we discussed

@brandonboesch brandonboesch changed the title Added conconcurrent/GPIO test Added conconcurrent testing Aug 4, 2017
Copy link
Contributor

@mray190 mray190 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once you have the checks for AnalogIn, I think this is good to go!

using namespace utest::v1;

volatile bool Result = false;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a check to see if AnalogIn is enabled before running this test

Multi_Thread_ID = Thread::gettid(); // update thread id for this thread
Thread_I2C.start(callback(test_I2C)); // kick off threads
Thread_SPI.start(callback(test_SPI)); // kick off threads
wait(0.1); // allow time for debug print statements to complete.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need this wait? Since it is waiting for threads to set their signal I do not believe you need the wait

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, both threads could complete their work, and the test could exit before the debug print finishes its printing.

#error [NOT_SUPPORTED] I2C not supported on this platform, add 'DEVICE_I2C' definition to your platform.

#endif // !DEVICE_SPI or !DEVICE_I2C

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here for AnalogIn

@screamerbg
Copy link
Contributor

LGTM. @brandonboesch thanks for this! Please check @mray19027's comments.

@mray190 mray190 merged commit f8cb240 into ARMmbed:HW2 Aug 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants