Hi everyone my arduino is doing somthing weird

I set a pwm (“analog write”) in order to control a circuit , it reaches the right point but than its going down to zero and start again

here is my code: please ignore the parts that aren’t related to the analog write

//

int PV= 0;
int COL=1;
int BASE=2;
int IS=3;
int batc = 9;
float DA=0.0048875 ;
float ratio=7.66 ;
float pv, col, base, is ;
 
int i=0;
void setup() {
  pinMode(batc, OUTPUT);
  Serial.begin(9600);
}

void loop() {
  // put your main code here, to run repeatedly:
charger(10);

}


void charger(int acc ){
  int j;
  switch (acc){ 
    
  
    case 10: 
     i=0;
      while (j<1000000000){
        j++;
        
        if(cond){
          i=i+10;
          analogWrite(batc,i); 
          delay(1000); 
        }
        else {
          i=i-10;
          analogWrite(batc,i); 
          delay(1000); 
        }  
      }
  
      
      
      break;
      
      case 0:
      break;

      delay(100000);
      
  } 
}
void DigAn(){                              //translate analog value to true voltage
  pv=(analogRead(PV)*(ratio*DA));
  col=(analogRead(COL)*(ratio*DA));
  base=(analogRead(BASE)*(DA));
 // is= (analogRead(IS)*(ratio*DA));
}

int cond(){                               
   DigAn();
  if(col>base){
      return 1;
  }
  else
    return 0;

}

Your problem is solvable if you use code tags :smiley:

Please edit your post and place
** **[code]** **
before the code and
** **[/code]** **
after the code.

By the way, an int can store a maximum value of 32767 and a minimum value of -32768; definitely not one million.

if(cond){I think you meant:-if(cond()){
And that function can be simplified. Also, it could return a bool, instead of an int:-

bool cond()
{                               
   DigAn();
   return (col>base);
}

Edit: I’m not so sure about that ‘delay(100000);’ that’s floating loose in the ‘switch’ statement, either.
Shouldn’t it be under “default:” ?

Thanks for editing your post and adding the code tags.

few lines less than what you had. I don’t know why people like typing so much when they only have to write it once

while (j<1000000000){
        j++;
       
        if(cond){
          i=i+10;
       }
        else {
          i=i-10; // why -10?
        } 
          analogWrite(batc,i);
          delay(1000);
      }
 
     
     
      break;

Thomas499:
few lines less than what you had. I don’t know why people like typing so much when they only have to write it once

You still left this error in there:-if(cond){
And if we’re shortening things, instead of these:-

i=i+10;
.
i=i-10;

These:-

i+=10;
.
i-=10;

You’re typing too much too. :smiley:

And we can save a couple more characters of typing by leaving out some braces:-

while (j<1000000000ul)  // Not sure if "ul" is absolutely necessary, but it can't hurt.
{
    j++;
    if(cond())
        i+=10;
    else
        i-=10;
    analogWrite(batc,i);
    delay(1000);
}
break;

Don't take the braces. Leave the braces. They don't cost anything and they seriously help readability.

void charger(int acc ){
  int j;
  switch (acc){ 
    
  
    case 10: 
     i=0;
      while (j<1000000000){

j is an int. You can eliminate the while loop completely and just put in an infinite loop here if that’s what you want. You can safely assume that j will ALWAYS be less than 32767 no matter how many you add to it. But keep in mind that with an in, 32767 + 1 == -32768. Be sure that’s what you want before you add one more to it.

analogWrite(batc,i);

Also note that analogWrite only takes values up to 255. If you try to put in 256 it becomes 0. Keep adding to it and it climbs up to 255 and resets back to 0 again. Just like with a byte type, if you notice, 255 +1 == 0. SO it keeps rolling over because you've ignored the limits on those numbers. Does that sound like the problem you're seeing?

Delta_G:
Don't take the braces. Leave the braces. They don't cost anything and they seriously help readability.

I usually prefer to not have braces in those situations, but that's just my preference.

I know that a lot of people prefer to use braces aroung the one-liners. I was really only playing around earlier. :smiley:
i = i + 10;is OK too, if that's what barak1984 prefers.

Thank you all very much , the problam was of course the missing () of cond

about the typing thing.. when it didn't work I moved all my code to the main and tryied different things so it would work , when everything is working right I improve my code until its shorter

It has been a while since I played with the arduino and now I have a smart sla solar charger :slight_smile:
(at least the first charging stage.. )

Thank you all again