stm32: Add support for analog-only "_C" pins. by dpgeorge · Pull Request #13764 · micropython/micropython

dpgeorge · 2024-02-27T06:08:45Z

This PR adds support for the analog-only pads/pins on STM32H7 parts. These pads/pins are called PA0_C/PA1_C/PC2_C/PC3_C in the datasheet. They each have an analog switch that can optionally connect them to their normal pin (eg PA0). When the switch is open, the normal and _C pin are independent pins/pads.

The approach taken in this PR to make these _C pins available to Python is:

put them in their own, independent row in the stm32h7_af.csv definition file, with only the ADC column defined (this seems logical to me, since they are going to be separate machine.Pin entities, and doing it this way keeps make-pins.py pretty clean)
allow a board to reference these pins in the board's pins.csv file by the name PA0_C etc (so a board can alias them, for example)
these pins (when enabled in pins.csv) now become available like any other machine.Pin through both machine.Pin.board and machine.Pin.cpu
BUT these _C pins have a separate pin type which doesn't have any methods, because they don't have any functionality
these _C pins can be used with machine.ADC to construct the appropriate ADC object, either by passing the string as machine.ADC("PA0_C") or by passing the object as machine.ADC(machine.Pin.cpu.PA0_C)
if a board defines both the normal and _C pin (eg both PA0 and PA0_C) in pins.csv then it must not define the analog switch to be closed (this is a sanity check for the build, because it doesn't make sense to close the switch and have two separate pins)

To show how this works, the ARDUINO_PORTENTA_H7 board is updated to define its analog pins, using these new _C pins. The code is also tested on this board to work correctly.

TODO:

update all relevant af.csv files with separate _C rows

dpgeorge · 2024-02-27T06:10:49Z

@iabdalkader FYI, this should allow full support for the dual-pad analog pins on Arduino boards.

jimmo · 2024-02-27T11:47:55Z

    def __init__(self, cpu_pin_name):
        super().__init__(cpu_pin_name)

+        # Pins ending in "_C" are special analog-only pins.


Can you add "H7" and "Dual-pad" to this comment -- if you don't know about dual pad, it would be very difficult to figure out what an "analog-only pin" is.

jimmo · 2024-02-27T11:49:31Z

+        .adc_num = p_adc_num, \
+        .adc_channel = p_adc_channel, \
+    }
+#endif


Could we add an #else here that did something like

#define PIN_ANALOG DUAL_PAD_SUPPORT_NOT_ENABLED__CONFIGURE__MICROPY_HW_ANALOG_SWITCH_PAx

jimmo · 2024-02-27T11:57:25Z

+// analog switch to be open.  The macro code below will produce a compiler warning
+// if the board defines the switch as closed.
+#if defined(pin_A0) && defined(pin_A0_C)
+#define MICROPY_HW_ANALOG_SWITCH_PA0 (SYSCFG_SWITCH_PA0_OPEN)


Is the idea that this triggers a redefinition of the macro?

Would doing this achieve the same:

#if defined(pin_A0) && defined(pin_A0_C) && defined(MICROPY_HW_ANALOG_SWITCH_PA0) && MICROPY_HW_ANALOG_SWITCH_PA0 == SYSCFG_SWITCH_PA0_CLOSED #error "Both pads for dual-pad A0 are enabled, switch must be closed." #endif

(It's a bit confusing to see a .c file defining a macro that it doesn't use)

As the comment says, this is intended to trigger a compiler warning if the board defines the switch as closed.

I originally had tried what you suggested above but it doesn't work because the comparison MICROPY_HW_ANALOG_SWITCH_PA0 == SYSCFG_SWITCH_PA0_CLOSED is too complex for the preprocessor.

iabdalkader · 2024-02-29T16:14:47Z

@dpgeorge I tested this by adding all analog/analog-only pins to the Giga board, and I can read values independently on dual pads pins, so it seems to be working fine, thanks!

A0,PC4
A1,PC5
A2,PB0
A3,PB1
A4,PC3
A5,PC2
A6,PC0
A7,PA0
A8,PC2_C
A9,PC3_C
A10,PA1_C
A11,PA0_C

Test:

import time
from machine import ADC
from machine import Pin

adc1 = ADC("A5")  # PC2
adc2 = ADC("A8")  # PC2_C
while True:
    print(adc1.read_u16(), adc2.read_u16())
    time.sleep_ms(500)

dpgeorge · 2024-03-01T05:24:39Z

Thanks @iabdalkader for testing.

I have now added the Giga pins.csv code as part of this PR.

Does anything need to be added for Nicla Vision?

iabdalkader · 2024-03-01T06:39:11Z

The Nicla closes 2 switches, and breaks out 3 single pad pins analog pins::

A0,PC4
A1,PF13
A2,PF3

Pins for all boards can be found here in <board>/variant.cpp https://github.com/arduino/ArduinoCore-mbed/tree/main/variants

Should we also add digital board pins in this PR ?

dpgeorge · 2024-03-04T05:42:40Z

