LaserWeb / LaserWeb4

Collaborative effort on the next version of LaserWeb / CNCWeb
GNU Affero General Public License v3.0
702 stars 190 forks source link

Rogue G0 from rasterizer affecting G0 speeds #224

Closed ghost closed 7 years ago

ghost commented 7 years ago

Extracted from a G+ discussion

There is a bug in LW4 which we have not been able to repeat consistently where the G0 feedrate often overrides our firmware settings (smoothie).

From @tbfleming I just repoed it. lw.raster-to-gcode is inserting a rogue "G0 F1500". Please file a bug.

tbfleming commented 7 years ago

I'm going to insert a simple post-processing step which strips this out and the lone S value. The "G0 F1500" throws off vector ops when running on Smoothie.

tbfleming commented 7 years ago

It now strips "G0 F..." and lone S values from lw.raster-to-gcode output.

jorgerobles commented 7 years ago

Found. That's a parameter on https://github.com/lautr3k/lw.raster-to-gcode rapidRate @lautr3k any way to prevent its use? anyway, G0 could be exposed as a machine setting or raster setting.

tbfleming commented 7 years ago

Traditionally G0 feed is a setting that gcode can't touch. Reprap people added G0 F because reprap is weird. Smoothie added it because of Reprap. We don't need it and it messes up vector ops which follow.

tbfleming commented 7 years ago

If we add it as a setting then we'll confuse users with grbl, which does the right thing by ignoring it.

jorgerobles commented 7 years ago

Ok, but when RepRap comes, this issue will arise for sure...

cprezzi commented 7 years ago

It's a feature, not a bug ;) Can't we just ignore the F value, if rapidRate is set to 0, and for the moment fix rapidRate to 0?

cprezzi commented 7 years ago

Later on, when/if we integrate 3dp, we can expose this feature to the user.

cprezzi commented 7 years ago

@tbfleming Isn't adding a postprocessor for fixing something that we could also fix on generation not a bit simplistic?

tbfleming commented 7 years ago

We can't fix it on generation :(

tbfleming commented 7 years ago

Unless we fork it and not use the npm package.

cprezzi commented 7 years ago

If @lautr3k doesn't want to move this repo into the Laserweb organisation (or at least give us push rights on his repo), then we realy should fork it, because we depend on it.

cprezzi commented 7 years ago

I found we already have a fork in the organisation (21 days old).

jorgerobles commented 7 years ago

Ehm.. I did a fork and contributed some code to his repo. Simple as that.

cprezzi commented 7 years ago

@jorgerobles Is your fork the one in the laserweb organisation? I would prefer to use that.

tbfleming commented 7 years ago

He was opposed to solving the lone S problem and claimed it was a Smoothie problem. We need to post process that out because it messes up both Smoothie and grbl-lpc.

jorgerobles commented 7 years ago

@cprezzi will do.

jorgerobles commented 7 years ago

https://github.com/LaserWeb/lw.raster-to-gcode/commit/6d82cd73a718689390cb9a1549cbfb325196a074

https://github.com/lautr3k/lw.raster-to-gcode/pull/3

jorgerobles commented 7 years ago

I've tested PR locally and works ok, but have problems building and using our git instead npm. Awating for now.

jorgerobles commented 7 years ago

@tbfleming Postprocessor is a nice idea commented before. How can we make that approach generic so make postprocessors contributed by community to their needs?

tbfleming commented 7 years ago

I haven't put any thought into that. There are potential security issues with supporting user-contributed code, especially under Electron since it provides direct file system and OS access.

jorgerobles commented 7 years ago

Yes. We could always let them contribute via PR and test before compile.

ghost commented 7 years ago

If @lautr3k doesn't want to move this repo into the Laserweb organisation (or at least give us push rights on his repo), then we realy should fork it, because we depend on it.

@cprezzi You have to do as @jorgerobles just did for the https://github.com/lautr3k/lw.raster-to-gcode/pull/4 PR :

  1. Make a new branch on the LaserWeb repo from lautr3k/lw.raster-to-gcode/ master branch
  2. Name it like set-rapidRate-optional, or something that describes the new feature/fix.
  3. Make your changes (without reformating the source) and following the code style.
  4. Make your PR without the build files, only the modified sources files.

@tbfleming I will fix the lone S value.

tbfleming commented 7 years ago

@lautr3k great!

ghost commented 7 years ago

@jorgerobles @tbfleming lw.raster-to-gcode@0.6.3 is out :

jorgerobles commented 7 years ago

@DarklyLabs please test to close :)

ghost commented 7 years ago

Experiencing the same problem with vectors. Seems to occur only after a job is aborted before completion. Once again not repeatable at the moment. I have asked the team to export/examine the gcode next time it occurs.

cprezzi commented 7 years ago

@DarklyLabs Please especially test for raster. Vector is completely different code.

ghost commented 7 years ago

Since the G0 is being stripped, we have not experienced this slowdown with raster engravings.

jorgerobles commented 7 years ago

@domenic-d that recent issue you told me is related to this one.

I've postprocessed gcode so the first move is prepended to Z move. Also, that fixed an open issue at https://github.com/lautr3k/lw.raster-to-gcode/issues/6

ghost commented 7 years ago

Looks like @jorgerobles changes have resolved this. Thx! Closing for now.