Kaliklipper
Kaliklipper

Reputation: 363

C++ Passing an array of char to a function, works for heap but not stack?

I'm writing this code on an Arduino uno and learning far more than I set out to in the process.

This code works, in that it reads in a string from the serial port and returns it with a Serial.print statement.

#include <Arduino.h>
#include <IRLibRecv.h>
#include <stdio.h>

#define BAUDRATE 9600
#define debug 1
#define develop 0
#define MSG_SRL_IN_SIZE 64

bool checkForSerialMsgs(char *message);
void test();

int msgIndex = 0;
char message[MSG_SRL_IN_SIZE] = {0};

void setup() {
  Serial.begin(BAUDRATE);
  while (!Serial);
}

void loop() {
  // char message[MSG_SRL_IN_SIZE] = {0};
  if(checkForSerialMsgs(message)) {
    Serial.println(message);
  }

}

bool checkForSerialMsgs(char *message)
{
  bool msgComplete = false;
  while (Serial.available() > 0) {
    char buf = Serial.read();
    if (buf == '\n') {
       msgComplete = true;
       msgIndex = 0;
       break;
     }
     else if(buf == '\r') {
       continue;
     }
     else if(msgIndex < MSG_SRL_IN_SIZE && msgComplete == false) {
       message[msgIndex++] = buf;
     }
  }
  return msgComplete;
}

I believe that this code declares the buffer on line 19 in the heap:

char message[MSG_SRL_IN_SIZE] = {0};

If I comment out that line and uncomment the declaration on line 27 in the void loop() method, I believe that then declares the the buffer on the stack as it is a local variable?

However, that then doesn't work.

In this case, because the buffer is of a fixed size, I don't think there will be any problem with it being resident in memory for the life of the program. But I will be writing other functions that need to create quite large dynamic arrays, for an Arduino, and I'm worried that if I do so, I'll quickly end up with a fragmented heap.

Whereas if the arrays were created on the stack, I think they would be cleanly cleared away every time the function at the top of the call stack exited.

So my question is, is there a way to create the array on the stack instead of the heap?

Before you answer, I'm often told that using more C++ objects like string and vector could help me out. But when I include the Arduino Cplusplus library to access them, my IDE throws a hundred deprecation error messages and nothing compiles any more. It would take me a month of Sundays to plough through that lot, so I'm sticking to this more C like approach.

======================================================================

Thanks for the responses, folks.

"Describe what does that then doesn't work mean."

I mean when the variable is global I can read in a string and that the line - Serial.println(message); - in the loop() {} prints it out. When I create the message array in loop(), message is empty when it is printed.

"Describe what does that then doesn't work mean. You use uninitialised msgIndex on second and further calls if buf == '\n' never met. It's better to reset the variable at the beginning. The condition msgComplete == false is always true"

No that's not the case, it's set to true when the '\n' is read in. The msgIndex is also reset to 0 then. There is a race condition. the function reads in chars before the sender has completed the transmission.

"The global char message is not allocated in the heap"

Thanks for that. The article I was reading this morning seems to have misled me. Not difficult to do considering how long it's been since I last wrote any C or C++.

"bool checkForSerialMsgs(char *message) is always fishy: either should be const, either the size is missing; the function cannot guess the array size."

I've come across a few comments like this while I've been Googling how to do this and I don't understand why this should be const. Or even what difference that makes to be honest. So I suspect that this is where the solution lies.

Although at the moment the function doesn't care about the size. I assume if I sent more than 64 characters they would be ignored because of the condition - msgIndex < MSG_SRL_IN_SIZE.

I have to conclude that my misunderstanding of the memory allocation on the Arduino has led me down a blind alley. If I use the global declaration of the buffer then I don't need the pointers at all anyway. So I could just use this:

#include <Arduino.h>
// #include <EEPROM.h>
#include <IRLibRecv.h>
#include <stdio.h>
#include <ArduinoJson.h>
#include <string>

using namespace std;

#define BAUDRATE 9600
#define debug 1
#define develop 0
#define MSG_SRL_IN_SIZE 64

bool checkForSerialMsgs();

int msgIndex = 0;
char message[MSG_SRL_IN_SIZE] = {0};

void setup() {
  Serial.begin(BAUDRATE);
  while (!Serial);
}

void loop() {
  if(checkForSerialMsgs()) {
    Serial.println(message);
  }

}

bool checkForSerialMsgs()
{
  bool msgComplete = false;
  while (Serial.available() > 0) {
    char buf = Serial.read();
    if (buf == '\n') {
       msgComplete = true;
       msgIndex = 0;
       break;
     }
     else if (buf == '\r') {
       continue;
     }
     else if(msgIndex < MSG_SRL_IN_SIZE && msgComplete == false) {
       message[msgIndex++] = buf;
     }
  }
  return msgComplete;
}

Apologies for wasting everyones time, but at least I've learned a few things.

Cheers

Upvotes: 0

Views: 140

Answers (1)

KIIV
KIIV

Reputation: 3739

So this doesn't work:

void loop() {
  char message[MSG_SRL_IN_SIZE] = {0};
  if(checkForSerialMsgs(message)) {
    Serial.println(message);
  }
}

Why is that? Because message gets created and cleared every time the loop() gets called. So it'll keep forgetting everything you wrote into it and you might get printed last few characters (but with such slow baudrate it would be usually just '\0' as you'll be fast enough to not get available() more than one character)

It's also on the stack btw.

Upvotes: 0

Related Questions