CapnBry / HeaterMeter

HeaterMeter and LinkMeter Arduino BBQ Controller
https://tvwbb.com/forums/heatermeter-diy-bbq-controller.85/
MIT License
502 stars 83 forks source link

[hm] Fan SMPS Patch #11

Closed R-Burk closed 9 years ago

R-Burk commented 9 years ago

Patch allows using the full range of the PWM. Stops the feedback from rolling over to a high value at low power. Minimizes the SMPS hunting for a stable value at low power.

CapnBry commented 9 years ago

I'm not sure what issue this patch is supposed to address. The original code can PWM 0-255, the "full range of the PWM".

Is there a combination of ffeedback, _lastBlowerOutput, and _feedvoltLastOutput that would cause an overflow? I don't think this much code needs to be added to solve a simple overflow and the complexity of the code is much more than is needed to control the output voltage.

R-Burk commented 9 years ago

Original code uses the arduino analogWrite which does PWM over 1-254 and the does pinmode on 0 or 255. AVR though in fast PWM mode treats 0 PWM differently than. Similar to how analogRead and digitalWrite have been rewritten as arduino code is generic not specific to the situation.

The main problem is that the code is trying to linear map(fan speed/_lastBlowerOutput) to an exponential requirement(_feedvoltLastOutput). The last 30 points of _lastBlowerOutput ends up matching to less than 30 points of _feedvoltLastOuput ( it's the left hand part of the knee).

So what happened is I was seeing ffeedback drive _feedvoltLastOuput past 0. Then it would spend a couple of seconds racing back down and then loop. To the ear it sounds like a really weird pulsing scheme.

Going to a signed int stopped the roll over and instead I would find the fan shutting off due to analog write's interpretation of 0 hence I took over from analogwrite with fanvoltwrite.

The last part with all the if sentences is really smoothing with progressively more division into the ffeedback. This could probably be shortened or maybe a different algorithm. I was kind of tired at that point and the fan no longer annoyed me listening to it.

Is the rollover needed? definitely Is the fanvoltwrite needed? I need the extra resolution and probably anyone else using an axial fan. Could try going to inverted fast PWM. Then you only lose resolution at the top. Would need a macro to invert the _feedvoltLastOuptut I suppose the 4 if sentences could be reduced to two - normal gain ( divided by 2, ie * 64) and a low gain ( divide by 16, ie *8). Would have to test if it slows down coming back up out of the bottom part of the power curve

CapnBry commented 9 years ago

I've gone over this code many times and I still can't see any code that can roll over.

// both operands are unsigned char, so they go 0-255. The int casts allow this math
// to work fine, error will be in the range -255 to 255.
int error = ((int)_lastBlowerOutput - (int)ffeedback);
// _feedvoltLast output is an unsigned char too, promoted to signed int means max final value
// -127 to 382
int newOutput = (int)_feedvoltLastOutput + (error / 2);
// newOutput constrained back to original size without overflow
_feedvoltLastOutput = constrain(newOutput, 0, 255);

Can you provide a set of _lastBlowerOutput, ffeedback, and _feedvoltLastOutput actual values that create the rollover condition?

I do see what you're saying about a negative value possibly shutting off the PWM. In my design that was the desired behavior. Anything higher than 254 means "turn on DC power" and anything lower than 1 means "power off". Sure, we could squeeze one more step out of the PWM but that's like 0.4% of the range so if you're down that low then you're pretty much outside the controllable range anyway and 1 more tick isn't going to solve all your problems. I could see the case for changing the constrain to (1,255) instead of (0,255) to prevent it from turning off completely.

R-Burk commented 9 years ago

Figured out that I got lead astray when I first turned on GRILLPID_FEEDVOLT_DEBUG. As the out= line is the temp newOutput instead of _feedvoltLastOuput.

So I'm wrong and _feedvoltLastOuput doesn't wrap.

Values: Speed _lastBlowerOuput, ffeedback, _feedvoltLastOuput 30 134 133-134 3-4 (last stable speed) 25 127 24-168 0-51 (audible speed pulsing) 20 121 24-167 0-48 15 114 23-163 0-45 (fan can stall then restart sometimes) 10 107 23-160 0-41 (have to approach slowly or fan stalls below here) 5 100 23-158 0-36 1 95 23-156 0-34

Noticed the fan would die if it got two 0's in a row. And a 0 _feedvoltLastOutput produced the really low feedback on the next read which then made it jump up high. So I would output sequences of 36, 12, 4, 0 repeat

Also ran the suggested: _feedvoltLastOutput = constrain(newOutput, 1, 255);

Speed _lastBlowerOuput, ffeedback, _feedvoltLastOuput 30 134 134-135 4 25 127 129-130 1 20 121 129-130 1 .... 1 95 129-130 1

I went back and ran this against my fanvoltWrite() . The simply change to the original code of making the constrain (1,255) seems to work just as well if not better

Glad you made this a simpler fix. Hopefully this make others fans run more reliably at lower power levels.

CapnBry commented 9 years ago

Awesome! I really appreciate your work and detailed explanations. I hope I don't sound too gruff about not immediately merging changes, but I work really hard to keep the code size as small as possible because it isn't an infinite resource. I'm glad the new constrain limit works for you.

Committed in 7d06e4d7cb80d34dcccf7cffca07de3f4c7d472f and forgot to tag this #11 in the comment, but thanks again!