code improvement two loops and a buffer >> one loop no buffer

Morning all! hope your having a wonderful day! and for those 'Mericans out there Happy Thanksgiving!!!

I have benn staring at this code of mine now for about 2 weeks, it was much longer and now i have gotten it down to this:

void ConvertMsg(String Msg)
{

  int MessageLen = Msg.length() + 1;
  char Message[MessageLen];
  Msg.toCharArray(Message, MessageLen);
  int BufferM[200][8];
  int ni = 0;
  for (int i = 0; i <= MessageLen - 2; i++)     // -2 characters from message length for null terminator...
  {
    for (int j = 0; j <= 7; j++)               // Lopp to Load Buffer (would like to not use anymore)
    {
      BufferM[i][j] = BitMask[Message[i]][j];   //LoadBuffer
    }
  }

  // Scanning across the column
  for (int i = 0; i <= MessageLen - 2; i++)
  {
    for (pxl_clmn = 0; pxl_clmn < 21; pxl_clmn++)       // Row selection
    {

      if (pxl_clmn > 19) {                            //19 is the actual number of columns
        pos++;
        pxl_clmn = 0;
      }
      if (pos >= ( MessageLen - 3 ) * 8 - 2) {
        pxl_clmn = 21;
      }
      for (int F = 0; F <= 150; F++)       // F controls speed valid variables are 40 to 200
      {
        digitalWrite (rowArray[ pxl_clmn], HIGH); // Make anode HIGH to turn LED on
        PORTL = BufferM[pxl_row][pxl_clmn + pos];  // Row value directly load to PORTL(pin 41 to 48)
        digitalWrite (rowArray[pxl_clmn], LOW);   // Turn off column
      }
    }
  }
  pos = 0;
}

I would like to get rid of loading the information into a buffer and instead just output it to the pins from the bitMask variable, BitMask is a 2 dimensional array that contains the port ON|OFF values for a matrix LED, what i am trying to do is skip loading that information from the BitMask Variable into the Buffer variable, saves me about 800 bytes of memory if i dont have to use the buffer. plus i am trying to get the program to consume as little time as possible and keep a good refresh rate so if i could remove that loop i imagine 800 to 1600 bytes takes a long time to transfer and load. Im pretty sure there is a bug in this code but i cant figure out a better way to implement it, the bug being that in the PORTL set, the variable pxl_row is always 0 but it still prints the entire message from the 2d array that is buffer.

Any suggestions would be appreciated... TY in adv.

int BufferM[200][8];

do you have any memory left after using up 3200 bytes here?

Yup, after getting help solving my PROGMEM issue, i have

Global variables use 977 bytes (11%) of dynamic memory, leaving 7215 bytes for local variables. Maximum is 8192 bytes.

also if im not mistaken

8 * 200 = 1600 bytes...

tbillion:
also if im not mistaken

8 * 200 = 1600 bytes...

Integers are two bytes each.

  for (int i = 0; i <= MessageLen - 2; i++)     // -2 characters from message length for null terminator...

The null terminator is only one byte. This would be better written as...

  for (int i = 0; i < MessageLen - 1; i++)     // -1 characters from message length for null terminator...

So you expected to see at least 1600 bytes used by this code? But you think you are safe because the compiler only reports 977 in use? I'm sorry to say that this is a "local variable" and must fit into the 7215 available. And you don't get to use all of that because you have other local variables, such as Message[], which could fill up all your available memory on its own, since you never check its size is reasonable.

It would be really helpful to see the declaration of BitMask[][] right now. And what is the contents of Msg? You are treating it like it's binary but I suspect you have characters there. So you're only able to access a couple of bytes within BitMask.

Getting rid of BufferM[][] is pretty easy, if that's all you need to do. Since BufferM[ i][j] is set to BitMask[Message[ i]][j], all you have to do is replace the one instance of BufferM with BitMask, and replace the indices in that way:

void ConvertMsg(String Msg)
{

  int MessageLen = Msg.length() + 1;
  char Message[MessageLen];                  // Can you do this?!?
  Msg.toCharArray(Message, MessageLen);
  int ni = 0;

  // Scanning across the column
  for (int i = 0; i <= MessageLen - 2; i++)
  {
    for (pxl_clmn = 0; pxl_clmn < 21; pxl_clmn++)       // Row selection
    {

      if (pxl_clmn > 19) {                            //19 is the actual number of columns
        pos++;
        pxl_clmn = 0;
      }
      if (pos >= ( MessageLen - 3 ) * 8 - 2) {
        pxl_clmn = 21;
      }
      for (int F = 0; F <= 150; F++)       // F controls speed valid variables are 40 to 200
      {
        digitalWrite (rowArray[ pxl_clmn], HIGH); // Make anode HIGH to turn LED on
        PORTL = BitMask[Message[pxl_row]][pxl_clmn + pos];  // Row value directly load to PORTL(pin 41 to 48)
        digitalWrite (rowArray[pxl_clmn], LOW);   // Turn off column
      }
    }
  }
  pos = 0;
}

You're using pxl_clm + pos to index into an array that's only declared as 8 characters big. pxl_clm can be 21? And that works?!?

Also, pxl_row doesn't seem to ever be set. Is it a constant?

You're using pxl_clm + pos to index into an array that's only declared as 8 characters big. pxl_clm can be 21? And that works?!?

weirdly enough it does work. I figured it was some sort of buffer overflow that actually made it work but i had also figured maybe it was a bug, that is remnant from the only of the code i have found on the internet that actually drives a matrix LED with the actual ports on the arduino instead of a shift register or max7219.

and

Also, pxl_row doesn't seem to ever be set. Is it a constant?

nope, its like a cork in a sinking boat its there to hold it all together, i have tried to change it to a few different things to make it work in a what that i would think it would work and it just messes things up. the same thing can be said for every time i remove the BufferM[][8]

So you expected to see at least 1600 bytes used by this code? But you think you are safe because the compiler only reports 977 in use? I'm sorry to say that this is a "local variable" and must fit into the 7215 available. And you don't get to use all of that because you have other local variables, such as Message[], which could fill up all your available memory on its own, since you never check its size is reasonable.

no i knew that the 1600 count against the 7215 remaining, thuis forum is frustrating at times, it forces uyou to either give too much information or too little there is no goldie locks zone.. seriously... for 1. message or buffer of any differentiation of their duplicates never exceed 100ish ascii characters, all of the messages are static, i literally sit here for the time being and where i am in the code and count the letters to be sure. at this point im not even sure this code is the best option for what i am doint i am just trying to work with what i have and make it at least decent. As crap as that looks, it does work ... i dont wanna waste a bunch of time on checking if my input is the right size if i am going to have a drastic revision next week and scrap it all...

It would be really helpful to see the declaration of BitMask[][] right now. And what is the contents of Msg? You are treating it like it's binary but I suspect you have characters there. So you're only able to access a couple of bytes within BitMask.

i would have posted the whole code but i got my butt chewed 2 days ago for posting too much code so at this point i have posted too little code, lemme see whats relevant at this point...

i have changed the code that was originally posted to accomadate the usage of a PROGMEM table below are the results. i reverted BitMask[][8] to its predecessor CharData[][8] when i made the switch to Progmem...

void ConvertMsg(String Msg)
{

  int MessageLen = Msg.length() + 1;
  char Message[MessageLen];
  Msg.toCharArray(Message, MessageLen);
  byte BufferM[200][8];
  int ni = 0;
  for (int i = 0; i <= MessageLen; i++)     // -2 characters from message length for null terminator...
  {
    for (int j = 0; j <= 7; j++)     // ByteRowLoop
    {
      BufferM[i][j] = pgm_read_byte(&CharData[Message[i]][j]);
    }
  }

  //while (int i <= MessageLen)                                     // Scanning across the column
  for (int i = 0; i <= MessageLen; i++)
  {
    for (pxl_clmn = 0; pxl_clmn < 21; pxl_clmn++)       // Row selection
    {

      if (pxl_clmn > 19) {                            //19 is the actual number of columns
        pos++;
        pxl_clmn = 0;
      }
      if (pos >= ( MessageLen - 3 ) * 8 -1) {
        pxl_clmn = 21;
      }
      for (int F = 0; F <= 150; F++)       // F controls speed valid variables are 40 to 200
      {
        digitalWrite (rowArray[ pxl_clmn], HIGH); // Make anode HIGH to turn LED on
        PORTL = BufferM[pxl_row][pxl_clmn + pos];  // Row value directly load to PORTTD(pin 1 to 7)
        digitalWrite (rowArray[pxl_clmn], LOW);   // Turn off column
      }
    }
  }
  pos = 0;
}

CharData[][8] is declared like this :

const uint8_t CharData[][8] PROGMEM = {
  {B11111111, B11111111, B11111011, B11111011, B11000010, B11111011, B11111011, B11111111, },
  {B11111111, B11111111, B11111011, B11111011, B11111011, B11111011, B11111111, B11111111, },
  {B11111111, B11111011, B11101010, B11000010, B11010011, B11000010, B11101010, B11111011, },
  {B11111111, B11111111, B11100110, B11101001, B01001001, B11101001, B11100110, B11111111, },
  {B11111111, B11111011, B11010011, B11000010, B01000000, B11000010, B11010011, B11111011, },
};

and is actually a table of 214 different characters, 8 bytes containing 8 bits = 64 LED ON|OFF values
each byte represents a row of leds 8 leds wide ... im not sure if im explaining this clearly ...

11111111     top row of matrix letter
11000000
01000000
01111011
01111011
01000000
11000000
11111111     bottom row

And what is the contents of Msg? You are treating it like it's binary but I suspect you have characters there. So you're only able to access a couple of bytes within BitMask.

where am i treating Msg like binary?

void ConvertMsg(String Msg)

its declared as a string in the function ...

the function is called from all over the place in the code, this is the simplest complete call i have

void ErrorMsg(int error)
{
  String Msg;
  String stringError =  String(error, DEC);
  if (error == 1) {
    Msg = String("    DHT 11 Error     ");
  }
  ConvertMsg(Msg);
}

i think the purpose of this code is confusing yall a bit... the function takes literal string characters in a sentence format, i have CharData[][8] configured in a manner that CharData[65][] for example is the letter A, so ...it takes Msg and converts it to the character array Message, then i defined buffer[200'][8] just as a patch to try it out really if i was to fix it it would say BufferM[MessageLen+2][8] from there the led matrix ON|OFF values which are stored as an ASCII table in PROGMEM are mapped to the buffer
so what the buffer actually holds is the LED Binary representation of the sentence. somthing like 142 ON|OFF values. then
i cycle through the buffer and do the whole manual LED thing.. which again is in my opinion wrong but works great. i was just trying to do away with the buffer.

dougp:
Integers are two bytes each.

which is hilarious on an 8bit processor, but i had forgotten that, need to change that to byte type, and i can come down even more memory :slight_smile:

The null terminator is only one byte. This would be better written as...

well this null terminator for some reason is represented by 2 characters, i had tried the -1 instead of -2 prior to this and i would continually get + and - as characters at the end of my strings on the Matrix LED

@tbillion, temporarily make all your variables global and see how much memory usage is reported when you compile your program.

Using local variables can make your code nice and neat by restricting scope but it may also result in a need for more SRAM than if you use global variables.

And everybody seems to avoiding the elephant-in-the-room which is on the very first line

void ConvertMsg(String Msg)

It is not a good idea to use the String (capital S) class on an Arduino as it can cause memory corruption in the small memory on an Arduino. Just use cstrings - char arrays terminated with 0.

...R

thats not an elephant.. call it for what it is .. a dirty hack.. lmao... it was the only solution i could come up with to make the ide stop giving me character mismatch errors. i normally avoid strings like the plague. but for this project it was the easiest way to go from quoted text to LED matrix...

Sketch uses 14256 bytes (5%) of program storage space. Maximum is 253952 bytes.
Global variables use 2584 bytes (31%) of dynamic memory, leaving 5608 bytes for local variables. Maximum is 8192 bytes.

give or take...