Skip to content

new experience on charging #3382

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+40 −18
Open

new experience on charging #3382

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 40 additions & 18 deletions omi/firmware/omi/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,31 +87,53 @@ static void boot_ready_sequence(void)

void set_led_state()
{
static bool charge_blink_state = false;

// Set LED state based on connection and charging status
if (is_charging) {
set_led_green(true);
// When charging, alternate blink between green and connection status color
charge_blink_state = !charge_blink_state;

if (charge_blink_state) {
// Green phase
set_led_green(true);
set_led_red(false);
set_led_blue(false);
} else {
// Connection status color phase
set_led_green(false);
if (is_connected) {
set_led_blue(true);
set_led_red(false);
} else {
set_led_red(true);
set_led_blue(false);
}
}
} else {
// Not charging - reset blink state and turn off green
charge_blink_state = false;
set_led_green(false);
}

// If device is off, turn off all status LEDs except charging indicator
if (is_off) {
set_led_red(false);
set_led_blue(false);
return;
}
// If device is off, turn off all status LEDs
if (is_off) {
set_led_red(false);
set_led_blue(false);
return;
}

if (is_connected) {
set_led_blue(true);
set_led_red(false);
return;
}
if (is_connected) {
set_led_blue(true);
set_led_red(false);
return;
}

// Not connected - RED
if (!is_connected) {
set_led_red(true);
set_led_blue(false);
return;
// Not connected - RED
if (!is_connected) {
set_led_red(true);
set_led_blue(false);
return;
}
}
Comment on lines +90 to 137
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 6, 2025

Choose a reason for hiding this comment

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

high

The logic in this function has become quite complex and introduces a potential behavioral change when the device is off.

  1. is_off behavior: When is_charging and is_off are both true, the LED will now blink. The previous behavior was to show a solid green light. A blinking light might incorrectly suggest to the user that the device is on. It's better to handle the is_off case explicitly to provide a clear and consistent user experience.
  2. Code Complexity & Duplication: The logic to determine the connection status color (red/blue) is duplicated. The overall structure can be simplified to be more readable and maintainable.

I suggest refactoring the function to address these points. The suggestion below first handles the is_off case, and then simplifies the logic for when the device is on, removing code duplication.

    static bool charge_blink_state = false;

    // Handle device off state first: solid green if charging, otherwise all off.
    if (is_off) {
        set_led_green(is_charging);
        set_led_red(false);
        set_led_blue(false);
        charge_blink_state = false; // Reset blink state for when device is turned on
        return;
    }

    // Device is on.
    if (is_charging) {
        charge_blink_state = !charge_blink_state;

        if (charge_blink_state) {
            // Green phase of charging blink
            set_led_green(true);
            set_led_red(false);
            set_led_blue(false);
            return;
        }
        // For the other blink phase, fall through to show connection status color.
    } else {
        // Not charging, so reset blink state.
        charge_blink_state = false;
    }

    // Show connection status (used for non-charging state and one phase of charging blink).
    set_led_green(false);
    if (is_connected) {
        set_led_blue(true);
        set_led_red(false);
    } else {
        set_led_red(true);
        set_led_blue(false);
    }

}

Expand Down