[SOLVED]My mystifying menu

Using Arduino Programming Questions
1

I’ve chased this critter for two days and now concede defeat. The attached sketch, comprising the menu navigation for my project, shows the most annoying erratic behavior. I deemed it rather long to put in code tags. If disabused of this notion I'll add a post with code tags used. Processor and development: Arduino Uno; Arduino IDE 1.8.2.

The menu has one level. After moving up or down to select a function/parameter the pushbutton on the encoder toggles a bool switch to allow you to manipulate that function or parameter. Pushing again toggles the menu back to up/down operation.

The part I’m having trouble with is the first two menu selections. Depending what code elsewhere is added/removed/modified, the display for these two can change. This can be as innocous as adding or commenting out a serial.print() statement. The second screen, “Mode” is usually well behaved, but not always. The first, “Current Position”, is the most problematic.

Depending what was changed, and there’s no rhyme or reason I can see, the second line of text will only print two, five, or some other incomplete number of characters. Sometimes the top line is also truncated, sometimes absent, sometimes correct. The various Serial.print were added to debug but they weren’t much help because whatever is sent to the LCD (1602) is duplicated on the serial monitor.

Along with this weirdness, traversing up and down through the options can be erratic or not. Erratic is defined as turning the rotary encoder CW, which is supposed to go ‘down’ the menu and having the selection - do nothing, go up, or go down. The reverse, CCW is also true. The encoder code is my own, proved correct, and rechecked, in a separate sketch. I have observed it operates much better with all Serial.print() statements out of the picture. Would driving this with an intrerrupt improve matters? Also, I’m using A0-A2 for the encoder inputs because the other side of the board was getting crowded.

An examination will show the code is very much a work in progress and there are functions yet to be added but I want to nail down the menu navigation before continuing.

Any clues to what is going on here are much appreciated.

montage1.png

turret_1.ino (10.2 KB)

montage1.png

2

Looks like you're running out of memory. Put a simple memory-free function in and test the free memory at the end of setup().

Look for tutorials on how to use the F() macro. That will get you a useful improvement in space. (No I did not look at the code.)

3

I noticed after uploading that only part of the file made it, I presume since the project was separated into tabs. This upload consolidates both into one file. Anyway, I added a small function to return free ram which gave the value, 999. The compiler now reports:

6012 program bytes used; 1042 bytes global vars used

turret_2.ino (15.2 KB)

4

I looked at your new code. There's nothing obviously wrong. No large arrays, no long delays. It all looks pretty sensible and normal.

I don't have your hardware so I can't run the code myself. Without observing it actually running, there's nothing more I can suggest.

5

No large arrays

But, the commented out code does waste SRAM, uselessly.

The non-commented code also wastes SRAM. There is no reason to have the turret position text or the index mode text in SRAM. Use PROGMEM to keep it out of SRAM.

6

MorganS:
I looked at your new code. There's nothing obviously wrong. No large arrays, no long delays. It all looks pretty sensible and normal.

Thanks, that's reassuring. It looked reasonable to me but it's good to have a second opinion.

7

PaulS:
But, the commented out code does waste SRAM, uselessly.

Hmmm. I was of the understanding that the compiler eliminated comments and they're not included in the final product.

PaulS:
The non-commented code also wastes SRAM. There is no reason to have the turret position text or the index mode text in SRAM. Use PROGMEM to keep it out of SRAM.

Off to research PROGMEM. Thanks.

8

Hmmm. I was of the understanding that the compiler eliminated comments and they're not included in the final product.

That is true. However, your problems happen when that code is NOT commented out. Or else I missed something.

9

I've made another observation. If I add a call to menuNavigation() at the end of setup() the desired top line text, Current Position , does appear and reappears, as it should, after moving to other selections. The bottom text however never appears. ???

10

Depending what code elsewhere is added/removed/modified, the display for these two can change. This can be as innocous as adding or commenting out a serial.print() statement. … Depending what was changed, and there's no rhyme or reason I can see,

This sort of thing is usually caused by code stomping over memory that it doesn't own. For instance, if your sprintf buffers are not big enough, writing stuff to the output char array can overwrite what's in the next bit of memory - often another char array containing static text.

Another cause of this is not nul-terminating your strings properly. eg

[nobbc]char someText[10] = "Some Text.";[/nobbc]

the array isn't big enough to hold the final '\0' that needs to be there, and the compiler assumes that this is what you meant to do.

Another cause is off-by-one bugs. eg, this snippet to fill a string full of 'x'es:

char foo[] = "Foo!";
char *p = foo;
while(*p++) {
  *p = 'x';
}
11

PaulMurrayCbr:
Another cause is off-by-one bugs. eg, this snippet to fill a string full of 'x'es:

char foo[] = "Foo!";

char *p = foo;
while(*p++) {
 *p = 'x';
}

At last my life can continue (sobbing). It's a very close cousin of PaulMurrayCbr's example. And, unsurprisingly, originated in a (seemingly) innocent section of code. Here's the little miscreant.

 if (EEPROM.read(50) != 255) { // address has been used so load parameter[] data from there
    Serial.println("eeprom load");
    //for (byte i = eepromLctn; i < eepromLctn + noOfMenuItems_s; i++) {
     // EEPROM.get(i, textAndParms[i].parmVal);
   // }

    for (byte i = 0; i < noOfMenuItems_s; i++) {
      EEPROM.get(i + eepromLctn, textAndParms[i].parmVal);
    }
  }

The active for() loop solves the issue with the commented one. Which is, index variable 'i' initialized to 50, ie. eepromLctn, to read the EEPROM. No trouble there, however, this - textAndParms.parmVal[eye]*- is a BIG problem since it resolves to 50 at minimum. Since there are only 14 menu items to begin with the oft-mentioned memory stomping is guaranteed.

Thank you all!

  • forum software workaround
12

Good job finding the bug.

13

PaulS:
Good job finding the bug.

Thanks. It's surprising sometimes what a bit of outside observation can do! BTW, how does one mark a thread [SOLVED]? I tried to edit the title but it didn't go through.

14

dougp:
Thanks. It's surprising sometimes what a bit of outside observation can do! BTW, how does one mark a thread [SOLVED]? I tried to edit the title but it didn't go through.

The title of a reply? Or the title of the initial thread?

15

PaulS:
The title of a reply? Or the title of the initial thread?

Initial thread, so it shows on the topic list of threads.

16

dougp:
Initial thread, so it shows on the topic list of threads.

I don't know what the problem is then. Other people are able to modify their thread titles.

You could use the Report to Moderator link, and ask a moderator to do it.

17

Which is, index variable 'i' initialized to 50, ie. eepromLctn, to read the EEPROM. No trouble there, however, this - textAndParms.parmVal[eye]*- is a BIG problem since it resolves to 50 at minimum.

Hah! I omitted that one from my list: array indexes that are out of bounds. C doesn't check array bounds - it relies on the programmer to get it right.

18

PaulMurrayCbr:
C doesn't check array bounds - it relies on the programmer to get it right.

And I walked right into it and got bit. :blush: I wrote up a test EEPROM read/write sketch and when it passed, put it in the little box of 'stuff that works' for later inclusion. Later came and I pasted it in and never gave a thought to the array index thing.

Hopefully, a small amount of carelessness has been burned out of me.