Loop stops working after some time - Only Interrupt is functional

I'm developing a Arduino program to read CAN bus data from a PLC and display it as a overlay on a video system.

I'm using a time interrupt to read CAN bus data and using the default loop function to overlay text. The program works fine for few minutes and then it stops executing the loop function and only the interrupt is functional at that point.

I have attached the code for your reference (I have used few println line for debugging purpose, please ignore them). I'm wondering on what's causing this issue. Please help me with this. Thanks in advance for your support.

Cheers

59.ino (6.34 KB)

I have attached the code

That was NOT necessary. You SHOULD have posted your code properly, using code tags:

#include <TimerOne.h>
#include <Canbus.h>
#include <defaults.h>
#include <global.h>
#include <mcp2515.h>
#include <mcp2515_defs.h>
#include <SPI.h>
#include <MAX7456.h>
int y = 0;



int x;
// Pin Mapping /////////////////////////////////////////////////////////////////

// pinValue = 0 means "not connected"

//  FDTI Basic 5V                   ---  Arduino  VCC      (AVCC,VCC)
//  FDTI Basic GND                  ---  Arduino  GND      (AGND,GND)
//  FDTI Basic CTS                  ---  Arduino  GND      (AGND,GND)
//  FDTI Basic DTR                  ---  Arduino  GRN
//  FDTI Basic TXO                  ---> Arduino  TXO [PD0](RXD)
//  FDTI Basic RXI                 <---  Arduino  RXI [PD1](TXD)


//  Max7456 +5V   [DVDD,AVDD,PVDD]  ---  Arduino  VCC      (AVCC,VCC)
//  Max7456 GND   [DGND,AGND,PGND]  ---  Arduino  GND      (AGND,GND)
//  Max7456 CS    [~CS]            <---  Arduino  10  [PB2](SS/OC1B)
//  Max7456 CS    [~CS]            <---  Mega2560 43  [PL6]
const byte osdChipSelect             =            10;
//  Max7456 DIN   [SDIN]           <---  Arduino  11  [PB3](MOSI/OC2)
//  Max7456 DIN   [SDIN]           <---  Mega2560 51  [PB2](MOSI)
const byte masterOutSlaveIn          =                      MOSI;
//  Max7456 DOUT  [SDOUT]           ---> Arduino  12  [PB4](MISO)
//  Max7456 DOUT  [SDOUT]           ---> Mega2560 50  [PB3](MISO)
const byte masterInSlaveOut          =                      MISO;
//  Max7456 SCK   [SCLK]           <---  Arduino  13  [PB5](SCK)
//  Max7456 SCK   [SCLK]           <---  Mega2560 52  [PB1](SCK)
const byte slaveClock                =                      SCK;
//  Max7456 RST   [~RESET]          ---  Arduino  RST      (RESET)
const byte osdReset                  =            0;
//  Max7456 VSYNC [~VSYNC]          -X-
//  Max7456 HSYNC [~HSYNC]          -X-
//  Max7456 LOS   [LOS]             -X-

union TEMP
{
  float f;
  byte b[8];
};

String date;
String mytime;
char rcv_str[8];
// Global Macros ///////////////////////////////////////////////////////////////

float final_speed;
float final_distance;


// Global Constants ////////////////////////////////////////////////////////////

const unsigned long debugBaud = 115200;         // Initial serial baud rate for
//   Debug PC interface





// Global Variables ////////////////////////////////////////////////////////////

HardwareSerial Debug = Serial;                // Set debug connection

MAX7456 OSD( osdChipSelect );
void setup()
{

  Serial.begin(9600); // For debug use
  Serial.println("CAN Read - Testing receival of CAN Bus message");
  delay(1000);

  if (Canbus.init(CANSPEED_500)) //Initialise MCP2515 CAN controller at the specified speed
    Serial.println("CAN Init ok");
  else
    Serial.println("Can't init CAN");

  delay(1000);
  // Initialize the digital pin as an output.
  // Pin 13 has an LED connected on most Arduino boards
  pinMode(13, OUTPUT);
  Serial.begin(115200);
  Timer1.initialize(1000000); // set a timer of length 100000 microseconds (or 0.1 sec - or 10Hz => the led will blink 5 times, 5 cycles of on-and-off, per second)
  Timer1.attachInterrupt( timerIsr ); // attach the service routine here

  InitializeDisplay();
}

