Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Grbl version check, timing fix, jogging improvement, bug fixes #24

Closed
wants to merge 11 commits into from

Conversation

jcl5m1
Copy link

@jcl5m1 jcl5m1 commented Jul 5, 2020

  • added check for grbl version/display to correctly set status report mask, rather than hard code it for 1.1. a lot of stuff from asia comes with grbl 0.9 out of the box

  • added small delay of 50ms to command message sending. using a Raspi 3 B+ or faster computer seems to spam the Arduino too much causing GCode errors. 10ms was not enough.

  • allow sending jog commands even while the robot is in Run status so you don't have to wait for it to go Idle again before moving again

  • jog control, and origin tool stay in workspace coordinates only rather than mess with G28 machine coordinates commands and modifying the EEPROM

  • estop/reset is always available

  • removed G01 prefixing which broke modal commands like G2/G3

@jcl5m1 jcl5m1 changed the title Grbl version check, timing fix, and jogging improvement Grbl version check, timing fix, jogging improvement, bug fixes Jul 11, 2020
@synman
Copy link
Owner

synman commented Aug 22, 2020

sorry for the huge delay on looking into this PR. I'll see about addressing it in the coming days/weeks. I need to circle back for python3 support / etc as it is.

@@ -479,15 +489,7 @@ def hook_gcode_sending(self, comm_instance, phase, cmd, cmd_type, gcode, *args,
# relative positioning
self.positioning = 1

#T2 # HACK:
Copy link
Owner

@synman synman Jan 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was this hack removed? This was put in place to address a gcode parsing issue specific to gcode generated using T2 Laser. Reverting it may cause harm to clients who use T2.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See issue #7 for further reference

return

if command == "origin":
# do origin stuff
self.ignoreErrors = True
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure why this was removed.... TBT, I'm not even sure why I put it in there to begin with

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I made this verbose like this because of variations across engravers. I really do like the streamlined model but need to validate we don't break other engravers

@synman
Copy link
Owner

synman commented Jan 20, 2021

two questions inline via code review


if subscribed_events.find(event) == -1:
return

if event == Events.CONNECTED:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall, I think I'm good with keeping track of the grbl version and doing version specific logic based on it

@@ -544,6 +546,7 @@ def hook_gcode_sending(self, comm_instance, phase, cmd, cmd_type, gcode, *args,
if currentTime > self.timeRef + 500:
# self._logger.info("x={} y={} z={} f={} s={}".format(self.grblX, self.grblY, self.grblZ, self.grblSpeed, self.grblPowerLevel))
self._plugin_manager.send_plugin_message(self._identifier, dict(type="grbl_state",
grblVersion=self.grblVersion,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't have to add the grbl version to the active state push loop. we should be storing the grbl version as a plugin setting during the connection phase and simply reading that setting value when needed, whether it is part of a knockout model or w/e.

@@ -641,6 +649,7 @@ def hook_gcode_received(self, comm_instance, line, *args, **kwargs):

if self.grblState == "Sleep" or self.grblState == "Run":
self._plugin_manager.send_plugin_message(self._identifier, dict(type="grbl_state",
grblVersion=self.grblVersion,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as prior

@@ -657,6 +666,7 @@ def hook_gcode_received(self, comm_instance, line, *args, **kwargs):
self.grblPowerLevel = round(float(match.groups(1)[1]))

self._plugin_manager.send_plugin_message(self._identifier, dict(type="grbl_state",
grblVersion=self.grblVersion,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as prior

@@ -879,59 +890,30 @@ def on_api_command(self, command, data):
self._logger.debug("move {} {}".format(direction, distance))

if direction == "home":
self._printer.commands("G91")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to make sure this doesn't break other engravers

@synman synman closed this Dec 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants