Array of Objects

I'm creating a class which parts of my code can put messages into, and other parts of my code can pull those messages off and deal with them. I think I need the .add and .delete functionality so when a piece of code comes a long and deals with a message, it can wipe it off of the class. But I don't really need to store more than say 10 objects at any given time.

I tried using SimpleList from Arduino Playground - LibraryList, but it's acting weird, so maybe I'm using it wrong... or maybe there's a completely better way to do things... Here's my code.

  #define MOVEMENT_REQUEST 0x01
  #define TEXT_UPDATE 0x01

  #include <SimpleList.h>

   
  // This is the struct that pollKeys should generate, and it should 
  // put it on a stack
  struct SystemMessage {
    uint8_t messageType;       // TEXT_UPDATE | MOVEMENT_REQUEST
    char data[100];
  };
  
  class SystemMessageCourier {
    public:
     SystemMessageCourier();

     void addMessage(SystemMessage systemMessage);

     SimpleList <SystemMessage> extractMessages(uint8_t messageType);
     SimpleList <SystemMessage> _messages;

     uint8_t count();
     
  };
  


SystemMessageCourier::SystemMessageCourier() {
}

void SystemMessageCourier::addMessage(SystemMessage systemMessage) {
  _messages.push_back(systemMessage);
}

// iterate over all _messages
// add matching messages to hits
// remove matching messages from _messages
SimpleList <SystemMessage> SystemMessageCourier::extractMessages(uint8_t messageType) {
  SimpleList<SystemMessage> hits;

  for (SimpleList<SystemMessage>::iterator itr = _messages.begin(); itr != _messages.end();) {
    SystemMessage thisMsg = *itr;
    if (thisMsg.messageType == messageType) {
      hits.push_back(thisMsg);
      _messages.erase(itr);
      continue;
    }

    ++itr;
  }

  return hits;
}

// displays how many messages it has
uint8_t SystemMessageCourier::count() {
  uint8_t count = 0;
  for (SimpleList<SystemMessage>::iterator itr = _messages.begin(); itr != _messages.end();) {
      count++;

      ++itr;
    }

  return count;

}



SystemMessageCourier* systemMessageCourier;





void setup() {
  Serial.begin(115200);
  Serial.println("Starting setup() ");
  
  systemMessageCourier = new SystemMessageCourier();
  struct SystemMessage s = { TEXT_UPDATE, "hello" };
  systemMessageCourier->addMessage(s);

  struct SystemMessage s2 = { TEXT_UPDATE, "good bye" };
  systemMessageCourier->addMessage(s2);

  SimpleList<SystemMessage> msg = systemMessageCourier->extractMessages(TEXT_UPDATE);
  
  
  uint8_t theCount = systemMessageCourier->count();

  printHex(theCount, 0);
  
  Serial.println("Setup complete");
}


void loop() {

}



void printHex(int num, int precision) {
  char tmp[16];
  char format[128];

  sprintf(format, "0x%%.%dX ", precision);

  sprintf(tmp, format, num);
  Serial.print(tmp);
}

The code hangs on the line systemMessageCourier->addMessage(s2);

  struct SystemMessage {
    uint8_t messageType;       // TEXT_UPDATE | MOVEMENT_REQUEST
    char data[100];
  };

How much memory are 10 of these going to use? There is the overhead of a class to add and remove instances, too.

I don't understand what you're saying. Is the SystemMessage struct too bloated. I actually think it only NEEDS data to be 36 chars large, but I left it at 100 for good measure. Does changing the size determine how to approach the problem? It probably won't need to be any larger than what the demo is set to now.

I actually think it only NEEDS data to be 36 chars large, but I left it at 100 for good measure.

For what definition of "good measure"? The Arduino doesn't have terabytes of memory, you know.

Does changing the size determine how to approach the problem?

No. It could possible change whether you have a problem, or not, though.

PaulS:
For what definition of "good measure"? The Arduino doesn't have terabytes of memory, you know.

So you're saying the reason the code doesn't work is because I've consumed all the memory on the Arduino?

Paul is telling you that needing 36 bytes of memory for an object, but defining 100 bytes for each of 10 instances of the object is pitching away 740 bytes of SRAM. Many new students don't realize that 32K of flash memory is rarely the limiting factor on program size. Instead, it's the amount of SRAM, since that's where the data are normally stored. This not only includes the data that you have defined for the program, but also the temporary (invisible) data the compiler needs in the form of overhead for things like function calls. So, figure out what your minimum array sizes need to be and use those. If you find you need more later, give it a try, but get it working first.

econjack:
Paul is telling you that needing 36 bytes of memory for an object, but defining 100 bytes for each of 10 instances of the object is pitching away 740 bytes of SRAM. Many new students don't realize that 32K of flash memory is rarely the limiting factor on program size. Instead, it's the amount of SRAM, since that's where the data are normally stored. This not only includes the data that you have defined for the program, but also the temporary (invisible) data the compiler needs in the form of overhead for things like function calls. So, figure out what your minimum array sizes need to be and use those. If you find you need more later, give it a try, but get it working first.

It seems a bit rude that it should take 2 vague, deliberately unhelpful posts and a helper to get that information, but never the less, thank you both for providing it. I appreciate all the help I can get. I hadn't realized how little sram was available. But I don't think the objects I'm placing in SRAM are what's causing the problem. It's constructive to point out things like the fact that you can read the amount of SRAM available using freeMemory() from the memory free library unless there's a better way for debugging potential memory issues with code???

Anyway, let's say I define that char data[] array with a size of 9, that's just enough to fit the string "good bye". It makes more sense to make it a pointer to memory, but let's just see what happens when we use it as a char array.

Have a look at the below snippet:

void setup() {
  Serial.begin(115200);
  Serial.println("Starting setup() ");

  systemMessageCourier = new SystemMessageCourier();
  struct SystemMessage s = { TEXT_UPDATE, "hello" };
  systemMessageCourier->addMessage(s);


  struct SystemMessage s2 = { TEXT_UPDATE, "good bye" };

  Serial.println(freeMemory());         //  A)  reports 1714
  systemMessageCourier->addMessage(s2);

  Serial.println(freeMemory());         //  B)  reports 1692
  // SimpleList<SystemMessage> msg = systemMessageCourier->extractMessages(TEXT_UPDATE);


  // uint8_t theCount = systemMessageCourier->count();

  // printHex(theCount, 0);

  Serial.println("Setup complete");
}

See how A) reports just over a thousand free bytes, and how B) also reports over a thousand free bytes? That's reassuring, but when we uncomment the line below B, that program seems to stall there. Upon closer investigation it seems that the iteration loop within extractMessages loops infinitely. I'm using the SimpleList library in that method, and apparently I'm not using it correctly.

I wouldn't be opposed to storing my structs in an array, but doing that isn't even as straight forward as you seem to think it is and a lot of things can go wrong. I think that's one of the main reasons people produce posts on forums, to either ask for advice or share advice with others.

It seems a bit rude that it should take 2 vague, deliberately unhelpful posts and a helper to get that information, but never the less, thank you both for providing it.

What is rude is you not understanding a question, and then taking offense - as though it was my fault you didn't understand.

If you didn't understand, you should SAY that.

I hadn't realized how little sram was available.

I can't imagine why not. Why didn't you check the specs FIRST?

Anyway, let's say I define that char data[] array with a size of 9, that's just enough to fit the string "good bye". It makes more sense to make it a pointer to memory, but let's just see what happens when we use it as a char array.

So, make it a pointer. As long as you don't plan to change the data that is pointed to, you'll be fine. If you do need to change the data, you need memory that is yours, not memory allocated by the compiler.

I'm using the SimpleList library in that method, and apparently I'm not using it correctly.

Your use looks fine. But, it's your class, so add some Serial.print() statements to it to understand what it is doing.

There is a significant amount of dynamic memory allocation going on in the SimpleList class. Compound that with the allocation that happens in your class, and I would not be surprised that memory is being fragmented/leaked.

I wouldn't be opposed to storing my structs in an array, but doing that isn't even as straight forward as you seem to think it is

Of course it is. That you are struggling says more about your skills than you might want to.

You have not posted any code that tries to use an array of structs, or explained what issues you are having, so we can't possibly help.

PaulS:
What is rude is you not understanding a question, and then taking offense - as though it was my fault you didn't understand.

If you didn't understand, you should SAY that.

Actually, I said I didn't understand your question. Look up at the post. I don't know why you would be so short with someone. If you don't want to help someone, then just don't post a response. Someone else will probably chime in. You aren't obligated to be unkind to people who make posts, so just let it go if you feel someone's question is making you upset.

PaulS:
Your use looks fine. But, it's your class, so add some Serial.print() statements to it to understand what it is doing.

...

You have not posted any code that tries to use an array of structs, or explained what issues you are having, so we can't possibly help.

Both of these statements appear to be untrue.

PaulS:
"I hadn't realized how little sram was available."

I can't imagine why not. Why didn't you check the specs FIRST?

PaulS:
"I wouldn't be opposed to storing my structs in an array, but doing that isn't even as straight forward as you seem to think it is"

Of course it is. That you are struggling says more about your skills than you might want to.

You are an unkind forum poster. Please do not submit responses to my posts in the future, they are unwelcome.

Both of these statements appear to be untrue.

Really? Where is your code with debug statements? Where is your code with an array of structs?

You are an unkind forum poster. Please do not submit responses to my posts in the future, they are unwelcome.

And you're an idiot.

PaulS tries to get you to think about your problem and work out the answer, rather than saying "the problem is in line 10, just replace it with xxx and it will all work". His method helps you to learn. Try to take it in good spirit.

  char format[128];

  sprintf(format, "0x%%.%dX ", precision);

You really need to think about that RAM shortage here. Does it really take 128 bytes to hold "0x%.3X" ?

If you are down to your last 120 bytes of RAM when you call that function, it will crash.

You refuse help from PaulS at your own peril. He knows what he writes about. Good luck.

It would be a good idea to listen to Nick Gammon and econjack as well.

I think you have a problem here:

SimpleList <SystemMessage> SystemMessageCourier::extractMessages(uint8_t messageType) {
  SimpleList<SystemMessage> hits;

  for (SimpleList<SystemMessage>::iterator itr = _messages.begin(); itr != _messages.end();) {
    SystemMessage thisMsg = *itr;
    if (thisMsg.messageType == messageType) {
      Serial.println ("extracting");  // <-------------- debugging
      Serial.flush ();    // <-------------- debugging
      hits.push_back(thisMsg);
      _messages.erase(itr);
      continue;
    }

    ++itr;
  }

  return hits;
}

Adding in the two debugging lines I see:

Starting setup() 
Free memory: 937
extracting
extracting
extracting
extracting
Starting setup() 
Free memory: 937
extracting
extracting
extracting
extracting
Starting setup()

So it is crashing after extracting 4 messages which is a bit strange as you only added two.

I should also point out:

  for (SimpleList<SystemMessage>::iterator itr = _messages.begin(); itr != _messages.end();) {
    SystemMessage thisMsg = *itr;
    if (thisMsg.messageType == messageType) {
...
      _messages.erase(itr);
      continue;
    }

    ++itr;
  }

On a match you are not incrementing itr, thus this loop will go on forever.

PaulS:
And you're an idiot.

The guy is literally name calling... what kind of a forum is this??? If I were an actual stake holder in at Arduino I'd try to keep the forums I friendly place. It's hard to consider this anything but passive aggressive and hostile. It would be great if PaulS were really trying to help people "figure it out on there own" but he's not really doing a good job of that, it's more likely that he's making his posts to please solely himself by being rude to others.

Nick Gammon, thanks for commenting on this thread. Is name calling really appropriate? You write on your posts that you're a "moderator"? What kind of atmosphere are you moderating here? You want me to take name calling in good spirit? I don't think PaulS is particularly smart or dignified. So when is it appropriate for someone to name call him? Are all names allowed or are certain names prohibited?

All the code I posted above should run when you paste it in as long as you download that Arduino library I linked to. Is that not working for others? I posted a complete block of demo code, that should compile and run. I clearly articulated my goals. I picked an effective thread title. I believe I've been very polite, especially under these circumstances, and have done what I can to write a productive thread, but I'm met with short, passive aggressive hostile posts from someone who seems to not want to be polite or helpful. Is PaulS a forum celeb? Are people less popular than PaulS supposed to put up with his rudeness until they become more popular? That seems odd. I'd prefer to not interact with him and leave that behind me.

I copy/pasted that printHex function from google and literally didn't consider it's contents assuming it was a sound function that could be used for general debugging purposes. I am not an expert Arduino programmer. I realize that it's easy to ridicule this, but at the same time, it's nonsensical to ridicule this; anyone coming to a forum to ask questions is acknowledging that they are still learning.

I solved my issue. I was not iterating over my SimpleList object correctly. It was unable to exit the iteration loop. Posting to this forum was an uncomfortable experience.

I think their example is flawed (or even the library, who knows?)

I modified it to have debugging info, and also to delete the last item, rather than the fourth one. Like this:

#include <SimpleList.h>

/*
* Open Monitor Serial and take a look.
*/

SimpleList<int> myList;

void setup()
{
  Serial.begin(115200);
  Serial.println ("Starting");

  myList.push_back(1);
  myList.push_back(2);
  myList.push_back(3);
  myList.push_back(4);
  myList.push_back(5);
  myList.push_back(6);  

  for (SimpleList<int>::iterator itr = myList.begin(); itr != myList.end();)
  {
    if ((*itr) == 6) // delete this value therefore will not printing
    {
      Serial.print ("Erasing ");
      Serial.println (*itr);
      myList.erase(itr);
      //itr = myList.erase(itr); // You can do this, this does the same as the previous
      // myList.erase(itr++); // You can't do this.
      continue;
    }

    Serial.println(*itr);
    ++itr;
  }
  
 Serial.println ("Done");
}

void loop()
{
  // Nothing here
}

Look at the output:

Starting
1
2
3
4
5
Erasing 6
1
2
3
4
5
Done

Two iterations through the list. Doesn't look right.

case_switch_basket:
Nick Gammon, thanks for commenting on this thread. Is name calling really appropriate? You write on your posts that you're a "moderator"? What kind of atmosphere are you moderating here? You want me to take name calling in good spirit? I don't think PaulS is particularly smart or dignified. So when is it appropriate for someone to name call him?

You started the personal attacks:

It seems a bit rude that it should take 2 vague, deliberately unhelpful posts and a helper to get that information, but never the less, thank you both for providing it.

Prior to you calling him vague and unhelpful he had posted:

PaulS:
How much memory are 10 of these going to use? There is the overhead of a class to add and remove instances, too.

That's a simple question.

And:

PaulS:
For what definition of "good measure"? The Arduino doesn't have terabytes of memory, you know.
No. It could possible change whether you have a problem, or not, though.

A simple assertion. I don't know how you can call that rude and unhelpful.

Then you (not him) launch into personal attacks:

case_switch_basket:
You are an unkind forum poster. Please do not submit responses to my posts in the future, they are unwelcome.

We are unpaid volunteers here. We try to help. Most times people thank us and realize that they have learned something. The other times someone who has just joined the forum starts spraying accusations around that we are unhelpful and starts sending in reports to moderators, claim this is an unpleasant forum, and so on. I'm starting to think these are people just gaming the system. That is, posting rude posts and then attacking us for responding to them.

Take it somewhere else if you don't want help here.

While you were busy posting an attack on me, I made about three posts trying to solve your problem. Sheesh.

If you want to lodge a formal complaint please address it to:

webmaster@arduino.cc

Feel free to explain to them how a poster who has made 56000+ posts (PaulS, he must sit at the keyboard all day helping people) is in fact unhelpful.

Then go on to explain about how bad the moderators are.

In the event that you get us both banned perhaps you will step into our shoes and spend half the day, every day of the week, helping people in our place.

Snarking aside,

If you know that you will to need to store no more than 10 objects at a a time, then a ring buffer will be simpler and probably take less memory than trying to use a set of C++ classes.

Be aware, though, that this:

struct SystemMessage {
    uint8_t messageType;       // TEXT_UPDATE | MOVEMENT_REQUEST
    char data[100];
  };

struct SystemMessage ringBuffer[10];

int buffer_read_index = 0;
int buffer_write_index = 0;

is 1020 bytes.

I can't tell if you're trolling me or if you're really not getting it. PaulS' comments were vague and accusitory as if I was supposed to know everything about Arduino before I posted. Why would I post a question if I already knew everything. You understood what he was talking about because you're aware of the high importance of SRAM conservation. A newcomer is not born with this information. Posting vague and accusatory comments to newcomers should be discouraged. I get that you're volunteering to participate in the internet, but EVERYONE is doing that. Should everyone be smug and demand gratitude by being as narrowly helpful as possible? The ironic thing is that if this community weren't promoting pointles troll posts like PaulS's first two, I wouldn't even need to post this thread, I could just lookup the information from google in a clean readable way, and maybe some day, I would even execute code that resulted in memory shortages (unlike in this case, where I wouldn't have actually produced more than 3 of those objects at a time) and I would revise my approach to programming the Arduino.

I NEVER made a personal attack. That word has a specific meaning and you aren't using it correctly. A personal attack is when someone attacks the person, say, calls them an idiot, or says they have no real skills and shouldn't be posting. Telling someone they've been unkind and requesting discontinuation of messaging is NOT a personal attack. And my telling you that the way you moderate is very backward and counterproductive is not a personal attack either, it's criticism of you official behavior on these forums.

Thanks anyone with good intentions.