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

Check in files for the FPGA CI Test Shield #10540

Merged
merged 5 commits into from May 24, 2019

Conversation

@c1728p9
Copy link
Contributor

commented May 7, 2019

Description

Bring all the FPGA CI Test Shield C and C++ driver files into mbed-os as the component FPGA_CI_TEST_SHIELD. When this component is enabled all the files that are needed to communicate with, update firmware on and run testing with the FPGA are built.

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[x] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@mrcoulter45 @maciejbocianski @mprse @fkjagodzinski @0xc0170 @jamesbeyond @donatieng @OPpuolitaival

Release Notes

Added files to control the FPGA CI Test Shield. This allows for enhanced testing.

Check in files for the FPGA CI Test Shield
Bring all the FPGA CI Test Shield C and C++ driver files into mbed-os
as the component FPGA_CI_TEST_SHIELD. When this component is enabled
all the files that are needed to communicate with, update firmware on
and run testing with the FPGA are built.

@ciarmcom ciarmcom requested review from 0xc0170, donatieng, fkjagodzinski, jamesbeyond, maciejbocianski, mprse, mrcoulter45, OPpuolitaival and ARMmbed/mbed-os-maintainers May 7, 2019

@ciarmcom

This comment has been minimized.

Remove inclusion of mbed.h from FPGA
And fix any build errors this caused.
@0xc0170
Copy link
Member

left a comment

initial review comments

#include "platform/Callback.h"
#include "drivers/DigitalInOut.h"

class MbedTester {

This comment has been minimized.

Copy link
@0xc0170

0xc0170 May 8, 2019

Member

would it be worth documenting this class (for instance as SPI class has some example and description). Isn't this the most important object for this shield?

This comment has been minimized.

Copy link
@c1728p9

c1728p9 May 8, 2019

Author Contributor

I added some documentation and example code.

std::list<PortType> matched_ports, not_matched_ports;
find_ports<PortType, FormFactorType>(matched_ports, not_matched_ports);

utest_printf("***Testing one %s pin configuration***\n", PortType::PinMap::name);

This comment has been minimized.

Copy link
@0xc0170

0xc0170 May 8, 2019

Member

why do we use utest_printf and not stdout?

This comment has been minimized.

Copy link
@c1728p9

c1728p9 May 8, 2019

Author Contributor

Since this is a test utility shouldn't it use the utest_printf? @maciejbocianski can you comment on this?

This comment has been minimized.

Copy link
@maciejbocianski

maciejbocianski May 9, 2019

Member

Exactly, this utility code is intended to be used in tests only, this is why utest_printf was used

}
};

#if DEVICE_SPI

This comment has been minimized.

Copy link
@0xc0170

0xc0170 May 8, 2019

Member

Can we describe here with a comment what these peripherals utils mean (if I add here, what should I add? ) - small description would help

This comment has been minimized.

Copy link
@c1728p9

c1728p9 May 8, 2019

Author Contributor

I added documentation to this.

return true;
}

class DefaultFormFactor {

This comment has been minimized.

Copy link
@0xc0170

0xc0170 May 8, 2019

Member

I've noticed these utils are not much documented in this header file - what this defaultformfactor does ?

This comment has been minimized.

Copy link
@c1728p9

c1728p9 May 8, 2019

Author Contributor

I added documentation to this.

Run astyle on all FPGA files
Run astyle to correct formatting.

@c1728p9 c1728p9 force-pushed the c1728p9:fpga_ci_test_shield branch from 1fd7b1a to 45301ea May 8, 2019

Add missing documentation
Add documentation to the MbedTester class and the test_utils.h file.
@c1728p9

This comment has been minimized.

Copy link
Contributor Author

commented May 9, 2019

Mbed OS CI won't be able to test this until the component FPGA_CI_TEST_SHIELD is added. To get some testing in I created a test branch where this is no longer a component and therefore built as part of CI and manually triggered a CI job on this branch.

@0xc0170

This comment has been minimized.

Copy link
Member

commented May 12, 2019

I've started first test job for this.

@0xc0170

This comment has been minimized.

Copy link
Member

commented May 13, 2019

internal CI fault, restarting IC

@mbed-ci

This comment has been minimized.

Copy link

commented May 13, 2019

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_greentea-test
@adbridge

This comment has been minimized.

Copy link
Contributor

commented May 17, 2019

CI started

@mbed-ci

This comment has been minimized.

Copy link

commented May 17, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 3
Build artifacts

@0xc0170 0xc0170 requested a review from bulislaw May 21, 2019

@0xc0170

This comment has been minimized.

Copy link
Member

commented May 21, 2019

It is almost ready, any more reviews done ?

Mbed OS CI won't be able to test this until the component FPGA_CI_TEST_SHIELD is added. To get some testing in I created a test branch where this is no longer a component and therefore built as part of CI and manually triggered a CI job on this branch.

@OPpuolitaival Can this be added?

@bulislaw
Copy link
Member

left a comment

Looks good. One thing I'd like to see is docs how to use it. I know that HW is not yet publicly available, but we'll need the docs soon anyway.

@mprse

mprse approved these changes May 23, 2019

Copy link
Member

left a comment

👍

Bring in more changes from FPGA repo
Bring in updates the the FPGA CI Test Shield repo.
@c1728p9

This comment has been minimized.

Copy link
Contributor Author

commented May 24, 2019

Added some changes that came in since this PR was created. This should be ready for re-test and merge.

@0xc0170

This comment has been minimized.

Copy link
Member

commented May 24, 2019

Ci restarted

@0xc0170

This comment has been minimized.

Copy link
Member

commented May 24, 2019

@maciejbocianski @c1728p9 Please make sure there is documentation added for this one. This is ready for merging once CI is done

@mbed-ci

This comment has been minimized.

Copy link

commented May 24, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 4
Build artifacts

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels May 24, 2019

@0xc0170 0xc0170 merged commit a4738fa into ARMmbed:master May 24, 2019

26 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/build-ARM Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage Success
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARM Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Success!
Details
travis-ci/docs Success!
Details
travis-ci/doxy-spellcheck Success!
Details
travis-ci/events Success! Runtime is 8783 cycles.
Details
travis-ci/gitattributestest Success!
Details
travis-ci/include_check Success!
Details
travis-ci/licence_check Success!
Details
travis-ci/littlefs Success! Code size is 8448B.
Details
travis-ci/psa-autogen Success!
Details
travis-ci/tools-py2.7 Success!
Details
travis-ci/tools-py3.5 Success!
Details
travis-ci/tools-py3.6 Success!
Details
travis-ci/tools-py3.7 Success!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.