Program only works when I use Serial.print in the function

Hallo, I struggle with fallowing problem:

The code bellow works perfectly:

.
.
.

long newTime, lastRequestTemperature = 0, lastRequestFanPWM = 0;
volatile long* pointerTimeOfLastRequest_Temperature = lastRequestTemperature, pointerTimeOfLastRequest_FanPWM = lastRequestFanPWM;

.
.
.


void setup() {
  Wire.begin();      
  Serial.begin(9600);  
  
  Wire.beginTransmission(adressDisplay);
  configurateLCD();
  Wire.endTransmission();
}

void loop() {
  processValueFromRegulator(temperature, intervalRequestTemperature, pointerTimeOfLastRequest_Temperature);
  processValueFromRegulator(fanPWM, intervalRequestFanPWM, pointerTimeOfLastRequest_FanPWM);
}

void processValueFromRegulator(byte requestValue, int interval, long* timeOfLastRequest) {
  //Serial.println(*timeOfLastRequest, HEX);
  if (intervalTimePassed(interval, *timeOfLastRequest)) {
    if (isDeviceResponding(adressRegulator)) {
      requestFromRegulator(requestValue);
    }
  }
  
  if (Wire.available()) {
    serveIncomingTWI(sizeof(float), &incomingTWI[0]);  
    displayOnLCD(requestValue, mergeBytesToFloat(&incomingTWI[0]));
  }  
}

bool intervalTimePassed(int intervalTime, long *lastSuccesfulTime) {
   Serial.println(*lastSuccesfulTime, HEX);
   newTime = millis();
   if (newTime - (*lastSuccesfulTime) >= intervalTime) {
     (*lastSuccesfulTime) = newTime;
     return 1;
   }
  
  return 0;
}

.
.
.

But if I comment all Serial.prints, LCD stops displaying values. So I assume intervalTimePassed() never returns true, so requestFromRegulator() won't ever be called thus displayOnLCD() also won't be called because there won't be any Wire.available().

However I don't know how to check it because when I use Serial.print in intervalTimePassed() OR processValueFromRegulator() everything works great.

Any help will be appreciated.

PS. It has to be Serial.println(*lastSuccesfulTime) in intervalTimePassed() or Serial.println(*timeOfLastRequest) in processValueFromRegulator() to make it work.

That is not a complete program. You need to post a complete program that illustrates the problem. The fault is often in the part of the program you are not looking at - which is why it is not spotted.

Serial.print() can have the effect of acting as a short delay()

All variables associated with millis() should be unsigned long.

...R

Robin2: Serial.print() can have the effect of acting as a short delay()

I know, I tried using delay() with no effect.

Robin2: All variables associated with millis() should be unsigned long.

Wow, I forgot about such a basic thing! Now it works correctly without Serial.print().

Thank you!

Warwick1156: Now it works correctly without Serial.print().

Oh that sounds bad. Bet you a buck that you make some small change in the near future that inexplicably breaks the code again. Because if this was all you needed then the print statement wouldn’t have made it work.

Check all you array boundaries. I bet you went off one.

You are dereferencing a NULL pointer.

volatile long* pointerTimeOfLastRequest_Temperature = lastRequestTemperature, pointerTimeOfLastRequest_FanPWM = lastRequestFanPWM;

This defines a pointer to long named pointerTimeOfLastRequest_Temperature and initializes it to the value of lastRequestTemperature (which is a long, so you should have gotten a compiler warning about converting a long to a pointer; if not, turn all verbose compiler output!).

This also defines a long (not a pointer to long) named pointerTimeOfLastRequest_FanPWM and initializes it to the value of lastRequestFanPWM.

Do you even need these two variables? Do they ever change? I don't see them change anywhere in the code you posted. You can probably just take the address of lastRequestTemperature and lastRequestFanPWM when you need pointers to those variables.

christop: You can probably just take the address of lastRequestTemperature and lastRequestFanPWM when you need pointers to those variables.

You are right, I fixed that after I stopped focusing on Serial.print(). And then I realized that code stops working after few moments because I was dereferencing a NULL pointer as arduino_new said.

Code below works stable:

[...]
unsigned long newTime, lastRequestTemperature = 0, lastRequestFanPWM = 0;
[...]

void loop() {
  processValueFromRegulator(temperature, intervalRequestTemperature, &lastRequestTemperature);
  processValueFromRegulator(fanPWM, intervalRequestFanPWM, &lastRequestFanPWM);
}

void processValueFromRegulator(byte requestValue, int interval, unsigned long timeOfLastRequest) {
  if (intervalTimePassed(interval, timeOfLastRequest)) {
    if (isDeviceResponding(adressRegulator)) {
      requestFromRegulator(requestValue);
    }
  }
  
  if (Wire.available()) {
    serveIncomingTWI(sizeof(float), &incomingTWI[0]);  
    displayOnLCD(requestValue, mergeBytesToFloat(&incomingTWI[0]));
  }  
}

bool intervalTimePassed(int intervalTime, unsigned long *lastSuccesfulTime) {
   newTime = millis();
   if (newTime - (*lastSuccesfulTime) >= intervalTime) {
     (*lastSuccesfulTime) = newTime;
     return 1;
   }  
   return 0;
}

[...]

