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

Add STM32F446VE controller board #522

Closed
wants to merge 1 commit into from

Conversation

petervanaken
Copy link

@petervanaken petervanaken commented May 16, 2019

Summary

This PR implements the following : Introduction of a new board called VAkE v1.0. This board is based upon the STM32F446VE. It can be used as 3D printer / CNC controller board with versatile stepper configuration.
All comments are welcome.

Cheers,
Peter

@fpistm fpistm self-requested a review May 16, 2019 12:12
Copy link
Member

@fpistm fpistm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @petervanaken
Thanks for the contribution.
I've merged yesterday an update on how the HAL configuration is done (See #518).
I've push a branch on my fork with all fixes mentioned in this review and rebased on top of the master --> https://github.com/fpistm/Arduino_Core_STM32/tree/PR-522-reviewed
Feel free to test and customize it. I will wait your question/feedback.
When you think you variant is ready made a force pushed on your branch to update this PR.
Thanks in advance

One more thing:
Please, could you provide a link on this board?

boards.txt Show resolved Hide resolved
@@ -758,6 +758,54 @@ GenF4.menu.upload_method.bmpMethod=BMP (Black Magic Probe)
GenF4.menu.upload_method.bmpMethod.upload.protocol=gdb_bmp
GenF4.menu.upload_method.bmpMethod.upload.tool=bmp_upload

###############################
Vake403d.name=VAkE 403d
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is Vake403d the manufacturer name or generic branding name ?
or is the board name ?
I guess it should be best to add it to 3dprinter entry

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just saw in your OP the board name is VAkE v1.0
I've update my branch with this name. Let me know if it's ok.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The boads name is VAkE but will first be tested in a 3D printer called Vake403d.
The update of the branch is fine, thx.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I guess the best is to use the PCB name (board name).
Vake403d is one use case of this board? right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how I've update it:
https://github.com/fpistm/Arduino_Core_STM32/blob/50713653535c1c8dbcd38fd9ca42253fc0186f5f/boards.txt#L935-L944

# VAkE v1.0
3dprinter.menu.pnum.VAKE_F446VE=VAkE v1.0
3dprinter.menu.pnum.VAKE_F446VE.upload.maximum_size=524288
3dprinter.menu.pnum.VAKE_F446VE.upload.maximum_data_size=131072
3dprinter.menu.pnum.VAKE_F446VE.build.mcu=cortex-m4 -mfpu=fpv4-sp-d16 -mfloat-abi=hard
3dprinter.menu.pnum.VAKE_F446VE.build.board=VAKE403
3dprinter.menu.pnum.VAKE_F446VE.build.series=STM32F4xx
3dprinter.menu.pnum.VAKE_F446VE.build.product_line=STM32F446xx
3dprinter.menu.pnum.VAKE_F446VE.build.variant=VAKE_F446VE
3dprinter.menu.pnum.VAKE_F446VE.build.cmsis_lib_gcc=arm_cortexM4l_math

# Vake F446VE
# Support: Serial1 (USART1 on PA10, PA9)
# Default SPI: SPI
Vake403d.menu.pnum.VAKE_F446VE=Vake F446VE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is It really the board name?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I change "Vake F446VE" to "VAkE" ? VAkE is infact the name of the PCB, VAke403d the name of the test printer for the PCB and since it's based on the F446...it started it's live a "Vake F446VE"

Vake403d.menu.pnum.VAKE_F446VE.build.cmsis_lib_gcc=arm_cortexM4l_math

# Upload menu
Vake403d.menu.upload_method.STLink=STLink
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since #514 STM32CubeProgrammer replace those upload method.

Vake403d.menu.upload_method.serialMethod.upload.tool=serial_upload
Vake403d.menu.upload_method.serialMethod.build.extra_flags_serial_auto=-DMENU_SERIAL_AUTO=SerialUART1

Vake403d.menu.usb.SerialUSB=Serial [Virtual COM port, PA11/PA12 pins]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems you worked before on STM32GENERIC core 😉
Theses definitions are not for this core.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the definitions. Note : next version will have the capability to use serial to upload new binaries without the need for an ST link programmer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already the case thanks the STM32CubeProgrammer use STM32 built-in bootloader.

/*----------------------------------------------------------------------------
* Headers
*----------------------------------------------------------------------------*/
#include "PeripheralPins.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No more include since #518

Suggested change
#include "PeripheralPins.h"

/*----------------------------------------------------------------------------
* Pins
*----------------------------------------------------------------------------*/
extern const PinName digitalPin[];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Define elsewhere since #518

Suggested change
extern const PinName digitalPin[];


// Timer Definitions
// Do not use timer used by PWM pins when possible. See PinMap_PWM.
#define TIMER_TONE TIM6
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if not needed could be removed

#define TIMER_TONE TIM6

// Do not use basic timer: OC is required
#define TIMER_SERVO TIM2 //TODO: advanced-control timers don't work
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if not needed could be removed

// pins are NOT connected to anything by default.
#define HAVE_HWSERIAL4
#define HAVE_HWSERIAL1
#define SERIAL_PORT_MONITOR Serial4
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use Generic Serial instance.
Like this if USB CDC is used then this will also work

Suggested change
#define SERIAL_PORT_MONITOR Serial4
#define SERIAL_PORT_MONITOR Serial

@petervanaken
Copy link
Author

petervanaken commented May 17, 2019 via email

@fpistm
Copy link
Member

fpistm commented May 17, 2019

Thanks for the link @petervanaken
The main issue is you do not rebased your PR on top of the master.
As mentioned in my review I've merged an update few days ago. That's why it's not build as the CI build try to build the PR on top of the master.

I've push a branch on my fork with all fixes mentioned in this review and rebased on top of the master --> https://github.com/fpistm/Arduino_Core_STM32/tree/PR-522-reviewed
Feel free to test and customize it. I will wait your question/feedback.
When you think you variant is ready made a force pushed on your branch to update this PR.
I think the easiest way is you use it.

To see why Travis failed simply click "Show all checks" then on "Details".

@petervanaken
Copy link
Author

petervanaken commented May 17, 2019 via email

@fpistm
Copy link
Member

fpistm commented May 17, 2019

and have no access to my hardware it will be for next week.

This is done in my fork...

@petervanaken
Copy link
Author

petervanaken commented May 17, 2019 via email

@fpistm
Copy link
Member

fpistm commented May 17, 2019

What should I do to push your branch ?

Simply push it forced on the same branch used for this PR.
You used master one.
so :
git push -f master

Note that I do not advise use master for PR. Use master to sync your fork up to date. Use an explicit branch name to make PR.
Anyway this is done so let's continue with it ;)

@petervanaken
Copy link
Author

petervanaken commented May 17, 2019 via email

@fpistm
Copy link
Member

fpistm commented May 17, 2019

Specify your git remote name of your fork.
To see it
git remote -v
Then
git push -f < your repo name > master

@fpistm
Copy link
Member

fpistm commented May 17, 2019

If you want I can submit my branch as a PR then we will review it and fix issue if any

@petervanaken
Copy link
Author

petervanaken commented May 17, 2019 via email

@fpistm
Copy link
Member

fpistm commented May 17, 2019

Replace by #526

@fpistm fpistm closed this May 17, 2019
STM32 core based on ST HAL automation moved this from In progress to Done May 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants