Change _pin arguments to _dio #146
Conversation
|
can you check if any examples/code use the old naming? |
|
Will do! |
|
It looks like all the pin arguments in the examples in the repo are named |
|
yah check the learn system, its kinda risky to change arg names :/ lots of code can break |
|
A quick search for ESP32 products and reading the guides shows that the examples (no references other than code examples) always use the same variable naming scheme of |
|
Is this all good? Happy to do anything needed if this is a viable PR still! |
|
@jerryneedell any thoughts? just cause you deal with this stuff more recently than i! |
|
No concerns from me. Thank you for asking! |
|
That said, I suppose it should be considered a "breaking" change and announced in the release. |
There was a problem hiding this comment.
I've only seen instantiation positional, typically with variables assigned from the DigitalInOuts:
esp = adafruit_esp32spi.ESP_SPIcontrol(spi, esp32_cs, esp32_ready, esp32_reset)
Tested PR code on PyPortal, change is transparent.
Portalbase may be the most prevalent library using ESP32SPI, and it instantiates positionally, with variables:
self.esp = adafruit_esp32spi.ESP_SPIcontrol(spi, esp32_cs, esp32_ready, esp32_reset, esp32_gpio0 )|
Any decision on this? Happy to change the names to something else as well if those aren't clear. |
There was a problem hiding this comment.
Looks good to me, I agree in thinking that *_dio makes it more clear what is expected to be passed for those arguments.
I tested these changes successfully with the requests simpletest on a PyPortal 7.2.0-alpha.1.
I also used git grep "cs_pin=" and similar to search for usages of these argument names in the Learning System repo and did not find any code using those names to instantiate this library.
However there are a few other libraries with similarly named *_pin arguments that are actually expecting DigitalIO objects like this one. I'll open issues on those repo's I would assume we want to be consistent about the argument naming across repos.
|
Sounds good! I'd be happy to look into how easily they can get a PR without breaking code! |
Updating https://github.com/adafruit/Adafruit_CircuitPython_24LC32 to 1.0.1 from 1.0.0: > Merge pull request adafruit/Adafruit_CircuitPython_24LC32#11 from tekktrik/doc/add-typing > Merge pull request adafruit/Adafruit_CircuitPython_24LC32#12 from tekktrik/hotfix/patch-cleanup-fix Updating https://github.com/adafruit/Adafruit_CircuitPython_ADS1x15 to 2.2.11 from 2.2.10: > First part of patch > Merge pull request adafruit/Adafruit_CircuitPython_ADS1x15#77 from nlantau/patch-1 > update rtd py version Updating https://github.com/adafruit/Adafruit_CircuitPython_BNO055 to 5.3.3 from 5.3.2: > Merge pull request adafruit/Adafruit_CircuitPython_BNO055#90 from adafruit/patch-fix > First part of patch > update rtd py version Updating https://github.com/adafruit/Adafruit_CircuitPython_CCS811 to 1.3.6 from 1.3.5: > Merge pull request adafruit/Adafruit_CircuitPython_CCS811#47 from sti320a/patch-1 > First part of patch > update rtd py version Updating https://github.com/adafruit/Adafruit_CircuitPython_CLUE to 3.0.2 from 3.0.1: > Merge pull request adafruit/Adafruit_CircuitPython_CLUE#54 from kevinjwalters/sample-fix > First part of patch > update rtd py version Updating https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI to 4.0.0 from 3.6.0: > Merge pull request adafruit/Adafruit_CircuitPython_ESP32SPI#146 from tekktrik/fix/rename-pin-args > Merge pull request adafruit/Adafruit_CircuitPython_ESP32SPI#155 from tekktrik/hotfix/fix-recv-into > First part of patch Updating https://github.com/adafruit/Adafruit_CircuitPython_SCD4X to 1.2.2 from 1.2.1: > Merge pull request adafruit/Adafruit_CircuitPython_SCD4X#12 from KeithTheEE/get-data-ready-status-fix > First part of patch > update rtd py version Updating https://github.com/adafruit/Adafruit_CircuitPython_STMPE610 to 1.3.0 from 1.2.7: > Merge pull request adafruit/Adafruit_CircuitPython_STMPE610#22 from CedarGroveStudios/main > First part of patch > update rtd py version Updating https://github.com/adafruit/Adafruit_CircuitPython_Thermistor to 3.4.0 from 3.3.8: > Merge pull request adafruit/Adafruit_CircuitPython_Thermistor#19 from raquo/main > First part of patch > update rtd py version Updating https://github.com/adafruit/Adafruit_CircuitPython_Bitmap_Font to 1.5.5 from 1.5.4: > Merge pull request adafruit/Adafruit_CircuitPython_Bitmap_Font#56 from tekktrik/feature/ignore-comments-parsing > First part of patch Updating https://github.com/adafruit/Adafruit_CircuitPython_Debouncer to 1.4.0 from 1.3.13: > Merge pull request adafruit/Adafruit_CircuitPython_Debouncer#34 from prplz/main > First part of patch > update rtd py version Updating https://github.com/adafruit/Adafruit_CircuitPython_Display_Button to 1.6.2 from 1.6.1: > Merge pull request adafruit/Adafruit_CircuitPython_Display_Button#34 from adafruit/patch-fix > First part of patch > update rtd py version Updating https://github.com/adafruit/Adafruit_CircuitPython_Logging to 3.7.4 from 3.7.3: > Merge pull request adafruit/Adafruit_CircuitPython_Logging#25 from JingleheimerSE/add-time-format-specifier > First part of patch Updating https://github.com/adafruit/Adafruit_CircuitPython_MacroPad to 2.0.4 from 2.0.3: > Merge pull request adafruit/Adafruit_CircuitPython_MacroPad#34 from adafruit/patch-fix > First part of patch > update rtd py version Updating https://github.com/adafruit/Adafruit_CircuitPython_Simple_Text_Display to 1.2.2 from 1.2.1: > Merge pull request adafruit/Adafruit_CircuitPython_Simple_Text_Display#10 from adafruit/patch-fix > First part of patch > update rtd py version
Addresses Issue #121 by changing relevant argument names from
*_pinto*_dio. I chose this instead of just using*argument names so it would be explicitly clear it should be aDigitalInOutinstance passed as the argument, and also because I know there is aresetparameter so that argument wouldn't be confused for a boolean argument or something like that.