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

Add ability to invert the PIR Notify LED #765

Merged
merged 1 commit into from
Sep 22, 2020

Conversation

jmw6773
Copy link
Contributor

@jmw6773 jmw6773 commented Sep 14, 2020

I've added the option to invert the PIR notification LED at build time. It's default value is set to drive an LED through a Mosfet. If the LED is driven directly from the AVR/ESP's pin, INVERT_LED_NOTIFY should be set to true.

@1technophile
Copy link
Owner

Thanks for the PR!

On other code parts, I'm using a different method (see LED_RECEIVE_ON) by specifying if the ON status is at HIGH or LOW,
I think we should uniformize the method used, I have no bias for one or another method as long as they are easily understandable and the same.

@jmw6773 jmw6773 marked this pull request as draft September 18, 2020 01:06
@jmw6773
Copy link
Contributor Author

jmw6773 commented Sep 18, 2020

@1technophile, I don't have a preference either. I made a change that I think matches what you are doing with LED_RECEIVE_ON, but I haven't tested it, so I converted this PR to a draft. I'll try to build and push to my OMG tonight to confirm it functionality.

@1technophile 1technophile added this to the v0.9.6 milestone Sep 18, 2020
@jmw6773
Copy link
Contributor Author

jmw6773 commented Sep 22, 2020

My OMG has been stable with this change.

@jmw6773 jmw6773 marked this pull request as ready for review September 22, 2020 01:12
@1technophile
Copy link
Owner

Thanks

@1technophile 1technophile merged commit 1780458 into 1technophile:development Sep 22, 2020
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