The Nicla closes 2 switches, and breaks out 3 single pad pins analog pins

OK, thanks, I added those analog pins.

Should we also add digital board pins in this PR ?

I also added digital pins to this PR. Can you please check them? I didn't add everything for the Giga, just the standard set up to D14.

iabdalkader · 2024-03-04T08:06:30Z

+D4,PC7
+D5,PC6


Suggested change

D4,PC7

D5,PC6

Note those two pins are broken out on the camera connector. I don't think it makes much sense to define them, since the camera connector isn't a generic B2B connector, but if we do then we should define the other pins on that connector.

These pins have not been added.

iabdalkader · 2024-03-04T08:11:56Z

Can you please check them? I didn't add everything for the Giga, just the standard set up to D14.

I double checked all pins, they all look good. No need to add the HD pins there's another open PR that adds them

dpgeorge · 2024-03-04T10:33:18Z

Hmm, maybe I should merge #14022 first, then this PR here just adds the _C analog pins.

iabdalkader · 2024-03-04T11:01:01Z

Hmm, maybe I should merge #14022 first, then this PR here just adds the _C analog pins.

Either way is fine with me, but re #14022, in the stm32 port, it's conventional to define actual board pins near the end of the file, after all Pxy,Pxy

sebromero · 2024-03-05T11:55:37Z

I just added the suggestions from the review in #14022 ✅

projectgus · 2024-03-07T23:48:07Z

This is an automated heads-up that we've just merged a Pull Request
that removes the STATIC macro from MicroPython's C API.

See #13763

A search suggests this PR might apply the STATIC macro to some C code. If it
does, then next time you rebase the PR (or merge from master) then you should
please replace all the STATIC keywords with static.

Although this is an automated message, feel free to @-reply to me directly if
you have any questions about this.

This commit adds support for the dual-analog-pads on STM32H7 parts. These pads/pins are called PA0_C/PA1_C/PC2_C/PC3_C in the datasheet. They each have an analog switch that can optionally connect them to their normal pin (eg PA0). When the switch is open, the normal and _C pin are independent pins/pads. The approach taken in this commit to make these _C pins available to Python is: - put them in their own, independent row in the stm32h7_af.csv definition file, with only the ADC column defined (they are separate machine.Pin entities, and doing it this way keeps make-pins.py pretty clean) - allow a board to reference these pins in the board's pins.csv file by the name PA0_C etc (so a board can alias them, for example) - these pins (when enabled in pins.csv) now become available like any other machine.Pin through both machine.Pin.board and machine.Pin.cpu - BUT these _C pins have a separate pin type which doesn't have any methods, because they don't have any functionality - these _C pins can be used with machine.ADC to construct the appropriate ADC object, either by passing the string as machine.ADC("PA0_C") or by passing the object as machine.ADC(machine.Pin.cpu.PA0_C) - if a board defines both the normal and _C pin (eg both PA0 and PA0_C) in pins.csv then it must not define the analog switch to be closed (this is a sanity check for the build, because it doesn't make sense to close the switch and have two separate pins) Signed-off-by: Damien George <[email protected]>

dpgeorge · 2024-03-08T01:40:06Z

I have removed all pin additions to Arduino boards. That will be done as part of #14022.

dpgeorge added the port-stm32 label Feb 27, 2024

jimmo reviewed Feb 27, 2024

View reviewed changes

Gadgetoid mentioned this pull request Feb 29, 2024

global: Remove the STATIC macro. #13763

Merged

dpgeorge force-pushed the stm32-adc-analog-only-pin-support branch from 750eadd to c727b3f Compare March 1, 2024 05:24

iabdalkader reviewed Mar 4, 2024

View reviewed changes

dpgeorge force-pushed the stm32-adc-analog-only-pin-support branch from eb0b79b to 27da007 Compare March 8, 2024 01:37

dpgeorge force-pushed the stm32-adc-analog-only-pin-support branch from 27da007 to a9efffc Compare March 8, 2024 01:38

dpgeorge merged commit a9efffc into micropython:master Mar 8, 2024

dpgeorge deleted the stm32-adc-analog-only-pin-support branch March 8, 2024 01:51

dpgeorge mentioned this pull request Mar 8, 2024

boards/Arduino: Add missing pin names from documentation. #14022

Merged

Uh oh!

Conversation

dpgeorge commented Feb 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dpgeorge commented Feb 27, 2024

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!

iabdalkader commented Feb 29, 2024

Uh oh!

dpgeorge commented Mar 1, 2024

Uh oh!

iabdalkader commented Mar 1, 2024

Uh oh!

dpgeorge commented Mar 4, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iabdalkader commented Mar 4, 2024

Uh oh!

dpgeorge commented Mar 4, 2024

Uh oh!

iabdalkader commented Mar 4, 2024

Uh oh!

sebromero commented Mar 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

projectgus commented Mar 7, 2024

Uh oh!

dpgeorge commented Mar 8, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dpgeorge commented Feb 27, 2024 •

edited

Loading

sebromero commented Mar 5, 2024 •

edited

Loading