Skip to content

Comments

Task: Create base manipulation module#1363

Closed
mustafab0 wants to merge 16 commits intomainfrom
task/mustafa-create-base-manipulation-module
Closed

Task: Create base manipulation module#1363
mustafab0 wants to merge 16 commits intomainfrom
task/mustafa-create-base-manipulation-module

Conversation

@mustafab0
Copy link
Contributor

Problem

ManipulationModule is a 1600-line monolith mixing core planning infrastructure with pick/place behaviors, GraspGen integration, and perception subscriptions. This makes it impossible to use the planner without pulling in perception and grasp generation dependencies.

Additionally, _BlueprintAtom.create in blueprints.py fails to resolve In/Out port annotations for modules using from __future__ import annotations with TYPE_CHECKING guards (e.g. RealSenseCamera), silently breaking autoconnect transport wiring.

Issue: DIM-451


Solution

Split ManipulationModule into a base class and a PickAndPlaceModule subclass:

  • ManipulationModule (base): Core planning infrastructure, motion execution, gripper control, state machine. Skills: move_to_pose, move_to_joints, go_home, go_init, get_robot_state, reset.
  • PickAndPlaceModule(ManipulationModule): Adds perception integration (objects port, obstacle monitor, scan_objects, get_scene_info), GraspGen, and long-horizon skills (pick, place, place_back, pick_and_place, clear_perception_obstacles).

PickAndPlaceModuleConfig extends ManipulationModuleConfig with GraspGen fields. Only xarm_perception and xarm_perception_agent blueprints switch to pick_and_place_module; all other blueprints use the base manipulation_module unchanged.

New blueprints: xarm7_planner_coordinator_agent for testing base module with an LLM agent.

Bug fixes:

  • move_to_pose now preserves current EE orientation when roll/pitch/yaw are omitted (previously forced identity quaternion).
  • reset changed from @rpc to @skill so agents can clear faults.
  • clear_perception_obstacles changed from @rpc to @skill for agent recovery from COLLISION_AT_START.
  • _BlueprintAtom.create now uses reversed-MRO namespace resolution for get_type_hints, fixing autoconnect transport wiring for all modules with from __future__ import annotations.

Breaking Changes

None. ManipulationModule retains all base skills and RPCs. PickAndPlaceModule is a drop-in replacement where perception/pick-place is needed. Both are exported from dimos.manipulation.


How to Test

  1. Test base agent: dimos run coordinator-mock then dimos run xarm7-planner-coordinator-agent
  2. Test perception agent: dimos run coordinator-mock then dimos run xarm-perception-agent

closes DIM-451

paul-nechifor and others added 16 commits February 22, 2026 03:31
…sts (#1350)

Busy-wait loops with no timeout could hang forever. Use threading.Event
for efficient blocking with 1s timeouts that fail clearly on expiry.
* added twist base protocol spec

* created mock twist base adapter

* added twistbase ConnectedHardware type

* updated coordinator to support twist messages

* added flowbase adapter

* added blueprint and test script for testing

* mypy test fixes

* fix  validates the adapter/hardware-type pairing upfront with a clear error

* fixed redundant blueprint assignments

* fixed mypy type ignore flags

* added thread locks to velocity read write for flowbase adapter

* fix mypy errors: portal import-untyped and adapter property override

* added echo cmd vel script back for testing will be deprecated

* removed echo_cmd_vel test script
…ntegration (#1351)

Move adding_a_custom_arm.md from docs/development/ to
docs/capabilities/manipulation/ where it belongs. Remove the outdated
depth_camera_integration.md and fix resulting broken links.
* Moving towards deprecating `Agent`. It has been efectivelly split between `McpClient` and `McpServer`.
* McpServer exposes all the `@skill` functions as MCP tools.
* McpClient starts a langchain agent which coordinates calling tools from McpServer.
* McpClient is not strictly necessary. You can register the MCP server in claude code and make it call tools.
* McpClient also listens to `/human_input` so it can be used through the `humancli` like `Agent` can.
* added go2 preflight checklist

* typo

---------

Co-authored-by: Paul Nechifor <paul@nechifor.net>
- Add docs/platforms/humanoid/g1/index.md with full G1 guide
- Update README.md to link G1 to new docs instead of todo.md
- Covers: installation, simulation, real robot, agentic control,
  arm gestures, movement modes, keyboard teleop, all blueprints

Closes DIM-576
…1348)

* feat(doclinks): validate and resolve .md links, fix broken doc links

Add Pattern 3 to doclinks that validates existing .md links in docs:
- Resolves relative .md paths to absolute links
- Validates absolute .md links exist on disk
- Falls back to doc_index search for broken links with disambiguation
- Handles index.md files by searching parent dir name
- Scores candidates by directory overlap + filename match

Delete duplicate docs/usage/transports.md (identical to transports/index.md).

Fixes 5 broken links across docs/ (agents/docs/index.md, capabilities/
navigation/readme.md, usage/lcm.md).

* refactor
* add missing command

* Apply suggestion from @greptile-apps[bot]

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

* fix

* Update bin/gen-diagrams

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

---------

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
* feat(blueprints): add scratch

* feat(blueprints): be able to run modules too

* remove scratch
@mustafab0 mustafab0 linked an issue Feb 25, 2026 that may be closed by this pull request
4 tasks
@mustafab0 mustafab0 changed the title Task/mustafa create base manipulation module Task: Create base manipulation module Feb 25, 2026
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 25, 2026

Too many files changed for review. (127 files found, 100 file limit)

result = await handle_request(body, request.app.state.skills, request.app.state.rpc_calls)
if result is None:
return Response(status_code=204)
return JSONResponse(result)

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.

Copilot Autofix

AI about 15 hours ago

In general, to fix this kind of issue, keep detailed exception information (messages, stack traces, internal paths, SQL, etc.) in server-side logs only, and return a generic, non-sensitive error message to clients. Ideally, include only high-level context that is safe to expose and, if needed, a correlation ID that lets operators look up the detailed error in logs.

For this specific code, the problematic part is the except block in _handle_tools_call. It both logs the exception (good) and then returns a JSON-RPC result whose text includes the full exception string (bad). We should keep the logging as is, but change the returned message so that it does not interpolate e. Instead, we can use a generic message like "Error running tool '<name>'. Check server logs for details." or even simpler, "Error running tool.". This preserves existing functionality from the client’s perspective (they still get an error and know which tool failed), while avoiding disclosure of internal exception messages.

Concretely:

  • In dimos/agents/mcp/mcp_server.py, in _handle_tools_call, replace:
    • return _jsonrpc_result_text(req_id, f"Error running tool '{name}': {e}")
  • With:
    • return _jsonrpc_result_text(req_id, f"Error running tool '{name}'. Please check server logs for details.")
      (or similar wording that excludes e).

No new methods or imports are needed: we continue to use logger.exception, _jsonrpc_result_text, and existing utilities.

Suggested changeset 1
dimos/agents/mcp/mcp_server.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/dimos/agents/mcp/mcp_server.py b/dimos/agents/mcp/mcp_server.py
--- a/dimos/agents/mcp/mcp_server.py
+++ b/dimos/agents/mcp/mcp_server.py
@@ -102,7 +102,10 @@
         result = await asyncio.get_event_loop().run_in_executor(None, lambda: rpc_call(**args))
     except Exception as e:
         logger.exception("Error running tool", tool_name=name, exc_info=True)
-        return _jsonrpc_result_text(req_id, f"Error running tool '{name}': {e}")
+        return _jsonrpc_result_text(
+            req_id,
+            f"Error running tool '{name}'. Please check server logs for details.",
+        )
 
     if result is None:
         return _jsonrpc_result_text(req_id, "It has started. You will be updated later.")
EOF
@@ -102,7 +102,10 @@
result = await asyncio.get_event_loop().run_in_executor(None, lambda: rpc_call(**args))
except Exception as e:
logger.exception("Error running tool", tool_name=name, exc_info=True)
return _jsonrpc_result_text(req_id, f"Error running tool '{name}': {e}")
return _jsonrpc_result_text(
req_id,
f"Error running tool '{name}'. Please check server logs for details.",
)

if result is None:
return _jsonrpc_result_text(req_id, "It has started. You will be updated later.")
Copilot is powered by AI and may make mistakes. Always verify output.
@mustafab0 mustafab0 closed this Feb 25, 2026
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.

Create base Manipulation Module

5 participants