Skip to content

[STM32F7] Modify folder structure #3649

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
Merged
merged 1 commit into from
Feb 9, 2017
Merged

[STM32F7] Modify folder structure #3649

merged 1 commit into from
Feb 9, 2017

Conversation

adustm
Copy link
Member

@adustm adustm commented Jan 26, 2017

Description

ST team has noticed that STM32F746 and STM32F756 should not have the same startup files (the F756 has a few more interrupts).
The folder F746_756 makes no more sense.

We have thought about introducing a TARGET_STM32F7[device] folder between TARGET_STM32F7 and TARGET_[platform]
With this new folder structure we can share files for several platforms that uses the same STM32 device. For instance, NUCLEO_F746ZG and DISCO_F746NG has the same STM32F746xG devices.

I will create the same kind of modifications in other families

Status

READY

Migrations

NO

TESTS:

I have passed tests-integration-basic on NUCLEO_F746ZG / NUCLEO_F756ZG / DISCO_F769NI / DISCO_F746NG / NUCLEO_F767ZI for GCC_ARM / ARM / IAR
It's OK

@adustm
Copy link
Member Author

adustm commented Jan 26, 2017

@LMESTM @jeromecoutant @bcostm @jamike

@adustm
Copy link
Member Author

adustm commented Jan 26, 2017

The other advantage is to remove the copies of linker files and startup files.

@bridadan
Copy link
Contributor

/morph test-nightly

@bridadan
Copy link
Contributor

@mbed-bot: TEST

HOST_OSES=ALL
BUILD_TOOLCHAINS=ALL
TARGETS=ALL

@bridadan
Copy link
Contributor

/morph export-build

@jeromecoutant
Copy link
Collaborator

Full OS5 non regression session with NUCLEO_F767ZI, DISCO_F746NG and NUCLEO_F746ZG done.
Result is OK.

@mbed-bot
Copy link

Result: SUCCESS

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

/morph export-build

Output

mbed Build Number: 95

All exports and builds passed!

@mbed-bot
Copy link

Result: FAILURE

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

/morph test-nightly

@bridadan
Copy link
Contributor

That failure was my fault, sorry! Rerunning now.

/morph test-nightly

@mbed-bot
Copy link

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

@JanneKiiskila
Copy link
Contributor

Please do not make the folder names longer! Long folder names will start breaking builds on IAR, because Windows doesn't tolerate the long folder names. This is already now a challenge with Jenkins jobs.

@mbed-bot
Copy link

Result: FAILURE

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

/morph test-nightly

Output

mbed Build Number: 1463

Test failed!

@bridadan
Copy link
Contributor

@geky same spurious failure with the NCS36510 timer.

/morph test-nightly

@mbed-bot
Copy link

Result: SUCCESS

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

/morph test-nightly

Output

mbed Build Number: 1466

All builds and test passed!

@adustm adustm mentioned this pull request Jan 27, 2017
Merged
@adustm
Copy link
Member Author

adustm commented Jan 27, 2017

Hi @JanneKiiskila , is it still ok with this PR, or is it already blocking ?
ST / ST_familly / ST_device / Target, I don't think we will have another reason for making the names longer.
It is a usual issue for IAR toolchain, I hope someone will fix it on their side someday...
Kind regards

@adustm adustm mentioned this pull request Jan 27, 2017
Merged
@JanneKiiskila
Copy link
Contributor

It is a usual issue for IAR toolchain, I hope someone will fix it on their side someday...

They can't fix it. It's an Windows OS level issue.

Can we consider moving out the TARGET_ out of the 2nd level path names or is there some mbed-cli dependency on that?
Folder TARGET/TARGET... -target there is just tautology.

@adbridge adbridge changed the title [STM32F7] Modify forder structure [STM32F7] Modify folder structure Jan 30, 2017
@0xc0170
Copy link
Contributor

0xc0170 commented Jan 30, 2017

Please do not make the folder names longer! Long folder names will start breaking builds on IAR, because Windows doesn't tolerate the long folder names. This is already now a challenge with Jenkins jobs.

Thanks @JanneKiiskila , this is a valid point. From the mcu tree perspective, this change makes sense to minimize the code duplication. however, we hit here the problem with very long file paths.

We need to think about the targets hierarchy, how to structure MCU families. This PR contains a nice example of how one family (F7) is structured

@bridadan
Copy link
Contributor

@JanneKiiskila is this already causing issues now? Can you change your folder structure at all in CI to be shorter? The change @adustm seems fairly reasonable (a change of +/- 5 characters it seems?) and more organized.

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 30, 2017

