STM32_gen_PeripheralPins.py script v1.12 #13036

jeromecoutant · 2020-05-29T14:20:08Z

Summary of changes

Goal is to help every one to make his own custom boards.
I remind script goal is to generate PeripheralPins.c and PinNames.h for ST MCU

Few addition in this PR:

alignment with ST internal version
license header update
PinNames.h is now complete

@ARMmbed/team-st-mcd

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

ciarmcom · 2020-05-29T15:00:30Z

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

tools/targets/STM32_gen_PeripheralPins.py

rwalton-arm · 2020-06-02T08:46:09Z

tools/targets/STM32_gen_PeripheralPins.py

+                BUTTON_list.append(EachPin)
+            elif "BTN" in PinLabel[EachPin].upper():
+                BUTTON_list.append(EachPin)
+        except:


We shouldn't really use a bare except here. I wouldn't expect we want to pass on KeyboardInterrupt, SystemExit or another low level error. A slightly more pleasant version of this would be to catch all errors which derive from Exception, but ideally we'd catch the specific exceptions we're expecting here (which I suspect is probably at least a KeyError given all the dictionary look-ups).

This should be addressed.

0xc0170 · 2020-06-03T13:53:30Z

tools/targets/STM32_gen_PeripheralPins.py

@@ -1203,20 +1327,22 @@ def parse_BoardFile(fileName):
    print("Please set your configuration in '%s' file" % config_filename)
    config_file = open(config_filename, "w")
    if sys.platform.startswith("win32"):
-        print("Platform is Windows")
-        cubemxdir = "C:\\Program Files (x86)\\STMicroelectronics\\STM32Cube\\STM32CubeMX"
+        # print("Platform is Windows")


shall we just remove this dead code?

0xc0170 · 2020-06-03T13:53:59Z

tools/targets/STM32_gen_PeripheralPins.py

+    l.sort(key=natural_sortkey)
+    previous_pin = ""
+    for p in l:
+        # print("DEBUG pin %s => %s" % (p, p[0]))


either use logging or remove this

tools/targets/STM32_gen_PeripheralPins.py

- license header update - TIM_MST default value - PinNames.h is now complete

jeromecoutant

Thx for comments

jeromecoutant · 2020-06-08T08:17:29Z

tools/targets/STM32_gen_PeripheralPins.py

+    l.sort(key=natural_sortkey)
+    previous_pin = ""
+    for p in l:
+        # print("DEBUG pin %s => %s" % (p, p[0]))


jeromecoutant · 2020-06-08T08:17:47Z

tools/targets/STM32_gen_PeripheralPins.py

@@ -1203,20 +1327,22 @@ def parse_BoardFile(fileName):
    print("Please set your configuration in '%s' file" % config_filename)
    config_file = open(config_filename, "w")
    if sys.platform.startswith("win32"):
-        print("Platform is Windows")
-        cubemxdir = "C:\\Program Files (x86)\\STMicroelectronics\\STM32Cube\\STM32CubeMX"
+        # print("Platform is Windows")


tools/targets/STM32_gen_PeripheralPins.py

Pull request has been modified.

0xc0170 · 2020-06-09T11:51:32Z

CI started

mbed-ci · 2020-06-09T13:10:38Z

Test run: SUCCESS

Summary: 6 of 6 test jobs passed
Build number : 1
Build artifacts

0xc0170 · 2020-06-10T10:08:38Z

All green but the incorrect status reported from mergify makes it red. I'll merge as it can't be overwritten.

rwalton-arm

Sorry, after taking a closer look at this, I have some more comments. I know some of these issues already existed in the code, but we should try and improve any new code we add.

rwalton-arm · 2020-06-10T10:35:18Z

tools/targets/STM32_gen_PeripheralPins.py

 }

+def print_debug(console_line):


We should use the logging module here. It will give us more informative output and allows the log level to be set.

rwalton-arm · 2020-06-10T10:43:30Z

tools/targets/STM32_gen_PeripheralPins.py

-                s += "// TIM<x> cannot be used because already used by the us_ticker\n"
-                s += "// You have to comment all PWM using TIM_MST defined in hal_tick.h file\n"
-                s += "//  or update python script (check TIM_MST_LIST) and re-run it\n"
+            DefaultTimerCore1 = "TIM5"


Is there a reason we use camel case here? The preferred style for python code is lowercase_with_underscores for variable names, CamelCase for class names. https://www.python.org/dev/peps/pep-0008/#function-and-variable-names

rwalton-arm · 2020-06-10T10:44:43Z

tools/targets/STM32_gen_PeripheralPins.py

+                        continue
+                    else:
+                        prev_p = p[0]
+                        p[0] += '_ALT%d' % alt_index


I'd prefer we didn't use single letter variable names. Can we come up with something more descriptive?

rwalton-arm · 2020-06-10T10:48:12Z

tools/targets/STM32_gen_PeripheralPins.py

+                if p[1] in PinLabel.keys():
+                    s2 += ' // Connected to ' + PinLabel[p[1]]
+                s2 += '\n'
+                out_c_file.write(s2)


