Help with function

Need help i
i want to move "for control " out of the void loop() in a function with any name
This is my code:
#include <glcd_Config.h>
#include <glcd.h>
#include “fonts/allFonts.h”
gText textArea;
String stringUNO,stringDOS;

int PosX;
int PosY;
int PrMax ;
int PrMin;
int PrVal;
int analogPin1 =A1;
int Promedio;
int B ;
int A;
int Fr=300;
void setup()
{
GLCD.Init();
GLCD.SetDisplayMode(NON_INVERTED);
GLCD.ClearScreen();
GLCD.SelectFont(System5x7, BLACK); // font for the default text area
GLCD.DefineArea(90,8,110,8);
stringUNO = String(“PrVal.”);
stringDOS = String (“mb”);
pinMode(12, OUTPUT);
pinMode(13, OUTPUT);
PosX=90;
PosY=0;
GLCD.GotoXY(PosX,PosY);
GLCD.print(stringUNO);
delay(500);
}

void loop()
{
// i want to move this to function out of loop()
for(int i=1;i<100;++i){
PrVal= analogRead(analogPin1);
Promedio=0;
Promedio=Promedio+PrVal;
// till here
}
digitalWrite(12, HIGH);
PosX=90;
PosY=8;
GLCD.GotoXY(PosX,PosY);
GLCD.EraseTextLine();
GLCD.print (Promedio);
delay(Fr);
PosX=115;
PosY=8;
GLCD.print(stringDOS);
digitalWrite(12, LOW);
delay(Fr);

}
ok so i did this but it does not work,compile is ok but always
i can’t get PrVal printed vin my lcd, always is =0

#include <glcd_Config.h>
#include <glcd.h>
#include “fonts/allFonts.h”
gText textArea;
String stringUNO,stringDOS;

int PosX;
int PosY;
int PrMax ;
int PrMin;
int PrVal;
int analogPin1 = A1;
int Fr=300;

void setup()
{
Serial.begin(9600);
GLCD.Init();
GLCD.SetDisplayMode(NON_INVERTED);
GLCD.ClearScreen();
GLCD.SelectFont(System5x7, BLACK); // font for the default text area
GLCD.DefineArea(90,8,110,8);
stringUNO = String(“PrVal.”);
stringDOS = String (“mb”);
pinMode(12, OUTPUT);
pinMode(13, OUTPUT);
PosX=90;
PosY=0;
GLCD.GotoXY(PosX,PosY);
GLCD.print(stringUNO);
delay(500);
}

void loop()
{
digitalWrite(12, HIGH);
PosX=90;
PosY=8;
GLCD.GotoXY(PosX,PosY);
GLCD.EraseTextLine();
int Promedio ();
////////////////////////////////////////////////////////////////////////////////////////
GLCD.print (PrVal);// here does not print anything
/////////////////////////////////////////////////////////////////////////////////////
Serial.print (PrVal);//For debug only
Serial.print (Fr);//For debug only
delay(Fr);
PosX=115;
PosY=8;
GLCD.print(stringDOS);

digitalWrite(12, LOW);
delay(Fr);

}
int Promedio (){
for(int i=0;i<100;i++){
int GetPrVal= analogRead(analogPin1);
int Total= 0 + GetPrVal;
PrVal= Total/100;

return PrVal;
}
}

thx

There are a number of issues with this code. Compare the for loop you removed from loop with the one you put in the function. They are very different and the one in the function does not do what you intend at all.

This is another issue (I think):

int Promedio ();

I can't decide what the compiler will do with this - can anyone tell me?

To the OP, you've called a bunch of functions in this code. Make the call to Promedio look more like them.

Ok you let do it more simple

#include <glcd_Config.h>
#include <glcd.h>
#include “fonts/allFonts.h”
gText textArea;
String stringUNO,stringDOS;

int PosX;
int PosY;
int PrMax ;
int PrMin;
int PrVal;
int analogPin1 = A1;
int Fr=300;

void setup()
{
Serial.begin(9600);
GLCD.Init();
GLCD.SetDisplayMode(NON_INVERTED);
GLCD.ClearScreen();
GLCD.SelectFont(System5x7, BLACK); // font for the default text area
GLCD.DefineArea(90,8,110,8);
stringUNO = String(“PrVal.”);
stringDOS = String (“mb”);
pinMode(12, OUTPUT);
pinMode(13, OUTPUT);
PosX=90;
PosY=0;
GLCD.GotoXY(PosX,PosY);
GLCD.print(stringUNO);
delay(500);
}

void loop()
{
digitalWrite(12, HIGH);
PosX=90;
PosY=8;
GLCD.GotoXY(PosX,PosY);
GLCD.EraseTextLine();
int Promedio ();
GLCD.print (PrVal);//
Serial.print (PrVal);//For debug only
Serial.print (Fr);//For debug only
delay(Fr);
PosX=115;
PosY=8;
GLCD.print(stringDOS);

digitalWrite(12, LOW);
delay(Fr);

}
int Promedio (){
for(int i=0;i<100;i++){
int GetPrVal= analogRead(analogPin1);
int Total= 0 + GetPrVal;
PrVal= Total/100;

return PrVal;
}

}

allways i get PrVal=0

int Total= 0 +  GetPrVal;

Total will always be == the last reading, it is not accumulating.

int Total = 0;

for ()
    Total += analogRead(analogPin1);

Also what is the expected magnitude of the analogRead(), you are doing it 100 times it could easily overflow an int.

This is still here, what does it do?

int Promedio ();

I suspect that's supposed to be a call to Promedio(), remove the "int".

PrVal is a global, there's no need to return it from Promedio().

Having said that it's not good practice to have a "side affect" like that, it's not obvious from reading that line of code that Promedio() changes PrVal .

PrVal = Promedio();

is much better, just looking at the line you know that Promedio puts a new value in PrVal.


Rob

you are doing it 100 times it could easily overflow an int.

Actually, it isn't happening 100 times. The return statement is IN the for loop body, so the for loop partially executes once.

This is why putting the { and } on separate lines, with proper indenting is so important.

Why the function is returning a global variable is another mystery.

Eland:

Could you learn to use the (#) code tag on the menu above -- just look for the button - maybe edit your posts and put the code between the tags. It makes the posts a lot easier to read and understand.

Thanks.

The return statement is IN the for loop body, so the for loop partially executes once.

Correct, I didn't see that in the dodgy formatting.


Rob

elandd2011:
Ok you let do it more simple

...

GLCD.EraseTextLine();
int Promedio ();

...

You had exactly the same problem in your other thread about this entitled "My function it does not work" :

Now you have a new thread called "Help with function" in which you haven't fixed the very line that was causing the function to not be called.

thx guys
the problem was solved, the Promedio() is global because i wil use it later