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

Waveform stopped by runtime limit in iSR doesn't deinit the timer #7236

Merged
merged 2 commits into from
Apr 23, 2020
Merged

Waveform stopped by runtime limit in iSR doesn't deinit the timer #7236

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cf9c5b1
Waveform stopped by runtime limit in iSR doesn't deinit the timer, bu…
dok-net Apr 19, 2020
b301807
Merge branch 'master' into fix_7230
earlephilhower Apr 23, 2020
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
20 changes: 9 additions & 11 deletions cores/esp8266/core_esp8266_waveform.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,17 +178,15 @@ int ICACHE_RAM_ATTR stopWaveform(uint8_t pin) {
}
// If user sends in a pin >16 but <32, this will always point to a 0 bit
// If they send >=32, then the shift will result in 0 and it will also return false
uint32_t mask = 1<<pin;
if (!(waveformEnabled & mask)) {
return false; // It's not running, nothing to do here
}
waveformToDisable |= mask;
// Ensure timely service....
if (T1L > microsecondsToClockCycles(10)) {
timer1_write(microsecondsToClockCycles(10));
}
while (waveformToDisable) {
/* no-op */ // Can't delay() since stopWaveform may be called from an IRQ
if (waveformEnabled & (1UL << pin)) {
waveformToDisable = 1UL << pin;
Copy link
Contributor Author

@dok-net dok-net Apr 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Everyone can read 1UL << pin, absolutely no need to introduce an object "mask" for this idiom. Use constness if you really feel the need to.

Copy link
Collaborator

@earlephilhower earlephilhower Apr 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Removing a named variable like this, used immediately after definition in the code, is not going to affect GCC's generated code unless it's got a really bad hangover. It's for readability by the next maintainer, only.

// Must not interfere if Timer is due shortly
if (T1L > microsecondsToClockCycles(10)) {
timer1_write(microsecondsToClockCycles(10));
}
while (waveformToDisable) {
/* no-op */ // Can't delay() since stopWaveform may be called from an IRQ
}
}
if (!waveformEnabled && !timer1CB) {
deinitTimer();
Expand Down