Can someone tell me what I am doing wrong?

I'm having some issues with a sketch I wrote for a fan controller using temperature sensors. The problem is that the fan curve is all screwed up...it goes from minimum speed to maximum speed and it skips all the steps in between. The code is too long to post (the forum won't let me)...so I am just posting the function that sets the fan speed. What do I need to change?

void SetFan(){                                                                       ///
  if (minTempC > maxTempC) (analogWrite(FanPin, 250));                               ///
  else{                                                                              ///
                                                                                     ///
  if (CTempArray[6] >= CTempArray[7]) (highestReading = CTempArray[6]);              ///
  else (highestReading = CTempArray[7]);                                             ///
                                                                                     ///
  degreeSeperation = (maxTempC - minTempC);                                          ///
  degreePerStep = (degreeSeperation / 12);                                           ///
                                                                                     ///
  if (highestReading >= maxTempC) (TMP07 = 250);                                     ///
  else if (highestReading >= (maxTempC - degreePerStep)) (TMP07 = 235);              ///
  else if (highestReading >= (maxTempC - (2 * degreePerStep))) (TMP07 = 220);        ///
  else if (highestReading >= (maxTempC - (3 * degreePerStep))) (TMP07 = 205);        ///
  else if (highestReading >= (maxTempC - (4 * degreePerStep))) (TMP07 = 190);        ///
  else if (highestReading >= (maxTempC - (5 * degreePerStep))) (TMP07 = 175);        ///
  else if (highestReading >= (maxTempC - (6 * degreePerStep))) (TMP07 = 160);        ///
  else if (highestReading >= (maxTempC - (7 * degreePerStep))) (TMP07 = 145);        ///
  else if (highestReading >= (maxTempC - (8 * degreePerStep))) (TMP07 = 130);        ///
  else if (highestReading >= (maxTempC - (9 * degreePerStep))) (TMP07 = 115);        ///
  else if (highestReading >= (maxTempC - (10 * degreePerStep))) (TMP07 = 100);       ///
  else if (highestReading >= (maxTempC - (11 * degreePerStep))) (TMP07 = 85);        ///
  else (TMP07 = 70);                                                                 ///
                                                                                     ///
  TMP08 = (lastFanSpeed * 31);                                                       ///
  TMP09 = (TMP08 + TMP07);                                                           ///
  lastFanSpeed = (TMP09 / 32);                                                       ///
  if (lastFanSpeed > 254) (lastFanSpeed = 254);                                      ///
                                                                                     ///
  analogWrite (FanPin, lastFanSpeed);                                                ///
  }                                                                                  ///
}                                                                                    ///

BTW...I uploaded the whole code to my website here: http://www.killerbug.net/TEMPSHARES/PS3_Slim_Sketch_v12_working_except_fan_curve.pde

Your use of braces {} and parentheses () is a little erratic. Don't use the latter when you should really be using the former. Why don't you break your problem down and concentrate on the bit that is giving you problems, instead of presenting us with a mass of code you can't post? If there's an unexplained in version, it may be a hardware issue.

Thanks for the reply. I'm afraid I don't quite understand...when I replace parentheses with braces the code fails checks. The only ones I can change are the single-line commands at the ends of the "IF" statements...and I tried that; it made no difference. As soon as the temperature hits the minimum, the fan goes from 28% to 86%.

I thought I had posted just the part that is giving me problems...that is, the function that determines the fan speed based on the temperatures read and the "minimum" and "maximum" temperatures set by the user. Are you saying that you think it is something elsewhere that is causing these issues?

Also, what did you mean by, "an unexplained in version"?

BTW...this code is being run on a Teensy 2.0++, just in case you were wondering why there were 8 temperature readings.

Your code looks terrible from that link. Could you save your code as a pdf or something and attach it at the bottom " Additional Options "

Your code looks terrible from that link. Could you save your code as a pdf or something and attach it at the bottom " Additional Options "

Why should the OP do that? Save the pde file and load it in the Arduino editor.

