Code optimisation - controlling 3 fans

I have three fans i am controlling using pwm, when i single click a corresponding button, it increases or decreases the pwm value and in turn the fan speed.

FanControl_Single(2);

When this function is called it uses the fan number to identify the fan

But this is where my code is repeated, id like a way of combining each case if possible.

Each fan has its own variables

  • fan1DutyCycle - stored pwm value - 0-255
  • fan1FadeAmount - stored pwm change amount - pos 15 or neg 15
  • fan1Ch - fan pwm channel

The code is working but id prefer to be able to have a single function rather then several repeated blocks of code.

As I am writing this im thinking maybe i could use an array for the values i.e rather than fan1DutyCycle write as fanDutyCycle[1]

void FanControl_Single(int fan){
  // PWM fade up and down 
  switch(fan){
    case 1:
        if (fan1DutyCycle <= 0 && fan1FadeAmount < 0) {
          fan1FadeAmount = -fan1FadeAmount;
        }
        else if (fan1DutyCycle >= 255 && fan1FadeAmount > 0) {
          fan1FadeAmount = -fan1FadeAmount;
        }  
        else{
          //NO CHANGE
        }
        fan1DutyCycle = fan1DutyCycle + fan1FadeAmount;
        ledcWrite(fan1Ch, fan1DutyCycle);
      break;
    case 2:
        if (fan2DutyCycle <= 0 && fan2FadeAmount < 0) {
          fan2FadeAmount = -fan2FadeAmount;
        }
        else if (fan2DutyCycle >= 255 && fan2FadeAmount > 0) {
          fan2FadeAmount = -fan2FadeAmount;
        }  
        else{
          //NO CHANGE
        }
        fan2DutyCycle = fan2DutyCycle + fan2FadeAmount;
        ledcWrite(fan2Ch, fan2DutyCycle);
      break;
     case 3:
        if (fan3DutyCycle <= 0 && fan3FadeAmount < 0) {
          fan3FadeAmount = -fan3FadeAmount;
        }
        else if (fan3DutyCycle >= 255 && fan3FadeAmount > 0) {
          fan3FadeAmount = -fan3FadeAmount;
        }  
        else{
          //NO CHANGE
        }
        fan3DutyCycle = fan3DutyCycle + fan3FadeAmount;
        ledcWrite(fan3Ch, fan3DutyCycle);
      break;     
    default:
      // statements
      break;
  }

why don't you make fanDutyCycle, fanFadeAmount and fan1Ch arrays and use the fan argument to the function, starting with 0, the index to the arrays?

thats what i am trying now, thanks greg

i have not tried it yet, but this is what i am thinking like you said.

void FanControl_Single(int fan){
  // PWM fade up and down 
  if (fanDutyCycle[fan] <= 0 && fanFadeAmount[fan] < 0) {
    fanFadeAmount[fan] = -fanFadeAmount[fan];
  }
  else if (fanDutyCycle[fan] >= 255 && fanFadeAmount[fan] > 0) {
    fanFadeAmount[fan] = -fanFadeAmount[fan];
  }  
  else{
    //NO CHANGE

its much more tidier and only one part of code to edit like i wanted

There are four pieces of information: pin number, stored duty cycle, change amount, and channel number. The channel number would be implicit except that it does not start at zero. Fortunately, these are all byte size quantities so a struct or a two-dimensional array could be used.

Or put the all in a structure

struct fanControl {
 byte dutyCycle;
 int fadeAMount;
 byte channel;
};

const int fanCount = 3;
struct fanControl fan[fanCount];

void setup() {};
void loop() {};
void FanControl_Single(int fanNum) {
 // PWM fade up and down
 if ( fanNum < 1 or fanNum > fanCount ) {
   // error - out of range
   return;
 }

 fanNum--;  // arrays are 0 based so fan is now 0...fanCount-1

 if (fan[fanNum].dutyCycle <= 0 && fan[fanNum].fadeAmount < 0) {
   fan[fanNum].fadeAmount *= -1;
 }
 else if (fan[fanNum].dutyCycle >= 255 && fan[fanNum].fadeAmount > 0) {
   fan[fanNum].fadeAmount *= -1;
 }
 else {
   //NO CHANGE
 }
 fan[fanNum].dutyCycle += fan[fanNum].fadeAmount;

 ledcWrite(fan[fanNum.channel, fan[fanNum].dutyCycle);
}

Your code does limit checking first, then increments and writes out the value. This will result in out of range problems.

I have not worked with structures before they look interesting, i will need to read up on them

as far as the limit checking of my code, is more of a reversal of the fade amount so the pwm increases by say 15

so 0 - 15 -30 - 45 etc but when it gets to 255 the fade amount is negated so then reverses 255 - 240 - 235 etc

for making debugging easier, a have added an extra value in the array which will not be used, so i can still use fan1 as 1 etc, i may change this when it is verified on my project.

int fanFadeAmount = {15, 15, 15, 15}; // how many points to fade the FAN by
int fanDutyCycle = {0, 0, 0, 0}; // how fast the fan runs

thank you for your suggestions, some great tips to learn from