Print not aborting when Write(char) returns Error

[stickbreaker]

the Print::write(const uint8_t *buffer, size_t size) and Print::print(const __FlashStringHelper *ifsh) continue calling write(char) after write(char) has returned a 0 (zero) to mark a write failure. This creates a unexpected result when a block of characters is printed.
I implemented cts/rts handshaking in HardwareSerial, if !cts and the tx buffer is full and a timeout has occured, the HardwareSerial::write(char) returns 0(zero) to mark the failure. The Current Print object continues sending until the buffer has been send to write(char) if buffer space becomes free or cts clears, the write() failed characters are lost, but the rest of the buffer is sent. Since Print functions returns the number of characters sent, the expectation is that if len!=sent, the last (len-sent) characters were not sent. This expectation is incorrect. With the current Print library if len!=sent, there is no way to identify which character were not sent!
Included is the simple fix for this problem.

// from Arduino\hardware\avr\cores\arduino\Print.cpp 

// Public Methods //////////////////////////////////////////////////////////////

/* default implementation: may be overridden */
size_t Print::write(const uint8_t *buffer, size_t size)
{
  size_t n = 0;
  while (size--) {
// abort on write() failure, Chuck Todd
    if(write(*buffer++))n++; 
    else break;  
 //   n += write(*buffer++); original code 1.6.5
  }
  return n;
}

size_t Print::print(const __FlashStringHelper *ifsh)
{
  PGM_P p = reinterpret_cast<PGM_P>(ifsh);
  size_t n = 0;
  while (1) {
    unsigned char c = pgm_read_byte(p++);
    if (c == 0) break;
// abort on write() failure, Chuck Todd
    if(write(c)) n++; 
    else break;    
//   n += write(c); original code 1.6.5
  }
  return n;
}

Chuck Todd

[Chris--A]

That looks like a nice simple fix. However is it compatible with what is already happening.

I can think of one case: EthernetServer.println() will write to all clients. If one client fails to write, all clients will only get a half finished message.

Maybe the class needs a toggle to 'turn on' this behavior if the return value is needed to find where the stream stopped.

But the return value really only has meaning when using write() directly, the formatting print() functions return value has very little resemblance to what was passed in (might not even be text before formatting).

[matthijskooijman]

@Chris--A, EthernetServer::write can decide whether to return 1 to continue or 0 to abort, with these changes applied (not that it currently returns the number of clients succesfully written to, see #1699).

This is a change that could potentially break things, though nothing comes to mind directly. @stickbreaker perhaps you could propose this change on the developer's mailing list too, see if anyone there objects? If not, it would be great if you could prepare a pullrequest with the changes (be sure to remove the old code instead of commenting it if you do).

The changes proposed look good to me. Perhaps the code shouldn't have n++ but instead increase n with the return value of write? Not sure if there is a valid usecase for write(char) to return more than 1, though.

[stickbreaker]

@Chris--A

However is it compatible with what is already happening.

Nope, it is not compatible, because what it is doing now is a bug. This fixes the bug.

But the return value really only has meaning when using write() directly, the formatting print() functions return value has very little resemblance to what was passed in (might not even be text before formatting).

But, if the programmer checks the return value, it should have the expected meaning. If the return value is unknown, why return it?

@matthijskooijman

perhaps you could propose this change on the developer's mailing list too, see if anyone there objects?

How do I access this mailing list?

If not, it would be great if you could prepare a pullrequest with the changes (be sure to remove the old code instead of commenting it if you do).

I can do this, but should I wait to make the pull until after the developers approve?

Chuck.

[matthijskooijman]

How do I access this mailing list?

See https://www.arduino.cc/en/Main/ContactUs

I can do this, but should I wait to make the pull until after the developers approve?

Depends - having a pullrequest might make it easier to motivate your question, but it might mean a bit more work on your end if things have to be changed or end up being rejected, so that's your call :-)

[facchinm]

Solved by #3651