Problems with sketch

Hi all,

I’m Nick, I’m dutch, so excuse me for my bad English. I’m working on a project with 2 arduino nano’s. I have a kind of master slave setup. De slave is a Motor controller working with 6 P-channel Fets on the PWM Pins.

De master just has a lcd display with 3 buttons and sends commands over serial to the second arduino. After adding a subroutine, my setup() suddenly doesn’t run. Deleting the subroutine makes everything fine again. It doesn’t mather if I remove Menu0 or Menu1 subroutine. On the compiler I don’t get anny errors.

I can’t find what is causing the problem, maybe someone could help me out?

I’m not a pro at programming, so if anyone has some tips, I would appreciate it.

#include <Wire.h>
#include <Adafruit_SSD1306.h>
#include <Adafruit_GFX.h>

// OLED display TWI address
#define OLED_ADDR   0x3C

Adafruit_SSD1306 display(-1);

#if (SSD1306_LCDHEIGHT != 64)
#error("Height incorrect, please fix Adafruit_SSD1306.h!");
#endif


const int knopR = 2;
const int knopL = 3;
const int PushButton = 4; 
int x,y, flag; 
const byte numChars = 32;
char receivedChars[numChars]; 
volatile unsigned int encoder0Pos = 0;
volatile int counter = 0;
volatile int lastEncoding = 0;
volatile int oldstate = 0;
volatile int EncoderState = 0;
volatile int Menu = 0; 
String Tekst = "";
volatile int M1Speed = 0;
volatile int M2Speed = 0;
volatile int M3Speed = 0;
volatile int M4Speed = 0;
volatile int M5Speed = 0;
volatile int M6Speed = 0;
volatile int knop = 0;

void doEncoder(){
  int currMillis = millis();
  if (currMillis - lastEncoding > 50) {
  
  if (digitalRead(knopL) == LOW) {
    if (encoder0Pos == 0) {
      encoder0Pos = 0;
    }
    else{
    encoder0Pos--;
    
    }
  }
  if (digitalRead(knopR) == LOW) {
    if (encoder0Pos == 255) {
      encoder0Pos = 255;
    }
    else{
    encoder0Pos++;
  }
  }
   lastEncoding = currMillis;
  }
}

void MENU0() {
  display.setTextSize(3);
  display.clearDisplay();
  display.setCursor(10,30);
  display.print("Motor1");
  display.display();
  Serial.println("Motor1");
  delay(1000);
  encoder0Pos = 1;
  Menu = 1;
  return;
}

void MENU1(){
if (knop==LOW) {
 if (encoder0Pos == 1){
      Tekst = "Speed M 1";
      encoder0Pos = M1Speed;
    }
    if (encoder0Pos == 2){
      Tekst = "Speed M 2";
      encoder0Pos = M2Speed;
    }
    if (encoder0Pos == 3){
      Tekst = "Speed M 3";
      encoder0Pos = M3Speed;
    }
    if (encoder0Pos == 4){
      Tekst = "Speed M 4";
      encoder0Pos = M4Speed;
    }
    if (encoder0Pos == 5){
      Tekst = "Speed M 5";
      encoder0Pos = M5Speed;
    }
    if (encoder0Pos == 6){
      Tekst = "Speed M 6";
      encoder0Pos = M6Speed;
    }
    display.setTextSize(2);
    display.clearDisplay();
    display.setCursor(10,10);
    display.print(Tekst);
    display.setTextSize(4);
    display.setCursor(30,30);
    EncoderState = encoder0Pos;
    if(encoder0Pos == 0) {
      display.print("Off");
    }
    else {
      display.print(encoder0Pos);
    }
    display.display();
    Serial.println(Tekst);
    delay(1000);
    Menu = 2;
} else {
if(EncoderState != encoder0Pos) {
    if(encoder0Pos >= 7) {
      encoder0Pos = 7;
      display.setTextSize(3);
      display.clearDisplay();
      display.setCursor(10,10);
      display.print("Reset  All");
      display.display();
      Serial.println("Reset all");
      delay(100);
      EncoderState = 7;
    }
    if(encoder0Pos ==6) {
      display.setTextSize(3);
      display.clearDisplay();
      display.setCursor(10,30);
      display.print("Motor6");
      display.display();
      Serial.println("Motor6");
      delay(100);
      EncoderState = 6;    
    }
    if(encoder0Pos == 5) {
      display.setTextSize(3);
      display.clearDisplay();
      display.setCursor(10,30);
      display.print("Motor5");
      display.display();
      Serial.println("Motor5");
      delay(100);
      EncoderState = 5;    
    }
    if(encoder0Pos == 4) {
      display.setTextSize(3);
      display.clearDisplay();
      display.setCursor(10,30);
      display.print("Motor4");
      display.display();
      Serial.println("Motor4");
      delay(100);  
      EncoderState = 4;
    }  
    if(encoder0Pos == 3) {
      display.setTextSize(3);
      display.clearDisplay();
      display.setCursor(10,30);
      display.print("Motor3");
      display.display();
      Serial.println("Motor3");
      delay(100);    
      EncoderState = 3;
    }
    if(encoder0Pos == 2) {
      display.setTextSize(3);
      display.clearDisplay();
      display.setCursor(10,30);
      display.print("Motor2");
      display.display();
      Serial.println("Motor2");
      delay(100);    
      EncoderState = 2;
    }
    if(encoder0Pos == 1) {
      display.setTextSize(3);
      display.clearDisplay();
      display.setCursor(10,30);
      display.print("Motor1");
      display.display();
      Serial.println("Motor1");
      delay(100);    
      EncoderState = 1;
    }
  }
} 
}

void setup() {
  // initialize and clear display
  Serial.begin(9600);
  delay(2000);
  display.begin(SSD1306_SWITCHCAPVCC, OLED_ADDR);
  display.clearDisplay();
  display.display();

  // display a line of text
  display.setTextSize(2);
  display.setTextColor(WHITE);
  display.setCursor(10,0);
  display.print("Jeugd de Veldmaat");
  

  // update display with all of the above graphics
  display.display();
  delay(2000);


  Serial.println("M Programmer");
  display.clearDisplay();
  display.setCursor(10,0);
  display.print("Searching devices");
  display.display();
  delay(2000);
  display.setTextSize(4);
  for (int i = 0; i <= 128; i++) {
  display.setCursor(i,15);
  display.print(".");
  display.display();
  i=i+9;
  }
  display.clearDisplay();
  display.setTextSize(2);
  display.setCursor(0,0);
  static byte ndx = 0;
  char endMarker = '\n';
  char rc;


  while (Serial.available() > 0) {
    rc = Serial.read();
    if (rc != endMarker) {
      receivedChars[ndx] = rc;
      ndx++;
      if (ndx >= numChars) {
        ndx = numChars - 1;
      }
    }
    else {
      receivedChars[ndx] = '\0'; // terminate the string
      ndx = 0;
    }
  }

  Serial.println(receivedChars);
  String Commando = receivedChars;
  Serial.println(Tekst);
  delay(1000);
  if (Commando.indexOf("P")>-1) {
    display.print("P-Channel MotordriveConnected!");
    display.display();  
        }
  else {
    if (Commando.indexOf("N")>-1) {
      display.print("N-Channel MotordriveConnected!");
      display.display();  
          }
    else {
      display.print("No device found!");
      display.display();
      flag = 2;
    }
  }
  
  pinMode(PushButton, INPUT_PULLUP);
  pinMode(knopR, INPUT_PULLUP);
  pinMode(knopL, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(knopR), doEncoder, CHANGE);  // encoder pin on interrupt 0 - pin 2
  attachInterrupt(digitalPinToInterrupt(knopL), doEncoder, CHANGE);
}


void loop() {
  Hervat:
knop = digitalRead(PushButton);
if (flag == 2){
  goto Hervat;
}
if(Menu==0) {
  if(knop == LOW){
    MENU0();
  }
}
if(Menu==1) {
  MENU1();
}
 

}

my setup() suddenly doesn't run.

What is your evidence for this ?

My display doesn't work and i get no serial data in the serial monitor.

all your variables associated with millis() need to be unsigned long, not int.

Your program will not compile for me. The screen height error is triggered

I changed the variables to unsigned long, no difference.

Screen error height, you have to change ssd1306 file to the correct height (64)

I really don't understand the problem with this sketch, it used to work but it seems to get stuck somewhere now. I tried recreating the file. First I did the declaration, then added setup, then added loop. I inserted subroutine MENU0. They all worked as they should. After adding subroutine MENU1 nothing works. Compiler has no errors.

Perhaps to many lines? Or a variable over writing? Is there any debugging software to see what is causing the problem with step through?

No, there is no single step debugger. You need to sprinkle Serial.print() statements throughout your code to see where things go wrong.

I decided to rewrite the code and trying to structure it a little better. Rewriting will, in my opinion, go faster then searching what is going wrong. What do you guys think of my new code? It is working fine, but not ready yet. If anybody has some suggestions what I could improve, please share it with me.

#include <Wire.h>
#include <Adafruit_SSD1306.h>
#include <Adafruit_GFX.h>

// OLED display TWI address
#define OLED_ADDR   0x3C

Adafruit_SSD1306 display(-1);

#if (SSD1306_LCDHEIGHT != 64)
#error("Height incorrect, please fix Adafruit_SSD1306.h!");
#endif


const int knopR = 2;
const int knopL = 3;
const int PushButton = 4; 
int  i, flag;
byte temp, oldtemp = 1; 
const byte numChars = 32;
char receivedChars[numChars]; 
volatile int counter = 0;
volatile int Menu = 0; 
String Tekst, displayTekst = "";
volatile int M1Speed = 0;
volatile int M2Speed = 0;
volatile int M3Speed = 0;
volatile int M4Speed = 0;
volatile int M5Speed = 0;
volatile int M6Speed = 0;

void Right()  {
 static unsigned long last_interrupt_time = 0;
 unsigned long interrupt_time = millis();
 // If interrupts come faster than 200ms, assume it's a bounce and ignore
 if (interrupt_time - last_interrupt_time > 200)
 {
    if (Menu == 1){
      if (temp >= 7){
        temp = 7; 
      }else{
        temp++;
      }
    }
    if (Menu == 2) {
      if (temp >= 255){
        temp = 255;
      }else{
        temp++;
      }
    }
    Serial.println(temp);
    }
 last_interrupt_time = interrupt_time;
}

void Left() {
  static unsigned long last_interrupt_time = 0;
 unsigned long interrupt_time = millis();
 // If interrupts come faster than 200ms, assume it's a bounce and ignore
 if (interrupt_time - last_interrupt_time > 200)
 {
    if (Menu == 1){
      if (temp <= 1){
        temp = 1; 
      }else{
        temp--;
      }
    }
    if (Menu == 2) {
      if (temp <= 0){
        temp = 0;
      }else{
        temp--;
      }
    }
    Serial.println(temp);
    }
 last_interrupt_time = interrupt_time;
}

void MENU0(){
    displayTekst = "Motor1";
    Serial.println(displayTekst);
    display.setTextSize(3);
    display.clearDisplay();
    display.setCursor(10,30);
    display.print(displayTekst);
    display.display();
    temp = 1;
    Menu = 1;
}

void MENU1(){
    if (digitalRead(PushButton) == LOW){
      if (temp == 1) {
        displayTekst = "Speed M 1";
        Tekst = M1Speed;
      }
      if (temp ==2) {
        displayTekst = "Speed M 2";
        Tekst = M2Speed;
      }
      if (temp == 3) {
        displayTekst = "Speed M 3";
        Tekst = M3Speed;
      }
      if (temp ==4) {
        displayTekst = "Speed M 4";
        Tekst = M4Speed;
      }
      if (temp == 5) {
        displayTekst = "Speed M 5";
        Tekst = M5Speed;
      }
      if (temp ==6) {
        displayTekst = "Speed M 6";
        Tekst = M6Speed;
      }
      if (temp ==7) {
        displayTekst = "Reset";
      }
      display.setTextSize(2);
      display.clearDisplay();
      display.setCursor(10,10);
      if (temp<=6){
          display.print(displayTekst);
          Serial.println(displayTekst);
          display.setTextSize(4);
        display.setCursor(30,30);
        if(Tekst == 0){
          display.print("Off");
        }else{
          display.print(Tekst);
        }
      }else{
        display.setTextSize(3);
        display.print(displayTekst);
      }
      display.display();
      Serial.println(Tekst);
      delay(1000);
      Menu = 2;
    }else{
      if (temp != oldtemp){
        if (temp == 1) {
          displayTekst = "Motor1";
          oldtemp = 1;
        }
        if (temp ==2) {
          displayTekst = "Motor2";
          oldtemp = 2;
        }
        if (temp == 3) {
         displayTekst = "Motor3";
         oldtemp = 3;
        }
        if (temp ==4) {
          displayTekst = "Motor4";
          oldtemp = 4;
        }
        if (temp == 5) {
          displayTekst = "Motor5";
          oldtemp = 5;
        }
       if (temp ==6) {
          displayTekst = "Motor6";
          oldtemp = 6;
        }
        if (temp ==7) {
          displayTekst = "Reset All";
          oldtemp = 7;
       }
       display.setTextSize(3);
       display.clearDisplay();
       if (temp <= 6) {
        display.setCursor(10,30);
       } else {
        display.setCursor(10,10);
       }
       display.print(displayTekst);
       display.display();
       Serial.println(displayTekst);
      };
    }
}

void MENU2(){
  if (digitalRead(PushButton) == LOW){
    if (oldtemp == 1){
      Menu = 1;
      delay(1000);
      oldtemp = 0;
      temp = 1;
    }
     if (oldtemp == 2){
      Menu = 1;
      delay(1000);
      oldtemp = 0;
      temp = 2;
    }
     if (oldtemp == 3){
      Menu = 1;
      delay(1000);
      oldtemp = 0;
      temp = 3;
    }
     if (oldtemp == 4){
      Menu = 1;
      delay(1000);
      oldtemp = 0;
      temp = 4;
    }
     if (oldtemp == 5){
      Menu = 1;
      delay(1000);
      oldtemp = 0;
      temp = 5;
    }
     if (oldtemp == 6){
      Menu = 1;
      delay(1000);
      oldtemp = 0;
      temp = 6;
    }
  }
}

