Weird problem with global array of struct item value changes..

Hi Everyone,
I tried to change value of a global array but it did not work, can you please advice me on what is the problem with this? I marked “//PROBLEM::” where the code part has problem.
The FieldCaption alway stays with “Level” after the first loop while the previous array item value has been changed successfully to value other than just “Level”!
Please share your help on this. I am a newbie in C++ and Arduino programming language. So hard to deal with char and string!
Thank you in advance!

typedef struct FormData {
  char* FieldCaption;
  unsigned int Type; // Data type  1,DD;2,DDD;3,HH;4,MIN;5,MM;6,NNN;7,YYYY;
  unsigned int Address; // EEPROM address 
};
FormData SubMenu[10];
int SubMenuSize=0;
const int MAX_SUB_MENU=10;

void setup(){
  Serial.begin(9600);
}
void loop(){
  String txtMenu = "Week day,2,7;On hour,3,8;On minute,4,9;Off hour,3,10;Off minute,4,11;Level,6,12";
  LoadSubMenu(txtMenu);
}

void LoadSubMenu(String SubMenuText){
  // This procedure will break the SubMenu array for later use
  // given: String command
  int numArgs = 0;
  int beginIdx = 0;
  String arg, fldCap;
  char charBuffer[20];
  boolean found = true;
 
  int idx = 0;
  int sub_idx; // for taking out of the text part
  
  while (found)
  { 
    idx = SubMenuText.indexOf(";", beginIdx);
    if (idx == -1) {
      found = false;
      idx = SubMenuText.length();    
    }
    
    arg = SubMenuText.substring(beginIdx, idx);
    if (arg == "") break; // escape at nothing left
   
    sub_idx = arg.indexOf(",");   
    
    //Field Caption
    fldCap = arg.substring(0,sub_idx);   
    fldCap.toCharArray(charBuffer, 20);
    SubMenu[numArgs].FieldCaption = charBuffer;
    //PROBLEM::if check here the array item is properly filled with new value of FieldCaption
    Serial.println(numArgs);
    Serial.println(SubMenu[numArgs].FieldCaption);
    
    // Field Type
    arg = arg.substring(sub_idx + 1);
    sub_idx = arg.indexOf(",");   
    fldCap = arg.substring(0,sub_idx);   
    SubMenu[numArgs].Type = fldCap.toInt();
    
    //EEPROM Address
    fldCap = arg.substring(sub_idx + 1);
    SubMenu[numArgs].Address = fldCap.toInt();
    
    beginIdx = idx + 1;   
    numArgs++;
  }
   
  // Remember max submenu item
  SubMenuSize = numArgs;

  for (int i=0; i<SubMenuSize; i++){
    //PROLEM:: but here the FieldCaption stay the same with value of: "Level", other changes as expected
    Serial.println(SubMenu[i].FieldCaption);
    Serial.println(SubMenu[i].Type);
    Serial.println(SubMenu[i].Address);
    delay(1000);
  }
  
  // cleaning the remaining array items
  for (beginIdx = numArgs; beginIdx < MAX_SUB_MENU; beginIdx++){
    SubMenu[beginIdx].FieldCaption="";
  }
}

If I change the struct a bit to the following:

typedef struct FormData {
  String FieldCaption;
  unsigned int Type; // Data type  1,DD;2,DDD;3,HH;4,MIN;5,MM;6,NNN;7,YYYY;
  unsigned int Address; // EEPROM address 
};

The later serial printing out is properly done as expected. However, the weird thing I spotted out here is:
Why the array element was correctly assigned previously, and just in one procedure flow, nothing change at all, its value then changed to just the last element value of “Level” in the later serial println.
I doubt the problem came up with struct but I have very limited knowledge on this and can not explain myself.
Please help to bright my mind with thanks.

You are using a single char array to hold the text, so every array element for FieldCaption is pointing to the same location in memory, and will print out whatever is currently in that location. Since "Level" was the last text copied into the char array, that is what gets printed. When you use String, a separate String is created for each FieldCaption, so you get the correct text.

Be careful using String variables, they often lead to memory corruption on a arduino, much safer to stick to char arrays.

david_2018:
You are using a single char array to hold the text, so every array element for FieldCaption is pointing to the same location in memory, and will print out whatever is currently in that location. Since “Level” was the last text copied into the char array, that is what gets printed. When you use String, a separate String is created for each FieldCaption, so you get the correct text.

Be careful using String variables, they often lead to memory corruption on a arduino, much safer to stick to char arrays.

Many thanks to David,
So in this case the char* thing should be declare as char* FieldCaption[] or something else. I am quite confused in array thing in C++ and Wiring. Logically, what I dont understand is: the Submenu was an array of struct already how can it be that the FieldCaption can be single char array? (because other element of the struct is stored correctly as expected)
Can you guide me on this please. Thank you in advance for your help!
Thank for your advice, I had it work with following changes:

typedef struct FormData {
  char FieldCaption[16];
  unsigned int Type; // Data type  1,DD;2,DDD;3,HH;4,MIN;5,MM;6,NNN;7,YYYY;
  unsigned int Address; // EEPROM address 
};

and in the code

strcpy(SubMenu[numArgs].FieldCaption, charBuffer);

the clean of array

// cleaning the remaining array items
  for (beginIdx = numArgs; beginIdx < MAX_SUB_MENU; beginIdx++){
    strcpy(SubMenu[beginIdx].FieldCaption,"");
  }

Working with char and array in C++ is so so complicated. but I am gradually get myself moving forward.
Thanks for your advice David!

why are you dynamically populating the SubMenu instead of statically initializing it

gcjr:
why are you dynamically populating the SubMenu instead of statically initializing it

Thank for your question, I have a static top level menu already, each top level menu will have some element that can be changed/set up by user so I approach menu in this way:

  • Build the top menu, each time user select a menu (using rotary button), that menu item will be stored to a variable and sub menu (actually list of setting element) will be built and remembered
  • Upon pressing of the button, sub menu item shall be shown for changing of setting values loaded from EEPROM. Upon the selection of last array element, top menu selection will be activated again for selection.
    I don't want to use menu library on github as it was too much and too complicated to follow. Doing like this will help me to build up knowledge on how Arduino programming work!

The static menu table is here (still a lot of problem but I want to develop myself bit by bit). This is yet completed but finally it will be what I described above;

const MenuItem Menu[]  PROGMEM = 
{  
  {"Set Voltage 1","Value,6,1;",1,SetBrightness},
  {"Set Voltage 2","Value,6,2;",1,SetPowerLevel},
  {"Power level 1","Min,6,3;Max,6,4;",0,SetPowerLevel},
  {"Power level 2","Min,6,5;Max,6,6;",0,SetPowerLevel},
  {"Dev 1: Alarm 1","Week day,2,7;On hour,3,8;On minute,4,9;Off hour,3,10;Off minute,4,11;Level,6,12",0,SetPowerLevel},
  {"Dev 1: Alarm 2","Week day,2,13;On hour,3,14;On minute,4,15;Off hour,3,16;Off minute,4,17;Level,6,18",0,SetPowerLevel},
  {"Dev 1: Alarm 3","Week day,2,19;On hour,3,20;On minute,4,21;Off hour,3,22;Off minute,4,23;Level,6,24",0,SetPowerLevel},
  {"Dev 1: Alarm 4","Week day,2,25;On hour,3,26;On minute,4,27;Off hour,3,28;Off minute,4,29;Level,6,30",0,SetPowerLevel},
  {"Dev 1: Alarm 5","Week day,2,31;On hour,3,32;On minute,4,33;Off hour,3,34;Off minute,4,35;Level,6,36",0,SetPowerLevel},
  {"Dev 1: Alarm 6","Week day,2,37;On hour,3,38;On minute,4,39;Off hour,3,40;Off minute,4,41;Level,6,42",0,SetPowerLevel},
  {"Dev 1: Alarm 7","Week day,2,43;On hour,3,44;On minute,4,45;Off hour,3,46;Off minute,4,47;Level,6,48",0,SetPowerLevel},
  {"Dev 2: Alarm 1","Week day,2,49;On hour,3,50;On minute,4,51;Off hour,3,52;Off minute,4,53;Level,6,54",0,SetPowerLevel},
  {"Dev 2: Alarm 2","Week day,2,55;On hour,3,56;On minute,4,57;Off hour,3,58;Off minute,4,59;Level,6,60",0,SetPowerLevel},
  {"Dev 2: Alarm 3","Week day,2,61;On hour,3,62;On minute,4,63;Off hour,3,64;Off minute,4,65;Level,6,66",0,SetPowerLevel},
  {"Dev 2: Alarm 4","Week day,2,67;On hour,3,68;On minute,4,69;Off hour,3,70;Off minute,4,71;Level,6,72",0,SetPowerLevel},
  {"Dev 2: Alarm 5","Week day,2,73;On hour,3,74;On minute,4,75;Off hour,3,76;Off minute,4,77;Level,6,78",0,SetPowerLevel},
  {"Dev 2: Alarm 6","Week day,2,79;On hour,3,80;On minute,4,81;Off hour,3,82;Off minute,4,83;Level,6,84",0,SetPowerLevel},
  {"Dev 2: Alarm 7","Week day,2,85;On hour,3,86;On minute,4,87;Off hour,3,88;Off minute,4,89;Level,6,90",0,SetPowerLevel},
  {"Set Date","Day,1,91;Month,5,92;Year,7,93;",0,SetPowerLevel},
  {"Set Time","Hour,3,94;Minute,4,95;",0,SetPowerLevel},
  {"Contrast","Value,6,96;",0,SetContrast},
  {"Brightness","Value,6,97;",1,SetBrightness},
  {"Time out","Value,6,98;",0,SetPowerLevel},
  {"Sleep","Value,6,99;",0,SetPowerLevel},
};

so your menu entries actually contain a value that gets changed rather than using a pointer to the variable?

gcjr:
so your menu entries actually contain a value that gets changed rather than using a pointer to the variable?

The static Menu item is a struct kept in PROGMEM with following attribute:
Caption: Menu title
Data: string store submenu
Type: 1: default, 0: setting mode
Function: What function to trigger at pressing the Rotary encoder switch

For Submenu: The value that gets changed are what read from EEPROM. a submenu item has following attribute:
Value,6,98
Value: title: to show on 2nd row
6 = Numeric type normally 1-100: this help me to validate the entry while turning the rotary encoder cc and ccw
98: address for EEPROM to read/update if user click the button to confirm.

This actually designed in a type of form cycle based on handling event of the rotary encoder:

  1. Roll the switch cc/ccw directly change value of the first default menu: such as setpowerlever to heater, directly change power supply to the heater;

  2. Press button - quick: activate default menu for quick access and set value to control device (i.e PWM for dimmer/ heater)

  • Press again >> roll cc/ccw >> press >> set and move to next submenu: change value of each submenu under this menuItem.
  • at the confirmation of last submenu item, go back to default menu selection (if more than 1 default menu is made in static menu array);
  1. Long press the button >. go to setting mode and do the same as describe above.
    I planned to publish the sketch after finalization of this for sharing but still have some problem for coding - I am totally new to C++ and wiring.
    In the sketch I use only rotary encoder, crystal liquid, EEPROM libraries to make thing simpler. Module included rtc 3231, a PWM board I designed for my self, a rotary encoder, a 16x2 LCD and a uno or micro.

I am making this project as my very first Arduino work - learning by doing and hopefully it will help someone else out there and myself.
The project is still at initial coding phase (I have all hardware tested already but still need the menu handler to be completed)
Please advice me a bit on programming as I am struggling hard to understand the logic of C++ and Wiring. These things are too far from what i knew in OOP programming with VB.
I did not draw the associated algorithms but the flow as below:

Project and Program flow description:
Controlling Hardware

  • Encoder
  • LCD 16x2
  • DS3231
  • UNO/Mini

Control and display via encoder

Encoder state as below:

  1. Quick roll the encoder CC/CCW
  • NO_PRESS_BEFORE: Change control sequence for 1st default menu
    i.e: Change power of heater up/down
  • QUICK_PRESS_BEFORE:
    ++ ONE_PRESS_BEFORE: reiterate through list of default menu
    ++ ENTER_SUB_MENU: changing value of attribute listed in Sub_menu
  1. SHORT_BUTTON_PRESS
  • FIRST_PRESS: show first default menu
  • SECOND_PRESS: Enter selected default menu setting
  • NEXT_PRESSES:

Moving around list of sub_menu items
At the last sub_menu: get out of adjusting mode of the selected menu

  1. LONG_PRESS: Show Setting menu: this only read/write/Update to EEPROM for default menu value and background task (alarm on/off/ set power of heater)
    Similar to description as above.

i’ve never had to dynamically create a menu item. the menu table entry identifies a type of entry and reference (ptr, index, …) and functions to execute for MENU, SELECT, UP and DOWN.

gcjr:
i've never had to dynamically create a menu item. the menu table entry identifies a type of entry and reference (ptr, index, ...) and functions to execute for MENU, SELECT, UP and DOWN.

This is the approach that I found in most of project on Arduino out there. However, I want to find out a bit more to learn and also want to go on a way that can combine different things together! And of course, I have little experience in this area. Will try to learn from you all as much as better.
Thanks for your sharing

i think more complete generic menu code is not trivial. of course you may just need a single level. but then try to expand that design rather start over. i’m biased, but i think having the code operate on static tables defining the menu items makes it easier to change and expand a menu

take a look at the attached code for a wifi based model railroad throttle that stores a number of locomotive addresses along with other parameters. i’m sure there other ways of doing this but i expected the number of menus to at least double

these ate the tables describing the menus.

// tables describing menu options

#include <stdlib.h>
#include <stdint.h>

#include "menu.h"
#include "menus.h"
#include "vars.h"

const char *sOnOff [] = { "Off", "On" };
const char *sWt []    = { "Light", "Low", "Med", "Heavy" };

// -------------------------------------------------------------------
// parameters
P_t pEng = { "Engine",  &engine, 0, 1,  sOnOff, { __,   __,   up, dn, dspP }};
P_t pTon = { "Tonnage", &tonnage, 0, 3, sWt,    { __,   __,   up, dn, dspP }};
P_t pLoc = { "Loco",    &loco,    0, 0, NULL, { __,  sft, inc, dec, dspV }};

P_t pAd0 = { "Adr[0]", &adr[0], 0, 0, & loco, {sel, sft, inc, dec, dspV}};
P_t pAd1 = { "Adr[1]", &adr[1], 0, 0, & loco, {sel, sft, inc, dec, dspV}};
P_t pAd2 = { "Adr[2]", &adr[2], 0, 0, & loco, {sel, sft, inc, dec, dspV}};
P_t pAd3 = { "Adr[3]", &adr[3], 0, 0, & loco, {sel, sft, inc, dec, dspV}};

P_t pAd4 = { "Adr[4]", &adr[4], 0, 0, & loco, {sel, sft, inc, dec, dspV}};
P_t pAd5 = { "Adr[5]", &adr[5], 0, 0, & loco, {sel, sft, inc, dec, dspV}};
P_t pAd6 = { "Adr[6]", &adr[6], 0, 0, & loco, {sel, sft, inc, dec, dspV}};
P_t pAd7 = { "Adr[7]", &adr[7], 0, 0, & loco, {sel, sft, inc, dec, dspV}};

P_t pAd8 = { "Adr[8]", &adr[8], 0, 0, & loco, {sel, sft, inc, dec, dspV}};
P_t pAd9 = { "Adr[9]", &adr[9], 0, 0, & loco, {sel, sft, inc, dec, dspV}};

P_t pHos = { "Host",  (int*)host, 0, 0,  NULL, { __,   sfA, inA, deA, dspA }};
P_t pPrt = { "Port",  &port,      0, 0,  NULL, { __,   sft, inc, dec, dspV }};

P_t pSsd = { "SSID",  (int*)ssid, 0, 0,  NULL, { __,   sfA, inA, deA, dspA }};
P_t pPsw = { "Pass",  (int*)pass, 0, 0,  NULL, { __,   sfA, inA, deA, dspA }};

// -------------------------------------------------------------------
// menus
Menu_t menuComm [] = {
    { "Host",   "",      T_STR,    (void*) & pHos },
    { "Port",   "",      T_PARAM,  (void*) & pPrt },

    { "SSid",   "",      T_STR,    (void*) & pSsd },
    { "Pass",   "",      T_STR,    (void*) & pPsw },
    { NULL,     NULL,    T_NULL,   NULL },
};

// -------------------------------------
// DCC loco addresses
Menu_t menuAdr [] = {
    { "ADDR 0", "",      T_LIST,   (void*) & pAd0 },
    { "ADDR 1", "",      T_LIST,   (void*) & pAd1 },
    { "ADDR 2", "",      T_LIST,   (void*) & pAd2 },
    { "ADDR 3", "",      T_LIST,   (void*) & pAd3 },
    { NULL,     NULL,    T_NULL,   NULL },
};

// -------------------------------------
// main menu
Menu_t menuMain [] = {
 // { NULL,      NULL,   T_NONE,   NULL },
 // { "Loco",    "Addr", T_MENU,   (void*) menuAdr, &loco },
    { "Loco",    "Addr", T_MENU,   (void*) menuAdr },
    { "Comm",    "Cfg",  T_MENU,   (void*) menuComm },
    { "Engine",  "",     T_PARAM,  (void*) & pEng },
    { "Tonnage", "",     T_PARAM,  (void*) & pTon },
    { "Options", "",     T_NONE,   NULL },
    { "Version", version, T_NONE,   NULL },
    { "",        "",     T_NULL,   NULL },
};

// menuTop makes menuMain accessible externally
Menu_t *menuTop = & menuMain [0];

menu.cpp (8.36 KB)

menu.h (2.28 KB)

vars.cpp (3.12 KB)

gcjr:
i think more complete generic menu code is not trivial. of course you may just need a single level. but then try to expand that design rather start over. i'm biased, but i think having the code operate on static tables defining the menu items makes it easier to change and expand a menu

Thank you for your examples and guidance, I am looking at your files and will get some learning from them.

void loop() {
  String txtMenu = "Week day,2,7;On hour,3,8;On minute,4,9;Off hour,3,10;Off minute,4,11;Level,6,12";
  LoadSubMenu(txtMenu);
}

You are going to a lot of trouble to store the menu contents in a String, then split out the individual elements to populate the submenu. It would be much simpler to just initialize the submenu to the appropriate values to begin with, and would be nearly the same amount of storage as the String initializer.

david_2018:

void loop() {

String txtMenu = "Week day,2,7;On hour,3,8;On minute,4,9;Off hour,3,10;Off minute,4,11;Level,6,12";
 LoadSubMenu(txtMenu);
}




You are going to a lot of trouble to store the menu contents in a String, then split out the individual elements to populate the submenu. It would be much simpler to just initialize the submenu to the appropriate values to begin with, and would be nearly the same amount of storage as the String initializer.

Thanks David,
I made this topic to raise a question on why array item was not changed and using your #1, I made a change to char array at #2, and it worked already. No more String was used in my sketch. This post is a following up to my previous post on PROGMEM. Total RAM used in my sketch was less than 20% (18) with only few Int/boolean dynamic variables.
The reason that I am using this approach is described as following:

  • Top menu was statically defined and kept in PROGMEM;
  • User select a menu, this menu item will be remembered in RAM as a struct item with an array of up to 10 elements to keep sub variables to use later.
  • By using a button, sub items under this menu will be accessed progressively and changed with previous value kept in EEPROM. This submenu/ item will have just a structure of FORM DATA with following element

Char array of Title/FieldCaption (for user friendly purpose)
Type int: for validation: date/month/ weekday ...etc
int Address: EEPROM Address to load unsigned int value kept in EEPROM.
In fact there will be only 1 menu item set kept in RAM each time associated with an array of up to 10 elements to keep the FORM DATA structure.
I tried to move on this path as I want the menu to be simply handled! The hard thing for me to overcome is transforming from a programming language structure (such as Instr >> substring, Len()>> length and difficulty in handling array and pointer in C++ ) that I used for long to Wiring/C++ with a lot of differences.
I strongly hope discussion here will enable to go further with C++ and Wiring language.
Thank you very much