Pages: [1]   Go Down
Author Topic: A function that won't work as expected.  (Read 522 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Newbie
*
Karma: 0
Posts: 20
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I have run into a problem with a function I have. I use Arduino as ISP to program an Attiny 45. Below is my current fix that is working - proving that my library work. The scopeSens1.getValue(samples, variation) is a simple routine that defines a pin, read the analogvalue and does some filtering to return an average whereby outlying values is discarded in calculating the average. I call it many times in the rest of my program.

Code:
//This function while run when there is no scope sensors attached. This is a manual mode. Left button increase temp by 25%
//right button decrease temp by 25%
void manualControl() {
  byte manualPower = 0;
  fastPulse(0); fastPulse(1);  //indicate with LED manual mode is active
  while(1) {
 
  if (scopeSens1.getValue(samples, variation) >=maxSensOut) {
    if (manualPower <= 230) {   
      manualPower += 25;
    }
    else {
      manualPower = 255;
    }
  }
  if(scopeSens2.getValue(samples, variation) >=maxSensOut) {
    if(manualPower >= 25) {
      manualPower -= 25;
    }
    else {
      manualPower = 0;
    }
  }

  analogWrite(heaterPin[0], manualPower);
  analogWrite(heaterPin[1], manualPower);
  delay(250);
  }
}

As you can see in the working code I used a byte value for power and I make sure that it does not go out of bounds, i.e. 0 - 255 only since it is used for analogWrite.

And here is the problematic code I first used:
Declaring "manualPower" as a byte below, fail, because the value goes out of bounds. But at least then code causes the output to step up and down as expected, but then "rolls over" because it can become out of range for a byte. (A valuable lesson I learned the hard way!  smiley-mr-green)

Anyway, so I changed the variable to an integer as below. And then it all goes wrong. Pressing a button to step the power up, both outputs goes full high immediately, releasing the outputs goes low immediately. Is this normal behaviour?

Code:
//This function while run when there is no scope sensors attahced. This is a manual mode. Left button increase temp by 25%
//right button decrease temp by 25%
void manualControl() {
  int manualPower = 0;
  fastPulse(0); fastPulse(1);
  while(1) {
 
  if (scopeSens1.getValue(samples, variation) >=maxSensOut) {
 
      manualPower += 25;

    if (manualPower > 255) {
      manualPower = 255;
    }
  }
  if(scopeSens2.getValue(samples, variation) >=maxSensOut) {
    if(manualPower >= 25) {
      manualPower -= 25;
    }
    if (manualPower < 0) {
      manualPower = 0;
    }
  }

  analogWrite(heaterPin[0], manualPower);
  analogWrite(heaterPin[1], manualPower);
  delay(250);
  }
}


And here is a similiar piece of code I use in the same Sketch that work as expected.
In this case "powerLevel" does the same. I've higlighted the specifics in red below. It is under the comment, "Power handling statements."


Code:
void adjustTemp(int n) {
  int powerLevel = heaterPower[n];
  //next two "if" statements set logic when a sensor is not attached. Possibly future can have the logic in the getTemp() function.
  //this is only when one scope Sensor is missing, it will set it the same as plugged in sensor
  if(sensorAttached[n] == false && n == 0) {   //if sensor 1 is not attached, set its value to the attached sensor.
    tempSens[n+1] = tempSens[2];
  }
  if(sensorAttached[n] == false && n == 1) {    //if sensor 2 is not attached, set its value to the attached sensor.
    tempSens[n+1] = tempSens[1];
  }
 
  tempSens[n+1] -= tempDiff[n];  //subtract the required difference to "load" the scope sensor. Thus scope sensor appear to be "colder"
  tempSens[n+1] += calibrationValue[n]; //add the initial calibration value


 //Power handling statements below   
  [color=red][font=Verdana]if (tempSens[n+1] >= tempSens[0]) {      //will decrease power in steps of 25 or 10%
    powerLevel = powerLevel - 25; 
    if (powerLevel < 0) {
      powerLevel = 0;[/font][/color]
    }
  }
 
  if (tempSens[n+1] < tempSens[0])  {  //increase power in steps of 25 or 10%
   [color=red] powerLevel += 25;
    if (powerLevel > 255) {
      powerLevel = 255;[/color]
    } 
  }
  //Below "if" will set heater outputs to 50% permanent on, when no scope sensors is connected.
  if (sensorAttached[0] == false && sensorAttached[1] == false) {
    powerLevel = 128;
  }

 
// heaterPower[n] = powerLevel;   // -- previous method without full power override
//set full power override mode.
 if (heaterFullPower[n] == true) {
   analogWrite(heaterPin[n], 255);   //write full power to heater
 }
 else {
   heaterPower[n] = powerLevel;
   analogWrite(heaterPin[n], heaterPower[n]);
 }
}

I would be glad to get some idea what is the difference between the two functions, and which one is the better way of ensuring steps does not overrun the max value of analogWrite().
Regards
PS. I am unable to "Copy for Forum" from the IDE. Lots of complaints about Java dispatchthread...
Logged

UK
Offline Offline
Shannon Member
****
Karma: 222
Posts: 12541
-
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

The way you've posted the code is actually the preferred way - the IDE's 'copy for forum' feature is a trap for the unwary.

So far I haven't noticed anything wrong with the way you increment or decrement the values, but I also see no trace output that would enable you to see what the sketch was doing. Can you write a simple sketch that demonstrates the value "going out of bounds" without involving any other libraries or external hardware?

The most suspicious thing I can see so far in that code is rather dodgy use of the argument to adjustTemp; that looks like a recipe for exceeding the array bounds for the various arrays indexed by [ n+1 ]. Since you haven't shown all of your code I can't say for sure, but it looks like a bug to me. If you're planning to use this as an index into a circular buffer, then you could resolve that problem by applying the modulo operator to (n+1).
Logged

I only provide help via the forum - please do not contact me for private consultancy.

London
Offline Offline
Edison Member
*
Karma: 46
Posts: 1368
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I have run into a problem with a function I have. I use Arduino as ISP to program an Attiny 45. Below is my current fix that is working - proving that my library work. The scopeSens1.getValue(samples, variation) is a simple routine that defines a pin, read the analogvalue and does some filtering to return an average whereby outlying values is discarded in calculating the average. I call it many times in the rest of my program.

Code:
    powerLevel = powerLevel - 25; 
    if (powerLevel < 0) {
      powerLevel = 0; 


Here you're puting powerLevel out of bounds and then correcting it.
Try instead
Code:

if (powerLevel<25){ //Check first if the change will take powerLevel out of bounds
powerLevel =0; //if so, set it to zero
}
else{
powerLevel = powerLevel - 25 //won't go out of bounds, so ok to change powerLevel
}
------------------------------------------------------------------------------
if (powerLevel>230){ //Check first if the change will take powerLevel out of bounds
powerLevel =255; //if so, set it to 255
}
else{
powerLevel = powerLevel + 25 //won't go out of bounds, so ok to change powerLevel
}
That way powerLevel can never go out of bounds. Treat all your variables in the same way.
Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 20
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Thank you.
My first example does the same as Henry_Best. And after a nights sleep, I realise I should change my code accordingly for the other examples to be on the safe side. Then I can use a byte variable  and I will also safe a few bytes of flash doing it.

Just about the [n+1] as PeterH mentioned. The coding is correct in this case.
I have three temperature sensors. One for air, two telescopes. I have only two heaters. My Sketch compare scope and air temperatures and adjust heaters accordingly.
I build my Sketch functionality in blocks, so I started by defining the air sensor as tempSens[0] and scope sensors as tempSens[1] and tempSens[2] as I tested and implemented. I call the function adjustTemp() at set times with millis() and to save code, I used a loop to check and adjust both sensors and heaters. Then I have several arrays that is connected to a specific sensor, like calibrate[], sensorAttached[] heaterPower[] etc, and of course they all start counting at 0. So tempSens[1] is linked to sensorAttached[0], tempdiff[0] etc. Of course with hindsight I should have used tempSens[2] for air, so that I can drop the [n+1] altogether! But for now I reckoned my project can add [n+1] faster and cheaper than I can correct my code.  smiley-wink

You've both provided me with quite some insight!
Logged

Pages: [1]   Go Up
Jump to: