search

arduino / arduino-examples

Arduino IDE bundled examples
Creative Commons Zero v1.0 Universal
90 stars 41 forks source link

Fix name collisions in ArduinoISP sketch #3

Closed per1234 closed 4 years ago

per1234 commented 4 years ago

Name collisions caused compilation of the ArduinoISP sketch to fail when:

per1234 commented 4 years ago

I saw you removed the friend class directive, why was it there?

Previously, BitBangedSPI::beginTransaction() was directly referencing the private member SPISettings::clock, which required friendship. The SPISettings class in ArduinoCore-API provides a getClockFreq() getter for this purpose: https://github.com/arduino/ArduinoCore-API/blob/45e4e5aba6ad1a5b67026b686099cc0fce437cdc/api/HardwareSPI.h#L73 and even provides guidance for referencing those private members:

// Core developer MUST use an helper function in beginTransaction() to use this data

So I just mirrored the way the SPISettings class from ArduinoCore-API was written in the SPISettings class declared in ArduinoISP that is used when the board being compiled for doesn't use ArduinoCoreAPI (and thus doesn't have the ARDUINO_API_VERSION macro defined). This way, SPISettings provides a consistent API to BitBangedSPI either way.

I'm definitely open to feedback on this. I'm not an expert in this subject, and only took on the project because the alternative to fixing the compilation failure was excluding the ArduinoISP sketch from the CI, which very much didn't sit well with me.

ubidefeo commented 4 years ago

Previously, BitBangedSPI::beginTransaction() was directly referencing the private member SPISettings::clock

right! I never do that, always loved getter/setter strategies

as I said, this looks well written