Thanks for help!

PS. I still have no idea how Serial.print() made previous code work although I was dereferencing a NULL pointer.

Warwick1156: PS. I still have no idea how Serial.print() made previous code work although I was dereferencing a NULL pointer.

Dereferencing a NULL pointer is undefined. So anything can happen and any change anywhere can change what that something is. It can be anything from code works as expected to making demons fly out from your nose when you turn it on. That's why I made my last comment about it being scary. I thought buffer over-run because that's probably the most common one, but I knew it was something creating an undefined state.

@jbooysen - we can’t see your code.

(New to Arduino - first attempt at a "delay-on timer") I have the same issue, using a "Serial.print()" making the function work, but I am not using any pointers, hence it cannot be a "dereferencing a NULL pointer" issue. Any more ideas? Pretty sure my data types are all good using "unsigned longs" with all "millis()" associated variables.

I suspect the above problem may not be properly fixed yet and that it may just appear to be good.

With no "Serial.print" commands, the code below does not work.

The code does work with a work-around printing "millis() - StartTm", but will not work with printing "ElapsedTm" (commented out), although they should be the same.

One can test by un-commenting "//Serial.print(ElapsedTm);" and commenting "Serial.print(millis() - StartTm);", and see what happens in the serial monitor.

This is uploaded to an Uno.

/*This is my first attempt at a timer*/

// *********  Variable Declaration  *********
  bool                  StartDelay;
  
  // Timer 1 - Delay ON
  bool                  Tmr1En;                 //Timer Enable Input
  const unsigned long   Tmr1Preset = 1000;      //Timer Preset Time
  bool                  Tmr1Q;                  //Timer Output

// *********  INITIALISATION *********
  //"setup" function runs once when you press reset or power the board
  void setup() {
    // initialize serial port.  
    Serial.begin(9600);
    
    Tmr1En = false;
    Tmr1Q = false;
  }

// *********  Main Program Body *********
  // the loop function runs over and over again forever
  void loop() {
    
    StartDelay = (millis() >= 500);
    
    Tmr1En = StartDelay && !Tmr1Q;
    Tmr1Q = TON(Tmr1En, Tmr1Preset);
    
    Serial.print("   Q");
    Serial.println(Tmr1Q);
  }

// *********  RE-USEABLE USER DEFINED FUNCTIONS *********
  // Delay ON Timer - TON
  bool  TON (bool En, unsigned long Preset){
          unsigned long StartTm;
          unsigned long ElapsedTm;
          bool Q;
          
          if (!En) {                              // Initialise the timer when NOT Enabled
            StartTm = millis();
            ElapsedTm = 0;
            Q = false;
          }
          else {                  
            ElapsedTm = (millis() - StartTm);     // Start timing once the timer is Enabled
            if (ElapsedTm >= Preset) {Q = true;}  // Switch the output ON
            else {Q = false;}
          }
          Serial.print(" ET");
          //Serial.print(ElapsedTm);
          Serial.print(millis() - StartTm);
          return Q;
  }

In your function TON() you are using a variable without giving it a value first.

/Users/john/Library/Arduino15/packages/arduino/hardware/avr/1.8.2/cores/arduino/main.cpp: In function 'main':
/Users/john/Documents/Arduino/sketch_jun20a/sketch_jun20a.ino:53:15: warning: 'StartTm' may be used uninitialized in this function [-Wmaybe-uninitialized]
     ElapsedTm = (millis() - StartTm);     // Start timing once the timer is Enabled
               ^

Even if you call the function with 'En' set to 'true' first, your local variable 'SetTm' is NOT marked 'static' so it gets created each time the function starts.

I recommend:

 static unsigned long SetTm = 0;

(or make it a global variable by declaring it outside any function)

You're recording StartTm in a local variable...

 bool  TON (bool En, unsigned long Preset){
          unsigned long StartTm;

Local variables are not preserved over function calls.

It's therefore anyone's guess what...

ElapsedTm = (millis() - StartTm);

...will give you. Complete garbage depending what was on the stack where the uninitialized StartTm is placed.

StartTm needs to be global.

Thanks johnwasser and pcbbc, I was not aware of the variables not being preserved over function calls. That would explain why my "StartTm" is "lost/garbage". I expected a user function to behave like an "instance", where each instance would remember its own variables.

Declaring variables as "static" will only solve my problem for a single instance (but it does work, thank you). A second instance will thus overwrite the first instance's variables.

Back to the drawing board for me...

If you want an instance, you need a class. Function calls are not “instances”.

Declaring variables as "static" will only solve my problem for a single function call (but it does work, thank you). A second function call will thus overwrite the first function call’s variables.

Thanks pcbbc,

I have come to realize that it is not necessary for a function (or class) at all for my delay-on timer I wanted to create, since it only requires three/two lines of code anyway (see below) - not worth the effort for a class.

However, I will research the "class" in the forum for some other "standard building blocks" I want to create.

Thanks again.

// Timer 1 - Delay ON
    Tmr1En = !Tmr1Q;  // this can be any timed condition - here i use the timer itself to create an astable
    if (!Tmr1En) {Tmr1StartTm = millis();}
    Tmr1Q = ((millis() - Tmr1StartTm) >= Tmr1Preset);