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

Compatibility and IRQ fixed for waveform/tone/pwm #4872

Merged
merged 3 commits into from
Jul 2, 2018
Merged

Compatibility and IRQ fixed for waveform/tone/pwm #4872

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a59cc74
Compatibility and IRQ fixed for waveform/tone/pwm
earlephilhower Jul 1, 2018
f55fc77
Merge branch 'master' into clifix
earlephilhower Jul 1, 2018
a812e27
Fix tone(int,int,int) infinite loop
earlephilhower Jul 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cores/esp8266/Arduino.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ unsigned long pulseIn(uint8_t pin, uint8_t state, unsigned long timeout = 100000
unsigned long pulseInLong(uint8_t pin, uint8_t state, unsigned long timeout = 1000000L);

void tone(uint8_t _pin, unsigned int frequency, unsigned long duration = 0);
void tone(uint8_t _pin, int frequency, unsigned long duration = 0);
void tone(uint8_t _pin, double frequency, unsigned long duration = 0);
void noTone(uint8_t _pin);

Expand Down
6 changes: 6 additions & 0 deletions cores/esp8266/Tone.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ void tone(uint8_t _pin, double frequency, unsigned long duration) {
}


// Fix ambiguous tone() binding when adding in a duration
void tone(uint8_t _pin, int frequency, unsigned long duration) {
tone(_pin, frequency, duration);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Correct me, I think this one is calling itself.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

D'oh! I've been away from the keyboard too long. Will fix that.

}


void noTone(uint8_t _pin) {
if (_pin > 16) {
return;
Expand Down
17 changes: 12 additions & 5 deletions cores/esp8266/core_esp8266_waveform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,6 @@
// Need speed, not size, here
#pragma GCC optimize ("O3")

// Map the IRQ stuff to standard terminology
#define cli() ets_intr_lock()
#define sei() ets_intr_unlock()

// Maximum delay between IRQs
#define MAXIRQUS (10000)

Expand Down Expand Up @@ -180,6 +176,12 @@ int startWaveform(uint8_t pin, uint32_t timeHighUS, uint32_t timeLowUS, uint32_t
if (!wave) {
return false;
}

// To safely update the packed bitfields we need to stop interrupts while setting them as we could
// get an IRQ in the middle of a multi-instruction mask-and-set required to change them which would
// then cause an IRQ update of these values (.enabled only, for now) to be lost.
ets_intr_lock();

wave->nextTimeHighCycles = MicrosecondsToCycles(timeHighUS) - 70; // Take out some time for IRQ codepath
wave->nextTimeLowCycles = MicrosecondsToCycles(timeLowUS) - 70; // Take out some time for IRQ codepath
wave->timeLeftCycles = MicrosecondsToCycles(runTimeUS);
Expand All @@ -193,13 +195,19 @@ int startWaveform(uint8_t pin, uint32_t timeHighUS, uint32_t timeLowUS, uint32_t
}
ReloadTimer(MicrosecondsToCycles(1)); // Cause an interrupt post-haste
}

// Re-enable interrupts here since we're done with the update
ets_intr_unlock();

return true;
}

// Stops a waveform on a pin
int stopWaveform(uint8_t pin) {
for (size_t i = 0; i < countof(waveform); i++) {
if (((pin == 16) && waveform[i].gpio16Mask) || ((pin != 16) && (waveform[i].gpioMask == 1<<pin))) {
// Note that there is no interrupt unsafety here. The IRQ can only ever change .enabled from 1->0
// We're also doing that, so even if an IRQ occurred it would still stay as 0.
waveform[i].enabled = 0;
int cnt = timer1CB?1:0;
for (size_t i = 0; i < countof(waveform); i++) {
Expand All @@ -211,7 +219,6 @@ int stopWaveform(uint8_t pin) {
return true;
}
}
cli();
return false;
}

Expand Down
3 changes: 2 additions & 1 deletion cores/esp8266/core_esp8266_wiring_pwm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@


static uint32_t analogMap = 0;
static int32_t analogScale = 255;
static int32_t analogScale = PWMRANGE;
static uint16_t analogFreq = 1000;

extern void __analogWriteRange(uint32_t range) {
Expand Down Expand Up @@ -59,6 +59,7 @@ extern void __analogWrite(uint8_t pin, int val) {
analogMap &= ~(1 << pin);
uint32_t high = (analogPeriod * val) / analogScale;
uint32_t low = analogPeriod - high;
pinMode(pin, OUTPUT);
if (low == 0) {
stopWaveform(pin);
digitalWrite(pin, HIGH);
Expand Down