Skip to content
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

[New Variant] generic STM32F401R pinout #784

Closed
MCUdude opened this issue Nov 20, 2019 · 17 comments
Closed

[New Variant] generic STM32F401R pinout #784

MCUdude opened this issue Nov 20, 2019 · 17 comments
Labels
new variant Add support of new bard

Comments

@MCUdude
Copy link
Contributor

MCUdude commented Nov 20, 2019

Hi!

As mentioned in #722 I'm working on a new pinout for a generic F401R. The pinout is not "board oriented" but rather "chip oriented". This means the Arduino style pinout matches the port numbers as closely as possible.

I've gotten all digital pins to work as they should, but I'm having a hard time getting the analog pins to work, due to the way the analogRead function is implemented. Is there a way I can get the pinout below to work by using analogRead(A0) or analogRead(PA0) without having to "align" all analog pins so that A0, A1...A15 comes after eachother?

/*----------------------------------------------------------------------------
 *        Pins
 *----------------------------------------------------------------------------*/
                // | ANALOG | USART     | TWI      | SPI                    | SPECIAL   |
                // |--------|-----------|----------|------------------------|-----------|
#define PA0  0  // | A0     |           |          |                        |           |
#define PA1  1  // | A1     |           |          |                        |           |
#define PA2  2  // | A2     | USART2_TX |          |                        |           |
#define PA3  3  // | A3     | USART2_RX |          |                        |           |
#define PA4  4  // | A4     |           |          | SPI1_SS, (SPI3_SS)     |           |
#define PA5  5  // | A5     |           |          | SPI1_SCK               |           |
#define PA6  6  // | A6     |           |          | SPI1_MISO              |           |
#define PA7  7  // | A7     |           |          | SPI1_MOSI              |           |
#define PA8  8  // |        |           | TWI3_SCL |                        |           |
#define PA9  9  // |        | USART1_TX |          |                        |           |
#define PA10 10 // |        | USART1_RX |          |                        |           |
#define PA11 11 // |        | USART6_TX |          |                        |           |
#define PA12 12 // |        | USART6_RX |          |                        |           |
#define PA13 13 // |        |           |          |                        | SWD_SWDIO |
#define PA14 14 // |        |           |          |                        | SWD_SWCLK |
#define PA15 15 // |        |           |          | SPI1_SS, (SPI3_SS)     |           |
                // |--------|-----------|----------|------------------------|-----------|
#define PB0  16 // | A8     |           |          |                        |           |
#define PB1  17 // | A9     |           |          |                        |           |
#define PB2  18 // |        |           |          |                        | BOOT1     |
#define PB3  19 // |        |           | TWI2_SDA | SPI1_SCK,  (SPI3_SCK)  |           |
#define PB4  20 // |        |           | TWI3_SDA | SPI1_MISO, (SPI3_MISO) |           |
#define PB5  21 // |        |           |          | SPI1_MOSI, (SPI3_MOSI) |           |
#define PB6  22 // |        | USART1_TX | TWI1_SCL |                        |           |
#define PB7  23 // |        | USART1_RX | TWI1_SDA |                        |           |
#define PB8  24 // |        |           | TWI1_SCL |                        |           |
#define PB9  25 // |        |           | TWI1_SDA | SPI2_SS                |           |
#define PB10 26 // |        |           | TWI2_SCL | SPI2_SCK               |           |
#define PB12 27 // |        |           |          | SPI2_SS                |           |
#define PB13 28 // |        |           |          | SPI2_SCK               |           |
#define PB14 29 // |        |           |          | SPI2_MISO              |           |
#define PB15 30 // |        |           |          | SPI2_MOSI              |           |
                // |--------|-----------|----------|------------------------|-----------|
#define PC0  31 // | A10    |           |          |                        |           |
#define PC1  32 // | A11    |           |          |                        |           |
#define PC2  33 // | A12    |           |          | SPI2_MISO              |           |
#define PC3  34 // | A13    |           |          | SPI2_MOSI              |           |
#define PC4  35 // | A14    |           |          |                        |           |
#define PC5  36 // | A15    |           |          |                        |           |
#define PC6  37 // |        | USART6_TX |          |                        |           |
#define PC7  38 // |        | USART6_RX |          |                        |           |
#define PC8  39 // |        |           |          |                        |           |
#define PC9  40 // |        |           | TWI3_SDA |                        |           |
#define PC10 41 // |        |           |          | SPI3_SCK               |           |
#define PC11 42 // |        |           |          | SPI3_MISO              |           |
#define PC12 43 // |        |           |          | SPI3_MOSI              |           |
#define PC13 44 // |        |           |          |                        |           |
#define PC14 45 // |        |           |          |                        | OSC32_IN  |
#define PC15 46 // |        |           |          |                        | OSC32_OUT |
                // |--------|-----------|----------|------------------------|-----------|
#define PD2  47 // |        |           |          |                        |           |
                // |--------|-----------|----------|------------------------|-----------|
#define PH0  48 // |        |           |          |                        | OSC_IN    |
#define PH1  49 // |        |           |          |                        | OSC_OUT   |
                // |--------|-----------|----------|------------------------|-----------|
@MCUdude MCUdude added the new variant Add support of new bard label Nov 20, 2019
@fpistm
Copy link
Member

fpistm commented Nov 20, 2019

@MCUdude
The Analog pins have to be consecutive and the first one have to get a number greater than or equal to the number of analog pins.
So your pinout cannot works.
In your case you have 16 analogs pins then A0 should have value 16 or greater.

@MCUdude
Copy link
Contributor Author

MCUdude commented Nov 20, 2019

The Analog pins have to be consecutive and the first one have to get a number greater than or equal to the number of analog pins.

OK, That's a huge bummer... But why don't every pinout have its own pins_arduino.c/h file, like other cores have? This makes custom boards and pinouts much more flexible because we don't have hardcoded pin definition rules in the core files.

It's not important for me to have access to the A0...A15 macros, what is important is to keep the pinout as clean as possible while still be able to read every analog pin.

@MCUdude
Copy link
Contributor Author

MCUdude commented Nov 20, 2019

When thinking a bit more about it; all existing variants use the same pins_arduino.h. It should be possible to make a copy of these two files and move them into each variant folder. This would let the actual pinout definitions control how pins_arduino.c/h should work.

@fpistm
Copy link
Member

fpistm commented Nov 20, 2019

I know but for Arduino compatibility, I had to do like this. I never found another way to met all requirement and allowing to be generic.
variant.h can be seen as an extension of pin_arduino.h and define the pinout.
so simply define a correct pins numbering.

@MCUdude
Copy link
Contributor Author

MCUdude commented Nov 20, 2019

variant.h can be seen as an extension of pin_arduino.h and define the pinout.
so simply define a correct pins numbering.

The problem is that the pins_arduino files set all the rules, and it's not possible to change them in the variants file. However, I may have found a solution that works well.

I moved the pins_arduino.c/h files out from the core files folder and into the Nucleo F401RE variants folder and to the folder where I keep the generic F401 pinout I'm working on. By doing this each variant can have it's very own pins_arduino rules, and therefore allow any combination of digital and analog pins. I successfully modified the pins_arduino files used for my own generic pinout, and it does not affect any of the other variants. The standard Nucle F401RE variant still remains untouched, even though I've copied a version of pins_arduino.c/h to this variant. All existing variants will have their own pins_arduino.c/h files.

What do you think about this solution? The only downside I can think if is that it will generate some more files. On the positive side;

  • Will not break any code for any existing variant
  • All variants can now have complete custom pin mapping without having "global" rules (just like the official Arduino core)
  • Compiled size is not changed
  • I will be able to create nice, clean, well-documented pinout for a variety of 48 and 64 pin chips.

I can be helpful by providing a separate PR for this if you like. Looking forward to your reply @fpistm !

@fpistm
Copy link
Member

fpistm commented Nov 20, 2019

The problem is that the pins_arduino files set all the rules, and it's not possible to change them in the variants file

Wrong. Some are defined if the variants does not define them. Variant.h is the first included.

allow any combination of digital and analog pins.

I'm not sure. You will break some arduino compatibility.
Requirements are:

  • Be able to loop across all Ax pins using an index:
for (uint8_t i=0; i<NUM_ANALOG_INPUTS; i++) {
  Serial.println(analogRead(i));
}

I do not want add more files in the variant folder except if there is no other choice.

@MCUdude
Copy link
Contributor Author

MCUdude commented Nov 20, 2019

Be able to loop across all Ax pins using an index:

This will indeed NOT work on the generic pinout I'm working on. It will still work on every other variant that exists today. This is simply a sacrifice that has to be made on a generic pinout like this. IMO it is difficult to combine both A0..A15, PA0..PHx, 0..49 and PA_x..PH_x to work perfectly with analogRead. Personally, I prefer to use Ax or the actual pin number (14 for instance) when using analogRead. This has to be properly documented for this particular pinout of course.

I do not want to add more files in the variant folder except if there is no other choice.

If the pins_arduino.c/h files are not moved to to the variants folder, I would have to rewrite the global pins_arduino.c/h files completely so that everything that is defined in variant.h will "override" what is defined in pins_arduino.c/h. Chances for breaking things are much higher. It would be much easier to keep backward compatibility if we decided not to touch the global pins_arduino.c/h and instead move it into the variant where it belongs.

(I just had a look at some other STM32 families, and their pinouts are VERY similar to the F401C and F401R. This means that with some solid groundwork it should be fairly quick and easy to use these new generic pinouts with other STM32 families. I may even create some nice pinout diagram to help developers using the generic pinouts. For reference, here's some other pinouts I've made for some new 8-bit AVRs.

@fpistm
Copy link
Member

fpistm commented Nov 20, 2019

This will indeed NOT work on the generic pinout I'm working on.

So I could not support that, several third party library use that.

@uzi18
Copy link

uzi18 commented Nov 20, 2019

maybe we can introduce analogReadPort(PC0) ?

@MCUdude
Copy link
Contributor Author

MCUdude commented Nov 20, 2019

This will indeed NOT work on the generic pinout I'm working on.

So I could not support that, several third party library use that.

Wait, it IS possible to support all this, we just have to be a little clever. First, I'll have to have control over the pins_arduino.c/h file. Then we can map the input of the analog pins this way.

  • If the user passes the number 0..15 to the analogRead function, it will interpret this as analog pin 0 to 15.
  • We assign A0 to A15 to it's digital pin values.
    • A0 = 0 ... A7 = 7
    • A8 = 16, A9 = 17
    • A10 = 31 ... A15 = 36

The key here is that the "next" analog pin after A7 is actually digital pin 16, which leaves 8-15 free for mapping to the other analog pins.

This way the user can use 0-15, A0-A15 and the digital pin number. I implemented it this way in MegaCoreX for the ATmega4809. It works great, but it requires full control over the pins_arduino.c/h

@fpistm
Copy link
Member

fpistm commented Nov 20, 2019

@MCUdude
And if user do analogRead(A8) ?

@MCUdude
Copy link
Contributor Author

MCUdude commented Nov 20, 2019

MCUdude
And if user do analogRead(A8) ?

It would work just fine! It would be exactly the same as writing analogRead(8), analogRead(16) or analogRead. See table below.

I can push some samples to my fork of this repo tomorrow so you can see yourself.

All these variants will work for each analogpin:

Analog channel A0..A15 constant Digital pin PIN macro
analogRead(0) analogRead(A0) analogRead(0) analogRead(PA0)
analogRead(1) analogRead(A1) analogRead(1) analogRead(PA1)
analogRead(2) analogRead(A2) analogRead(2) analogRead(PA2)
analogRead(3) analogRead(A3) analogRead(3) analogRead(PA3)
analogRead(4) analogRead(A4) analogRead(4) analogRead(PA4)
analogRead(5) analogRead(A5) analogRead(5) analogRead(PA5)
analogRead(6) analogRead(A6) analogRead(6) analogRead(PA6)
analogRead(7) analogRead(A7) analogRead(7) analogRead(PA7)
analogRead(8) analogRead(A8) analogRead(16) analogRead(PB0)
analogRead(9) analogRead(A9) analogRead(17) analogRead(PB1)
analogRead(10) analogRead(A10) analogRead(31) analogRead(PC0)
analogRead(11) analogRead(A11) analogRead(32) analogRead(PC1)
analogRead(12) analogRead(A12) analogRead(33) analogRead(PC2)
analogRead(13) analogRead(A13) analogRead(34) analogRead(PC3)
analogRead(14) analogRead(A14) analogRead(35) analogRead(PC4)
analogRead(15) analogRead(A15) analogRead(36) analogRead(PC5)

@fpistm
Copy link
Member

fpistm commented Nov 20, 2019

Ok 😉
Would be happy to remove the restriction.

@MCUdude
Copy link
Contributor Author

MCUdude commented Nov 20, 2019

Would be happy to remove the restriction.

Great! I will continue working on it tomorrow. I can provide the necessary PRs, but first I'll have to sort everything out. I'll pretty much have to redesign the entire pins_arduino.c/h for the generic pinout, I'm working on. But hey, I've done this several times before 😉

@MCUdude
Copy link
Contributor Author

MCUdude commented Nov 21, 2019

I'm soon finished working on the generic pinout. All analog pins behave like they should (according to the table above). Is it OK if I move the contents from pins_arduino.c into pins_arduino.h instead, and delete the pins_arduino.c fir this pinout? This means one file less to handle (for this particular pinout).

@fpistm
Copy link
Member

fpistm commented Nov 21, 2019

@MCUdude
Don't know exactly what you do.
Do like you want then it will be review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new variant Add support of new bard
Projects
None yet
Development

No branches or pull requests

3 participants