STM32 : targets.json file simplification #4610

jeromecoutant · 2017-06-22T09:03:14Z

Description

"FAMILY_STM32" has been created with all common features.
All STM32 devices can now inherit from it.
This will simplify readability and maintainability.

Status

READY

theotherjimmy

Could you get rid of the core and progen lines? I really like this diff so far!

theotherjimmy · 2017-06-22T14:52:37Z

targets/targets.json

@@ -668,367 +668,321 @@
        "extra_labels_remove": ["FRDM"],
        "supported_form_factors": []
    },
+    "FAMILY_STM32": {
+        "inherits": ["Target"],
+        "core": "",


You should not add this line. Assigning core like this can cause problems if a child incorrectly does not define it's core attribute. Further, each child must define the core attribute to function, overriding this one.

no, I can't. It seems that core is mandatory in the parse script ?

Replace it with "public": false, maybe

It's required for public targets, but you don't really want to be able to build for the non-target "FAMILY_STM32".

OK, it's working.
I think it's even better to add this public false

theotherjimmy · 2017-06-22T14:55:16Z

targets/targets.json

        "is_disk_virtual": true,
-        "macros": ["HSE_VALUE=26000000", "TRANSACTION_QUEUE_SIZE_SPI=2"],
+        "macros_add": ["HSE_VALUE=26000000"],
        "progen": {"target": "mts-mdot-f405rg"},


I know that you did not change this, but could you get rid of this progen key? It's not used anymore.

jeromecoutant · 2017-06-22T15:34:56Z

Could you get rid of the core and progen lines?

done

theotherjimmy

Awesome! Thanks for addressing my comments so quickly.

0xc0170

Just one missing macro, the rest looks good, +1 for simplifying the stm32 targets

0xc0170 · 2017-06-26T07:38:33Z

targets/targets.json

-        "macros": ["TRANSACTION_QUEUE_SIZE_SPI=2", "RTC_LSI=1", "HSE_VALUE=12000000", "GNSSBAUD=9600"],
-        "inherits": ["Target"],
-        "device_has": ["ANALOGIN", "ANALOGOUT", "I2C", "I2CSLAVE", "INTERRUPTIN", "PORTIN", "PORTINOUT", "PORTOUT", "PWMOUT", "SERIAL", "SERIAL_FC", "SLEEP", "RTC", "SPI", "SPISLAVE", "STDIO_MESSAGES", "TRNG"],
+        "macros_add": ["RTC_LSI=1", "HSE_VALUE=12000000"],


I dont see here GNSSBAUD=9600

jeromecoutant · 2017-06-26T08:41:19Z

Hi
Missing macro added and rebase done
Thx

"FAMILY_STM32" has been creeated with all common features. All STM32 devices can now inherit from it. This will simplify readability and maintainability.

theotherjimmy · 2017-06-26T14:08:28Z

👏 Thanks @jeromecoutant

theotherjimmy · 2017-06-26T15:04:02Z

/morph test

mbed-bot · 2017-06-27T03:12:07Z

Result: FAILURE

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

/morph test

Output

mbed Build Number: 641

Test failed!

theotherjimmy · 2017-06-27T17:10:58Z

Clearly not related.

/morph test

theotherjimmy · 2017-06-27T17:11:17Z

@mazimkhan Did this one fail to report as well?

theotherjimmy · 2017-06-27T19:35:09Z

retest uvisor

chrissnow · 2017-06-28T08:43:40Z

@jeromecoutant could there be an additional layer of inheritance for the processor family? for example L0,L1(soon),L4 support Flash but the changes would have to be made to all targets individually.

mbed-bot · 2017-06-28T11:27:30Z

Result: SUCCESS

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

/morph test

Output

mbed Build Number: 668

All builds and test passed!

0xc0170 · 2017-06-28T11:49:36Z

@jeromecoutant could there be an additional layer of inheritance for the processor family? for example L0,L1(soon),L4 support Flash but the changes would have to be made to all targets individually.

Can you elaborate? Is this required for this patch or can be done on top of this one?

chrissnow · 2017-06-28T12:05:53Z

It could be done on top of this,

My thoughts were that a lot of things are common to a processor family and splitting out what the HAL for the family supports from the individual targets would tidy it up further.

My example being I have implemented the flash HAL for all L1 devices but currently would need to go through any L1 target and modify them all to enable flash.

jeromecoutant · 2017-06-28T13:00:50Z

@chrissnow
I am not in favor of this additional level... 2 levels are enough to see what is common for all STM32, and what makes the difference.

@0xc0170
Please merge, I am waiting for this PR for #4421

theotherjimmy suggested changes Jun 22, 2017

View reviewed changes

jeromecoutant force-pushed the PR_TARGETJSON branch from 066a807 to 898aa6e Compare June 22, 2017 15:33

theotherjimmy approved these changes Jun 22, 2017

View reviewed changes

theotherjimmy added the needs: review label Jun 23, 2017

0xc0170 reviewed Jun 26, 2017

View reviewed changes

jeromecoutant force-pushed the PR_TARGETJSON branch 2 times, most recently from e3cc7ef to f5b440d Compare June 26, 2017 08:39

STM32 : targets.json file simplification

8e28d9e

"FAMILY_STM32" has been creeated with all common features. All STM32 devices can now inherit from it. This will simplify readability and maintainability.

jeromecoutant force-pushed the PR_TARGETJSON branch from f5b440d to 8e28d9e Compare June 26, 2017 09:00

theotherjimmy added needs: CI and removed needs: review labels Jun 26, 2017

theotherjimmy mentioned this pull request Jun 27, 2017

NUCLEO_L496ZG: Add new Platform #4650

Merged

0xc0170 added ready for merge and removed needs: CI labels Jun 28, 2017

theotherjimmy merged commit 4fc4405 into ARMmbed:master Jun 28, 2017

theotherjimmy removed the ready for merge label Jun 28, 2017

theotherjimmy mentioned this pull request Jun 28, 2017

STM32L486RG/mbedtls: add aes hw acceleration #4163

Merged

theotherjimmy added the release-version: 5.5.2 label Jun 28, 2017

theotherjimmy mentioned this pull request Jun 28, 2017

STM32L1 Flash API and xDot bootloader support #4640

Merged

jeromecoutant deleted the PR_TARGETJSON branch June 29, 2017 07:53

STM32 : targets.json file simplification #4610

STM32 : targets.json file simplification #4610

Uh oh!

Conversation

jeromecoutant commented Jun 22, 2017

Description

Status

Uh oh!

theotherjimmy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeromecoutant commented Jun 22, 2017 • edited by theotherjimmy Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theotherjimmy left a comment

Choose a reason for hiding this comment

Uh oh!

0xc0170 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeromecoutant commented Jun 26, 2017

Uh oh!

theotherjimmy commented Jun 26, 2017

Uh oh!

theotherjimmy commented Jun 26, 2017

Uh oh!

mbed-bot commented Jun 27, 2017

Output

Uh oh!

theotherjimmy commented Jun 27, 2017

Uh oh!

theotherjimmy commented Jun 27, 2017

Uh oh!

theotherjimmy commented Jun 27, 2017

Uh oh!

chrissnow commented Jun 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mbed-bot commented Jun 28, 2017

Output

Uh oh!

0xc0170 commented Jun 28, 2017

Uh oh!

chrissnow commented Jun 28, 2017

Uh oh!

jeromecoutant commented Jun 28, 2017

Uh oh!

Uh oh!

jeromecoutant commented Jun 22, 2017 •

edited by theotherjimmy

Loading

chrissnow commented Jun 28, 2017 •

edited

Loading