Support multiple tone(), analogWrite(), and Servo #4640
Conversation
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. 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. Adds in control of DRV8825 or A4988 interfaces stepper motors to the timer infrastructure. The interrupt handles acceleration and velocity of stepper pulses including jerk (dA/dt) calculations, but it is up to the user application to do path planning and generate S-curve accelerations. All stepper DIR pins are connected together, and then the STEP from each stepper interface board is routed to an individual pin. For motions which involve multiple steppers, there is an option to wait before beginning the next stepper motion until all active steppers have completed their moves (i.e. moving to an X, Y where there may be 1-2us delay between the X arrival and Y arrival). A generic "startWaveform(pin, high-US, low-US, runtime-US)" and "stopWaveform(pin)" allow for further types of interfaces. Using a simple USB DSO shows that with only a single channel running, a "tone(1000)" generates a waveform of 999.8Hz (500.1us per high/low). Running additional generators or steppers will potentially increase the jitter. The implementation also is limited in the time of the shortest pulse it can generate (mostly an issue for analogWrite at values <1% or >99% of the scale). Presently measurements of ~2-3us are seen on the shortest of pulses. There is no optimization to allow generating two edges on a single interrupt, which leads to this minimum pulse width. The current implementation was written focusing on readability and flexibility, not squeezing every cycle out of interrupt loops. There are probably many places where the code could be optimized (after it's fully validated!) to reduce CPU load and timing errors.
|
@earlepillhower you wrote:
It seams, you didn't quite catch my idea. It wasn't not only to have more timers, but especially high accuracy timers. Two timers are meant for Arduino, which has tone and pwm. Two further timers are meant to be used by implementations like yours. One timer may be declared to be a high accuracy timer with no jitter. The minimum time between the end if the timer isr callback to the begin of the next isr callback again is 20 CPU ticks (for 80 MHz). This would mean, that PWM pulses also for analogWrite(1) with a correct duration of 78 ticks should be possible. This is possible, because short pulses are performed by a loop within the timer1 isr. Such short timer steps in a loop may be used for PWM pulses, for unregularely fast following or overlapping timers, but not for square waves with higher frequencies as maybe 50 kHz, because not leaving the loop would cause a watchdog reset. In case, that there shall not be something else like normal code context, wifi or communication also short square waves maybe could be possible by watch dog retriggering. And the timimng of the high prio timer is exact, maybe sometimes one tick error. Of course other timers jitter, when overlapping happens. There is no automatic TIM_LOOP now, because this would cost time, which I wanted to spare. I deleted the features DIV1, DIV16, DIV256 and TIM_LOOP, which I first had implemented. Now there is only DIV1 but there is also a replacement for TIM_LOOP. For using my timers there isn't much to change. The isr callback has to be of type uint32_t instead of void. And ticks are triggered by "return ticks;" for additive after jitter, by "return ticks | 0x8FFFFFFF;" for not additive after jitter - replacement for TIM_LOOP, or "return 0xFFFFFFFF;" for stopping the timer. Of course also a function for arming or stopping a timer from outside the isr exists, and for initializing the timers and for attaching a callback function. My idea, only four timers, but with very high accuracy and exactness and allowing very small steps and one timer jitterless, so that more timers or implementations like yours may be based on one of these timers without destroying tone or pwm and letting one timer remaining for use by the user. |
|
What I want to have, is a correct working analogWrite already for the value one. If you think, you could replace timer1, then I tell you my requirements for PWM. GPIO costs 20 ticks, 10 ticks for D0 and ten ticks for the other digital pins. I calculate for code about 38 ticks. This means, that about 20 ticks remain for requesting the next ISR. For saving time I thought to return this 20 ticks. Subtracting return from ISR and call the ISR, until it's beginning, there will be left about 13 ticks from receiving the required ticks until calling the ISR. Can you do this? |
|
And because I wouldn't be pleased very much by a jitter of 1 tick, I would rate a jitter of more than 200 ticks as completely inadmissable. |
|
I think we're talking about different things here. This PR allows very fllexible usage, including use of all of pwm, servo, tone, and steppers at the same time. However, at first glance it looks like a top-down design, which means it doesn't solve the low level issues. How about this for a plan: we review this PR first, then once merged we add in @AlfonsMittelmeyer 's work to optimize and reduce jitter? |
|
@devyte you wrote: @AlfonsMittelmeyer 's work is a bottom-up design at timer level, that optimizes first with assembler to solve the low level issues, but at top level doesn't provide the features or flexibility provided here. I had also first the idea for offering multiple tones and waveforms. And it wasn't only an idea, because I soon had presented dimming (PWM) combined with multichord tone for Oh Susanna. But then I saw, that there are problems with pwm about accuracy, exactness and jitter. My second thought then was, that we first would need a stabile fundament, which offers these requirements and upon we could build then also, what earlepillhower did. This overbuilding I had thought to do after. And of course in the same manner, as we can make of one hardware timer more shared timers, we may also make of one shared timer a lot of further shared timers. I had thought also, that this wouldn't need first a call to the callback, which offers further callbacks, but that we may also make a shortcut, which leaps the second level of abstraction and so behaves as further timers of the second level would have been added to the first level. What I do, is a stabile fundament for a combination of timers with critical requirements and of further multiple timers, with less critical requirements, which may be added for a lot of waveforms. My current implementation is not offering this lot of multiple timers yet, but only the fundament for this by offering very accurate and exact timers, which further may be divided into further timers. |
earlephilhower
force-pushed
the
new_timer1_irq
branch
from
8cc7859
to
e571262
Compare
The closest time between interrupts is about 3us, so rework the waveform generator to support phases down to 1us, while limiting the time in the IRQ handler to avoid WDT or WiFi errors. Waveforms now are based off of absolute cycle, not delta-cycles, and the GPIOs are converted from a bit to a mask (to save clocks in the IRQ) costing about 40 more bytes in global storage and reducing the minimum period down from 3.5us to 1.20us at 160MHz. Steppers still based off of delta-times.
earlephilhower
force-pushed
the
new_timer1_irq
branch
from
e571262
to
2b1c0f0
Compare
|
Cool, @AlfonsMittelmeyer . I think we're working on orthogonal things, like @devyte was saying. And I'm not sure that unlimited tone, pulse, servo, and steppers are needed by the general populace. Optimizing the original code, I've been able to get the general, straight C code to generate down to 1.2us measured pulses ( Also, you may want to look at the SDK. It seems like it already has a NMI based PWM generator at 14 bits of precision implemented. See chapter 12 of the ESP8266-Technical-Reference.pdf doc. If you need a very high precision PWM for something, that may be the way of least resistance... |
|
I just did some measurements on the existing analogWrite and my version using a DSO and running at 160MHz. All measurements were at 1KHz, 1000 scale (so that 1==1us in analogWrite and avoid any issues of rounding error). The one already in there is pretty darn good, with the exception at very small ranges and having a constant offset of ~1.5us from requested length of each phase (i.e. 3.0us add'l per full cycle, split between the high and low periods). The redesigned waveform generator is a little better and ~2x more accurate at 1-3us thanks to the busy-looping, but otherwise shrinks the delta in half to average of 1.5us total period offset. But, frankly, either version is monotonically increasing and seems quite stable over the range so should be usable in any sort of feedback loop. I also noticed that analogWrite does not actually load the TIMER1 so it takes 0.1s (TIMER1 full 7fffff count) for the 1st analogWrite to take effect in GIT head. After that, timer's already running so it's OK. This patch reloads when starting the timer for the first time so it doesn't have the same quirk, but again in GIT it's only on the 1st write from no PWM running. |
Remove unneeded curTimeHigh/curTimeLow from the waveform structure, saving 88 bytes (11 pins x 8bytes) of heap. Also reduces inner loop in the IRQ by an couple stores. Also add measured offset to the waveform generation request, enabling a 1/1000 reqest to produce a 1.083us high pulse, and a 999/1000 to give a 1.083 low period, and the cycle time to be exactly 1.00ms over the range. IRQ doesn't need to be disabled on waveform changes as the code order between the irq and start/stop is safe. Allow GPIO16 as a stepper out pin
|
I ran some tests with this PR and an analogWrite@1000 range, 1khz while doing HTTP page fetching in a tight loop (~1.5 requests/second) and dumping it to Serial, and recorded the waveforms with the logic analyzer and post-processed to get the mean and stddev over a 10 second period @ 24MHz sampling. With just the use of the TIMER1 standard IRQ and the ESP cycle counter, I'm getting what seems to be only ~8us worst case add'l delays in the waveform, but the average and standard deviation are very nice with the code at 1, 10, and 100us pulses: So, I'd say that even w/o NMI and using the ESP cycle counter as the real waveform base (and TIMER1 only as a "hint") you can get some pretty nice numbers. |
Remove the stepper code due to its size. If it's included here it will take a large amount of IRAM in *all* sketches, even those which do not use it. Add in a traditional callback option, which will allow Stepper to be put in as a library, only being linked when necessary.
earlephilhower
changed the title
|
Our IRAM is tighter than even the AVR's PROGMEM. Ouch! Removed the stepper bit, will use a callback interface instead so that it won't be linked in unless you actually use it, and place into a separate library. |
There was a problem hiding this comment.
I only have tried PWM and it's working better. A pulsing LED like in #4321 stays smooth even with busy loop and also seems to fix its wdt.
The clarity of the code, its genericity and the sub-microsecond standard deviation measurements are convincing enough.
Thank you for this goldsmith's work.
|
Thanks, but I haven't had time recently to get back to this or the stepper work with path planning, unfortunately. Let me clean up the comments (which still reference the stepper bits) and remove the WIP and see if other folks can give it a go, too. |
earlephilhower
force-pushed
the
new_timer1_irq
branch
from
c1d2d72
to
cd8de43
Compare
earlephilhower
changed the title
|
I've been pulled away from my eggbot/3d printer work, but wanted to move this from WIP to the "give it a go" stage. The comments have been cleaned up to remove the references to stepper motors. Moved them out to free IRAM as most users will not need this (and the linker will put it in already-tight IRAM if it's included). |
|
Hi earle, did you implement multichannel tone? And does your solution work well with a lot of tone channels? I would be interested, whether your solution performs well. I have written a test program which uses 7 pins for tone and one pin for PWM. For PWM I have chosen 2 kHz and a range of 40000. What is the resolution of your implementation? Are 1/80 µs possible for 80 MHz CPU clock? Here is my test program: |
|
Howdy @AlfonsMittelmeyer,
Yes, it runs multichannel anything. The architecture is each pin can have a waveform with definable high and low periods, forever or for a limited time. So it'll do multiple tone + PWM + servo up to one per GPIO pin. Because it is tickless (interrupts on need, not every X us) the jitter will vary depending on how the edges line up (and interrupts are not NMI, so if WiFi is busy it'll have to wait to get control until the WiFi handler relinquishes the CPU). That's what the outliers were in my jitter analysis came from on Apr 22.
The internal representation uses CPU cycles, not timer cycles, so theoretically you can specify down to 1/80Mhz. However, I can't guarantee skew or jitter at that level since it's a single core with other things to do, so the "set waveform (pin, hiper, lowper)" only allows microseconds. It does try and compensate for this core jitter by setting the timer to fire a little before it needs to work with dynamic adjustment. Whether or not it performs "well" really depends on the requirements. For me, it lets me use all the pwm, servo, tone (and steppers, but I'm trying to do better than constant jerk) that I need and gives me the numbers I measured on Apr 22 while doing WiFi while using little CPU. I don't have your physical setup, so would only be able to run your sample code and look at my USB "DSO", but since it's playing different notes at different times, I wouldn't be able to really give a good measurement. It does compile just fine and should run, but I can't test it. You can apply this pull request to your own GIT copy from master if you want to give it a go: |
|
Howdy @earlephilhower
The physical setup is very simple. I have a NodeMCU and connected the tone pins by resistors (470 Ohm) and then with a satellite box of my hifi-system. But I noticed, that this code example cannot be used for stability tests. Using a lot of tickers may cause a reset after minutes or hours. I have to do a sequential conversion of the tone data, so that the tone data may be processed without tickers. Maybe I will give your implementation a go tomorrow or the day after tomorrow. Tomorrow or the day after tomorrow I would like to present my own solution of the tone and pwm incompatibiliy problem. Individual pwm ranges and frequencies and duration per pin and waveform integration I haven't implemented yet, but will do it later. Now for tone I use gaps in the Interrupt between PWM, because a little jitter for tone, if it's intended for speakers, doesn't matter. The advantage is less load for the interrupt and a very nice PWM. About FRC1 or NMI interupt: it may be configured. |
Allow tone to take a double/float as wekk as an integer frequency. Set the pin to an output on a tone() call to match original code.
|
I've just pushed in a simple method that can take a float/double as well as an int. On the AVR Arduino you wouldn't want to run anything with floating point, which is horribly slow and uses lots of the small program flash. ESP8266 is also very slow, but it's not so bad and the code space isn't nearly so tight. Also fixed that pin direction, thanks. I'm looking at the scope and seeing definite (uneven) pauses between notes, so you probably don't want to use Ticker to send notes or you'll get stuttering between notes like someone just learning to play an instrument. Probably related to the stuff you said about its jitter. [begin plug] If you really want to send polyphonic music out, check my audio library https://github.com/earlephilhower/ESP8266Audio which has everything from MP3 to MIDI and can even use the I2S to generate analog outputs to a speaker w/o a DAC. [end plug] |
Of course I don't use float within the ISR. The ISR only uses ticks. So float as parameter for tone doesn't matter for the speed. I use also float frequency for PWM. If pin D0 isn't used, jitter free PWM steps >= 33 ticks (for 80 MHz) are possible (2.4 kHz and range 1024). The problem which I have with these tickers, is, that for each tone I enable a timer and disable it, when the duration ends. This enabling and disabling and arming costs too much time, because each time a search for a free timer has to be done. And for 7 tone channels it's too much time. So I should use only one timer for tone (and for advanced PWM per pin), which I can divide in further timers for pins. A search then needn't to be done. This will then be similar, as you do it. But the normal PWM I also want to have. Because it starts with all pins at one time, the load during the ISR is only about half as much as for waves like tone and it may be adjusted to be completely exact also, when combining the wave forms. Because there is a setting and clearing of pins in each time slot, when combining tone and PWM waves, clearing will be about 10 ticks later than setting, which would result in not exact PWM time spans. So I want to shift the first time slot (setting pins for PWM) about 10 ticks into the future for complete exact PWM, also when the wave forms are combined. |
Now it works correct. Instead of tickers I use delays. And there were bugs in the music data. The note D4 was wrong, because this is a pin. It had to be d4. And I had declared pin D4 as tone pin. It should have been D5 instead. D4 we need for flashing, so it's better not to use it. Now this works fine: |
|
Just confirmed this fixes #4349 as well. |
|
So is this fix now in Milestone 2.4.2? |
|
Guys, I've observed rare resets of PWM to zero or to maximum. I use AnalogWrite in my weather station to set adaptive backlight of the screen. AnalogWrite was used for several times in a second (probably, it is a reason of spontaneous resets). I've optimized the algorithm and now AnalogWrite is used only when the lighting is changed. It's been a while since the changes were made, but so far everything is working fine. |
|
@X-Stas-EEE, can you open a new bug for this? I'd like to get more details and, if possible, some way to reproduce it without having a whole weather station attached. Also, "resets to 0 or 1" means the analogWriteten pin is stuck high or low (how long), not a system reset, yes? About how frequent was it (1/30 mins, 1/hour, 1/day, etc.), how often you were doing analogWrite (3/sec, 300/sec, etc.?), if they were the same or different values you were writing, if there was anything going on at the time (i.e. serving a web page, reassociating with a AP, etc.). And did it fix itself after (#) of analogWrite loops, or was it dead forever at that point? If it's reproducible relatively easily with your original code, I also can see about making a PR with the PWM a non-maskable interrupt to see if that helps things. On my own I've not visually seen anything like it on constantly analogWriting the same value to the built in LED. Same good result with a USB logic analyzer with a trigger window (i.e. if pulse high > 10us, capture) for 15-30 minutes. That said, the clone, $10 analyzer I've got can only run for 15-30 minutes before suffering a USB error (sometimes only seconds), so I can't get longer runs to do an automated check like that. |
Yes, backlight is off or very bright in such cases. If I cover the photoresistor with my finger (which means analogWrite(0)) and then take it away, the adaptive backlight is back to life. Just to be on the safe side I've checked if the AnalogRead (from the photoresistor) works properly in the described cases. Proper values have been written to serial console even when the backlight was stuck. Thus, the problem is clearly in AnalogWrite. From time to time my station plays a primitive music using tone(). Perhaps this can influence. |
|
I succeed in replication with the following sketch: #define LED_PIN D1 // GPIO5
#define BUZ_PIN D2 // GPIO4
int bright;
bool increase;
unsigned long cur_millis = millis();
unsigned long prev_millis = cur_millis;
void setup() {
pinMode(LED_PIN, OUTPUT);
pinMode(BUZ_PIN, OUTPUT);
bright = 0;
increase = true;
}
void loop() {
cur_millis = millis();
if (abs(cur_millis - prev_millis) > 100) {
analogWrite(LED_PIN, bright);
if (increase) {
bright++;
} else {
bright--;
}
if (bright >= 105) {
increase = false;
} else if (bright <= 10) {
increase = true;
}
prev_millis = cur_millis;
tone(BUZ_PIN, (double)bright+200, 85);
}
}
I ran it in evening and left for the night (with the unplugged buzzer, of course). In the morning the led stuck at one brightness level. I suppose the bug can be reproduced without tone(). Will create new issue soon. |
|
@X-Stas-EEE Thanks! Open the bug when you can. I'll see if I can get it running locally over the weekend and see if it's an obvious goof. One other question: Are you running at 80mhz or 160mhz? There are slightly different code paths involved, and I do most of my work by default at 160mhz (so SSL connections don't take forever...) |
|
@earlephilhower |
|
@earlephilhower |
| return b; | ||
| } | ||
|
|
||
| static inline ICACHE_RAM_ATTR uint32_t min_s32(int32_t a, int32_t b) { |
There was a problem hiding this comment.
what happens here for negative values?
-edit 5/14 to remove WIP -
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.
This setup enables multiple tones, analogWrites, servos, and
to be concurrent 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.
Using a simple USB DSO shows that with only a single channel running,
a "tone(1000)" generates a waveform of 999.8Hz (500.1us per high/low).
Running additional generators or steppers will potentially increase the
jitter when multiple edges need to be generated in close proximity (.5-1us).