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.

Sign up for GitHub

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

Jump to bottom

synthio: improvements based on feedback #8024

Merged
merged 4 commits into from
May 24, 2023
Merged

synthio: improvements based on feedback #8024

merged 4 commits into from
May 24, 2023

Conversation

jepler
Copy link
Member

@jepler jepler commented May 24, 2023

Thanks @todbot!

  • Incompatible change: Synthesizer.blocks replaces Synthesizer.lfos; the functionality is unchanged
  • Incompatible change: voct_to_hz added, replaces onveo_to_hz (which was totally incorrect in its implementation).
  • Docs: correctly show filter in the Synthesizer constructor
  • Docs: correctly show the type of Note.frequency as float.

jepler added 4 commits May 23, 2023 19:38
@jepler
synthio: fix, rename voct_to_hz
1847c2b 
todbot discovered that this function wasn't working right, and wasn't
well-named.
@jepler
synthio: note frequency can only be a float, for now
c20e862
@jepler
synthio: rename synthesizer.lfos to .blocks
11d8a6e 
.. since math blocks can be placed in it too (and that's useful)
@jepler
synthio: list filter in the constructor
9f138dc
@jepler jepler marked this pull request as ready for review May 24, 2023 01:41
gamblor21
gamblor21 approved these changes May 24, 2023
View reviewed changes
Copy link
Member

@gamblor21 gamblor21 left a comment

Choose a reason for hiding this comment

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

Tested on a Metro M7. blocks and voct_to_hz changes work. Documentation changes look good.

dhalbert
dhalbert approved these changes May 24, 2023
View reviewed changes
Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Seems fine!

@dhalbert dhalbert merged commit 9359d81 into adafruit:main May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gamblor21 gamblor21 gamblor21 approved these changes

@dhalbert dhalbert dhalbert approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jepler @dhalbert @gamblor21