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

Feature/bsp for nucleo f303re #376

Merged
merged 1 commit into from
Apr 14, 2020
Merged

Feature/bsp for nucleo f303re #376

merged 1 commit into from
Apr 14, 2020

Conversation

luxarf
Copy link
Contributor

@luxarf luxarf commented Apr 13, 2020

I would like to use a Nucleo F303RE with modm and therefore added board support for this development board.

This pull request contains nothing major, the board support files are slightly modified copies of the very similar Nucloe F303K8, corrected for the different form factor.

The examples are slightly modified copies from the examples for the aforementioned Nucleo F303K8 and the Nucleo F103RB.

I verified that the examples are running on the Nucleo F303RE Board that is available to me.

What else needs to be done to make this Pull Request mergeable?

rleh
rleh previously requested changes Apr 13, 2020
Copy link
Member

@rleh rleh left a comment

Choose a reason for hiding this comment

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

Hi @luxarf, thanks for your first contribution to modm!

The pull request looks mostly fine to me!

examples/nucleo_f303re/blink/main.cpp Show resolved Hide resolved
src/modm/board/nucleo_f303re/board.hpp Show resolved Hide resolved
src/modm/board/nucleo_f303re/module.lb Show resolved Hide resolved
Copy link
Member

@salkinium salkinium left a comment

Choose a reason for hiding this comment

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

I think the copyright is fine for files you only copied, but the module.lb should have your name.
Apart from that, can you squash your commits onto just one?

@salkinium
Copy link
Member

(We really need to generate these examples from a common source, this is copy-paste is really getting out of control 🤪🙈)

@rleh
Copy link
Member

rleh commented Apr 13, 2020

See #359 😉

@luxarf
Copy link
Contributor Author

luxarf commented Apr 13, 2020

After a little back and forth with git (I am doing a squash for the first time), I am hopeful I got everything.

@salkinium
Copy link
Member

Hm sorry, I forgot: You also need to add your new examples to the CI in the .circleci/config.yml:

- (cd examples && ../tools/scripts/examples_compile.py stm32f3_discovery nucleo_f303k8)
+ (cd examples && ../tools/scripts/examples_compile.py stm32f3_discovery nucleo_f303k8 nucleo_f303re)

I need to write a test that checks that all examples are in the CI, I've also added examples and forgot to add them to the CI. Whoops.

@luxarf
Copy link
Contributor Author

luxarf commented Apr 14, 2020

Although the ci checks pass, I am seeing deprecation warnings.
I suspect that stems from the fact that I copied the itm example about a week ago and rebased my branch onto develop before creating this pull-request. Obviously I did not exhibit the necessary care before determining the examples were ready.
But I am glad the CI catched it. :-)

Copy link
Member

@salkinium salkinium left a comment

Choose a reason for hiding this comment

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

Excellent work, thanks!

@salkinium salkinium merged commit 141aa71 into modm-io:develop Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants