Inherit from stream

[AFontaine79]

Description

TCP is a stream protocol and telnet, operating on top of TCP, represents a bidirectional stream. As such, it would be nice if ESPTelnet inherited from Stream, allowing it to be used in any context that accepts a Stream object.

The main realization here is that WiFiClient already inherits from Stream thus providing full access to all functionality of Stream and Print. Since ESPTelnet acts as a proxy relaying print functions to the currently active WiFiClient, it is already set up well to do this. It is unnecessary to implement the print() and println() functions as ESPTelnet was doing. It need only extend Stream itself and then implement the pure virtual functions of Stream and Print. It will then inherit all the print, read, and find functions of both Stream and Print.

The already implemented print() and println() functions of ESPTelnet were removed and the pure virtual functions of Stream and Print were added, following the same model. That is, these calls are relayed to the active WiFiClient, if there is one, otherwise they are ignored. The pure virtual functions constitute available(), read(), peek(), and flush() for Stream and write() for Print.

Type of change

Breaking feature enhancement to have ESPTelnet extend Stream.

• Commit 06077b9 is a non-breaking change to extend Stream. The direct implementation of the print functions is removed, but their implementation is now inherited from Print. Thus, functionality is added and no functionality is lost.
• Commit 6a48200 is a breaking change. It removes the functionality of the input received callback since that would interfere with the normal use of a Stream object.

Testing

The changes have been tested in PlatformIO using the Arduino framework on a DevKitC board. The platformio.ini is as follows:

[env:esp32dev]
platform = espressif32
board = esp32dev
framework = arduino

The test application simply connects together the input and output of the serial monitor with the connected telnet client. The code for this is as follows.

#if defined(ARDUINO_ARCH_ESP32)
  #include <WiFi.h>
  #include <WebServer.h>
#elif defined(ARDUINO_ARCH_ESP8266)
  #include <ESP8266WiFi.h>
  #include <ESP8266WebServer.h>
#endif

#include <ESPTelnet.h>


#define INFRA_SSID  "Secret"
#define INFRA_PSWD  "AnotherSecret"

ESPTelnet telnet;


void telnetConnected(String ip)
{
  Serial.print(ip);
  Serial.println(" connected.");
}

void telnetDisconnected(String ip)
{
  Serial.print(ip);
  Serial.println(" disconnected.");
}

void telnetReconnect(String ip)
{
  Serial.print(ip);
  Serial.println(" reconnected.");
}


void setup() {
  Serial.begin(115200);
  Serial.println("ESP Telnet Test");

  WiFi.mode(WIFI_STA);
  WiFi.begin(INFRA_SSID, INFRA_PSWD);
  while(WiFi.status() != WL_CONNECTED) {
    delay(100);
  }

  telnet.onConnect(telnetConnected);
  telnet.onDisconnect(telnetDisconnected);
  telnet.onReconnect(telnetReconnect);

  Serial.print("Telnet.begin: ");
  if(telnet.begin()) {
    Serial.println("Successful");
  } else {
    Serial.println("Failed");
  }
}

void loop() {
  telnet.loop();
  if(Serial.available() > 0) {
    telnet.println(Serial.readString());
  }
  if (telnet.available() > 0) {    
    Serial.println(telnet.readString());
  }
}

Tera Term was used for both sides of the connection (serial monitor and telnet).

[LennartHennigs]

Hey Aaron, that is quite neat! The one thing I am wondering though, if there might be a way to keep the onInputReceived()functionality or add some other mechanism to detect a certain strings within the data stream.

I looked at the Stream and Print class and there is not really something use. But I found the JSON Streaming Parser 2 where they implement handler class (JsonHandler) - is you can use to define your callbacks. I think this would be overkill for the Telnet lib but they implement in JsonStreamingParser::parse(char c) code that tracks the input stream and calls the handler if needed.

Maybe we could build something like this: whenStringIsFound(String someString) and internally we keep an Array or a vector of strings to look at and we use the parse function to look for similarities...

What do you think?

[AFontaine79]

Well, the idea behind the change is to allow ESPTelnet to be treated as a Stream object and usable in any API that takes Stream or Print object as parameters. That means it is limited to the APIs of those classes and cannot have other functionality that interferes with those APIs.

I agree that it would be nice to keep onInputReceived(). One possibility here is to pass a mode parameter to the constructor, such as bool legacyMode. Default would be false so that it is fully compatible with Stream but people that want the input received callback could set it to true explicitly. I could do another commit to make this change and add the onInputReceived functionality back in.

I would recommend against adding a parser to ESPTelnet as this would imply knowledge of a syntax and possibly a language and therefore violate the single responsibility principle. Best to keep it simple and not "overload" it. The responsibility, as I understand it, is to manage telnet connections and provide the active connection as a Stream.

[AFontaine79]

I have used a JSON parser in the past, but it wasn't with Arduino. It was Dave Gamble's cJSON, which is included in Espressif's native IoT framework ESP-IDF.

JSON Streaming Parser 2 has a similar idea to what I'm doing, though; Process an input stream and provide event callbacks based on the input sequence. I'm developing a command line interface though. This means I will have a hierarchy of command strings with the command entered invoking the specified callback. It also means live echo back of each character typed with proper handling of backspace.

Since the Stream class doesn't provide processing of the stream directly, to me that means it is up to whatever code is using Stream. Otherwise, if you try to add it to concrete implementations of Stream, such as ESPTelnet, you will run into all kinds of interoperability problems. I want interoperability because I want my CLI to operate over telnet or serial monitor (or both at the same time).

I'm surprised that JSON Streaming Parser 2 doesn't have processing of the stream built in. My plan is to be able to provide one or more Stream objects to the CLI and then call an execute() or loop() function every time through the main loop, the same way you already have ESPTelnet set up. And then everything will magically work due to the magic of OO. The key here is that the CLI code doesn't know anything about the streams other than that they are Stream objects. They might be instances of HardwareSerial, ESPTelnet, or anything else I fancy.

[LennartHennigs]

Hey Aaron, thanks for your comments. Give me a couple of days to test your code and think about it a bit.

[davidji]

I'm keen to see this get merged - at the moment it's impossible to deal with streams that contain null, AFAICT.

[LennartHennigs]

I will merge this - in about two weeks. Holidays \o/

[riraosan]

Hi @AFontaine79

I'll incorporate your ideas into my project one step ahead of time. Thank you.

[LennartHennigs]

Hey @AFontaine79, I still owe you an answer - sorry for the delay. I will change the class, so that it inherits from Stream. Just not now, as I currently use it to debug another project. When that is done, (and the dependency is gone) I have the time to do so. I will either make two versions of the class or maybe move the stream incompatible part to a helper class. Not sure about it yet. Again, sorry for the wait.

And thanks for reminding me, @riraosan.

[AFontaine79]

No hurry. I've been super busy since April with new job and this class I'm taking. I haven't had time for hobby stuff. I'll probably won't be looking at this again until Nov or Dec.

[LennartHennigs]

Hey @AFontaine79, I decided to make your Stream class a separate class. So that we have both options available. Will push the change and release it soon officially. Thx again for your work!