Make TwoWire methods virtual

[ahnolds]

This allows them to be overridden by downstream projects that need to subclass TwoWire

For an example of a project that currently subclasses TwoWire and would benefit from this functionality, see https://github.com/Testato/SoftwareWire

[ahnolds]

I still think this is a valuable change with basically no downsides - let me know if anyone has feedback, but I'd love to see this get merged!

[bxparks]

Are you sure about this?

basically no downsides

• This seems to be adding ~20 new virtual functions, which increases memory consumption of the v-table by 40 bytes (8-bit processors) or 80 bytes (32-bit processors). And every subclass of this would suffer the same amount of extra memory increase.
• Inline functions can no longer be inlined, because of the dynamic dispatch.
• Virtual dispatch is slower. Are any of these functions sensitive to timing?

In one of my libraries, adding a single virtual method in the entire library caused the flash memory consumption to go up by 50% (from ~1100 bytes to ~1600 bytes). This class already has virtual methods, so the difference won't be as drastic, but it might be worth double-checking.

Anyway, this is just a drive-by comment, and I'm not an expert on the TwoWire library. But adding 1-2 virtual methods to class that already has a handful of other virtual methods wouldn't concern me too much. But adding ~20 virtual methods to a class seems like something to be more careful about, to avoid unintended side-effects.

[ahnolds]

Valid points that I hadn't considered! My end goal is to make it possible for this software implementation of TwoWire to neatly subclass TwoWire and override the relevant methods. The specific use case I had could certainly have been accomplished through a narrower set of virtual methods, but since the class already has some virtual methods (in particular, read and write, which are presumably the most frequently called and most likely to be timing-sensitive methods in the class), I would argue that making the rest virtual won't incur a substantial penalty for the enhanced functionality and consistency.

[CLAassistant]

All committers have signed the CLA.

[bxparks]

Superceded by #396.

I noted in that PR that adding virtual to these 21 methods causes the <Wire.h> library to increase flash consumption by ~650 bytes, and static RAM consumption by 42 bytes. So #396 was reverted. Which means that this PR won't make it either.

[ahnolds]

Fair enough - I no longer even remember the details of what I wanted to accomplish with this subclassing, but if it matters to me in the future, I'll go with the approach from 396.