I'm having trouble with Arduino UNO. I bought a cheap clone (what obviously may be the reason) along with display+keypad shield (site in Polish). For my project I wrote a menu library, which (along with simple demo project) may be downloaded or cloned from my Bitbucket repository.

My problem is, that when I push "up" or "down" button and hold it, program behaves as it should for couple of seconds and then board freezes. This is definitely not a case of program hang, because when I used serial debugging, and when such freeze happened, message sent from board was cut in half and no new messages were comming. After such hang I can only reset the board.

I investigated my code a lot, but couldn't find a problem. My questions are:

  • In what circumstances board may freeze in such way?
  • If someone owns similar hardware (display, 5-button keypad) and original UNO, could you try running my program and check, if the hang happens as well?

Edit: Complete sources of the project, as requested (now fixed), though the newest version is on repository:

Arduinomenu.ino:

#include "Display.h"
#include "Keypad.h"
#include "Menu.h"

LiquidCrystal lcd(8, 9, 4, 5, 6, 7);
Display display(lcd, 16, 2);
Keypad keypad;

BaseMenuItem * subItems[] =
{
  new ActionMenuItem("Subaction 1", 4),
  new ActionMenuItem("Subaction 2", 5),
  new ActionMenuItem("Subaction 3", 6),
  new IntMenuItem("Int item", 0, 10, 5),
  new BoolMenuItem("Bool item", true)
};

BaseMenuItem * mainItems[] = 
{
  new ActionMenuItem("Action 1", 1),
  new ActionMenuItem("Action 2", 2),
  new ActionMenuItem("Action 3", 3),
  new SubMenuItem("Submenu", subItems, sizeof(subItems) / sizeof(BaseMenuItem *))
};

MainMenu mainMenu(display, mainItems, sizeof(mainItems) / sizeof(BaseMenuItem *));

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

  mainMenu.Show();
}

void loop()
{
  int key = keypad.ReadKey();
  int result = mainMenu.Action(key);
}

Display.h:

#ifndef DISPLAY_H__
#define DISPLAY_H__

#include <LiquidCrystal.h>

struct Display
{
  LiquidCrystal & lcd;
  unsigned char cols, rows;

  Display(LiquidCrystal & lcd, unsigned char cols, unsigned char rows);
  void Print(unsigned char col, unsigned char row, char * text);
};

#endif

Display.cpp:

#include "Display.h"

Display::Display(LiquidCrystal & lcd, unsigned char cols, unsigned char rows)
  : lcd(lcd)
{
  this->cols = cols;
  this->rows = rows;
  lcd.begin(cols, rows);
}

void Display::Print(unsigned char col, unsigned char row, char * text)
{
  this->lcd.setCursor(col, row);
  this->lcd.print(text);
}

Keypad.h:

#ifndef KEYPAD_H__
#define KEYPAD_H__

#define USE_KEYPAD

#define KEY_RIGHT  0
#define KEY_UP     1
#define KEY_DOWN   2
#define KEY_LEFT   3
#define KEY_SELECT 4
#define KEY_NONE   5

#define LAST_READ_DELAY 250

#define keypadPort 0

class Keypad 
{
private:
  unsigned long lastRead;
  unsigned char lastKey;
public:
  Keypad();
  unsigned char ReadKey();
};

#endif

Keypad.cpp:

#include "Keypad.h"
#include "Arduino.h"

Keypad::Keypad() 
{
  this->lastRead = millis();
  this->lastKey = KEY_NONE;
}

// Button values (approximate):
// Right    0
// Up       144
// Down     329
// Left     504
// Select   741
unsigned char Keypad::ReadKey() 
{
  unsigned long now = millis();
  if (this->lastKey != KEY_NONE && (now - this->lastRead < LAST_READ_DELAY) && (now > this->lastRead))
  {
    return KEY_NONE;
  }

  int key = analogRead(keypadPort);      

  if (key > 1000)
    this->lastKey = KEY_NONE;
  else if (key < 50)
    this->lastKey = KEY_RIGHT;
  else if (key < 250)
    this->lastKey = KEY_UP;
  else if (key < 450)
    this->lastKey = KEY_DOWN;
  else if (key < 650)
    this->lastKey = KEY_LEFT;
  else if (key < 850)
    this->lastKey = KEY_SELECT;
  else
    this->lastKey = KEY_NONE;

  this->lastRead = now;

  return this->lastKey;
}

Menu.h:

#ifndef MENU_H__
#define MENU_H__

#include <stdio.h>
#include <stdlib.h>
#include "Display.h"
#include "Arduino.h"

#ifdef USE_KEYPAD
  #define MENU_ACTION_RIGHT KEY_RIGHT
  #define MENU_ACTION_UP KEY_UP
  #define MENU_ACTION_DOWN KEY_DOWN
  #define MENU_ACTION_LEFT KEY_LEFT
  #define MENU_ACTION_SELECT KEY_SELECT
#else
  #define MENU_ACTION_RIGHT 0
  #define MENU_ACTION_UP 1
  #define MENU_ACTION_DOWN 2
  #define MENU_ACTION_LEFT 3
  #define MENU_ACTION_SELECT 4
#endif

#define TEXT_BACK "Back"
#define TEXT_BACK_LEN 4

#ifndef SELECTION_CHAR
#define SELECTION_CHAR '>'
#endif

#define MENU_TYPE_INT 0
#define MENU_TYPE_BOOL 1
#define MENU_TYPE_SUB 2
#define MENU_TYPE_ACTION 3

#ifndef MAX_MENU_LEVELS
#define MAX_MENU_LEVELS 5
#endif

// Forward declarations -------------------------------------------------------

class MainMenu;

// BaseMenuItem ---------------------------------------------------------------

class BaseMenuItem
{
protected:
  const char * name;
  unsigned char len;

public:
  BaseMenuItem(const char * name);
  void ShowListItem(Display & display, int row, bool selected);
  const char * GetName();
  virtual int GetType() = 0;
};

// MenuItem -------------------------------------------------------------------

class MenuItem : public BaseMenuItem
{
friend class MainMenu;

private:
  void ShowEditMode(Display & display);

protected:
  void ShowEditHeader(Display & display);
  virtual void ShowEditValue(Display & display) = 0;
  virtual void HandleAction(int key) = 0;  

public:
  MenuItem(const char * name);  
};

// IntMenuItem ----------------------------------------------------------------

class IntMenuItem : public MenuItem
{
friend class MainMenu;

private:
  int value;
  int min;
  int max;

protected:
  void ShowEditValue(Display & display);
  void HandleAction(int key);

public:
  IntMenuItem(const char * name, int min, int max, int defaultValue);
  int GetType();
};

// BoolMenuItem ---------------------------------------------------------------

class BoolMenuItem : public MenuItem
{
friend class MainMenu;

private:
  bool value;

protected:
  void ShowEditValue(Display & display);
  void HandleAction(int key);

public:
  BoolMenuItem(const char * name, bool defaultValue);
  int GetType();
};

// SubMenuItem ----------------------------------------------------------------

class SubMenuItem : public BaseMenuItem
{
friend class MainMenu;

private:
  BaseMenuItem ** items;
  int count;

public:
  SubMenuItem(const char * name, BaseMenuItem ** items, int count);
  int GetCount();
  BaseMenuItem * GetItem(int index);
  int GetType();
};

// ActionMenuItem -------------------------------------------------------------

class ActionMenuItem : public BaseMenuItem
{
private: 
  int actionId;

public:
  ActionMenuItem(const char * name, int actionId);
  int GetActionId();
  int GetType();
};

// MainMenu -------------------------------------------------------------------

class MainMenu : public SubMenuItem
{
private:
  SubMenuItem ** itemStack; 
  short * positionStack;
  unsigned char stackTop;
  short selectedItem;
  short firstItem;
  Display & display;
  MenuItem * editedItem;

  void ShowBack(int row, bool isSelected);

public:
  MainMenu(Display & display, BaseMenuItem ** items, int count);
  ~MainMenu();
  int Action(int keyAction);
  void Show();
};

#endif

Menu.cpp:

#include "Menu.h"

char * NewEmptyLine(int width)
{
  char * line = new char[width + 1];

  memset(line, 32, width);
  line[width] = 0;
  return line;  
}

void DeleteLine(char * line)
{
  delete line;
}

// BaseMenuItem ---------------------------------------------------------------

void BaseMenuItem::ShowListItem(Display & display, int row, bool selected)
{
  char * line = NewEmptyLine(display.cols);

  if (selected)
    line[0] = SELECTION_CHAR;

  strncpy(line + 2, this->name, min(display.cols - 2, this->len));
  display.Print(0, row, line);

  DeleteLine(line);
}

BaseMenuItem::BaseMenuItem(const char * name)
{
  this->name = name;
  this->len = strlen(this->name);
}

const char * BaseMenuItem::GetName()
{
  return this->name;
}

// MenuItem -------------------------------------------------------------------

void MenuItem::ShowEditHeader(Display & display)
{
  char * line = NewEmptyLine(display.cols);

  strncpy(line, this->name, min(display.cols, this->len));
  display.Print(0, 0, line);  

  DeleteLine(line);
}

void MenuItem::ShowEditMode(Display & display)
{
  this->ShowEditHeader(display);
  this->ShowEditValue(display);
}

MenuItem::MenuItem(const char * name) : BaseMenuItem(name)
{  

}

// IntMenuItem ----------------------------------------------------------------

IntMenuItem::IntMenuItem(const char * name, int min, int max, int defaultValue)
  : MenuItem(name)
{
  this->min = min;
  this->max = max;
  this->value = defaultValue < this->min ? this->min : (defaultValue > this->max ? this->max : defaultValue);
}

void IntMenuItem::ShowEditValue(Display & display)
{
  char * line = NewEmptyLine(display.cols);

  line[0] = 127;
  line[display.cols - 1] = 126;

  int i = display.cols - 3;
  int temp = abs(this->value);
  do
  {
    line[i] = temp % 10 + 48;
    temp /= 10;
    i--;
  }
  while (temp > 0);

  if (this->value < 0)
    line[i] = '-';

  display.Print(0, 1, line);

  DeleteLine(line);
}

int IntMenuItem::GetType() 
{
  return MENU_TYPE_INT;  
}

void IntMenuItem::HandleAction(int key)
{
  if (key == MENU_ACTION_LEFT)
  {
    if (this->value - 1 >= this->min)
      this->value--;
  }
  else if (key == MENU_ACTION_RIGHT)
  {
    if (this->value + 1 <= this->max)
      this->value++;
  }
}

// BoolMenuItem ---------------------------------------------------------------

BoolMenuItem::BoolMenuItem(const char * name, bool defaultValue)
  : MenuItem(name)
{
  value = defaultValue;    
}

void BoolMenuItem::ShowEditValue(Display & display)
{
  char * line = NewEmptyLine(display.cols);

  line[0] = 127;
  line[display.cols - 1] = 126;

  if (this->value)
  {
    strncpy(line + display.cols - 4, "On", 2);    
  }
  else
  {
    strncpy(line + display.cols - 5, "Off", 3);
  }

  display.Print(0, 1, line);

  DeleteLine(line);
}

int BoolMenuItem::GetType() 
{
  return MENU_TYPE_BOOL;
}

void BoolMenuItem::HandleAction(int key)
{
  if (key == MENU_ACTION_LEFT || key == MENU_ACTION_RIGHT) 
  {
    value = !value;
  }
}

// SubMenuItem ----------------------------------------------------------------

SubMenuItem::SubMenuItem(const char * name, BaseMenuItem ** items, int count) : BaseMenuItem(name)
{
  this->items = items;
  this->count = count;
}

int SubMenuItem::GetCount()
{
  return this->count;
}

BaseMenuItem * SubMenuItem::GetItem(int index)
{
  return this->items[index];
}

int SubMenuItem::GetType() 
{
  return MENU_TYPE_SUB;
}

// ActionMenuItem -------------------------------------------------------------

ActionMenuItem::ActionMenuItem(const char * name, int actionId) : BaseMenuItem(name)
{
  this->actionId = actionId;
}

int ActionMenuItem::GetActionId()
{
  return this->actionId;
}

int ActionMenuItem::GetType() 
{
  return MENU_TYPE_ACTION;  
}

// MainMenu -------------------------------------------------------------------

void MainMenu::ShowBack(int row, bool isSelected)
{
  char * line = NewEmptyLine(display.cols);
  if (isSelected)
  {
    line[0] = SELECTION_CHAR;  
  }
  strncpy(line + 2, TEXT_BACK, TEXT_BACK_LEN);
  display.Print(0, row, line);

  DeleteLine(line);
}

void MainMenu::Show()
{  
  if (editedItem == NULL)
  {
    bool isSubMenu = stackTop > 0;

    for (int i = 0; i < display.rows; i++)
    {
      if (isSubMenu && i + firstItem == itemStack[stackTop]->GetCount())
      {
        ShowBack(i, i + firstItem == selectedItem);
      }
      else
      {
        itemStack[stackTop]->GetItem(i + firstItem)->ShowListItem(display, i, i + firstItem == selectedItem);
      }
    }
  }
  else
  {
    editedItem->ShowEditMode(display);
  }
}

MainMenu::MainMenu(Display & display, BaseMenuItem ** items, int count) : 
  SubMenuItem(NULL, items, count),
  display(display)
{
  // Assuming initially 5 levels of menu max
  itemStack = new SubMenuItem * [MAX_MENU_LEVELS];
  positionStack = new short[MAX_MENU_LEVELS * 2];
  stackTop = 0;
  itemStack[stackTop] = this;
}

MainMenu::~MainMenu()
{
  delete itemStack;
  delete positionStack;
}

int MainMenu::Action(int keyAction)
{  
  int lastItemId = stackTop > 0 ? itemStack[stackTop]->GetCount() : itemStack[stackTop]->GetCount() - 1;

  if (editedItem == NULL)
  {
    if (keyAction == MENU_ACTION_UP || keyAction == MENU_ACTION_DOWN) 
    {      
      if (keyAction == MENU_ACTION_UP)
        this->selectedItem--;
      else
        this->selectedItem++;

      if (selectedItem < 0)
        selectedItem = lastItemId;      
      if (selectedItem > lastItemId)
        selectedItem = 0;    

      // Adjust items in view

      while (firstItem > selectedItem)
        firstItem--;
      while (firstItem + display.rows - 1 < selectedItem)
        firstItem++;

      Show();
      return -1;
    }
    else if (keyAction == MENU_ACTION_SELECT) 
    {
      if (selectedItem == lastItemId && stackTop > 0) 
      {
        // Chosen Back

        stackTop--;
        selectedItem = positionStack[stackTop * 2];
        firstItem = positionStack[stackTop * 2 + 1];

        Serial.print("Restored sel/first: ");
        Serial.print(selectedItem);
        Serial.print("/");
        Serial.print(firstItem);
        Serial.print("\n");

        Show();
      }
      else
      {
        int type = itemStack[stackTop]->GetItem(selectedItem)->GetType();
        switch (type)
        {
          case MENU_TYPE_INT:
          {
            editedItem = (MenuItem *)itemStack[stackTop]->GetItem(selectedItem);
            Show();
            break;
          }
          case MENU_TYPE_BOOL:
          {
            editedItem = (MenuItem *)itemStack[stackTop]->GetItem(selectedItem);
            Show();
            break;
          }
          case MENU_TYPE_SUB:
          {
            Serial.print("Entering ");
            Serial.print(((SubMenuItem *)(itemStack[stackTop]->GetItem(selectedItem)))->GetName());
            Serial.print("\n");

            if (stackTop >= MAX_MENU_LEVELS - 1)
              return -1;

            Serial.print("Storing sel/first: ");
            Serial.print(selectedItem);
            Serial.print("/");
            Serial.print(firstItem);
            Serial.print("\n");

            positionStack[stackTop * 2] = selectedItem;
            positionStack[stackTop * 2 + 1] = firstItem; 

            itemStack[stackTop + 1] = ((SubMenuItem *)(itemStack[stackTop]->GetItem(selectedItem)));
            stackTop++;
            selectedItem = 0;
            firstItem = 0;

            Show();

            break;
          }
          case MENU_TYPE_ACTION:
          {
            int actionId = ((ActionMenuItem *)(itemStack[stackTop]->GetItem(selectedItem)))->GetActionId();
            return actionId;
          }
        }
      }
    }
  }
  else
  {
    if (keyAction == MENU_ACTION_LEFT || keyAction == MENU_ACTION_RIGHT || keyAction == MENU_ACTION_UP || keyAction == MENU_ACTION_DOWN) 
    {
      Serial.print("Item action\n");
      editedItem->HandleAction(keyAction);  
      Show();
    }
    else if (keyAction == MENU_ACTION_SELECT)
    {
      editedItem = NULL;
      Show();
    }
  }
  return -1;
}
share|improve this question
    
Let me guess... You are making heavy use of the String class...? – Majenko 2 days ago
    
@Majenko Did you look in the sources? I don't use String at all. – Spook 2 days ago
    
No, I was using my phone at the time. So that rules that common problem out then. Kudos to you for shunning String BTW ;) – Majenko 2 days ago
    
@Majenko I've got a project that uses String pretty heavily. Although I haven't experienced any issues, looks like I have some reading to do to get an understanding why all of a sudden I'm seeing that the specific class is frowned upon :) – stevieb 2 days ago
3  
Stack exchange sites really require that the information needed to understand the problem be included in the post itself. – Chris Stratton 2 days ago
up vote 10 down vote accepted

@Michel Keijzers provided overall answer.

And now probable cause:

char * GetEmptyLine(int width)
{
  char * line = new char[width + 1];

In the Menu.cpp. There is no single delete in the code at all. But so many calls of GetEmptyLine.

Using the Arduino String might have done less evil than this kind of memory leak.

share|improve this answer
3  
Oh my. This is how programmer ends when dealing with automatic memory-managed languages for too long :( But knowing that this is my fault in the end is a relief anyway :) – Spook 2 days ago

I wouldn't assume by default that it's a hardware problem.

It seems like a software problem, and there can be many reasons that the Arduino hangs, e.g.:

  • Too little stack space
  • Too little heap space
  • Pointer problems

The Arduino does not have safety against too little memory, that means it will overwrite memory -- causing lots of problems -- and hanging is a normal symptom.

What I suggest is:

  • Check how many global / local variables you have (and their size), especially arrays and instances of objects; strings can be memory consuming
  • Check that you don't have an overly deep stack trace
  • To check, you can print out the available memory in several places

For debugging statements, keep in mind that the debug strings can be cut in half just when the microprocessor hangs; maybe the print statements are still in the buffer but will not be sent, so minimize the (length of the) debug statements in such cases.

share|improve this answer
1  
There could be a hardware problem as well. Cheap clone board work in most cases, but cheap USB cables are bad. Since the display has a backlight, perhaps there is too much voltage drop in the USB cable. – Jot 2 days ago
1  
Good comment (I didn't say it IS a software problem, just not to rule it out). – Michel Keijzers 2 days ago

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.