Differential D12771

GrblPlugin CNC Love
ClosedPublic
Actions

Authored by rizzitello on May 9 2018, 2:48 AM.

Details

Reviewers
patrickelectric
tcanabrava
laysrodrigues
Commits
R232:df426834581d: Check for multi line in grbl and ok returns
R232:cb3f207f7afb: GrblPlugin CNC Love
Summary

Check for multi line in grbl and ok returns

Grbl for cnc is a bit odd .. this helps with some of that.

Test Plan

Printing this totally normal grbl file

cncteste.gcode1 KBDownload

You will get an error from the () style comments D12772 addresses that.

Diff Detail

Repository
R232 AtCore
Branch
CNC
Lint
No Linters Available
Unit
No Unit Test Coverage
rizzitello requested review of this revision.May 9 2018, 2:48 AM
rizzitello created this revision.
rizzitello edited the summary of this revision. (Show Details)May 9 2018, 2:49 AM
rizzitello added reviewers: patrickelectric, tcanabrava, laysrodrigues.
rizzitello added a project: Atelier: AtCore.
rizzitello added a subscriber: Atelier: AtCore.
rizzitello edited the test plan for this revision. (Show Details)May 9 2018, 2:57 AM
rizzitello edited the test plan for this revision. (Show Details)
patrickelectric requested changes to this revision.May 9 2018, 3:10 AM
patrickelectric added inline comments.
src/plugins/grblplugin.cpp
51

Can you get without reference ?

52

Regex are not easy to read, can you explain what this will get and why ?

This revision now requires changes to proceed.May 9 2018, 3:10 AM
rizzitello updated this revision to Diff 33862.May 9 2018, 3:25 AM
rizzitello edited the summary of this revision. (Show Details)

patrick suggestions of comments

rizzitello marked 2 inline comments as done.May 9 2018, 3:26 AM
rizzitello added inline comments.
src/plugins/grblplugin.cpp
51

No its in the base class were overiding here

patrickelectric added inline comments.May 9 2018, 3:31 AM
src/plugins/grblplugin.cpp
52

This will send with arguments ?

rizzitello marked 2 inline comments as done.May 9 2018, 3:42 AM
rizzitello added inline comments.
src/plugins/grblplugin.cpp
52

No it does not. The Error case presented does not show commands with arguements ever being on the same line. Im not sure if that is a thing or not.

patrickelectric added inline comments.May 11 2018, 11:38 AM
src/plugins/grblplugin.cpp
52

We should hold this to make sure

rizzitello marked 3 inline comments as done.May 15 2018, 11:25 AM
patrickelectric added inline comments.May 16 2018, 12:31 AM
src/plugins/grblplugin.cpp
52

Do you know someone that can test this PR ?

tcanabrava accepted this revision.May 22 2018, 2:01 PM

After testing this is good to go.

src/plugins/grblplugin.cpp
53

statit const auto regEx = QRegul...

rizzitello marked an inline comment as done.May 22 2018, 2:19 PM
rizzitello updated this revision to Diff 35214.EditedMay 30 2018, 7:00 PM
  • Grab any command args when breaking up the commands
patrickelectric requested changes to this revision.May 30 2018, 7:04 PM
patrickelectric added inline comments.
src/plugins/grblplugin.cpp
53

Can you put a better explanation in this regex ?
It appears to do something in the end of the regex that is not described.

This revision now requires changes to proceed.May 30 2018, 7:04 PM
rizzitello updated this revision to Diff 35215.May 30 2018, 7:05 PM
  • add comment for new regex
rizzitello updated this revision to Diff 35218.May 30 2018, 7:32 PM
  • Remove "T" commands
patrickelectric accepted this revision.May 30 2018, 7:38 PM

Everything appears to be fine now, but we need to test before pushing. Please ping me in the weekend to do it.

This revision is now accepted and ready to land.May 30 2018, 7:38 PM
rizzitello closed this revision.May 30 2018, 7:39 PM

Revision Contents
Changeset List

PathPackages
Msrc/plugins/grblplugin.h (7 lines)
Msrc/plugins/grblplugin.cpp (34 lines)

Diff 35218

View Options

src/plugins/grblplugin.h

Loading...
View Options

src/plugins/grblplugin.cpp

Loading...