I'm trying to create a reusable function from working code for a digital flow detector, but I guess I'm doing something wrong.
When I test the code written in the loop() function it works fine, but when I created a function from the code the serial monitor still prints the detected change but doesn't change the 'flowPresent' boolean.
I'm sure there are better ways to tackle this flowdetecting problem, but my main question is why the flowDetection() function isn't working whilst it is the same code?
I'm using a Lolin D32 ESP32.
here is a copy of the code that works fine:
static bool flowPresent = 0;
static bool lastStateFlow = 0;
static unsigned long lastChange = 0;
unsigned long pulse = 0;
const int flowPin = 13;
void setup() {
pinMode(flowPin, INPUT);
Serial.begin(115200);
while (!Serial);
Serial.println("Serial Connected");
}
void loop() {
bool flow = digitalRead(flowPin);
if (flow == !lastStateFlow) {
//Serial.println("change detected");
lastStateFlow = flow;
if (millis() - lastChange > 5000) {
// nog even negeren indien laatste verandering 5s of langer geleden is
lastChange = millis();
} else {
// verandering is nog niet zolang geleden: reactie na tien pulsen
pulse += 1;
lastChange = millis();
if (pulse > 10) {
flowPresent = 1;
pulse = 0;
}
}
} else {
if (millis() - lastChange > 1000) { flowPresent = 0; pulse = 0; }
}
Serial.println(flowPresent);
}
and this is a copy of the code with the flowDetection() function
const int flowPin = 13;
void setup() {
pinMode(flowPin, INPUT);
Serial.begin(115200);
while (!Serial) { Serial.print("."); };
Serial.println("Serial Connected");
}
void loop() {
bool waterFlow = flowDetection(flowPin);
//Serial.print("waterFlow = ");
//Serial.println(waterFlow);
}
bool flowDetection(int flowPin) {
static bool flowPresent = 0;
static bool lastStateFlow = 0;
static unsigned long lastChange = 0;
unsigned long pulse = 0;
bool flow = digitalRead(flowPin);
if (flow == !lastStateFlow) {
Serial.println("change detected");
lastStateFlow = flow;
if (millis() - lastChange > 5000) {
// nog even negeren indien laatste verandering 5s of langer geleden is
lastChange = millis();
} else {
// verandering is nog niet zolang geleden: reactie na tien pulsen
pulse += 1;
lastChange = millis();
if (pulse > 10) {
flowPresent = 1;
pulse = 0;
}
}
} else {
if (millis() - lastChange > 1000) { flowPresent = 0; pulse = 0;}
}
Serial.println(flowPresent);
return flowPresent;
}
This is my first post in a forum like this one, so I hope the layout of the code is a success.
Thanks in advance for your help and insights!
all looking good - congrats for providing lots of information in the right format
passing a pin to your flowDetection() function seems to indicate you would want that to work for multiple pins possibly, but that won't happen because you have only one set of static variables inside the function. So if you were to call flowDetection on two different pins, that would fail as states would get mixed up.
➜ it's not a good candidate for a function with a parameter, but could still be made as a function working on the global parameters.
the issue in your code is probably linked to
unsigned long pulse = 0;
in the first code it's a global variable, so it will be remembered across calls to the loop()
in the second code, it's a non static local variable to the function, so will be reset to 0 at every call.
➜ make it static
EDIT: basically exactly what @alto777 said :), he was faster typing !
thanks, you're right about the static variable. I must have overlooked that a 1000 times :).
You just saved me another 100 times looking over the code!
indeed, a7 was right. Thanks for the extra information, I'll keep it in mind for the future. I didn't think about that before. It was never really the plan to use it for more than one flowsensor (I only have one), but more an exercise to create functions to keep the code more readable.
It says check if the incremented pulse value is larger than 10. Kinda does the same as adding one first and then doing the if, but all in one line
In the old days that was helping the compiler know it could keep the result of the addition in a register and use that for the comparison, so was faster. Nowadays the compiler/ optimizer are smart enough to do this even if you wrote it in two lines
For old timers it’s natural to embed the pre-increment into the test .
Kids can learn the longer way and not risk getting the ++ on the wrong side of the variable
Having a constant first in an inequality is not useful (it’s more difficult to read as it’s not natural) and does not bring any safety. It’s more useful for == (avoids writing an assignment statement instead of the comparison by mistake) but you’ll likely get a warning nowadays as well if you wrote = instead of ==