I programmed one timer and two IRQ's on my Nano.
IRQ1 fires unwanted every time at 50 ms.
Question: Can IRQ0, IRQ1 and Timer1 be used together?
Within the Setup part:
//Prepairing D2 (pin5) for update display (IRQ0)
pinMode(2, INPUT); //SQW signal from RTC (1 Hz)
pinMode(2, INPUT_PULLUP); //Pull up resistance
attachInterrupt(digitalPinToInterrupt(2), SecondPuls, FALLING); //Interupt 0 is coupled to D2 (pin 5)
//Prepairing D3 (pin6) as input external limit switch (IRQ1)
pinMode(3, INPUT); //external limit switch to GND
pinMode(3, INPUT_PULLUP); //Pull up resistance
attachInterrupt(digitalPinToInterrupt(3), Limit, RISING); //Interrupt 1 is coupled to D3 (pin 6)
//set timer1 interrupt at 20 Hz timetick (used to update display) zie https://forum.arduino.cc/index.php?topic=575956.0
//runs ISR(TIMER1_COMPA_vect)
noInterrupts(); //disable all interrupts
TCCR1A = 0; // set entire TCCR1A register to 0
TCCR1B = 0; // same for TCCR1B
OCR1A = 12499; // 20 Hz
TCCR1B |= (1 << WGM12); // turn on CTC mode:
TCCR1B |= (1 << CS10); // Set CS10 and CS11 bits for 64 prescaler:
TCCR1B |= (1 << CS11);
TIMSK1 |= (1 << OCIE1A); // enable timer compare interrupt
interrupts(); //enable all interrupts
The routine for IRQ1 is specifies as follows:
//Interrupt request for limit switch S6 on pin D3 (IRQ1)
void Limit(){
blnOnTop= true;
//Serial.println("Limitswitch");
} //End routine
The part where I can see that it goes wrong (row with if (blnOnTop==true):.......):
//Striking the hammer a number of pings
if (blnBellCycle== true){
if (i <= numberPings){
//Serial.print(cnt1);tab();Serial.println(numberPings);
if (cnt1!=cnt1_previous){
if (cnt1 == 1) //Action on the first count
{previousMillis = millis();
Serial.println (String("Slag ") + i); tab();
digitalWrite(Bellpin, HIGH);
}
if (cnt1>=(cnt1_max+1) or blnOnTop==true) //Stop the puls (regular or forced), see row 683
{
//Serial.println (String("Rij 535, blnOnTop ") + blnOnTop + "cnt1 = " + cnt1 + "cnt1_max = " + cnt1_max);
cnt1 =0;
//Show pulsduration on serial monitor
if (blnOnTop==true){Serial.print("Onderbroken op ");}; Serial.println (millis() - previousMillis);
//Show pulsduration on LCD
lcd.setCursor( ((i-1)%4)*5, 1+ int((i-1)/4) );
lcd.print(millis() - previousMillis);
blnOnTop = false;
i++;} //reset counter
cnt1_previous = cnt1;
} //End if (cnt1!=cnt1_previous)
} //Stop when number of strikes has reached maximum
else
{blnBellCycle = false;digitalWrite(Bellpin, LOW); cnt1=1; i=1; delay(10000); blnIntro=true;} //End if
} //End if blnBellCycle //End striking hammer
Looks like a LOT of state variables. I see: i, numberPings, cnt1, cnt1_previous, cnt1_max, blnOnTop, previousMillis, blnBellCycle, blnIntro. I think the code would be MUCH easier to understand if you wrote it as a finite state machine.
Is that 'delay(10000);' really necessary?
This bit looks a little confusing:
//Show pulsduration on serial monitor
if (blnOnTop==true){Serial.print("Onderbroken op ");}; Serial.println (millis() - previousMillis);
Spaced out more normally:
//Show pulsduration on serial monitor
if (blnOnTop == true)
{
Serial.print("Onderbroken op ");
}
;
Serial.println (millis() - previousMillis);
So you have one line where the first statement is conditional and the second isn't. So you show the time difference, even when you don't label it. And you have an extra null statement ( between them.
I agree that I use al lot of variables. A lot of programming concerns a menu with the 5 options, needed for people to operate the future church bell control.
I know it not decently to use a delay of 10.000 ms but in this case I keep the results of the time measurements for a while on my LCD (to write over or to keep a photo).
Now I am struggling within the rows 532..560. Menu 4 (Service position) enables to do some experiments with a number of beats of the bell. I give pulses with a fixed duration (adjustable between 10 and 80 counts of 50 ms = 0.5 upto 4 seconds).
Each pulse has to be shortened by the IRQ2 (limit switch); I make it now about 2400 ms but it has to shortened (depending on external factors) to about 2250 ms.
Encloses my complete code and a photo of the display (the first beat is shortened without activating the limit switch).
OK.
Variables which are set in an ISR, and therefore could be updated asynchronously from the main program flow, must be declared as volatile to prevent unwanted compiler optimisation keeping stale copies of them. This applies also to blnOnTop
Your loop() is pretty massive. It would be much better to write separate functions for each of your case statements so loop() is much shorter and then each of those individual functions handles 1 item, like menu 1 or menu 2, etc.
Even better than that would be to make it a state machine.
Those variables in your ISRs need to be 'volatile' so the compiler does the correct thing.
There is no potential conflict, that I can see, between Timer1 and the two external interrupts, which was your first question.
If that highlighted code section is behaving as if blnOnTop is prematurely set to true, then first suspect the missing volatile storage specifier.
The other general problem you'll have is that you are making extensive use of the String class for the display and this can lead to mysterious behaviour because the heap storage can become fragmented over time resulting in memory allocation failures. There are ways to limit this risk, such as using the reserve() method, but if you don't eliminate them they will always be a focal point if you request help.
You could make the code easier to maintain by by breaking it down into separate functions, as has already been suggested, each maintaining their own state variables, with only a minimum of shared variables.
I'd probably break it down like this, keeping the functions as autonomous as possible
NormalBellCycle()
TestBellHandling()
Configuration() (menu setting of EEPROM parameters)
BatteryMonitoring()
Clearly, there will be some shared states, for example if you are in BellTestHandling(), you want to stop the NormalBellCycle() being active. Also, the display must be shared somehow.
In the main loop() you'd simply call each function and that function would simply act as required and return.
Just out of curiosity: It looks like the code handles only the bell ringing functions. How is this synchronised with the hands of this tower clock and how is daylight saving time handled ?
IRQ1 = hardware interrupt of limit switch as I read.
Have you tried connecting the input to the ground to make sure you do not EMC static picked up by the IO?
50Hz interference (but that would be 20ms or a multiple of it).
I found a practical solution for my problem.
The first 500 ms of a cyclus (= 10 counts of cnt1) I neglect chancing of the parameter blnOnTop (interrupts are expected between 1000 and 2400 ms).
The wrong triggering of the ISR maybe will caused by the inrushcurrent of the AC motor (350 W), which can cause a spike in the system. Unfortunately I don't have anoscilloscoop.
//Interrupt request for limit switch S6 on pin D3 (IRQ1)
void Limit()
{if (cnt1>= 10)
{blnOnTop=true;}
}
//End routine
between them.![P4105345 [320x200].JPG](https://europe1.discourse-cdn.com/arduino/original/3X/d/b/dbc1cbce14df94ef5dcc38f326601c7dcf59b1a7.jpg)