String type printing odd chars eventually (*) ?

Hello!

this code here is a piece of a huge code. Apart it works fine. Within the whole code the function outputs odd chars.
I have a similar function doing almost the same thing (without the looping) but I have declared the returning char var as Static (before this, it was also printing odd chars).

I wasnt lucky with String type (it wont compile)… Also tried ‘+=’ instead of ‘.append’ withing the looping, but it wont compile either. Argh!

well, testing the code will show what the function was intented to do. I’m looking for replacements to stablelize it.

Any suggestions? Thanks in advance.

#include <WString.h>


char *items[50] = {"A21","3mA","T2z"};//--------- I expect that '50' is the max number of items this array can handle
int itemsLEN;


void setup()
{
  //items[0]    = "A21";
  //items[1]    = "3mA";
  //items[2]    = "T2z";
  itemsLEN    =     3;//--------------------------is there any how to set this dynamicaly accordingly to the number of items?
  
  Serial.begin(57600);

}


void loop()
{
  delay(1000);
  Serial.println(showItems());

}


char *showItems()//-----------------------------------THIS IS THE ISSUE
{
  
  String output;//------------------------------------wonder if I can replace String type and...
  int i;
  for(i=0; i<itemsLEN;i++)
  {
    output.append(items[i]);//------------------------...  .append to avoid including WString.h
    if(i != itemsLEN-1) output.append('|');  
  }
  return output;//------------------------------------ !*** this piece of code works as expected. The whole code, dont
                                                       // Sometimes returns odd characters.
                                                       // Couldn't declare 'static' String output.
                                                       // Probably I am overriding allocated memory, or pointing it wrong...

}

almost
Merry Christmas to everyone!
:wink:
Btp~

There's a memory leak in the String Library that may be causing the problem... http://www.arduino.cc/cgi-bin/yabb2/YaBB.pl?num=1241618944/8#8

Please report back if this does or does not help.

If you don’t need to grow the number of strings in the array over time, just leave the brackets empty and let the compiler count your strings. This makes an array of three pointers.

char* items[glow][][/glow] = { "A21", "3mA", "T2z" };

If you have code later that needs to know how many elements were defined in the array, do this:

#define countof(x)   ( sizeof(x) / sizeof(*(x)) )
...
    for (i = 0; i < countof(items); i++)
        ...

As for your final function, you can’t declare a String inside a function, and return a char* from the function. The String is dying when you return, so the returned pointer is invalid. The characters at that memory area are being trampled after the String has been destroyed.

Thanks for the replying!

*Coding Badly,
I have seen that link you’ve suggested but I understand it has to do with memory, when its getting out of space, no?.
However, that " ~String() { free(_array); } " line is very important for other parts of my code.

*halley,
the number of strings in array may vary between 1 to 50, the same as the strings length ( “A21” , “3mAx”, “T2” ) may vary between 2 to 5 characters.
The code to count how may elements were defined in array is very useful, thanks a lot. I have been looking for that in other forums but couldnt find it ** EDITED ** BUT it seems to work only when I declare the array like you suggested with empyt . like *items = {“it1”,“it2”,“it3”}
As for the String line. Here is a new doubt: When the function gets to its ending line, everything inside is dying, but also setting free space? Or, before killing it I should add the line Coding Badly pointed (~String() {free(_array);} ? Here, Im just trying to understand why this code I’ve post is working “fine” (?) and seems to print the Output correctly even if it gets destroyed.

Evolving a little more with other tests I’ve made yesterday, I figured that maybe the “Items” array isnt being built as correclty as I’m showing in the code I’ve posted before.

Here is how its happening indeed:

This message is sent to serial so Arduino will read it:

 "HEADER|A21|3mA|T2zs|3g"

(where header isn’t important here)

Using Messenger library I can split this string and fill an array (items) within a while loop:

#include <Messenger.h>

char    *items[50];


void msgComplete()//------------------------------------ this function takes place whenever arduino reads data from serial
                                                           //Messenger already tokenized the incoming string with "|" and I can
                                                           //access it incrementing the var msgSection, like it follows:
{
  
    int msgSection = 0;
    
    while(msg.available() && !msgSection)
    {
        //check header
        msgSection ++ ;
    }
    
    while(msg.available() && msgSection)
    {
        char incoming[5];  
        msg.copyString(incoming,5); //------------------- this transfers the being reading item (eg: "A21") to the 'incoming' var
    
        items[msgSection] = incoming;
        itemsLen ++ ;//------------------------------------- used this before getting to know how to count str elements within array
        msgSection ++ ;
        break; 
    }

}

Ok, now, with the items filled with the incoming items, I would call the showItems function. But it seems items[] is already kind of messed.

Again, is items being declared in a wrong way, am I missing the pointer again?

Thanks so much for the enlightenment, guys!

Btp~

You've got a lost of questions. I'll try to answer some of them.

Or, before killing it I should add the line Coding Badly pointed (~String() {free(_array);} ?

No. That code gets added to the string class. It defines (and implements) a a destructor. When the string onject goes out of scope (or is otherwise deleted), the destructor is called, which frees the memory that the string object uses internally. That makes the space available to your program for other purposes.

Until that memory is reallocated or overwritten, a pointer to that memory will still return correct results. You have no control over when that space gets overwritten, so don't rely on the memory being available.

In your last code, items is allocated correctly. What is being stored in it is not always.

In msgComplete, incoming is a local array, with a static location. The msg.copyString function copies data into that location. Then, you store that memory location in the items array.

With the next message section, the same memory is overwritten, by msg.copyString, and you store another copy of the same memory location.

What you need to do is allocate another memory location, copy incoming's data there, and store (in items) that memory location. Use strdup to do this:

items[msgSection] = strdup(incoming);

The incoming variable points to the first address of a block of 5 addresses. It never changes. It's like your house address. You might have different things in the house at any given time, but the house never moves.

To keep track of what's in the house at any given time, you can't store the location of the house (the house number). That never changes. What you have to do is make a copy of the house somewhere else, and keep track of where you put that particular copy.

That's what strdup does. Well, it does it for strings, not houses, but the idea is the same.

am I missing the pointer again?

Cute play on words. But, yes, I think you have missed the point of pointers.

Pauls! this is amazing.

As I work with animation, this analogies makes me cleary "see" the houses being built above the other ruining which was already there, hehe.

I admit I will take time to understand pointers, C built-in functions, memory issues and it wont happen from one day to another.

I'm reading a lot this days, but only handling with my own issues will make me understand how to code it right. I have found a site ( http://www.cplusplus.com/reference/ ) with a lot of samples (which helps me most "seing" how things works in different ways) to study, but sometimes I just don't get a good reference to append to my code, and the messy happens - which is natural.

Men, thanks so much again!! May Santa bring a huge gift for all of you ;D

The only thing I'm trying to fix to solve 100% of the problems here is to be getting the number of elements within items[], since the code halley posted is fantastic, but it seems to work while I declare the items[] already filled with x elements. As this lenght may vary I'm lost again. Not SO lost, actually, because I'm using an increment statement to get the number of elements put in items[]. It would improve my code to know how to dynamically get that length whenever I need to deal with it in other ways.

Btp~

You could get the number of items in the array the same way that the strlen tells how many characters there are in an array. A character array is a string when the array contains a NULL after the last valid character.

You can use this same principal with your arrays. When putting values in the array:

items[msgSection] = strdup(incoming);
items[msgSection+1] = NULL;

Then, later:

int itemCount = 0;
while(items[itemCount])
{
    itemCount++;
}
// Now, itemCount is the number of items in the items array

That’s true.

Once you understand it, everything gets clear.
Treating the array of elements as a array of chars (with last element as null) is pretty logical now.

this code works as expected. Thanks to everybody here. Hope this might suit other people too.

#include <WString.h>

char   *items[50]; // max of 50 items
int    itemsLEN;
int    itemCount = 0;
String output;


void setup()
{
  buildItems();
  Serial.begin(57600);
}


void buildItems()
{
  //this within a loop{
  items[0]    = "A2";
  items[1]    = "3mA";
  items[2]    = "Tw2z";
  items[3]    = "k33" ;
  items[4]    = NULL ;  //------- placed while detecting k33 is the last item.
  //}
  
  while(items[itemCount])//------ while not reading null
  {
      itemCount++;
  }
  
}


void loop()
{

  delay(1000);
  Serial.println(itemCount);
  Serial.println(showItems());
  output = "";  //--------------is this enough to avoid output var to grow bigger and begin runing out of RAM? OR: Is it rewriting the same memory block?
    
}


char *showItems()
{   
  for(int i=0 ; i<itemCount ; i++)
  {
    output.append(items[i]);
    if(i != itemCount-1) output.append('|');  
  }
  return output;
}