OpenBuilds / OpenBuilds-CAM

Online CNC CAM System
https://cam.openbuilds.com
GNU Affero General Public License v3.0
257 stars 623 forks source link

Todo: Optimise G-Code Generator: Don't repeat F/S commands unless changed, also remove modal commands that repeated) #9

Closed swarfer closed 4 years ago

swarfer commented 6 years ago

Hi Peter I was looking at the generated Gcode and noticed 2 things: [code] ; GCODE Generated by cam.openbuilds.com G21 ; mm-mode G21 G90 G17 F1000 M3 ; Operation 0: CNC: Vector (path inside) G0 Z10 <---------- G0 F1000 X94.4630 Y103.6576

G0 Z0 G1 F300 Z-3.0000 G1 F1000 X97.6498 Y103.3799 Z-3.0000 S100 G1 F1000 X99.2483 Y103.4581 Z-3.0000 S100 G1 F1000 X99.2483 Y103.4581 Z-3.0000 S100 G1 F1000 X100.7043 Y103.6917 Z-3.0000 S100 G1 F1000 X101.9992 Y104.0741 Z-3.0000 S100 [/code] that first G0 Z10
it not safe. I had my cutter positioned away from the cut area and while Z10 is safe within the cut area outside of that may have clamps or a vice etc. so the initial 'go to start position should be at safe height rather than clearance height. code like this this would be much safer for initial positioning: [code] G53 G0 Z0 ; safe height G0 F1000 X94.4630 Y103.6576 ; go to start; G0 Z10 ; lower to clearance height [/code]

The other thing is the repetition of F1000 and S100 (and Z-3), and even the G1's, on every line. All those codes only need be output when they change since they are modal. Having such long lines can mean that GRBL cannot plan ahead efficiently because only a few actual lines fit in the buffer. (but Marlin will need a G1 or G0 on every line)

The S100 should be output when M3 is output as well, since some controllers will error or default to S0 so even though the spindle is enabled it will not be up to speed before the first cut starts.

This is what we can aim for [code] M3 S100 ; turn spindle on and set speed G0 Z0 ; rapid down to surface - not the best idea but it does work ok for softer materials G1 F300 Z-3.0000 X97.6498 Y103.3799 F1000 X99.2483 Y103.4581
X99.2483 Y103.4581
X100.7043 Y103.6911
X101.9992 Y104.0741
G1 Z0 G0 Z10 [/code]

PS: (I realize that laser engraving will need the S words more often, and possibly F as well, but ordinary routing definitely does not.) also, the buffer size and timing issues apply to GRBL on an Arduino, on the upcoming ARM version it may not matter at all.

I don't speak Java but I might have a look at doing some of this.... maybe...

petervanderwalt commented 6 years ago

Hey man, thanks will check it out

Just a note about the repeated g-code words - thats a legacy from the old Laserweb code days (where a lot of this come from - my past lol) - there's two reasons why, one was some firmware bug at the time but I think thats been fixed (can't even recall the details - although your mention of Marlin might be what I recall... ) - the other is I need some time to rewrite the gcode viewer (lines without a G-word prefix doesnt parse). I started on it last week (moving it to webworker, but theres a couple todo's before I can call that work done. Just got it in so it works basically almost good enough (: but still lots todo - modal values is a part of that work. The old parser does (but its not a webworker)

https://github.com/OpenBuilds/SW-Machine-Drivers/issues/3 relates as well (no doubt you already saw the previewer doesnt render G2/3 yet)

petervanderwalt commented 6 years ago

Going to close out this issue for now. I'll revisit it once the Gcode viewer work is done (Gimme a week (: or so )

petervanderwalt commented 6 years ago

Okay, the Gcode viewer work is done: https://github.com/OpenBuilds/SW-Machine-Drivers/releases/tag/v1.0.89 or newer has the parser fixed to understand gcode lines without a G-word So re-opening this issue, will work on optimising the gcode generator tomorrow or next week

swarfer commented 6 years ago

awesome! Sketchucam does this by storing all co-ordinates and the last G command, and F value in class variables. Then code like if currentX != previousX gcode += wordformat('X',currentX) decides whether or not to include that word in the output, obviously repeated for each axis and F and the G command. don't forget to force the G command when needed!

the one massive problem I had with this was Sketchup's override of the = operator that forces all comparisons to be at the resolution of 0.001" which meant that some things that really should have been not equal would resolve to equal and prevent the output of needed code. I doubt you will have this issue in js but equality in floating point math often requires some extra code so just a heads up.

petervanderwalt commented 6 years ago

Yip we do the same (store and compare) (: - in JS we also sometimes have that issue, but less common - more decimals to work with (;
Just got to find some time to work on it - theres a long todo list for the gcode generator (tsp problem, plunge, optimise output, more postprocessor style of output to support more firmwares, etc thats been on the todo since the laserweb3 days!

swarfer commented 6 years ago

and I can add to that list: I need to engrave a line drawing for my daughter with a V bit. but I can select an outside cut or an inside cut under router, but no 'just follow this line' like under laser. but selecting laser does not give me Z height control to get a Z depth.

petervanderwalt commented 6 years ago

I have a workaround for now, you can use a 0mm tool Diameter (:

On Sat, Sep 8, 2018, 14:23 David the Swarfer notifications@github.com wrote:

and I can add to that list: I need to engrave a line drawing for my daughter with a V bit. but I can select an outside cut or an inside cut under router, but no 'just follow this line' like under laser. but selecting laser does not give me Z height control to get a Z depth.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/OpenBuilds/cam/issues/9#issuecomment-419638288, or mute the thread https://github.com/notifications/unsubscribe-auth/Ad_gt8Q1tf_14h4_cSCeFoD3Zb1WNc0Vks5uY7aogaJpZM4Wa2-d .

petervanderwalt commented 5 years ago

No-offset cut: added https://github.com/OpenBuilds/cam/commit/f6c4ae1aa3cf07f10c8068e28bb17ba9ffcdaf60 Operation is called "CNC Vector (No Offset)"

swarfer commented 4 years ago

hi, me again.. looks like the fix is relatively simple: num.toFixed(4).replace(/0+$/,''); // replace trailing 0's with nothing

by adding the .replace() call to all the string conversions when generating Gcode we should end up without any trailing zeros BUT never do this for RPM or feedrate , only XYZ

swarfer commented 4 years ago

yes, 5.0000 will end up as 5. and GRBL does not care. in fact on some controllers the . is required on the end of all numbers (Fanuc is one IIRC)

another way would be to extend the number object to have a method that trims all trailing 0's and then adds one back on if you end up with a trailing .

petervanderwalt commented 4 years ago

Good start, but I really want to redo the whole generator - theres too many "if "this feature added later" kinda hacks in there (;

swarfer commented 4 years ago

that is a common problem with software that grows. a rewrite is great but you still have to deal with the options OR you have to have copies of the 'same' code (the basic gcode output) but one for laser, one for router, one for router with ramping and so on. Then it becomes much easier to forget to propagate changes from one to the rest. SketchUcam has 2 such routines, one for plain milling and one for ramping because when I started adding the ramping to the already 'if' rich routine I quickly realized that it would become just too complicated so copied and pasted. Now the one without ramping also does the laser output. to have all 3 sets of options in one routine is just too much I think and there is no way to simplify it with function calls, it just gets longer and longer.

'they' say that all software should be written 3 times and only the 3rd version gets released. reality (certainly my reality) is just not like that, there is no money for the time.