Skip to content

Conversation

beserge
Copy link
Contributor

@beserge beserge commented Oct 19, 2021

Summary

This PR fixes/implements the following bugs/features

  • Add Daisy Patch SM variant

This PR adds the Daisy Patch SM as a variant.

The Patch SM is an upcoming hardware platform from Electrosmith following in the footsteps of the Daisy Seed.

The one thing we might want to change is the ordering of the Analog pins (A0, A1, etc).
We tried simply reordering the analogInputPin array, which worked for some pins, but eventually just broke.

@beserge beserge marked this pull request as draft October 19, 2021 22:12
@beserge beserge marked this pull request as ready for review October 21, 2021 15:43
@fpistm fpistm self-requested a review October 22, 2021 17:11
@fpistm fpistm added the new variant Add support of new bard label Oct 22, 2021
@fpistm
Copy link
Member

fpistm commented Oct 22, 2021

Hi @beserge
I will review it next week.
I guess it should be fine to wait the final board version? or is it ready and the pin-out will not change?

@beserge
Copy link
Contributor Author

beserge commented Oct 22, 2021

@fpistm The hardware is finalized, so the pinout won't change. Do you have any advice for setting up those Analog Pins I mentioned?

@fpistm
Copy link
Member

fpistm commented Oct 22, 2021

I wiil check deeply on monday.

the PIN_Ax defined in the .h should match in the analogInputPin[] of the .c:
If you revert PIN_A1 and PIN_A2, then the number in the array have to be reverted also.
The number is the index in the digitalPin array in the .c.

@beserge
Copy link
Contributor Author

beserge commented Oct 22, 2021

@fpistm I was able to get them working, thanks for the help!

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.

PR LGTM
Anyway it misses:

  • a reference in the README.md
  • Clean up useless pin definitions
  • Squash all commit in one
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.

Create variant .h .cpp and pinperiphs.c files Use daisy seed linker script for patch sm Add variant to boards.txt Add Patch SM with link to README.md
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.

LGTM
Is it OK to merge?

@stephenhensley
Copy link

Everything checks out on our end. OK to merge.

Thanks for all the thorough reviews.

@fpistm fpistm merged commit 342cda8 into stm32duino:main Oct 29, 2021
@fpistm
Copy link
Member

fpistm commented Oct 29, 2021

Thanks for all the thorough reviews.

Welcome.
Thank you for following them 😉

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
3 participants