fix(ota-hil): set recovery mode before capture logs #903

chrisgalanis · 2026-01-05T23:08:18Z

This pull request introduces several improvements and enhancements to the OTA update and reboot process, focusing on reliability, observability, and configurability. Key changes include more robust time synchronization checks, improved handling of boot log capture, and better control over the reboot sequence to ensure devices do not inadvertently enter recovery mode.

OTA Update Flow Improvements:

Added a new --skip-time-sync-before-reboot flag to allow skipping the NTP time synchronization check before the first reboot, with clear logging and fallback behavior. [1] [2]
Created log directories as needed before OTA operations, improving robustness when writing logs.

Reboot and Recovery Handling:

The reboot process now waits for SSH disconnection before manipulating the recovery pin, ensuring the pin is held only during the actual shutdown window. The duration for holding the recovery pin is increased to 20 seconds, and a short delay is added after releasing the pin to allow for USB device re-enumeration.
The logic for setting the recovery pin during reboot is now parameterized based on the recovery state, improving clarity and correctness. [1] [2]
Increased the sleep duration after reboot from 4 to 10 seconds to provide more time for device stabilization.

Boot Log Capture Enhancements:

Boot log capture is now performed in a background task, running concurrently with SSH reconnection attempts. The capture waits until a login prompt is detected, writes data incrementally to disk, and provides detailed logging on success or failure. [1] [2] [3]
The boot log capture function is refactored into a static method for easier background invocation and improved testability.

Time Synchronization Robustness:

The time synchronization check now automatically detects and uses either timedatectl or chronyc, with command timeouts and more robust output parsing. If neither tool is found, a clear error is raised.
Time sync is now always checked after reboot and before starting the update, regardless of the pre-reboot check configuration.

Utility and Observability Improvements:

Added a utility method to wait for SSH disconnection, improving reliability in detecting device shutdown before proceeding with hardware-level operations.

These changes collectively improve the reliability, observability, and flexibility of the OTA update and device reboot process, making it easier to diagnose issues and adapt to different environments.

OTA update and logging enhancements:

Added --skip-time-sync-before-reboot flag to optionally skip NTP check before the first reboot, with clear logging and fallback. [1] [2]
Ensured log directories are created before writing logs, improving robustness.

Reboot sequence and recovery pin handling:

Waits for SSH disconnect before holding the recovery pin, increases hold duration to 20 seconds, and adds a delay after releasing the pin for USB re-enumeration.
Parameterized recovery pin state during reboot for clarity and correctness. [1] [2]
Increased post-reboot sleep from 4 to 10 seconds for device stabilization.

Boot log capture improvements:

Boot log capture runs in the background, waits for login prompt, writes incrementally to disk, and provides detailed success/failure logs. [1] [2] [3]
Refactored boot log capture into a static method for better background usage.

Time synchronization checks:

Automatically detects and uses timedatectl or chronyc for time sync, with timeouts and robust output parsing. Raises error if neither is found.
Always checks time sync after reboot, regardless of pre-reboot check.

Utility and reliability improvements:

Added utility to wait for SSH disconnection, improving reliability in detecting shutdown before hardware operations.

AlexKaravaev · 2026-01-11T16:38:40Z

hil/src/commands/ota/mod.rs

        let _start_time = Instant::now();
        info!("Starting OTA update to version: {}", self.target_version);

+        // Create log directory if it doesn't exist


i think we can remove comments like this one, they don't bring much value

AlexKaravaev · 2026-01-11T16:39:21Z

hil/src/commands/ota/mod.rs

-                info!("Overlays wiped successfully, rebooting device");
+                info!("Overlays wiped successfully");
+
+                if !self.skip_time_sync_before_reboot {


why we need this I wonder ?

So, basically the problem was with the worldcoin-key-retrival service. With pearl, teh harware clock is not working so if it NTP server is not synced then worldcoin-attest is failing and also key-retrieval. This should be fixed with new key-retrieval fix, that is why i ketp it as a flag, so we can test older commits, that don't have key-retrival fixed

I am also not getting it sorry.

With pearl, teh harware clock is not working so if it NTP server is not synced then worldcoin-attest is failing and also key-retrieval.

That is not true. The clock will synchronize and https requests is going to succeed. And the delay is not big for that

By default this is not correct. Because reboot is manually triggered faster than the NTP server syncrhonizes the clock....

AlexKaravaev · 2026-01-11T16:40:02Z

hil/src/commands/ota/reboot.rs

+        // Brief delay to allow USB device to be re-enumerated and udev rules to apply
+        // after FTDI GPIO is released. The FTDI device detaches/reattaches kernel
+        // drivers which causes /dev/ttyUSB* to be recreated.
+        tokio::time::sleep(Duration::from_millis(200)).await;


timings like this might be weak spot and break often - but if works now i am fine

I have tested and look reliable enough. Even though if we don't ahve logs in the future we will now the root of failure and fix it.

Better then make it a constant

AlexKaravaev · 2026-01-11T16:41:41Z

hil/src/commands/ota/reboot.rs

    #[instrument(skip_all)]
-    async fn capture_boot_logs(&self, log_suffix: &str) -> Result<()> {
-        let platform_name = format!("{:?}", self.platform).to_lowercase();
+    async fn capture_boot_logs_static(


i think it's bad practice to have static in the end of the name

I will fix!

AlexKaravaev · 2026-01-11T16:45:27Z

hil/src/commands/ota/reboot.rs

            }

-            if start_time.elapsed() >= timeout && !found_login_prompt {
+            if found_login_prompt {


where do we set timeout for this?

The timeout is the basically if the login prompth is fouund or not. If login prompt is never found, then the timeout of ssh will fail later so the process will exit with error

AlexKaravaev · 2026-01-11T16:47:23Z

hil/src/commands/ota/system.rs

-        let result = session
-            .execute_command("TERM=dumb timedatectl status")
+        // Timeout for individual command execution (10 seconds is generous for timedatectl/chronyc)
+        const COMMAND_TIMEOUT: Duration = Duration::from_secs(10);


better to define it in top of the function for readibility

AlexKaravaev · 2026-01-11T16:48:56Z

hil/src/boot.rs

    .await
    .wrap_err("task panicked")??;
-    tokio::time::sleep(Duration::from_secs(4)).await;
+    tokio::time::sleep(Duration::from_secs(10)).await;


move to the top of the file or maybe have as separate arg to cli so you don't tune it in sources every time?

I will fix!

…o chris/ota-hil

AlexKaravaev · 2026-01-12T20:11:05Z

hil/src/commands/ota/reboot.rs


 use super::Ota;

+const DELAY_CAPTURE_LOGS: u64 = 200;


nit: better to specify MS here as it's not clear what 200 is (seconds, ms, etc.)

AlexKaravaev · 2026-01-12T20:12:30Z

hil/src/commands/ota/system.rs


    const MAX_ATTEMPTS: u32 = 60; // 60 attempts = 2 minutes max wait
    const SLEEP_DURATION: Duration = Duration::from_secs(2);
+    // Timeout for individual command execution (10 seconds is generous for timedatectl/chronyc)


nit: (10 seconds is generous for timedatectl/chronyc) is not helpful as a comment

AlexKaravaev · 2026-01-12T20:12:59Z

hil/src/boot.rs

    .await
    .wrap_err("task panicked")??;
-    tokio::time::sleep(Duration::from_secs(4)).await;
+    tokio::time::sleep(Duration::from_secs(10)).await;


You are not using the const here

this is only blocknig comment - everything else is great job!

…o chris/ota-hil

chrisgalanis added 30 commits July 21, 2025 21:00

fix: save persistent

d2fea56

Merge branch 'main' of github.com:worldcoin/orb-software

195d75e

Merge branch 'main' of github.com:worldcoin/orb-software

d02e2b5

Merge branch 'main' of github.com:worldcoin/orb-software

9040984

Merge branch 'main' of github.com:worldcoin/orb-software

9ba8e91

Merge branch 'main' of github.com:worldcoin/orb-software

8866016

Merge branch 'main' of github.com:worldcoin/orb-software

b7f0532

Merge branch 'main' of github.com:worldcoin/orb-software

a950a82

Merge branch 'main' of github.com:worldcoin/orb-software

f85882a

Merge github.com:worldcoin/orb-software

912df00

Merge branch 'main' of github.com:worldcoin/orb-software

68cb5cd

Merge github.com:worldcoin/orb-software

1543d27

Merge branch 'main' of github.com:worldcoin/orb-software

b75b812

Merge github.com:worldcoin/orb-software

8dda1fb

Merge branch 'main' of github.com:worldcoin/orb-software

3725ede

Merge branch 'main' of github.com:worldcoin/orb-software

7b12495

Merge github.com:worldcoin/orb-software

2466287

Merge branch 'main' of github.com:worldcoin/orb-software

7940035

Merge branch 'main' of github.com:worldcoin/orb-software

f92ac94

fix: replace mcu-reboot with button reboot

9a55d7c

fix: remove upload.sh

7973a70

fix: fmt

a8d615b

fix: clippy & build

9fe311c

fix(ota-hil): Fix target version in update_version_json

8a42a30

Merge github.com:worldcoin/orb-software into chris/ota-hil

368513b

fix(ota-hil): Fix target version Clippy

b50d887

fix(ota-hil): Fix Format

e27f1cc

feat(ota-hil): Set Non Recovery Mode after Reboot from the MCU

19fb770

fix(ota-hil): Remove unused command

7e24ebf

fix(ota-hil): Set BTN Pin to High to not trigger button reboot

7a7c7c5

Merge branch 'main' into chris/ota-hil

3cb60a9

AlexKaravaev requested changes Jan 11, 2026

View reviewed changes

chrisgalanis added 4 commits January 12, 2026 13:59

fix: clean code

22e027c

fix: format

7a1b966

Merge github.com:worldcoin/orb-software into chris/ota-hil

fe82002

Merge branch 'chris/ota-hil' of github.com:worldcoin/orb-software int…

1df5421

…o chris/ota-hil

chrisgalanis requested review from AlexKaravaev and sfikastheo January 12, 2026 13:16

fix: replace delay value with constant

6f2a845

sfikastheo approved these changes Jan 12, 2026

View reviewed changes

AlexKaravaev requested changes Jan 12, 2026

View reviewed changes

chrisgalanis and others added 11 commits January 13, 2026 01:39

Merge branch 'chris/ota-hil' of github.com:worldcoin/orb-software int…

73b249a

…o chris/ota-hil

fix: try chrony first & fallback to timedatectl

022ae7b

Merge branch 'chris/ota-hil' of github.com:worldcoin/orb-software int…

d2ff64c

…o chris/ota-hil

fix: time sync bug, unhandled error

269826c

fix: default hold for 5 secs

3568b0b

feat: Include More Serial Logs Capture When Orbs is not Reachable

857ec45

fix: format

7662565

Merge branch 'main' into chris/ota-hil

b113e94

feat: Introduce Ethernet Through Usb for Ota orb

bec8b5d

Merge branch 'chris/ota-hil' of github.com:worldcoin/orb-software int…

b2b4038

…o chris/ota-hil

feat: Ethernet through USB

2c89aa4

chrisgalanis force-pushed the chris/ota-hil branch from c044b7c to 2c89aa4 Compare January 14, 2026 11:53

chrisgalanis marked this pull request as draft January 14, 2026 11:56

fix: restore from main / nix conf

65dacd1

chrisgalanis force-pushed the chris/ota-hil branch from 6bed7d8 to 65dacd1 Compare January 14, 2026 11:57

chrisgalanis and others added 4 commits January 14, 2026 12:58

Merge branch 'main' into chris/ota-hil

0e6ac47

fix: make test only for linux

3b35464

Merge branch 'chris/ota-hil' of github.com:worldcoin/orb-software int…

aef3ffe

…o chris/ota-hil

Merge branch 'main' into chris/ota-hil

2a7ff45

fix(ota-hil): set recovery mode before capture logs #903

Are you sure you want to change the base?

fix(ota-hil): set recovery mode before capture logs #903

Uh oh!

Conversation

chrisgalanis commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

chrisgalanis commented Jan 5, 2026 •

edited

Loading