Simple, useful additions to HardwareSerial

/dev: Another point: this opens the door to memory leaks. :(

Cheers, /dev

Memory leaks are caused by poor programming.

Krupski: Memory leaks are caused by poor programming.

And heap fragmentation.

The latter of which is only solved by more code (when possible) or resetting.

westfw: Has anyone ever explained why Strings seem to cause problems on AVR Arduinos (when used a lot like this), other than handwaving and mumbling about fragmentation and how malloc() is bad on embedded systems?

Don't honestly know the answer to this. I've never used Strings, but I suspect most of the "problems" are users who don't know how to properly use the class and/or those who just repeat all the negatives that they "heard online" without actually knowing if any of it is true.

O M G ! I T B L O C K S ! ! ! ! ! ! ! !

My "full" version of readline actually has a few extra features that I left out of the one I posted (didn't want people bursting arteries in their heads). The "full" version also has an "echo" flag (true or false - echo input or not) as well as a "password mode" flag which echos whatever character is used for the flag (that is, 0x00 echos the keys as pressed, and for example 0x2A ('*') echos an asterix for each key pressed).

Krupski: (3) I'll have to check that. I expected that most people would be using Windows or Linux or Mac terminal programs (all of which send a CR when "enter" is pressed). Some implementations of a "terminal" (like using Screen in Linux as a terminal) send a LF as end of line. However, since the code will consider a CR or an LF as an end of line, a CR/LF would end the line (via the CR), then the LF would be waiting in the buffer and would immediately exit the readline next time around (this time via the LF). Very unlikely to run into this, but it should be handled for the sake of cleanliness.

I'd expect the most common terminal program used by Arduino people is the Arduino IDE Serial Monitor, which does have a "Both NL & CR" option but it's not the default.

The Arduino Serial object wraps a C UART core, so I gutted the C++ and cured the C with some seasoning to taste. I use picocom and python-serial on Raspbian and Ubuntu to interact with a parser I use with the resulting UART core. I'm probably not handling the command line correctly but it seems to work with picocom and Python when I simply emit a "\r\n" after receiving either of them alone.

I added some Stream declarations to the UART core for stdio, that way I can use the standard library functions. I assign the stdout and stdin from the initializer.

stdout = stdin = uartstream0_init(BAUD);

Then use recognizable standard library functions.

printf("hello world");
char mychar = getchar();
printf_P(PSTR("%dx the flash %s help"),2,"string");

http://www.nongnu.org/avr-libc/user-manual/group__avr__stdio.html

I don't want to criticize the heap and stack usage in C++ cause I don't understand it, but I have had few bad thing going on with plain C and no heap memory usage at all.

RayLivingston: Embrace OOP! Rather than modifying a standard library (BAD idea!), which will break when you update the IDE, create a new class that inherits from HardwareSerial, and simply add your new functionality to that. Put the new class in your local library directory, and it will automatically work on updated IDEs.

Regards, Ray L.

+1

I use this all the time. It works great!

-jim lee

Krupski: (2) It "blocks". So what? What else is a program going to do while it's waiting for user input?

So you have a statemachine running through numerous states and just happily stop it doing what it is doing to wait for user input. Your call ;)

Krupski: (4) Of course not! If you looked at the demo sketch, the buffer is freed AFTER it's used. Freeing it upon exiting readline would return nothing to the caller.

You may or may not know that the standard GNU Readline library (used by Linux, the Bash shell and countless other programs) all return a pointer to the buffer containing the read line which must be freed after use by the user program.

I already mentioned in an earlier reply that I overlooked your demo sketch. And I suggested freeing it when entering the method, not when leaving ;)

Krupski: Then I'd save 1K by not using malloc, and by using FLASH memory for double-quoted string literals, with the F macro."

What on earth are you talking about? You propose to place a dynamic buffer in flash?

Surely you jest.

Um, you said "The whole point for doing this is to use as little ram as possible." But then you wrote this code:

    buf = Serial.readline ("\nType something: ");
    if (buf) {
      Serial.print ("\nYou typed: '");
               other prints...

Those double-quoted strings should be in FLASH. You just wasted 80 bytes of RAM. You should learn how to use the F macro and how to use __FlashStringHelper in your own routines. It looks like you don't know about println, either. These

        Serial.print ("'\n");
        Serial.print ("\n");

... can be replaced by these:

        Serial.println( '\'' );
        Serial.println();

Krupski: Don't honestly know the answer to this. I've never used Strings, but I suspect most of the "problems" are users who don't know how to properly use the class and/or those who just repeat all the negatives that they "heard online" without actually knowing if any of it is true.

It's not just a rumour. Read all about it here, like I mentioned earlier. If you only read one of the links, read "The Evils of Arduino Strings".

Krupski: the standard GNU Readline library (used by Linux, the Bash shell and countless other programs) all return a pointer to the buffer containing the read line which must be freed after use by the user program.

That's the problem. You are obviously coming from a multi-tasking environment with virtual memory. The techniques that you use there are not appropriate here. A multi-tasking OS lets other things run while your app is waiting for input (blocked). An OS with virtual memory lets you allocate more memory than is physically available. Neither of those are available in the typical Arduino environment: 16MHz, 8-bit, 2K RAM.

It's great that you want to contribute, and that you're willing to throw something out there for comment. All the comments seem to be constructive, so I hope you are trying to learn from them.

Cheers, /dev

ron_sutherland: I added some Stream declarations to the UART core for stdio, that way I can use the standard library functions. I assign the stdout and stdin from the initializer.

Since you did that, check out this library: Stdinout

It provides you with an object named "STDIO" and to connect anything to it, you merely do something like this:

``` STDIO.open (Serial); ```

If you are done using it, you can free the few bytes it uses with:

``` STDIO.close (); ```

To close one and connect something else, merely open the new device... open() automatically does a close() first, to save a bit of grief and prevent memory leaks.

You can also specify more than one device. If you specify two, the first one becomes stdin, the second one becomes stdout and stderr.

If you specify all three, then each one connects to it's own stream (i.e. stdin, stdout and stderr).

It's really nothing new, but it's packaged to be foolproof (that is, prevent memory leaks and open/close things in the proper order which, amazingly enough, is VERY important).

pert: I'd expect the most common terminal program used by Arduino people is the Arduino IDE Serial Monitor, which does have a "Both NL & CR" option but it's not the default.

You are 100% correct... and I've been thinking how to handle that occurrence cleanly. I need to think more.... :)

sterretje: I already mentioned in an earlier reply that I overlooked your demo sketch. And I suggested freeing it when entering the method, not when leaving ;)

Can't free the buffer upon entry, because the FIRST time in, the buffer was never allocated and you get a "double free corrupt" error.

/dev: Um, you said "The whole point for doing this is to use as little ram as possible." But then you wrote this code:

    buf = Serial.readline ("\nType something: ");
    if (buf) {
      Serial.print ("\nYou typed: '");
               other prints...

Those double-quoted strings should be in FLASH. You just wasted 80 bytes of RAM. You should learn how to use the F macro and how to use __FlashStringHelper in your own routines. It looks like you don't know about println, either.

Condescending and presumptuous, aren't we?

Why on earth would I put 80 bytes of string data used by a DEMO program when they will fit with tons of room to spare even in a lowly ATtiny25?

And, if I were to put the strings in flash, I would use PROGMEM, not hand-holding wrappers.

As far as "println()" is concerned, know that I DESPISE the whole idea of building a simply line of text up by using a half dozen "Serial.print......." lines (ala the sadistic programming style of Java and Javascript).

Here in the privacy of my own home, I use fprintf and standard printf formatting characters (yes, even for floating point, which causes my "sketches" to be 1400 bytes larger and leave me with "only" about 200K of free flash).

The only reason I even posted the demo using the stupid "setup" and "loop" crap was to prevent confusion in newbies and insults from the "used to be newbies last week" crowd.

Most of my Arduino / AVR programming is done using a text editor, a Makefile and AVR-GCC. I rarely use the IDE. It's great for newbies, it's a heavy, clunky, useless crutch for those who know a little bit more.

Ever see Bambi? Remember what Thumper's mother told Thumper?

Krupski: Can't free the buffer upon entry, because the FIRST time in, the buffer was never allocated and you get a "double free corrupt" error.

No, if it's a static pointer, you can test for NULL.

Why on earth would I put 80 bytes of string data used by a DEMO program when they will fit with tons of room to spare even in a lowly ATtiny25?

You said the goal was minimal RAM, so I thought it was ironic for you to waste RAM. If that’s not the goal, why would you worry about wasting RAM on a buffer that is too big? You said,

The alternative is to allocate a “best guess sized” buffer and hope that (A) it’s big enough and (B) that it’s not too big, thereby wasting ram.

But this is exactly what you do when you use sprintf!

I guess I’m having trouble following your logic. I’m not trying to be condescending, but if you set the criteria, expect us to refer to them.

And, if I were to put the strings in flash, I would use PROGMEM

Ok, but you’ll have to use the __FlashStringHelper type if you want to reuse all the methods that have been written for it. It’s present in the core and many, many libraries. There is no other language element to distinguish between FLASH and RAM: you have to use a distinct type.

As far as “println()” is concerned, know that I DESPISE the whole idea of building a simply line of text up by using a half dozen “Serial.print…” lines (ala the sadistic programming style of Java and Javascript).

Did you read this reply? A series of prints is not from Java nor Javascript. The streaming operator…

    Serial << "RAM string " << f << ',' << HEX << i << endl;

… would be considered the contemporary C++ way of printing multiple items, as opposed to the ancient printf family. And I don’t mean “ancient” in a derogatory way. printf et al have endured because they are so useful. But they are old routines, not the “C++ right way.”

To summarize:

  • You didn’t know that parseInt blocks.
  • You didn’t suggest a derived class of HardwareSerial. This is a classic example of extending a class.
  • You didn’t know why malloc is bad, or that there are good and bad malloc strategies.
  • You didn’t handle CR and/or LF. This is a classic portability issue. Classic.

There is nothing wrong with not knowing these things… we like sharing the knowledge (that’s not condescension). And we can’t read your mind, so we don’t know what you know (that’s presumptive, not presumptuous). But if you don’t like the forum scrutiny, don’t stand on the podium and suggest “simple, useful additions” to the core.

Cheers,
/dev

sterretje: No, if it's a static pointer, you can test for NULL.

You are not seeing the problem. The way it works, in sequence, is this:

(1) Pointer is NULL (2) User types in some characters. Ram is allocated for each byte (3) Upon pressing Enter, the allocated, valid pointer with user text is returned to the caller (4) The caller uses the data (5) The caller frees the allocated ram.

If the pointer were freed before exiting the readline (or upon entering it), the function would always return a null pointer to the caller (i.e. with no data).

Picture a small whiteboard. Imagine I am the user and you are the calling function. You ask me to write something on the board, I do it, then hand the board to you. You do whatever you want with the data, then erase the board and hand it back to me.

However, if I erased the board before I handed it to you, what good would it do you? See?

/dev:
1 You didn’t know that parseInt blocks.
2 You didn’t suggest a derived class of HardwareSerial. This is a classic example of extending a class.
3 You didn’t know why malloc is bad, or that there are good and bad malloc strategies.
4 You didn’t handle CR and/or LF. This is a classic portability issue. Classic.

(1) Which “parseint” are you talking about? If there is some other function with that name and it blocks, then OK. But the one I posted does not block anything. It merely calls “strtol()” and returns the long int value of the numeric text string supplied to it. Sure it uses a few CPU cycles, but that’s not “blocking”.

(2) I fully intended to extend the HardwareSerial class, giving it a few more useful functions. As it is, the HardwareSerial class in the newer 1.6.x IDE has a few added functions (i.e. it’s been extended) beyond what came with the 1.0.x IDE.

(3) Disagree. You still have not told me WHY “malloc is bad”, nor provided any references or links to info to back up that claim.

(4) You’ve got me there. I did indeed “screw up” by not properly handling the odd “CR AND LF” end of line combo.

Quote

As far as “println()” is concerned, know that I DESPISE the whole idea of building a simply line of text up by using a half dozen “Serial.print…” lines (ala the sadistic programming style of Java and Javascript).
Did you read this reply? A series of prints is not from Java nor Javascript.

The streaming operator…
Serial << "RAM string " << f << ‘,’ << HEX << i << endl;

I said “programming style” of Java and Javascript.

And, maybe I can’t see it, but this:

[b]Serial << "RAM string " << f << ',' << HEX << i << endl;[/b]

looks NOTHING like this:

[b]Serial.print ("RAM string ");
Serial.print (f);
Serial.print (",");
Serial.print (i);
Serial.print ("\n");[/b]

which looks a lot like this (Java):

[b]String str = new String;
str += "RAM string ";
str += f;
str += ",";
str += i;
str += "\n";
System.out.print (str);[/b]

As I said, sadistic… making programmers type all that cr…um junk.

Krupski: Which "parseint" are you talking about?

This one. This tells me that you have not investigated the base classes.

You still have not told me WHY "malloc is bad", nor provided any references or links to info to back up that claim.

Dear Sweet Fuse Bits, this deserves Presumptuousness and Condescension, because I've told you twice. Instead, I'll suggest that you go back and read reply #14:

/dev: There is no hand-waving or mumbling here, just the facts. Several links discuss why it's generally bad to use it, but be sure to read "The Evils of Arduino Strings" -- it is focused on our environment.

... or reply #25:

/dev: It's not just a rumour. Read all about it here, like I mentioned earlier. If you only read one of the links, read "The Evils of Arduino Strings".

I must be presumptive and point out that all [u]underlined teal text/u is a link, and that the evil part of the String class is its use of malloc.

Out of cheer, Out of patience, and Out of pearls, /dev

Krupski:
(1) Which “parseint” are you talking about? If there is some other function with that name and it blocks, (3) Disagree. You still have not told me WHY “malloc is bad”, nor provided any references or links to info to back up that claim.

After decades of people researching dynamic memory allocation and, more recently, Google’s indexing efficiency, taking the time to spoon feed you a link should not be necessary but here you go…
https://www.google.com/search?q=realloc+fragmentation

@Krupski, the code you posted has a rather nasty bug. (@sterretje copied the bug.)