Improvements to work with Marlin by hydrogen18 · Pull Request #366 · pcb2gcode/pcb2gcode

hydrogen18 · 2019-11-16T14:56:15Z

I've branched off of a version from Sep 25 as newer branches no longer build for me :(

A little context to this

I run Marlin on a Cartesian machine based off a 3018 type chassis
I use a drill motor
Marlin believes the motor is just a fan, so there is no toolhead contro. Just M106
Toolhead control doesn't make sense anyways. I have no forward/reverse,RPM control, etc.
Worth mentioning that on Marlin 1.x that G0 & G1 "share" the feed rate so changing it on one changes the other.

I did some experiments with pcb2gcode and just manually edited the files (to delete things like M4) and managed to mill & drill a single sided board. It works fine.

So here are my changes

Use g0-something-speed for movement only operations between actual mill & drill operations. This should save a huge amount of time on drill & milldrill operations as I can only do like 50 mm/minute for milldrill but movement between positions I can run 1000+ mm/minute.
Add option to omit toolhead control entirely. Toolhead control commands are still there by default. This is useful for my use case because there is no tool to control basically
Add sanity checks around the prior point. If you omit toolhead control, you have to be using a single tool.
Refactor some G0 commands that retract the tool to G0 + G1. On an X-Y-Z machine these are equivalent. For other machines it makes a difference as G0 can be interpreted as "just put the toolhead here now, I don't care how you get there". So basically you want to G0 back to the surface of the work then G1 to the safe position at full speed.
Consolidate some of the construction of the start/end operations
Convert some commands that were like F100 to G1 F100. I don't think commands like F100 are valid in the first place and the obvious intent was to setup the feed rate for the remaining commands.

Note that for #1 there were options for this already, but they didn't appear to be used except for planning.

Let me know what you think of these changes and if I need to do anything more to get these merged.

Allow disabling of tool control commands

eyal0

Good stuff!

Would you like to try running ./integration_tests.py and then ./integration_tests.py --fix --add? It runs a bunch of tests and creates new expected outputs. Those can useful show the coverage of the code and guide you in making new test cases.

If you don't want to do it, I can do it and update this PR.

The steps to do it are in the .travis.yaml file. You can also click on the TravisCI link and see the log of the continuous integration testing.

It may require installing a different version of boost or gerbv if it isn't working for you. An easier way might be to just download https://api.travis-ci.org/v3/job/612815074/log.txt and run it with patch, which will modify all the expected files as needed. And we can look at those and see if pcb2gcode is still doing the right thing.

Thanks again!

eyal0 · 2019-11-18T19:43:39Z

+
+                            // Pull the drillbit out of the workpiece
+                            of << "G1 Z0"  
+                               << " F" << driller->feed * cfactor << '\n';


As I understand, this code will cause the CNC to backout of the drilled hole first slowly and then quickly, right?

However, the G81 gcode always just backs out quickly. So maybe you can delete these two lines?

https://www.cnccookbook.com/g81-g73-g83-drill-peck-canned-cycle/

I believe you are correct, but I am not a machinist. My understanding is the more advanced machines have such commands. Everything I've worked with is all G0 / G1. I don't think this is inherently bad, but what it really comes down to is I have no "real" machines to test something like a G81 cycle with.

eyal0 · 2019-11-18T19:45:16Z

-        of << "G0 Z" << cutter->zsafe * cfactor << "\n\n";
+
+        // Pull mill out of workpiece
+        of << "G1 Z0 F" << cutter->vertfeed * cfactor << '\n';


Again, do we need this?

eyal0 · 2019-11-18T19:47:36Z

        // Start one step above Z0 for optimal entry
        of << "G1 Z" << -1.0/stepcount * cutter->zwork * cfactor
-           << " F" << cutter->vertfeed * cfactor << '\n';
+           << " F" << cutter->g0_horizontal_speed * cfactor << '\n';


This should be remain vertfeed, no? Why this change?

I noticed that the code is "G1" but the variable is called "g0".

I think this is moving down from zsafe to the top of the work piece. In my mind this motion can be as fast as possible since no cutting should occur. Maybe I'm wrong about this, I'll change it back. The time difference for most simple cartesian setups is only going to be milliseconds anyways

eyal0 · 2019-11-18T19:49:44Z

-        of << "G0 Z" << cutter->zsafe * cfactor << "\n\n";
+
+        // Pull mill out of the workpiece
+        of << "G1 Z0 F" << cutter->vertfeed * cfactor << '\n';


Again, can we delete these two lines?

eyal0 · 2019-11-18T19:51:35Z

+        gcode_end << "G53 ";      
+    }
+
+    gcode_end << "G0 Z" << target->zchange * cfactor;


Did we already compute zchange above? So we shouldn't need target->zchange * cfactor again, right?

You're correct, this can be eliminated now.

eyal0 · 2019-11-18T19:53:49Z

 /******************************************************************************/
 class Driller: public Mill
-{
+{ 


No need for this change, right?

If at all, I'd have put { } on the end of line 99.

eyal0 · 2019-11-18T19:54:02Z

  size_t path_finding_limit;
-  double g0_vertical_speed;
-  double g0_horizontal_speed;
+


Don't need the blank line here.

hydrogen18 · 2019-11-19T00:12:13Z

I will work on the CI & whitespace changes soon and update this PR again

coveralls · 2019-11-26T02:29:17Z

Pull Request Test Coverage Report for Build 1381

108 of 115 (93.91%) changed or added relevant lines in 4 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.05%) to 90.885%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
ngc_exporter.cpp	36	37	97.3%
options.cpp	5	11	45.45%

Totals
Change from base Build 1376:	-0.05%
Covered Lines:	3839
Relevant Lines:	4224

💛 - Coveralls

hydrogen18 added 3 commits November 13, 2019 19:37

Allow rapid moves between drill & milling

f844f11

Allow disabling of tool control commands

Set G1 speed only the same way in drill.cpp as in ngc_exporter.cpp

d6da2cf

Add some sanity checks around omitting toolhead control

3b4be59

hydrogen18 commented Nov 16, 2019

View reviewed changes

Comment thread main.cpp

eyal0 requested changes Nov 18, 2019

View reviewed changes

eyal0 added 2 commits November 24, 2019 21:53

Merge branch 'master' into edu

f1529e4

Integration tests

516c16a

eyal0 force-pushed the master branch 10 times, most recently from a8e62e7 to 6d15f72 Compare January 6, 2021 22:27

Conversation

hydrogen18 commented Nov 16, 2019

Uh oh!

Uh oh!

eyal0 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hydrogen18 commented Nov 19, 2019

Uh oh!

coveralls commented Nov 26, 2019

Pull Request Test Coverage Report for Build 1381

💛 - Coveralls

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

eyal0 left a comment •

edited

Loading