New variant BLACK_F407VE #269

edogaldo · 2018-06-22T16:58:20Z

Hello I would like to push a new variant for this board.
I could successfull test Serial and SPI (on a NRF24L01P).

Best, E.

BennehBoy · 2018-06-25T14:43:48Z

@edogaldo I applied this over the boards manager delivered version of the core (1.3.0) And get the following when I try compile any of my sketches:

`C:\Users\xxxx\AppData\Local\Arduino15\packages\STM32\hardware\stm32\1.3.0\cores\arduino\stm32\low_power.c: In function 'LowPower_EnableWakeUpPin':

C:\Users\xxxx\AppData\Local\Arduino15\packages\STM32\hardware\stm32\1.3.0\cores\arduino\stm32\low_power.c:98:12: error: 'SYS_WKUP1' undeclared (first use in this function)

   case SYS_WKUP1 :

        ^~~~~~~~~`

I also tried compiling one of the core SPI examples and get the same error - but only when this board selected.

Perhaps my installation issue - I cloned your repo, switched branch to BLACK407 one, and then copied the board.txt and variant folder into the STM core folder

fpistm · 2018-06-25T15:44:18Z

@BennehBoy , in fact there is 2 solutions:

If you want use Low power then it misses only PinNamesVar.h in variant folder, available here:
https://github.com/stm32duino/Arduino_Tools/blob/master/src/genpinmap/Arduino/STM32F407V(E-G)Tx/PinNamesVar.h

Else disable HAL_PWR_MODULE_ENABLED in hal conf
https://github.com/stm32duino/Arduino_Core_STM32/pull/269/files#diff-fe9e261429e73c44def3a88b08c3b54eR73

BennehBoy · 2018-06-25T16:25:31Z

I put the PinNamesVar.h in the variants folder and it compiles now.

I had not explicitly intended to use lowpower - presumably some of the library code or funtioncality that my project references requires it.

As above, I also tested BarometricPressureSensor.ino before making your suggested change and also had the same failure mode - does SPI rely on lowpower?

I've not got time to go digging in the dependencies I'm afraid.

edogaldo · 2018-06-25T18:19:21Z

@fpistm: there is no reference to PinNamesVar.h in the wiki page.
Anyway, I used v1.2.0 to add the variant and had no problems.
Looks like the problem rised with v1.3.
Is it?

BennehBoy · 2018-06-25T18:32:34Z

Just another note, I set the SPI pins back to the defaults (I'm porting a project over from the GENERIC core), I think it may make more sense to use those in variants.h:

#define PIN_SPI_MOSI            PA7
#define PIN_SPI_MISO            PA6
#define PIN_SPI_SCK             PA5

But otherwise, great work!

Just need USB Serial & a bootloader now ;)

edogaldo · 2018-06-25T18:40:29Z

I set the SPI pins used by the NRF24 adapter..

fpistm · 2018-06-25T19:03:48Z

1.3.0 releases the rtc and low power drivers. That's why if you enable the HAL_PWR_MODULE_ENABLED then you need to add PinNamesVar.h
I've updated the wiki. I forgot to do it... :'(

About default SPI. Don't know what is the best definition as I do not have this board.
Anyway, it is possible to change them at sketch level if required.

BennehBoy · 2018-06-25T19:08:43Z

I went by this -> http://wiki.stm32duino.com/images/3/3e/Black_STM32f4VET6_Pinouts_Left.pdf

But point taken at sketch level redefine - I just thought most people might expect it on the 'defaults'.

fpistm

First review.
Nice variant, good job ;).
See my comments.
Also update the commit message to be more explicit, for example:
Add Black F407VE support

fpistm · 2018-06-25T19:08:13Z

variants/BLACK_F407VE/stm32f4xx_hal_conf.h

+#define HAL_PWR_MODULE_ENABLED
+#define HAL_RCC_MODULE_ENABLED
+/* #define HAL_RNG_MODULE_ENABLED   */
+/* #define HAL_RTC_MODULE_ENABLED   */


To use RTC you could uncomment this

Do you suggest to enable it? I'm fine with your suggestion.

Yes moreover it is required for LowPower

fpistm · 2018-06-25T19:08:36Z

variants/BLACK_F407VE/stm32f4xx_hal_conf.h

+#define HAL_SPI_MODULE_ENABLED
+#define HAL_TIM_MODULE_ENABLED
+#define HAL_UART_MODULE_ENABLED
+#define HAL_USART_MODULE_ENABLED


HAL USART is not used. You could comment it.

fpistm · 2018-06-25T19:09:24Z

variants/BLACK_F407VE/stm32f4xx_hal_conf.h

+/* #define HAL_MMC_MODULE_ENABLED   */
+#define HAL_SPI_MODULE_ENABLED
+#define HAL_TIM_MODULE_ENABLED
+#define HAL_UART_MODULE_ENABLED


UART is now enabled thanks menu, so best way is to not enable it to be able to save space is no uart required.

So, the UART define should be commented, correct?

Maybe there's something to change also in boards.txt?!

It's already up to date in board.txt
https://github.com/stm32duino/Arduino_Core_STM32/pull/269/files#diff-fce09ad94aad2ba67472cd9142f00146R570

fpistm · 2018-06-25T19:10:01Z

variants/BLACK_F407VE/PeripheralPins.c

+    {PC_3,  ADC1,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 13, 0)}, // ADC1_IN13
+    {PC_4,  ADC1,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 14, 0)}, // ADC1_IN14
+    {PC_5,  ADC1,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 15, 0)}, // ADC1_IN15
+/*


Only ADC1 will be available ?

Well.. I followed the approach seen in other variants but of course open to improvement suggestions..

In fact you could enable as many ADC you want and use also other ADC instance.

If you are ok, I'd stick with only ADC1 as default, leaving extentions to future improvements.
Else, please suggest a better configuration.

I've no objection :)
I don't know well this board.
This could be update later if several user require them.

fpistm · 2018-06-25T19:11:24Z

variants/BLACK_F407VE/variant.cpp

+  PC_2,
+  PC_3,
+  PC_4,
+  PC_5,


one extra ","

?!?
edit: ok, got it!

fpistm · 2018-06-25T19:12:09Z

variants/BLACK_F407VE/variant.h

+extern const PinName digitalPin[];
+
+// Enum defining pin names to match digital pin number --> Dx
+// !!!


Those comments could be removed.

fpistm · 2018-06-25T19:22:48Z

variants/BLACK_F407VE/variant.h

+// First analog pin value (A0) must be greater than or equal to NUM_ANALOG_INPUTS
+#define NUM_ANALOG_FIRST        75
+
+// Below ADC, DAC and PWM definitions already done in the core


what's "ditto"?!? :)

ditto noun [ C, usually singular ]
the symbol (〃) that means 'the same' and is written immediately under a word or phrase that you are repeating so you do not have to write it out again

so, same comment as before ;)
Those comments could be removed.

fpistm · 2018-06-25T19:23:01Z

variants/BLACK_F407VE/variant.h

+//#define ADC_RESOLUTION          12
+//#define DACC_RESOLUTION         12
+
+// PWM resolution


I would keep commented defines for future use..

fpistm · 2018-06-25T19:23:06Z

variants/BLACK_F407VE/variant.h

+#define PIN_SPI_MISO            PB4 // NRF24 connector
+#define PIN_SPI_SCK             PB3 // NRF24 connector
+
+// I2C Definitions


fpistm · 2018-06-25T19:23:16Z

variants/BLACK_F407VE/variant.h

+// DEBUG_UART Tx pin name, default: the first one found in PinMap_UART_TX for DEBUG_UART
+//#define DEBUG_PINNAME_TX        PX_n // PinName used for TX
+
+// UART Emulation (uncomment if needed, required TIM1)


fpistm · 2018-06-25T19:50:39Z

One question about board menu, maybe it could be a good idea to add this under "Generic STM32F407 series" ?

edogaldo · 2018-06-25T20:15:42Z

Thanks @fpistm! I'll deeply check your comments later. Respect to your last question: in my opinion it could make sense to create groups "generic F1", "generic F4", etc.. so that you don't end up with too many "generic" groups. Il 25 Giu 2018 21:50, "Frederic Pillon" <[email protected]> ha scritto:

…

One question about board menu, maybe it could be a good idea to add this under "Generic STM32F407 series" ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#269 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ATTZbkcxkcgUS4r5b1lMTCwISLpMzxtXks5uAT8RgaJpZM4U0ICv> .

edogaldo · 2018-06-25T20:46:56Z

@BennehBoy: as a board user I'd expect that the default board configuration leverages the board features.
In this board the NRF24 adapter and the SPI flash both use SPI1 over PB3, PB4 and PB5 (ref. http://wiki.stm32duino.com/images/5/5c/STM32_F4VE_SCHEMATIC.PDF) so it makes sense to me that these are used as default SPI configuration for this board.

edogaldo · 2018-06-25T20:51:51Z

@fpistm: as a general rule, do you need me to apply the proposed changes in my PR?

fpistm · 2018-06-25T20:52:36Z

Yes, it should be fine. Thanks

BennehBoy · 2018-06-25T21:47:10Z

@edogaldo - my bad I thought the board you were using was the Black F407VET V2.0 - way too many boards coming out of China!

@fpistm I may be straying into unwanted territory on this PR, but I've tried to get SD working - I've copied sdconf.h & ffconf.h from the DISCO_F746NG variant, and redefined the SD_DETECT_PIN to PA4 where SS is mapped on this board.

Compile time however I get a whole raft of errors from FatFs & STM32SD.... is it OK to post those here for some assistance, or should I open an issue on STM32SD?

@fpistm

Commit stm32duino#2 based on @fpistm suggestions

edogaldo · 2018-06-25T21:56:59Z

@BennehBoy: the board is this one, which one are you referring to?!

fpistm · 2018-06-25T21:57:30Z

@BennehBoy please, open a new issue to not mix Variant addition and SD support.
I think I know why it is not working.

BennehBoy · 2018-06-25T22:01:05Z

@edogaldo same board, I just don't have the nrf24 module - lucky you :)

edogaldo · 2018-06-25T22:06:36Z

how can't you have it?!

BennehBoy · 2018-06-25T22:32:03Z

@edogaldo you misunderstand me, my board has the header, but I do not have an NRF24L01 module to insert into the header.

fpistm · 2018-07-10T06:33:10Z

@edogaldo
Respect to your last question: in my opinion it could make sense to create groups "generic F1", "generic F4", etc.. so that you don't end up with too many "generic" groups.

You're right, it should be fine to add a genericF4 menu and then add black F407 in this one.

fpistm · 2018-07-10T07:59:02Z

@edogaldo
I've made a set of patch on top of your commits with some cosmetic update.
I've updated the menu according to our discussion.
And update the peripheral pins to use different ADC instance. (not always the ADC1
blackf407_patch.tar.gz

you can do git am < patchxxx on each patch to apply them.

edogaldo · 2018-07-10T08:25:37Z

Hi Fredric, do you need me to commit your patch in the PR?
If yes, I've never used git am, could you please provide exact instructions to apply the patch?
I.e.:

is there any specific place to unzip the file?
which path should I position to run the command?
I guess patchxxx is a placeholder, correct? If yes, what exactly should I write instead?

Thanks in advance and best, E.

fpistm · 2018-07-10T09:05:27Z

The archive contains several patch file.
extract it in your root git repo
then for each one (they are numbered) do

git am < blackf407/0001-Update-menu.patch
git am < blackf407/0002-Clean-space-and-tabs.patch
git am < blackf407/0003-Cosmetic-update.patch
git am < blackf407/0004-Update-Peripheral-Pins.patch
git am < blackf407/0005-Fix-warning-on-section-alignement.patch

Signed-off-by: Frederic.Pillon <[email protected]>

fpistm · 2018-08-02T07:40:50Z

Hi @edogaldo,
do you made some tests?
It should be fine to end this PR.
About ADC I let you choose what you preferred.
Thanks in advance

uzi18 · 2018-08-07T19:59:04Z

What about variant BLACK_F407ZE?

edogaldo · 2018-08-16T10:20:55Z

Dear @fpistm, first of all please apologize for late reply.
I didn't answer so far as I still had no time to perform further tests (and I still don't know when I can..) anyway, as long as:

this is a new variant and thus it's not expected to introduce regressions in the core
tests on the original PR were ok and
last changes were almost "cosmetical"

I'd suggest to include it as is so that other people can benefit of it and also perform further tests (and maybe provide fixes and improvements).
What about it?

Thanks in advance and best regards, E.

@uzi18: feel free to go with your new variant, instructions on how to implement it are in the wiki and you can of course benefit of this variant as an example.
Cheers, E.

fpistm · 2018-08-16T12:12:25Z

No worries @edogaldo
Your suggestions is right. I could add it.

madias123 · 2018-08-22T08:09:33Z

I come a little bit late to the party, but one suggestion:
If SPI is mapped to the NRF24, maybe a little hint in the board description would be helpful?
something like:
"Black F407VE (NRF24 SPI1)"
I speak from experience, because there are many software "variants" for that board using this or that SPI settings, misleading to the pinout maps everybody is using -> https://tinyurl.com/y6ujo4ae

- Define PIN_SPI_SS1 as W25Q16 (on board flash) CS pin - Add comment to describe which is the default SPI configuration in boards.txt

ghent360 · 2018-08-29T04:56:27Z

Is there some work needed on this PR? I have the board and can volunteer.

edogaldo · 2018-08-29T07:36:43Z

Hi @ghent360: at the moment there's the need to perform some tests and check if everything is ok; if you could do it, it would be helpful.

Thanks and best, E.

ghent360 · 2018-08-29T08:00:19Z

What tests do you need done?

…

On Wed, Aug 29, 2018 at 12:36 AM edogaldo ***@***.***> wrote: Hi @ghent360 <https://github.com/ghent360>: at the moment there's the need to perform some tests and check if everything is ok; if you could do it, it would be helpful. Thanks and best, E. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#269 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHxY1uX-73HPBlGlem1AFqPy12mbbvTuks5uVkSNgaJpZM4U0ICv> .

madias123 · 2018-08-29T08:08:32Z

@ghent360 : I don't know if this issue is only board related: If you own an ILI9341 maybe you can observe this:
http://stm32duino.com/viewtopic.php?f=48&t=4028&p=48388#p48388
So I don't know it's about a) SPI speed (or a wrong implementation (library OR code)) or Clock speed.

regards
Matthias

edogaldo · 2018-08-29T09:27:54Z

Well @madias123, this seems not specifically related to this variant in any case, is it?
I'd sugget to perform general tests on Serial, SPI, I2C, ADC, other (up to one's imagination) and in any case taking into account generic core issues and limitations (e.g. I2C).

madias123 · 2018-08-29T10:01:33Z

@edogaldo This is the big question if this symptoms occurs only to this variant. Sadly this is my only *F4 board I own, so I cannot test it with another variant and my logic analyzer is only capturable to about 12MS/s so I cannot do full speed SPI testing (or measuring)

ghent360 · 2018-08-30T06:55:49Z

Strange error:
Board GenF4 (platform stm32, package STM32) is unknown

Error compiling for board Generic STM32F4 series.

Maybe I'm doing something wrong:

In the .arduino15/packages/STM32/hardware/stm32 folder I renamed the 1.3.0 folder to ___.
I created a sym link to the folder where git cloned the pull request. I can now see the "Generic STM32 F4" board menu and the "BLACK 407VE" board part number.

I'm trying to compile the "blink" example, but I'm getting the above error.

edogaldo · 2018-08-30T07:10:55Z

Which OS? If Win I'm not sure you can use a symlink. Did you activate verbose logging? Il Gio 30 Ago 2018, 08:55 ghent360 <[email protected]> ha scritto:

…

Strange error: Board GenF4 (platform stm32, package STM32) is unknown Error compiling for board Generic STM32F4 series. Maybe I'm doing something wrong: In the .arduino15/packages/STM32/hardware/stm32 folder I renamed the 1.3.0 folder to ___. I created a sym link to the folder where git cloned the pull request. I can now see the "Generic STM32 F4" board menu and the "BLACK 407VE" board part number. I'm trying to compile the "blink" example, but I'm getting the above error. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#269 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ATTZbn2VF2GgHaLt5E-g9ujVdKX9N5vXks5uV4x3gaJpZM4U0ICv> .

ghent360 · 2018-08-30T07:12:33Z

Ubuntu 18.04. Yes I have verbose logging.

ghent360 · 2018-08-30T07:19:21Z

If I only copy the boards.txt file and the variants/BLACK_F407VE folder it seems to work fine.

ghent360 · 2018-08-31T09:38:26Z

I have ILI9341 display - 2.4 inch. This is the test I compiled:
https://gist.github.com/ghent360/1386c2b5ab660fd65f94a816d41a8c75

I hooked the SPI_MISO (PB4) to pin 7 of the NRF24L01 connector. Display power to 5V, display LED to 3.3V.

Here is a video of the sketch working (https://youtu.be/Fgqo3emIVrw).

fpistm · 2018-09-13T09:41:31Z

FYI, I've get a Black F407 so I will be able to validate and also try to Fix SD part.

fpistm · 2018-09-16T15:25:05Z

Hi guys,
I've made some tests and found that pins for I2C are not properly defined, as by default it is D14/D15 -->PA4/ PA5 but those pins have no I2C capabilities, I've updated like this:

 // I2C Definitions
-//#define PIN_WIRE_SDA            14 // Default for Arduino connector compatibility
-//#define PIN_WIRE_SCL            15 // Default for Arduino connector compatibility
+#define PIN_WIRE_SDA            PB9
+#define PIN_WIRE_SCL            PB8

Edit:
PR Squashed and rebased on top in my forked repo : fpistm@6148356

fpistm · 2018-09-16T15:38:30Z

variants/BLACK_F407VE/variant.h

+
+// I2C Definitions
+//#define PIN_WIRE_SDA            14 // Default for Arduino connector compatibility
+//#define PIN_WIRE_SCL            15 // Default for Arduino connector compatibility


#define PIN_WIRE_SDA PB9 #define PIN_WIRE_SCL PB8

Hello @fpistm: thank you for your update, do you still need me to post a commit for it?

Hi @edogaldo,
I will push my branch as it is rebased on top of the master.
I've resolved all merge conflicts.
I've raised here to track this issue.

fpistm · 2018-09-19T08:01:04Z

This PR is replace by #327
Thanks all for your contributions.

First commit

83f6cc4

fpistm self-requested a review June 23, 2018 09:18

fpistm added the new variant Add support of new bard label Jun 23, 2018

fpistm reviewed Jun 25, 2018

View reviewed changes

Add Black F407VE support

ae90e85

Commit stm32duino#2 based on @fpistm suggestions

fpistm added 2 commits July 10, 2018 11:42

Update menu

1b8389a

Signed-off-by: Frederic.Pillon <[email protected]>

Clean space and tabs

f3d3fc4

Signed-off-by: Frederic.Pillon <[email protected]>

fpistm added the waiting feedback Further information is required label Aug 2, 2018

edogaldo added 2 commits August 24, 2018 11:34

SPI configuration slight improvement

5f2409c

- Define PIN_SPI_SS1 as W25Q16 (on board flash) CS pin - Add comment to describe which is the default SPI configuration in boards.txt

Change default SPI description

9223297

fpistm removed the waiting feedback Further information is required label Sep 13, 2018

fpistm added the review on going label Sep 14, 2018

fpistm added this to the 1.3.1 milestone Sep 14, 2018

fpistm reviewed Sep 16, 2018

View reviewed changes

fpistm mentioned this pull request Sep 19, 2018

Add support of BLACK F407VET #327

Merged

fpistm removed the review on going label Sep 19, 2018

fpistm closed this Sep 19, 2018

edogaldo deleted the BLACK_F407VE branch September 19, 2018 09:09

New variant BLACK_F407VE #269

New variant BLACK_F407VE #269

Conversation

edogaldo commented Jun 22, 2018

BennehBoy commented Jun 25, 2018 • edited Loading

fpistm commented Jun 25, 2018

BennehBoy commented Jun 25, 2018

edogaldo commented Jun 25, 2018

BennehBoy commented Jun 25, 2018 • edited Loading

edogaldo commented Jun 25, 2018

fpistm commented Jun 25, 2018

BennehBoy commented Jun 25, 2018 • edited Loading

fpistm left a comment • 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

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

edogaldo Jun 25, 2018 • 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

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

fpistm commented Jun 25, 2018

edogaldo commented Jun 25, 2018 via email

edogaldo commented Jun 25, 2018

edogaldo commented Jun 25, 2018

fpistm commented Jun 25, 2018

BennehBoy commented Jun 25, 2018 • edited Loading

edogaldo commented Jun 25, 2018

fpistm commented Jun 25, 2018

BennehBoy commented Jun 25, 2018

edogaldo commented Jun 25, 2018

BennehBoy commented Jun 25, 2018

fpistm commented Jul 10, 2018

fpistm commented Jul 10, 2018 • edited Loading

edogaldo commented Jul 10, 2018

fpistm commented Jul 10, 2018

fpistm commented Aug 2, 2018

uzi18 commented Aug 7, 2018

edogaldo commented Aug 16, 2018 • edited Loading

fpistm commented Aug 16, 2018

madias123 commented Aug 22, 2018

ghent360 commented Aug 29, 2018

edogaldo commented Aug 29, 2018

ghent360 commented Aug 29, 2018 via email

madias123 commented Aug 29, 2018

edogaldo commented Aug 29, 2018 • edited Loading

madias123 commented Aug 29, 2018

ghent360 commented Aug 30, 2018

edogaldo commented Aug 30, 2018 via email

ghent360 commented Aug 30, 2018

ghent360 commented Aug 30, 2018

ghent360 commented Aug 31, 2018

fpistm commented Sep 13, 2018

fpistm commented Sep 16, 2018 • edited Loading

fpistm Sep 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BennehBoy commented Jun 25, 2018 •

edited

Loading

BennehBoy commented Jun 25, 2018 •

edited

Loading

BennehBoy commented Jun 25, 2018 •

edited

Loading

fpistm left a comment •

edited

Loading

edogaldo Jun 25, 2018 •

edited

Loading

BennehBoy commented Jun 25, 2018 •

edited

Loading

fpistm commented Jul 10, 2018 •

edited

Loading

edogaldo commented Aug 16, 2018 •

edited

Loading

edogaldo commented Aug 29, 2018 •

edited

Loading

fpistm commented Sep 16, 2018 •

edited

Loading

fpistm Sep 16, 2018 •

edited

Loading