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

Add tone support to StandardFirmata. #204

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

damellis
Copy link
Member

@damellis damellis commented Jun 1, 2015

As described here:
https://github.com/firmata/protocol/blob/master/tone-proposal.md

The tone branch provides support for tone() and noTone() in ConfigurableFirmata, but it would be great to have them in StandardFirmata too.

@soundanalogous
Copy link
Member

I've been hesitant to add Tone to StandardFirmata, especially since it has been proven to achieve using digital write alone. However the use of Tone would be more efficient.

If including Tone consumes very little memory and is not likely to create conflicts with other features in StandardFirmata it may be okay to add. I just want to make sure it won't prevent adding features (in terms of memory usage and any potential timer or other resource conflicts) such as SPI and Software/Hardware Serial.

@@ -41,6 +41,7 @@

// extended command set using sysex (0-127/0x00-0x7F)
/* 0x00-0x0F reserved for user-defined commands */
#define TONE_DATA 0x5F // send a tone or noTone command
Copy link
Contributor

Choose a reason for hiding this comment

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

TONE_MESSAGE

@damellis
Copy link
Member Author

damellis commented Jun 2, 2015

Wow, I figured that sending repeated digital write commands would be too slow or jittery to generate good tones. Do you have a sense of how well that works?

This pull request uses a little bit of memory. Flash usage (on the Uno) goes from 12,192 bytes (37%) to 13,732 bytes (42%) and RAM usage from 940 bytes (45%) to 963 bytes (47%).

I would agree that serial and SPI are more important than tone, but it doesn't seem so likely that these bytes will make the difference in being able to support serial and SPI. Do you have a sense of how much memory those features will need?

I don't think there should be timer conflict problems because, I think, SPI and serial don't use the timers. (SoftwareSerial uses the pin change interrupts and tuned assembly delays. The rest are handled by their own hardware, I believe.)

@soundanalogous
Copy link
Member

I still need to do memory profiling for SPI and Serial. Serial may use a fair amount of memory depending on where I land with the buffer situation. I haven't looking into SPI support further than an initial protocol (in that I haven't written any code yet) so that will take more work before I have a good idea about memory consumption for SPI.

Tone doesn't consume too much RAM which is nice. One thing to test with your branch is using Tone, a servo and PWM (adjust LED brightness for example) simultaneously and in a few different pin combinations (using PWM pins). See if anything unexpected happens in any of those combinations.

Another thing you'll need to add to your branch is call noTone() on any TONE pins in the system reset callback function.

Wow, I figured that sending repeated digital write commands would be too slow or jittery to generate good tones. Do you have a sense of how well that works?

@rwaldron would know (this is regarding the performance of piezo.js)

@rwaldron
Copy link
Contributor

rwaldron commented Jun 2, 2015

Actually, let's get @Resseguie @julian_duque @lyzadanger to weigh in on that, since they've done the most work with Piezo

@soundanalogous
Copy link
Member

NM about calling noTone() in system reset, I see now that will happen in lines 235-237 when the pins mode are reset in the system reset callback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants