Differential D15214

axiscontrol: Add spindle in text
Needs RevisionPublic
Actions

Authored by patrickelectric on Sep 2 2018, 2:59 AM.

Details

Reviewers
tcanabrava
rizzitello
Summary

Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>

grblplugin: Add G91, G90, G1 and M84 commands

  • G91: This command does not exist without parameters for grbl
  • G90: Does parameter does not exist also without parameters for grbl
  • G1: If in relative mode, use G91
  • M84: Does not exist, set spindle velocity to zero

Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>

Diff Detail

Repository
R232 AtCore
Branch
corrections
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2447
Build 2465: arc lint + arc unit
patrickelectric requested review of this revision.Sep 2 2018, 2:59 AM
patrickelectric created this revision.
patrickelectric added reviewers: tcanabrava, rizzitello.Sep 2 2018, 3:00 AM
patrickelectric added a project: Atelier: AtCore.

I DON'T WANT TO BREAK THE BINARY COMPATIBILITY BETWEEN THE PLUGINS !!!!
pls, review with love.

rizzitello requested changes to this revision.Sep 2 2018, 3:50 AM
rizzitello added inline comments.
src/plugins/grblplugin.cpp
70

what if this is a G91 or G90?

89

If not else If

Since grbl can have several commands on one line we want to check for each of our possible commands to be replaced so if we have command like "G91 G1 X5 M84 S0"

New Translation strings are missing test cases.

91

temp.replace("G91", "");

100

temp.replace( "M84", "M3 S0");

check also for M84 S<int>
a Regex like "M84( \\Sd+)?[^0-9]" perhaps?
that should grab M84 S<numbers> and not M840-M849

This revision now requires changes to proceed.Sep 2 2018, 3:50 AM
patrickelectric added inline comments.Sep 2 2018, 1:33 PM
src/plugins/grblplugin.cpp
100

M84 does not exist in grbl. Why do you want a check for M84 SX ? What is the meaning of that command ? I was not able to find it in the documentation.

M840-849 does not exist as well, Bit I'll change the check to "M84 "

rizzitello added inline comments.Sep 2 2018, 1:49 PM
src/plugins/grblplugin.cpp
100

On Marlin, Repetier and RepRapFirmware, M84 can also be used to configure or disable the idle timeout. For example, "M84 S10" will idle the stepper motors after 10 seconds of inactivity. "M84 S0" will disable idle timeout; steppers will remain powered up regardless of activity.

when we send disable motors we send a M84 S0 command.

patrickelectric added inline comments.Sep 2 2018, 1:51 PM
src/plugins/grblplugin.cpp
100

Got it.

rizzitello added inline comments.Sep 2 2018, 9:52 PM
src/plugins/grblplugin.cpp
74

for (const QString & string : QString temp )?

74

have you tested this with our cnc test file?

cncteste.gcode1 KBDownload

patrickelectric marked an inline comment as done.Sep 2 2018, 9:58 PM
patrickelectric added inline comments.
src/plugins/grblplugin.cpp
74

Can't be const, I'm editing it.
But it's possible to change to Qstring& cmd : temp. This is a WIP commit.
This does not work with file yets, only with manual control. So, I think that solves 1 issue of many.

rizzitello added inline comments.Sep 3 2018, 10:58 AM
src/plugins/grblplugin.cpp
74

Ok

rizzitello requested changes to this revision.Sep 3 2018, 10:58 AM

just to keep it off my review list until its updated.

This revision now requires changes to proceed.Sep 3 2018, 10:58 AM

Revision Contents
Changeset List

Diff 40875

View Options

src/plugins/grblplugin.h

Loading...
View Options

src/plugins/grblplugin.cpp

Loading...
View Options

src/widgets/axiscontrol.cpp

Loading...