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 turn off heartbeat LED blinking #3674

Merged
merged 7 commits into from
Apr 22, 2024

Conversation

ndoo
Copy link
Contributor

@ndoo ndoo commented Apr 20, 2024

Fixes #3635 and depends on protobufs PR #485.

This only affects the GPIO-controllable LED and may be desirable behavior for Meshtastic users who sleep in the vicinity of their node and are annoyed by the blinking LED, especially for boards that indicate charging when USB power is connected, resulting in a 1 second LED illumination cycle.

Most boards however, have a non-GPIO controlled power LED (i.e. hardwired to VBUS/VCC/etc.) that can only be turned off by desoldering or otherwise removing that LED. This can't be addressed by firmware changes.

Implemented as inhibiting rather than status_led because having status_led = true by default will cause status LED to get disabled for users upgrading from previous firmware due to installDefaultConfig not being run.

@thebentern
Copy link
Contributor

thebentern commented Apr 20, 2024

Looks good but needs a trunk fmt. I hate how bright that stupid white LED is on the heltec!

Copy link
Member

@garthvh garthvh left a comment

Choose a reason for hiding this comment

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

This is not going to provide a dark mode, can we just reduce the brightness of the heltec and not affect every device?

@thebentern
Copy link
Contributor

This is not going to provide a dark mode, can we just reduce the brightness of the heltec and not affect every device?

This just allows the user to disable the blinking white LED. I think it potentially also has some utility if we can repurpose the status LED for use with ext. notifications to control the behavior instead, in #3673

@ndoo
Copy link
Contributor Author

ndoo commented Apr 20, 2024

This is not going to provide a dark mode, can we just reduce the brightness of the heltec and not affect every device?

The default behavior of this change does not affect any device unless the user specifically flips the configuration toggle.

Reducing brightness adds a lot of complexity/refactoring to the blink routine as we have to configure and attach the ESP32 PWM LEDC to the pin, as well as adding the necessary protobuf config to support configuring the brightness to make this feature available to all other boards. Hardcoding the brightness is also possible, but then we become responsible to choose the brightness that most users would find comfortable.

Hardcoding this for just the Heltec will still require changing the main.cpp ledBlinker routine, it seems more straightforward to just implement a flag to turn the LED off entirely for users who are affected by the blinking behavior.

Fixes meshtastic#3635 and depends on [protobufs PR meshtastic#485](meshtastic/protobufs#485)

Signed-off-by: Andrew Yong <me@ndoo.sg>
@ndoo
Copy link
Contributor Author

ndoo commented Apr 20, 2024

Looks good but needs a trunk fmt. I hate how bright that stupid white LED is on the heltec!

trunk fmt fixed with latest force push

@thebentern thebentern changed the title Add ability to turn off status LED blinking Add ability to turn off heartbeat LED blinking Apr 21, 2024
@thebentern thebentern merged commit 250cf16 into meshtastic:master Apr 22, 2024
70 checks passed
@ndoo
Copy link
Contributor Author

ndoo commented Apr 22, 2024

Thanks for the major refactoring work and glad to see its inclusion.

@ndoo ndoo deleted the status-led-off branch April 22, 2024 13:55
@punkpar
Copy link

punkpar commented Apr 26, 2024

Thankyou all for following up my initial feature request. I'm still new to GitHub, will this feature be in the next alpha release of meshtastic?

@todd-herbert
Copy link
Contributor

todd-herbert commented Apr 26, 2024

will this feature be in the next alpha release of meshtastic?

This pull request was included in 2.3.7, but I believe there was a bug in the code, so the feature might not be usable until 2.3.8.

I don't believe either of the phone Apps allow you to change this setting just now. You can set it with the python CLI though, if you're interested to see what happens.

@punkpar
Copy link

punkpar commented Apr 26, 2024

Thank you todd-herbert, does this need requesting in the Android and iPhone firmware section, or is this something that will naturally be implemented?

@todd-herbert
Copy link
Contributor

todd-herbert commented Apr 26, 2024

To be honest, I don't know exactly what will happen with the different apps, but I'm sure the developers behind them are aware of the situation; no need to worry about opening requests.

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.

[Feature Request]: Option to turn off LED inducators on ESP32 Heltec V3
5 participants