Skip to content

Use saner names for the pin attributes #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 16, 2017
Merged

Use saner names for the pin attributes #4

merged 3 commits into from
Dec 16, 2017

Conversation

deshipu
Copy link
Contributor

@deshipu deshipu commented Dec 16, 2017

I think that d_or_c is as confusing as dc, possibly more, when the parameter is still called dc. If we wanted to be hyper-correct and stick to Python's style, we should call it data_or_command, but then for consistency we would also need things like master_in_slave_out and so on, and I think this is an overkill. Since the pin is usually labelled dc on the physical modules anyways, I think it's perfectly fine to use that — it's also consistent with the usage in Arduino and other drivers. I added _pin to make the linter be quiet and also to clarify that this is indeed a pin. I also changed the name of res to reset_pin to be consistent (the label for that pin on the modules is sometimes rs and sometimes rst so keeping the shortcut is not helpful).

I think that `d_or_c` is as confusing as `dc`, possibly more, when the parameter is still called `dc`. If we wanted to be hyper-correct and stick to Python's style, we should call it `data_or_command`, but then for consistency we would also need things like `master_in_slave_out` and so on, and I think this is an overkill. Since the pin is usually labelled `dc` on the physical modules anyways, I think it's perfectly fine to use that — it's also consistent with the usage in Arduino and other drivers. I added `_pin` so make the linter shut up and also to clarify that this is indeed a pin. I also changed the name of `res` to `reset_pin` to be consistent (the label for that pin on the modules is sometimes `rs` and sometimes `rst` so keeping the shortcut is not helpful).
@dhalbert
Copy link
Contributor

Woud you be willing to add a docstring to SSD1306_SPI.init() that documents the args, maybe in RTD style? For instance, say that "dc" means "data or command", "res" means "reset, etc. Ideally the whole library would be documented that way, but that's more work

@deshipu
Copy link
Contributor Author

deshipu commented Dec 16, 2017

Added that.

Copy link
Contributor

@kattni kattni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

travis build failed, fixed issue. Looks great! Thank you for doing this!

@kattni
Copy link
Contributor

kattni commented Dec 16, 2017

Tested successfully with display.

@kattni kattni merged commit d6e50f4 into adafruit:master Dec 16, 2017
tannewt pushed a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Dec 17, 2017
Updating https://github.com/adafruit/Adafruit_CircuitPython_CircuitPlayground to 1.0.1 from 1.0.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_CircuitPlayground#25 from adafruit/tannewt-patch-1

Updating https://github.com/adafruit/Adafruit_CircuitPython_MPR121 to 1.0.1 from 0.1.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_MPR121#4 from adafruit/travis-yaml-typo
  > Merge pull request adafruit/Adafruit_CircuitPython_MPR121#3 from tannewt/lint

Updating https://github.com/adafruit/Adafruit_CircuitPython_PCF8523 to 1.0.0 from 0.1.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_PCF8523#4 from tannewt/lint

Updating https://github.com/adafruit/Adafruit_CircuitPython_SSD1306 to 2.1.0 from 2.0.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_SSD1306#4 from deshipu/patch-1

Updating https://github.com/adafruit/Adafruit_CircuitPython_TSL2561 to 3.0.0 from 2.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_TSL2561#9 from kattni/driver-lint-update
  > Merge pull request adafruit/Adafruit_CircuitPython_TSL2561#7 from tannewt/lint

Updating https://github.com/adafruit/Adafruit_CircuitPython_HID to 2.0.0 from 1.1.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_HID#8 from mrmcwethy/lint

Updating https://github.com/adafruit/Adafruit_CircuitPython_SimpleIO to 1.0.0 from 0.3.1:
  > add pylint to project and updated code

Updating https://github.com/adafruit/Adafruit_CircuitPython_Waveform to 1.0.0 from 0.1.1:
  > add pylint to project (adafruit/Adafruit_CircuitPython_Waveform#1)
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