Is inheriting from usb_serial_class a bad idea ?

Status
Not open for further replies.

Tactif CIE

Well-known member
I wanted to have a kinda Serial with several fancy methods to handle VT100/ANSI codes (bold, colors, clear screen, positioning and so on)

So I did :
Code:
class Terminal : public usb_serial_class {
  public:
    Terminal() : usb_serial_class(){};
/* ... methods omitted */
};

I did not had any problem until I started coding a small test suite that bypass the usual setup(); loop(); yield(); and runs all the tests instead (but yield() at the end of the tests)
Now, the Teensy crashes randomly during the tests (serial output stops at more or less random places and the teensy is frozen press button + upload are mandatory).

Of course it might be a bug in my code, but just to be sure :

- Since the beginning I wonder if there's any kind of conflicts that could occur with the Serial instance...
- I tried to yield() during the tests - that changed nothing...

What do you think about this ? Am I doing things wrong ?
 
What do you think about this ? Am I doing things wrong ?

If Teensy is randomly crashing, I'd say that's a pretty sure sign you're doing something wrong.

Usually I recommend passing Serial to your class by reference, either in the constructor or a begin() method. Usually you would store it as a pointer inside your class, then use the pointer to send data. For example, if the pointer is "usb_serial_class *port", then you'd use it like this:

Code:
  port->write(buffer, length);

This way avoids all sorts of thorny problems with inheritance, like creating duplicate copies of stuff that was meant to be static or only a single instance.

As a general rule, if you can, try to avoid doing stuff in your constructor. Ideally, use "constexpr" on your constructor. Not only does that allow the compiler to generate much more efficient code, but it guarantees you'll avoid tough problems known as "C++ static initialization order fiasco". Add a begin() function to actually start up, if you need to do initialization that's more than just initializing you class variables.

If you're sending terminal control sequences, you might consider calling Teensy's Serial.send_now() function at certain times. This is similar to Arduino's flush(), but doesn't wait for the data to actually transmit. By default Teensy's Serial will try to efficiently group writes together into maximum size USB packets. The send_now() function causes a partial packet to be given to the USB hardware. That's less efficient for bandwidth, but it minimizes latency. If you're sending codes to a VT100 or ANSI terminal, there are probably some cases where you care most about latency.
 
Usually I recommend passing Serial to your class by reference, either in the constructor or a begin() method. Usually you would store it as a pointer inside your class, then use the pointer to send data.

That's what I thought at first glance but I would end with a mix of Serial.print (or any other methods provided by usb_serial_class) and term.bold (or any other methods provided by Terminal class) - But I presume I've no other choice because of

This way avoids all sorts of thorny problems with inheritance, like creating duplicate copies of stuff that was meant to be static or only a single instance.
 
You could have your Term class inherit from Stream and implement the special functions as required:
Code:
#include "Arduino.h"

class Term: public Stream {
public:
	Term(Stream *p) :
			streamPtr(p) {
	}
	virtual ~Term() {
	}

	using Print::write;

	size_t write(uint8_t b) {
		return streamPtr->write(b);
	}

	int available() {
		return streamPtr->available();
	}
	int read() {
		return streamPtr->read();
	}
	int peek() {
		return streamPtr->peek();
	}

	void bold() {
		// special Term bold function
	}

private:
	Stream *streamPtr;
};

Term term(&Serial);

void setup() {
	Serial.begin(115200);

	term.println("Hello World");
	term.bold();
	term.println("Hello World in Bold");
}

void loop() {
}
 
Status
Not open for further replies.
Back
Top