cc @sg- @theotherjimmy

@theotherjimmy
Copy link
Contributor

theotherjimmy commented Jan 30, 2017 •
edited
Loading

@0xc0170 What do you need from me? I'll let you know if I come up with a way to reduce the directory structure length.

@sg-
Copy link
Contributor

sg- commented Feb 2, 2017

At some point if this is a problem we should consider TGT_ in place of TARGET which would save even more paths. This removes 4 characters per depth or sub folder.

Seems good to me, @JanneKiiskila is there something currently broken by this? If not thoughts on the above proposal before marking this ready for merge?

@sg- sg- requested a review from JanneKiiskila February 2, 2017 17:13
@sg- sg- requested review from sg- and 0xc0170 February 2, 2017 17:13
@adbridge
Copy link
Contributor

adbridge commented Feb 6, 2017

Another thought. If the top level dir is e.g. TARGET_STM , do we need 'STM' in any child directories ?
i.e. In
TARGET_STM/TARGET_STM32F7/TARGET_NUCLEO_F756ZG/stm32f7_eth_init.c
Could TARGET_STM32F7 just become TARGET_32F7 ? These kinds of tricks would save a few more characters if it starts to become a real problem?

@adustm
Copy link
Member Author

adustm commented Feb 6, 2017

Could TARGET_STM32F7 just become TARGET_32F7 ?

in this case I suggest TARGET_F7 (no more 'STM32'), so basically TGT_F7
But please keep STM32 for the directory that contains the name of the device.

Who will be a volonteer ?
This is the kind of pull request that everybody loves (each pending PR must be rebased, isn't it ? ) I suggest that someone from ARM does both together (TARGET to TGT + STM32Fx to Fx).
People would need to rebase only once.

Kind regards

@adustm
Copy link
Member Author

adustm commented Feb 7, 2017

Hello @sg-
I've seen that you've merged the other PRs with the same goal. Is there any reason not merging this one ?

Kind regards
Armelle

@sg- sg- merged commit e0fb062 into ARMmbed:master Feb 9, 2017
@adustm adustm deleted the STM32F7_folderstruct branch March 2, 2017 08:43
@adustm adustm mentioned this pull request Mar 14, 2017
Closed
aisair pushed a commit to aisair/mbed that referenced this pull request Apr 30, 2024
@0xc0170
Release 136 of the mbed library
da44cf6 
Ports for Upcoming Targets


Fixes and Changes

3432: Target STM USBHOST support ARMmbed/mbed-os#3432
3181: NUCLEO_F207ZG extending PeripheralPins.c: all available alternate functions can be used now ARMmbed/mbed-os#3181
3626: NUCLEO_F412ZG : Add USB Device +Host ARMmbed/mbed-os#3626
3628: Fix warnings ARMmbed/mbed-os#3628
3629: STM32: L0 LL layer ARMmbed/mbed-os#3629
3632: IDE Export support for platform VK_RZ_A1H ARMmbed/mbed-os#3632
3642: Missing IRQ pin fix for platform VK_RZ_A1H ARMmbed/mbed-os#3642
3664: Fix ncs36510 sleep definitions ARMmbed/mbed-os#3664
3655: [STM32F4] Modify folder structure ARMmbed/mbed-os#3655
3657: [STM32L4] Modify folder structure ARMmbed/mbed-os#3657
3658: [STM32F3] Modify folder structure ARMmbed/mbed-os#3658
3685: STM32: I2C: reset state machine ARMmbed/mbed-os#3685
3692: uVisor: Standardize available legacy heap and stack ARMmbed/mbed-os#3692
3621: Fix for #2884, LPC824: export to LPCXpresso, target running with wron ARMmbed/mbed-os#3621
3649: [STM32F7] Modify folder structure  ARMmbed/mbed-os#3649
3695: Enforce device_name is valid in targets.json ARMmbed/mbed-os#3695
3723: NCS36510: spi_format function bug fix ARMmbed/mbed-os#3723
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sg- sg- sg- approved these changes

@JanneKiiskila JanneKiiskila Awaiting requested review from JanneKiiskila

@0xc0170 0xc0170 Awaiting requested review from 0xc0170

Assignees
No one assigned
Labels
release-version: 5.3.5
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@adustm @bridadan @jeromecoutant @mbed-bot @JanneKiiskila @0xc0170 @theotherjimmy @sg- @adbridge