Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

"Phase Locked" Waveform: fix significant jitter, that stresses servos and is clearly audible in Tone output#7022

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

Merged
d-a-v merged 153 commits intoesp8266:masterfromdok-net:waveform
Nov 19, 2020

Conversation

@dok-net
Copy link
Contributor

@dok-netdok-net commentedJan 19, 2020
edited
Loading

  • Improves jitter that occurs when multiple GPIOs are served
  • Improves on the poor timing which results in servos with narrow deadband constantly re-adjusting
  • Also the Tone library example showed serious interference on various frequencies across 6 or 8 octaves
  • Fixes 0 duration duty of off-cycle handling
  • Allows finishing on HIGH, instead of always forcing LOW on waveform stop. This is required for instance to use waveform for software serial output signal generation, something I am tinkering with for for well over a year now (reminder: I am the current maintainer and author of EspSoftwareSerial).
  • Allows setting waveform at CPU cycle precision instead of just microseconds.
  • Proper semantics of stopping (startWaveformCycles(pin, timeHighCycles, timeLowCycles, runTimeCycles)), runTimeCycles gets applied to the next started FULL period, instead of randomly relative to time of call.

@dok-netdok-netforce-pushed thewaveform branch 2 times, most recently from9938de8 to1ccd820CompareFebruary 5, 2020 06:29
@dok-netdok-net mentioned this pull requestFeb 5, 2020
6 tasks
@dok-net
Copy link
ContributorAuthor

dok-net commentedFeb 5, 2020
edited
Loading

Dear@earlephilhower , I would like to ask to you to pay attention to this major change to the waveform lib, which resolves serious problems I was/am having using it as it is in master/release 2.6.x. This PR shall (I'll fix if not ;-) ) supersede#7057.

@dok-netdok-net mentioned this pull requestFeb 5, 2020
@dok-netdok-net changed the titleFix significant jitter, that stresses servos and is clearly audible in Tone outputWaveform: fix significant jitter, that stresses servos and is clearly audible in Tone outputFeb 7, 2020
@s-hadinger
Copy link
Contributor

I tested this PR with Tasmota and confirm it solved the flikering LEDs issues.

@s-hadinger
Copy link
Contributor

@dok-net Actually the phase change when changing duty cycle is visible. When fading from bright to low-brightness, the fading seems to bounce on the low value and increase a bit when stabilizing.

Copy link
Collaborator

@earlephilhowerearlephilhower left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! I know you've got some neat ideas, but the style changes do add friction when reviewing. I've gone over the code w/Mk1 eyeballs, but need to build and run it to see how it performs. I'm uncertain about the timeout stuff and how it adds to IRQ runtime and for what purpose. Some comments in the code as the the state transitions/etc. would be helpful.

Copy link
Collaborator

Choose a reason for hiding this comment

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

There's a whole lot of new logic for expiry detection here. What's the intended use case? The major use seems like it would be the Tone() call, which seems to not warrant so much precision.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

There were two requirements that had set myself: more precise timing (actually Servo rather than Tone, but it comes down to the same issues).
I also wanted to be able to stop a waveform precisely, and keep the line level at the moment instead of always going LOW.
All this works now - if someone could put an oscilloscope to the new code and check max frequency etc, that would be great.

Copy link
Contributor

@Tech-TXTech-TXFeb 15, 2020
edited
Loading

Choose a reason for hiding this comment

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

All this works now - if someone could put an oscilloscope to the new code and check max frequency etc, that would be great.

With your current code as of 4 days ago (last change), here's what I'm seeing:
The phase between 2 pins PWM-controlled jumps by 60us when the duty cycle hits the rails (0 & 1023). Maybe my little test program below is in error. Scope is triggered on the rising edge of the top trace.

PWM_test

Here's a short 1.5meg/8sec video of it hitting the limits and suddenly jumping by 60us:http://www.mediafire.com/file/mnc28ogkzc366or/VID_PWM_%25237022.mp4/file
(sorry for the poor quality, my phone won't focus at that distance)

constintch1Pin=14;constintch2Pin=2;voidsetup() {}voidloop() {// increase duty cycle, decrease for second channelfor(intdutyCycle=0;dutyCycle<1023;dutyCycle++){analogWrite(ch1Pin,1023-dutyCycle);analogWrite(ch2Pin,dutyCycle);delay(3);  }// decrease duty cycle, increase for second channelfor(intdutyCycle=1023;dutyCycle>0;dutyCycle--){analogWrite(ch1Pin,1023-dutyCycle);analogWrite(ch2Pin,dutyCycle);delay(3);  }}

The duty cycle didn't have any visible jitter, only the phase between multiple channels was changing.

Further test: I split that 3ms delay, putting 1ms between PWM updates and 2ms after, and it jumps phase by about 50%,
http://www.mediafire.com/file/3czuei8qaciwjj0/VID_PWM_%25237022_2.mp4/file

Copy link
ContributorAuthor

@dok-netdok-netFeb 15, 2020
edited
Loading

Choose a reason for hiding this comment

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

@Tech-TX Absolutely fantastic, awesome, thank you so much!
What happens proves my point - and now I should adapt analogWrite to make use of the zero-duration cycle, hold level, enhancement. Right now it calls stopWaveform, so phase can only be lost at 0 and 1023. EDIT: done.
Wit the latest change now, waveform has to be stopped by explicitdigitalWrite instead ofanalogWrite(0) oranalogWrite(PWMRANGE), to indicate that phase no longer matters. I've also checked all other uses of waveform included in core (plus core libraries), no further changes are necessary.

As for the performance impact of my PR, are there any known-good min and max frequency values for analogWrite(), that you could also graph for accuracy, whether better, same, or worse than before?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm seeing strange results atall frequencies, even with the current git. For a simple test (if you don't have a 'scope), hook up an LED to a port pin and drive it with 20KHz PWM, slowly increasing from 1 to 1023 duty cycle with at least a few ms between duty cycle changes. It's not a smooth progression. Most LEDs will be able to handle 20KHz PWM (a few will even hit 200KHz). The aberrant behavior at higher frequency is pretty obvious on an LED.

It's less noticeable at 1KHz, but there aren't 1023 steps there, more like 170-180 discrete steps from full-off to full-on. At 20KHz it's more like 8 to 10 steps, with lots of jitter above 80% duty cycle, and the frequency jumps around above 90% duty cycle. I think Earle is going to look at it to make sure I haven't made a dumb newbie mistake. ;-)

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand the PWM resolution is 1 microsecond (80 cycles). So to get full resolution you need a complete cycle of 1024 microseconds or 976Hz. That's why in Tasmota we chose 880Hz which is close enough.

I believe linearity tests should be done at a similar frequency.

20KHz base pwm frequency means roughly being able to drive at 20MHz which is not realistic with an esp at 80MHz - unless you spend all your CPU time in the PWM loop.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

@Tech-TX when you say current git, is that the master branch or the latest push to this PR? Besides that, I am left a bit clueless how I am not seeing what you described in yesterday's oscilloscope videos. Anyway, is the phase loss part fixed now?

Copy link
Contributor

Choose a reason for hiding this comment

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

Current git, not your PR. I'll start doing a more exhaustive look at yours tomorrow. I had a weird error here, and thought the current git was all screwy. Git said I was even with master, but it wouldn't work right, so I mistrusted anything I'd seen on yours. I eventually tried current git on my main desktop and it turns out the laptop had one or more corrupt files that git didn't notice. Wipe, reinstall, re-pull, and it's back again. I hate days like that...

Copy link
Contributor

@Tech-TXTech-TXFeb 29, 2020
edited
Loading

Choose a reason for hiding this comment

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

Sorry, I was having problems and then Real Life rudely intruded. Looking at your latest code (I just pulled it this morning, Feb 29) I see significant jitter if edges on other pins are walking past the servo edges.

For comparison, I see around 6.5us total jitter with the current master. I've checked that my master code is correct, but grabbing a copy of your PR files may be iffy as I haven't tried that before.

Update: I had a bad compile initially, but I'm still seeing ~25us jitter when other pin edges walk past each other. It's down to a rare 3.4us jitter (otherwise stable) if it's only the servo running. 99% of the time or better it's within a 900ns range, which is reasonable. I can't hear the servo when it's like that.

Here's the revised test code I'm using:
https://gist.github.com/Tech-TX/572aab942d44638566e07036fe77a4c6
If you only want to run servos, hit 'c ' for single channel PWM mode, then 'h ' to freeze the sweep, then '0d ' to disable CH1 PWM. 90s turns on the servo to mid-range. With no asynchronous edges, here's what I see:
waveform PR servo jitter, no async

Copy link
Contributor

Choose a reason for hiding this comment

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

Next issue looks like a math error on setting the next edge. The current master works in multiples of 1us step size for the PWM duty cycle. That means the fastest PWM that has all 1022 steps will be 977Hz; at 978Hz it drops the 1/1022 step and the signal goes away. At slower PWM frequency the step size changes from 1us to 2us, and in between that it misses up to half of the PWM steps before it shifts to the next longer / shorter 1us multiple.

Your PR on the other hand is running 7.76us step sizes at 977Hz, resulting in about 1/8th the total number of steps. You can actually see it if you hook up an LED to the PWM pin, set the pause between duty cycle changes to 20ms, and then watch closely as the duty cycle approaches minimum or maximum, depending on whether you ground the other leg or put it at 3.3V. As the LED is going very dim to nearly off, you can see the last 5 or 6 levels before it goes to minimum brightness. At higher levels you can't perceive the change. With the current master the step size is smaller, and there's no visual step-wise drop in brightness. Here's the screen caps from the scope showing the difference (all settings were identical):
master 977Hz PWM step sizes at 977Hz
waveform PR PWM step sizes at 977Hz
waveform PR PWM zoomed on step sizes
That last one is zoomed in to get a more accurate number on the PWM step size.

The final capture is from a logic analyzer. The top 2 traces are PWM channels with the decoded PWM duty cycle below each one. The bottom trace toggles each time the duty cycle is changed. You can see that the actual duty cycle is only changing about every 8 increments, which is that 7.6us step size.
waveform PR 977Hz 1ms pause

@dok-netdok-netforce-pushed thewaveform branch 2 times, most recently from65cc3f9 toceca205CompareFebruary 11, 2020 18:14
@dok-netdok-net mentioned this pull requestFeb 18, 2020
@dok-netdok-netforce-pushed thewaveform branch 2 times, most recently from4a911c4 tofc15ce8CompareFebruary 27, 2020 09:09
@dok-net
Copy link
ContributorAuthor

dok-net commentedMar 1, 2020
edited
Loading

@Tech-TX I find#7121 (that's in master HEAD now) completely breaks my test case (I'm running a web server to set PWM for fans and a servo) - the wdt triggers and while it's not the servo jitters much worse than before this commit.

@dok-net
Copy link
ContributorAuthor

dok-net commentedMar 1, 2020
edited
Loading

@Tech-TX Otherwise, I've found a bug and commited a fix to this PR. I am sorry, I am bit overwhelmed by your signal traces, but what I grasp pointed to a timing error and that bug would cause the transitions from duty to off cycle to happen at rather random times. I can only ask you to re-test, thanks for the great support.

@Tech-TX
Copy link
Contributor

Tech-TX commentedMar 1, 2020
edited
Loading

@Tech-TX Otherwise, I've found a bug and commited a fix to this PR. I am sorry, I am bit overwhelmed by your signal traces, but what I grasp pointed to a timing error and that bug would cause the transitions from duty to off cycle to happen at rather random times. I can only ask you to re-test, thanks for the great support.

No worries! If this was easy, it'd be called 'playtime'. 😄 I'm having some more strange issues here, so I'm going to do another complete wipe and then pull your latest changes. I suddenly don't trustanything on the PC as git isn't tracking when I make changes to files any longer, or at least it's not doing what itused to.

The scope is set for 'infinite persistence', so that it shows a history over about a minute's time. I'm triggered on the rising edge of the CH1 PWM, then I've moved out into the middle of the cycle so that the edges or steps in PWM change should be roughly even. The granularity of the current master is 1us, so at 977KHz PWM = 1 the pin is high for 1us, then low for 1023us. The next step PWM = 2 the pin is high for 2us. On your PR as of yesterday, that 1us step size was 7.76us, so you have/had roughly 1/8th as many steps between 0 and 1023 as the master has. Since the scope is set to STORE, it captures all of the edges as the PWM increases or decreases, showing the PWM step size.

@dok-net Dirk,here's a short video showing what it looks like. First it's zoomed out on infinite persistence to show the whole waveform (I'm triggering on the bottom channel) and then zoomed in to show the edges near the end-points of (PWM = 1 / 1022). I didn't let it go to zero or 1023 so that the phase would stay constant. I have it set for 20ms at each PWM step, slowly going up and then down. Every place you see a line on the scope, the signal spent enough time at that level for the scope to record it. In this case, 20ms on each discrete PWM edge as it moves through time.

Here's what I've seen so far, working back and forth between the current master and this 7022 PR:

  1. Neither master nor PR actually change the PWM frequency until the duty cycle is changed (minor)

  2. Above 977Hz PWM duty cycle 1 = low / disabled (no output) due to 1us granularity of time (master)

  3. Duty cycle steps generally only change in integer multiples of 1us increments (1us, 2us, 3us, etc) so there are missing steps at any frequency not a multiple of 1us * 1023, max 977Hz. Above that it gets progressively worse as the 1us step size is the minimum increment. At 9.8KHz there are only ~ 100 discrete steps, at 40KHz there are only ~26 steps. All 1022 PWM steps are available at 977Hz, 488Hz, 325Hz, 244Hz, 195Hz, 162Hz, 139Hz, 122Hz, 108Hz, 97Hz, 88Hz, 81Hz, 75Hz, etc. Calculation = truncate(1/(1E-6 * 1023)) for the PWM frequencies with all (or most) discrete PWM steps. (master)

  4. Current master: minor error in timing 9 > duty cycle > 1013 (wider steps & missing steps)

  5. Up to 7us jitter on any output if other edges are walking past it (master), PR is/was worse. It's unavoidable when the time to the next edge is less than ~ 5 to 6us due to the calculations required.

  6. PWM = 0 the pin is low, PWM = 1023 the pin is high, so the range is 1-1022 for actual PWM; the other two states (0 and 1023) the PWM isn't running and the pins are fixed level.

  7. Many analog servos today work better at 200Hz frequency (faster response), default 50Hz is fixed in servo.h (should be user-configurable without modifying a core header file) (future enhancement)

  8. Tone output only seems to go up to 5KHz maximum (master)

Please understand that I haven't used PWM on the ESP8266 before, so some of those observations may be "well, of course it does that" to the rest of you. ;-)

@Tech-TX
Copy link
Contributor

Quick update (still testing): your latest changes seem to have fixed the timing until the next edge, so it's running integer multiples of 1us for the PWM step size (depending on PWM frequency) just like the master branch. I haven't seen any really bad things thus far. It looks like you may be missing more edges near 1 and 1022, I'll have to dig into it to see for sure.

@Tech-TX
Copy link
Contributor

Tech-TX commentedMar 2, 2020
edited
Loading

Bad news: as far as your original intent (reduce servo jitter) it's not doing what you'd hoped. Your code is roughly the same overall range of jitter as the current git, but it hits a wider range more frequently. Both run around 12us maximum jitter, but the git does most of it's jitter within about a 2us window, about 1/2 of a degree of servo travel. Your code is mostly jittering over a range of 6us, with occasional excursions beyond that. 1 degree is 5.55us, and I can hear my old Hitec micro servo buzz more frequently with your code than with the current git / master.

If you fire up my test program and drop the PWM sweep delay to 1 (1ms between changes, 1p ) then turn on both servos (90s and 90a) you can hear the clicks or buzz as the jitter hits the servo(s). Even if you don't have a speaker or piezo connected, turning on a tone will add more edges. The jitter happens when edges cross each other.

Here's my latest test code, simplified a bit more with devyte's suggestion and with most of the delay() elements replaced by his polledTimer:
https://gist.github.com/Tech-TX/572aab942d44638566e07036fe77a4c6

Here's some screen captures of the jitter for both your PR and the current master. The delta-t at the bottom is showing the total excursion over about a minute's worth of time. After running for several minutes, they both have around 12.75us worst-case jitter, min to max.
PR servo jitter
master servo jitter

I get nearly identical results for the tone jitter. Master is primarily within a 2us band with occasional excursions wider than that (probably up to 12.75us total), and yours is within a 6us band with occasional wider excursions. I can't hear any difference on the piezo, however. I've pushed too many rounds down my Colt 45 1911 to have decent hearing.
PR tone jitter
master tone jitter

Ignore the grass on that second capture: I had a loose ground connection.

If I turn off all of the 'walking edges' and have only tone output running, here's the results:
PR only tone jitter
master only tone jitter

@dok-net
Copy link
ContributorAuthor

@Tech-TX I've made a few improvements, but the biggest thing is that I have brought back the gcc optimize pragmas from master, and now my servo is very, if not perfectly, quiet. I hope you can retest and will find that this is now a real improvement over master (phase, precision).

@dok-net
Copy link
ContributorAuthor

@Tech-TX Just letting you know that there is a problem in master HEAD related to testing#7022, if you merge the PR instead of checking out my branch, please maybe revert#7036, otherwise I am getting WDT resets all the time in my sketch, which includes the OTA update feature. I guess this doesn't hit your sketch, though.

@dok-net
Copy link
ContributorAuthor

dok-net commentedMar 3, 2020
edited
Loading

@devyte I know you've been waiting for this a while, now that I think everything works, here are some timings compared.
The test sketch runs two PC PWM fan controls (startWaveform(...)), oneServo, and one PWM channel (analogWrite(...)). These are all set up insetup().
The average latency for iterating an idleloop()is measured, this should give a good indication of the load the various waveform implementation entail.
Idle (no waveforms) is 6us everywhere, so the waveform generating load given in percent of idle is:

esp8266/master150%esp8266/master + s-hadinger's #7057333%esp8266/master + dok-net/waveform #7022266% or 600% (press reset, either runs at 266% or 600% each time)dok-net/master - waveform (all pending dok-net PRs except dok-net/waveform)150%dok-net/master incl. waveform (all pending dok-net PRs, including #7022)250%dok-net/master - waveform + s-hadinger (all pending dok-net PRs except dok-net/waveform merged with  s-hadinger's #7057)183%

@Tech-TX
Copy link
Contributor

@dok-net I've pulled your branch for testing, Dirk. I'll re-pull tonight when I get home. GMT-6 so you'll be asleep. No OTA in that stress test I'm running. :-)

@dok-net
Copy link
ContributorAuthor

dok-net commentedMar 3, 2020
edited
Loading

Way to go! Please keep an eye on the mysterious slowness every other reset or so, that I have seen earlier. I suspect it's a boot order problem, that may not hit your sketch without WiFi, though :-)

…emove this code.Maximum period duration limit is implicit to timer, consider it documented constraint, don't runtimecheck in ISR.
@d-a-v
Copy link
Collaborator

d-a-v commentedNov 19, 2020
edited
Loading

This work is going to be merged.

We will have two waveform implementations, together with#7231.
Both have their benefits. This one will be known as "Locked Phase" flavour, and the other "Locked PWM" flavour.

Another PR (#7712) will immediately follow this one. It will install a new Arduino IDE menu (and a PIO #define).
After that,#7231 will be adapted to fit with the new menu and will be setup to be the default flavoured version.

@d-a-vd-a-v merged commit0e735e3 intoesp8266:masterNov 19, 2020
@d-a-vd-a-v changed the titleWaveform: fix significant jitter, that stresses servos and is clearly audible in Tone output"Phase Locked" Waveform: fix significant jitter, that stresses servos and is clearly audible in Tone outputNov 19, 2020
earlephilhower added a commit that referenced this pull requestNov 20, 2020
* Re-implement PWM generator logicAdd special-purpose PWM logic to preserve alignment of PWM signals forthings like RGB LEDs.Keep a sorted list of GPIO changes in memory.  At time 0 of the PWMcycle, set all pins to high.  As time progresses bring down theadditional pins as their duty cycle runs out.  This way all PWM signalsare time aligned by construction.This also reduces the number of PWM interrupts by up to 50%.  Before,both the rising and falling edge of a PWM pin required an interrupt (andcould shift arround accordingly).  Now, a single IRQ sets all PWM risingedges (so 1 no matter how many PWM pins) and individual interruptsgenerate the falling edges.The code favors duty cycle accuracy over PWM period accuracy (since PWMis simulating an analog voltage it's the %age of time high that's thecritical factor in most apps, not the refresh rate).  Measurements giveit about 35% less total error over full range at 20khz than master.@me-no-dev used something very similar in the original PWM generator.* Adjust running PWM when analogWriteFreq changedUse fixed point math to adjust running PWM channels to the newfrequency.* Also preserve phase of running tone/waveformsCopy over full high/low periods only on the falling edge of a cycle,ensuring phase alignment for Tone and Servo.* Clean up signed/unsigned mismatch, 160MHz operat'n* Turn off PWM on a Tone or digitalWriteEnsure both the general purpose waveform generator and the PWM generatorare disabled on a pin used for Tone/digitalWrite.* Remove hump due to fixed IRQ deltaA hump in the dueling PWMs was very prominent in prior pulls.The hump was caused by having a PWM falling edge just before the cyclerestart, while having the other channel requesting a 1->0 transitionjust outside the busy-loop window of 10us. So it gets an IRQ for channelB 0->1, then waits 2..8us for the next PWM full cycle 0->1, and ends upreturning from interrupt and not scheduling another IRQ for 10us...hencethe horizontal leg of the bump...Reduce the minimum IRQ latency a little bit to minimize this effect.There will still be a (significantly smaller) hump when things cross, butit won't be anywhere near as bad or detectable.* Speed PWM generator by reordering data structBreaking out bitfields required a load and an AND, slowing things downin the PWM loop. Convert the bitfield into two separate natural-sizedarrays to reduce code size and increase accuracy.* Remove if() that could never evaluate TRUE* Add error feedback to waveform generationApply an error term to generated waveform phase times to adjust for anyother ongoing processes/waveforms.  Take the actual edge generationtimes, subtract them from the desired, and add 1/4 of that (to dampenany potential oscillations) to the next similar phase of that waveform.Allows the waveform to seek its proper period and duty cycle withouthardcoding any specific calibrations (which would change depending onthe codepaths, compiler options, etc.) in the source.* Move _stopPWM and _removePWMEntry to IRAMThanks to@dok-net for noticing these need to be in IRAM as they may becalled by digitalWrites in an IRQ.* Avoid long wait times when PWM freq is low* Fix bug where tone/pwm could happen on same pin* Adjust for random 160MHZ operationThe WiFi stack sometimes changes frequency behind our backs, so ESP'scycle counter does not count constant ticks.We can't know how long it's been at a different than expected frequency,so do the next best thing and make sure we adjust any ESP cycles we'rewaiting for by the current CPU speed.This can lead to a blip in the waveform for 1 period when the frequencytoggles from normal, and when it toggles back, but it should remainfor the intervening periods.Should avoid a lot of LED shimmering and servo errors during WiFiconnection (and maybe transmission).* Clean up leftover debugs in ISR* Subtract constant-time overhead for PWM, add 60khzPWM has a constant minimum time between loops with a single pin, so pullthat time out of the desired PWM period and shift the center of the PWMfrequency closer to the desired without any dynamic feedback needed.Enable 60khz PWM, even though it's not terribly useful as it causes anIRQ every ~8us (and each IRQ is 2-3us).  The core can still run w/o WDT,but it's performance is about 5x slower than unloaded.* Fix GPIO16 not toggling properly.* Remove constant offset to PWM periodanalogWrite doesn't know about the change in total PWM cycles, so it ispossible for it to send in a value that's beyond the maximum adjustedPWM cycle count, royally messing up things.  Remove the offset.Also, fix bug with timer callback functions potentially disabling thetimer if PWM was still active.* Remove volatiles, replace with explicit membarrierVolatiles are expensive in flash/IRAM as well as in runtime because theyintroduce `memw` instructions everywhere their values are used.Remove the volatiles and manually mark handshake signals forre-read/flush to reduce code and runtime in the waveform generator/PWM.* Consolidate data into single structureSave IRAM and flash by using a class to hold waveform generator state.Allows for bast+offset addressing to be used in many cases, removing`l32r` and literals from the assembly code.* Factor out common timer shutdown code* Remove unneeded extra copy on PWM start* Factor out common edge work in waveform loop* Factor out waveform phase feedback loop math* Reduce PWM size by using 32b count, indexesByte-wide operations require extra instructions, so make index and counta full 32-bits wide.* GP16O is a 1-bit register, just write to itTesting indicates that GP16O is just a simple 1-bit wide register in theRTC module.  Instead of |= and &- (i.e. RmW), use direct assignment inPWM generator.* Increase PWM linearity in low/high regionsBy adjusting the PWM cycle slightly to account for the fixed timethrough the compute loop, increase the linear response near the min andmax areas.* Remove redundant GetCycleCount (non-IRQ)* Factor out common timer setup operations* Fix clean-waveform transition, lock to tone fasterNew startWaveform waveforms were being copied over on the falling edgeof the cycle, not the rising edge.  Everything else is based on risingedge, so adjust accordingly.Also, feedback a larger % of the error term in standard waveformgeneration.  Balances the speed at which it locks to tones underchanging circumstances with it not going completely bonkers when atransient error occurs due to some other bit.* Reduce IRAM by pushing more work to _setPWMSimply mark pins as inactive, don't adjust the ordered list until thenext _startPWM call (in IROM).* Fix typo in PWM pin 1->0 transitionActually check the pin mask is active before setting the PWM pin low.D'oh.* Combine cleanup and pin remove, save 50 bytes IROMThe cleanup (where marked-off pins are removed from the PWM time map)and remove (where a chosen pin is taken out of the PWM map) doessentially the same processing.  Combine them and save ~50 bytes ofcode and speed things up a tiny bit.* Remove unused analogMap, toneMapSave ~100 bytes of IROM by removing the tone/analog pin tracking fromthe interface functions.  They were completely unused.* Save IRAM/heap by adjusting WVF update structThe waveform update structure included 2 32-bit quantities (so, used8 * 17 = 136 bytes of RAM) for the next cycle of a waveform.Replace that with a single update register, in a posted fashion.  Thelogic now sets the new state of a single waveform and returnsimmediately (so, no need to wait 1ms if you've got an existing waveformof 1khz).  The waveform NMI will pick up the changed value on its nextcycle.Reduces IRAM by 40 bytes, and heap by 144 bytes.* Don't duplicate PWM period calculationLet the waveform generator be the single source of truth for the PWMperiod in clock cycles.Reduces IRAM by 32 bytes and makes things generally saner.* Factor out common PWM update codeReplace repeated PWM update logic with a subroutine, and move thePWMUpdate pointer into the state itself.  Reduces IROM and IRAM,removes code duplication.Also remove single-use macros and ifdef configurable options as theIRAM and IROM impact of them are now not very large.* Fix regression when analogWrite done coldLost an `initTimer()` call in a refactoring, resulting in the corehanging forever while waiting for the NMI which will never happen.Re-add as appropriate.* Save 16b of IRAM by not re-setting edge intr bitPer@dok-net, drop the rewrite of the edge trigger flag in the timerinterrupt register.  It's set on startup and never cleared, so this isredundant.  Drops ~16 bytes of IRAM.* Allow on-the-fly PWM frequency changesWhen PWM is running and analogWriteFreq is called, re-calculate theentire set of PWM pins to the new frequency.  Preserve the rawnumerator/denominator in an unused bit of the waveform structure toavoid wasting memory.* Adjust for fixed overhead on PWM periodPulls the actual PWM period closer to the requested one with a simple,0-overhead static adjustment.* Fix value reversal when analogWrite out of rangeSilly mistake, swapped high and low values when checking analogWrite forover/under values.  Fixed* Don't optimize the satopWaveform callSave a few bytes of IRAM by not using -O2 on the stopWaveform call.  Itis not a speed-critical function.* Avoid side effects in addPWMtoList* Adjust PWM period as fcn of # of PWM pinsResults in much closer PWM frequency range over any number of PWM pins,while taking 0 add'l overhead in IRAM or in the IRQ.* Fix occasional Tone artifactsWhen _setPWMFreq was called the initial PWM mask was not set to 0leading to occasional issues where non-PWM pins would be set to 1on the nextPWM cycle.  Manifested itself as an overtone at the PWMfrequency +/-.* Reduce CPU usage and enhance low range PWM outputBorrow a trick from#7022 to exit the busy loop when the next event istoo far out.  Also reduce the IRQ delta subtraction because it wasinitially not NMI so there was much more variation than now.Keep the PWM state machine active at a higher prio than the standardtone generation when the next edge is very close (i.e. when we're atthe max or min of the range and have 2 or more near edges).  Adds alot of resolution to the response at low and high ranges.Go from relative to absolute cycle counts in the main IRQ loop so thatwe don't mingle delta-cycles when the delta start was significantlydifferent.* Update min IRQ time to remove humps in PWM linearityKeep PWM error <2.0% on entire range, from 0-100%, and remove thehump seen in testC by fixing the min IRQ delay setting.* Remove minor bump at high PWM frequenciesThe IRQ lead time was a tiny bit undersized, causing IRQs to come backtoo late for about .25us worth of PWM range.  Adjust the constantaccordingly
@devytedevyte mentioned this pull requestNov 21, 2020
2 tasks
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@earlephilhowerearlephilhowerAwaiting requested review from earlephilhower

3 more reviewers

@TD-erTD-erTD-er left review comments

@s-hadingers-hadingers-hadinger left review comments

@Tech-TXTech-TXTech-TX left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@dok-net@s-hadinger@Tech-TX@earlephilhower@TD-er@devyte@BigMike71@Jason2866@d-a-v

[8]ページ先頭

©2009-2025 Movatter.jp