search

adafruit / Adafruit_CircuitPython_ESP32SPI

ESP32 as wifi with SPI interface
MIT License
103 stars 75 forks source link

Change _pin arguments to _dio #146

Closed tekktrik closed 2 years ago

tekktrik commented 2 years ago

Addresses Issue #121 by changing relevant argument names from *_pin to *_dio. I chose this instead of just using * argument names so it would be explicitly clear it should be a DigitalInOut instance passed as the argument, and also because I know there is a reset parameter so that argument wouldn't be confused for a boolean argument or something like that.

ladyada commented 2 years ago

can you check if any examples/code use the old naming?

tekktrik commented 2 years ago

Will do!

tekktrik commented 2 years ago

It looks like all the pin arguments in the examples in the repo are named esp32_* like esp32_reset. Should I check the Learn guides as well, or would those reference/be linked to the examples?

ladyada commented 2 years ago

yah check the learn system, its kinda risky to change arg names :/ lots of code can break

tekktrik commented 2 years ago

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 esp32_* and the arguments are never done as keywords, only positional.

tekktrik commented 2 years ago

Is this all good? Happy to do anything needed if this is a viable PR still!

ladyada commented 2 years ago

@jerryneedell any thoughts? just cause you deal with this stuff more recently than i!

jerryneedell commented 2 years ago

No concerns from me. Thank you for asking!

jerryneedell commented 2 years ago

That said, I suppose it should be considered a "breaking" change and announced in the release.

tekktrik commented 2 years ago

Any decision on this? Happy to change the names to something else as well if those aren't clear.

tekktrik commented 2 years ago

Sounds good! I'd be happy to look into how easily they can get a PR without breaking code!