Support for elecgator EtherCATduino board

[pieterjanbuntinx]

Summary

This PR adds support for the elecgator EtherCATduino board. This board is based on a STM32F746ZET microcontroller and uses a LAN9252 chip as the 2-port EtherCAT device controller (ESC). It has a built-in DC-DC that supports voltages from 7 to 28 Volts, a MicroUSB port, 2 RJ45 connectors, and the same pinout and overall form factor as an Arduino Mega. We are also in the process of creating a library for using the EtherCAT supported on this board.

This PR fixes/implements the following features

• Added support for the elecgator EtherCATduino

Validation

• Passed the CheckVariant example sketch
• Successfully builds and uploads using DFU and StLink (SWD)

[fpistm]

Hi @pieterjanbuntinx
Thanks for this PR.
Do you have some schematics? user manual ?
On the official site except the picture of the boards it seems there is nothing.

[pieterjanbuntinx]

Hey @fpistm
Thank you for the feedback!
I think there is some misunderstanding about the ESC part of this board. This not an RC speed controller, this a board with an EtherCAT Slave Controller. Coincidentally, they have the same abbreviation but they are not the same. So I don't think they belong in the same category.
At the moment we don't have any documentation or schematics (except for the complete PCB schematics). We will certainly make those and provide them on our website. The website URL provided in the repo currently points to a non-existing page where we will provide all of this together with a link to buy the product. We first wanted to have our board supported by the main repo before finishing the rest.
Concerning the Astyle checks, is there some documentation on how can check them myself? I tried running the Astyle.py script in the CI directory but it complained about some connection it couldn't make.

[fpistm]

OK. for the ESC as no documentation is available at this time hard to check 😉
For Astyle, you need to have astyle installed and available in the path or providing the path where to find it.
https://github.com/stm32duino/wiki/wiki/Astyle

Then you can call it from repo root path like this (specify the path to check instead of checking the full repo):
python CI/astyle/astyle.py -r variants/STM32F7xx/F745Z(E-G)T_F746Z(E-G)(T-Y)_F750Z8T_F756ZG(T-Y)/

[fpistm]

Hi @pieterjanbuntinx

I made a deeper review and found several issue.
I've made a branch on my fork:
https://github.com/fpistm/Arduino_Core_STM32/tree/pr-1414-ethercatduino-review

• This PR has been rebased and squashed on top of the master
• Fix Analog pin definition
  (only 3 defined in the variant.h have the Ax capabilities)
• Comment DAC as no pin defined
• Add USB pin definition (PA11/PA12)
• Add link to the board section in the README.md
• Reorder boards.txt (alphabetical order for manufacturer)
• Remove node definition
• Add custom peripheral pins as not all pins available and allow to save space
• All pins not defined in the variant.h has been commented
• Comment some extra pins or HAL module definition
  as not all pins available for the same peripheral instance
  Ex: No QSPI CLK defined or SDMMC CMD or ETH_REF_CLK
• Fixed system core clock config

About analog pin, I saw that it should be 12 so you have to defined in the variant.h all the pins which can be used. Same for ethernet (I guess you need Ethernet?)
About the clock config, you probably test it before adding the Error_handler()?
The value used set the freq at 675 MHz because the default HSE_VALUE is 25 MHz.
I guess you have a 8MHz ? In that case HSE_VALUE have to be defined in the variant_ETHERCAT_DUINO.h

#define HSE_VALUE 8000000

My update are based on the variant.h definition. With a schematics or documentation it is easier to properly review it.

[pieterjanbuntinx]

Hi @fpistm,

Thank you for the review! (please ignore the accidental closing and reopening of this PR, I'm kind of a Github noob)
I've merged the modifications in your fork with mine.

• I now have fixed the analog Pin definitions. They now should be correctly defined and they all work physically (I've tested them).
• I've added pin definitions for DAC because they are physically present on the board (I really have to add a pin definition to the manual)
• USB pins added
• readme merged
• boards.txt merged
• removed unused HAL modules (QSPI and ETHERNET are used in the library so these HAL modules have to be defined in the library itself)
• merged the clock config (changes in clock config for QSPI, ETHERNET and I2C have to be made in the library)

[fpistm]

Thanks @pieterjanbuntinx
About the clock config what is the value of your HSE ? About QSPI, ETHERNET and I2C clock config you should have nothing to do if you used library. Anyway if you manage them in your library then yes you will have to handle them.
I will fetch again the PR and review it.

[pieterjanbuntinx]

@fpistm , the HSE value is 8 MHz. I see I then may have merged a little too much from your modifications. I'm going to test all of this thoroughly with my board and I'll get back to you on this.

[fpistm]

@fpistm , the HSE value is 8 MHz. I see I then may have merged a little too much from your modifications. I'm going to test all of this thoroughly with my board and I'll get back to you on this.

So you have to add this in the variant header:
#define HSE_VALUE 8000000

[pieterjanbuntinx]

Hi @fpistm,

How can I properly override the SystemClock_Config() function in my library? I tried copy+pasting my initial systemclock_config function to the library and calling it in the init function but that doesn't seem to apply everything. I have to change these two:

  RCC_OscInitStruct.PLL.PLLM = 25;
  RCC_OscInitStruct.PLL.PLLN = 432;

in SystemClock_config in the variant.cpp file back to respectively 4 and 216 to make the USB serial work again. I also have to define the following in the variant.h file to get my QSPI for the EtherCAT chip working:

#if !defined(HAL_QSPI_MODULE_DISABLED)
  #define HAL_QSPI_MODULE_ENABLED
#endif

Copying the same lines into my main library file doesn't seem to work. In hal_conf.custom.h #define HAL_QSPI_MODULE_ENABLED is defined, I would think that would be enough for everything to work.

[fpistm]

@pieterjanbuntinx
as the system clock config relies to your variant you can simply do what you need in this one. To redefine the clock config if the file is a cpp file then you need to add extern "C" before.
https://github.com/stm32duino/wiki/wiki/Custom-definitions#systemclock_config

Same for the QSPI enable it by default in the variant. I don't think the hal_conf_custom.h works if you add it in the library as library is build after the core moreover adding it in the library would prevent other user to add it. If you really need it then it should be in sketch example like this a user can complet it and the core will use it.
https://github.com/stm32duino/wiki/wiki/HAL-configuration#customize-hal-or-variant-definition

I will review this PR and add proper config.

[pieterjanbuntinx]

@fpistm
I already was using the extern "C" but omitting the WEAK keyword seemed to make it work now.
hal_conf.custom.h wasn't needed after all so I removed it from my library.
I've committed the changes in the variant.h file.

[fpistm]

What are the pins used by the QSPI and the Ethernet? Seems they are not defined?
Do you used SD?

[pieterjanbuntinx]

Ethernet of the STM32 chip is not used, the Ethernet jacks are driven by an external controller that is interfaced using QSPI. The QSPI pins are configured internally in the library using the STM32 method instead of Arduino's. No, we don't use SD

[fpistm]

Ethernet of the STM32 chip is not used, the Ethernet jacks are driven by an external controller that is interfaced using QSPI. The QSPI pins are configured internally in the library using the STM32 method instead of Arduino's. No, we don't use SD

OK it is clear, anyway some SQPI pins are not defined, mainly PB2 for the clock so probably other one are not defined.
I've almost finished to clean up the PR it misses only those information.

[pieterjanbuntinx]

I'll add all the pin definitions used to communicate with EtherCAT chip

[fpistm]

OK.
I've updated my branch with all the fix (including the correct clock config with an HSE at 8MHz
https://github.com/fpistm/Arduino_Core_STM32/tree/pr-1414-ethercatduino-review

[pieterjanbuntinx]

I have now added all the used pins definitions of the EtherCAT controller. I have also reverted the PLLM and PLLN values. Otherwise USB virtual serial won't work.

[fpistm]

Thanks @pieterjanbuntinx
I've updated my branch with the new pins.
Anyway your branch is not complete you don't have the PeripheralPins_ETHERCAT_DUINO.c
I've also made some formatting and other minor update. That should be fine you replace your branch by the mine which is also squashed in only one commit.

[fpistm]

@pieterjanbuntinx I also can push my branch on your fork and replace your branch if it is more simple for you?
Just a tip for future PR. Avoid to use the master/main branch this ease to keep your fork synced with the original repo and rebase.

[pieterjanbuntinx]

Hi @fpistm
I've already merged your changes but that could be easier in the future :)
I think now all should be fine except perhaps the ALT pins definitions.

[fpistm]

Your merge has some side effect, you have reverted last commits of the original repo (as your master was not synced).

• One pins ordering issue.

[fpistm]

LGTM

[pieterjanbuntinx]

Great! Thanks for approving and all your help!

[fpistm]

FYI, when you create a PR by default you allow maintainers to edit. This means you allow me to push on your master branch as you used this branch for the PR. That's why I always advice to create a dedicated branch for PR to avoid master edition on your fork.

Note that you created some tags on your master "1" and "2.0.1". They are always present.

[fpistm]

Great! Thanks for approving and all your help!

Welcome, anyway it would be good you do a final tests session to avoid any issue. Rerun check variant and stuff you consider useful to validate your board (USB, Serial, I2C, SPI,...)

When done let me know if all is OK then we could merge.

[pieterjanbuntinx]

FYI, when you create a PR by default you allow maintainers to edit. This means you allow me to push on your master branch as you used this branch for the PR. That's why I always advice to create a dedicated branch for PR to avoid master edition on your fork.

Okay, I'll try to think of that next time I do a PR. I've given you access now.

Great! Thanks for approving and all your help!

Welcome, anyway it would be good you do a final tests session to avoid any issue. Rerun check variant and stuff you consider useful to validate your board (USB, Serial, I2C, SPI,...)

When done let me know if all is OK then we could merge.

At the moment I'm busy doing that, I will let you know if everything works.

[fpistm]

Okay, I'll try to think of that next time I do a PR. I've given you access now.

Thanks but no need to invite me. It is a feature of the PR mechanism 😉
https://docs.github.com/en/github/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

[fpistm]

Hi @pieterjanbuntinx

At the moment I'm busy doing that, I will let you know if everything works.

Is this OK?

[pieterjanbuntinx]

Hey @fpistm
I'm currently having some problems gettings all peripherals to work. For example, HardwareSerial serial(PC10, PC11) (UART4 or 3) works perfectly but doing the same for PE7 and PE8 (UART7) or the other UARTs don't seem to work. The same for I2C, I2C1 on PB8 and PB9 works perfectly but the other I2C busses won't work. I also wanted to add support for the remaining Analog pins, the DAC0 and DAC1 pins (PA5, PA4) and D44 and D45 (PF4, PF3), but those also won't work. I was thinking this all has to do with Alternate Functions but those seem to be correctly set in the PeripheralPins_*.c file. I tried using for example HardwareSerial serial((PE8 | ALT8),(PE7 | ALT8) but that doesn't seem to solve it. Am I still missing something?

All pins work perfectly as digital input or output and A0 to A12 also work perfectly as Analog inputs. I've attached a pin diagram I have created for our board.

[fpistm]

I tried using for example HardwareSerial serial((PE8 | ALT8),(PE7 | ALT8)

There is no link between AF (Alternate Function) and the ALTx pins which stands for alternate pins for a peripheral.
So simply use HardwareSerial serial(PE8, PE7);

For the ALTx usage, an example is if you want to use USART3 with PC10 and PC11 then you have to do:
HardwareSerial serial(PC11_ALT1, PC10_ALT1);

About the DAC it is managed by default when you used the analogWrite() API with those pins.

[pieterjanbuntinx]

Hey @fpistm,

There is no link between AF (Alternate Function) and the ALTx pins which stands for alternate pins for a peripheral.
So simply use HardwareSerial serial(PE8, PE7);

That's the way I started doing it. I was just experimenting to try and get it to work. Currently the following works correctly: I2C4 (PD12 PD13), I2C1 (PB8 PB9), UART6 (PC6 PC7), UART6 (PG9, PG14), UART4 (PC10, PC11), UART3 (PC10, PC11), UART2 (PA2, PA3), UART4 (PA0, PA1), all analog pins except the 4 listed below, all digital inputs/outputs.
The following won't work: I2c4 (PF14, PF15), I2c2 (PB11, PB10), I2C3 (PA8, PC9), UART1 (only tx broken out -> PA9), UART7 (PE7, PE8), UART3 (PB11, PB10), UART2 (PD6, PD5), both DAC outputs (PA5, PA4) as DAC or as ADC and PF4 and PF3 used as ADC.

I've tried this on multiple boards so hardware failure seems unlikely. I've done most testing on PlatformIO but I've now also tried most of the testing in the Arduino IDE by flashing over DFU.

[fpistm]

@pieterjanbuntinx
Why have you merged branch... you add again commits in this PR not related to it...

About your issue with the pins I have no clue for you. This is generic and know to work.

[pieterjanbuntinx]

@fpistm sorry, I shouldn't have done that. I've seen you have reverted it.

I'll try to see with the debugger if the registers get correctly set. Have you had anything similar to this before?

[fpistm]

Don't know how you managed your branch but you loose some commit from the master.
I've realigned it properly but this is the last time. So please pay attention and fetch your master branch.

[fpistm]

I've fix the astyle issue.

I'll try to see with the debugger if the registers get correctly set. Have you had anything similar to this before?

No, I've tested the Nucleo F746 and this is fine.

[fpistm]

Just to confirm, I've retest with the Nucleo F746ZG:

• I2c4 (PF14, PF15)
• UART3 (PB11, PB10

About this: UART1 (only tx broken out -> PA9)
You can configure the Serial to be half duplex so only Tx pin are required --> https://github.com/stm32duino/wiki/wiki/API#enable-half-duplex-mode

[pieterjanbuntinx]

Hey @fpistm,
Thank you for retesting! I have just found out I have made a stupid mistake. The order of one header of pins was drawn inverted on the schematics relative to the PCB design. This explains some of the non-working peripherals. I have also found out I have a Nucleo F746ZG board over here so I can now check everything out on there when in doubt. I'll try your suggestion for UART1.
Sorry for the trouble. I will post a new comment here when everything is fixed.

[pieterjanbuntinx]

Hey @fpistm,
Thanks for testing! I have now solved all issues I was having. I made the stupid mistake to look at the wrong pinout. I also found out we have a Nucleo F746 so I was able to double-check it on there as well.

[fpistm]

@pieterjanbuntinx
Fine!
So is this PR now OK on your side ?

[pieterjanbuntinx]

Yes, for me it's ready to be merged :)

[fpistm]

LGTM