My project is complete and is working quite well (almost) except an odd event where one of my MegMoteino freezes after about 15-20 mins of what should be complete inactivity.
My base station MegaMoteino has the following functions.
1) Main Loop function sits and waits for an Interrupt on INT1 generated by one of four momentary buttons all tied to INT1 by diodes (and ground on the other end) and then the ISR sets an interrupt flag and calls a debounce routine that reads for the button pin change. The interrupt flag is cleared after the following routines are completed:
2) the button pressed equates to a specific 'command' (integer) and the base Mega turns associated LEDs on or turn off as needed.
3) following the button "command" selection, the base MegaMoteino updates via I2C an LCD listing the commands,etc.
4) the button "command" is then sent via the standard RFM69 library routines to a remote MegaMoteino which returns an ACK after receive and the remote interprets the command which activates sensors, motor relays, etc.
5) The Remote Mega, in response to the received "command", sends a sensor reading to the base MegaMoteino who sends an ACK back
6) the base Mega updates the LCD with the sensor data and then waits in idle for another interrupt.
When no new button is sent, there are no interrupts, and the base station Mega does NOTHING but Loops waiting for an interrupt. The remote will NOT sending anything back to the base Mega unless requested. The Base Moteino does NOT check for any radio packets unless a command button was first pressed. Once it receives sensor data from the remote, the base Mega is only in idle for the next interrupt.
It is during this idle time of about 15mins that the base Mega hangs and no longer responds to button presses. In my preliminary discovery of this problem, I think this maybe is related somehow to a Moteino internal timer because if I press a button and send a new command every few minutes, the freeze occurs much later.
My base MegaMoteino sketch is fairly long and my naming notation is not the clearest. I certainly don't mind posting the code but maybe something in the problem I described sounds familiar to someone?? Any advice in how to go about debugging this would be appreciated.
Could you perhaps Serial.print() something in the loop every X seconds or so to see at least how long it takes for the "freeze" to happen?
Hi Felix. First congrats on being selected a semi-finalist in the Hackaday Prize !
My problem just got more strange. I programmed one of the base station Mega buttons to have the Remote Mega continuously send back a sensor data packet (compass heading) every 300 msecs. After 20 minutes, the base Mega buttons no responded HOWEVER the base Mega is otherwising running correctly -- it is receiving packets of sensor data correctly and updating the LCD and blinking a LED with every received packet... but the buttons don't generate an interrupot anymore !!
Does this make any sense to you (or anyone else)??
That's strange. So it's working except the buttons doesn't interrupt any more?
Which interrupt(s) are you using for the button(s)?
I am using pin 11 on the Mega (SER1). My interrupt code is:
void setup();{
digitalWrite(11,INPUT_PULLUP);
attachInterrupt(1, debounceInterrupt, FALLING); // on mega INT1 is pin 11
//more stuff
}
...
...
...
void debounceInterrupt(){
if(((long)(micros() - last_micros) >= debouncing_time * 1000)) {
last_micros = micros(); // I reset last_micros =0 after the interrupt_flag is cleared to false in other subroutine
interrupt_flag = true; //set flag but make sure to reset it later
Serial.println("INTERRUPT"); // for debugging
}
}
I had it running overnight with a Serial.print("Step xx); Serial.println(millis()); at each functional step in my code.
This morning:
1) the LCD screen and LED blink were still receiving and updating data packets from the remote Mega
2) the buttons would NOT generate an interrupt
3) on the serial monitor, all Serial.print statements seemed to be in the correct sequence as I scanned hundreds of them. I looked carefully at the prints between 10 and 20 minutes timing and at the very end. The serial monitor ran for 28656225 millis (= 7.9 hours) until the Arduino IDE Java heap memory overflowed at 7.9 hours.
I know the buttons freeze around 10-20 minutes.
I don't have any potential infinite loops.. no Do or While loops. The only 'For' loops are for fixed integer incrementing such as reading or writing a data packet -- and those are working fine.
Is this bizarre? I am now creating a detailed flowchart of the base Mega code (should have done this from start) to make sure I have every IF statement covered by a debugging serial print.
My next steps are 1) switch to INT0 (pin 10) and see if that makes a difference. If not then I will swap my two Mega Moteino's (only have two with RF69 radios) and see if the freezing is occurring with a specific MegaMote and maybe not code.
I was toying with an idea to program a third standard Arrduino to exercise the button pins with programmed pinchange states at intervals but not sure how that info might help me debug this further.
Any other ideas?
Not sure if it would make sense to remove the pullup and change the attach interrupt from "falling" to "rising" ??
The first thing I did when I received my Moteino-Megas was use the gateway and node sketches to send packets for long periods of time, and they worked fine for as long as I tested them - about 5-hours. I did modify the sketches to send constant-length 32-byte packets.
Besides the pull up issue, you should never use Serial.print() statements inside an ISR. This violates the basic 'rule' that ISRs should be short and efficient, and quickly returned from. Similarly, don't use a delay() in there. Eg, you could set a flag inside the ISR, and then use this flag in the main loop to print your msg.
Yes, if I program the example Gateway/Node into both Moteinos they both work continously fine but that example code doesnt exercise all the functions of the Amtel chips nor all the interrupts. My serial print in the ISR was placed there after the problem came up just to see if the interrupt was being called randomly, eg noise on the tristate pin from something. The freeze occurs with the serial.print removed from the interrupt. I dont have any delays in the ISR. All of my subroutines are working fine in the background on my Base Mega ... I2C communications with the LCD, RF packet exchanges between the two Moteino's (including ACK's), LEDs being turned on and off in correct sequence.... all fine EXCEPT no more interrupts on button press.
I changed the interrup from INT1 to INT0 (pin 10) and no improvment. I will let it go thru another freeze cycle and check if the button pins are still high as they as supposed to be with the pullups. Maybe something pulls them down so the interrupt has no Falling edge to trigger on.
Quote from: K1JOS on September 02, 2014, 12:59:09 PM
Not sure if it would make sense to remove the pullup and change the attach interrupt from "falling" to "rising" ??
You can leave internal pullups enabled and use falling interrupts.
Like
oric_dan mentioned, if the default sketch is working then I have no reason to worry there is any issue with the MEGAs, and BTW they were intensely tested in the prototype stage to make sure they are stable.
Felix and All,
I am not trying to imply that anything is wrong with the Mega. I am looking for general advice how to debug what appears to be a very odd problem. If this were two regular Arduino's hardwired I would go to the Arduino forum and ask but given the unique nature (and libraries) of the Moteino's I am hoping you guys can point me in the right direction.
1. normally, a debounce period should probably be on the order of 10-20 MILLI-seconds, or more. They are mechanical switches, so you could use millis() here.
2. also, the timing variables should be declared as (unsigned long) rather than (long), although this probably wouldn't matter here.
3. I assume your debouncing_time is declared a long too, and not an int. For my part I always make equivocal statements into unequivocal, ie .... ((unsigned long)debouncing_time * 1000UL), etc, through the use of casts and parentheses. Many people are bit in the butt by deferring to use of precedence tables, and doing it wrong.
4. for reference, the millis() value wraps around after 49.7 days, and micros() after 71.6 minutes, and using a long rather than unsigned long may cause trouble in half that time.
5. also, just to see, you might try using an external pullup-R, like 5-10K. The internal pullup is rather high value, so you might have a noise problem, especially if you have long wires going to your switches, although that would probably have the opposite effect to what you're seeing. For my part, I would probably put a small value cap on the interrupt pin.
6. in any case, you need to first bullet-proof ISRs as much as possible: short functionality, no print statement or delay() use, proper casting of variables, low-noise external cktry, and [also] use of 'volatile' type-qualifications if the variables are declared inside the ISR. Then, you have a firm basis for everything else.
There are a lot of things going on here.
Thanks for taking the time to offer this solid advice.
The micros() are used because the ISR only sets an interrupt flag that has to be picked up in the Loop and immediately checked for which button was pushed. I think using millis() instead of micros() would missed the digitalRead on the 4 button pins. Here is all the code around the interrupt:
//declarations
long debouncing_time = 200;
volatile unsigned long last_micros;
boolean interrupt_flag = false;
void setup(){
digitalWrite(10,INPUT_PULLUP); //INT0 is pin 10 on MegaMote
attachInterrupt(0, debounceInterrupt, FALLING);
digitalWrite(18,INPUT_PULLUP); // these are the 4 buttons
digitalWrite(19,INPUT_PULLUP);
digitalWrite(20,INPUT_PULLUP);
digitalWrite(21, INPUT_PULLUP);
}
void debounceInterrupt(){
if(((long)(micros() - last_micros) >= debouncing_time * 1000)) {
last_micros = micros();
interrupt_flag = true;
}
}
void Loop(){
if (interrupt_flag){
button1 = !digitalRead(18);
button2 = !digitalRead(19);
button3 = !digitalRead(20);
button4 = !digitalRead(21);
ProcessButtons(); // this does all the work
interrupt_flag = false; //reset for next interrupt
last_micros = 0; // must reset for next interrupt to prevent overflow
}
}
The subroutine ProcessButtons () does all the work (checking for radiopackets, sending packets, updating LCD and LEDS) and this is obviously still working fine when the button interrupts stop working. I have no infinite loops in ProcessButtons to cause a freeze and if it did freeze the LCD and LED updating would stop as well - which it doesnt.
You can also see that i was declaring my timing variables all as you had suggested (except using micros() instead of millis() as I explained). Do you think the button digitalRead would occur fast enough if millis() were used in the debounce? I am less concerned about double pushes and the freezing is occuring without any button pushing.
Normally, I would have had the 4 digitalRead() calls inside the ISR, and then process the results in the main loop. They go "fairly" fast, just a few usec I think, and should not bog down the ISR. But I would also rather use direct port/pin reads, rather than Arduino calls, since they're much faster. I believe all you need here, given the way the mote-mega pins are defined is:
inside ISR:
button = PORTC;
outside:
volatile int button;
#define BUTTON1 (button & 0x04)
#define BUTTON2 (button & 0x08)
....
Not completely portable, but very efficient.
Since interrupt_flag, etc, are declared external to the ISR, I'm not completely sure whether they need to volatile or not. ?
It's always difficult to troubleshoot these sorts of things, when you don't have the complete code, and are unable to spend several days working on it.
K1JOS
I recommend you to disable the interrupt when using var that can be modified in isr AND in the loop
in loop the line
last_micros = 0; is one line, but generated assembler code will be more because it's long var, 4 bytes to reset. So if interrupt is fired in the middle you'll have "unknown state" of your var and unpredictable results.
Interrupt are so powerfull ;D, but attention need to be at the same level to deal with them ;)
Would you mind test this way just to see ?
void Loop(){
// just a reading bit no problem on this one test
if (interrupt_flag){
button1 = !digitalRead(18);
button2 = !digitalRead(19);
button3 = !digitalRead(20);
button4 = !digitalRead(21);
ProcessButtons(); // this does all the work
cli();
interrupt_flag = false; //reset for next interrupt
last_micros = 0; // must reset for next interrupt to prevent overflow
sei();
}
}
QuoteI recommend you to disable the interrupt when using var that can be modified in isr
Yes, good thinking. This may be "it".
Good conversation going here and I don't think I have anything to add. Charly's suggestion seems legit.
OK thanks to everyone -- great advice and gives me lots to check out tonight. Right now I am running everyhting after taking Felix's advice to place a 10K on the INT pin to Vcc and I placed a 0.01uF on the INT pin to ground. Waiting to see if it locks up. If so then onto the INT changes your recommend.
Is there a way to disable only a specific interrupt or is it All or Nothing?
I defer to Felix but do I have to be concerned what may happen to the RFM69 module as the radio uses a Moteino interrupt. Will shutting off all interrupts possible corrupt data packets that may be in the middle of transfer between the RF69 module and the ATMega??
KIJOS,
of course you can disable only the interrupt you're using, it's my favorite way also ;)
in your case INT1
bitClear(EIMSK, INT1); // instead of cli();
....
bitSet(EIMSK, INT1); // instead of sei();
disabling interrupt do not forget the IRQ, it's just delay the time when it will be treated.
By the way I think's it's not a problem for 2 reasons :
1st : code is very fast between cli/sei
2nd : RFM69 fire interrupt when a packet available, so even if you get it some ms later, it should be fine (until another one arrive so fast you did not had time to catch the 1st) but in this case CRC is you friend, frame could be missed but not seen as a valid (I think)
Charly86,
Can I turn off interrupts at the top of the loop
before I do the digitalRead of the pins and then re-enable the interrupt as the last step in the Loop??
Quote from: Charly86 on September 02, 2014, 05:07:51 PM
K1JOS
I recommend you to disable the interrupt when using var that can be modified in isr AND in the loop
in loop the line
last_micros = 0; is one line, but generated assembler code will be more because it's long var, 4 bytes to reset. So if interrupt is fired in the middle you'll have "unknown state" of your var and unpredictable results.
Interrupt are so powerfull ;D, but attention need to be at the same level to deal with them ;)
Would you mind test this way just to see ?
void Loop(){
// just a reading bit no problem on this one test
if (interrupt_flag){
button1 = !digitalRead(18);
button2 = !digitalRead(19);
button3 = !digitalRead(20);
button4 = !digitalRead(21);
ProcessButtons(); // this does all the work
cli();
interrupt_flag = false; //reset for next interrupt
last_micros = 0; // must reset for next interrupt to prevent overflow
sei();
}
}
K1JOS,
I'm not sure to understand, without a schematic to have a global overview, If I understand each button is connected to a port and also grouped to go to the interrupt line INT1 that's it ?
If so, may be it can be easier to use the PIN change Interrupt on all 4 buttons where they are connected, this will give you 4 independent interrupt (one for each button) and no need to use and group INT1 with diodes. The trick is that you do not need any hardware change, and deboucing can be done individually for each button ;)
Just an idea
Hi Charly,
I got this advice originally on the Arduino forum to stick with hardware interrupt for several reasons listed on the Arduino page (see quotes below):
Quote
This library was designed for the Arduino Uno/Duemilanove, and has been reported to work fine on the Nano, but it has not been tested there. As mentioned above, MEGA support is included but support for that platform is weak.
Since the MegaMoteino is a different ATMega than the MEGA AtmEGA 328 Arduino I was more concerned
Quote
Furthermore, the pin change interrupts are grouped into 3 "port"s on the MCU, so there are only 3 interrupt vectors (subroutines) for the entire body of 20 pins. This makes the job of resolving the action on a single interrupt even more complicated. The interrupt routine should be fast, but complication is the enemy of speed
So I thought the using the single hardware INT (all 4 'button' pins are pulled-up and tied to INT pin so that when the button is shorted to ground, the other buttons remain high and only the presed button is read low. It seemed clean and straight forward with little code overhead.
Jerry
Jerry,
you're right, I forgot this point, one ISR per port, so one ISR for 8 bits ;-)
Mega works same way as 328, more functions, more port, but very similar, the deal is that it's not standard for Arduino and thus no classic files exists for pin definition and even on library, so each one can have it's own pin definitions (the new ones ont 1284) that make some headaches. Felix choosed the mighty1284p format which seems to be a good one, but unfortunatly on Arduino IDE 1.5.x the pins definitions are not the same even for IRQ. Arduino is just a nice "presentation" and simplifies things and have so much libraries !!!!!!!
But behind the scene it's just a microcontroller as other, so as you're confortable with IRQ on them, whatever the target you will use, it will work the same. I did not had a change to test on multiple port pin change IRQ because until now I did not need it, but it's really worth it to be able to do so ;)
But you're choice will work fine also ;-)
Well I used the following in my Loop:
void loop() {
// LOOP STEP 1 Button interrupt and debounce
if (interrupt_flag){
bitClear(EIMSK, INT0); // turn off INT0
button1 = !digitalRead(M_CW_BUTTON);
button2 = !digitalRead(M_OFF_BUTTON);
button3 = !digitalRead(RESET_BUT);
button4 = !digitalRead(M_CCW_BUTTON);
ProcessButtons();
interrupt_flag = false; //reset for next interrupt //@ do i need some delay first to finisjh digital Read?
last_micros = 0; // must reset for next interrupt
bitSet(EIMSK, INT0); // turn on INT0
}
And it still freezes after 20 min while the rest of the subroutines chug away correctly. Next I will swap Mega's.
If I remember correctly you're attached to INT1 not INT0 you need to change in bitset and bitclear according to INT1
But at first I suggest cli sei to be sure at first.
I wanted to make sure it wasnt a bad "pin" so I changed my interrupt pin from INT1 (pin 11) to INT0 (pin10) and just kept it that way. Sorry for the confusion. OK I will try cli and sei now.
ah ok, sorry, that makes sense now with your explanations ;)
Well, good news is its not the MegaMoteinos as swapping them made no difference. So i8ts the code.
I placed the cli() and sei() instead of the bitClear(EIMSK, INT0) and something strange happened. Two of the buttons worked for more than an hour but as soon as I hit the first button the wholoe Mega stopped cold... including the LCD updating and LED blinking. I think this gives me a clue that its something in the code that processes some of the buttons actions and not others. It is still weird that with no button press nothing is going on except radio receive and LCD updating but I now can focus in more. Has to wait until later today, work comes first :-)
You know, watching this conversation it reminds me of learning how to use interrupts the hard way. And in fact I don't know if there's an easy way. After so many hours/days of pulling hairs you develop that 6th sense of how these things work. You will still make mistakes, but the more you code the better you get at it, and fewer mistakes.
+1 Felix,
IRQ are ..... need to be mastered, what I've learned with them is when using them and have a problem major pb is coming from IRQ.
What you can do is to try to isolate the pb, just HW with your buttons, diodes and IRQ and just code to deal with the button and the IRQ. You seem to have a display and other things, so we're not even sure it's coming from IRQ (more if you use other people library)
You have no idea of # of lib I trusted before seeing that bugs where not in my code but on the libs !!!
Using cli() and sei() as Charly suggested, the freeze occurs in Button Processing subroutine where only one of the four button calls my LCD update subroutine directly from this subroutine rather than the usual call from my Loop(). Now this LCD subroutine is called liberally and its weird that before introducing cli and sei the freeze would not affect LCD updating. None of my subroutines is passing any variables and I have all flags and variable as 'volatile' now.
Well I'm digging in to further isolate and will report back. I think your right, debuggin is an important part of getting more proficient with coding.
I would be very interessted to have a schematic and full ino code just to see ;)
OK, I will post it as an attachment, remeber I am a 'newbie' and my coding habits are not very polished. I think I have it fairly well commented though.
Well, I have gone through the code line by line and cannot find anything that should cause a hangup, memory overflow, unexpected interrupt call, etc certainly not something that would otherwise flawlessly for 20 minutes. I inserted the cli() and sei() in various places as Charly suggested but the Mega still freezes after 20mins. I thought the LCD updating subroutine was an issue so i simplified it but still freezing.
I am attaching my Base MegMoteino code and I hope someone has the generosity to give it a once over to see if something is fundmentally wrong. I am sure there are more elegant ways of doing what I am trying to accomplish so be kind :-)
Like charly, I've also had a *LOT* of problems using 3rd party libraries. Many are not very well proofed, and also don't play well with others.
1. in many cases when stacking multiple shields, the board interrupts and pin assignments clash.
2. I think the most-common problem with many libraries is they are not very well tested in a range of situations. Eg, the original jeelib for RFM12 is a major case in point, and I'm sure felix had his day in trying to adapt it. jeelib has a dozen bugs at least in it. I also had hang-up troubles with the mikem library for the RFM22 radio. It would run for a short time and then crash. I finally gave up on it - and came over to moteinos and the RFM69 - and glad I did (so far).
3. you can often tell that very poor testing was done by looking at whether the example sketches are trivial little splats of code, or whether they indicate a significant amount of thought and testing. Eg, the mikem examples are trivial, and indicate he never tested the library via sending long data packets for long periods of time, which is the first thing I wanted to do (as with moteino, where this works).
4. LCD may be one of these problem libraries - but I've never used it.
5. re your sketch, oof, not trivial. I still do not like your **mixing** of longs and unsigned longs, with use of micros, as this can cause wraparound problems after 36-minutes or so, as indicated in reply #10 of this thread.
if(((long)(micros() - last_micros) >= debouncing_time * 1000)) {
6. offhand, how often do you call this function?
void resetUsingWatchdog(boolean reset_flag)
7. one way to approach troubleshooting for this complex type of program is to eliminate stuff, until the problem goes away, and then start adding stuff back in.
8. eg, you could throw away all of the LCD stuff, and simply blink an Led to indicate button presses, and see if that hangs.
9. I would also totally rewrite the blinker, it's written very poorly, and will hang your program. First, throw away the Serial.println() statement - you do have the blink after all, and secondly, chuck the stupid delay() statements.
void Blink(byte PIN, int DELAY_MS)
{
Serial.print("Blink");
Serial.println(millis()); //@
pinMode(PIN, OUTPUT);
digitalWrite(PIN,HIGH);
delay(DELAY_MS);
digitalWrite(PIN,LOW);
}
10. use the code from the IDE "Blink without delay" sketch. What I would do is move the blinker out into its own routine in the main loop, timed via the "Blink without delay" scheme, and setting a flag in various routines to enable the blinker scheduler. Then no problem with hangups.
11. always avoid using delay() at all costs - ever, always, and forevermore, until eternity.
Thanks oric_dan for the great tips.
I removed the LCD updating completely and just use LEDs and it still hangs after about 20 mins. Tough to debug this as each change I have to wait 20-30 mins to see if it hangs. VERY TIME CONSUMING!!
I only call the WDT reset by a specific button press so basically I never use it routinely. I thought to include it only to call the remote to reset if their were any problems with the remote so I wouldn't have to go outdoors and climb a ladder to reach it :-) I will eliminate it from the Base Mega controller code for now.
I will also replace the Blink with your suggested code and test before moving to next step
About the micros(). Did you catch one of my earlier reply? I got these from Arduino Forum or Nick Gammons's site. If I change to millis(s) i dont think i will catch the digitalRead Button 1,2,3,4 in time unless I really hold those buttons down for a while. I also can switch to millis and test for now to see if it fixes the freezing
After that it will be stopping the RF69 routines and replace it with a dummy received data packet.
With a long pause between each step... I am tempted to make all the changes at once and see if that works and then go add back one thing at a time to see what createdd the problem
Folks, Please keep those ideas coming in !!!
Yep I would advice the same as Oric_dan, remove all, then test with minimal and add new features if previous step rock solid.
The steps could be :
- Check button with interrupt debounce and lit LED to see if button pressed
- Add Radio Send (on button detection for example and lit let on start send, then led off after sending)
- Add Radio Receive (lit led on receive)
- Add LCD
Another advice looking quickly into the code
// STEP 3 NOW CHECK FOR INCOMING (NEW) DATA FROM REMOTE MEGA-MOTEINO
if (radio.receiveDone())
{
LCD_RSSI = radio.readRSSI();
Serial.println(LCD_RSSI);
if (radio.DATALEN != sizeof(Payload)) // this should only be PAYLOAD data
{
lcd.setCursor(0, 3);
lcd.print("Remote OFF-Line");
}
else
{
theData = *(Payload*)radio.DATA; //assume radio.DATA actually contains our struct and not something else
newdata_flag = true; // SET TO CHECK IF LCD UPDATING NEEDED IN STEP 4
Blink(LED,3); //use same led as M_OFF fpor general blinking
if (radio.ACK_REQUESTED) // only ack if good payload
{
byte theNodeID = radio.SENDERID;
radio.sendACK();
}
}
} // END OF LOOP 3
Do nothing in receiving mode until ACK has been sent quickly, use flag as follow
// STEP 3 NOW CHECK FOR INCOMING (NEW) DATA FROM REMOTE MEGA-MOTEINO
if (radio.receiveDone())
{
LCD_RSSI = radio.readRSSI();
if (radio.DATALEN != sizeof(Payload)) // this should only be PAYLOAD data
{
flag_bad_payload = true ;
}
else
{
// send ACK ASAP
if (radio.ACK_REQUESTED) // only ack if good payload
{
byte theNodeID = radio.SENDERID;
radio.sendACK();
}
theData = *(Payload*)radio.DATA; //assume radio.DATA actually contains our struct and not something else
newdata_flag = true; // SET TO CHECK IF LCD UPDATING NEEDED IN STEP 4
flag_blink=true;
}
} // END OF LOOP 3
// Step 3.5
Serial.println(LCD_RSSI);
if (flag_bad_payload ) {
lcd.setCursor(0, 3);
lcd.print("Remote OFF-Line");
}
if (flag_blink)
Blink(LED,3)
// Step 4
of course you can do step 3.5 after step 4 depending on you needs
And now for the fun, here the blink routine I use in my loop code at no cost except ulong var, quick, efficient, no delay (simplified code)
#define BLINK_LED_MS 50 /* 50 ms */
unsigned long rf12_led_timer ;
unsigned long rf69_led_timer ;
loop()
{
// received data ?
if (packetReceived && good packet && good data && whatever)
{
blah blah ;
if (receive_from_rf12)
{
// Light on the LED
ledON(LED_RF12);
// Start led blink virtual timer
rf12_led_timer = millis() ;
}
if (receive_from_rf69)
{
// Light on the LED
ledON(LED_RF69);
// Start led blink virtual timer
rf69_led_timer = millis() ;
}
}
// Do there other stuff
// blah blah
// now last step in loop code
// Do we have led timer expiration ?
if (rf12_led_timer && (millis()-rf12_led_timer >= BLINK_LED_MS))
{
ledOFF(LED_RF12); // Light Off the LED
rf12_led_timer=0; // Stop virtual timer
}
if (rf69_led_timer && (millis()-rf69_led_timer >= BLINK_LED_MS))
{
ledOFF(LED_RF69); // Light Off the LED
rf69_led_timer=0; // Stop virtual timer
}
}
Thank you Charly86 and oric_dan. I started stripping off and changing things before I saw Charly86's email.
GOOD NEWS!!! NO FREEZING with the following changes:
1) made all long --> unsigned long
2) got rid of blink with delay()
3) removed LCD updating in response to button interrupt but kept LCD updating for displaying Remote Mega Sensor data
4) replaced blink with a very simple:
volatile boolean toggle_LED_flag == false;
...
...
void Blink(byte PIN)
{
//new version below
if (toggle_LED_flag == false)
{
digitalWrite(PIN,HIGH);
toggle_LED_flag = true;
}
else if (toggle_LED_flag == true)
{
digitalWrite(PIN,LOW);
toggle_LED_flag = false;
}
}
I think it must have been the mix of long with unsigned long as the program would freeze before even when Blink was not called (only two of the 4 buttons called Blink).
I will add back the full LCD updating now and then recheck. If all well I will still make the further enhancements to the code that you suggested to clean things up.
I haven't done any C or Basic programming for a project for more than 20 years and this was a project of need for my ham radio pneumatic 40 foot antenna mast. I needed to control air inflation/deflation, motor rotation (CW and CCW) and incorporate a sensor to tell me compass heading direction. While I was doing this i was also designing a custom utility tilt-over mount to hold the rotating antenna mast (7 feet when fully reduced) . I plan to use two 12v AGM deep cycle batteries with solar charger to power the air pump, antenna rotor motor (5vdc 10A) and the remote electronics (Remote Mega, Sensor,etc). One year in the works and I was overly confident that I could do the programming for both Moteinos at the last minute :-) Before this freezing glitch my biggest problem was learning the nuances of RS485 and to debug the proprietary rotor motor controller that was using an unknown data packet for rotor commands. I finally learned how to really decode RS232 on a scope. Thank goodness I don't do this for a profession !!
Will keep you posted
best
jerry
A huge Thank You to Charly86 and oric_dan for their time and effort in looking at pages of my code. The problem now appears solved as the program ran overnight without freezing. This thread has a lot of great advice on not so easy to debug programming mistakes (on my part).
Thanks Felix for providing a great forum!
Good to see this solved. Good discussion and many thanks to Charly86 and oric_dan for helping out!
Good. In this case, it may have been one specific thing that fixed the problem, whatever that might be, but the code in general is probably all a lot cleaner.
Also, one day you might also discover the scheme I mentioned in reply #12. You will notice that, as your program gets more and more complicated, there can be a seriously long delay before the buttons are actually read at the top of the main loop, as it takes *many* msec to execute Serial.print()'s and LCD writes, on and on.
Quote from: oric_dan on September 02, 2014, 04:49:46 PM
Normally, I would have had the 4 digitalRead() calls inside the ISR, and then process the results in the main loop. They go "fairly" fast, just a few usec I think, and should not bog down the ISR. But I would also rather use direct port/pin reads, rather than Arduino calls, since they're much faster. I believe all you need here, given the way the mote-mega pins are defined is:
inside ISR:
button = PORTC;
outside:
volatile int button;
#define BUTTON1 (button & 0x04)
#define BUTTON2 (button & 0x08)
....
Not completely portable, but very efficient.
Since interrupt_flag, etc, are declared external to the ISR, I'm not completely sure whether they need to volatile or not. ?
Thanks again oric_dan, i had forgotten about that and I do want to incorporate it now (it will stay in my 66yo memory a lot better ). I see tht the MegaMot uses PORTC for I2C on PC0 and PC1. I guess as long as I am careful not to mess with those I should be OK?
You're only doing a read on the port, and then masking bits, so shouldn't affect I2C, I think. I don't believe a read, as I showed it, will change pin configurations, but you might double-check this. Always good to double-check.
You never know. Eg, using the Arduino digitalRead() may just set the configuration bit. For Arduino, I actually spend a lot of time reading through the IDE source files to figure out what the functions really do.
It's good to know it's solved, right on the way to add more core and function to your project (and some headache :))
oric_dan, yes did you saw what DigitalRead/Write are doing ? Amazing !!!!!! I love to go also on Core func, sometimes you really need to know what it's done.
I even thought sometimes going to Visual Studio and let the Arduino IDE for my projects as I've got very often multiples files and Arduino IDE really have strange behavior on this, the last I found is the following :
#ifdef MOD_RFM12
#include <WirelessHEX.h>
#endif
#ifdef MOD_RFM69
#include <WirelessHEX69.h>
#endif
Trust me it does not work and both .h are included at compile time whatever RFM12 and RFM69 are defined or not