Skip to content

Add DIYMROE_STM32F407VGT support #376

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 2 commits into from
Jan 7, 2019
Merged

Conversation

fpistm
Copy link
Member

@fpistm fpistm commented Nov 20, 2018

PR linked to #374 thanks @AndySymons
Reviewed by @fpistm

  • Clean up file format, trailing space and code format
  • Fix some typo
  • Define default SPI pins and I2C used for SPI and Wire instances.
  • Fix analog pins definition

@fpistm fpistm added the new variant Add support of new bard label Nov 20, 2018
@AndySymons
Copy link
Contributor

How do I download the modified core to test it?

@fpistm
Copy link
Member Author

fpistm commented Nov 20, 2018

Best way would be to use git.
Anyway, here the zip of modified file.
Note that boards.txt is the last version (on top of the master branch) and differs from yours (which I guess is the 1.4.0 version)
DIYMROE_F407VGT.zip <-- updated

@AndySymons
Copy link
Contributor

AndySymons commented Nov 21, 2018 via email

@fpistm
Copy link
Member Author

fpistm commented Nov 23, 2018

To use the git repo with the core:
https://github.com/stm32duino/wiki/wiki/Using-git-repository

Then to pull the PR, add in your .gitconfig:

[Alias]
        pr = "!f() { git fetch -fu ${2-origin} refs/pull/$1/head:pr/$1 && git checkout pr/$1; }; f"

then you can do:
git pr 376
This will pull the PR in your git repo in a branch PR/376.
By default, use origin as remote name, to use a custom one:
git pr 376 remote_name

@AndySymons
Copy link
Contributor

AndySymons commented Nov 24, 2018 via email

@fpistm
Copy link
Member Author

fpistm commented Dec 7, 2018

Hi @AndySymons
Did you made some tests?
Is it ok to merge?

@AndySymons
Copy link
Contributor

AndySymons commented Dec 7, 2018 via email

@BennehBoy
Copy link
Contributor

BennehBoy commented Jan 4, 2019

I tested this today, I can get a simple blink sketch to work just fine.

I also tested using something more complex, SPI1 & Usarts are functional.

Main comment is the naming, the manufacturer is called DIYMORE & not DIYMROE (another manufacturer has identical board named CJMCU-407). Although ironically they also misprint the name on some of their boards.

I merged this into the CDC PR but could not get a functional USB Serial - perhaps I missed some changes. (I used the USB config from the Black407VE PinNamesVar.h & Peripheralpins.c

PS I did add a 1.5k pullup resistor between vcc & PA12 as per this page -> http://dubstylee.net/v/diy-more-stm32f407vgt6/ (translate to english with google).

@fpistm
Copy link
Member Author

fpistm commented Jan 4, 2019

@BennehBoy
About the name it seems not to be a mistake. @AndySymons comment on that on the forum.
See picture here http://www.stm32duino.com/viewtopic.php?f=2&t=4153&p=49844#p49806
But if this is a clone maybe we can rename it to DIYMORE to support the "official" one and user having a clone can use it as well.

About USB, the PinNamesVar.h and PeripheralPins.c do not contains USB info. I will update the PR.

@fpistm
Copy link
Member Author

fpistm commented Jan 4, 2019

@AndySymons let me know what you think about the renaming?
If this is the same pin mapping then I guess it could be the best choice

@BennehBoy
Copy link
Contributor

BennehBoy commented Jan 4, 2019

@BennehBoy
About USB, the PinNamesVar.h and PeripheralPins.c do not contains USB info. I will update the PR.

Yes I saw that and copied the appropriate sections from the black407VE variant (also checked against Disco 407VG)

PS, I just tried the device with Roger's core and CDC works although exhibits the same bug with the device params from PR #388. I know this is OT for this PR, just for info.

@fpistm
Copy link
Member Author

fpistm commented Jan 4, 2019

Thanks @BennehBoy
About name I think it should be better to keep it as I'm not able to find an official DIYMORE board.
Even if the brand name is DIYMORE on some sellers all picture show DIYMROE.
https://www.amazon.com/Diymore-STM32F407VGT6-Cortex-M4-Discovery-Development/dp/B01M7YB5HF

@BennehBoy
Copy link
Contributor

Yes it's a bit strange, even in the link you posted the board has diymroe on the upper side, and diymore on the lower 🤣

@fpistm
Copy link
Member Author

fpistm commented Jan 4, 2019

Right, ;) did not see that before. anyway this is only a name but I would not use a brand name if this is not the right one. And for this it seems this is not the right one as not referenced on official DIYMORE site.

@BennehBoy
Copy link
Contributor

👍

@fpistm
Copy link
Member Author

fpistm commented Jan 4, 2019

Ok then I will rebase it and update PinNamesVar.h and PeripheralPins.c to add USB info then merge it.

@BennehBoy
Copy link
Contributor

Ok then I will rebase it and update PinNamesVar.h and PeripheralPins.c to add USB info then merge it.

Hmm, I did try this already and it did not work.... Hopefully your attempt will be more successful.

@fpistm
Copy link
Member Author

fpistm commented Jan 4, 2019

Rebased and pinmap update with USB.

@BennehBoy
Copy link
Contributor

I noticed that you replaced the '0' with 'GPIO_AF_NONE'

	Line 362:   {PA_9,  USB_OTG_FS, STM_PIN_DATA(STM_MODE_INPUT, GPIO_NOPULL, GPIO_AF_NONE)}, // USB_OTG_FS_VBUS
	Line 388:   {PB_13, USB_OTG_HS, STM_PIN_DATA(STM_MODE_INPUT, GPIO_NOPULL, GPIO_AF_NONE)}, // USB_OTG_HS_VBUS

This fails to compile on my system:

error: 'GPIO_AF_NONE' undeclared here (not in a function)

   {PA_9,  USB_OTG_FS, STM_PIN_DATA(STM_MODE_INPUT, GPIO_NOPULL, GPIO_AF_NONE)}, // USB_OTG_FS_VBUS

Where is this defined?

@BennehBoy
Copy link
Contributor

NM I just saw the commit on master.

@fpistm fpistm self-assigned this Jan 5, 2019
@BennehBoy
Copy link
Contributor

I've retested USB CDC FS multiple times now, reconstructing my core each time from a fresh master clone, + pr 388, + manual merge in of the DIYMROE last commit (your force push) + the Bdevice changes.

With or without the TIM changes for 388 I do not get a working USB device (unrecognized)

Using the exact same hardware I get a working USB Serial device using either STM32GENERIC, or Roger's core.

I've tested this on both Windows 10 & Ubuntu 18.04.1 LTS

However, pinpointing the issue is beyond my knowledge/skill level.

@fpistm
Copy link
Member Author

fpistm commented Jan 5, 2019

I will update USB PR with all input next week and do several tests on my side. I've got an USB shield to test across all series. Hum, one things I did not check on this PR is the clock config to ensure USB get a 48MHz clock.

@AndySymons
Copy link
Contributor

AndySymons commented Jan 5, 2019 via email

@BennehBoy
Copy link
Contributor

BennehBoy commented Jan 5, 2019

@AndySymons yeah with hindsight I think the naming makes sense - I searched diymore.cc and they don't offer this product.

@fpistm, looking instm32f4xx_hal_conf.h I can see some differences compared to the black407VE:

there is no typing of most of the defines..

eg BLACK407VET:
#define VDD_VALUE ((uint32_t)3300U

and DIYMROE:
#define VDD_VALUE 3300U

The only other difference (other than where items appear in order), is TICK_INT_PRIORITY,

For BLACK407VET it is: ((uint32_t)0U)

and DIYMROE: 0x0FU

Although this matches the DISCO407VG config.

PS - although I'm probably looking in the wrong place 🤣

@BennehBoy
Copy link
Contributor

OK, I've resolved this....

In Variants.cpp

RCC_OscInitStruct.PLL.PLLM = 25;

Needs to be

RCC_OscInitStruct.PLL.PLLM = 8;

@fpistm
Copy link
Member Author

fpistm commented Jan 5, 2019

Right, this is what I though. In fact HSE oscillator is a 8MHz right ? while I thought it was a 25 MHz.
Thanks for all you help. This is really appreciated 👍 🥇

@BennehBoy
Copy link
Contributor

Right, this is what I though. In fact HSE oscillator is a 8MHz right ? while I thought it was a 25 MHz.

Yes it's 8mhz. I added a PR for this to your DIYMROE branch.

@fpistm
Copy link
Member Author

fpistm commented Jan 5, 2019

I saw it and merge it, so it is now part of this PR. You will be a new contributor ;)
I guess this PR is ready to merge. From my point of view it is OK, even if I could not test it as I do not have the board.

@BennehBoy
Copy link
Contributor

One other small change, this board has 1024KB flash, boards.txt configures only 512KB.

AndySymons and others added 2 commits January 6, 2019 08:03
@fpistm
Copy link
Member Author

fpistm commented Jan 6, 2019

Right PR updated with 1024Kb

@fpistm fpistm added this to the 1.5.0 milestone Jan 7, 2019
@fpistm fpistm merged commit d555758 into stm32duino:master Jan 7, 2019
@fpistm fpistm deleted the DIYMROE branch January 7, 2019 07:59
benwaffle pushed a commit to benwaffle/Arduino_Core_STM32 that referenced this pull request Apr 10, 2019
Add DIYMROE_STM32F407VGT support
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

Successfully merging this pull request may close these issues.

3 participants