Arduino my void loop stops after 1 cycle

Using Arduino Programming Questions
1

Hi, I've recently started programming Arduino sketches.
I'm tring to make a controller of temperature, read current consumption of a pc and a monitor, moreover I need to read the rpm of two set of fans.
I only knew how to acquire temperature and use that information to control the fans, for read the current value I've copied an already written program that use a specific library to run; the program is called Energy Monitor and it's based on the Emonlib, at last for read the fan rpm I've copied a program that incorporates the attach interrupts.
Now when i run the program, on the serial monitor are printed all the values that i need but after the first cycle it doesn't print anything.
I'm asking for some help to understad why it stops, i post the code below, if you have some suggestion about how to acquire the current consumption without using a library based program I'll be grateful (I'm using a LEM HXS-20NP hall effect sensor).
I'm thinking that the Energy monitor program generates some problems in the regular flow of the program.

Here there is the link from which I copied the Energy monitor program:
https://openenergymonitor.org/emon/buildingblocks/how-to-build-an-arduino-energy-monitor

I've uploaded the library files of the Energy Monitor program that i used on dropbox:

Here it starts the code:

#include "EmonLib.h"
EnergyMonitor emon1;
EnergyMonitor emon2;

const byte errorPin=13;
int ris=6, ven=10, ven2=11, door=8,ledDoor=12;
word val;
float temperatura, temp;
int NbTopsFan,NbTopsFan1,calc,calc1, hallsensor=2,sta;
byte libero=5, libero1=9; //unused pins that i'll need afterwards
volatile byte state=LOW;

typedef struct{ //Defines the structure for multiple fans and their dividers

char fantype;
unsigned int fandiv;
}fanspec; //Definitions of the fans
fanspec fanspace[3]={{0,1},{1,2},{2,8}};
char fan=1;

void activate()
{
NbTopsFan++;
}

void activate1()
{
NbTopsFan1++;
}

void setup() {
Serial.begin(9600);

pinMode(errorPin,OUTPUT);
pinMode(hallsensor,INPUT);
pinMode(3,INPUT);
pinMode(door,INPUT);
pinMode(ledDoor,OUTPUT);
pinMode(ris,OUTPUT);
pinMode(ven,OUTPUT);
attachInterrupt(0,activate,RISING);
attachInterrupt(1,activate1,RISING);

emon1.voltage(A2,234.26,1.7);
emon1.current(A1,111.1);

emon2.voltage(A4,234.26,1.7);
emon2.current(A3,111.1);

}

void loop() {

Serial.flush();

emon1.calcVI(20,2000);
Serial.println("PC_Sensor:");
emon1.serialprint();
emon1.statusPC();

emon2.calcVI(20,2000);
Serial.println("Monitor_Sensor:");
emon2.serialprint();
emon2.statusMonitor();

val=analogRead(A0);
temperatura=(float)val/1023*5;
temp=(float)temperatura/0.01-50;

Serial.print("temperatura: ");
Serial.print(temp);
Serial.println("^C");

NbTopsFan = 0;

sei(); //Enables interrupts
delay (1000);

cli(); //Disable interrupts
calc = ((NbTopsFan * 60)/fanspace[fan].fandiv);

//Times NbTopsFan (which is apprioxiamately the fequency the fan is spinning at) by 60 seconds //before dividing by the fan's divider

Serial.print (" set ventole 1: ");
Serial.print (calc, DEC);
Serial.print (" rpm\r\n");

NbTopsFan1 = 0;

sei(); //Enables interrupts
delay (1000);

cli(); //Disable interrupts
calc1 = ((NbTopsFan1 * 60)/fanspace[fan].fandiv);

Serial.print (" set ventole 2: ");
Serial.print (calc1, DEC);
Serial.print (" rpm\r\n");

sta=digitalRead(door);
digitalWrite(ledDoor,sta);

digitalWrite(errorPin,state);

if(val<143){
digitalWrite(ris,HIGH); //controllo temperatura interna
digitalWrite(ven,LOW);
digitalWrite(ven2,LOW);} //superata la soglia avvia le ventole
if(143<=val<=174){
digitalWrite(ris,LOW);
digitalWrite(ven,LOW);
digitalWrite(ven2,LOW);}
if(val>174){
digitalWrite(ris,LOW);
digitalWrite(ven,HIGH);
digitalWrite(ven2,HIGH);}

if(digitalRead(ven)==HIGH && NbTopsFan==0){ //controlla se supera la temperatura
digitalWrite(errorPin,HIGH); //e se le ventole entrano in funzione
Serial.println("errore: il set di ventole 1 non sta girando");} //in caso non funzionasse segnala l'errore
if(digitalRead(ven)==HIGH&&NbTopsFan!=0){
digitalWrite(errorPin,LOW);}

if(digitalRead(ven)==HIGH && NbTopsFan1==0){

digitalWrite(errorPin,HIGH);

Serial.println("errore: il set di ventole 2 non sta girando");}

if(digitalRead(ven)==HIGH&&NbTopsFan1!=0){
digitalWrite(errorPin,LOW);}

}

Thank you for any suggestion.

2

Probably because you keep calling Serial.prinnt with interrupts disabled. Turning interrupts on an off like that is just wrong. If you think you need to do that, then you need to re-think your design.

Regards,
Ray L.

3

Also, what are you trying to do here:

  if (143 <= val <= 174) {

If val is less than or equal to 143, clearly it's also less than 174.

Also, with the cursor in the IDE's source code window, use Ctrl-T to reformat your code into a common C style before posting your code.

4

econjack:
Also, what are you trying to do here:

  if (143 <= val <= 174) {

If val is less than or equal to 143, clearly it's also less than 174.

Also, with the cursor in the IDE's source code window, use Ctrl-T to reformat your code into a common C style before posting your code.

Never mind the fact that the expression will NOT do what he expects, since one of the tests (and I don't recall which one) will reduce to a compare against either 0 or 1....

Regards,
Ray L.

5

RayLivingston:
Never mind the fact that the expression will NOT do what he expects...

@Ray: So, are you saying I shouldn't have called it to his attention? That makes no sense to me.

6

econjack:
@Ray: So, are you saying I shouldn't have called it to his attention? That makes no sense to me.

No. It seems to me that what the OP wanted was:

if (143 <= val && val <= 174)

If that is the case, then your comment is misleading. val could be 100, 150, or 200. If it is 100, it is less than 143, so the whole statement is false.

If it is 200, it is greater than 174, so the whole statement is false.

If it is 150, it is greater than 143, so that part is true. It is less than 174, so that part is true. Since both parts are true, the statement evaluates to true.

7

PaulS:
If it is 150, it is greater than 143, so that part is true. It is less than 174, so that part is true. Since both parts are true, the statement evaluates to true.

If it's 150 and the test is val <= 143, how is that logic true?

8

econjack:
and the test is val <= 143

But nobody wrote that...

9

I guess I was more hung up on the way the expression was written than what it said. I read it in my mind in the more conventional way. My bad...

10

econjack:
@Ray: So, are you saying I shouldn't have called it to his attention? That makes no sense to me.

No, what I'm saying is, even if he took your advise, it STILL would not work as he appears to have intended.

if (143 <= val <= 174)

Suppose val = 200. 200 is >= 143, and also >= 173. So, the OP would want that value to fail the test, and NOT execute the if. But, that expression will be evaluated left-to-right, which means it will first be reduced to:

if (1 <= 174)

This is because the expression (143 <= 200) will be evaluated first, and return true, which is defined as '1'. To the whole expression will resolve to true, which is NOT what was wanted.

It must be broken down into two separate expressions to evaluate correctly:

if ((val >= 143) && (val <= 174))

Regards,
Ray L.

Regards,
Ray L.

11

@Ray: Read #8 which I posted before your #9.

12

The current version of the arduino core should be smart enough to work with serial print statements with interrupts disabled. The comment in the implementation of write() for hardware serial explains it:

size_t HardwareSerial::write(uint8_t c)
{
  _written = true;
  // If the buffer and the data register is empty, just write the byte
  // to the data register and be done. This shortcut helps
  // significantly improve the effective datarate at high (>
  // 500kbit/s) bitrates, where interrupt overhead becomes a slowdown.
  if (_tx_buffer_head == _tx_buffer_tail && bit_is_set(*_ucsra, UDRE0)) {
    *_udr = c;
    sbi(*_ucsra, TXC0);
    return 1;
  }
  tx_buffer_index_t i = (_tx_buffer_head + 1) % SERIAL_TX_BUFFER_SIZE;
	
  // If the output buffer is full, there's nothing for it other than to 
  // wait for the interrupt handler to empty it a bit
  while (i == _tx_buffer_tail) {
    if (bit_is_clear(SREG, SREG_I)) {
      // Interrupts are disabled, so we'll have to poll the data
      // register empty flag ourselves. If it is set, pretend an
      // interrupt has happened and call the handler to free up
      // space for us.
      if(bit_is_set(*_ucsra, UDRE0))
	_tx_udr_empty_irq();
    } else {
      // nop, the interrupt handler will free up space for us
    }
  }

  _tx_buffer[_tx_buffer_head] = c;
  _tx_buffer_head = i;
	
  sbi(*_ucsrb, UDRIE0);
  
  return 1;
}

Not that I condone turning off interrupts the way they do in there - I'm sure it destroys timekeeping, particularly with all those serial prints....

13

thank you for notice me that, i wouldn't know the right syntax to write it , i will write it like this (if(val>=143)&&(val<=174)).

14

Liandranth:
thank you for notice me that, i wouldn't know the right syntax to write it , i will write it like this (if(val>=143)&&(val<=174)).

No, do yourself a favour and get some white space in there.

if ((val >= 143) && (val <= 174))

// to make what you're doing more obvious

if ((143 <= val) && (val <= 174))

I fixed the parenthesis as well.

15

I made some changes at the code because I've found a program that reads the Irms from a current sensor, however the interrupts still interfere with the "Serial.print" but I need them in order to acquire the fans rpm.

now my program looks like this:

const byte errorPin=13;
const int sensorIn = A1;
int mVperAmp = 66; // use 100 for 20A Module and 185 for 5 Module
int ris=6, ven=10, ven2=11, door=8,ledDoor=12;
word val;
double Voltage = 0,VRMS = 0,AmpsRMS = 0;
float temperatura, temp;
int NbTopsFan,NbTopsFan1,calc,calc1,sta;
int hallsensor_fan=2,hallsensor_fan1=3;

typedef struct{             //Defines the structure for multiple fans and their dividers     
char fantype;
unsigned int fandiv;}
fanspec;                    //Definitions of the fans
fanspec fanspace[3]={{0,1},{1,2},{2,8}};
char fan=1;                 //This is the varible used to select the fan and it's divider, 
                            //set 1 for unipole hall effect sensor and 2 for bipole hall effect sensor 

void setup() {
 Serial.begin(9600);
 pinMode(errorPin,OUTPUT);
 pinMode(hallsensor_fan,INPUT);
 pinMode(hallsensor_fan1,INPUT);
 pinMode(door,INPUT);
 pinMode(ledDoor,OUTPUT);
 pinMode(ris,OUTPUT);
 pinMode(ven,OUTPUT);
 pinMode(ven2,OUTPUT);
 attachInterrupt(0,activate,RISING);
 attachInterrupt(1,activate1,RISING);
}

void activate(){NbTopsFan++;}
void activate1(){NbTopsFan1++;}

void loop() {
val=analogRead(A0);
temperatura=(float)val/1023*5;
temp=(float)temperatura/0.01-50;

sta=digitalRead(door);
digitalWrite(ledDoor,sta);

NbTopsFan = 0;  
   sei();   //Enables interrupts
   delay (1000);  
   cli();   //Disable interrupts
   calc = ((NbTopsFan * 60)/fanspace[fan].fandiv);

NbTopsFan1 = 0;  
   sei();   //Enables interrupts
   delay (1000);  
   cli();   //Disable interrupts
   calc1 = ((NbTopsFan1 * 60)/fanspace[fan].fandiv);

Voltage = getVPP();
VRMS = (Voltage/2.0) *0.707;
AmpsRMS = (VRMS * 1000)/mVperAmp;

Serial.print("temperatura: ");
Serial.print(temp);
Serial.println("^C"); 
Serial.print (" set ventole 1: ");
Serial.print (calc, DEC);
Serial.print (" rpm\r\n");
Serial.print (" set ventole 2: ");
Serial.print (calc1, DEC);
Serial.print (" rpm\r\n");
Serial.print(AmpsRMS);
Serial.println(" Amps RMS");
Serial.print(VRMS);
Serial.println(" V RMS");

if(val<117){
    digitalWrite(ris,HIGH);                        //controllo temperatura interna
    digitalWrite(ven,LOW);
    digitalWrite(ven2,LOW);}                        //superata la soglia avvia le ventole
if ((val >= 117) && (val <= 174)){
    digitalWrite(ris,LOW);
    digitalWrite(ven,LOW);
    digitalWrite(ven2,LOW);}
if(val>174){
    digitalWrite(ris,LOW);
    digitalWrite(ven,HIGH);
    digitalWrite(ven2,HIGH);}
  
if(digitalRead(ven)==HIGH && calc==0){                           //controlla se supera la temperatura
    digitalWrite(errorPin,HIGH);                                         //e se le ventole entrano in funzione
    Serial.println("errore: il set di ventole 1 non sta girando");}     //in caso non funzionasse segnala l'errore
if(digitalRead(ven)==HIGH && calc!=0){
      digitalWrite(errorPin,LOW);}
   
if(digitalRead(ven2)==HIGH && calc1==0){                           
    digitalWrite(errorPin,HIGH);                                         
    Serial.println("errore: il set di ventole 2 non sta girando");}     
if(digitalRead(ven2)==HIGH && calc1!=0){
      digitalWrite(errorPin,LOW);}

delay(350);
}

float getVPP()
{
float result;
int readValue; //value read from the sensor
int maxValue = 0; // store max value here
int minValue = 1023; // store min value here
uint32_t start_time = millis();
while((millis()-start_time) < 1000) //sample for 1 Sec
{
readValue = analogRead(sensorIn);
// see if you have a new maxValue
if (readValue > maxValue)
{
/*record the maximum sensor value*/
maxValue = readValue;
}
if (readValue < minValue)
{
/*record the maximum sensor value*/
minValue = readValue;
}
}
// Subtract min from max
result = ((maxValue - minValue) * 5.0)/1023.0;
return result;
}
16

however the interrupts still interfere with the "Serial.print" but I need them in order to acquire the fans rpm.

Nonsense. That method of turning interrupts off except during that delay() is the worst possible way to deal with interrupts.

Get rid of those three lines. NOW!

Then, look at the blink without delay example. See how it manages to make something happen (like calculating how many interrupts happened in a given time frame) periodically.

On every pass through loop(), see if it is time to calculate RPM. If not, move on to the rest of the code. If it is, make a copy of NbTopsFan and/or NbTopsFan1 and then reset them (inside a critical block) and then compute RPM based on the values in the copies and the time since you last computed RPM.

A critical block is one in which interrupts are disabled.

cli(); // disable interrupts while making the copy
copyNbTopsFan = NbTopsFan;
sei(); // enable them again.

Whoever published that crappy example you started from should be horsewhipped.

17

I made it, i've just moved the sei() function at the top of the void loop and the cli() at the bottom of the void loop and now the program works without problems, the serial monitor doens't stop printing anymore and I can read all the values.

18

Liandranth:
I made it, i've just moved the sei() function at the top of the void loop and the cli() at the bottom of the void loop and now the program works without problems, the serial monitor doens't stop printing anymore and I can read all the values.

Which is completely pointless... DELETE the sei() and cli() entirely. They are NOT needed.

Regards,
Ray L.

19

Done. Thank you all for your suggestions, and I'm sorry for being so ignorant about programming, this is my first arduino project and now it works. I'm so happy about this.

20