- Notifications
You must be signed in to change notification settings - Fork13.3k
Waveform stopped by runtime limit in iSR doesn't deinit the timer#7236
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
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.
earlephilhowerApr 21, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
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.
…t stopWaveform refuses to do anything if the waveform was stopped by runtime, either.Fixesesp8266#7230.
earlephilhower left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
stopWaveform is in IRAM because it's possible to turn off a waveform via a digitalWrite/etc. from inside an IRQ.
If you are concerned about generated code size, you should move this out of theO2 code block.
Please note thatO2 does not always generate larger code and you'd want to try both ways and then pick the smallest.
OTW, I'm fine w/this PR and moving the delay into the if-clause.
earlephilhowerApr 21, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
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.
Uh oh!
There was an error while loading.Please reload this page.
But
stopWaveform()refuses to do anything about it if the waveform was previously stopped by expired runtime from inside ISR, either. Fix for#7230 from original issue reporter.For some reason
stopWaveform()is in IRAM cache and optimized for performance over size.Also,
stopWaveform()would be explicitly called (if indirectly) from user code, which for waveform with a runtime argument can be reasonably expected to be well adjusted in time to avoid race conditions between ISR-based cutoff vs.stopWaveform().Both facts talk back to#7232 (comment), which curiously mentions that 10µs isn't a big thing - for a function that bloats under gcc -O2 and is in IRAM cache :-)