Simple computation requires presence of Serial.print statement in for loop

CrazyDog

Active member
I'm using a T4.1 (600MHz clock) in a fairly complex program that regulates the current in a sensor using PWM to control the voltage in a resonant circuit(No problems here). I am interleaving two A/Ds to achieve 1MHz 12 bit sampling of an analog signal the average value of which R_AVG is used as feedback for the regulation routine. In addition, the samples( placed in a 2000 value buffer) are converted to cr terminated strings and sent as UDP Ethernet messages to a Windows 11 PC using a pair of Ethernet to wifi bridges ( no problems here either). I have verified the integrity of received sample data using the Wireshark network analysis program. The regulation routine relies on the current value of R_AVG ( global variable) equal to the arithmetic average of the 2000 samples in the buffer. The Regulate() function is called from the Loop() after the average is computed and interrupts are re-enabled just prior to leaving the loop.
In loop(), I disable interrupts on entry to prevent any A/D conversions from being triggered by the A/D service interrupt routine while I process the 2000 samples in the buffer array. A semaphore( bufferdone) indicates that 2000 samples have been acquired and ready to process. The samples are accessed using a pointer p1 to the buffer. All of this is no problem and works perfectly if the arithmetic R_AVG of the buffer is computed following the completion of the for loop that performs the summation of the samples. I have verified that the summation of the 2000 samples is being executed properly.
The PROBLEM is that unless there is Serial.print() statement in the for loop, the AVG is not computed properly afterwards and is set to =0. This terminates sensor regulation because an AVG value less than 400 typically indicates an open sensor or a power switch failure.
I'm attaching only the code for Loop() as the rest of the program is of no consequence as to why the presence of a print statement is required for computation of the average.
Code:
```cpp
void loop() {
  volatile uint32_t dwt_L = 0;
  uint32_t deltacount_L = 0;
  volatile uint32_t dwtlast_L = 0;
  unsigned int j = 0;
  noInterrupts();  //disable interrupts to stop a/d sampling
  dwt_L = ARM_DWT_CYCCNT;

  if (error) Serial.println("99999");  // if tried to start A/D conversion while busy
  p1 = value1;                         // reset pointer to a/d buffer1
  L_AVGSUM = 0.0;                      // clear sum
  if (bufferdone) {
    for (j = 0; j < 1999; j++) {
      p1++;
      L_AVGSUM += *p1;
      Serial.println();  // REQUIRE A Serial.print STATEMENT HERE or R_AVG will not
      //  be computed after the for loop  and will = 0. This will stop the regulation
      // as a result of detecting an AVG value < OpenProbeValue=400
      // DONT KNOW WHY ??????
      sprintf(string1, "%u", *(p1));  // convert buffer value to string
      strcat(string1, string2);       //add a comma delimiter
                                      // dwt = ARM_DWT_CYCCNT;

      // dwtlast = ARM_DWT_CYCCNT;
      string1_len = strlen(string1);
      // send string1, to the IP address and port of PC
      Udp.beginPacket(ipremote, localPort);
      Udp.write(string1, string1_len);  // send ASCII cr delimited string of A/D sample from buffer
      Udp.endPacket();
    }
    dwtlast_L = ARM_DWT_CYCCNT;
    R_AVG = (unsigned int)(L_AVGSUM / 2000);  // compute avg here
    Serial.print(" AVG= ");
    Serial.println(R_AVG);
    //dwtlast_L = ARM_DWT_CYCCNT;
    deltacount_L = (dwtlast_L - dwt_L);
    //Serial.println(deltacount_L);
  }
  p1 = value1;         //reset pointer to a/d buffer
  bufferdone = false;  //clear bufferdone flag

  interrupts();  //reenable interrupts for A/D service

  Regulate();
}
```
 
Last edited:
If you replace the println with a short delay, does the problem still occur? Perhaps you are just overflowing some buffer in the Udp stack
 
You need to post at least enough code so that it can be compiled. We can't see how most of the variables are declared or know what their types are, we can't see where/how bufferdone is being set, we don't know how the value1 array is being filled...
My best guess is that bufferdone is being set before the array is properly filled, resulting in an average of less than 400. The Serial.println() statement merely causes a delay that allows enough samples to be collected to raise the average.
 
If you replace the println with a short delay, does the problem still occur? Perhaps you are just overflowing some buffer in the Udp stack
A delay doesnt change anything. I tried this. Not overflowing the UDP stack. The UDP data is correct and intact as received as evidenced by the Wireshark UDP stream received when the println statement is present in the for loop.
You need to post at least enough code so that it can be compiled. We can't see how most of the variables are declared or know what their types are, we can't see where/how bufferdone is being set, we don't know how the value1 array is being filled...
My best guess is bufferdone is being set before the array is properly filled, resulting in an average of less than 400. The Serial.println() statement merely causes a delay that allows enough samples to be collected to raise the average.
There are a number of reasons I cannot post a lot of the code for reasons of project confidentiality. The variable R_AVG is a global volatile unsigned integer. L_AVGSUM is a global volatile float. bufferdone is a global boolean and is set true in the A/D interrupt service routine which keeps track of the number of samples placed in the array. The array is being filled by the A/D interrupt service function. As conversions are read they are placed in the array referenced by a pointer which is incremented as required until the array is filled. Your "best guess" is incorrect, i.e. the buffer IS filled with the correct values and when the print statement is present, the summation of the buffer values and the computed average ARE BOTH correct. I have verified the numerical value of the computed average by measurement on an Oscope. See the prior question by Jmarsh re introduction of a delay which has no effect.
Should also mention I am using rawHID for the USB serial activity.
 
Last edited:
And what is value1? How do we know if it's statically or dynamically allocated, which region of memory it exists in and if it's cacheable? How can we know it's not being overwritten by some code overruning a buffer stored next to it? How do we know bufferdone is being set at the correct time? How do you know the buffer is being filled correctly when the sum appears to be zero?

Removing the Serial.println() should have no effect so there is obviously some undefined behaviour happening (possibly cache related...) and it's not possible to figure out where without seeing the full picture. There's also several things I would consider "wrong" in the posted code such as calling Serial.print and Udp functions with interrupts disabled, and summing only 1999 values but dividing by 2000 to calculate the average.
 
Last edited:
And what is value1? How do we know if it's statically or dynamically allocated, which region of memory it exists in and if it's cacheable? How can we know it's not being overwritten by some code overruning a buffer stored next to it? How do we know bufferdone is being set at the correct time? How do you know the buffer is being filled correctly when the sum appears to be zero?

Removing the Serial.println() should have no effect so there is obviously some undefined behaviour happening (possibly cache related...) and it's not possible to figure out where without seeing the full picture.
volatile unsigned int value1[2000]; // buffer for A/D values
volatile unsigned int *p1 = value1; // pointer to A/D sample buffer1

I dont know how it is being allocated or how to find out. this is the allocation info from the compilation if this helps:
Memory Usage on Teensy 4.1:
FLASH: code:263096, data:73880, headers:8592 free for files:7780896
RAM1: variables:35012, code:94264, padding:4040 free for local variables:390972
RAM2: variables:1664 free for malloc/new:522624
there are no errors or warnings returned in verbose compilation output.

bufferdone is set by the interrupt service routine when 2000 samples have been converted and written to value1.
The sum appears to be zero in the absence of the println statement because the computed R_AVG is 0 ,but I can see the accumulated non-zero sum increasing in the for loop if I print L_AVGSUM instead of a blank Serial.println() statement. If I do this, R_AVG prints the correct value after executing the for loop and the regulation function operates correctly as does the UDP transmission.
You are correct, There is obviously some undefined behavior occurring otherwise this would not be a problem.
 
The println() has no expected affect - except blasting out 'CR+NL' 1999 times. Except noInterrupts() won't let that happen? So, if not that, then something unshown/unseen is causing trouble. 1Msamp/sec means this is happening 500 times/sec?

What about something like this to limit the print and maybe stop compiler from helping?
Code:
if ( 0 == (j%1000) )
      Serial.println();  // REQUIRE A Serial.print STATEMENT HERE or R_AVG will not

This skips the first value? If first value went into L_AVGSUM instead of 0.0- it would make sense to p1++ first, and also the for(;J<1999;) instead of "<=" - but that first value will also never make it to UDP.
1733559096921.png
 
I have not read in detail what you are doing, but noticed that you said that you dis-abled interrupts in loop. This will stop serial print working. All serial stuff uses interrupts. I would suggest it would be better to have a flag variable which your interrupt routine inspects and only do it's stuff when needed. As I said I did not have a detail read so if this is a load of rubbish I apologise.
 
I have not read in detail what you are doing, but noticed that you said that you dis-abled interrupts in loop. This will stop serial print working. All serial stuff uses interrupts. I would suggest it would be better to have a flag variable which your interrupt routine inspects and only do it's stuff when needed. As I said I did not have a detail read so if this is a load of rubbish I apologise.
Interesting idea. Despite your suggestion, the Serial.print in loop does work to print . The problem I have is that in the absence of that print statement in the for (j=0;j<1999;j++) loop, computation of the average value after completion of the iteration doesnt give a correct result.
The println() has no expected affect - except blasting out 'CR+NL' 1999 times. Except noInterrupts() won't let that happen? So, if not that, then something unshown/unseen is causing trouble. 1Msamp/sec means this is happening 500 times/sec?

What about something like this to limit the print and maybe stop compiler from helping?
Code:
if ( 0 == (j%1000) )
      Serial.println();  // REQUIRE A Serial.print STATEMENT HERE or R_AVG will not

This skips the first value? If first value went into L_AVGSUM instead of 0.0- it would make sense to p1++ first, and also the for(;J<1999;) instead of "<=" - but that first value will also never make it to UDP.
View attachment 36489

Defragster,
The blank print statement does in fact blast out nothing 1999 times followed by the correct value of R_AVG. I too thought that no interrupts would prevent that but the print statement does output to the serial monitor. Will try your code suggestion to limit the print and see what happens. Thanks for the suggestion. Not concerned about first value getting to UDP.
 
Last edited:
Why are you turning off interrupts? If bufferdone is a flag that tells you that you have a buffer full of A/D values, you can process them just as well without turning off interrupts. When you're done with that processing you can start a new set of samples or you can try to interleave them if you need continuous ssampling, but it's just very bad practice to turn off interrupts for such an extended period of time. Without seeing your code, noone can know the consequences this might have.
 
Presumably, somewhere in the Serial.println code, it turns interrupts back on which allows the code to proceed. If it isn't called, interrupts stay off and the print buffer fills up and then println hangs.

[edit] I suspect that if instead there was just one println immediately before the for loop, it would still "fix" the problem.

Pete
 
Why are you turning off interrupts? If bufferdone is a flag that tells you that you have a buffer full of A/D values, you can process them just as well without turning off interrupts. When you're done with that processing you can start a new set of samples or you can try to interleave them if you need continuous ssampling, but it's just very bad practice to turn off interrupts for such an extended period of time. Without seeing your code, noone can know the consequences this might have.
Hi Joe,
I thought if I dont turn off interrupts, the A/D interrupt service routine called by a Timer at 1.0 MHz will overwrite the existing buffer of samples while I'm processing them and change the pointer p1 in doing so. This would upset the for loop indexing to the sample array.
 
Presumably, somewhere in the Serial.println code, it turns interrupts back on which allows the code to proceed. If it isn't called, interrupts stay off and the print buffer fills up and then println hangs.

[edit] I suspect that if instead there was just one println immediately before the for loop, it would still "fix" the problem.

Pete
Hi Pete,
Tried one println before the for loop, doesnt "fix" problem. I suspect there is something else going on.
 
I thought if I dont turn off interrupts, the A/D interrupt service routine called by a Timer at 1.0 MHz will overwrite the existing buffer of samples while I'm processing them and change the pointer p1 in doing so. This would upset the for loop indexing to the sample array.
Rather than turn interrupts on and off, define variables that provide communication between the interrupt-level and task(loop) code. For example, when the array of ADC samples is full, the interrupt routine could set bufferfull = true, and it could stop adding to the buffer as long as bufferfull == true. loop() could test for bufferfull == true, and when that occurs, it could process the existing buffer and then set bufferfull = false. That would re-enable the interrupt processing and the cycle would repeat. There are lots of other ways to do it, too, such as writing to a queue from the interrupt routine and reading from the queue in loop(), but that is trickier, so start with the simple flag interlock.
 
Rather than turn interrupts on and off, define variables that provide communication between the interrupt-level and task(loop) code. For example, when the array of ADC samples is full, the interrupt routine could set bufferfull = true, and it could stop adding to the buffer as long as bufferfull == true. loop() could test for bufferfull == true, and when that occurs, it could process the existing buffer and then set bufferfull = false. That would re-enable the interrupt processing and the cycle would repeat. There are lots of other ways to do it, too, such as writing to a queue from the interrupt routine and reading from the queue in loop(), but that is trickier, so start with the simple flag interlock.
Joe, That is what I do in the interrupt service. I set bufferdone=true. Loop checks to see if bufferdone= true and processes the buffer.When loop completes it sets bufferdone=false. I think the difference between what you suggest and what I am doing is that setting bufferdone=true in the interrupt service doesnt in itself stop further sampling by any means such as stopping the Timer that calls the service. That is why I turned off interrupts in Loop. Will try your suggestion by modifying the interrupt service. Good suggestion. Thanks very much for that.
 
USB (Serial) communication uses (requires?) interrupts, and I’m guessing that it’s not okay to turn them and then do a ton of Serial printing.
 
You've disabled interrupts, but calling some of these functions like Serial.print() might be re-enabling interrupts again. And if they're not, you have a serious risk of a deadlock condition.

I know this doesn't answer your original question (and doing so would take an enormous investigation if we had all the code and hardware to reproduce the problem), but the only answer I can give is to recommend you restructure your code to not call anything while interrupts are disabled. Only copy variables into a local buffer, then turn interrupts back on again before you call any functions, and especially any functions like Serial or Ethernet which might use interrupts. When you do call functions, only give them the copied data, never access to the stuff the interrupt might change. I know that's a lot of work to re-do things so much, but without that sort of approach you're probably never going to get fully consistent results.
 
Another thing to consider, and I'll admit this is a completely blind guess, is the possibility of a buffer overflow. In other words, R_AVG might be written with the correct data but then gets overwritten with zero by some other unrelated code before the rest of your program reads it and acts on the too-low value. Adding Serial.print() might cause the compiler to restructure the order of variables in memory, such that this hypothetical write of zero corrupts something else, leaving R_AVG unaffected. We've seen these sorts of buffer overflow problems many times on this forum and they're usually very tricky to diagnose because even subtle code changes can cause the compiler to generate quite different code. Especially common is removal of function calls lets the compiler keep more local variables in the CPU registers.

A couple things to try are using or not using volatile and for global or static variables, try using DMAMEM to allocated the variable in RAM2 rather than RAM1.
 
the A/D interrupt service routine called by a Timer at 1.0 MHz will overwrite the existing buffer of samples while I'm processing them and change the pointer p1 in doing so. This would upset the for loop indexing to the sample array.
Now hang on a moment... p1 is also a global variable and the ISR is using it to track the current write position? How does that work when loop() is constantly resetting it to the beginning, regardless of whether bufferdone is set or not?

This would definitely explain how you're getting a zero average - instead of filling the array the ISR ends up constantly overwriting the first element, (because loop() is resetting p1 after every update) which then gets skipped because of the bug in your summing algorithm.
 
Another thing to consider, and I'll admit this is a completely blind guess, is the possibility of a buffer overflow. In other words, R_AVG might be written with the correct data but then gets overthans for weighnwritten with zero by some other unrelated code before the rest of your program reads it and acts on the too-low value. Adding Serial.print() might cause the compiler to restructure the order of variables in memory, such that this hypothetical write of zero corrupts something else, leaving R_AVG unaffected. We've seen these sorts of buffer overflow problems many times on this forum and they're usually very tricky to diagnose because even subtle code changes can cause the compiler to generate quite different code. Especially common is removal of function calls lets the compiler keep more local variables in the CPU registers.

A couple things to try are using or not using volatile and for global or static variables, try using DMAMEM to allocated the variable in RAM2 rather than RAM1.
Paul, thanks for weighing in on this. I will try to restructure some code per your suggestions.
Now hang on a moment... p1 is also a global variable and the ISR is using it to track the current write position? How does that work when loop() is constantly resetting it to the beginning, regardless of whether bufferdone is set or not?

This would definitely explain how you're getting a zero average - instead of filling the array the ISR ends up constantly overwriting the first element, (because loop() is resetting p1 after every update) which then gets skipped because of the bug in your summing algorithm.
Good point. I caught that possibility and have insured that p1 is reinitialized only in the interrupt service when bufferdone is set true. Have also modified interrupt service to take samples and write them to value1 While bufferdone is false. When the 2000 samples have been written, bufferdone is set true, and p1=value1.
You've disabled interrupts, but calling some of these functions like Serial.print() might be re-enabling interrupts again. And if they're not, you have a serious risk of a deadlock condition.

I know this doesn't answer your original question (and doing so would take an enormous investigation if we had all the code and hardware to reproduce the problem), but the only answer I can give is to recommend you restructure your code to not call anything while interrupts are disabled. Only copy variables into a local buffer, then turn interrupts back on again before you call any functions, and especially any functions like Serial or Ethernet which might use interrupts. When you do call functions, only give them the copied data, never access to the stuff the interrupt might change. I know that's a lot of work to re-do things so much, but without that sort of approach you're probably never going to get fully consistent results.
Lots to consider here but will see what I can do to restructure code per your guidelines. Many thanks for your input
 
Looks like I have a bit of work to do. Thanks to all of you for your suggestions. When I get this thing working, I'll update everyone on what I've got.
 
I would recommend looking at standard ring buffers for passing data to and from an ISR - this is how its normally done - using ring buffer an a queue which the producer adds entries to and the consumer takes entries from. Using separate insert and extract pointers allows the event-counter approach to synchronization which is low overhead (no semaphores/locks needed). This resource might be useful: https://www.downtowndougbrown.com/2013/01/microcontrollers-interrupt-safe-ring-buffers/
 
I would recommend looking at standard ring buffers for passing data to and from an ISR - this is how its normally done - using ring buffer an a queue which the producer adds entries to and the consumer takes entries from. Using separate insert and extract pointers allows the event-counter approach to synchronization which is low overhead (no semaphores/locks needed). This resource might be useful: https://www.downtowndougbrown.com/2013/01/microcontrollers-interrupt-safe-ring-buffers/
Thanks MarkT will consider this. I think my problem has to do with accessing the same pointer in both the interrupt service and the main loop thus leading to a conflict.
 
Back
Top