Skip to content

gluon-setup-mode: provide LED feedback for setup-mode activation#3499

Open
blocktrron wants to merge 1 commit intofreifunk-gluon:mainfrom
blocktrron:pr-setup-mode-indication
Open

gluon-setup-mode: provide LED feedback for setup-mode activation#3499
blocktrron wants to merge 1 commit intofreifunk-gluon:mainfrom
blocktrron:pr-setup-mode-indication

Conversation

@blocktrron
Copy link
Member

This adds a fast-blink animation when Gluon started the procedure to enter setup mode.

This is helpful, as some devices might not provide feedback by the status LED on reboot.
This can lead to unwanted activation of a bootloader recovery procedure in case the reset button is not released prior.

@github-actions github-actions bot added the 3. topic: package Topic: Gluon Packages label May 5, 2025

wait_setup_mode() {
sleep $wait
/lib/gluon/setup-mode/led.sh confirm
Copy link
Member

Choose a reason for hiding this comment

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

There is a race condition here; the shell process could be killed between setting the led and running gluon-enter-setup-mode. Maybe we should move the LED trigger into gluon-enter-setup-mode?

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is the target-specific implementation of the diag.sh script which throws unrelated errors for our case.

On pure DT platforms we would not need it (only real consumer for us is x86 with apu)

Copy link
Member

Choose a reason for hiding this comment

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

I don't understand how this answers my comment

Copy link
Member Author

@blocktrron blocktrron May 9, 2025

Choose a reason for hiding this comment

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

Hey, sorry i did not finish it.

So the race condition is for the cause of gluon-enter-setup-mode setting set -eu.

The LED definition is sourced from diag.sh which is included in other contexts where additional variables are set.

Including it within the gluon-enter-setup-mode script results in the script aborting early.

I hope this is more understandable.

Copy link
Member

Choose a reason for hiding this comment

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

The race condition I meant was wait_setup_mode being killed after /lib/gluon/setup-mode/led.sh confirm, but before gluon-enter-setup-mode, if the button is released at exactly the wrong moment.

Copy link
Member Author

Choose a reason for hiding this comment

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

Now i get it. I'll think about how to tackle this.

Copy link
Member

Choose a reason for hiding this comment

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

The easiest way to fix this would be to call /lib/gluon/setup-mode/led.sh confirm from a subprocess together with gluon-enter-setup-mode instead of wait_setup_mode - either by moving the call into gluon-enter-setup-mode itself, or by introducing a new shell using sh -c for the two commands.

@Djfe
Copy link
Contributor

Djfe commented May 9, 2025

I like this idea a lot actually. Since I did it many times I know by now that I can release the button way earlier but this is actual useful feedback for our end-users!

This adds a fast-blink animation when Gluon started the procedure to
enter setup mode.

This is helpful, as some devices might not provide feedback by the
status LED on reboot. This can lead to unwanted activation of a
bootloader recovery procedure.

Signed-off-by: David Bauer <mail@david-bauer.net>
@blocktrron blocktrron force-pushed the pr-setup-mode-indication branch from c234331 to fb06163 Compare May 9, 2025 19:56
@AiyionPrime AiyionPrime added the 2. status: waiting-on-author Waiting on some action from the author label Jul 6, 2025
@rotanid
Copy link
Member

rotanid commented Nov 18, 2025

@blocktrron are you still interested in getting this merged, then it would be good to follow up with the remark by @neocturne and fixing the linting :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. status: waiting-on-author Waiting on some action from the author 3. topic: package Topic: Gluon Packages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants