Skip to content

Make it working with modern gcc#20

Merged
Testato merged 1 commit intoTestato:masterfrom
dwrobel:tip
Feb 23, 2019
Merged

Make it working with modern gcc#20
Testato merged 1 commit intoTestato:masterfrom
dwrobel:tip

Conversation

@dwrobel
Copy link
Copy Markdown
Contributor

@dwrobel dwrobel commented Feb 22, 2019

  • fixes lack of overrides keyword and correct wrong prototypes (caused compilation errors),
  • removes size_t SoftwareWire::write(char* data) as it caused compilation errors.

@Koepel
Copy link
Copy Markdown
Collaborator

Koepel commented Feb 22, 2019

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
Copy link
Copy Markdown
Contributor Author

dwrobel commented Feb 23, 2019

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: adafruit/Adafruit-MCP23017-Arduino-Library#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
Copy link
Copy Markdown
Collaborator

Koepel commented Feb 23, 2019

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
Copy link
Copy Markdown
Owner

Testato commented Feb 23, 2019

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
Copy link
Copy Markdown
Collaborator

Koepel commented Feb 23, 2019

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

dwrobel, thank you for your contribution to this library.

@Testato
Copy link
Copy Markdown
Owner

Testato commented Feb 23, 2019

Thanks @dwrobel

@Testato Testato closed this Feb 23, 2019
@Testato Testato reopened this Feb 23, 2019
@Testato Testato merged commit 1bc0e6d into Testato:master Feb 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants