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.

Sign up for GitHub

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

Jump to bottom

STM32_gen_PeripheralPins.py : TargetName correction #14594

Merged
merged 1 commit into from
May 3, 2021
Merged

STM32_gen_PeripheralPins.py : TargetName correction #14594

merged 1 commit into from
May 3, 2021

Conversation

jeromecoutant
Copy link
Collaborator

Summary of changes

Fixes #14590

@leichunyang

Impact of changes

Migration actions required

Documentation

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 

[x] 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

Reviewers

@jeromecoutant jeromecoutant mentioned this pull request Apr 26, 2021
Closed
@ciarmcom ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Apr 26, 2021
@ciarmcom
Copy link
Member

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

@ciarmcom ciarmcom requested a review from a team April 26, 2021 13:30
0xc0170
0xc0170 reviewed Apr 26, 2021
View reviewed changes
targets/TARGET_STM/tools/STM32_gen_PeripheralPins.py Outdated
@@ -83,6 +83,7 @@
MCU_USERNAME= ""
TIM_MST = ""
ALTERNATE_DEFINITION = 0
TargetName = ""
Copy link
Contributor

Choose a reason for hiding this comment

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

The rest are in capital letters, why not TargetName?

Why it was actually put into global scope?

Is this fixing -m option as issue reported ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To be honest, I don't follow any strict coding rules for my personal script :-)
About the issue, it came since the addition of the target name in the PinNames.h file.

I have added it, but script can also create mbed files from only a MCU
So script is now keeping the empty field, up to user to manually update it.

Copy link
Contributor

Choose a reason for hiding this comment

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

To be honest, I don't follow any strict coding rules for my personal script :-)

Just to be consistent , it would be TARGET_NAME

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@0xc0170 done

@ciarmcom ciarmcom requested a review from a team April 26, 2021 14:00
@ciarmcom
Copy link
Member

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

@mergify mergify bot added needs: CI and removed needs: review labels Apr 27, 2021
@0xc0170
Copy link
Contributor

0xc0170 commented Apr 30, 2021

CI started

@mbed-ci
Copy link

mbed-ci commented Apr 30, 2021

Jenkins CI Test : ✔️ SUCCESS

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

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_cmake-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_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 99cf33c into ARMmbed:master May 3, 2021
@mergify mergify bot removed the ready for merge label May 3, 2021
@jeromecoutant jeromecoutant deleted the PR_GEN_MCU branch May 3, 2021 07:59
@JojoS62
Copy link
Contributor

JojoS62 commented May 3, 2021

I'm missing some standard names in 6.10.0 for DISCO_F469NI like:
I2C_SCL = PB_8,
I2C_SDA = PB_9,
SPI_MOSI = PB_15,
SPI_MISO = PB_14,
SPI_SCK = PD_3,
SPI_CS = PH_6,

have these been renamed or is it still a problem with the automatic generation?

@jeromecoutant
Copy link
Collaborator Author

Hi
Not related to this PR, but I can answer :-)

These aliases are defined on arduino pins, and are now defined in:
https://github.com/ARMmbed/mbed-os/blob/master/hal/include/hal/PinNameAliases.h

I think you are right... I should add back
#define I2C_SDA ARDUINO_UNO_I2C_SDA
#define I2C_SCL ARDUINO_UNO_I2C_SCL
etc....

@MarceloSalazar

@JojoS62
Copy link
Contributor

JojoS62 commented May 3, 2021

Thanks, yes, otherwise it will break a lot of projects with this incompatible change.

@0xc0170
Copy link
Contributor

0xc0170 commented May 3, 2021

@jeromecoutant pull request incoming? or still worth creating an issue for this ?

@jeromecoutant
Copy link
Collaborator Author

Agree

@JojoS62 please raise a new issue

I will push a global PR for all STM targets

@jeromecoutant jeromecoutant mentioned this pull request May 17, 2021
Closed
@mbedmain mbedmain removed the release-type: patch Indentifies a PR as containing just a patch label May 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xc0170 0xc0170 0xc0170 approved these changes

Assignees
No one assigned
Labels
devices: st release-version: 6.11.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

STM32_gen_PeripheralPins.py, V1.20 doesn't work with -m option
6 participants
@jeromecoutant @ciarmcom @0xc0170 @mbed-ci @JojoS62 @mbedmain