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

Adding support for SDP-K1. #10000

Merged
merged 9 commits into from
Mar 27, 2019
Merged

Adding support for SDP-K1. #10000

merged 9 commits into from
Mar 27, 2019

Conversation

malavikasajikumar
Copy link
Contributor

@malavikasajikumar malavikasajikumar commented Mar 7, 2019

Description

This pull request contains files to support SDP-K1 board on Mbed. This board uses STM32F469NI Cortex-M4 processor.

Pull request type

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

Reviewers

Release Notes

@ciarmcom ciarmcom requested review from screamerbg and a team March 7, 2019 22:00
@ciarmcom
Copy link
Member

ciarmcom commented Mar 7, 2019

@malavikasajikumar, thank you for your changes.
@screamerbg @ARMmbed/mbed-os-maintainers please review.

@ghost
Copy link

ghost commented Mar 7, 2019

Conditionally Approved for 5.12 - if risk is low and no side effects to other targets, this is approved.
(Also congrats on # 10,000)

@cmonr cmonr requested a review from a team March 8, 2019 00:58
@cmonr
Copy link
Contributor

cmonr commented Mar 8, 2019

@malavikasajikumar Please update the description.

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 8, 2019

Adding support for SDP-K1. #10000

💯x💯 👍 🎉 🍾

@cmonr What description should be updated?

entire PR template - PR description and PR type - provide information about what are you introducing

Copy link
Member

@bulislaw bulislaw left a comment

Choose a reason for hiding this comment

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

Please remove the dead (commented out) code and provide test results for all 3 compilers.

targets/targets.json Outdated Show resolved Hide resolved
targets/targets.json Outdated Show resolved Hide resolved
@0xc0170
Copy link
Contributor

0xc0170 commented Mar 8, 2019

New target needs pasting test logs for all 3 toolchains, please attach them to the pull request

Besides these minor comments, LGTM

@malavikasajikumar
Copy link
Contributor Author

What is causing this CI test failure?
https://travis-ci.org/ARMmbed/mbed-os/jobs/503729968

Fixing a few more alignment issues.
@malavikasajikumar
Copy link
Contributor Author

malavikasajikumar commented Mar 11, 2019

@cmonr @0xc0170 @bulislaw

Test log files for GCC_ARM. Separately attached the log file for mbed-os-tests-mbedmicro-rtos-mbed-mutex timeout.

SDP-K1_GCC-ARM.log
SDP-K1_GCC-ARM_rtos-mbed-mutex.log

Test log files

SDP-K1_ARM.log
SDP-K1_ARM_mbed_platform-stream.log
SDP-K1_ARM_mbed_platform-critical_section.log

@malavikasajikumar
Copy link
Contributor Author

Moved back for RC2 to make sure this isn't lost.

If it can pass testing in time, it can make it into RC2. Otherwise, we can look at a branch or other alternatives.

CC @bentcooke

Test log files uploaded.

@cmonr
Copy link
Contributor

cmonr commented Mar 12, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Mar 12, 2019

Test run: FAILED

Summary: 3 of 9 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-GCC_ARM
  • jenkins-ci/mbed-os-ci_build-IAR8
  • jenkins-ci/mbed-os-ci_build-ARMC6

@@ -3694,6 +3694,42 @@
"device_name": "STM32F469NI",
"bootloader_supported": true
},
"SDP-K1": {
Copy link
Contributor

Choose a reason for hiding this comment

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

SDP_K1 should be the name (as any other target). The failure in the build might be related (as it cant find the target) - this could be a bug in our tools or rather unsupported feature

cc @ARMmbed/mbed-os-tools

Copy link
Contributor Author

@malavikasajikumar malavikasajikumar Mar 12, 2019

Choose a reason for hiding this comment

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

Would that be a cause of CI test failures? The name with hyphen is consistent across all the supporting files. @bentcooke

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe yes. You should be able to reproduce the build failure locally. To be consistent, it should use _ instead. (review any other targets). This follows also directory naming TARGET_ and others.

Copy link
Contributor Author

@malavikasajikumar malavikasajikumar Mar 15, 2019

Choose a reason for hiding this comment

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

@0xc0170 @bentcooke Fixed the hyphen to underscore, ran the tests on IAR toolchain.

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 12, 2019

Moving back to 5.12.1, 5.12.0-rc2 is now ready and going to be released

@malavikasajikumar
Copy link
Contributor Author

Moving back to 5.12.1, 5.12.0-rc2 is now ready and going to be released

Guess we will have to go with a separate branch then, that can be imported into mbed compiler. @cmonr

@theotherjimmy
Copy link
Contributor

@malavikasajikumar Please don't use another branch (it would have to be a fork, FYI) to import into the online compiler. Mbed OS is cached and this strategy skips the cache and costs ARM GB of storage for every import.

@malavikasajikumar
Copy link
Contributor Author

Fixed the issues. Attached are the test log files. @0xc0170 @bulislaw @ashok-rao @bentcooke Could you approve the changes?

SDP_K1_ARM_test.log

SDP_K1_IAR_test.log
SDP_K1_IAR_test_TIMEOUTS.log

SDP_K1_GCC_ARM_test.log
SDP_K1_GCC_ARM_test_TIMEOUTS.log

@bulislaw
Copy link
Member

We can't merge the platform support with failing tests. Can you please debug it to get to the bottom of the issues?

@malavikasajikumar
Copy link
Contributor Author

malavikasajikumar commented Mar 22, 2019

We can't merge the platform support with failing tests. Can you please debug it to get to the bottom of the issues?

The tests that timed out were run again and were successful. The second log files (SDP_K1_IAR_test_TIMEOUTS.log and SDP_K1_GCC_ARM_test_TIMEOUTS.log) shows these tests succeeding. Could you confirm? @bulislaw

@malavikasajikumar
Copy link
Contributor Author

Thank you for the approval. @bulislaw

@ashok-rao @0xc0170 Please let us know if there are anymore changes/updates needed.

@cmonr
Copy link
Contributor

cmonr commented Mar 26, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Mar 26, 2019

Test run: SUCCESS

Summary: 13 of 13 test jobs passed
Build number : 2
Build artifacts

@cmonr cmonr dismissed 0xc0170’s stale review March 26, 2019 16:30

Changes addressed

cmonr pushed a commit to cmonr/mbed-os that referenced this pull request Mar 26, 2019
@cmonr cmonr merged commit b872d08 into ARMmbed:master Mar 27, 2019
@malavikasajikumar
Copy link
Contributor Author

Thank you! @cmonr @bentcooke @0xc0170 @bulislaw

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants