Skip to content

Comments

Add ClockSyncConfigurator for Unitree WebRTC#1346

Open
leshy wants to merge 12 commits intodevfrom
ivan/feat/clock-sync-configurator
Open

Add ClockSyncConfigurator for Unitree WebRTC#1346
leshy wants to merge 12 commits intodevfrom
ivan/feat/clock-sync-configurator

Conversation

@leshy
Copy link
Contributor

@leshy leshy commented Feb 22, 2026

Problem

Closes #1344
Closes DIM-575

When running Unitree robots over WebRTC, clock drift between the host and robot causes timestamp mismatches. There was no automated check or fix for this.

Solution

  • ClockSyncConfigurator pure py SNTP query detects clock offset (threshold: ±100ms). NTP unreachable -> gracefully warns (works offline). tries ntpdate > sntp > date -s fallback.

  • system_checks() bridge - wraps SystemConfigurator(s) into a Blueprint.requirements() compatible callable

  • system_configurator refactored into a package - base.py (framework), lcm.py (multicast/buffer), clock_sync.py (new).

  • dimos/utils/human.py - consolidated 6 scattered duration/byte/number formatters into human_duration(), human_bytes(), human_number().

  • Integrated into unitree_go2_basic and unitree_g1_basic hardware blueprints.

Breaking Changes

None

How to Test

# offset system clock, run configurator
sudo date -s "+1 second"
dimos --replay run unitree-go2

Issues

configurator API needs some improvements

  • always print comands that will be executed in a consistent way (no per-configurator str building)
  • OS checks belong in the configurator itself, inconvinent to check for OS and use different configurators externally

Contributor License Agreement

  • I have read and approved the CLA.

Split the monolithic system_configurator.py into a package for better
organization as more configurators are added. base.py has the ABC and
helpers, clock_sync.py has ClockSyncConfigurator, lcm.py has LCM
configurators. __init__.py re-exports everything for backward compat.
Drop requirements.py and its tests — the global_config-based connection
type check was unnecessary. Blueprints now call
system_checks(ClockSyncConfigurator()) directly.
…n.py

Replace 6 scattered duration/byte formatting implementations with two
shared functions: human_duration() and human_bytes().
Replace hardcoded timedatectl + systemd-timesyncd with a fallback chain:
ntpdate > sntp > date -s. Avoids touching systemd on systems where
timesyncd isn't installed (e.g. Arch). Also removes duplicate warning
printed from check() (explanation() already handles it).
Move msgs/sec formatter from benchmark heatmap into dimos/utils/human.py
as human_number() (42, 1.5k, 3.2M).
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 22, 2026

Greptile Summary

This PR adds automated clock synchronization checks for Unitree WebRTC deployments and consolidates human-readable formatters across the codebase.

Key Changes:

  • ClockSyncConfigurator performs pure-Python SNTP queries to detect clock drift (±100ms threshold) with graceful degradation when NTP is unreachable
  • system_checks() bridge function integrates SystemConfigurator into Blueprint.requirements() workflow
  • system_configurator refactored from single file into package (base.py, lcm.py, clock_sync.py) with backward-compatible re-exports
  • dimos/utils/human.py consolidates 6 scattered formatters into 3 functions: human_duration(), human_bytes(), human_number()
  • Integrated into unitree_go2_basic and unitree_g1_basic hardware blueprints

Breaking Changes (intentional):

  • BandwidthUnit enum removed from lcmspy.py
  • Topic.kbps_hr() and Topic.total_traffic_hr() now return str instead of tuple[float, BandwidthUnit]

Testing:

  • 67 total tests including comprehensive clock sync coverage (NTP reachable/unreachable, offset scenarios, platform-specific fixes)
  • All breaking changes have corresponding test updates

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • Clean refactoring with comprehensive test coverage (67 tests), graceful error handling, backward compatibility maintained via package re-exports, and breaking changes are intentional/documented
  • No files require special attention

Important Files Changed

Filename Overview
dimos/protocol/service/system_configurator/clock_sync.py New ClockSyncConfigurator for detecting/fixing clock drift via SNTP, handles errors gracefully
dimos/protocol/service/system_configurator/base.py Extracted base SystemConfigurator framework and added system_checks() bridge function
dimos/utils/human.py Consolidated human-readable formatters from 6 locations into 3 functions
dimos/robot/unitree/go2/blueprints/basic/unitree_go2_basic.py Added ClockSyncConfigurator to blueprint requirements for WebRTC deployments
dimos/robot/unitree/g1/blueprints/basic/unitree_g1_basic.py Added ClockSyncConfigurator to blueprint requirements for WebRTC deployments
dimos/utils/cli/lcmspy/lcmspy.py Breaking change: Topic.kbps_hr() and total_traffic_hr() now return str instead of tuple

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Blueprint.requirements] --> B[system_checks wrapper]
    B --> C[configure_system]
    C --> D{CI environment?}
    D -->|Yes| E[Skip all checks]
    D -->|No| F[Run ClockSyncConfigurator.check]
    F --> G{NTP query}
    G -->|Timeout/Error| H[Pass gracefully]
    G -->|Success| I{abs offset > 100ms?}
    I -->|No| J[Pass]
    I -->|Yes| K[Show explanation]
    K --> L{User approves fix?}
    L -->|No non-critical| M[Continue with warning]
    L -->|Yes| N[Run fix chain]
    N --> O{Platform?}
    O -->|Linux| P[ntpdate > sntp > date -s]
    O -->|macOS| Q[sntp -sS]
    P --> R[Blueprint starts]
    Q --> R
    J --> R
    H --> R
    M --> R
Loading

Last reviewed commit: f2c499b

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

12 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

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.

1 participant