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

Avoid unnecessary waveform de-initialization #4913

Merged
merged 1 commit into from
Jul 10, 2018
Merged

Avoid unnecessary waveform de-initialization #4913

merged 1 commit into from
Jul 10, 2018

Conversation

shimarin
Copy link
Contributor

which harms frequent digitalRead/Write like SoftwareSerial.

shimarin referenced this pull request Jul 10, 2018
Remove and rewrite all the parts of the core/libraries using TIMER1
and consolidate into a single, shared waveform generation interrupt
structure.  Tone, analogWrite(), Servo all now just call into this
shared resource to perform their tasks so are all compatible
and can be used simultaneously.

This setup enables multiple tones, analogWrites, servos, and stepper
motors to be controlled with reasonable accuracy.  It uses both TIMER1
and the internal ESP cycle counter to handle timing of waveform edges.
TIMER1 is used in non-reload mode and only edges cause interrupts.  The
interrupt is started and stopped as required, minimizing overhead when
these features are not being used.

A generic "startWaveform(pin, high-US, low-US, runtime-US)" and
"stopWaveform(pin)" allow for further types of interfaces.  Minimum
high or low period is ~1 us.

Add a tone(float) method, useful when working with lower frequencies.

Fixes #4321.  Fixes 4349.
@earlephilhower
Copy link
Collaborator

Looks good to me, thanks! Like I was mentioning in the original discussion, we may want to remove the stopWaveform call completely from digitalRead as it feels like anyone doing a read on a pin playing an output should expect to get garbage, anyway.

Also, we could move the .enabled check to the beginning of the condition since it evaluates LTR and it's not often that pins are actually playing a tone. Would save some cycles in the digitalWrite case as well.

We could also set up an inverse map and make this O(1) for pin->waveform entry. No for needed then.

If the SWSerial still has issues, let's try some of these.

@earlephilhower earlephilhower merged commit e6af980 into esp8266:master Jul 10, 2018
@shimarin
Copy link
Contributor Author

shimarin commented Jul 11, 2018

After posting PR, I noticed that by adding

if (!timerRunning) return false;

on the very start of stopWaveform helps to avoid searching the table everytime digitalRead is called entirely in case waveform is not used at all. (I assume the timer always is running during any of pins use waveform)

Just removing stopWaveform from digitalRead still sounds a reasonable option, though.

@earlephilhower
Copy link
Collaborator

@shimarin Can you please give PR #4920 a look? I included your very good suggestions and also an optimization for when there actually is a PWM running so it may help you even more with SoftwareSerial and others. Thanks!

@shimarin
Copy link
Contributor Author

@earlephilhower looks good!

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

Successfully merging this pull request may close these issues.

3 participants