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

Support for the NUCLEO_G0B1RE board #15199

Merged
merged 1 commit into from
Jan 17, 2022
Merged

Conversation

majcher
Copy link
Contributor

@majcher majcher commented Jan 6, 2022

Summary of changes

Support for the NUCLEO_G0B1RE board

Documentation

None


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

I need some help here.

mbed test -m NUCLEO_G0B1RE -t GCC_ARM

provides an info that the new target is not supported, but when I do the same changes with custom_targets.json it works fine for sample app.

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

@ciarmcom ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Jan 6, 2022
@ciarmcom ciarmcom requested a review from a team January 6, 2022 20:30
@ciarmcom
Copy link
Member

ciarmcom commented Jan 6, 2022

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

@jeromecoutant
Copy link
Collaborator

I need some help here.
mbed test -m NUCLEO_G0B1RE -t GCC_ARM
provides an info that the new target is not supported, but when I do the same changes with custom_targets.json it works fine for sample app.

This is OK on my side

Note that we need to update local mbedls database:
mbedls -m 0872:NUCLEO_G0B1RE

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.

Very good!

Maybe I can suggest to try this command:
python targets/TARGET_STM/tools/STM32_gen_PeripheralPins.py -t NUCLEO-G0B1RE

to verify PeripheralPins.c and PinNames.h

Comment on lines 2877 to 2880
"mbed_rom_start": "0x08000000",
"mbed_rom_size": "0x80000",
"mbed_ram_start": "0x20000000",
"mbed_ram_size": "0x24000",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe this part could be removed ?
(same info available with "device_name": "STM32G0B1RETx")

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, thanks, it works without it now. I added it when I had a non-matching device name with an index.json file and there were some errors related to these attributes.

@majcher
Copy link
Contributor Author

majcher commented Jan 9, 2022

Very good!

Maybe I can suggest to try this command: python targets/TARGET_STM/tools/STM32_gen_PeripheralPins.py -t NUCLEO-G0B1RE

to verify PeripheralPins.c and PinNames.h

Thanks! I used the generated ones. I only had to fill in the TARGET_FF_ARDUINO_UNO and BUTTON1 as these were defined with Px_x. Button works correctly, not sure about ARDUINO mapping - I took it from the TARGET_NUCLEO_G071RB.

0xc0170
0xc0170 previously approved these changes Jan 10, 2022
@0xc0170
Copy link
Contributor

0xc0170 commented Jan 10, 2022

This is OK on my side

All tests are passing @jeromecoutant ?

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 10, 2022

CI started

@jeromecoutant
Copy link
Collaborator

All tests are passing ?

Please @majcher maybe you can add a test report ?

@mergify mergify bot added needs: CI and removed needs: review labels Jan 10, 2022
@majcher
Copy link
Contributor Author

majcher commented Jan 10, 2022

This is OK on my side

All tests are passing @jeromecoutant ?

Do you mean:
mbed test -m NUCLEO_G0B1RE -t GCC_ARM
?

I cannot get it running as it keeps printing that the device is not supported, even after adding to mbedls locally.

argument -m/--mcu: NUCLEO_G0B1RE is not a supported MCU. Supported MCUs are:...

mbedls

| platform_name | platform_name_unique | mount_point         | serial_port            | target_id                | interface_version |
|---------------|----------------------|---------------------|------------------------|--------------------------|-------------------|
| NUCLEO_G0B1RE | NUCLEO_G0B1RE[0]     | /Volumes/NOD_G0B1RE | /dev/tty.usbmodem14403 | 08720221142763682B63FA27 | V2J37M27          |

Is there anything else we need to do in order to get it supported like other targets?

@mbed-ci
Copy link

mbed-ci commented Jan 10, 2022

Jenkins CI Test : ✔️ SUCCESS

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_build-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_greentea-test ✔️

@jeromecoutant
Copy link
Collaborator

jeromecoutant commented Jan 10, 2022

Do you mean: mbed test -m NUCLEO_G0B1RE -t GCC_ARM ?

Is there anything else we need to do in order to get it supported like other targets?

To be honest, I don't understand why this is working for you...
I cherry-picked your patch and copy paste your test command, it is OK...

@majcher
Copy link
Contributor Author

majcher commented Jan 10, 2022

Do you mean: mbed test -m NUCLEO_G0B1RE -t GCC_ARM ?
Is there anything else we need to do in order to get it supported like other targets?

To be honest, I don't understand why this is working for you... I cherry-picked your patch and copy paste your test command, it is OK...

Thanks. Is there anything left here or are we're good to merge?

@jeromecoutant
Copy link
Collaborator

But why this doesn't work for you... ?

PS: I made some tests on my side, I can make a PR, but maybe you could add a new line ther:
https://github.com/ARMmbed/mbed-os/blob/master/targets/TARGET_STM/TARGET_STM32G0/flash_api.c#L60
EraseInitStruct.Banks = FLASH_BANK_1;

Thx

@mergify mergify bot dismissed 0xc0170’s stale review January 10, 2022 17:11

Pull request has been modified.

@majcher
Copy link
Contributor Author

majcher commented Jan 10, 2022

But why this doesn't work for you... ?

Good question. Where should the .mbedls-mock file be located - the root of the mbed-os project? I tried here and also user home, but still it doesn't detect it when I run mbed test. mbedls works file, when the mock file is in the current folder.

I'm running:

➜  ~ mbedls --version
1.8.8

➜  ~ mbed --version
1.10.5

PS: I made some tests on my side, I can make a PR, but maybe you could add a new line ther: https://github.com/ARMmbed/mbed-os/blob/master/targets/TARGET_STM/TARGET_STM32G0/flash_api.c#L60 EraseInitStruct.Banks = FLASH_BANK_1;

Thx

Done, thanks! One more issue disappeared ;)

0xc0170
0xc0170 previously approved these changes Jan 11, 2022
@mergify mergify bot added the needs: CI label Jan 11, 2022
@majcher
Copy link
Contributor Author

majcher commented Jan 11, 2022

Go in mbed-os directory, and execute in the same place mbedls -m xxx nad mbed test xxxcommands
@jeromecoutant I tried it, without luck, but I'll play with it. Thanks for your help here.

Is there anything else left in the scope of this PR? I have another one in a queue for this board related to CAN support, but I prefer to have this merged first.

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 12, 2022

@majcher almost all is done, just a new target requires to have test results attached as part of the pull request (test proof). Please execute the tests on your board (I hope the problems with ls and test commands could be resolved).

@majcher
Copy link
Contributor Author

majcher commented Jan 13, 2022

@0xc0170 @jeromecoutant
I finally found where the issue was. In ~/.mbed/.mbed there was a path to some other folder of mbed-os, where there where no mbedls mock files. This was causing problems, even when running from correct location. It works after I removed this file.

Here it is:
g0b1re_test_03.log

Unfortunatelly, it seems that there are some issues with Flash as few tests are failing in this area.

I'd be glad if you could hint me on where the problems could be or how to debug this.

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 13, 2022

You can run flashiap to find out where it is failing and debug what is going on and just write an application to debug it in detail.

One way would be to disable flash and create a tracking issue to enable it later once it's fixed.

@jeromecoutant any suggestions?

@jeromecoutant
Copy link
Collaborator

@0xc0170 @jeromecoutant I finally found where the issue was. In ~/.mbed/.mbed there was a path to some other folder of mbed-os, where there where no mbedls mock files. This was causing problems, even when running from correct location. It works after I removed this file.

Yes, .mbed files is a nightmare...

Unfortunatelly, it seems that there are some issues with Flash as few tests are failing in this area.

I see the issue. STM32G0Bxx are dual bank. I am correcting it right now

@majcher
Copy link
Contributor Author

majcher commented Jan 13, 2022

I see the issue. STM32G0Bxx are dual bank. I am correcting it right now

@jeromecoutant thanks! It works now:

mbedgt: test suite report:
| target                | platform_name | test suite                     | result | elapsed_time (sec) | copy_method |
|-----------------------|---------------|--------------------------------|--------|--------------------|-------------|
| NUCLEO_G0B1RE-GCC_ARM | NUCLEO_G0B1RE | hal-tests-tests-mbed_hal-flash | OK     | 20.85              | default     |
mbedgt: test suite results: 1 OK
mbedgt: test case report:
| target                | platform_name | test suite                     | test case                    | passed | failed | result | elapsed_time (sec) |
|-----------------------|---------------|--------------------------------|------------------------------|--------|--------|--------|--------------------|
| NUCLEO_G0B1RE-GCC_ARM | NUCLEO_G0B1RE | hal-tests-tests-mbed_hal-flash | Flash - clock and cache test | 1      | 0      | OK     | 0.54               |
| NUCLEO_G0B1RE-GCC_ARM | NUCLEO_G0B1RE | hal-tests-tests-mbed_hal-flash | Flash - erase sector         | 1      | 0      | OK     | 0.12               |
| NUCLEO_G0B1RE-GCC_ARM | NUCLEO_G0B1RE | hal-tests-tests-mbed_hal-flash | Flash - init                 | 1      | 0      | OK     | 0.52               |
| NUCLEO_G0B1RE-GCC_ARM | NUCLEO_G0B1RE | hal-tests-tests-mbed_hal-flash | Flash - mapping alignment    | 1      | 0      | OK     | 0.05               |
| NUCLEO_G0B1RE-GCC_ARM | NUCLEO_G0B1RE | hal-tests-tests-mbed_hal-flash | Flash - program page         | 1      | 0      | OK     | 0.14               |
mbedgt: test case results: 5 OK
mbedgt: completed in 21.83 sec

I'll rebase this PR after your changes are merged.

@mergify mergify bot added the needs: work label Jan 14, 2022
@mergify
Copy link

mergify bot commented Jan 14, 2022

This PR cannot be merged due to conflicts. Please rebase to resolve them.

@mergify mergify bot removed the needs: CI label Jan 14, 2022
@0xc0170
Copy link
Contributor

0xc0170 commented Jan 14, 2022

Awesome ! #15204 was merged, please rebase and we start CI soon.

@mergify mergify bot dismissed 0xc0170’s stale review January 14, 2022 12:05

Pull request has been modified.

@majcher
Copy link
Contributor Author

majcher commented Jan 14, 2022

Rebased - now all tests are 'green':
mbedgt: test case results: 662 OK

a full report below:
g0b1re_test_05.log

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 17, 2022

CI restarted

@mbed-ci
Copy link

mbed-ci commented Jan 17, 2022

Jenkins CI Test : ✔️ SUCCESS

Build Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_build-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_greentea-test ✔️

@0xc0170 0xc0170 merged commit de5b459 into ARMmbed:master Jan 17, 2022
@mergify mergify bot removed the ready for merge label Jan 17, 2022
@mbedmain mbedmain added release-version: 6.16.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Jun 14, 2022
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.

6 participants