Add room() function to Serial Class to see free TXbuffer space (HWS & SWS)

[RobTillaart]

Sometimes one wants to know the amount of free places in the transmit buffer

The function to check this is straightforward.

int room()
{
  return (SERIAL_BUFFER_SIZE + tx_buffer.head - tx_buffer.tail) %  SERIAL_BUFFER_SIZE;
}
// % can be optimized with an if ()

this makes it possible to check and fill up the ring buffer to the max (esp when using low baud rates) without overflow.

• http://arduino.cc/forum/index.php/topic,165574.msg1235362.html#msg1235362

The name room() is not as ambiguous as space which already has a meaning in serial communication. Available() is already used for receive.

[pwbecker]

int room()
{
return (SERIAL_BUFFER_SIZE + _tx_buffer->head - _tx_buffer->tail - 1) % SERIAL_BUFFER_SIZE + 1;
}

[RobTillaart]

There allready exists the following function in hardwareSerial. Think it is
pretty much what you propose, just another name.

int HardwareSerial::available(void)
{
return (unsigned int)(SERIAL_BUFFER_SIZE + _rx_buffer->head -
_rx_buffer->tail) % SERIAL_BUFFER_SIZE;
}

On Sat, Sep 21, 2013 at 3:11 AM, pwbecker [email protected] wrote:

int room()
{
return (SERIAL_BUFFER_SIZE + _tx_buffer->head - _tx_buffer->tail - 1) %
SERIAL_BUFFER_SIZE + 1;
}

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/1408#issuecomment-24853333
.

[RobTillaart]

Sorry, your proposal is on the transmittting site. Posted too quick. :)

Have you checked - https://code.google.com/p/arduino/issues/list - if a
similar proposal is made as it sounds familiar..

On Sat, Sep 21, 2013 at 3:13 PM, Rob Tillaart [email protected]:

There allready exists the following function in hardwareSerial. Think it
is pretty much what you propose, just another name.

int HardwareSerial::available(void)
{
return (unsigned int)(SERIAL_BUFFER_SIZE + _rx_buffer->head -
_rx_buffer->tail) % SERIAL_BUFFER_SIZE;
}

On Sat, Sep 21, 2013 at 3:11 AM, pwbecker [email protected]:

int room()
{
return (SERIAL_BUFFER_SIZE + _tx_buffer->head - _tx_buffer->tail - 1) %
SERIAL_BUFFER_SIZE + 1;
}

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/1408#issuecomment-24853333
.

[PaulStoffregen]

Just a quick mention, on the Arduino Developers Mailing List, this feature was discussed. Cristian decided this function would be named writeBufferFree(), if it's ever implemented in Arduino.

Here's the mail list conversation, for reference.

https://groups.google.com/a/arduino.cc/forum/#!msg/developers/ls3hkviFYM4/XWT2LbfzNcgJ

[RobTillaart]

Seen (parts of) the discussion, think it is convergating in the right direction.
Thanks for the notification.

[bperrybap]

In my view this should be handled up in the stream class just like available() rather than down in HardwareSerial.
Having it is stream ensures that it is handled the same way for all libraries that use the stream class libraries vs being just a capability that is proprietary to HardwareSerial.

[matthijskooijman]

Better have it in Print rather than Stream, but otherwise agreed. Moving it upwards does complicate backward compatibility for implementations of Print that do not have this new function implemented yet, but discussion about this is already happening in the same thread linked above.

[bperrybap]

Print currently doesn't inherit from Stream so I don't see the issue of backward compatibility for Print if the function was added to Stream. The potential issue is for all of those that inherit from Stream, but if there was a default function in Stream, then even those would be able to still link.

[jantje]

Funny
Lots of the discussion Paul linked to is what I went through in issue #1930
I opted for -1 as the default implementation as it gives a way (at programming time) to see that the default has not been overloaded.
I do think having the method in stream is vital (at least it is for me) and only hardware Serial is really bad (and no solution to me) because replacing the serial to a software serial is going to break the code (and not only the logic)

[matthijskooijman]

@bperrybap, but I'm proposing to put the function in Print, not in Stream (since the former is about outputting things, just like the new writeBufferFree() function).

@jantje, the downside of returning -1 (as opposed to 1 or a large positive value) to signal "not implemented" is that callers are forced to check the return value - there is no automatic, graceful fallback. In the regard, using an unsigned return value and then returning -1 (== MAX_INT as unsigned) might be better as it allows automatic fallback at least in some cases, as well as not requiring a signed return value (which doesn't make any sense for available buffer space).

As for the naming of the functions - I think we discussed that on the mailing list and a decision was made, so I'm not going into that. I agree that we should put this method in Print (which is a superclass of Stream, so it ends up in Stream as well) and not just in HardwareSerial and other implementations.

[PaulStoffregen]

Jan, could you elaborate on "I do think having the method in stream is vital (at least it is for me) and only hardware Serial is really bad (and no solution to me) because replacing the serial to a software serial is going to break the code"?

Sometimes explaining just one important real application can be much more persuasive that simply claiming you need a feature. When you say "is going to break the code", if you explain what code this is, how it will break, and why that's important, the use of a specific case can really add weight to your argument.

Just to be clear, I'm not trying to argue either for or against this idea. I'm only trying to help you make a stronger case for it. So I guess in an indirect way, that means I'm for it?

I agree with Matthijs about the name. Cristian already said he made a final decision. Only Cristian can change the decision on the name.

[jantje]

@PaulStoffregen

Theory

The whole point of having a base class is to make abstraction from the implementation. By putting the solution in hardware serial we make a mistake agains basic oo principles. Here are the consequences I think of.

My case

In my case I need it for the leonardo (that is also yun and esplora and probably your teens as well) they do not have a hardware serial.
leonardo code

Serial_ Serial;
....
class Serial_ : public Stream

Board compatibility: "is going to break the code"

So when my code working on the UNO is ported to the leonardo it will not compile as Serial_ does not contain the new method (whatever its name)
I don't know the due, tre, zero but I won't be surprised when something similar will be there.

The BridgeClass; a Arduino example why it is needed.

Look at the bridge (which is written with the leonardo in mind)
BridgeClass(Stream &_stream);
This clearly proves there is usage for stream in libraries. By putting the method only in HardwareSerial we seriously limit the possibilities to use this method in libraries.

Flexibility: "is going to break the code"

But it goes further than that. Suppose I have my code working on a UNO and then I need to move it to alt software serial because I need to free up Serial for something else.
My code won't even compile because ... see above

[matthijskooijman]

So when my code working on the UNO is ported to the leonardo it will not compile as Serial_ does not contain the new method (whatever its name)

This is of course fixed by implementing a method with the same name in CDCSerial (the class the Leonardo uses for the virtual serial port). Same goes for SoftwareSerial and others.

That's just a small remark on that particular argument - I totally agree with your point. Especially the example of the BridgeClass is important - we want to support calling the writeBufferFree method on an arbitrary Stream implementation without knowing which it is.

[jantje]

@matthijskooijman

@jantje, the downside of returning -1 (as opposed to 1 or a large positive value) to signal "not implemented" is that callers are forced to check the return value - there is no automatic, graceful fallback. In the regard, using an unsigned return value and then returning -1 (== MAX_INT as unsigned) might be better as it allows automatic fallback at least in some cases, as well as not requiring a signed return value (which doesn't make any sense for available buffer space).

Technically I see very little difference between returning -1 or MAX_INT as default value.
Why?
As this is a new method we do not need to bother about downwards compatibility. So we are speaking about new code. We are also speaking about a coder who is concerned about delays (and other impact) that may be caused by a buffer being overflowed.
So the only difference is that if you are concerned about the impact, you can avoid to test the absense of a proper implementation and suffer the impact anyways. (need a deuuuuch smily here) It just doesn't make sense to me.
I for one will want to test and write a message to the serial port to alert the absence of the method in all cases.
I would consider that good practise.

From a "architectural point of view" I think -1 is the way to go.
Using MAX_INT means we assume/agree that the buffer will never reach maxint as buffer size. The due has 96 KBytes of SRAM. When I bought a pc with 20MB of hard disk I was told I would never be able to to fill 20MB. If I make a app on a due that only grabs audio and sends it over serial, why would I need to limit the max buffer size because of this?
This mistake has been made plenty of times in the past. Please lets not make this mistake again.

PS I tried to use MAX_INT but it seems to be unknown in the arduino development environment.

[jantje]

about print or stream
Currently available is defined in Stream so I added my code to Stream.
Available is a pure virtual method. Moving available to print means following classes will no longer compile: Server Keyboard_
At this point in time I have no technical preference to putting the code in Print or in Stream.
From a organisational point of view I consider having the code in Stream a big victory. As adding the code to Print creates more change I support the Stream solution.

[jantje]

Thinking about Stream versus print again.
Available should stay in Stream (as it is bi directional) and the new method should go to Print (which is output only)
This solution (when implemented as I proposed) is technically better and will not break anything. It still has the "Harder to get agreement on" drawback. Therefore my conclusion does not change.

[matthijskooijman]

Available should stay in Stream (as it is bi directional) and the new method should go to Print (which is output only)

That's exactly what I meant.

[bperrybap]

On 07/08/2014 06:59 AM, Matthijs Kooijman wrote:

Available should stay in Stream (as it is bi directional) and the new method should go to Print (which is output only)
That's exactly what I meant.

—
Reply to this email directly or view it on GitHub #1408 (comment).

Yes, but I'm not sure that Jan agrees with you.
His previous post was a bit ambiguous on the language.
My interpretation of what was said:

Thinking about Stream versus print again.
Available should stay in Stream (as it is bi directional) and the new method should go to Print (which is output only)
This solution (when implemented as I proposed) is technically better and will not break anything. It still has the "Harder to get agreement on" drawback. Therefore my conclusion does not change.

Was that the second sentences was simply explaining the proposal for re-consideration
Then the third sentence was explaining how Jan's current solution of
having it in Stream was "technically better".
and the 5th/last sentence said "Therefore my conclusion does not change".
I didn't take any of that to mean a preference to moving it to Print
but rather that even after further consideration having it in Stream is
still the best solution.

We will need to get clarification from Jan to be sure what was meant.

My view is that this should not be added to Print
Print is for data output formatting. It is essentially the "printf" of
arduino. Formatting output really doesn't have anything to do with the actual i/o services
so it should not implement any i/o services or i/o control but merely
use i/o services implemented elsewhere.

I believe that Stream should be the i/o class and an output formatting class
should not have to be included just to get some of the the i/o services.

If I'm doing i/o but not doing any formatted data output, why should I have to include
a class that does output formatting?
Granted it is currently transparently included by Stream but it seems very twisted.

In terms of going down the path of adding any new virtual functions to Print,
Keep in mind that MANY libraries use Print for formatted output without
Stream so if you add new virtual functions to Print you must have a default
function to keep from breaking all that existing code.

[jantje]

Seems my last comment is not fully understood.
With "implemented as I did" I meant: a -not pure- virtual function returning -1 as default

I think technically the new function is -again from a technical/coding point of view- best placed in print.
The main reasoning is that print is actually a one directional buffered class. This is pretty clear if you see keyboard is inherited. (don't ask me why server is as that seems bidirectional to me.)

Class wise it is defend-able that you derive a 2-directional class from a 1 directional class. ( I would not do so but that is what Arduino did.)
In this solution the "how much room is there in my output buffer" is going to be in a different class then "how much room is there in my input buffer"
So in Arduino world the following is logical : One directional buffer=print class; so print class should own: "how much room is there in my input buffer" ("input buffer" because Stream adds "output buffer")

However due to the naming (print and stream) and due to the OO rules (print should not know about stream) you get a big naming issue. Available (not specifying by name what is available in a 2 buffered class) is a really nice example of how quickly it all becomes confusing.

Rereading the above I'm not sure it is making thing clearer. So I'll try to answer the questions:
I agree with matthijs that if you see the print class as a single direction communication channel and the Serial class as a dual communication channel that the most common sense thing to do is to have a "available method" in both the Serial and print class. Each one for the channel/buffer they add.

However .......

......this will open a whole discussion on
class names (FI print is not a good name for a single direction communication channel ), method names (You can not name the method in print writebufferfree as print only knows one buffer)
inheritance (Wouldn't it be it more logic that Serial has 2 members of "single communication channel")
architecture (would it not be better to have a "buffered channel" and derive print from there?)

The whole discussion will make that the solution will never be implemented. So therefore my vote goes for adding the new method to the Serial class.
So for me the implementation as described in #1930 is the most viable solution.

in stream.h add following methods:

    int  availableInReceiveQue(){ return available();};
    virtual int  availableInSendQueue() {return -1;};

In any class you want the available send buffer size do like in hardware serial:
in hardwareSerial.h add

virtual int  availableInSendQueue();

in hardwareSerial.cpp add

int  HardwareSerial::availableInSendQueue()
{
    return (unsigned int)(SERIAL_TX_BUFFER_SIZE + _tx_buffer_head - _tx_buffer_tail) % SERIAL_TX_BUFFER_SIZE;
}

Note as I only added stuff this is 100% downwards compatible.
And this is the whole solution.

[matthijskooijman]

@bperrybap

If I'm doing i/o but not doing any formatted data output, why should I have to include a class that does output formatting? Granted it is currently transparently included by Stream but it seems very twisted.

I don't really think Print is intended as an output formatting class, it just represents a writable (but not readable) stream of data. It's not like there is an Output class that you wrap inside a Print class to add output formatting, Print itself does the outputting (through the virtual write method). The names of Print and Stream might not be perfect, but I've always understood them to mean in output, and input/output interface respectively. In that regard it also makes sense that Stream inherits from Print, since it allows code that works with output only (Print) to also work with classes that offer I/O (Stream).

@jantje

PS I tried to use MAX_INT but it seems to be unknown in the arduino development environment.
I just used that as an example. The real constant is probably named differently and might not even be available on AVR. Using ((int)-1) should work to get to the same value, though.

(don't ask me why server is as that seems bidirectional to me.)

AFAIU, Server allows you to write to all clients connected to the server simultaneously. Reading from all clients at once doesn't make too much sense (will you get bytes interleaved?), so they just implemented Print on Server. The Client class, which models a connection to a specific client, does implement Stream.

Otherwise I mostly agree with what jantje is saying. Two caveats:

• I'm not entirely convinced about the naming of the methods - I like jantjes proposal, but Christian already picked another name - so this should probably be discussed on the mailing list again.
• I don't like returning an int (e.g., size_t or unsigned int would make more sense). I see jantjes point about not wanting to disallow a buffer of size 2^16-1. On the other hand, if you switch to int, you disallow all buffer sizes >= 2^15 sine the range is halved, so I'm not sure if this point really holds. If some future piece of hardware has allows bigger buffers, then the return type will have to increase in size anyway (hence using size_t makes sense - it is guaranteed to contain the largest possible object in memory on the architecture). If we also use a constant (e.g. WRITE_BUFFER_FREE_UNSUPPORTED) to return as default, then we can always make sure that that constant is the biggest possible value (e.g. (size_t)-1) for the return type, which should not unreasonably limit the buffer sizes.

[jantje]

I'm not entirely convinced about the naming of the methods - I like jantjes proposal, but Christian already picked another name - so this should probably be discussed on the mailing list again.

This code predates Christian his decision. I'm to old/experienced to fight for a name. Name them like Christian wants it.

I don't like returning an int (e.g., size_t or unsigned int would make more sense). I ....

Very good points.
Note that my only argument for int is that available returns an int. If christian is happy with 2 methods I agree size_t is the best way to go. If only 1 method is added ... I would go for the path of least resistance.
Indeed having -1 as a return value forces signed and halves the potential range.
A custom(?) Define at the max size is a better idea. I'm not so well known about size_t so I leave that up to the experts. (I would not be surprised -1 is the solution in C (2-complement of -1 = 0XFFFFF....) but as GCC goes more and more to the C++ specification that may become incompatible in future versions.)

[jantje]

I just did the mod in Arduino IDE 1.5.7. Works great.

[bperrybap]

There have been so many mods discussed, which one did you do?

[jantje]

mine :-) because I did that one in 1.5.5 as well.

[cmaglie]

Fixed with #2193