Gateway MoteinoMEGA freezes after 20 mins [SOLVED]

[Charly86]

I would be very interessted to have a schematic and full ino code just to see 

[K1JOS]

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.

[K1JOS]

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 :-)

[oric_dan]

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.

Code Select Expand

if(((long)(micros() - last_micros) >= debouncing_time * 1000)) {

6. offhand, how often do you call this function?

Code Select Expand

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.

Code Select Expand

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.

[K1JOS]

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 !!!

[Charly86]

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

Code Select Expand

  // 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

Code Select Expand

  // 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)

Code Select Expand

#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
  }
}

[K1JOS]

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:

Code Select Expand

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

[K1JOS]

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!

[Felix]

Good to see this solved. Good discussion and many thanks to Charly86 and oric_dan for helping out!

[oric_dan]

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.

[K1JOS]

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?

[oric_dan]

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.

[Charly86]

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 :

Code Select Expand

#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