Conversation
A bare number like '6000' in a macro, hook, or dialect command field is not a valid G-code command and gets sent to the machine as-is, producing errors like in #393. Show a non-blocking warning (banner and row icon) in the macro editor and dialect editor dialogs when a field contains only a numeric literal, suggesting a prefixed command instead. Fixes #393
Pass a real parent window to the dialogs and avoid direct attribute access on PreferencesRow for dynamically attached attributes.
knipknap
changed the title
Enabling 'Continuous laser mode' on a dialect whose movement
templates have no {s_command} placeholder makes the exporter
compute laser power for every segment and then silently drop
it: continuous mode also suppresses the separate laser_on power
updates, so the machine never sees any power at all (seen in
#403: 'M4 S0' with no S value anywhere).
Show a non-blocking warning in the dialect editor for that
combination: the affected movement template rows and the toggle
itself get a warning icon, and the banner names the first
offending template. The warning updates live when templates are
edited or the toggle flips, and never blocks saving.
Ref #403
widget_map values are typed as Adw.PreferencesRow, so pyright (rightly) rejects the set_text call without an isinstance check, mirroring the other tests.
The check for movement templates that would silently drop laser power in continuous laser mode is domain logic, not UI logic: move POWER_MOVE_TEMPLATE_KEYS, has_s_command() and the new find_missing_s_command_templates() onto GcodeDialect. The dialog now applies the current editor values to a throwaway copy and delegates the detection to the model, keeping only the warning wording in the UI layer.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #393
A bare number like
6000in a macro, hook, or dialect command field is not a valid G-code command. It is passed through verbatim and sent to the machine as-is, producing the error reported in #393.This PR shows an unobtrusive, non-blocking warning in the dialogs whenever a G-code field contains only a numeric literal (e.g.
6000,-296.429,.5):GcodeEditorDialog, used for user macros and hooks): anAdw.Bannerunder the header bar names the offending line and suggests a prefixed command (e.g.M4 S6000). It updates live as the text changes.DialectEditorDialog): the same banner for script fields (preamble/postscript) and command template rows, plus a warning icon with tooltip on the affected row, mirroring the existing error-icon pattern.The warning never disables the Save button, so it stays purely advisory. Comment lines (
;6000) and lines with inline values (G1 X10 F6000) do not trigger it.Continuous laser mode validation (ref. #403)
Enabling "Continuous laser mode" on a dialect whose movement templates have no
{s_command}placeholder makes the exporter compute laser power for every segment and then silently drop it — continuous mode also suppresses the separate laser-on power updates, so the machine never sees any power at all (M4 S0with noSvalue anywhere).The dialect editor now warns for that combination, using the same non-blocking pattern:
travel_move,linear_move,arc_cw,arc_ccw,bezier_cubic) get a warning icon when they lack{s_command}(empty templates are ignored).Test plan
tests/ui_gtk/machine/test_number_only_warning.pycovers the pure detection helpers and both dialogs (warning shown/cleared, save stays enabled, errors still block save), plus the continuous-laser-mode warning (shown when templates lack{s_command}, cleared by adding the placeholder or toggling the mode off, quiet for the built-in Grbl (Raster) dialect).pixi run uitest: machine + varset suites pass.pixi run lintequivalents (ruff format/check, flake8, pyflakes) pass; pyright reports no new errors vs. baseline.