[PR#895] feat: add support for M6 tool change policy

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

Issue #895 | 状态: 已关闭 | 作者: cheton | 创建时间: 2024-11-13

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


Description

– Introduced new constants for tool change actions: pause, passthrough, and ignore.
– Implemented tool change action handling for M6 command across multiple controllers (Grbl, Marlin, Smoothie, TinyG).
– Added configuration options to determine the tool change action in controllers.
– Enhanced the settings UI to allow users to select a tool change action.
– Updated settings state management to include tool change action.

_

Changes walkthrough 📝

Relevant files
Enhancement
constants.js

Define constants for tool change actions                                 

src/app/containers/Settings/Controller/constants.js

  • Added constants for tool change actions: pause, passthrough, and
    ignore.
  • +3/-0     
    GrblController.js

    Add tool change action handling in GrblController               

    src/server/controllers/Grbl/GrblController.js

  • Implemented tool change action handling for M6 command.
  • Added configuration to determine tool change action.
  • +30/-13 
    MarlinController.js

    Add tool change action handling in MarlinController           

    src/server/controllers/Marlin/MarlinController.js

  • Implemented tool change action handling for M6 command.
  • Added configuration to determine tool change action.
  • +25/-4   
    SmoothieController.js

    Add tool change action handling in SmoothieController       

    src/server/controllers/Smoothie/SmoothieController.js

  • Implemented tool change action handling for M6 command.
  • Added configuration to determine tool change action.
  • +25/-4   
    TinyGController.js

    Add tool change action handling in TinyGController             

    src/server/controllers/TinyG/TinyGController.js

  • Implemented tool change action handling for M6 command.
  • Added configuration to determine tool change action.
  • +25/-4   
    constants.js

    Define constants for tool change actions                                 

    src/server/controllers/constants.js

  • Added constants for tool change actions: pause, passthrough, and
    ignore.
  • +5/-0     
    Controller.jsx

    Add UI and handlers for tool change action selection         

    src/app/containers/Settings/Controller/Controller.jsx

  • Added UI for selecting tool change action.
  • Implemented handlers for tool change action selection.
  • +105/-24
    Settings.jsx

    Integrate tool change action into settings management       

    src/app/containers/Settings/Settings.jsx

  • Integrated tool change action into settings state management.
  • Added methods to update tool change action.
  • +22/-5   

    _

    > 💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information


    评论 (4)

    #1 – codesandbox[bot] 于 2024-11-13

    #### Review or Edit in CodeSandbox

    Open the branch in source=ghapp”>VS Codesource=ghapp”>Preview


    #2 – codiumai-pr-agent-free[bot] 于 2024-11-13

    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

    Code Duplication
    The M6 tool change handling logic is duplicated across multiple controller files (Grbl, Marlin, Smoothie, TinyG). Consider extracting this into a shared utility function to improve maintainability.

    State Management
    The toolChangeAction state is initialized in multiple places with different default values. Consider centralizing the default value to avoid inconsistencies.

    Error Handling
    The tool change action radio buttons don’t have error handling for invalid states or network failures when saving settings.


    #3 – codiumai-pr-agent-free[bot] 于 2024-11-13

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    Category Suggestion                                                                                                                                    Score
    Maintainability

    Extract duplicated code into a reusable method to improve maintainability

    _

    Extract the M6 tool change handling logic into a separate method to avoid code
    duplication and improve maintainability, since the same logic is repeated in
    multiple places.

    [src/server/controllers/Grbl/GrblController.js [241-251]](https://github.com/cncjs/cncjs/pull/895/files#diff-4dacbcae20bbf70f0461da55cad9e5a4315835c0150c2f99d6f99bf626afa28aR241-R251)

    -const toolChangeAction = config.get('state.controller.toolChange.action', TOOLCHANGEACTION_PAUSE);
    -if (toolChangeAction === TOOLCHANGEACTION_PASSTHROUGH) {
    -  // Passthrough
    -} else if (toolChangeAction === TOOLCHANGEACTION_IGNORE) {
    -  // Ignore
    -  line = line.replace('M6', '(M6)');
    -} else {
    -  // Pause (default)
    -  this.feeder.hold({ data: 'M6', msg: originalLine }); // Hold reason
    -  line = line.replace('M6', '(M6)');
    -}
    +line = this.handleM6ToolChange(line, originalLine);
    

    – [ ] Apply this suggestion

    Suggestion importance[1-10]: 8

    Why: The suggestion addresses significant code duplication across multiple controller files. Extracting the M6 tool change logic into a shared method would improve maintainability and reduce the risk of inconsistencies.

    8

    Refactor repetitive UI elements into a data-driven approach

    _

    Move the radio button options into a configuration array to make the code more
    maintainable and reduce repetition.

    [src/app/containers/Settings/Controller/Controller.jsx [108-123]](https://github.com/cncjs/cncjs/pull/895/files#diff-15fb584cf92020f1965b9467b595343da93f0275512e8f42e10a4b7e54c9f850R108-R123)

    -
    - - - - {i18n._('Temporarily pause the operation to allow for a manual tool change.')} - - -
    +{TOOLCHANGEOPTIONS.map(option => ( +
    + + + + {i18n._(option.description)} + + +
    +))}

    – [ ] Apply this suggestion

    Suggestion importance[1-10]: 7

    Why: The suggestion significantly improves code maintainability by reducing repetition in the UI code and making it more data-driven. This would make future modifications easier and reduce the chance of errors.

    7

    Possible issue

    Add defensive programming to handle potential configuration access errors

    _

    Add error handling around the config.get() call in case the configuration is not
    properly initialized or accessible.

    [src/server/controllers/Grbl/GrblController.js [241]](https://github.com/cncjs/cncjs/pull/895/files#diff-4dacbcae20bbf70f0461da55cad9e5a4315835c0150c2f99d6f99bf626afa28aR241-R241)

    -const toolChangeAction = config.get('state.controller.toolChange.action', TOOLCHANGEACTION_PAUSE);
    +const toolChangeAction = config?.get('state.controller.toolChange.action', TOOLCHANGEACTIONPAUSE) || TOOLCHANGEACTIONPAUSE;
    

    – [ ] Apply this suggestion

    Suggestion importance[1-10]: 5

    Why: The suggestion adds error handling for configuration access, which could prevent runtime errors if the config object is not properly initialized. While valid, the impact is moderate as the default value already provides some protection.

    5

    >💡 Need additional feedback ? start a PR chat


    #4 – codiumai-pr-agent-free[bot] 于 2025-01-01

    CI Failure Feedback 🧐

    #### (Checks updated until commit https://github.com/cncjs/cncjs/commit/aba6c9958fcac6e5fa866226efdfec383f93349d)

    Action: build-linux (18.x)

    Failed stage: Checkout code [❌]

    Failed test name: “”

    Failure summary:

    The action failed because Git was unable to fetch the PR’s commit. Specifically:

  • Git failed to fetch commit 1f62d2e38fbd3cc06029bd9a0064ebe1bfa7046d
  • The error “not our ref” indicates that the commit is not available in the remote repository
  • The action retried 3 times with waiting periods but consistently failed
  • This typically happens when:
    – The PR branch was force-pushed/rebased
    – The commit was deleted
    or garbage collected
    – The PR was closed/merged before the action completed

  • Relevant error logs:

    1:  ##[group]Operating System
    2:  Ubuntu
    ...

    84: [command]/usr/bin/git config --local --name-only --get-regexp core\.sshCommand 85: [command]/usr/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :" 86: [command]/usr/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader 87: [command]/usr/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'http\.https\:\/\/github\.com\/\.extraheader' && git config --local --unset-all 'http.https://github.com/.extraheader' || :" 88: [command]/usr/bin/git config --local http.https://github.com/.extraheader AUTHORIZATION: basic * 89: ##[endgroup] 90: ##[group]Fetching the repository 91: [command]/usr/bin/git -c protocol.version=2 fetch --no-tags --prune --progress --no-recurse-submodules --depth=1 origin +1f62d2e38fbd3cc06029bd9a0064ebe1bfa7046d:refs/remotes/pull/895/merge 92: ##[error]fatal: remote error: upload-pack: not our ref 1f62d2e38fbd3cc06029bd9a0064ebe1bfa7046d 93: The process '/usr/bin/git' failed with exit code 128 94: Waiting 13 seconds before trying again 95: [command]/usr/bin/git -c protocol.version=2 fetch --no-tags --prune --progress --no-recurse-submodules --depth=1 origin +1f62d2e38fbd3cc06029bd9a0064ebe1bfa7046d:refs/remotes/pull/895/merge 96: ##[error]fatal: remote error: upload-pack: not our ref 1f62d2e38fbd3cc06029bd9a0064ebe1bfa7046d 97: The process '/usr/bin/git' failed with exit code 128 98: Waiting 10 seconds before trying again 99: [command]/usr/bin/git -c protocol.version=2 fetch --no-tags --prune --progress --no-recurse-submodules --depth=1 origin +1f62d2e38fbd3cc06029bd9a0064ebe1bfa7046d:refs/remotes/pull/895/merge 100: ##[error]fatal: remote error: upload-pack: not our ref 1f62d2e38fbd3cc06029bd9a0064ebe1bfa7046d 101: ##[error]The process '/usr/bin/git' failed with exit code 128


    ✨ CI feedback usage guide:

    The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
    The tool analyzes the failed checks and provides several feedbacks:
    – Failed stage
    – Failed test name
    – Failure summary
    – Relevant error logs

    In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:

    /checks "https://github.com/{reponame}/actions/runs/{runnumber}/job/{job_number}"
    

    where {reponame} is the name of the repository, {runnumber} is the run number of the failed check, and {job_number} is the job number of the failed check.

    #### Configuration options
    enableautochecks_feedback – if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
    excludedcheckslist – a list of checks to exclude from the feedback, for example: [“check1”, “check2”]. Default is an empty list.
    enablehelptext – if set to true, the tool will provide a help message with the feedback. Default is true.
    persistent_comment – if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
    finalupdatemessage – if persistent_comment is true and updating a previous checks message, the tool will also create a new message: “Persistent checks updated to latest commit”. Default is true.

    See more information about the checks tool in the docs.


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

    喜欢 (0)