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

add NUCLEO_G031K8 target #13384

Closed
wants to merge 2 commits into from
Closed

add NUCLEO_G031K8 target #13384

wants to merge 2 commits into from

Conversation

pilotak
Copy link
Contributor

@pilotak pilotak commented Jul 31, 2020

Summary of changes

Added support for NUCLEO_G031K8 target

Impact of changes

Migration actions required

Documentation

Based on #13006


Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

At the moment there is a problem with link scripts, but i have no clue how to fix this

c:/programdata/chocolatey/lib/gcc-arm-embedded/tools/bin/../lib/gcc/arm-none-eabi/9.3.1/../../../../arm-none-eabi/bin/ld.exe:./BUILD/NUCLEO_G031K8/GCC_ARM/.link_script.ld:90 cannot move location counter backwards (from 20002540 to 20001c00)
collect2.exe: error: ld returned 1 exit status
[ERROR] c:/programdata/chocolatey/lib/gcc-arm-embedded/tools/bin/../lib/gcc/arm-none-eabi/9.3.1/../../../../arm-none-eabi/bin/ld.exe:./BUILD/NUCLEO_G031K8/GCC_ARM/.link_script.ld:90 cannot move location counter backwards (from 20002540 to 20001c00)
collect2.exe: error: ld returned 1 exit status
[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[x] Tests / results supplied as part of this PR

Reviewers

@0xc0170, @jeromecoutant


@ciarmcom ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Jul 31, 2020
@ciarmcom ciarmcom requested review from 0xc0170, jeromecoutant and a team July 31, 2020 13:00
@ciarmcom
Copy link
Member

@pilotak, thank you for your changes.
@jeromecoutant @0xc0170 @ARMmbed/mbed-os-maintainers please review.

@AGlass0fMilk
Copy link
Member

AGlass0fMilk commented Jul 31, 2020

The linker error indicates to me that the program you're trying to build does not fit in the small memory size of the NUCLEO_G031K8.

I recall having to build with the baremetal profile to get a small enough output binary that would fit on this target. There are some other targets with similar memory constraints, can't remember which ones at the moment but if you look through the targets folder you should be able to find one. Maybe try looking at some of the STM32F0 or other Cortex-M0(+) targets for examples?

I would suggest building this example: https://github.com/ARMmbed/mbed-os-example-blinky-baremetal

@pilotak
Copy link
Contributor Author

pilotak commented Aug 4, 2020

yes using a baremetal fixes the problem

@jeromecoutant
Copy link
Collaborator

Could you check greentea tests?

@pilotak
Copy link
Contributor Author

pilotak commented Aug 11, 2020

@jeromecoutant I know this is a silly question but how am i going to run greentea tests? I did discuss it with @AGlass0fMilk and he doesn't know either

@jeromecoutant
Copy link
Collaborator

mbed test -m NUCLEO_G031K8 -t ARM -v --test-config tests/configs/baremetal.json ?

@pilotak
Copy link
Contributor Author

pilotak commented Aug 11, 2020

here is the result - hard for me judge if it passed or failed
G031K8.txt

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 11, 2020

@pilotak Are you running these tests from an app (there are linker errors about main defined 2x) ? All tests failed to build in the log

@AGlass0fMilk
Copy link
Member

@0xc0170 Can you reopen #13006? I have updated my branch.

@AGlass0fMilk
Copy link
Member

@jeromecoutant I can't get my Nucleo G031K8 ST Link to cooperate with testing. It times out constantly and was very difficult to even get to connect to the target.

🤷‍♂️

@jeromecoutant
Copy link
Collaborator

Same on my side... :-(
I escalated the issue internally...

@pilotak
Copy link
Contributor Author

pilotak commented Aug 11, 2020

I deleted the app and here is the result
updated_G031K8.txt

@jeromecoutant
Copy link
Collaborator

Note that as long as ARMmbed/mbed-os-tools#235
is not part of your config, you can use this command:
mbedls -m 0852:NUCLEO_G031K8

@pilotak
Copy link
Contributor Author

pilotak commented Aug 12, 2020

is it possible to test without real nucleo board? i have custom a PCB with serial and SWD breakout

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 12, 2020

@0xc0170 Can you reopen #13006? I have updated my branch.

I reopened it, what's the next? As this one is still opened as well.

@pilotak
Copy link
Contributor Author

pilotak commented Aug 12, 2020

@0xc0170 in #13006 there are still a few things to be fix, this PR has everything up to date. Also @AGlass0fMilk has a rights to push into this PR it makes no sense to have two PRs opened

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 12, 2020

@AGlass0fMilk Please close the other one and let's collaborate here

@jeromecoutant
Copy link
Collaborator

@AGlass0fMilk
It seems that we got some production issue...

You can solve the issue by using ST programmer tool: https://www.st.com/en/development-tools/stm32cubeprog.html
Open the Option Bytes (OB) page,
in User Configuration, NRST_mode is currently set to 2
=> change value to 3, and Apply

Copy link
Collaborator

@jeromecoutant jeromecoutant left a comment

Choose a reason for hiding this comment

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

Good job!
Please check my review commit: jeromecoutant@cd6373b
Thx

@@ -3074,7 +3074,7 @@ __STATIC_INLINE void LL_RTC_BKP_SetRegister(TAMP_TypeDef *TAMPx, uint32_t Backup
*/
__STATIC_INLINE uint32_t LL_RTC_BKP_GetRegister(TAMP_TypeDef *TAMPx, uint32_t BackupRegister)
{
const register __IO uint32_t *tmp;
register __IO uint32_t *tmp;
Copy link
Collaborator

Choose a reason for hiding this comment

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

To remove

@AGlass0fMilk
Copy link
Member

To be honest I’m a bit irked that my original commits for this target are omitted from this PR. It’s not good git practice to rewrite history like that. Also it was quite a bit of work getting it that far...

I’ll close my original PR.

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 12, 2020

To be honest I’m a bit irked that my original commits for this target are omitted from this PR. It’s not good git practice to rewrite history like that. Also it was quite a bit of work getting it that far...

Fair point. Did not spot the changed history here.
@pilotak could you help here to either get it close to what it was, or at least if its just "add new target" - one commit - co author it please (in the description you can reference previous PR and write details as well).

@pilotak
Copy link
Contributor Author

pilotak commented Aug 13, 2020

The main reason behind this PR is that the original one was opened in May and for two months nobody made requested changes, maybe due to lack of time, covid, etc. it can happen i'm fine with that.
I did have a time so i wanted to push it further and get it merged ASAP because I needed it for my custom board but due to lack of rights to fix those requested changes and fix the one i found, i opened this one.

I have no intention to take the author credits whatsoever

@pilotak pilotak closed this Aug 13, 2020
@mergify mergify bot removed needs: review release-type: patch Indentifies a PR as containing just a patch scope: new-target labels Aug 13, 2020
@0xc0170
Copy link
Contributor

0xc0170 commented Aug 13, 2020

Also this PR is closed now 😞

@0xc0170 in #13006 there are still a few things to be fix, this PR has everything up to date. Also @AGlass0fMilk has a rights to push into this PR it makes no sense to have two PRs opened

@pilotak I thought we agreed on proceeding here. Both of you took an effort to push this to this stage, let's keep the boat moving please. Shall we reopen it ? I can fix the commits in this PR (if I have write access).

@pilotak
Copy link
Contributor Author

pilotak commented Aug 13, 2020

@0xc0170 i know we did agree but that was before @AGlass0fMilk got irked. I would suggest let him decide, it's his work

@AGlass0fMilk
Copy link
Member

@pilotak I appreciate you continuing my work and am glad you may have found it useful in Mbed-enabling your new custom target. There's no hard feelings.

If you're not familiar with this process, the best way to go about finishing/extending someone else's branch on GitHub would be to do the following steps:

  1. Fork the repository to your account (which you have: https://github.com/pilotak/mbed-os)
  2. Clone your fork locally: git clone https://github.com/pilotak/mbed-os.git
  3. Go to the fork of the user who's work you want to continue (my fork in this case: https://github.com/AGlass0fMilk/mbed-os)
  4. Add the other user's fork repo as another remote in your local repository:
    git remote add milks-fork https://github.com/AGlass0fMilk/mbed-os.git
  5. Check out the branch you want to continue, in this case my feature branch was named add-nucleo-g031k8:
    git fetch milks-fork && git checkout milks-fork/add-nucleo-g031k8

Now you can make any changes you need to make and push the branch up to your fork. You can either open a new PR (if there wasn't one originally) or submit a PR to my fork on the feature branch. It's the same as contributing to mbed-os itself.

In this case both of our original commits are retained in history, which is important both for proper credit and when looking at git blame in the future if/when any bugs arise from code we contributed.

Would you want to check out my branch as above, apply the changes you made here, and then submit a PR to my mbed-os fork? I can then open #13006 and we can go from there.

@AGlass0fMilk
Copy link
Member

I will add that in some cases trying to submit a PR to someone's fork can be fruitless if the user doesn't seem to be very active.

I work with Mbed on a daily basis both professionally and as a hobbyist so I am usually pretty responsive. Some of my PRs aren't work-related however (adding Nucleo G031K8 being one of them) and I don't always get the time to update them...

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.

6 participants