Testato / SoftwareWire

Creates a software I2C/TWI bus on every pins
GNU General Public License v3.0
148 stars 33 forks source link

Make it working with modern gcc #20

Closed dwrobel closed 5 years ago

dwrobel commented 5 years ago
Koepel commented 5 years ago

The goal is not to be modern, but to be a drop-in replacement for the Arduino Wire library.

I see that the function parameters of Arduino Wire library for the AVR microcontrollers differs from SoftwareWire. Some changes might be needed, but I'm not sure about the "override" keyword. I rather add the inline functions from the Arduino Wire.h file. I agree that the write(char* data) has to go.

Arduino AVR Wire library: https://github.com/arduino/ArduinoCore-avr/tree/master/libraries/Wire/src

dwrobel commented 5 years ago

The goal is not to be modern, but to be a drop-in replacement for the Arduino Wire library.

FYI, I'm using it as a replacement of Wire for Adafruit-MCP23017-Arduino-Library (see: https://github.com/adafruit/Adafruit-MCP23017-Arduino-Library/pull/26).

I'm not sure about the "override" keyword.

Basically it's a must if you intent to correctly override a virtual method, if you omit it, you can simply end up creating a new method instead of overwriting the existing one (see example here: https://github.com/Testato/SoftwareWire/pull/20/files#diff-aa9086e443c0537f539acb314ad36cfeR326).

Koepel commented 5 years ago

Let's wait a few days for Testato.

The Arduino AVR Wire library and the Arduino SAMD Wire library use virtual. So I prefer to use virtual and add also the extra inline functions. Your changes adds override, that could make the SoftwareWire behave different from the Arduino Wire library.

If you use the official Arduino Wire library with the modified Adafruit library, will a new method be created? Perhaps Adafruit and/or Arduino has to fix that to prevent it. Then we can follow.

Testato commented 5 years ago

Override seems that do not change the behavior of the method, so if can compile with arduino official ide and confirm that there is no problem, for me this PR is acceptable

Koepel commented 5 years ago

Okay. Testato, I leave it up to you if the version should be increased.

dwrobel, thank you for your contribution to this library.

Testato commented 5 years ago

Thanks @dwrobel