Add Print::availableForWrite

[eric-wieser]

If available() is in the base Stream class, then availableForWrite() should be in the base Print class

For backwards compatibility, this can't be pure virtual. So we return INT_MAX, indicating no limit.

Ideally, we'd use size_t and SIZE_MAX, but that's probably not a backwards-compatible change

[eric-wieser]

@matthijskooijman: Shall i rebase this so you can merge?

[cmaglie]

The real question is that isn't clear is if the "default" method should return INT_MAX, 1 or 0.

IIRC an old discussion on the developers list, the consensus was that it should return 0 meaning that, since the method is not implemented, you can't tell in advance if the incoming call to write will block or not. Or in other words the only guarantee is that only a write of 0 bytes will not block.

[eric-wieser]

Thankfully whatever it returns, it won't break existing code.

I think you're right, 0 would make more sense.

[PaulStoffregen]

I would like to suggest the default should be either 1 or a large number, but not zero.

The most common use case looks something like this:

void loop() {
  if (haveDataWaitingToSend) {
    if (Serial1.availableForWrite()) {
      Serial1.write(myData[myDataIndex++]);
    }
  }
  // timing sensitive tasks
}

All availableForWrite() implementations should only return zero when they will later return non-zero. Returning zero will cause the user's code to avoid Serial.write(). Users will expect zero means they should not send their data now, because a better time will become available in the future.

Returning zero doesn't mean "don't know". Zero means "will block, avoid writing if you can't wait".

Please make the default either 1 or a large number, but not zero.

[cmaglie]

The // timing sensitive tasks will continue to work using 1 or INT_MAX?

The point is that in the example above we relies on the availableForWrite() to be sure that write will not block too much and the "timing sensistive task" is handled without too much delay.

If availableForWrite returns always 1 or INT_MAX the while loop will quickly fill the serial buffer and block until all myData is emptied => long delay => timing sensitive task not handled.

So we have to decide if we should let fail the "stream" or the "timing sensitive task":

• if we return 0: the timing sensitive task is handled, but probably no data will go through the stream
• return anything >0: data will pass through the stream but the timing sensitive task will almost certainly fail

[cmaglie]

(well, looking again at the example, there is no explicit while loop, but you've got the idea)

[eric-wieser]

Does this default really matter at all? We could just as easily put while(1); in there, or even just _cxa_pure_virtual().

Let me recap why we need this at all. Consider the following code:

class MyClass : Print {
    void write(...);  // and all the other method existing stuff prior to this patch
}

void doSomeStuff(Print& p) {
    // note that it was never possible to call `p.availableForWrite` in here, since that function didn't exist
}

Now we can either:

1. Make availableForWrite void - this causes a compilation error in the above code
2. Implement a default value - note the above code is not affected, as it never called availableForWrite anyway

If the user now edits their code to call availableForWrite, well then they've almost certainly introduced a bug, because they forgot to actually implement it. We should do everything we can to make this bug more noticeable

[PaulStoffregen]

Indeed any use of the default rather than a real implementation will be something less than ideal. This is a matter of choosing the "least bad" outcome.

Returning 0 pretty much guarantees data will not flow for sketches and libraries which check availableForWrite().

Returning 1 allows data to flow, but with disruption to the timing sensitive tasks. Both do still get to function, though the timing sensitive task is likely to suffer delays. While this isn't good, I'd like to suggest having all parts working but some ir all not performing well is much easier on end users than some parts performing well with other parts not working at all.

This default is likely to be forever used for SoftwareSerial, Soft I2C and other no buffering cases. In the short term, it's also likely to be used for most Print objects which don't yet implement availableForWrite().

[eric-wieser]

it's also likely to be used for most Print objects which don't yet implement availableForWrite().

Which objects are these? I was under the impression that all these objects derive fromStream, which forces them to define their own default anyway as the method is pure virtual?

[eric-wieser]

Ah nevermind, I'm wrong about that - I forgot that this is being moved from HardwareSerial, not Stream

[eric-wieser]

Perhaps -1, indicating "don't know"? Again, there are no backward-compatibility concerns here

[PaulStoffregen]

However, I believe any of these default values is a better decision than "choosing" indecision which leaves this important contribution languishing. Please just pick one.

[PaulStoffregen]

But not -1. Either 0, 1 or INT_MAX. Any of those defaults is better than another release where library authors can't access this function from the base class, and are forced to design libraries which aren't compatible with a wider range of communication objects.

[eric-wieser]

I don't follow your argument against -1 - can you elaborate with a code example?

[cmaglie]

I think that the only sane default is 0.

• If a stream doesn't implement buffering it should return 0: this is the only correct answer when there is no buffering.
• If the method is not implemented the only guarantee is that writing 0 bytes will not block: any other uncontrolled writes will, before or later, block.
• Returning -1 as a "function not implemented" error is not so useful: if you think of it, the only thing you can do in this case is to not use availableForWrite, that is more or less like using it with 0 as default value, so I'd avoid adding this special case.
• There is a trick to check if a stream is not buffered (or if the availableForWrite is not implemented), you can call the availableForWrite() before actually doing any write(): if the function returns 0 the stream cannot be used (because is unbuffered or the function is not implemented).

[PaulStoffregen]

Ok, 0 it is. Really, this is your call Cristian.

My position is any of these 3 choices is far better than not merging only because of this default value choice.

[cmaglie]

@eric-wieser could you update the PR?

[eric-wieser]

Updated, including a comment explaining the choice, and when overriding is necessary

[eric-wieser]

#2387 is very similar to this PR, so would be good to get that in too, while we're thinking about Print

[eric-wieser]

Great, thanks!

[eric-wieser]

Oops, this wasn't needed

[cmaglie]

Thanks! fixed here: d4a9452