Skip to content

ncs36510 from ON Semiconductor #2531

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

Merged
merged 6 commits into from
Aug 27, 2016
Merged

Conversation

radhika-raghavendran
Copy link
Contributor

Added support for NCS36510 in mbed-os. GCC, IAR and ARM toolchains are supported. All tests pass.

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 24, 2016

Hi,

can you please share the test results? Please rebase, there is a conflict with master.

cc @sg- @screamerbg - new target

@@ -37,7 +37,7 @@
GCC_CR_PATH = "C:/code_red/RedSuite_4.2.0_349/redsuite/Tools/bin"

# IAR
IAR_PATH = "C:/Program Files (x86)/IAR Systems/Embedded Workbench 7.3/arm"
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be changed here.

@radhika-raghavendran
Copy link
Contributor Author

Attached are the test reports for IAR, ARM and ARM_GCC. Please confirm merge capability.

TestResults-ARM-26-08-2016.txt
TestResults-GCC-26-08-2016.txt
TestResults-IAR-26-08-2016.txt

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 26, 2016

Something went wrong with a rebase, run again rebase master to get rid of the commits that are already on master (they are now displayed here)

@radhika-raghavendran
Copy link
Contributor Author

I have rebased. It now says that the master is upto date.

@c1728p9
Copy link
Contributor

c1728p9 commented Aug 26, 2016

It looks like this isn't showing up correctly since 051e608 is a merge rather than a rebase. If you did an interactive rebase and then force pushed back that should clean this up. To do this you need to run commands similar to this:
git checkout master
git rebase -i 6a1208af084645213d4282d7a863e57ccc7b27e2
You'll be prompted with window with several files. Remove the ones from the merge:

pick b75379f Adding NCS36510 support in mbed-os5.1
pick f5f8deb heap and stack test
pick daea440 Adding NCS36510 support in mbed-os5.1
pick db54c21 heap and stack test
pick 3a995aa Adding NCS36510 support in mbed-os5.1          <- delete this line
pick 5e01aea heap and stack test                            <- delete this line
pick 31e1b61 Adding NCS36510 support in mbed-os5.1          <- delete this line
pick c0a082c heap and stack test                            <- delete this line
pick dd1ae51 rebasing with mbed-os latest
pick 54df4ab Formatting code according to ARM guidelines. Ran pylint and astyle. Rebased latest ARMmbed-os code.

If the rebase succeeds then force push the commits back up
git push origin master -f

@radhika-raghavendran
Copy link
Contributor Author

@c1728p9 thanks for the help. The rebase was successful and I was able to force push the changes.
This is the result of the merge
$>git checkout master
Already on 'master'
Your branch is up-to-date with 'origin/master'.

$>git rebase -i 6a1208a
Successfully rebased and updated refs/heads/master.

$>git push origin master -f
Counting objects: 79, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (78/78), done.
Writing objects: 100% (79/79), 25.29 KiB | 0 bytes/s, done.
Total 79 (delta 74), reused 0 (delta 0)
remote: Resolving deltas: 100% (74/74), completed with 71 local objects.
To https://github.com/radhika-raghavendran/mbed-os5.1-onsemi

  • 54df4ab...2dc3806 master -> master (forced update)

Please let me know if there is anything more that I need to do.
Thank you.

@c1728p9
Copy link
Contributor

c1728p9 commented Aug 26, 2016

Hi @radhika-raghavendran, it looks like the rebase was successful. Thanks for taking care of this. The PR looks read to merge to me.

@sg- sg- removed the needs: review label Aug 26, 2016
@sg-
Copy link
Contributor

sg- commented Aug 26, 2016

/morph test

@mbed-bot
Copy link

Result: ABORTED

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

/morph test

Output

mbed Build Number: 725

Build failed!

@bridadan
Copy link
Contributor

I had to abort the test temporarily as some infrastructure changes are going through right now, I'll retrigger them after I've verified things are ready to go.

@bridadan
Copy link
Contributor

/morph test

@c1728p9
Copy link
Contributor

c1728p9 commented Aug 26, 2016

@mbed-bot: TEST

HOST_OSES=ALL
BUILD_TOOLCHAINS=GCC_ARM
TARGETS=K64F,NRF51_DK,NRF51_MICROBIT,NUCLEO_F411RE

@mbed-bot
Copy link

[Build 851]
FAILURE: Something went wrong when building and testing.

@mbed-bot
Copy link

[Build ${MBED_BUILD_ID}]
FAILURE: Something went wrong when building and testing.

@bridadan
Copy link
Contributor

@mbed-bot: TEST

HOST_OSES=ALL
BUILD_TOOLCHAINS=GCC_ARM
TARGETS=K64F,NRF51_DK,NRF51_MICROBIT,NUCLEO_F411RE,NCS36510

@mbed-bot
Copy link

Result: FAILURE

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

/morph test

Output

mbed Build Number: 734

Test failed!

@mbed-bot
Copy link

[Build 856]
SUCCESS: Building succeeded and tests were run! Be sure to check the test results

@bridadan
Copy link
Contributor

Morph tests look ok, all failing due to a known issue measuring timing-based tests on host PCs. 👍

@sg- LGTM 👍

@sg- sg- merged commit ea56684 into ARMmbed:master Aug 27, 2016
@danclement
Copy link
Contributor

danclement commented Aug 29, 2016

I just tried importing the mbed-os-example-blinky and compiling for the target NCS36510 and it did not work. I get the following:

[mbed] ERROR: "python" returned error code 2.
[mbed] ERROR: Command "python -u C:\Users\ffwxjw\Documents\GitHub\mbed-os-example-linky\mbed-os\tools\make.py -t GCC_ARM -m NCS36510 --source . --build ..build\NCS36510\GCC_ARM" in "C:\Users\ffwxjw\Documents\GitHub\mbed-os-example-blinky" ---

I was under the impression this should have worked since we are now merged. Any ideas?

@sg-
Copy link
Contributor

sg- commented Aug 29, 2016

@danclement There are a few things that need to happen or you can do to try this out.

  • Merged to master means you can update the example to point at the latest master and build the example. Keep in mind, master is not a release and there may be unstable code when using this branch in an application
  • wait for a release (later this week) when the example will be updated.

@danclement
Copy link
Contributor

Thanks for the clarification Sam.

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 30, 2016

@radhika-raghavendran Please read my comments in #2540, regarding HAL (sleep api.c file not used or enabling/disabling clocks for gpio or TODO/dead code). I did not see them addressed here neither there.

@radhika-raghavendran
Copy link
Contributor Author

@0xc0170 I did fix most of the things mentioned in #2540(removed TODO wherever not required, disable typo in GPIO fixed, removed dead code). We are yet to fix the sleep API from our side. We had asked for clarification about the API since our SoC has 3 low power modes. The issue was raised during our workshop also and it is logged in the docs. Please let us know if there are changes expected at an API level in the near future. If not, we will fix this issue as soon as possible.
Thank you.

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 30, 2016

Please let us know if there are changes expected at an API level in the near future. If not, we will fix this issue as soon as possible.

there's no API extension planned for now. Use the API as it is now, a lot of targets in the current code base provide more than 2 low power modes. Either they provide some logic in the deep_sleep to go to the lowest possible power mode, or select the lowest one possible as default there.

Thanks for the update.

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.

7 participants