feat(control): add extension sideloading#3175
Conversation
Co-authored-by: cc <55869557+TomCC7@users.noreply.github.com>
# Conflicts: # dimos/control/tasks/registry.py # dimos/hardware/drive_trains/registry.py # dimos/hardware/manipulators/registry.py # dimos/hardware/whole_body/registry.py
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Greptile SummaryAdds explicit extension sideloading for ControlCoordinator hardware adapters and task factories.
Confidence Score: 5/5The PR appears safe to merge. No blocking failures remain within the scope of the previous review threads. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
EXT[External package] --> API[dimos.control.extensions]
API --> HWREG[Hardware adapter registry]
API --> TASKREG[Control task registry]
BP[Blueprint configuration] --> COORD[ControlCoordinator]
COORD --> HWREG
COORD --> TASKREG
HWREG --> ADAPTER[External adapter]
TASKREG -->|lazy import| TASK[External control task]
TASK --> TICK[Coordinator tick loop]
TICK --> ADAPTER
Reviews (3): Last reviewed commit: "fix(control): reject malformed factory p..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| resolved later by the control task registry when a coordinator creates a | ||
| matching ``TaskConfig``. | ||
| """ | ||
| control_task_registry.register_path(task_type, factory_path) |
There was a problem hiding this comment.
When an external task consumes coordinator streams or exposes task commands, this facade stores only its factory path, so bindings_for() returns an empty binding set and the coordinator creates no routes, subscriptions, or declared commands. The task is constructed but never receives its streamed commands, while configurations using stream_bind are rejected as unknown inputs.
Knowledge Base Used: Control Coordinator and Control Tasks
| """Validate a lazy factory path of the form ``module:function``.""" | ||
| module_name, separator, attr = factory_path.partition(":") | ||
| if not factory_path.strip() or separator != ":" or not module_name or not attr: |
There was a problem hiding this comment.
The validation accepts paths such as module:factory:extra even though the documented format is module:function. Resolution later treats factory:extra as the attribute name and fails during task creation, so reject paths containing more than one separator at registration time; the adapter registry uses the same validation pattern and needs the corresponding check.
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #3175 +/- ##
==========================================
- Coverage 74.17% 73.80% -0.37%
==========================================
Files 1108 1112 +4
Lines 104052 104622 +570
Branches 9530 9621 +91
==========================================
+ Hits 77176 77221 +45
- Misses 24131 24706 +575
+ Partials 2745 2695 -50
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 36 files with indirect coverage changes 🚀 New features to boost your workflow:
|
| module_name, separator, attr = factory_path.partition(":") | ||
| if not factory_path.strip() or separator != ":" or not module_name or not attr: | ||
| raise ValueError(f"Invalid task factory path: {factory_path!r}") |
There was a problem hiding this comment.
When an extension registers a path such as pkg.tasks:create_task:extra, this validator accepts the second separator and stores the path. Lazy resolution then calls getattr(module, "create_task:extra"), causing task creation to fail with AttributeError; the adapter registry has the same incomplete validation.
|
Addressed Greptile's factory-path validation finding in 9f92f2b. Both control-task and hardware-adapter registration now require exactly one |
github-actions
Bot
added
the
ready-to-merge
1 participant
Problem
External robot packages need a supported way to register ControlCoordinator hardware adapter names and control task types without placing files inside the DimOS source tree. Without this, an outside package can define a blueprint but still cannot use
HardwareComponent(adapter_type=...)orTaskConfig(type=...)for custom hardware/control unless DimOS itself is modified.Closes DIM-1031
Solution
Add a public
dimos.control.extensionsfacade for explicit extension registration:register_hardware_adapter(HardwareType, adapter_type, factory)dispatches to the existing manipulator, base, or whole-body adapter registry.register_control_task(task_type, factory_path)registers lazy control task factories without importing the target module at registration time.This keeps blueprint usage unchanged: external packages register before coordinator construction, then use normal
HardwareComponentandTaskConfignames.How to Test
Manual QA:
Expected output includes
[external_test_robot]logs for:BASE/external_test_baseexternal_test_driveExternalTestBaseAdapterExternalTestDriveTaskContributor License Agreement