Passing char Arrays to function

Using Arduino Programming Questions
1

Hey there,

I am on my first Arduino project and I am stuck in a corner.
I have a project with different buttons that switch on different LED segments.
Now I want the Arduino to play switch when the LEDs are switched on and off depending on the switched buttons.

I used the PCM library for playing sounds with was created by Michael Smith (if needed i can link it).

This library needs to create the sound files as a char Array and then you can playback these with the function startPlayback(array, sizeof(array));

If I declare these sound files I am able to play them back, but if I create a function where I check the button states and I try to pass these array into the function they do not play. If I call the array in the button function, it is no problem. I think I am passing the array to the function in a wrong way.
ChatGPT says passing them by reference

void myFunction(char* array) {
    // Do something with the array
}

myFunction(myArray);

is the right way but it does not work.
Here are extract of my code, it would be cool if you can tell me what I am doing wrong.

//Soundplayback
const unsigned char computer_on[] PROGMEM = {126, etc.
const unsigned char computer_off[] PROGMEM = {123, etc.

//Button state function
bool toggleButtonState(byte buttonPin, bool &ButtonState, byte &lastButtonState, unsigned long &lastTimeButtonStateChanged, byte debounceDuration, char* if_button_on, char* if_button_off) {
  if (millis() - lastTimeButtonStateChanged >= debounceDuration){
   byte buttonState = digitalRead(buttonPin);
   if(buttonState != lastButtonState){
    lastTimeButtonStateChanged = millis();
    lastButtonState = buttonState;
    
    //Button off
    if(buttonState == false) {
     if(ButtonState == true){
      ButtonState = false;
      switch(buttonPin){ //switch button with calling does work
        case 2:
        startPlayback(computer_off, sizeof(computer_off));
        break;
        case 3:
        startPlayback(computer_off, sizeof(computer_off));
        default:
        startPlayback(computer_login, sizeof(computer_login));
      }
      // this part does not work
      //startPlayback(computer_login, sizeof(computer_login));
      //startPlayback(if_button_on, sizeof(if_button_on));

//calling the function in the programm
Button1State = toggleButtonState(BUTTON_PIN_1, Button1State, lastButtonState1, lastTimeButtonStateChanged, debounceDuration, computer_on, computer_off);
2

It is a right way and it works, if you use it properly.

In your code you use PROGMEM arrays, it can't be accessed directly.

3

Can ChatGPT suggest a solution ?

Be aware that most members of the forum do not hold AI generated code in high regard and will not provide solutions for its code erorrs

4

IMHO this ChatGPT thing is getting too overrated. If one wants to be a programmer should think with ones head, and learn how to do what needs the project should do (by reading articles, forums, documentation...) instead of asking an AI to do the work.

For instance, that "switch(buttonPin)" looks like it misses a couple of "break" statements, I suggest to correct it.
And, even if correct, there's no apparent reason why calling the same function with the same parametrs from two different places could run in one case and fail on another.

5

the code wasn't created by chatgpt i only wanted to confirm, that i pass the argument the right way.

6

okay,
because i just did what i have to do that it works, in this case use PROGMEM, I do not know what this "function" does. is it interchangeable with a normal declaration? otherwise I have to search what it does.

8

hey you are right.

as i wrote down below i just wanted to verify with chatgpt that passing the array is right.
the switch statement uses the global variable stored with progmem.
the other part uses the variable of the function. so the global stored variable with progmem gets handled over the the function via an array pointer and this seems not to work.

it seems as if i have to put the possibilities in switch functions and then pass the pin numbers and the corresponding playfunctions to the button state function instead of passing the arrays.

9

@b707 already answered that.

Anyway, I wonder why "toggleButtonState()" has function parameters passed by reference/address like "ButtonState", "lastButtonState" etc instead of simply let them be accessed as global variables (or simple arrays if you need more instances, like one for each button), a simpler and quicker way to let function read and/or write on them.

What I'm saying about the "switch" statement is it apparently misses a couple of "break;" statements:

      ButtonState = false;
      switch(buttonPin){ //switch button with calling does work
        case 2:
          startPlayback(computer_off, sizeof(computer_off));
          break;
        case 3:
          startPlayback(computer_off, sizeof(computer_off));
          break; //<-- I MEAN THIS
        default:
          startPlayback(computer_login, sizeof(computer_login));
          break; //<-- AND THIS
      }

If you're talking about this:

      // this part does not work
      //startPlayback(computer_login, sizeof(computer_login));
      //startPlayback(if_button_on, sizeof(if_button_on));

because you posted just small portions of the code and the behaviour is not clear to me (I still don't know what you mean with "seems not to work"), I can't say anything about the culprits.
But as far as I can see, some programming bad habits are present, like defining variables with the same exact name except the first letter (e.g. "buttonState" and "ButtonState"), together with the missing breaks, and who knows what else can be found around...

10

The robot has read that in a C textbook. Maybe you should do that too.

1 Like
11

all right this is the used code:

//Soundplayback
const unsigned char computer_on[] PROGMEM = {126, etc.
const unsigned char computer_off[] PROGMEM = {123, etc.

//Button Part Anfang
const byte BUTTON_PIN_1 = 2;
const byte BUTTON_PIN_2 = 3;
const byte BUTTON_PIN_3 = 4;
const byte BUTTON_PIN_4 = 7;

byte lastButtonState1 = 0;
byte lastButtonState2 = 0;
byte lastButtonState3 = 0;
byte lastButtonState4 = 0;

bool Button1State = false; 
bool Button2State = false; 
bool Button3State = false;
bool Button4State = false;

unsigned long lastTimeButtonStateChanged = millis();
const byte debounceDuration = 50;

bool toggleButtonState(byte buttonPin, bool &ButtonState, byte &lastButtonState, unsigned long &lastTimeButtonStateChanged, byte debounceDuration, char if_button_on, char if_button_off) {
  if (millis() - lastTimeButtonStateChanged >= debounceDuration){
   byte ButtonState_Pin = digitalRead(buttonPin);
   if(ButtonState_Pin != lastButtonState){
    lastTimeButtonStateChanged = millis();
    lastButtonState = ButtonState_Pin;
    
    //Button off
    if(ButtonState_Pin == false) {
     if(ButtonState == true){
      ButtonState = false;
      switch(buttonPin){
        case 2:
        startPlayback(computer_off, sizeof(computer_off));
        break;
        case 3:
        startPlayback(computer_off, sizeof(computer_off));
        break;
        default:
        startPlayback(computer_login, sizeof(computer_login));
        break;
      }
      //startPlayback(computer_login, sizeof(computer_login));
      //startPlayback(if_button_on, sizeof(if_button_on));
      //delay(100);
      
     }
    //Button on
    else{
      ButtonState = true;
      startPlayback(computer_on, sizeof(computer_on));
      //delay(100);
    }
    }
   }
  }
  return ButtonState;
}


//Button Check
Button1State = toggleButtonState(BUTTON_PIN_1, Button1State, lastButtonState1, lastTimeButtonStateChanged, debounceDuration, computer_on, computer_off);
Button2State = toggleButtonState(BUTTON_PIN_2, Button2State, lastButtonState2, lastTimeButtonStateChanged, debounceDuration, computer_on, computer_off);
Button3State = toggleButtonState(BUTTON_PIN_3, Button3State, lastButtonState3, lastTimeButtonStateChanged, debounceDuration, computer_on, computer_off);
Button4State = toggleButtonState(BUTTON_PIN_4, Button4State, lastButtonState4, lastTimeButtonStateChanged, debounceDuration, computer_on, computer_off);
12

It didn't say it or it was wrong. The example is a pointer, not a reference.

I'll just repeat what everyone is saying: you need to learn programming, the language, there are not shortcuts – AI response will almost alway do you more harm than help.

I suppose It can be a good tool if you already know what you're doing, although I haven't found it better than just google, look for answers and understand what's going on. Simply copy–paste of ANYTHING is never enough.

Yeah, it would be better if you gave the link, I'd not have to google for it ;p

So… I guess you're talking about this?

As you can see, it takes unsigned char const *data – u can now use https://cdecl.org/ to decipher it a little bit:

unsigned char const *data => declare data as pointer to const unsigned char

And here is an example: Arduino/libraries/pcm/examples/playback/playback.pde at master · RayHightower/Arduino · GitHub

Try it and see if it works.

13

Good thanks. Ok, so, there are many issues here. Let's start dive in.

This is your function definition:

bool toggleButtonState(byte buttonPin, bool &ButtonState, byte &lastButtonState, unsigned long &lastTimeButtonStateChanged, byte debounceDuration, char if_button_on, char if_button_off)

The first thing I see is the fact it's useless to pass global variables as parameters like "debounceDuration" and "lastTimeButtonStateChanged": it's not an error, but as they can be accessed freely you can avoid to pass them.

Ok, this is how you call it:

Button1State = toggleButtonState(BUTTON_PIN_1, Button1State, lastButtonState1, lastTimeButtonStateChanged, debounceDuration, computer_on, computer_off);

You can see the last two parameters are "computer_on" and "computer_off", and they are char arrays:

const unsigned char computer_on[] PROGMEM = {126, etc.
const unsigned char computer_off[] PROGMEM = {123, etc.

But the big problem here is that the function (see function definition above) expects just two char not char arrays! A char array is a 2 bytes pointer, so you are passing just a single char/byte to the function just, and the call "startPlayback(if_button_on, sizeof(if_button_on));" will always fail.
If you want to pass those arrays, you must change the function last parameters type from "char if_button_on, char if_button_off" to "const unsigned char if_button_on[], const unsigned char if_button_off[]". But you can't use PROGMEM.

Said that, a few notes about that programming style. You have 4 buttons to manage, right? Why using four separate variables for each purpose instead of using arrays? Together with the global variables usage it will let you completely avoid any "pass by reference" hat trick and headaches!

See this example code, derived from yours (the code is not complete and I can't test it, so you should check it by yourself), and tell me if it's more readable and easily manageable. If you have questions, write here and we'll explain what you need to know:

const int C_MAX_SIZE = 30;
// Soundplayback as a matrix
const unsigned char sound[3][C_MAX_SIZE] PROGMEM = {
  {126, etc.}, // [0] is "computer on"
  {123, etc.}, // [1] is "computer off"
  {123, etc.}  // [2] is "computer login"
};
const byte soundSize[3] = { 20, 16, 28 }; // Adjust to real length, must be less or equal to C_MAX_SIZE
// Sounds indexes
const byte C_ON = 0;
const byte C_OFF = 1;
const byte C_LOGIN = 2;

// Button global variables
const byte ButtonPin[4] = {2,3,4,7};
byte lastButtonState[4] = {0,0,0,0};
bool ButtonState[4] = {false,false,false,false};

unsigned long lastTimeButtonStateChanged = millis();
const byte DEBOUNCE = 50;

bool toggleButtonState(byte btn, byte if_button_on, byte if_button_off) {
  if (millis() - lastTimeButtonStateChanged >= DEBOUNCE){
   byte CurrentButtonState = digitalRead(ButtonPin[btn]);
   if(CurrentButtonState != lastButtonState[btn]){
    lastTimeButtonStateChanged = millis();
    lastButtonState[btn] = CurrentButtonState;
    
    //Button off
    if(!CurrentButtonState) 
    { // Button off
      if(ButtonState[btn]){
        ButtonState[btn] = false;
        switch(buttonPin){
          case 2:
            startPlayback(sound[C_OFF], soundSize[C_OFF]);
            break;
          case 3:
            startPlayback(sound[C_OFF], soundSize[C_OFF]);
            break;
          default:
            startPlayback(sound[C_LOGIN], soundSize[C_LOGIN]);
            break;
        }
        startPlayback(sound[C_LOGIN], soundSize[C_LOGIN]);
        startPlayback(sound[if_button_on], soundSize[if_button_on]);
        delay(100);
      }
    } else {
      //Button on
      ButtonState[btn] = true;
      startPlayback(sound[C_ON], soundSize[C_ON]);
      delay(100);
    }
  }
  return ButtonState[btn];
}


//Button Check and update
toggleButtonState(1, C_ON, C_OFF);
toggleButtonState(2, C_ON, C_OFF);
toggleButtonState(3, C_ON, C_OFF);
toggleButtonState(4, C_ON, C_OFF);
...

PS: Stop using the damn ChatGPT, if you want to learn something you better search, read, study, and use this forum properly!

14

hey thx a lot,
putting all this into array was the next step, i wanted to have a code that at least functions for my hobby project.

later in the code i check the status of the different buttons and if on execute other functions

if(CurrentStateButton2 == 1){etc.}

with naming the buttons with variables it is in this step easier for me to check which button is used, because in an array 1 correspondents to 0 because of the item counting.

but you are right this would drastically minimize the code.

I could keep the CurrentButtonState Variables and change the others to arrays.

15

Unfortunately you have to get used to it, because, sorry, that's how all programming languages work (for the last 30 years or so). :wink:

If you're still stuck to "index 1 is element 1", you could just add 1 to the array size (e.g. "const byte ButtonPin[5]") and ignore element [0]. But I really disagree, that's not a good programming, and I encourage you to start "thinking as a real programmer"...

Said that, you wrote:

It isn't a problem at all, but, as I said before, you must start thinking like a programmer, and throw that damn ChatGPT away from your mind!
If you want to give your buttons a name, just use constants or "#define" symbols and assign them the corresponding index.
Example:

// Button global variables
const byte ButtonPin[4] = {2,3,4,7};
byte lastButtonState[4] = {0,0,0,0};
bool ButtonState[4] = {false,false,false,false};
#define BTN_UP 0
#define BTN_DOWN 1
#define BTN_LEFT 2
#define BTN_RIGHT 3

This way in your code you can forget about the index or numbers, and use mnemonic names instead of numbers, you demonstrate a good habit in programming, and moreover it's more readable than that "CurrentStateButton2" (PS: remember also bool values can be directly tested, you don't need to compare them with a number!):

if (ButtonState[BTN_DOWN]) { // Instead of  "if (ButtonState1 == 1)"
16

A much better and cleaner solution is to use references:

// Button global variables
const byte ButtonPin[4] = {2,3,4,7};
byte lastButtonState[4] = {0,0,0,0};
bool ButtonState[4] = {false,false,false,false};

bool &btnUp = ButtonState[0];
bool &btnDown = ButtonState[1];
bool &btnLeft = ButtonState[2];
bool &btnRight = ButtonState[3];

Then:

  if(btnDown) {
    // Do stuff for button down
  }
17

alright thx then i really have to get used to the counting stuff ;).

all the years back when i studied electrical engineering they said you should not use define stuff in the code. any pro or cons in using them?

18

that the way i‘ll go thank you :slight_smile:

19

That's right, using preprocessor macros should be a last resort. For example, use 'const' varaibles to define constants. That provides the protection of the compiler's type checking ability.

As I said, the code the code I posted using references is cleaner, clearer, and safer.

FWIW
Just about every algorithm you'd ever see while studying EE is zero base-indexed. For example, FFT.

20

It's surely cleaner for the "if()" but that's all: IMHO is completely useless and you'll lose any contact to the real meaning of the variables.

The "#define"preprocessor directives tell the compiler to change every instance of that symbol with its value before compiling and there's no real drawback in it. The use of either "#define" or "const" variables for indexes and/or pin numbers, as an example, is just a matter of taste and optimization (a "const byte" variable is referenced only once and used everywhere, while "#define" is always converted to an "int" for each instance).

Using pointers, especially for newbies (like you) is quite dangerous, and I discourage their use.
Anyway, if you prefere const variables the syntax is just a bit different:

// Button global variables
const byte ButtonPin[4] = {2,3,4,7};
byte lastButtonState[4] = {0,0,0,0};
bool ButtonState[4] = {false,false,false,false};
const byte BTN_UP = 0;
const byte BTN_DOWN = 1;
const byte BTN_LEFT = 2;
const byte BTN_RIGHT = 3;
...
  // Their usage remains exactly the same:
  if (ButtonState[BTN_DOWN]) { 
...
21

It's actually a matter of Best Practice.

1 Like