0

I'm trying to create a function that will take a String as an input, and at every 20 characters, it will instert "\n" so that the string doesn't go off the side of my screen (I'm using an oled with the SSD1306Ascii library).

This is my code so far, but when I run it nothing appears on the screen (I have another part of the code which passes the string to the funtion):

String message = "";
String newMessage = "";

void printMessage(String message) {
  oled.clear();
  if(message.length() > 20) {
    current = 20;
    while(current < message.length()) {
      newMessage = message.substring(current-20, current) + "\n";
      newMessage = newMessage + message.substring(current+1);
      current+20;
    }
    oled.println(newMessage);
  }
  else {
    oled.println(message);
  }
}
3
  • Sorry, forgot to mention the variables I have decalered, will change that now
    – user63880
    Commented Jun 27, 2020 at 13:47
  • One problem that strikes me is that if the length of the passed message is 20 or less, newMessage will remain empty.
    – DarioP
    Commented Jun 27, 2020 at 14:02
  • @DarioP Good point, I've updated the code.
    – user63880
    Commented Jun 27, 2020 at 14:06

2 Answers 2

0

I think that your implementation is fundamentally flawed. In the while loop, when you assign newMessage to a message.substring you are discarding all the work done in the previous iterations.

Also I would recommend keeping the splitting and the printing separated. The operator += also is very handy here. This code (untested) should implement your original idea in a correct way

String splitMessage( const String & message, const unsigned limit) {
  String newMessage = "";
  unsigned i = 0;
  while ( i+limit < message.length() ) {
    newMessage += message.substring(i, i+limit);
    newMessage += '\n';
    i += limit;
  }
  newMessage += message.substring(i);
  return newMessage;
}

Then you can do:

whatever.println(splitMessage(myMessage, 20));

Note that I retained the use of String from your original implementation, although, being in an embedded environment, it is likely better to avoid it in favor of stack-allocated c-strings.

I have found a substantial amount of pretty enlightening information in the various answers to this question: Is using malloc() and free() a really bad idea on Arduino?

5
  • Ah, thats far better than what I came up with, thanks. Your code worked perfectly, however I did need to change "message.size()" to "message.length()"
    – user63880
    Commented Jun 27, 2020 at 14:51
  • String object create a memory block at the heap using malloc(), dynamically create a String object newMessage and return the String object from a function is not a good practise as it could create memory leak in long run because the heap memory for newMessage never get free up. For each call to the splitMessage(), a new block of memory is created at the heap....
    – hcheung
    Commented Jun 28, 2020 at 14:55
  • @hcheung I got you. I am a C++ programmer still getting familiar with the flaws of Arduino "language". Would it be better to return the result through a non const reference argument? Like void splitMessage(String & out, const String & in, const unsigned limit)?
    – DarioP
    Commented Jun 30, 2020 at 9:22
  • As a c++ programmer, you should know the pitfall of malloc() and heap usage well. String object behind he scene uses malloc() to create a variable when you do something like String newMessage = "";, and each iteration of concatenation also cause heap fragmentation. The general idea is not to use String in Arduino (with only 2k RAM), if you really want to use it, in this particular case, I would declare it as a global, so that you can free it afterward, but again, if you are doing it that way, it make not much different to just use c array instead of String object.
    – hcheung
    Commented Jul 1, 2020 at 0:58
  • @hcheung as a C++ programmer I always enforce RAII so that the allocated memory is automatically freed when the variable goes out of scope, and I have no further concerns about the heap.
    – DarioP
    Commented Jul 1, 2020 at 11:11
0

One issue with both your failed implementation and the previous answer is that they do loads of dynamic allocations and copies. Every time you use the String’s + or += operators you are allocating heap memory for a brand new String, copying the original character data into the newly allocated space, and eventually destroying the original Strings when you don’t need them anymore.

The simplest way to avoid all these copies and allocations is to not build the String you want. Instead, just write() the pieces one after the other on the display. Here is a zero-copy, zero-dynamic allocation solution:

void printMessage(const char *message) {
    size_t length = strlen(message);
    while (length > 20) {
        oled.write(message, 20);
        oled.write('\n');
        message += 20;  // point to the rest of the message
        length  -= 20;  // remaining length
    }
    oled.write(message);
}

Note that the method Print::write(const char *, size_t) is used here to print a slice of the message without ever building it as a string in memory.

On these memory-constrained devices it is always preferable to avoid heap allocation, and thus String objects. However, if for whatever reason you really need to print a String object, then you can use the following overload to print it’s internal buffer without making any extra copy:

void printMessage(const String &message) {
    printMessage(message.c_str());
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.