Where do we open this out_c_file? Looks like we have the potential to leak a file descriptor, as this seems to be used throughout the script. We should use the with statement to ensure the resources are released if an exception is raised. Same comment for out_h_file.

rwalton-arm · 2020-06-10T10:48:49Z

tools/targets/STM32_gen_PeripheralPins.py

@@ -979,6 +937,141 @@ def print_usb(lst):
        out_c_file.write( "#endif\n" )


+def print_PinList(l):


Can we have a more descriptive parameter name than l?

rwalton-arm · 2020-06-10T10:55:27Z

tools/targets/STM32_gen_PeripheralPins.py

+        NameCounter += 1
+
+    NameCounter = 1
+    if len(BUTTON_list) == 0:


Suggested change

if len(BUTTON_list) == 0:

if not BUTTON_list:

rwalton-arm · 2020-06-10T10:56:26Z

tools/targets/STM32_gen_PeripheralPins.py

@@ -1148,42 +1241,63 @@ def parse_BoardFile(fileName):
    board_file.close()

    for EachPin in PinData:
-        try:
+        PinLabel[EachPin] = ""
+        if "Signal" in PinData[EachPin].keys():


We don't need to call keys - the in operator will just check dictionary keys by default.

rwalton-arm · 2020-06-10T10:57:39Z

tools/targets/STM32_gen_PeripheralPins.py

        cubemxdir = "<Set CubeMX install directory>"
    config_file.write(json.dumps({"CUBEMX_DIRECTORY":cubemxdir}))
    config_file.close()
-    exit(1)
+    print("Default path set: %s\n" % cubemxdir)
+    config_file = open(config_filename, "r")


We should use the with statement to ensure this file object is cleaned up when an exception occurs.

rwalton-arm · 2020-06-10T10:58:46Z

tools/targets/STM32_gen_PeripheralPins.py

+            "DISCO_H747XIH": "DISCO_H747I"
+        }
+
+        if TargetName in target_rename.keys():


Don't need to call keys

rwalton-arm · 2020-06-10T10:59:29Z

tools/targets/STM32_gen_PeripheralPins.py

    epilog=textwrap.dedent('''\
 Once generated, you have to check and comment pins that can not be used (specific HW, internal ADC channels, remove PWM using us ticker timer, ...)
 '''),
    formatter_class=RawTextHelpFormatter)
-group = parser.add_mutually_exclusive_group()
+
+group = parser.add_mutually_exclusive_group(required = True)


Suggested change

group = parser.add_mutually_exclusive_group(required = True)

group = parser.add_mutually_exclusive_group(required=True)

rwalton-arm

I'm going to go ahead and approve this as-is. This is a small script that doesn't form part of the core tools so I'm not as concerned about the quality. We can schedule in some improvement work into our backlog at a later date.

jeromecoutant · 2020-06-11T11:09:08Z

@rwalton-arm as it is fresh in your mind, I suggest you to propose a PR with all your good ideas just after merge ?

rwalton-arm · 2020-06-11T11:14:49Z

@rwalton-arm as it is fresh in your mind, I suggest to propose a PR with all ,your good ideas just after merge ?

Hi Jerome. Unfortunately I'm not going to have time for that this sprint. I will schedule the work for next sprint. I can refer back to this PR when I get time to make the changes so things won't be forgotten. Thanks for your hard work on this contribution!

ATM-HSW · 2020-06-11T11:31:35Z

@rwalton-arm as it is fresh in your mind, I suggest you to propose a PR with all your good ideas just after merge ?

I have a good idea: please document how to use the script! ;-)

It is mentioned at a Mbed STM page but without any usage instructions.

I like the idea of the script very much but I was not able to try it. Using google I found some information snippets but unfortunatly that was not enough.

jeromecoutant · 2020-06-11T11:34:28Z

I have a good idea: please document how to use the script! ;-)

Should be on #13053 !

adbridge · 2020-06-11T12:32:58Z

@jeromecoutant isn't this just modifying ST target code and thus should be patch type ?

jeromecoutant · 2020-06-15T07:14:15Z

Let's start CI ?

adbridge · 2020-06-17T09:16:31Z

CI restarted

mbed-ci · 2020-06-17T10:00:19Z

Test run: SUCCESS

Summary: 6 of 6 test jobs passed
Build number : 2
Build artifacts

ciarmcom requested review from a team May 29, 2020 15:00

ciarmcom added the needs: review label May 29, 2020

rwalton-arm reviewed Jun 2, 2020

View reviewed changes

tools/targets/STM32_gen_PeripheralPins.py Outdated Show resolved Hide resolved

rwalton-arm reviewed Jun 2, 2020

View reviewed changes

0xc0170 previously requested changes Jun 3, 2020

View reviewed changes

mergify bot added needs: work and removed needs: review labels Jun 3, 2020

rwalton-arm reviewed Jun 3, 2020

View reviewed changes

tools/targets/STM32_gen_PeripheralPins.py Outdated Show resolved Hide resolved

rwalton-arm reviewed Jun 3, 2020

View reviewed changes

tools/targets/STM32_gen_PeripheralPins.py Outdated Show resolved Hide resolved

rwalton-arm reviewed Jun 3, 2020

View reviewed changes

tools/targets/STM32_gen_PeripheralPins.py Outdated Show resolved Hide resolved

STM32_gen_PeripheralPins.py script v1.12

dbe8e93

- license header update - TIM_MST default value - PinNames.h is now complete

jeromecoutant force-pushed the PR_GENPERIPH_V12 branch from 6821bee to dbe8e93 Compare June 8, 2020 09:10

jeromecoutant commented Jun 8, 2020

View reviewed changes

0xc0170 added needs: review and removed needs: work labels Jun 8, 2020

0xc0170 requested review from 0xc0170 and rwalton-arm June 8, 2020 12:12

0xc0170 approved these changes Jun 8, 2020

View reviewed changes

mergify bot added needs: CI and removed needs: review labels Jun 8, 2020

mergify bot added ready for merge and removed needs: CI labels Jun 9, 2020

0xc0170 added needs: review and removed ready for merge labels Jun 10, 2020

rwalton-arm suggested changes Jun 10, 2020

View reviewed changes

mergify bot added needs: work and removed needs: review labels Jun 10, 2020

rwalton-arm approved these changes Jun 11, 2020

View reviewed changes

adbridge added needs: CI release-type: patch Indentifies a PR as containing just a patch and removed needs: work labels Jun 17, 2020

mergify bot added ready for merge and removed needs: CI labels Jun 17, 2020

adbridge added mergify skip needs: CI and removed ready for merge labels Jun 17, 2020

0xc0170 added ready for merge and removed mergify skip needs: CI labels Jun 17, 2020

0xc0170 merged commit 31748ed into ARMmbed:master Jun 17, 2020

mergify bot removed the ready for merge label Jun 17, 2020

jeromecoutant deleted the PR_GENPERIPH_V12 branch June 17, 2020 11:42

adbridge added release-version: 6.1.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Jun 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

STM32_gen_PeripheralPins.py script v1.12 #13036

STM32_gen_PeripheralPins.py script v1.12 #13036

jeromecoutant commented May 29, 2020 •

edited

Loading

ciarmcom commented May 29, 2020

rwalton-arm Jun 2, 2020 •

edited

Loading

rwalton-arm Jun 10, 2020

0xc0170 Jun 3, 2020

jeromecoutant Jun 8, 2020

0xc0170 Jun 3, 2020

jeromecoutant Jun 8, 2020

jeromecoutant left a comment

jeromecoutant Jun 8, 2020

jeromecoutant Jun 8, 2020

0xc0170 commented Jun 9, 2020

mbed-ci commented Jun 9, 2020

0xc0170 commented Jun 10, 2020

rwalton-arm left a comment

rwalton-arm Jun 10, 2020

rwalton-arm Jun 10, 2020

rwalton-arm Jun 10, 2020

rwalton-arm Jun 10, 2020

rwalton-arm Jun 10, 2020

rwalton-arm Jun 10, 2020

rwalton-arm Jun 10, 2020

rwalton-arm Jun 10, 2020

rwalton-arm Jun 10, 2020

rwalton-arm Jun 10, 2020

rwalton-arm left a comment

jeromecoutant commented Jun 11, 2020 •

edited

Loading

rwalton-arm commented Jun 11, 2020

ATM-HSW commented Jun 11, 2020

jeromecoutant commented Jun 11, 2020

adbridge commented Jun 11, 2020

jeromecoutant commented Jun 15, 2020

adbridge commented Jun 17, 2020

mbed-ci commented Jun 17, 2020

		@@ -979,6 +937,141 @@ def print_usb(lst):
		out_c_file.write( "#endif\n" )


		def print_PinList(l):

	group = parser.add_mutually_exclusive_group(required = True)
	group = parser.add_mutually_exclusive_group(required=True)

STM32_gen_PeripheralPins.py script v1.12 #13036

STM32_gen_PeripheralPins.py script v1.12 #13036

Conversation

jeromecoutant commented May 29, 2020 • edited Loading

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

ciarmcom commented May 29, 2020

rwalton-arm Jun 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeromecoutant left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xc0170 commented Jun 9, 2020

mbed-ci commented Jun 9, 2020

0xc0170 commented Jun 10, 2020

rwalton-arm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rwalton-arm left a comment

Choose a reason for hiding this comment

jeromecoutant commented Jun 11, 2020 • edited Loading

rwalton-arm commented Jun 11, 2020

ATM-HSW commented Jun 11, 2020

jeromecoutant commented Jun 11, 2020

adbridge commented Jun 11, 2020

jeromecoutant commented Jun 15, 2020

adbridge commented Jun 17, 2020

mbed-ci commented Jun 17, 2020

jeromecoutant commented May 29, 2020 •

edited

Loading

rwalton-arm Jun 2, 2020 •

edited

Loading

jeromecoutant commented Jun 11, 2020 •

edited

Loading