Code can be made much simpler with the map function as it seems to be a linear function.

void SetFan()
{
  if (minTempC > maxTempC)
  {
    analogWrite(FanPin, 250));
    return;
  }
  highestReading = max(CTempArray[6], CTempArray[7]);
  TMP07 = map(highestReading,  minTempC, maxTempC, 70, 250);
  TMP07 = constrain(TMP07, 70, 250);
  lastFanSpeed = (lastFanSpeed * 31 + TMP07)/ 32;
  analogWrite (FanPin, lastFanSpeed);
}

Sorry, a space crept in, I meant ‘inversion’

What are the values of maxTempC and minTempC?

KillerBug: it goes from minimum speed to maximum speed and it skips all the steps in between

How about you try to write a program that sets the speed to "middle" and does nothing else. Can you do that? If so, how about a program that sets the speed to "minimum," "middle" or "maximum" depending on either a button, or potentiometer, or perhaps temperature sensor? Once you're there, you can probably go from that to a finer resolution program.

First, let me appologize for how the code looks in the link...I have not done ANY optimization whatsoever...I am trying to get it working before I do that so I know if there are any issues after optimization I know they are from optimization and not from my starting point.

robtillaart - I will have to read up on map...I never even heard of it as I am an extreme beginner at all this (I am sure that was obvious from the liberal use of "IF" statements). I copied your SetFan() function to my sketch, deleted the extra parentheses, and it works. I don't quite understand how it works yet, but it does work, so you have my sincere thanks. I'll make sure to give you credit for that function in future revisions of the source code (don't worry...I won't give you credit for the rest of my mess of a program)

DC42 - The values of maxTempC and minTempC are set by the user. This is version 1.2, version 1.0 didn't work and version 1.1 just had maxTempC, with minTempC locked to 10C lower. The value range of maxTempC is 10-85 and the value range of minTempC is 10-75 (and if minTempC is ever higher than maxTempC due to incorrect settings, it just sets the fan to about 96%).

jwatte - I need more than 3 steps. From previous experiments with this fan, 12 steps is the least I can get away with without the fan changing speed so quickly that it is like a reving engine.

This forum is great...hopefully one day I will know enough about all of this to give something back.

Video of what I have so far…

when I replace parentheses with braces the code fails checks

Yes, they would, e.g.if (CTempArray[6] >= CTempArray[7]) (highestReading = CTempArray[6]); compiles, but if (CTempArray[6] >= CTempArray[7]) {highestReading = CTempArray[6]}; obviously doesn't. However, using correct C syntax if (CTempArray[6] >= CTempArray[7]) {highestReading = CTempArray[6];} does compile. Indentation would also help legibility. Cose a style and stick with it.

But, yes, "map" is a better option.

I usually only use brackets for multiple things like:

if (1 = 1) (doSomething());

...but if I need multiple things I use brackets like:

if (1 = 1){ doSomething(); doSomethingElse(); }

It seems to get the same result, and doing it that way I never confuse ( for {.

Just to confirm, these two statements are identical, right? if (1 = 1) (doSomething()); if (1 = 1) {doSomething();}

I usually only use brackets for multiple things like:

if (1 = 1) (doSomething());

You can’t assign a value to a literal. The ()s around doSomething() are not needed, and look like you don’t know what you are doing. Stop using them. Period.

Curly braces, on the other hand, should be used for every block, needed or not. It makes it easier to add code to a block when the { and } are already there.

KillerBug: jwatte - I need more than 3 steps.

Yes. I was giving you advice on how to get there, one step at a time. When you have a problem, it's usually helpful to eliminate all possible sources of the problem, and start from something very small that works. Then add one thing at a time, until it breaks. Because it worked before, you now know what it is that breaks.

So: can you build the system to have only 3 speeds? Does that work? If so, you know that you can control the speed of the fan, and your problem lies in the logic. If you cannot build that system, then the problem is in controlling the fan in the first place.