Extract Stream methods into an IStream.

[eric-wieser]

This allows streams to exist that are unidirectional.

A summary of this diff, which github does not show well:

• Rename the existing Stream to IStream, adjusting comments in the same way
• Remove the Print base class from the new IStream
• Add a brand new Stream.h with class Stream : public Print, public IStream {}

I can't see any reason for this not to be backwards-compatible.

Example usage:

class MyInputWithThisPatch : public IStream {
public:
    virtual int read() override { return ...; }
    virtual int peek() override { return ...; }
    virtual int available() override { return ...; }
    virtual void flush();

    // if this were : public Stream, we would have to override `write` as well
}

void write_base64_to(Print& print, int x) {
    // This was always possible
    print.write(...); // etc
}

int read_base64_from(IStream& stream) {
    //               ^
    // But there was no good type to put here
    stream.read(...); // etc
}

void main() {
    MyInputWithThisPatch stream;

    int x = read_base64_from(stream); //ok
    int y = read_base64_from(Serial); //ok

    // this is a compiler error, as it should be. Without this patch, it would not be.
    // write_base64_to(stream, 1);
    write_base64_to(Serial, 1); // ok
}

I'm open to other names here instead of IStream, such as:

• InputStream
• Read (to match Print)
• Reader (which avoids naming classes after verbs)

[Chris--A]

I do not think this is necessary. You can make the Print features private, hiding them from the user.

class MyStream : public Stream{
  public:
    int available()              { return 0; }  //Fill in these as appropriate.
    void flush()                 { return; }
    int peek()                   { return -1; }
    int read()                   { return -1; }
 
  private:
    using Print::print;
    using Print::println;
    using Print::write;
    size_t write( uint8_t data ) { return (void)data, 0; }
};

MyStream stream;

void setup() {
  stream.read();  //Fine
  stream.println();  //Compile error
}

void loop() {}

flush() is part of Stream, but you can also move it to the private section.

[eric-wieser]

@Chris--A: That's a workaround, but not a good one:

void print_some_things(Print& p) {
   p.println("Hello world");
}

MyStream stream; // using your definition

void setup() {
    stream.read();

    print_some_things(stream);  // oops. This compiles just fine

    static_cast<Stream&>(stream).println("oops again");

    Print* p = &stream;
    p->println("Oops");
}

The existence of an ineffective workaround is not sufficient justification for an improvement being unnecessary.

[Chris--A]

That isn't really an issue either. My workaround hides the print features when using the object directly, and for the cases that the object is cast to one of its inherited types, the print functionality has no effect.

A library shouldn't be left up to assumptions, if you don't support print features, then make it well known, or don't even mention it, then no one would have a reason to use them.

Another option using my original example is to make the Stream base private and only expose what you want:

class MyStream : private Stream{
  public:
    int available()              { return 0; }
    void flush()                 { return; }
    int peek()                   { return -1; }
    int read()                   { return -1; }

    using Stream::find;
    using Stream::findUntil;
    using Stream::parseInt;
    using Stream::readBytes;
    using Stream::readBytesUntil;
 
  private:
    size_t write( uint8_t data ) { return (void)data, 0; }
};

MyStream stream;

void setup() {
  stream.read();  //Fine
  stream.find('a'); //Fine

  Stream &s = stream; //Compile error
  Print &p = stream; //Compile error
}

void loop() {}

Similar to the above, you can also have a Stream reference or pointer as a member variable and only expose the input functions.

[eric-wieser]

You've made the valid point that I've justified this poorly, so I've added some example code in my description.

---

A library shouldn't be left up to assumptions, if you don't support print features, then make it well known

That's kinda the point of this patch. There is currently no way to make it known that you support read features but not print features, yet there is a way to express the inverse. Why should things be this way?

What if I want to write a function that can read from Serial and stream? That seems perfectly sane, but your latest response doesn't allow that without templates.

Look at this another way - if IStream did exist, can you think of a single reason it should be removed?

[eric-wieser]

flush() is part of Stream, but you can also move it to the private section.

Good catch, this probably doesn't belong in IStream as I have it here, since it deals with output and not input

[matthijskooijman]

flush() is part of Stream, but you can also move it to the private section.

Also see #2387.

[eric-wieser]

@matthijskooijman: Was tempted to fix that too. What's stopping that being merged?

[matthijskooijman]

@eric-wieser, Nothing that I can see, apart from maybe lack of developer time.

[eric-wieser]

Now that #2387 is merged, I've rebased this to be even simpler

[eric-wieser]

This is the only line that actually changed in this file other than renaming Stream to IStream - it was previously class Stream : public Print

[eric-wieser]

Nothing is left of the original file apart from the name Stream, so this copyright notice has moved to IStream.h

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}