avoid flicker when changing from lockscreen to waiting screen #1618

benma · 2025-10-01T11:44:32Z

The BitBox shows the lockscreen ("See the BitBoxApp") first, and when the noise connection is initiazed, switches to the BitBox logo, which stays there as the waiting/default screen until reconnected.

The noise connection is initiated by the host after unlock finishes, so in between, "See the BitBoxApP" flashes for a split second.

This commit solves this by

not pushing the lockscreen as an extra component to be popped, but folding it into the top/default component
updating that screen when the app initiates the connection if the device is not initialized
if initialized, updating the screen on OP_UNLOCK (see comment in code)

This prevents flashing/flickering if the device is initialized. There is still a small flicker when the device is uninitialized, as the screen is updated before the pairing confirmation appears. This was the case before this commit too.

NickeZ

Left some comments

NickeZ · 2025-10-06T07:56:18Z

src/ui/components/waiting.c

+    }
+    data->host_connected = true;
+
+    if (component->sub_components.amount != 1) {


this should be an ASSERT. The subcomponent is clearly added in the constructor.

The problem with ASSERT is that they don't run in prod, and if the assumption is wrong for some reason, there will be a memory bug. I could do Abort(...) instead?

yeah, Abort is fine as well if you want it to be checked in prod

NickeZ · 2025-10-06T08:00:01Z

src/ui/screen_process.c

    UG_SendBuffer();
 }

+static component_t* _waiting_screen = NULL;


This static could be inside _get_waiting_screen.

No, as it is needed by screen_process_host_connected() (now renamed to screen_process_waiting_switch_to_logo().

NickeZ · 2025-10-06T08:04:36Z

src/ui/components/waiting.c

    waiting->position.left = 0;
+    waiting->data = data;
+
+    ui_util_add_sub_component(waiting, lockscreen_create());


Since the lockscreen isn't a screen anymore, but a subcomponent, should we rename it to something like locked_device_background instead?

I don't see a need, in the end it's doing the same as before 🤷

src/ui/components/waiting.c

NickeZ · 2025-10-06T08:20:47Z

src/ui/screen_process.h

 component_t* screen_process_get_top_component(void);

+/**
+ * Wraps `waiting_host_connected()` for the waiting screen.


Shouldn't this be either in communication_mode module or in some connection_status module? screen_process doesn't feel like the right place to keep track of if the device is connected to a host.

I don't see a point in this function just wrapping waiting_host_connected(), why can't callers of screen_process_host_connected() just call waiting_host_connected()?

Because they don't have access the component_t* static instance.

Shouldn't this be either in communication_mode module or in some connection_status module? screen_process doesn't feel like the right place to keep track of if the device is connected to a host.

I don't think so, because the moment we call it is not generic, it's tailored to update this specific screen in a way that reduces flashing/flickering (e.g. delayed until OP_UNLOCK is called, even though the host has connected potentially much earlier already). If you prefer, I could still move it somewhere else and document it that way, but I don't see much benefit in that.

Maybe better to just rename "waiting_host_connected" to "waiting_update_display" or similar?

why do you think it needs the indirection via screen_process. How is it related to the screen processing?

screen_process.c is where static component_t* _waiting_screen = NULL; is defined, because screen_process_get_top_component(void) calls _get_waiting_screen(void) which instantiates it. So if you want to call waiting_host_connected(<waiting component>) (or waiting_switch_to_logo or whatever the name will be), it's wrapped in screen_process.c because it needs to pass the component instance.

Renamed now

NickeZ · 2025-10-06T12:13:44Z

src/rust/bitbox02-rust/src/hww.rs

+    // connected. When the device is initialized, we delay this until the unlock call, otherwise
+    // there would be a flicker where the logo would be shown before the host invokes unlock.
+    if !bitbox02::memory::is_initialized() || usb_in.as_slice() == [OP_UNLOCK] {
+        bitbox02::ui::screen_process_host_connected();


i think the name screen_process_host_connected() here must be clearer, I have a very hard time understanding what it does. Is it supposed to be waiting_screen_switch_to_logo()?

host_connected() makes me think it is check, like if host_connected() { do_something }.

Yeah I like your name (switch_to_logo), will use that 👍

NickeZ · 2025-10-06T12:15:56Z

src/ui/components/waiting.c

-    ui_util_component_render_subcomponents(component);
-}
+typedef struct {
+    bool host_connected;


based on other comments I think the name of this variable is wrong. Is this truly true if and only if the host is connected?

Well no, as explained in another comment :) will make an update and rename everything

Renamed everything now

NickeZ · 2025-10-06T12:19:13Z

Could be done in a later PR. But what do you think about renaming waiting_screen to idle_screen and have ui_screen_stack_top return the Idle screen in case the stack is empty?

benma · 2025-10-09T15:56:19Z

Could be done in a later PR. But what do you think about renaming waiting_screen to idle_screen and have ui_screen_stack_top return the Idle screen in case the stack is empty?

screen_process_get_top_component() basically does what you suggest, and it seems better to keep it in screen_process than to have ui_screen_stack take care of it, but not a very strong opinion.

Renaming waiting_screen to idle_screen, I also don't have a strong opinion, can do.

NickeZ · 2025-10-09T16:37:35Z

Could be done in a later PR. But what do you think about renaming waiting_screen to idle_screen and have ui_screen_stack_top return the Idle screen in case the stack is empty?

screen_process_get_top_component() basically does what you suggest, and it seems better to keep it in screen_process than to have ui_screen_stack take care of it, but not a very strong opinion.

Renaming waiting_screen to idle_screen, I also don't have a strong opinion, can do.

Let's refactor later...

The BitBox shows the lockscreen ("See the BitBoxApp") first, and when the noise connection is initiazed, switches to the BitBox logo, which stays there as the waiting/default screen until reconnected. The noise connection is initiated by the host after unlock finishes, so in between, "See the BitBoxApp" flashes for a split second. This commit solves this by 1) not pushing the lockscreen as an extra component to be popped, but folding it into the top/default component 2) updating that screen when the app initiates the connection if the device is not initialized 3) if initialized, updating the screen on OP_UNLOCK (see comment in code) This prevents flashing/flickering if the device is initialized. There is still a small flicker when the device is uninitialized, as the screen is updated before the pairing confirmation appears. This was the case before this commit too.

benma force-pushed the waitingscreen branch 2 times, most recently from 3b9dfa7 to 05b8fcb Compare October 1, 2025 12:15

benma requested a review from NickeZ October 1, 2025 12:15

benma mentioned this pull request Oct 1, 2025

Async bip39 #1600

Merged

NickeZ reviewed Oct 6, 2025

View reviewed changes

benma force-pushed the waitingscreen branch from 05b8fcb to 8f34a0e Compare October 9, 2025 20:16

avoid flicker when changing from lockscreen to waiting screen #1618

Are you sure you want to change the base?

avoid flicker when changing from lockscreen to waiting screen #1618

Uh oh!

Conversation

benma commented Oct 1, 2025

Uh oh!

NickeZ left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NickeZ commented Oct 6, 2025

Uh oh!

benma commented Oct 9, 2025

Uh oh!

NickeZ commented Oct 9, 2025

Uh oh!

Uh oh!