void setup() {
  // initialize and clear display
  Serial.begin(9600);
  delay(2000);
  display.begin(SSD1306_SWITCHCAPVCC, OLED_ADDR);
  display.clearDisplay();
  display.display();

  // display a line of text
  display.setTextSize(2);
  display.setTextColor(WHITE);
  display.setCursor(10,0);
  display.print("Jeugd de Veldmaat");
  

  // update display with all of the above graphics
  display.display();
  delay(2000);


  Serial.println("M Programmer");
  display.clearDisplay();
  display.setCursor(10,0);
  display.print("Searching devices");
  display.display();
  delay(2000);
  display.setTextSize(4);
  for (int i = 0; i <= 128; i++) {
  display.setCursor(i,15);
  display.print(".");
  display.display();
  i=i+9;
  }
  display.clearDisplay();
  display.setTextSize(2);
  display.setCursor(0,0);
  static byte ndx = 0;
  char endMarker = '\n';
  char rc;
  String Commando;


  while (Serial.available() > 0) {
    Commando = Serial.readStringUntil('\n');
      }

  

  delay(1000);
  if (Commando == "P") {
    display.print("P-Channel MotordriveConnected!");
    display.display();  
        }
  else {
    if (Commando == "N") {
      display.print("N-Channel MotordriveConnected!");
      display.display();  
          }
    else {
      display.print("No device found!");
      display.display();
      flag = 2;
    }
  }
  
  pinMode(PushButton, INPUT_PULLUP);
  pinMode(knopR, INPUT_PULLUP);
  pinMode(knopL, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(knopR), Right, LOW);  // encoder pin on interrupt 0 - pin 2
  attachInterrupt(digitalPinToInterrupt(knopL), Left, LOW);
}




void loop() {
  Hervat:

if (flag == 2){
  goto Hervat;
}
if(Menu==0) {
  if( digitalRead(PushButton) == LOW){
    MENU0();
  }
}
if(Menu==1) {
  MENU1();
}
if(Menu==2) {
  MENU2(); 
}

}

If anybody is interested in the whole project, please let me know. I will post it here on the forum. Tank you guys for the help so far.

Get rid of the goto so that you don;t get into the bad habit of using it To stop the program in its tracks use something like this

  if (flag == 2)
  {
    while(true);
  }

Why do all of the MxSpeed variables have a value of zero ?

const int knopR = 2;
const int knopL = 3;
const int PushButton = 4;

Why int when they could be byte ? Save memory when you can

Do you really need to use interrupts to read the inputs ?

    Serial.println(temp);

Serial.print() does not work in an ISR because interrupts are disabled

UKHeliBob: Get rid of the goto so that you don;t get into the bad habit of using it To stop the program in its tracks use something like this

  if (flag == 2)
  {
    while(true);
  }

That'll do it alright.

a7

I replaced the goto loop and changed the variables in byte. De serial print in the ISR are for debugging purpose, they will get removed when the program is finished. The motor variables will be read in serial from the second arduino.

The purpose of this project:

I have build in 1 arduino with 6 N channel fets and 2 arduino's with 6 P channel fets each. The code here in the topic is for a arduino that I can connect with ethernet cable on the 3 motor boards so I can set the speeds of the motors individual per motor. I want to get rid of using my laptop to configure the speeds and allow people with 0 programming experience to set the speeds of the motors.

De serial print in the ISR are for debugging purpose, they will get removed when the program is finished.

They won't work reliably inside the ISR whatever their purpose and if they are for debugging they must be reliable

The motor variables will be read in serial from the second arduino.

Wherever the values come from it would be sensiible to make the variables the appropriate data type

UKHeliBob: const int knopR = 2; const int knopL = 3; const int PushButton = 4;

Why int when they could be byte ? Save memory when you can

They all get optimized away by the compiler so you save nothing by making those byte vs. int.

They all get optimized away by the compiler so you save nothing by making those byte vs. int.

It is surely better to get into the habit of using appropriately sized variables whether or not they are optimised away because they are declared as const

Again I'm getting troubles with my program, added a few lines of codes, sketch size is 17904 bytes. No problems when compiling, no problems with uploading. Running the sketch on my arduino, it doesn't handle my serial commands I send. Could it be that I'm using to much SRAM?

Only you know what your new sketch is doing since you did not post it.