sensorium / Mozzi

sound synthesis library for Arduino
https://sensorium.github.io/Mozzi/
GNU Lesser General Public License v2.1
1.07k stars 186 forks source link

Added experimental support for SAMD51 #135

Closed diyelectromusic closed 2 years ago

diyelectromusic commented 2 years ago

I've attempted to add support for SAMD51 microcontrollers that are using the Adafruit SAMD51 Arduino core (the official Arduino SAMD core doesn't support the SAMD51, only the SAMD21).

This has largely mirrored the support already existing for SAMD21 boards but is using hardware timer TC0 rather than TC5. Key to differentiating the two architectures is the test for the SAMD51 define that is set in the Adafruit Arduino core for SAMD51 boards (there isn't a corresponding SAMD21 definition as far as I can see).

I propose that the test for SAMD21 boards becomes:

define IS_SAMD21() (defined(ARDUINO_ARCH_SAMD) && !IS_SAMD51())

There is more background information for this change in this blog post: https://diyelectromusic.wordpress.com/2021/04/16/samd-usb-midi-multi-pot-mozzi-synthesis/

I believe this functions as required on SAMD51, SAMD21 and AVR architectures, but I can't compile or test for any others.

(I am still a bit of a GitHub newbie, so please forgive any GitHub etiquette mistakes! I think I've done the right things for this PR but do please correct me if necessary!)

Many Thanks, Kevin

tfry-git commented 2 years ago

Hi!

Thanks for sharing (and sorry about the long delay). Definitely something to add.

Unfortunately - as you have no witnessed - MozziGuts.cpp has grown into quite one hell of #ifdefs. I think the time has come to try to sort this out, and I hope to make a proposal on that, soon. However, this means you patch will have to wait a little longer, and it will need some adjustments.

So, this is just a heads-up, that your work is being seen, and it will be merged, eventually.

Regards Thomas

diyelectromusic commented 2 years ago

No problem, thanks for getting back to me. As you can (obviously) see I was trying to resync my own fork with the latest Mozzi (and mostly failing... I need to find a simple repository to learn about git properly). I hope I haven't broken anything here, I was trying to be really careful, so my apologies if I've just created more work for everyone... It might be easier if I just nuke my fork and start again keeping things in separate branches! In short do whatever you need to do to close this off and I'll revisit it at a better time in the future!

Yes MozziGuts is a bit of a monster! I noticed that pschatzmann has already done some refactoring as part of their own fork for their own purposes to add in RP2040 support (something else I'm interested in). I'll also look at the Nano Every (ATmega4809) too, but wait until things settle down before I try anything else here. I might just send someone else the changes when the time comes :)

I wish I could help more but suspect my knowledge of C++ templates and git is more of a liability than an asset at present.

Thanks for all your (and everyone's) work on Mozzi. It is giving me a lot of fun at the moment and I do really appreciate it (as indicated by the number of articles on my blog!) :)

Once again, my apologies if I've just caused more work.

Best wishes, Kevin

tfry-git commented 2 years ago

Yes MozziGuts is a bit of a monster! I noticed that pschatzmann has already done some refactoring as part of their own fork for their own purposes to add in RP2040 support (something else I'm interested in).

Yes, it's essentially what @pschatzmann was suggesting, except with some more manual work in order to slice out the parts that are (and should remain) common between the platforms.

I wish I could help more but suspect my knowledge of C++ templates and git is more of a liability than an asset at present.

A PR never hurts. But yes, one of those git lessons that everybody learns the hard way is: "always work in branches".

Regards Thomas

diyelectromusic commented 2 years ago

Closing for now - will attempt a new PR when the refactoring described above has settled down!