Syntax is broken, but it compiled anyway

Greetings fellows,

I come tonight in search of assistance with the unholy abomination I have created.
The project consists of:
1x HCR04 ultrasound "tape measurer",
1x Generic rotary encoder;
1x 2004 LCD with i2c board;
2x PWM motor control modules
1x Arduino Nano

The premise is:
I need to control 2 motor modules with PWM signals, with a editable look-up table.
There has to be a readout on the LCD for currently selected power level [0-19] (from a 20x3 array with pre-defined values, which can be edited).

Selection of columns and modifying the contents is done via the rotary encoder - the first column of the array is power.
When on the first column, turning the encoder cycles between the rows of the array.
When on the second or third column, the rows don't cycle, but instead the value changes by rotating the encoder.
Pressing the encoder switches from first column to the second, then the third, and back to the first column, allowing one to edit the second and third column's contents for the "current" row.
The first column's contents shouldn't be editable.
Then the values of the second and third column for the current row get digitalWrite'd to 2 analog pins.

The problem is:
I've made a dog's dinner out of it.
It compiles fine, but nothing is printed out on the LCD, nor on the serial monitor.
I was looking through examples of the individual functions and I thought I could cobble together something that works, instead I'm here grasping at straws.

Here's the code:

#include <Wire.h> 
#include <HCSR04.h>
HCSR04 hc(6, 7);   // trig=5 , echo=6
#include <LiquidCrystal_I2C.h>
LiquidCrystal_I2C lcd(0x27,20,4); 
int pwmout1= 0;
int pwmout2= 1;
int const row=20;
int const col=3;
int powerlvl[row][col] = 
{ {0, 10, 10},
  {1, 13, 13},
  {2, 16, 16},
  {3, 19, 19},
  {4, 21, 21},
  {5, 25, 25},
  {6, 30, 30},
  {7, 35, 35},
  {8, 40, 40},
  {9, 45, 45},  
  {10, 50, 50},
  {11, 55, 55},
  {12, 60, 60},
  {13, 65, 65},
  {14, 70, 70},
  {15, 75, 75},
  {16, 80, 80},
  {17, 85, 85},
  {18, 90, 90},
  {19, 99, 99},
  };

// ints for encoder debounce////////////
enum PinAssignments {
  encoderPinA = 3,   // right
  encoderPinB = 2,   // left
  Button = 4         // press
};

int encoderPos = 0;  // a counter for the dial
unsigned int lastReportedPos = 1;   // change management
static boolean rotating = false;    // debounce management

// interrupt service routine vars
boolean A_set = false;
boolean B_set = false;


void setup() {
  //PWM OUT//////
  pinMode(pwmout1, OUTPUT);
  pinMode(pwmout2, OUTPUT);
////////////////////setup for encoder debounce////////////

  pinMode(encoderPinA, INPUT);
  pinMode(encoderPinB, INPUT);
  pinMode(Button, INPUT);
  // turn on pullup resistors
  digitalWrite(encoderPinA, HIGH);
  digitalWrite(encoderPinB, HIGH);
  digitalWrite(Button, HIGH);

  // encoder pin on interrupt 0 (pin 2)
  attachInterrupt(0, doEncoderA, CHANGE); 
  // encoder pin on interrupt 1 (pin 3)
  attachInterrupt(1, doEncoderB, CHANGE); 

  Serial.begin(9600);  // output

  lcd.init();
  lcd.backlight(); 
  }
    
void loop() 
{
//////////////loop for encoder debounce, and encoderPos to ++/-- ////////////

rotating = true;  // reset the debouncer

  if (lastReportedPos != encoderPos) {
   //   Serial.println(encoderPos); /// broken, doesnt print 
      lastReportedPos = encoderPos;
  }
  if (digitalRead(Button) == LOW )  {
   //encoderPos=0;
//////// cycle between colums on press <<<missing>>> ///////

 ///pwm out//////
int x = map(powerlvl[row][1], 10, 99, 0, 255);
 analogWrite(pwmout1, x);
 
int y = map(powerlvl[row][2], 10, 99, 0, 255);
 analogWrite(pwmout2, y);
  }
      lcd.setCursor(2,2);
      lcd.print(60 - hc.dist()); ///// flip measurment
      if(hc.dist() >= 30)       ////////alarm at 30
    {digitalWrite(8, HIGH); 
      }
    else
      {digitalWrite(8, LOW); 
        }
}
     
      
// Interrupt on A changing state
void doEncoderA() {
  // debounce
  if ( rotating ) delay (1);  // wait a little until the bouncing is done

  // Test transition, did things really change?
  if ( digitalRead(encoderPinA) != A_set ) { // debounce once more
    A_set = !A_set;

    // adjust counter + if A leads B
    if ( A_set && !B_set )
      encoderPos += 1;

    rotating = false;  // no more debouncing until loop() hits again
  }
}

// Interrupt on B changing state, same as A above
void doEncoderB() {
  if ( rotating ) delay (1);
  if ( digitalRead(encoderPinB) != B_set ) {
    B_set = !B_set;
    //  adjust counter - 1 if B leads A
    if ( B_set && !A_set )
      encoderPos -= 1;

    rotating = false;
  }
      
    //  need to somehow limit encoderPos to (0,99)
    //  powerlvl{[row][]} = encoderPos;
     
//////////////loop for LCD prints and row/col select////////////
      lcd.setCursor(1,0);
      lcd.print("P=");
      lcd.setCursor(3,0);
      lcd.print(powerlvl[encoderPos][0]);

      lcd.setCursor(6,0);
      lcd.print("B%=");
      lcd.setCursor(9,0);
      lcd.print(powerlvl[encoderPos][1]);
      
      lcd.setCursor(14,0);
      lcd.print("M%=");
      lcd.setCursor(17,0);
      lcd.print(powerlvl[encoderPos][2]);

      lcd.setCursor(1,2);
      lcd.print("encoderPos=");
      lcd.setCursor(12,2);
      lcd.print(encoderPos); /////////////////none of these print, WTF/////
  ///////// somehow add a cursor symbol to indicate selected column///////
  
     
  }

I'm pretty sure some of the functions dont belong wherever I've shoved them, thus breaking them.
Each individual part was tested by itself on the Nano and it worked. The issue arose when the separate sketches had to be merged into a single one.

As stated, it compiles and uploads just fine, but doesn't do anything

Any help would be greatly appreciated.

The best approach toward a complex project is the get all the pieces working individually.

Assuming you have already done that, add them together, one at a time, verifying proper operation as you go. If it stops working, go back and figure out what the problem is before adding yet another function.

In glancing through the code, I can see that the encoder routines cannot work as you intend.

  1. Variables shared with interrupt routines must be declared volatile
  2. delay() does not work in an interrupt.
  3. multibyte variables shared with interrupt routines must be protected against corruption by the interrupt, when accessed by the main program. Make a copy of the variable with the interrupts turned off.

I recommend to use a library, rather than write your own encoder routines.

1 Like

That's not syntax, that's semantics!

In one of my first IT courses I heard of the DWIM key:
Do What I Mean - not what I write!
Unfortunately this key was never found on any keyboard :frowning:

alright, maybe i should have pinned down some actual questions, so:

  1. why isnt the display or serial monitor showing anything despite the numerous .print routines?
  2. can anyone suggest a solution about adding a cursor and have it jump between columns on press?
  3. how do i translate "when this col is selected, and its not the first, change the current value by [encoderPos]" into code?
  4. im limited on memory space on the Nano, this sketch takes up over 50% of program memory. will including a library (like for the encoder) save space over a home-brew solution? I might have to make additions to it later and im worried about the available space.
  5. while i was testing the LCD by itself, a number was printed on it. whenever the number goes above 9 (two digits) it is displayed properly, however when it goes down to a single-digit number, the second digit stays displayed as a leftover "0". this gets worse if it's a 3-digit number or a negative one - there's 2 or 3 phantom digits that remain.

Click bait!

explain?

Broken syntax doesn’t compile

  1. Because the encoder routines don't work. As explained.
  2. Use cursor control
  3. fix the other problems first
  4. ditto
  5. Erase a field before you print on it.
int encoderPos = 0;  // a counter for the dial
unsigned int lastReportedPos = 1;   // change management

Why is one signed and the other unsigned?

void doEncoderB()
{
  if ( rotating ) delay (1);

DO NOT PUT delay() in an interrupt.

  // encoder pin on interrupt 0 (pin 2)
  attachInterrupt(0, doEncoderA, CHANGE);
  // encoder pin on interrupt 1 (pin 3)
  attachInterrupt(1, doEncoderB, CHANGE);

You should use 'digitalPinToInterrupt()' instead of interrupt numbers:

  // encoder pin on interrupt 0 (pin 2)
  attachInterrupt(digitalPinToInterrupt(encoderPinA), doEncoderA, CHANGE);
  // encoder pin on interrupt 1 (pin 3)
  attachInterrupt(digitalPinToInterrupt(encoderPinB), doEncoderB, CHANGE);
  pinMode(encoderPinA, INPUT);
  pinMode(encoderPinB, INPUT);
  pinMode(Button, INPUT);
  // turn on pullup resistors
  digitalWrite(encoderPinA, HIGH);
  digitalWrite(encoderPinB, HIGH);
  digitalWrite(Button, HIGH);

You should use INPUT_PULLUP instead of INPUT followed by digitalWrite(HIGH):

  pinMode(encoderPinA, INPUT_PULLUP);
  pinMode(encoderPinB, INPUT_PULLUP);
  pinMode(Button, INPUT_PULLUP);

an update: i did away with setting the encoder manually, instead i imported the EncButton.h library. that gives me Left and Right ticks and so on. now the problem is I want to call those left and right functions inside a few switch cases and the IDE spits out this error:

In function 'void setup()': Arrays_ldc_encoder2:49:29: error: 'myRight' was not declared in this scope enc.attach(RIGHT_HANDLER, myRight); ^~~~~~~ Arrays_ldc_encoder2:50:28: error: 'myLeft' was not declared in this scope enc.attach(LEFT_HANDLER, myLeft); ^~~~~~ exit status 1 'myRight' was not declared in this scope

it runs perfectly fine if I call them between the loop() and setup(), like in the example sketch, but i want to call it locally in the loop()/ switch...cases so i can change different values in each case.

#include <Wire.h>
#include <HCSR04.h>
#include <EncButton.h>
EncButton<EB_CALLBACK, 2, 3, 4> enc;   
HCSR04 hc(7, 8); /// trig=7 , echo=8
#include <LiquidCrystal_I2C.h>
LiquidCrystal_I2C lcd(0x27, 20, 4);
byte x = 0;
byte y = 0;
int pwmout1 = 5;
int pwmout2 = 6;
int const row = 20;
int const col = 3;
int rowSelect;
int alarm = 30;
int colNum;
char line1[21];
int powerlvl[row][col] =
{ {0,  0,  0},
  {1, 13, 13},
  {2, 16, 16},
  {3, 19, 19},
  {4, 21, 21},
  {5, 25, 25},
  {6, 30, 30},
  {7, 35, 35},
  {8, 40, 40},
  {9, 45, 45},
  {10, 50, 50},
  {11, 55, 55},
  {12, 60, 60},
  {13, 65, 65},
  {14, 70, 70},
  {15, 75, 75},
  {16, 80, 80},
  {17, 85, 85},
  {18, 90, 90},
  {19, 99, 99},


};

void setup() {
  pinMode(pwmout1, OUTPUT);
  pinMode(pwmout2, OUTPUT);


  enc.attach(RIGHT_HANDLER, myRight);
  enc.attach(LEFT_HANDLER, myLeft);
  enc.attach(PRESS_HANDLER, myPress);
  attachInterrupt(0, isr, CHANGE);
  attachInterrupt(1, isr, CHANGE);
  // Serial.begin(9600);  // output

  lcd.init();
  lcd.backlight();



}

void myPress() {
  colNum++;
  if (colNum == 3) colNum = 0;
}

void isr() {
  enc.tickISR();  // ticker

}



void loop()
{
  enc.tick();

  lcd.setCursor(10, 3); ////lcd.setCursor(col, row)
  lcd.print("  "); // clear digits on row 4, col 4,5,6 if they were left over
  lcd.setCursor(0, 3);
  lcd.print("Litri:");
  lcd.setCursor(6, 3);
  lcd.print(60 - hc.dist(), 1); ///// flip measurment; Serial.print (xxx, 0); for no decimals

  switch (colNum) {
    case 0: {
        void myRight();
        { rowSelect++;
        }
        void myLeft();
        { rowSelect--;
        }

        lcd.setCursor(0, 0);
        lcd.print("o");
        lcd.setCursor(6, 0);
        lcd.print(" ");
        lcd.setCursor(13, 0);
        lcd.print(" ");
        lcd.setCursor(1, 0);

        lcd.print("P=");
        lcd.setCursor(3, 0);
        lcd.print(powerlvl[rowSelect][0]);

        lcd.setCursor(7, 0);
        lcd.print("B%=");
        lcd.setCursor(10, 0);
        lcd.print(powerlvl[rowSelect][1]);

        lcd.setCursor(14, 0);
        lcd.print("M%=");
        lcd.setCursor(17, 0);
        lcd.print(powerlvl[rowSelect][2]);

        lcd.setCursor(0, 0);
        lcd.print("o");
        lcd.setCursor(6, 0);
        lcd.print(" ");
        lcd.setCursor(13, 0);
        lcd.print(" ");
        lcd.setCursor(1, 0);

        lcd.print("P=");
        lcd.setCursor(3, 0);
        lcd.print(powerlvl[rowSelect][0]);

        lcd.setCursor(7, 0);
        lcd.print("B%=");
        lcd.setCursor(10, 0);
        lcd.print(powerlvl[rowSelect][1]);

        lcd.setCursor(14, 0);
        lcd.print("M%=");
        lcd.setCursor(17, 0);
        lcd.print(powerlvl[rowSelect][2]);


        x = map((powerlvl[rowSelect][1]), 0, 99, 0, 255);
        analogWrite(pwmout1, x);

        y = map((powerlvl[rowSelect][2]), 0, 99, 0, 255);
        analogWrite(pwmout2, y / 5);

        break;
      }
    case 1: {



        break;
      }
    case 2: {


        break;
      }


  }


  if (hc.dist() >= alarm)      ////////alarm at 30
  { digitalWrite(9, HIGH);
    lcd.setCursor(12, 3);
    lcd.print("NALIVAI!");
  }
  else
  { digitalWrite(9, LOW);
    lcd.setCursor(12, 3);
    lcd.print("        ");

  }
  if (rowSelect <= 9)    //////// clear left over digits below powerlvl 9
  { lcd.setCursor(2, 2);
    lcd.print("  ");
    lcd.setCursor(4, 0);
    lcd.print("  ");
    lcd.setCursor(12, 0);
    lcd.print("  ");
    lcd.setCursor(19, 0);
    lcd.print("  ");
  }





  if (rowSelect == 0)    //////// clear left over digits at powerlvl 1
  {
    lcd.setCursor(4, 0);
    lcd.print("   ");
    lcd.setCursor(11, 0);
    lcd.print("  ");
    lcd.setCursor(18, 0);
    lcd.print("  ");

    lcd.setCursor(2, 2);
    lcd.print("  ");
  }


}

how do I call the void myRight(); and void myLeft(); properly in the loop() cases????
do i need to explain what intended the code to do?

and if you havent guessed by now, im a f*cking imbecile, so dumb down any responses to beginner levels please.

You are getting those errors because (it appears) you put the function definitions in the case statement. ???????? C/C++ does not allow to define a function within a function.

        void myRight();
        { rowSelect++;
        }
        void myLeft();
        { rowSelect--;
        }

Proper definitions for these functions would be:

        void myRight()
        { 
           rowSelect++;
        }
        void myLeft()
        { 
           rowSelect--;
        }

In the code you would call them by the statement myRight(); or myLeft();.

Also, you should make sure rowSelect is initialized and check the bounds whenever you increment or decrement it; otherwise you may print garbage when accessing the powerlvl table.

You should review fundamental C++ programming.

the semicolons only change the amount of errors.

In function 'void setup()': Arrays_ldc_encoder2:48:29: error: 'myRight' was not declared in this scope enc.attach(RIGHT_HANDLER, myRight); ^~~~~~~ Arrays_ldc_encoder2:49:28: error: 'myLeft' was not declared in this scope enc.attach(LEFT_HANDLER, myLeft); ^~~~~~ In function 'void loop()': Arrays_ldc_encoder2:88:9: error: a function-definition is not allowed here before '{' token { (rowSelect++); ^ Arrays_ldc_encoder2:91:9: error: a function-definition is not allowed here before '{' token { rowSelect--; ^ exit status 1 'myRight' was not declared in this scope

and theres no argument that i need to brush up on the basics, but this "weekend" project is on its way to make me give up all together and chuck everything in the can.

Like I said. C/C++ does NOT allow you to define a function within a function. Move those function definitions outside of loop() and any other functions! In the loop() function you would call them by the statement myRight(); or myLeft(); .

1 Like

Because you are trying to run before you can walk. I suggest you start again, but this time break the project into lots of small parts, which you can write, test and debug individually. Build it up little bit by little bit. Right now you are being swamped with a tsunami of errors. This is telling you your approach is wrong.

I'm not going to start over because it's already 90% done, it's missing a single feature.

If I break this simple project down I get the example sketches from the libraries.

What gets in my way is that I don't remember exactly how to write the code out, exactly like Todd said. It's been at least 15 years since my last programing class.
BTW #11 and #13 were the most informative out of all the replies so far - the man told me "this is wrong, it should look like that, put it there". Now it works, and I've relearned where/how to define functions, and where/how to call them.
Don't get me wrong, I appreciate the old sage advice, but I don't want to deal with "programming 101" for a week just to realize I had an extra goddamn semicolon and I was trying to call a function by defining it.

And I bet the last bit of code that I need wont be longer than 5 lines, I just don't remember enough to put the concept into proper code.

@valkirov: See the above post from @jremington . It's not just me telling you that.

Of course, if you want to ignore our advice, that's up to you. But you're just making things hard for yourself. Bye.

Good advice is welcome, however I wanted instructions like "do this, change that". I need to get this thing running yesterday, I don't have the time to delve into semantics. Simple and straightforward, not "maybe read up on x y z", if you know the answer just say it, wtf.

I'll delete this thread later tonight when I get off work. I don't think I'll derive any further benefit from it.

OK, well you were probably expecting too much, then. It looks like there are multiple problems with your code, which means it isn't going to be a simple "do this, change that" solution. Sorry.

"Yesterday" is your need, not ours. Nobody's going to come in and wave a magic wand over your project and fix it for you unless you offer to pay them some money.

1 Like

That’s kind of selfish. This is a public world wide forum and you are not the only one who benefits from this thread.

It’s not just for you to derive benefit, then scuttle. Ppl will read this and benefit themselves in ways you have not chosen to let yourself be.

There are probably 50 people who come here (or vector in on a google jet), find what they need and leave w/o ever originating or posting a question for every one that actually posts to a thread.

I wouldn’t play here unless I learned something almost every day. Pleas consider just leaving this whole “unholy dog’s dinner” as is.

No one knows who you are, no one cares, there’s no reason to erase or edit or obscure this.

The advice has been very good, if not what you were looking to accomplish here dropping by. iit is because this is a particular kind of forum, one where the idea is to move ppl along the learning curve.

a7