void loop()
{
  // Main code loop
  // TODO: Put your regular (non-ISR) logic here
  String str(rcv_str);
  Serial.println(final_speed);
  Serial.println(final_distance);
  Serial.println(rcv_str);
  Serial.println(date);

  while (OSD.notInVSync());                   // Wait for VSync to start to

  OSD.setCursor(0, 1);
  OSD.print("DATE ");
  OSD.print(date);
  OSD.print("                     ");


  OSD.setCursor(16, 1);
  OSD.print("TIME ");
  OSD.print(mytime);
  OSD.print("                     ");

  OSD.setCursor(12, 11);
  OSD.print("HOLEID:");
  OSD.print(str);
  OSD.print("                     ");

  OSD.setCursor(12, 9);
  OSD.print("SPEED:");
  OSD.print(final_speed);
  OSD.print("                     ");

  OSD.setCursor(12, 13);
  OSD.print("DISTANCE:");
  OSD.print(final_distance);
  OSD.print("                     ");
  while (true) {
    int k = 0;

    for (; k < 401; k++) {}

    if (k == 401) {
      // Serial.println(x);

      x = 2;

      break;
    }
  }


  y = 0;
  delay(100);
}
/// --------------------------
/// Custom ISR Timer Routine
/// --------------------------
void timerIsr()
{

  OSD.clear();

  Serial.println("----------------------------------");


  while (true) {
    tCAN message;

    if (mcp2515_check_message())
    {
      if (mcp2515_get_message(&message))
      {

        if (message.id == 0x101)
        {
          byte k[6];
          for (int cnt = 0; cnt < 8; cnt++)
          {
            k[cnt] = message.data[cnt];
          }
          date = String((int) k[2]) + "/" + String((int) k[1]) + "/" + String((int) k[0]);
          mytime = String((int) k[3]) + ":" + String((int) k[4]) + ":" + String((int) k[5]);


        } else if (message.id == 0x102) {

          for (int i = 0; i < message.header.length; i++)
          {

            rcv_str[i] = (char)message.data[i];

          }


        } else if (message.id == 0x103) {
          int speed;

          speed = message.data[0] + message.data[1] * 256;
          if (speed > 32768) {
            final_speed = (speed - 32768) / 10;
          } else {
            final_speed = speed / 10;
          }
          int distance;
          distance = message.data[2] + message.data[3] * 256;
          if (distance > 32768) {
            final_speed = (distance - 32768) / 10;
          } else {
            final_distance = distance / 10;
          }
        }
      }
    }
    if (message.id == 257) {
      y = 1;

    }

    if (message.id == 257) {
      y = 2;

    }

    if (message.id == 258) {
      y = 3;
    }

    if (y == 3) {
      Serial.println("OK");
      break;
    }
  }


}


void InitializeDisplay()
{
  unsigned char system_video_in = NULL;
  // Initialize the Serial connection to the debug computer:
  Debug.begin( debugBaud );


  // Initialize the SPI connection:
  SPI.begin();
  SPI.setClockDivider( SPI_CLOCK_DIV2 );      // Must be less than 10MHz.

  // Initialize the MAX7456 OSD:
  OSD.begin();                                // Use NTSC with default area.
  //OSD.setCharEncoding( MAX7456_ASCII );       // Only needed if ascii font.

  system_video_in = OSD.videoSystem();
  if (NULL != system_video_in)
  {
    OSD.setDefaultSystem(system_video_in) ;
  }
  else
  {
    OSD.setDefaultSystem(MAX7456_NTSC) ;
  }

  OSD.display();
}

There is NO excuse for using the String class. It can cause memory fragmentation, and cause your program to stop working. Hey, that sounds familiar...

    int k = 0;

    for (; k < 401; k++) {}

This is completely useless code.

    if (k == 401) {

Well, of course k will contain 401, because that is what terminated the useless for loop, so after accomplishing nothing, you break out of the useless while loop UNCONDITIONALLY.

Get rid of the entire useless while loop.

  delay(100);

Why? You'd better have a good reason.

Some PROOF that loop() stops would be useful. Since it contains other do-nothing-until code, I'd suspect that something is causing that do-nothing-until code to never do anything, because the until event never occurs.

Moderator edit: less profanity please

By the way, you clearly do NOT understand what you can, and what you can NOT do in an ISR. An ISR should NEVER have a while(true) statement that runs forever.

          byte k[6];
          for (int cnt = 0; cnt < 8; cnt++)
          {
            k[cnt] = message.data[cnt];
          }

Writing to memory you don't own is NEVER a good idea.

Moderator edit: less profanity please

Thank You very much for your reply.

At the beginning, I didn't have a while loop in ISR , then the interrupt program was unable to read all the CAN bus packets. That's the reason why I deliberately added the while loop.

The infinite while loop is used to suppress artifacts on the display, but if I allow to run the while loop infinitely, the interrupt program will never active and the display will never be updated.

So, I introduced the for loop as a counter and when it reaches 401, it would break the infinite while loop.

for(;k<401;k++){}

This might be a crazy idea, but I added this to temporarily remedy this problem and it worked.

In a nut shell, the infinite while condition should be working all the time in the Arduino loop to suppress the artifacts. But if it get stuck in the while loop, it will never read the CAN bus data again and the display also won't be updated.

If you could suggest a solution for the above issue, that would be fantastic and I would be able to get rid of unnecessary code.

a) use and set a flag within the ISR. Outside (in loop), test the condition of the flag to determine if the ISR has been triggered and run your while loop there and reset the flag in preparation for the next interrupt to trigger. As an aside, if it will take a long time to process your interrupt code in loop, you might think about detaching the interrupt until finished and the reattaching it. It's all about timing.

b)for(;k<401;k++){} will do nothing except cycle k to 401. In fact, the compiler will recognize that it does nothing and replace it with a constant, k = 401. Generally, with a for loop there is code within the braces.

PaulS:
There is NO excuse for using the String class. It can cause memory fragmentation, and cause your program to stop working.

A minor problem in comparison to...

...
/// --------------------------
/// Custom ISR Timer Routine
/// --------------------------
void timerIsr()
{
...
          date = String((int) k[2]) + "/" + String((int) k[1]) + "/" + String((int) k[0]);
          mytime = String((int) k[3]) + ":" + String((int) k[4]) + ":" + String((int) k[5]);
...

...corrupting the heap.