Skip to content

docs: clarify native module config migration#2692

Open
dashitongzhi wants to merge 1 commit into
dimensionalOS:mainfrom
dashitongzhi:docs/native-config-20260702091026
Open

docs: clarify native module config migration#2692
dashitongzhi wants to merge 1 commit into
dimensionalOS:mainfrom
dashitongzhi:docs/native-config-20260702091026

Conversation

@dashitongzhi

Copy link
Copy Markdown

Summary

  • Document DimOS module config as the preferred source of truth for native module settings
  • Add guidance for migrating native-only CLI flags into typed NativeModuleConfig fields
  • Include examples for blueprint and CLI option overrides while discouraging normal-use extra_args passthroughs

Test Plan

  • Ran git diff --check

Copilot AI review requested due to automatic review settings July 2, 2026 01:11
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a new "Configuration source of truth" subsection to docs/usage/native_modules.md, establishing guidance that typed NativeModuleConfig fields should be preferred over ad-hoc extra_args passthroughs for native binary flags.

  • Introduces a five-step migration pattern for converting native-only CLI flags into typed Python config fields, with notes on deprecation and eventual removal.
  • Adds concrete Python and CLI examples (all marked skip) showing the preferred blueprint() override pattern versus the discouraged extra_args approach.

Confidence Score: 5/5

Documentation-only change with no executable code; all code blocks are annotated skip and serve as illustrative examples only.

The new section's examples (config class definition, blueprint() call, CLI override syntax) are consistent with existing patterns already demonstrated earlier in the same document. The migration steps are logically ordered and the extra_args guidance does not contradict the existing config table. No executable logic is touched.

No files require special attention — only docs/usage/native_modules.md is changed.

Important Files Changed

Filename Overview
docs/usage/native_modules.md Adds a well-structured "Configuration source of truth" section with migration guidance; code examples are consistent with existing patterns in the document and all blocks are annotated skip.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Dev as Developer
    participant Cfg as NativeModuleConfig (Python)
    participant DimOS as DimOS Runtime
    participant Bin as Native Binary (C++/Rust)

    Dev->>Cfg: Define typed fields (host_ip, frequency, …)
    Dev->>DimOS: "MyLidar.blueprint(host_ip="192.168.1.10")"
    DimOS->>Cfg: Override field values
    DimOS->>DimOS: Auto-generate CLI args from config fields
    DimOS->>Bin: spawn ./build/my_lidar --host_ip 192.168.1.10 --frequency 20.0
    Bin-->>DimOS: stdout / stderr (piped through structlog)
    DimOS-->>Dev: Log output at info/warning level

    Note over Dev,Bin: Migration path: native-only --host-ip flag in extra_args → typed host_ip field → deprecate raw flag → remove
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Dev as Developer
    participant Cfg as NativeModuleConfig (Python)
    participant DimOS as DimOS Runtime
    participant Bin as Native Binary (C++/Rust)

    Dev->>Cfg: Define typed fields (host_ip, frequency, …)
    Dev->>DimOS: "MyLidar.blueprint(host_ip="192.168.1.10")"
    DimOS->>Cfg: Override field values
    DimOS->>DimOS: Auto-generate CLI args from config fields
    DimOS->>Bin: spawn ./build/my_lidar --host_ip 192.168.1.10 --frequency 20.0
    Bin-->>DimOS: stdout / stderr (piped through structlog)
    DimOS-->>Dev: Log output at info/warning level

    Note over Dev,Bin: Migration path: native-only --host-ip flag in extra_args → typed host_ip field → deprecate raw flag → remove
Loading

Reviews (1): Last reviewed commit: "docs: clarify native module config migra..." | Re-trigger Greptile

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR clarifies NativeModuleConfig as the preferred, typed “source of truth” for native-module settings, and documents a recommended migration path away from ad-hoc native-binary flags (using extra_args only as a temporary compatibility mechanism).

Changes:

  • Adds guidance emphasizing typed NativeModuleConfig fields over native-only flags.
  • Documents a step-by-step migration pattern from native-only flags to typed config fields.
  • Adds examples showing blueprint and CLI (-o ...) overrides, and discourages extra_args as the default user interface.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


### Configuration source of truth

Prefer DimOS module configuration over ad-hoc native-binary flags. Define operational settings as typed fields on the `NativeModuleConfig` subclass, then pass values through blueprint overrides, config files, environment-backed global config, or `dimos run -o ...`. This keeps configuration discoverable from Python, visible to `--help`, and reusable across local replay, simulation, and hardware blueprints.
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.

2 participants