Skip to content
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

Update led_update_kb example #7451

Merged
merged 4 commits into from
Nov 23, 2019
Merged

Conversation

yanfali
Copy link
Contributor

@yanfali yanfali commented Nov 23, 2019

Provide a more succinct and less verbose example for a custom LED function.

Description

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@noroadsleft
Copy link
Member

Related question: does this or a similar method work if the board uses Input/Output to toggle the LED instead of High/Low? And if not, can we make it do that?

@fauxpark
Copy link
Member

fauxpark commented Nov 23, 2019

I kept these as discrete if statements because I figured it would be more understandable for newbs, but if you're going to change them to writePin() I guess a small bit of explanation as to what's going on here would be helpful. Especially with the ! inverting the logic as this is not always the case; it depends whether the LED is between the pin and VCC or GND.

@noroadsleft I think (haven't tested...I should buy some breadboards tbh) that high/low will work for all cases, as we just need current flow. If both sides of the LED are at VCC or GND, it won't light up. When it's pointing from VCC to GND, it will light up. Whether the pin is input or output probably has no effect electrically, it's simply the fact that putting the pin into Hi-Z state basically disconnects one end of the circuit.

@yanfali
Copy link
Contributor Author

yanfali commented Nov 23, 2019

I kept these as discrete if statements because I figured it would be more understandable for newbs, but if you're going to change them to writePin() I guess a small bit of explanation as to what's going on here would be helpful. Especially with the ! inverting the logic as this is not always the case; it depends whether the LED is between the pin and VCC or GND.

@noroadsleft I think (haven't tested...I should buy some breadboards tbh) that high/low will work for all cases, as we just need current flow. If both sides of the LED are at VCC or GND, it won't light up. When it's pointing from VCC to GND, it will light up. Whether the pin is input or output probably has no effect electrically, it's simply the fact that putting the pin into Hi-Z state basically disconnects one end of the circuit.

I've updated the comment in the code example to reflect this information. Thanks!

@zvecr zvecr merged commit 3541f01 into qmk:master Nov 23, 2019
@yanfali yanfali deleted the yanfali-update-custom-docs branch November 24, 2019 05:25
noroadsleft pushed a commit that referenced this pull request Dec 2, 2019
* for initial commit

* for initial commit

* for initial commit

* Update led_update_kb example (#7451)

* Update led_update_kb example

* Update comment to explain pin behavior

* wordsmith

* wordsmithing 2

* Remove CR when computing BOOTLOADER_SIZE. (#7453)

* Set up language fallback for docs, and update translation guidelines (#7403)

* Set up language fallback for docs, and update translation guidelines

* Title Case

* Add ID example

* Link to emoji flag cheatsheet

* Move docs preview section to contributing.md

* Point to docs preview in the readme

* [Keyboard] Added Cans12er keyboard (#7455)

* added cans12er keyboard

* updated readme

Updated the readme to conform with the provided template from the qmk_firmware githubpage

* Update keyboards/cans12er/README.md

Co-Authored-By: James Young <18669334+noroadsleft@users.noreply.github.com>

* Changed configuration

Changed the configuration based on the Change Request from PR #7455 made by github user noroadsleft

* [Keyboard] Update ATmega32A readme files to match template (#7462)

* Update atmega32a readme files to match template

* Update atmega32a readme files to match template - fixes

* Apply suggestions from code review

* update files based on comments

* update files based on comments

* update files based on comments

* update files based on comments

* update files based on comments

* update files based on comments

* update files based on comments

Co-Authored-By: Takeshi ISHII <2170248+mtei@users.noreply.github.com>
Co-Authored-By: shela <shelaf@users.noreply.github.com>
ripxorip pushed a commit to ripxorip/qmk_firmware that referenced this pull request Dec 3, 2019
* Update led_update_kb example

* Update comment to explain pin behavior

* wordsmith

* wordsmithing 2
ripxorip pushed a commit to ripxorip/qmk_firmware that referenced this pull request Dec 3, 2019
* for initial commit

* for initial commit

* for initial commit

* Update led_update_kb example (qmk#7451)

* Update led_update_kb example

* Update comment to explain pin behavior

* wordsmith

* wordsmithing 2

* Remove CR when computing BOOTLOADER_SIZE. (qmk#7453)

* Set up language fallback for docs, and update translation guidelines (qmk#7403)

* Set up language fallback for docs, and update translation guidelines

* Title Case

* Add ID example

* Link to emoji flag cheatsheet

* Move docs preview section to contributing.md

* Point to docs preview in the readme

* [Keyboard] Added Cans12er keyboard (qmk#7455)

* added cans12er keyboard

* updated readme

Updated the readme to conform with the provided template from the qmk_firmware githubpage

* Update keyboards/cans12er/README.md

Co-Authored-By: James Young <18669334+noroadsleft@users.noreply.github.com>

* Changed configuration

Changed the configuration based on the Change Request from PR qmk#7455 made by github user noroadsleft

* [Keyboard] Update ATmega32A readme files to match template (qmk#7462)

* Update atmega32a readme files to match template

* Update atmega32a readme files to match template - fixes

* Apply suggestions from code review

* update files based on comments

* update files based on comments

* update files based on comments

* update files based on comments

* update files based on comments

* update files based on comments

* update files based on comments

Co-Authored-By: Takeshi ISHII <2170248+mtei@users.noreply.github.com>
Co-Authored-By: shela <shelaf@users.noreply.github.com>
patrl pushed a commit to patrl/qmk_firmware that referenced this pull request Dec 29, 2019
* Update led_update_kb example

* Update comment to explain pin behavior

* wordsmith

* wordsmithing 2
patrl pushed a commit to patrl/qmk_firmware that referenced this pull request Dec 29, 2019
* for initial commit

* for initial commit

* for initial commit

* Update led_update_kb example (qmk#7451)

* Update led_update_kb example

* Update comment to explain pin behavior

* wordsmith

* wordsmithing 2

* Remove CR when computing BOOTLOADER_SIZE. (qmk#7453)

* Set up language fallback for docs, and update translation guidelines (qmk#7403)

* Set up language fallback for docs, and update translation guidelines

* Title Case

* Add ID example

* Link to emoji flag cheatsheet

* Move docs preview section to contributing.md

* Point to docs preview in the readme

* [Keyboard] Added Cans12er keyboard (qmk#7455)

* added cans12er keyboard

* updated readme

Updated the readme to conform with the provided template from the qmk_firmware githubpage

* Update keyboards/cans12er/README.md

Co-Authored-By: James Young <18669334+noroadsleft@users.noreply.github.com>

* Changed configuration

Changed the configuration based on the Change Request from PR qmk#7455 made by github user noroadsleft

* [Keyboard] Update ATmega32A readme files to match template (qmk#7462)

* Update atmega32a readme files to match template

* Update atmega32a readme files to match template - fixes

* Apply suggestions from code review

* update files based on comments

* update files based on comments

* update files based on comments

* update files based on comments

* update files based on comments

* update files based on comments

* update files based on comments

Co-Authored-By: Takeshi ISHII <2170248+mtei@users.noreply.github.com>
Co-Authored-By: shela <shelaf@users.noreply.github.com>
HokieGeek pushed a commit to HokieGeek/qmk_firmware that referenced this pull request Feb 21, 2020
* Update led_update_kb example

* Update comment to explain pin behavior

* wordsmith

* wordsmithing 2
HokieGeek pushed a commit to HokieGeek/qmk_firmware that referenced this pull request Feb 21, 2020
* for initial commit

* for initial commit

* for initial commit

* Update led_update_kb example (qmk#7451)

* Update led_update_kb example

* Update comment to explain pin behavior

* wordsmith

* wordsmithing 2

* Remove CR when computing BOOTLOADER_SIZE. (qmk#7453)

* Set up language fallback for docs, and update translation guidelines (qmk#7403)

* Set up language fallback for docs, and update translation guidelines

* Title Case

* Add ID example

* Link to emoji flag cheatsheet

* Move docs preview section to contributing.md

* Point to docs preview in the readme

* [Keyboard] Added Cans12er keyboard (qmk#7455)

* added cans12er keyboard

* updated readme

Updated the readme to conform with the provided template from the qmk_firmware githubpage

* Update keyboards/cans12er/README.md

Co-Authored-By: James Young <18669334+noroadsleft@users.noreply.github.com>

* Changed configuration

Changed the configuration based on the Change Request from PR qmk#7455 made by github user noroadsleft

* [Keyboard] Update ATmega32A readme files to match template (qmk#7462)

* Update atmega32a readme files to match template

* Update atmega32a readme files to match template - fixes

* Apply suggestions from code review

* update files based on comments

* update files based on comments

* update files based on comments

* update files based on comments

* update files based on comments

* update files based on comments

* update files based on comments

Co-Authored-By: Takeshi ISHII <2170248+mtei@users.noreply.github.com>
Co-Authored-By: shela <shelaf@users.noreply.github.com>
@esinlayo
Copy link
Contributor

Thanks for this example. I had to set the pin as an output using SetPinOutput in my init to get it to work though.

BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
* Update led_update_kb example

* Update comment to explain pin behavior

* wordsmith

* wordsmithing 2
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
* for initial commit

* for initial commit

* for initial commit

* Update led_update_kb example (qmk#7451)

* Update led_update_kb example

* Update comment to explain pin behavior

* wordsmith

* wordsmithing 2

* Remove CR when computing BOOTLOADER_SIZE. (qmk#7453)

* Set up language fallback for docs, and update translation guidelines (qmk#7403)

* Set up language fallback for docs, and update translation guidelines

* Title Case

* Add ID example

* Link to emoji flag cheatsheet

* Move docs preview section to contributing.md

* Point to docs preview in the readme

* [Keyboard] Added Cans12er keyboard (qmk#7455)

* added cans12er keyboard

* updated readme

Updated the readme to conform with the provided template from the qmk_firmware githubpage

* Update keyboards/cans12er/README.md

Co-Authored-By: James Young <18669334+noroadsleft@users.noreply.github.com>

* Changed configuration

Changed the configuration based on the Change Request from PR qmk#7455 made by github user noroadsleft

* [Keyboard] Update ATmega32A readme files to match template (qmk#7462)

* Update atmega32a readme files to match template

* Update atmega32a readme files to match template - fixes

* Apply suggestions from code review

* update files based on comments

* update files based on comments

* update files based on comments

* update files based on comments

* update files based on comments

* update files based on comments

* update files based on comments

Co-Authored-By: Takeshi ISHII <2170248+mtei@users.noreply.github.com>
Co-Authored-By: shela <shelaf@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants