Skip to content

Analog pins definition clean up #800

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.

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

Merged
merged 34 commits into from
Dec 19, 2019
Merged

Conversation

fpistm
Copy link
Member

@fpistm fpistm commented Nov 28, 2019

This PR allows to define non contiguous analog pins.

Based on @MCUdude works.

After several investigations, it is not possible to provide a generic way to define non contiguous analog pins.
This PR simply clean the analog pins definition

Finally, found a way to fit all requirements.

@fpistm fpistm added the enhancement New feature or request label Nov 28, 2019
@MCUdude
Copy link
Contributor

MCUdude commented Nov 28, 2019

Looks good to me!

@fpistm fpistm force-pushed the Analog_pins_def branch 2 times, most recently from df1ac73 to 5103588 Compare December 2, 2019 07:08
@fpistm fpistm changed the title Analog pins redefinition Analog pins definition clean up Dec 2, 2019
@fpistm fpistm force-pushed the Analog_pins_def branch 4 times, most recently from 7d07dce to 7451cb8 Compare December 6, 2019 17:16
@fpistm
Copy link
Member Author

fpistm commented Dec 9, 2019

@MCUdude
any feedback on my last proposal?

@MCUdude
Copy link
Contributor

MCUdude commented Dec 9, 2019

Sorry, I've been on a business trip for a while, but I'm back home tonight. I'll get into it in a day or two

@fpistm fpistm marked this pull request as ready for review December 16, 2019 15:15
@fpistm fpistm added this to the 1.8.0🎄 🎅 milestone Dec 16, 2019
@fpistm fpistm self-assigned this Dec 16, 2019
@fpistm
Copy link
Member Author

fpistm commented Dec 16, 2019

@MCUdude
I've finish to validate and all seems OK.
If you have any feedback, this would be great 😉

@MCUdude
Copy link
Contributor

MCUdude commented Dec 17, 2019

Just had a quick look at the code. So this means that if NUM_ANALOG_FIRST is not defined, the core will use the analog pins array, and the only way to use analogRead if with the analog channel number (0..15) or the analog pin number (A0..A15). Pxy and digital pin number will not work, am I right?

@fpistm
Copy link
Member Author

fpistm commented Dec 17, 2019

No it works for all

@MCUdude
Copy link
Contributor

MCUdude commented Dec 17, 2019

I'm trying to wrap my head around how this works, but I'm struggling. Would you like to give a short explanation of how it's able to determine the difference between:

analogRead(4); // Digital pin 4
analogRead(4); // Analog channel 4
analogRead(A4); // Analog pin 4

And I'll assume that the analog pins aren't reserved for analog use only?

@fpistm
Copy link
Member Author

fpistm commented Dec 17, 2019

analogRead(4); // Digital pin 4 is the same of analogRead(4); // Analog channel 4
Using analogRead implies tu use an analog pin.

@fpistm
Copy link
Member Author

fpistm commented Dec 17, 2019

Using PYn will works as now an analog PYn is define as Ax.

@MCUdude
Copy link
Contributor

MCUdude commented Dec 17, 2019

OK, so analogRead will necessarily not support Arduino digital pin numbers (since it may/will conflict with analog channel numbers), but rather analog channel number (0..15), analog pin number (A0..A15) and PYx.

Here's the 16 first pins of the generic F401Rx pinout. Is this the correct way of defining them?

//                  | DIGITAL | ANALOG | USART     | TWI      | SPI                    | SPECIAL   |
//                  |---------|--------|-----------|----------|------------------------|-----------|
#define PA0  A0  // | 0       | A0     |           |          |                        |           |
#define PA1  A1  // | 1       | A1     |           |          |                        |           |
#define PA2  A2  // | 2       | A2     | USART2_TX |          |                        |           |
#define PA3  A3  // | 3       | A3     | USART2_RX |          |                        |           |
#define PA4  A4  // | 4       | A4     |           |          | SPI1_SS, (SPI3_SS)     |           |
#define PA5  A5  // | 5       | A5     |           |          | SPI1_SCK               |           |
#define PA6  A6  // | 6       | A6     |           |          | SPI1_MISO              |           |
#define PA7  A7  // | 7       | A7     |           |          | SPI1_MOSI              |           |
#define PA8  8   // | 8       |        |           | TWI3_SCL |                        |           |
#define PA9  9   // | 9       |        | USART1_TX |          |                        |           |
#define PA10 10  // | 10      |        | USART1_RX |          |                        |           |
#define PA11 11  // | 11      |        | USART6_TX |          |                        |           |
#define PA12 12  // | 12      |        | USART6_RX |          |                        |           |
#define PA13 13  // | 13      |        |           |          |                        | SWD_SWDIO |
#define PA14 14  // | 14      |        |           |          |                        | SWD_SWCLK |
#define PA15 15  // | 15      |        |           |          | SPI1_SS, (SPI3_SS)     |           |

With the following analogInPin table:

// Analog (Ax) pin number array
const uint32_t analogInPin[] = {
  0,  // A0
  1,  // A1
  2,  // A2
  3,  // A3
  4,  // A4
  5,  // A5
  6,  // A6
  7,  // A7
  16, // A8
  17, // A9
  31, // A10
  32, // A11
  33, // A12
  34, // A13
  35, // A14
  36  // A15
}

@fpistm
Copy link
Member Author

fpistm commented Dec 17, 2019

yes this is like this.

@MCUdude
Copy link
Contributor

MCUdude commented Dec 17, 2019

Great! I'll push some changes to #784 to reflect these changes. BTW this PR looks good to me!

@fpistm fpistm force-pushed the Analog_pins_def branch 2 times, most recently from 1a97546 to a692737 Compare December 18, 2019 20:56
fpistm and others added 2 commits December 19, 2019 09:57
AEND was used by firmata and kept for backward compatibility
with Firmata.
It could be now safely removed as Firmata does not use it anymore.

Signed-off-by: Frederic Pillon <[email protected]>
Signed-off-by: Frederic Pillon <[email protected]>
Co-Authored-By: MCUdude <[email protected]>
@fpistm
Copy link
Member Author

fpistm commented Dec 19, 2019

Firmata compatibility:
firmata/arduino#440

@fpistm fpistm merged commit 370ad02 into stm32duino:master Dec 19, 2019
@fpistm fpistm deleted the Analog_pins_def branch December 19, 2019 15:28
fpistm added a commit to fpistm/Arduino_Core_STM32 that referenced this pull request Oct 25, 2020
stm32duino#800 introduced a regression on analog pins Ax usage as CS pin.

Signed-off-by: Frederic Pillon <[email protected]>
@fpistm fpistm mentioned this pull request Oct 25, 2020
fpistm added a commit that referenced this pull request Oct 26, 2020
#800 introduced a regression on analog pins Ax usage as CS pin.

Signed-off-by: Frederic Pillon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants