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

Remove LED_INVERTED, see below for why ;-) #4382

Merged
merged 1 commit into from
Aug 5, 2024

Conversation

geeksville
Copy link
Member

While working on #4378 I noticed a funny problem: the blinking system LED was on during deep-sleep. Initially I thought it was some weird sleep hw config thing but it turns out it was easier but more pervasive.

We had two different preprocessor symbols which both meant approximately the same thing LED_INVERTED and LED_STATE_ON (though their polarity was opposite). Some variant files were setting one, others were setting the other, and others were setting both. heh.

In the case of the board I was testing (seeed tracker wio 1100) it was only setting one and the default behavior for the other (for all boards) was incorrect. So I did a grep and it seems like LED_STATE_ON was used more often, so I kept that one and removed LED_INVERTED everywhere.

While working on meshtastic#4378 I noticed a funny problem: the blinking system
LED was on during deep-sleep.  Initially I thought it was some weird
sleep hw config thing but it turns out it was easier but more pervasive.

We had two different preprocessor symbols which both meant approximately the same
thing LED_INVERTED and LED_STATE_ON (though their polarity was opposite).
Some variant files were setting one, others were setting the other, and others were
setting both. heh.

In the case of the board I was testing (seeed tracker wio 1100) it was only setting one
and the default behavior for the other (for all boards) was incorrect.  So I did a grep
and it seems like LED_STATE_ON was used more often, so I kept that one and removed
LED_INVERTED everywhere.
@geeksville geeksville marked this pull request as ready for review August 3, 2024 19:50
@caveman99
Copy link
Member

All changed variant targets are esp32 boards. Could it be this was tool chain dependant at one time?

@geeksville
Copy link
Member Author

Good theory. IDK but I kinda doubt it because both are just using standard cpp predefines. It might just be back when I started it I had both (because stupid or just random codereuse history ;-) ).

@thebentern thebentern merged commit 1a38c4e into meshtastic:master Aug 5, 2024
100 checks passed
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