Skip to content

Don't let users play a tone equal or less than 0 #55

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
Merged
merged 1 commit into from
Feb 15, 2021
Merged

Don't let users play a tone equal or less than 0 #55

merged 1 commit into from
Feb 15, 2021

Conversation

KTibow
Copy link
Contributor

@KTibow KTibow commented Feb 15, 2021

Previously, this would have caused a crash.

FoamyGuy
FoamyGuy approved these changes Feb 15, 2021
View reviewed changes
Copy link
Contributor

@FoamyGuy FoamyGuy left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I tested this change successfully with Adafruit CircuitPython 6.2.0-beta.2 on 2021-02-11; Adafruit MagTag with ESP32S2

I did not ever witness it raise an exception from a negative frequency value, it always seem to fail silently for me (No error, but also no sound played)

This change does fix it though by checking for negative value and sending a message to the user to let them know it needs to be positive.

Thanks for working on this fix!

@KTibow
Copy link
Contributor Author

KTibow commented Feb 15, 2021

I found it hard crash for me:
image
image

@FoamyGuy
Copy link
Contributor

@KTibow Ah I see. I was testing with negative values (I tried -10 and -100 at first) those seem to fail silently for me.

0 specifically though does result in a hard crash and device reset for me as well.

Good catch! Thank you for finding this and submitting a fix!

makermelissa
makermelissa approved these changes Feb 15, 2021
View reviewed changes
Copy link
Collaborator

@makermelissa makermelissa left a comment

Choose a reason for hiding this comment

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

Thanks for finding and fixing this.

@makermelissa makermelissa merged commit 491d0ee into adafruit:main Feb 15, 2021
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Feb 16, 2021
@adafruit-adabot
Automated update by Adabot (adafruit/adabot@f49a93b)
4dd2a11 
Updating https://github.com/adafruit/Adafruit_CircuitPython_IS31FL3731 to 2.7.0 from 2.6.7:
  > Merge pull request adafruit/Adafruit_CircuitPython_IS31FL3731#35 from sandyjmacdonald/master

Updating https://github.com/adafruit/Adafruit_CircuitPython_MagTag to 1.6.1 from 1.6.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_MagTag#55 from KTibow/patch-1

Updating https://github.com/adafruit/Adafruit_CircuitPython_MatrixPortal to 2.1.1 from 2.1.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_MatrixPortal#62 from makermelissa/master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FoamyGuy FoamyGuy FoamyGuy approved these changes

@makermelissa makermelissa makermelissa 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
@KTibow @FoamyGuy @makermelissa