> 未分类 > [PR#905] feat: support full manual control over the tool change process

[PR#905] feat: support full manual control over the tool change process

未分类 bolang 6个月前 (10-14) 51次浏览

Issue #905 | 状态: 已关闭 | 作者: cheton | 创建时间: 2025-02-10

标签: Review effort [1-5]: 3

PR Type

Enhancement, Bug fix

_

Description

– Introduced TOOLCHANGEPOLICYMANUALTOOLCHANGECUSTOM_PROBING for enhanced tool change handling.

– Renamed toolProbeOverrides to toolProbeCustomCommands for better clarity.

– Added functionality to pause operations when TOOLCHANGEPOLICYIGNOREM6_COMMANDS is active.

– Updated UI and backend logic to reflect the new tool change policy and command naming.

_

Changes walkthrough 📝

Relevant files
Enhancement
9 files

constants.js

Updated tool change policy constants.                                       
+1/-1     
api.tool.js

Renamed toolProbeOverrides to toolProbeCustomCommands.
 +1/-1     
GrblController.js

Added support for pausing operations and updated tool change logic.
 +18/-7   
MarlinController.js

Enhanced tool change handling and added pause functionality.
 +18/-7   
SmoothieController.js

Updated tool change logic and added operation pause support.
 +18/-7   
TinyGController.js

Improved tool change handling and added pause functionality.
 +18/-7   
constants.js

Updated tool change policy constants.                                       
+1/-1     
Tool.jsx

Updated UI for custom probing tool change policy.               
+33/-33 
index.jsx

Refactored tool configuration handling for new policy.     
+8/-8     

_

>

Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.

    • 评论 (3)

    #1 – codesandbox[bot] 于 2025-02-10

    #### Review or Edit in CodeSandbox

    Open the branch in source=ghapp”>VS Code • source=ghapp”>Preview

    #2 – codiumai-pr-agent-free[bot] 于 2025-02-10

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Handling

    The new pause functionality added for TOOLCHANGEPOLICYIGNOREM6_COMMANDS should include error handling for edge cases where messageSlot.take() returns null or workflow.pause() fails.

    State Management

    The component’s state management for toolProbeCustomCommands could be improved by using a single source of truth instead of maintaining both state and props versions.

    #3 – codiumai-pr-agent-free[bot] 于 2025-02-10

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    Category Suggestion                                                                                                                                    Impact
    Possible issue

    Validate custom probe commands

    _

    Add validation for custom probe commands to prevent empty or malformed commands
    from being executed during tool change operations.

    [src/app/widgets/Tool/Tool.jsx [574-578]](https://github.com/cncjs/cncjs/pull/905/files#diff-a401a7973526e77e8931e59448c9404d71669a3b2e31a615ef726bce6f023aecR574-R578)

    -{!this.state.isToolProbeCustomCommandsEditable && ensureString(toolProbeCustomCommands).length === 0 && (
+{!this.state.isToolProbeCustomCommandsEditable && (!toolProbeCustomCommands || !toolProbeCustomCommands.trim() || toolProbeCustomCommands.split('\n').some(cmd => !cmd.trim())) && (
   
    
-    {i18n._('Warning: No custom tool probe commands are defined')}
+    {i18n._('Warning: No custom tool probe commands are defined or contains empty commands')}
   
    
 )}

    – [ ] Apply this suggestion

    Suggestion importance[1-10]: 8

    __

    Why: The suggestion significantly improves input validation by checking for empty lines and whitespace-only commands, preventing potential execution issues with invalid tool probe commands.

    		 Medium
    General

    Add fallback for undefined messages

    _

    Consider adding error handling for cases where this.messageSlot.take() returns
    undefined and originalLine is also undefined, to prevent potential runtime
    errors.

    [src/server/controllers/Marlin/MarlinController.js [477-480]](https://github.com/cncjs/cncjs/pull/905/files#diff-fbe0adc8dc9c94442f2b6132545200cdeb22e08a021ff2985fd2d6a339d2bb2bR477-R480)

     this.feeder.hold({
   data: 'M6',
-  msg: this.messageSlot.take() ?? originalLine,
+  msg: this.messageSlot.take() ?? originalLine ?? 'Tool change required',
 });

    – [ ] Apply this suggestion

    Suggestion importance[1-10]: 7

    __

    Why: The suggestion adds a useful fallback message when both messageSlot.take() and originalLine are undefined, preventing potential runtime issues and improving user feedback during tool changes.

    		 Medium

    原始Issue: https://github.com/cncjs/cncjs/pull/905

    喜欢 (0)