Pinmap extensions #9449

c1728p9 · 2019-01-22T03:10:17Z

Description

Add pinmap getters and utility functions, along with form factor information in preparation for enhanced testing.

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change
[x] New feature

Release Notes

Added pinmap support to the HAL API. For more information see the porting targets docs page - https://os.mbed.com/docs/mbed-os/latest/porting/porting-targets.html.

c1728p9 · 2019-01-22T03:10:33Z

This PR depends on #9438

0xc0170 · 2019-01-22T08:05:47Z

CI started to test this

jeromecoutant · 2019-01-22T08:43:11Z

targets/targets.json

+            },
+            "default-form-factor": {
+                "help": "Default form factor of this board taken from supported_form_factors. This must be a lowercase string",
+                "value": "arduino"


Hi
Default value should be "null"...?
And specified at target level?
Which is already done:
"supported_form_factors": ["ARDUINO"]

Note also, that this ARDUINO means Arduino Uno, some STM32 targets are Arduino Nano compatible.

Default value should be "null"...?
And specified at target level?

Yeah, I'll make those changes.

Which is already done:
"supported_form_factors": ["ARDUINO"]

The goal with default-form-factor is to allow a default to be specified when multiple form factors are present, such as ["ARDUINO", "MORPHO"] with the NUCLEO_F401RE.

Note also, that this ARDUINO means Arduino Uno, some STM32 targets are Arduino Nano compatible.

I didn't realize this. What are your thoughts @jeromecoutant on having a separate ARDUINO_NANO for those targets? That way supported_form_factors represents the form factors that can be physically connected to a board.

mbed-ci · 2019-01-22T09:22:54Z

Test run: FAILED

Summary: 6 of 7 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_mbed2-build-ARM
jenkins-ci/mbed-os-ci_mbed2-build-GCC_ARM
jenkins-ci/mbed-os-ci_mbed2-build-IAR
jenkins-ci/mbed-os-ci_build-IAR
jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-GCC_ARM

bulislaw · 2019-01-22T15:42:12Z

How we'll test the extensions to HAL APIs?

c1728p9 · 2019-01-22T16:13:43Z

@bulislaw these extensions will be used to test hardware peripherals. Any bugs in the pinmaps will show up as a peripheral test failure. Does that answer your question?

hal/analogin_api.h

bulislaw · 2019-01-23T12:07:53Z

targets/targets.json

@@ -34,6 +34,10 @@
            "mpu-rom-end": {
                "help": "Last address of ROM protected by the MPU",
                "value": "0x0fffffff"
+            },
+            "default-form-factor": {


Hm you are not setting default value here, but in the code sort of circumventing the config system and hiding the fact. Wouldn't it be cleaner to set it here?

Setting the default here to arduino would also be misleading since some boards are not in the arduino form factor. Because of this @jeromecoutant suggested keeping this null and specifying it at the target level, which I think is a good idea.

Adding this new config is OK.
But who is going to replace the supported_form_factors line by this new config?
Why you are mixing both configurations ?

Hi
No answer for this question?
A new config is created, but never used...!

Hi @jeromecoutant sorry for the late response, I just noticed your question. The default-form-factor is intended to be used with supported_form_factors, not to replace it. Some boards may have multiple form factors that are testable and this value is intended to let you pick the one to test with. This is currently not needed by any targets so it is not used. I added it here for completeness. If you are concerned with it I can remove it an submit it as a PR only when it is needed. Let me know if you have a preference on this.

@jeromecoutant even though this PR has been merged please provide any feedback you may have. I'll address any changes needed in a separate PR.

c1728p9 · 2019-01-23T23:33:25Z

I created a docs PR to go along with this here:
ARMmbed/mbed-os-5-docs#930

cmonr · 2019-01-23T23:40:17Z

Testing with CI

mbed-ci · 2019-01-24T00:24:00Z

Test run: FAILED

Summary: 6 of 8 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_mbed2-build-IAR
jenkins-ci/mbed-os-ci_mbed2-build-GCC_ARM
jenkins-ci/mbed-os-ci_mbed2-build-ARM
jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-IAR

jeromecoutant · 2019-01-24T12:40:49Z

hal/mbed_pinmap_default.c

+#include "PeripheralPins.h"
+
+//*** Common form factors ***
+#ifdef TARGET_FF_ARDUINO


Should be somthing like MBED_CONF_TARGET_DEFAULT_FORM_FACTOR==ARDUINO ?

I was thinking that all the form factors a board supports can be present at the same time. So for example ST boards can have both TARGET_FF_MORPHO and TARGET_FF_ARDUINO. Setting the value of MBED_CONF_TARGET_DEFAULT_FORM_FACTOR would just pick one of these to be used during testing. What do you think @jeromecoutant

To be honest, for me, TARGET_FF_MORPHO and TARGET_FF_ARDUINO are not used at all...

So be free to make the changes you need :-)

c1728p9 · 2019-01-24T17:17:50Z

Based on the test results, a lot of targets are failing because pinmaps are not publicly exposed. For example the LPC1768 has its serial pinamp static to serial_api.c. To address this I'm removed the weak definitions of the pinmap getter functoins from mbed_pinmap_default.c and have instead added them to each target. This simplifies things since the table name doesn't matter and there is no weak pinmap getter function to worry about.

cmonr · 2019-01-24T20:06:45Z

Testing with CI

Add pinmap tables to the NRF5X families to allow testing.

Add testing pinmaps to the Ameba along with fixing build errors.

Add pinmap tables to the LPC15XX and LPC8XX families to allow testing.

Add a test which sanity checks all pinmaps.

c1728p9 · 2019-02-08T15:13:12Z

Rebased on top of master and fixed merge conflicts

c1728p9 · 2019-02-08T15:13:36Z

Re-triggered CI

mbed-ci · 2019-02-08T17:05:07Z

Test run: SUCCESS

Summary: 12 of 12 test jobs passed
Build number : 15
Build artifacts

0xc0170 · 2019-02-11T09:57:12Z

Functionality change

One last addition, please add Release notes section , following https://os.mbed.com/docs/mbed-os/v5.11/contributing/workflow.html#functionality-change

cc @AnotherButler to review release notes section

This is required since PR ARMmbed#9449 commit "Add HAL API for analog in pinmap"

This is required since PR #9449 commit "Add HAL API for analog in pinmap"

c1728p9 mentioned this pull request Jan 22, 2019

Add RFC for PinMap's extension #8693

Closed

c1728p9 requested review from fkjagodzinski, maciejbocianski, mprse, donatieng, bulislaw and mray190 January 22, 2019 03:35

c1728p9 force-pushed the pinmap-extension branch from a8b907e to e124309 Compare January 22, 2019 03:57

jeromecoutant reviewed Jan 22, 2019

View reviewed changes

c1728p9 force-pushed the pinmap-extension branch from e124309 to 165eee3 Compare January 22, 2019 18:29

AnotherButler suggested changes Jan 22, 2019

View reviewed changes

hal/analogin_api.h Outdated Show resolved Hide resolved

hal/analogin_api.h Outdated Show resolved Hide resolved

cmonr added needs: work needs: preceding PR labels Jan 23, 2019

bulislaw reviewed Jan 23, 2019

View reviewed changes

c1728p9 force-pushed the pinmap-extension branch 2 times, most recently from 7c40fde to 21055be Compare January 23, 2019 22:42

c1728p9 removed the needs: preceding PR label Jan 23, 2019

AnotherButler approved these changes Jan 23, 2019

View reviewed changes

jeromecoutant reviewed Jan 24, 2019

View reviewed changes

c1728p9 force-pushed the pinmap-extension branch from 21055be to 0ff0bde Compare January 24, 2019 17:16

c1728p9 added 4 commits February 8, 2019 09:10

Add testing pinmaps to NRF5X devices

8d205ab

Add pinmap tables to the NRF5X families to allow testing.

Add pinmaps to Ameba

c7a1946

Add testing pinmaps to the Ameba along with fixing build errors.

Add testing pinmaps to LPC15XX and LPC8XX devices

8007085

Add pinmap tables to the LPC15XX and LPC8XX families to allow testing.

Add a sanity check test for pinmaps

d5892ce

Add a test which sanity checks all pinmaps.

c1728p9 force-pushed the pinmap-extension branch from 60fe934 to d5892ce Compare February 8, 2019 15:12

NirSonnenschein added ready for merge and removed needs: CI labels Feb 10, 2019

0xc0170 added needs: work and removed ready for merge labels Feb 11, 2019

maciejbocianski approved these changes Feb 11, 2019

View reviewed changes

0xc0170 added ready for merge release-version: 5.12.0-rc1 and removed needs: work labels Feb 11, 2019

0xc0170 merged commit 8f932a4 into ARMmbed:master Feb 11, 2019

0xc0170 removed the ready for merge label Feb 11, 2019

c1728p9 deleted the pinmap-extension branch February 11, 2019 17:47

c1728p9 mentioned this pull request Feb 12, 2019

Pinmap design doc #9690

Merged

LMESTM added a commit to LMESTM/mbed that referenced this pull request Mar 12, 2019

STM32WB: Add missing analogin_pinmap

369ced8

This is required since PR ARMmbed#9449 commit "Add HAL API for analog in pinmap"

LMESTM added a commit to LMESTM/mbed that referenced this pull request Mar 15, 2019

STM32WB: Add missing analogin_pinmap

731975f

This is required since PR ARMmbed#9449 commit "Add HAL API for analog in pinmap"

LMESTM added a commit to LMESTM/mbed that referenced this pull request Mar 29, 2019

STM32WB: Add missing analogin_pinmap

e2cdb19

This is required since PR ARMmbed#9449 commit "Add HAL API for analog in pinmap"

0xc0170 pushed a commit that referenced this pull request Apr 5, 2019

STM32WB: Add missing analogin_pinmap

9345e5c

This is required since PR #9449 commit "Add HAL API for analog in pinmap"

amq mentioned this pull request Feb 5, 2020

EFM32: pinmap test fails #12371

Closed

Pinmap extensions #9449

Pinmap extensions #9449

Uh oh!

Conversation

c1728p9 commented Jan 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Release Notes

Uh oh!

c1728p9 commented Jan 22, 2019

Uh oh!

0xc0170 commented Jan 22, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbed-ci commented Jan 22, 2019

Uh oh!

bulislaw commented Jan 22, 2019

Uh oh!

c1728p9 commented Jan 22, 2019

Uh oh!

Uh oh!

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!

c1728p9 commented Jan 23, 2019

Uh oh!

cmonr commented Jan 23, 2019

Uh oh!

mbed-ci commented Jan 24, 2019

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!

c1728p9 commented Jan 24, 2019

Uh oh!

cmonr commented Jan 24, 2019

Uh oh!

c1728p9 commented Feb 8, 2019

Uh oh!

c1728p9 commented Feb 8, 2019

Uh oh!

mbed-ci commented Feb 8, 2019

Uh oh!

0xc0170 commented Feb 11, 2019

Uh oh!

Uh oh!

c1728p9 commented Jan 22, 2019 •

edited

Loading