[#23896] Create tools for inpection (ros2 cli)#5
Conversation
cferreiragonz
left a comment
There was a problem hiding this comment.
First review just checking the code. We can consider creating some tests to check all tools.
I am concerned about the optional input parameters. I don't know how all LLMs will treat them.
I will re-review after reviewing Textualize PR
0acb027 to
17a2083
Compare
cferreiragonz
left a comment
There was a problem hiding this comment.
The default tools also require the user to provide a ROS2 node to the console, which implies building code to use them, we should be able to provide a real default tools where the user could use them without coding
Should we disable the publisher/subscriber tools, or add a SharedNode class in |
| _default_tools: bool = True | ||
| ): | ||
| super().__init__(model, registry, validator, k, max(3, hist_depth), logger) | ||
| super().__init__(model, registry, validator, k, max(3, hist_depth), logger, _default_tools) |
There was a problem hiding this comment.
| super().__init__(model, registry, validator, k, max(3, hist_depth), logger, _default_tools) | |
| super().__init__(model=model, registry=registry, validator=validator, k=k, hist_depth=max(3, hist_depth), logger=logger, default_tools=default_tools) |
There was a problem hiding this comment.
Remember to apply this
|
Also managed the exception in the copy feacture when the user does not have installed a copy
|
|
I will test them soon to check each tool individually. In the meantime, could you rebase onto main to solve the conflicts Github complains about. |
ba95ec8 to
3eb4508
Compare
There was a problem hiding this comment.
If no tests are introduced, we should document or list a series of commands to which VulcanAI should correctly answer.
For example:
- Run a publisher in background and input: "Subscribe to topic /chatter with max message 1".
Note that this could also be done in a test-suite running in a docker container with Vulcanexus and checking the blackboard to verify the output of each call.
Lastly, I am still missing providing a default ROS 2 Node if default tools are enabled and no ROS 2 node is given. Otherwise tools won't work
622e2d4 to
33b292a
Compare
Execution failed for 'ros_subscribe': '<=' not supported between instances of 'str' and 'int'
Done in 3d25f8d |
cferreiragonz
left a comment
There was a problem hiding this comment.
I added the missing default ROS 2 Node and moved it to the same file as the default tools. I also corrected a bug in the ROS2TopicTool and created tests for its main arguments. I still think we have to much variability by allowing optional input arguments which only work with specific commands (for example the topic_name is required for the info subcommand, but it is not required for the find subcommand), this makes the LLM success rate very low. I also improved some logs.
- We would need to extend the test suits with a specific test for each tool sub-command to avoid having failing tests with every update.
- Calling
/toolsreturns: Error: ColorParseError("'topic_name' is not a valid color") - It would be great to parse the message:
WARNING: topic \[/chatter] does not appear to be published yetshown when subscribing or publishing messages to avoid the trailing forward slash before the bracket. - I could not find any informative message stating how to stop a subscriber or publisher. Something like "press Ctrl+C to stop".
- We need to review how things are displayed because right now "ros2 topic list" does not show any result. The user needs to check the Output dictionary, which right now does not use a pretty format. Additionally, subscribing to any message with
max_linesargument automatically closes the new pop-up windows, so no output can be read.
[EXECUTOR] Invoking 'ros_subscribe' with args:'{'topic': '/chatter', 'max_lines': '15'}'
[TOOL ros_subscribe] Subprocess created!
[TOOL ros_subscribe] Returning only the last 10 lines in result['output'].
[EXECUTOR] Executed 'ros_subscribe' in 229.9 ms with result: //<-- This means I had 229.ms to visualize the data
[EXECUTOR] PlanNode PARALLEL succeeded on attempt 1/1```
| if max_duration is not None and not isinstance(max_duration, (int, float)): | ||
| max_duration = None | ||
| if max_duration is not None and max_duration <= 0: | ||
| max_duration = None |
There was a problem hiding this comment.
| if max_duration is not None and not isinstance(max_duration, (int, float)): | |
| max_duration = None | |
| if max_duration is not None and max_duration <= 0: | |
| max_duration = None | |
| if not isinstance(max_duration, (int, float)) or max_duration <= 0: | |
| max_duration = None |
1e6d1cc to
a4be233
Compare
I will answer the previous review comment here.I still think we have to much variability by allowing optional input arguments which only work with specific commands (for example the topic_name is required for the info subcommand, but it is not required for the find subcommand), this makes the LLM success rate very low.I have refactored the tools in multiple commands. And added a new optional variable in the tools called "group_name". This variable groups tools, making it look like subtools. When registering, editing the active tools or executing "/tools" it is displayed with group style. The tools have better description and tags that makes the LLM choose rapidly the tool to execute with the input of the user. The time execution of the queries have decreased significantly. Calling
|
Signed-off-by: danipiza <dpizarrogallego@gmail.com>
Signed-off-by: danipiza <dpizarrogallego@gmail.com>
Signed-off-by: danipiza <dpizarrogallego@gmail.com>
…aky test Signed-off-by: danipiza <dpizarrogallego@gmail.com>
Signed-off-by: danipiza <dpizarrogallego@gmail.com>
Signed-off-by: danipiza <dpizarrogallego@gmail.com>
Signed-off-by: danipiza <dpizarrogallego@gmail.com>
Signed-off-by: danipiza <dpizarrogallego@gmail.com>
Signed-off-by: danipiza <dpizarrogallego@gmail.com>
Signed-off-by: danipiza <dpizarrogallego@gmail.com>
Added keyboard support in modal screens.RadioList (suggestions)
CheckList (
|
Signed-off-by: danipiza <dpizarrogallego@gmail.com>
Signed-off-by: danipiza <dpizarrogallego@gmail.com>

Description
This branch adds a built-in ROS 2 default tool suite to VulcanAI and wires in the runtime, planner, validator, console, and CI changes needed to support it cleanly.
ROS 2 default tools
ros2_default_toolsentry point inpyproject.toml.ROS2DefaultToolNodeto support default tools when no main node is provided.Tool registry and tool metadata
ToolRegistrynow supports auto-loading default tools, with the ability to disable that behavior where needed.group_name.tool_descriptionfor user-facing tool listings andinput_defaultsfor documented/defaulted inputs.Planner, validator, and executor
string?,int?,float?, etc.input_defaultsin schema order.Console and streaming UX
/debugcommand and debug-only plan output logging./rerunhandling and validation.F3toF4.Shared tool utilities
src/vulcanai/tools/utils.py.CI and tests
test_default_tools.py.