reduce library size by 2036 bytes of flash (~50%) and 157 bytes of ram (~73%)

[bxparks]

According to my testing, this library currently consumes about 3926 bytes of flash and 216 bytes of static ram on an Arduino Nano (after subtracting away an empty program that does nothing, i.e. empty setup() and empty loop()). This PR reduces the flash consumption by 2036 bytes, and ram consumption by 157 bytes, through the following steps:

1)) Remove virtual on destructor. Saves 592 bytes of flash and 14 bytes of ram by preventing the virtual destructor from pulling in malloc() and free(). Making the destructor virtual has no benefit for the SoftwareWire class as far as I can see, because neither of its parent classes (TwoWire and Stream) defines a virtual destructor. That means that SoftwareWire cannot be deleted polymorphically. In other words, in the following code:

TwoWire* wire = new SoftwareWire();
delete wire;

the delete operator calls ~TwoWire(), not ~SoftwareWire(). I have actually tested this, and verified that "wrong" destructor (~TwoWire()) is called. (It is the correct destructor as far as the C++ language spec is concerned, just not the one that we wanted). When the above code is compiled with warnings enabled, the compiler prints the following warning:

warning: deleting object of polymorphic class type 'TwoWire' which has non-virtual 
destructor might cause undefined behavior  [-Wdelete-non-virtual-dtor]

which is an additional indication that making the destructor virtual does not have the desired effect.

2)) Remove inheritance from TwoWire. Saves 304 bytes of flash and 30 bytes of ram. The PR that attempted to make TwoWire polymorphic (arduino/ArduinoCore-avr#396) was reverted (arduino/ArduinoCore-avr#412) because it causes TwoWire to increase its flash memory by about 650 bytes. I think it is safe to assume that TwoWire will not support subclassing in the future. So we can save memory by changing SoftwareWire to no longer inherit from TwoWire.

3)) Remove including Wire.h. Saves 1140 bytes of flash and 113 bytes of ram. Just the inclusion of the TwoWire header file (through #include <Wire.h>) causes over 1kB of flash to be consumed, even if theWire object is never used. For reasons which are not obvious to me, the compiler is unable to determine that Wire is never used in the program, and does not optimize it away during link time. Since (2) above changes SoftwareWire so that it is no longer a subclass of TwoWire, we no longer need to include <Wire.h>.

Total flash savings: 2036 bytes, from 3926 bytes down to 1890 bytes
Total ram savings: 157 bytes, from 216 bytes down to 59 bytes

This PR causes the following effects:

• Obsoletes Match function prototypes from TwoWire #29
• Reverts Made destructor virtual to allow object deletion #24
• Reverts Inherit from TwoWire base class so that it can better operate with 3rd party libraries. #14
• Resolves Deriving from TwoWire pointless? #28

For people who are interesting in using this patch, I will probably keep around my fork (https://github.com/bxparks/SoftwareWire) while this PR remains unmerged.

[Testato]

For me it's ok,
What do you think ?
@Koepel
@bperrybap
@SukkoPera

[SukkoPera]

Sounds good to me. As I said in #28, as the TwoWire class does not have virtual methods, deriving from it is totally pointless. I would have preferred "the other solution" (i.e.: Making the methods virtual in the parent class), but since that is not going to happen, the only other meaningful thing to do is this one, as we are currently wasting flash and RAM for no useful purpose.

When this is merged in, #28 can be closed. I see it's already been linked.

[bperrybap]

It sounds good, but I need to test it with the hd44780 library to make sure it actually works as expected.

[Koepel]

The TwoWire and Wire.h were used to be compatible with other libraries that use them. As a result a unused "Wire" object is sitting in memory.

This is related to: #28 were @bperrybap already wrote about it.

The goal is to be able to use both Wire (hardware I2C bus) with multiple SoftwareWire ojects in the same sketch and be compatible with all the sensor libraries. Most libraries use pointers to TwoWire, others might use a template.
Since more and more libraries store a TwoWire pointer in the object, that should still be supported.

I have even seen Wire.print(), but I don't care if the Print class is not supported.

The virtual destructor is a bug. Changing the other things will result into trouble in one way or the other in my opinion.
I'm afraid we will end up with defines or templates to select an appropriate version of the library.

[bperrybap]

The patch appears to revert code back to the 1.5.0 release with a few minor changes.

• using size_t vs uint8_t for the byte count in write()
• adding write(char* data)
• change to i2c_repstart() as to how a delay and the SDA signal is handled.

I'm not sure the purpose of that last change, particularly the altering of the handling of SDA (setting it low)
But the patch effectively rolls back the 1.5.1 changes.
I did some limited testing and it compiles and works with my hd44780 speed test example for SoftWire.
This new code is a couple of bytes bigger than the 1.5.0 code but that is likely do to the minor changes made.

@Koepel
I'm not understanding your comment:

The goal is to be able to use both Wire (hardware I2C bus) with multiple SoftwareWire ojects in the same sketch 
and be compatible with all the sensor libraries. 
Most libraries use pointers to TwoWire, others might use a template.
Since more and more libraries store a TwoWire pointer in the object,
that should still be supported.

While I think that compatibility goal is desirable, I don't think it is technically possible to do what you have sated.
i.e. be able to use both Wire and SoftwareWire objects simultaneously (multiple buses) in the same sketch and also be compatible with all the sensor libraries.
I don't think all those desires are technically possible.
I have seen quite a few libraries for i2c h/w slaves that just include <Wire.h> and use the global Wire object, so I'm not sure I would agree that "most libraries use pointers to TwoWire".

Also, I don't think it is possible to have both Wire and Software library classes use the class name "TwoWire" to allow libraries to work that use pointers to TwoWire and be able to use both libraries simultaneously in the same sketch.

I think the only way a library can guarantee to be compatible with various i2c libraries is to use a template with pointers so that it no longer cares about the object name or its class.
(The classname can be passed into the template using typeof(WIREOBJENAME) )
So far I've seen very few libraries that use templates as the typical Arduino library puts code in a .cpp file for separate compilation and that isn't how you do it when using templates.

Currently, the hd44780 library does not use templates yet to handle the i2c class and object. It currently depends on the sketch to include that proper i2c library header and then ensure that a Wire object exists.
Eventually, I'll switch to templated class so that it will work with anything that provides a "Wire" s/w API.

[Koepel]

Let me rephrase that: Most libraries that try to be able to select a Wire library use a pointer to a TwoWire object.
A typical way would be Sensor.begin( int i2cAddress, TwoWire *wire);
Adafruit uses this sometimes: bool begin(uint8_t i2c_addr = SENSOR_ADDR, TwoWire *wire = &Wire);
I think that we can not drop support for the TwoWire pointer way at this point.

Personally, I like templates make the library compatible at source code level, but the TwoWire pointer way has also some advantages if it is consistently used.

You see problems with both a hardware and one or more software I2C buses in the same sketch ? That would be disappointing.

[bperrybap]

Let me rephrase that: Most libraries that try to be able to select a Wire library use a pointer to a TwoWire object.
A typical way would be Sensor.begin( int i2cAddress, TwoWire *wire);
Adafruit uses this sometimes: bool begin(uint8_t i2c_addr = SENSOR_ADDR, TwoWire *wire = &Wire);

Yes I definitely agree with that.

Maybe I misunderstood you.
What did you mean by:
The goal is to be able to use both Wire (hardware I2C bus) with multiple SoftwareWire ojects in the same sketch and be compatible with all the sensor libraries.

I took that to mean being able to use objects from both Wire and Software at the same time (multiple i2c buses) and using TwoWire as the name of the class for both.
Is that what you meant? If so, I don't see how that can technically work.

i.e. in the absence of having a wrapper class named TwoWire provided by the bundled IDE i2c library you can either have the TwoWire class name used by all the i2c libraries (like this this one) and provide a global predefined object named Wire, which provides great compatibility with the Arduino.cc bundled Wire library but also prevents using more than one i2c library at a time

Or you can have unique class names for each i2c library which allows concurrent use but will not be compatible with libraries that hard code the class name to TwoWire or use a hard coded wire object name like Wire.

So I guess I still don't understand what you mean by:
I think that we can not drop support for the TwoWire pointer way at this point.

Are you pushing for making SoftwareWire fully compatible with the Wire library (which means you can't use the Wire library when using the SoftwareWire library) or something else that allows using both at the same time?
Given your response so far, I think you are pushing for something else, but I'm not sure what that is and how it can work to preserved 100% compatibility with Wire and also allow both Wire and SoftwareWire to be used at the same time.

BTW, this same issue exists in other libraries like Arduino libraries that sit on top of libraries for hd44780 displays.
i.e. there are a number of lcd menu libraries that allow passing in a lcd object pointer but then hardcode the lcd class name to LiquidCrystal so they don't work with any other lcd library.

The only solution that I have seen that works for all cases (multiple objects for using multiple buses including using multiple libraries using different class names) is to use templates where the user passes in the class to the template when the object is declared.

[bxparks]

Adafruit may use a TwoWire* pointer in its code (e.g. the bool begin(uint8_t i2c_addr = SENSOR_ADDR, TwoWire *wire = &Wire);), but I am not sure that it is relevant to us because TwoWire cannot be used polymorphically. As discussed in #28, no subclass of TwoWire can be used though a pointer to TwoWire, because wire->beginTransmission() calls TwoWire::beginTransmission not SoftwareWire::beginTransmission().

If the alternative proposal is that SoftwareWire.h should be a drop-in replacement forWire.h, by renaming SoftwareWire to TwoWire, then I'm pretty sure that would not work. Since SoftwareWire.h would cause a name collision with any 3rd party library that happens to do a #include <Wire.h>.

I do think it is beneficial for the SoftwareWire class to be syntactically (i.e. source code) compatible with the TwoWire class, because we can use C++ templates to achieve compile-time polymorphism, instead of using runtime polymorphism through dynamic dispatch. But I don't think it buys us much for SoftwareWire to be a subclass of TwoWire. I have convinced myself through this PR that it does harm because it pulls in large amount of dead code which is never used and cannot be optimized away by the linker.

[Koepel]

Thanks for the explanation.

Suppose someone has a sensor with a library with a TwoWire pointer for the hardware I2C bus and wants to add a software I2C bus with a sensor library that also uses a TwoWire pointer. I understand now that it is not possible.

If someone wants something extraordinary, then we have to tell to change the sensor library.

[Testato]

So we're all finally in agreement?

I will merge this PR and publish a V1.5.2 to the library manager

thank you all

[SukkoPera]

Agreed, but I'd rather make that 1.6.0.

And maybe you can also check that all method signatures match TwoWire's and close #27 as well.

[bxparks]

@SukkoPera:
Agree with 1.6.0 versioning.
Regarding #27, different platforms have slightly different TwoWire APIs, but I guess SoftwareWire works only on AVR so that's the target. But it's not clear to me that having 100% API compatibility with the AVR version of TwoWire is actually worth the effort.

[Koepel]

Added this page to the Wiki: Using a library that needs a TwoWire pointer
Feel free to improve it.

[bxparks]

@Koepel: That's good information. I have another way of doing this, but it's not quite ready for public consumption. I'll add some info on that wiki when it's ready.

I recently evaluated many of the libraries listed in this wiki page, https://github.com/Testato/SoftwareWire/wiki/Arduino-I2C-libraries. I added a few entries, added some comments, and refactored the page a bit. The biggest change is that I grouped the libraries according to their target architecture.

[bxparks]

FYI: I did another substantial update to the Arduino-I2C-libraries wiki page. Three big changes:

1. I added a "RX/TX Buffer Sizes" column and filled it for each 3rd party library on that page. The buffer sizes range from 0 bytes (no buffer) to 256 bytes on the SAMD implementations.
2. I added a separate <Wire.h> entry for each of the major platforms that I am familiar with (Arduino SAMD, SparkFun SAMD, STM32, ESP8266, ESP32) because each platform has a slightly different implementation compared to the AVR Wire library, with slightly different RX/TX buffer sizes.
3. I added an entry for my AceWire library that I recently finished, but I realized that it is solving a slightly different problem than the one described in the Using a library that needs a TwoWire pointer wiki page created by @Koepel . Specifically, AceWire provides a thin adapter layer that allows compile-time polymorphism for the application code so that different I2C implementations can be easily selected with little to no code changes at the application layer. That's a different problem than adapting an existing library that is hardcoded to use <Wire.h> for use with a different I2C library. So left the Using a library that needs a TwoWire pointer wiki page alone.