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

examples: bgColorLEDWEB : Fixed color position and value #252

Merged
merged 2 commits into from
Oct 19, 2015
Merged

examples: bgColorLEDWEB : Fixed color position and value #252

merged 2 commits into from
Oct 19, 2015

Conversation

ks156
Copy link
Contributor

@ks156 ks156 commented Oct 19, 2015

LED was not correctly set, and values must be inverted (LED ON = 0; LED OFF = 1)

@drasko
Copy link
Contributor

drasko commented Oct 19, 2015

LGTM, but unfortunetly because of the inverted logic whole examples become harder to exmplain to beginers and newbies.

Can we work something around this inverted logic? I know that this should have been done in HW, but we are where we are and maybe some SW wrap about GPIO logic that control LED can be done so that LED is ON when these GPIOs are wtitten HIGH by userspace (and leter inverted in the weioLib behind the users back)?

@ks156
Copy link
Contributor Author

ks156 commented Oct 19, 2015

We need to add cases for pin 18, 19 and 20. But in this case, if someone want use these pins for something else than the LED, the signals will be inverted for him ...

@drasko
Copy link
Contributor

drasko commented Oct 19, 2015

True, but this can be noted in the doc. Only question is is it worth (and intelligent) to break the consistency because of the examples... Not sure. Opinions?

@ks156
Copy link
Contributor Author

ks156 commented Oct 19, 2015

I would prefer add a note to explain why the LEDs are inverted instead of adding special case for them

drasko added a commit that referenced this pull request Oct 19, 2015
examples: bgColorLEDWEB : Fixed color position and value
@drasko drasko merged commit 41e92d9 into nodesign:next Oct 19, 2015
@drasko
Copy link
Contributor

drasko commented Oct 19, 2015

ACKed and merged.

@ks156
Copy link
Contributor Author

ks156 commented Oct 19, 2015

@drasko : Did you see my second commit ?

@drasko
Copy link
Contributor

drasko commented Oct 19, 2015

Yeap, nice enough documented IMHO.

@ks156
Copy link
Contributor Author

ks156 commented Oct 19, 2015

Ok. I was not sure because I think you were occupied to merge my first commit when I've pushed the second one 😛

@ks156 ks156 deleted the example branch October 21, 2015 06:27
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.

2 participants