Added support for gcode states and work coordinates #1078

breiler · 2018-07-11T14:02:20Z

Added support for handling gcode states from the responses of the controller. It's also now possible to set and handle work coordinates.

Further improvements for #1071

A small demo can be found here:
https://www.youtube.com/watch?v=_kmXCvtE1Pc

…ort for changing work coordinates.

…ender into feature/tinyg Conflicts: ugs-core/src/com/willwinder/universalgcodesender/TinyGController.java

codecov-io · 2018-07-11T14:08:44Z

Codecov Report

Merging #1078 into master will increase coverage by 0.5%.
The diff coverage is 73.52%.

@@             Coverage Diff             @@
##             master    #1078     +/-   ##
===========================================
+ Coverage     30.09%   30.59%   +0.5%     
- Complexity     1087     1115     +28     
===========================================
  Files           153      153             
  Lines         10431    10502     +71     
  Branches        943      963     +20     
===========================================
+ Hits           3139     3213     +74     
+ Misses         6989     6978     -11     
- Partials        303      311      +8

Impacted Files	Coverage Δ	Complexity Δ
...iversalgcodesender/model/WorkCoordinateSystem.java	`100% <100%> (+100%)`	`9 <7> (+9)`	⬆️
...llwinder/universalgcodesender/TinyGController.java	`53.9% <58.33%> (+1.34%)`	`25 <1> (ø)`	⬇️
...om/willwinder/universalgcodesender/TinyGUtils.java	`71.65% <69.44%> (+8.27%)`	`42 <18> (+17)`	⬆️
...inder/universalgcodesender/AbstractController.java	`60.97% <0%> (+0.24%)`	`95% <0%> (+1%)`	⬆️
...ersalgcodesender/gcode/GcodePreprocessorUtils.java	`78.21% <0%> (+0.38%)`	`92% <0%> (+1%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update afde061...9395392. Read the comment docs.

winder · 2018-07-11T19:07:21Z

ugs-core/src/com/willwinder/universalgcodesender/TinyGUtils.java

+                axis.name() + Utils.formatter.format(coordinate);
+    }
+
+    private static int convertOffsetGcodeToCode(Code offsetGcode) {


This could go right into GcodeUtils.java, maybe even some of the other WCS helpers you have here as well. There is also WorkCoordinateSystem.java, so something like this might technically work:

WorkCoordinateSystem.valueOf(offsetGcode.toString()).getPValue();

I'd probably lean towards keeping your static method and maybe getting rid of WorkCoordinateSystem

winder · 2018-07-11T19:25:26Z

ugs-core/src/com/willwinder/universalgcodesender/TinyGUtils.java

+     * @param controller the controller to update
+     * @param response   the response to parse the gcode state from
+     */
+    public static void updateGcodeState(TinyGController controller, JsonObject response) {


Not a big fan of the side effects in here, is there any reason why this can't be merged with the code updating the ControllerStatus? You could check with getCurrentParserState to get the state.

The GRBL version could be updated to use getCurrentParserState instead of GrblFeedbackMessage. The second line here literally sends the grbl $G output to the gcode parser to make sure it is up to date:

else if (GrblUtils.isGrblFeedbackMessage(response, capabilities)) { GrblFeedbackMessage grblFeedbackMessage = new GrblFeedbackMessage(response); // Convert feedback message to raw commands to update modal state. this.updateParserModalState(new GcodeCommand(GrblUtils.parseFeedbackMessage(response, capabilities))); this.verboseMessageForConsole(grblFeedbackMessage.toString() + "\n"); setDistanceModeCode(grblFeedbackMessage.getDistanceMode()); setUnitsCode(grblFeedbackMessage.getUnits()); dispatchStateChange(COMM_IDLE); }

winder · 2018-07-11T19:30:03Z

ugs-core/src/com/willwinder/universalgcodesender/TinyGController.java

-            GcodeCommand gcodeCommand = new GcodeCommand(gcode);
-            super.updateParserModalState(gcodeCommand);
-        }
+        // This is handled by the rawResponseHandler


This calls parser.addCommand which updates the on-the-fly GcodeParser, that may need to be called somewhere. I think my plan was to deprecate distanceModeCode / unitsCode in favor of the real gcode parser.

winder

The only critical feedback is to make sure the gcode parser stays up to date with the tinyg state responses. Some other minor feedback in addition to that.

breiler · 2018-07-12T04:46:14Z

It seems that I've been missing a key feature by not adding the parser. =)

I figured that because the controller reports all state changes in the status report I don't need to parse the sent gcode. The way I'm updating the state felt a bit dodgy from the start so this clearly needs to be updated.

I'll see what I can do!

winder

Thanks 👍

breiler · 2018-07-12T08:03:04Z

Are you referring to the machine coordinates?

Yes according to the documentation they are always in MM: https://github.com/synthetos/g2/wiki/Status-Reports

This reverts commit b77ec1c.

winder · 2018-07-12T08:09:20Z

Yes, maybe put a comment there. Looked strange since the other coordinates were in the current units.

Those comments were made directly in a commit, I’m not sure why github didn’t show the context.

…oring codacy warnings).

breiler · 2018-07-12T08:12:15Z

I noticed some problems with the distance mode parsing, need to test it further later.

…ixes problem with parsing of gcode state for arc-distance mode.

breiler · 2018-07-13T05:56:03Z

There was a bit back and forth with this, but I'm starting to feel satisfied.
Does this work for you @winder?

…ing error messages

justinclift · 2018-07-16T09:41:57Z

ugs-core/src/com/willwinder/universalgcodesender/TinyGController.java

    @Override
    public void performHomingCycle() throws Exception {
-        sendCommandImmediately(new GcodeCommand("G28.2 Z0 X0 Y0"));
+        sendCommandImmediately(new GcodeCommand("G28.2 X0 Y0 Z0"));


As a data point, this won't make any functional difference. The Z axis will be homed first anyway, as g2core regards that as being safer than homing it after the others.

Sorry, this is from a bad merge. It's better having the Z probe first as it will reflect the correct order in g2core, will fix this soon!

No worries at all. 😄

winder · 2018-07-16T13:53:47Z

@breiler looks good to me!

justinclift · 2018-07-17T06:00:10Z

Just tried the newest build which according to the Jenkins job info includes this... and the state change bits don't seem to be working right.

If I go to the State tab and change (say) the Units mode from G21 to G20, the "Controller state" (DRO) goes to "RUN" and keeps there. It doesn't seem to recognise that the state change has completed.

I need to press the Cancel button in the toolbar to get the DRO back to IDLE.

Is this expected? 😄

breiler · 2018-07-17T06:15:13Z

I had the same issue when running g2core 101.03 which doesn't report the correct status. I posted a bug report for this in synthetos/g2#367

I could create a timer that polls the status, but it felt wrong when g2core has this status report function.

justinclift · 2018-07-17T06:27:39Z

Ahhh, good point. Yeah, that's probably the thing I'm hitting. I'll drop back to 100.26, and see how that goes. 😄

justinclift · 2018-07-17T06:58:48Z

Yep, 100.26 is working properly. Thanks @breiler. 😄

justinclift · 2018-07-17T17:17:42Z

Found what seems like a bug in this, when testing today on a Win 7 x64 PC. Although the state change bit works, the "Home Machine" button is greyed out.

Maybe another side effect of the bad merge?

justinclift · 2018-07-17T18:10:36Z

Oh that's weird. Now the "Home Machine" button isn't being greyed out. Same Win7 PC, but it's been restarted. For the past few hours (actually using UGS to cut stuff), I've been having to manually type G28.2 (etc) to home things.

I'll keep an eye on this, and let you know if the homing button starts playing up again.

breiler · 2018-07-18T09:28:25Z

Hmm, I'm not able to reproduce this. The common actions panel checks if the controller has homing by its capabilities:

Universal-G-Code-Sender/ugs-core/src/com/willwinder/universalgcodesender/uielements/panels/ActionButtonPanel.java

Line 114 in d071e25

    
           boolean hasHoming = backend.isConnected() && backend.getController().getCapabilities().hasHoming();

The capability is populated when connecting to the TinyG/g2core controller:

Universal-G-Code-Sender/ugs-core/src/com/willwinder/universalgcodesender/TinyGController.java

Line 168 in d071e25

capabilities.addCapability(CapabilitiesConstants.HOMING);

So this should work. =/

I've seen cases though when connecting to the controller the initialising commands can't be properly sent. The only way to resolve this is to disconnect the controller so that it can reboot and connect it again.

justinclift · 2018-07-18T12:25:51Z

Thanks @breiler. The PC it was connected to turned out to have some hardware problems, so took some time to rebuild it from the ground up. Just got it all together again and configured. This time UGS is showing the homing button. I've captured the contents of the initial UGS<->g2core exchange from the console to a text file, so if the homing problem does show up again I'll have something to compare against.

Hopefully it was just a transient weirdness from the failing PC yesterday. 😄

justinclift · 2018-07-19T17:47:55Z

Cutting stuff today, and no issues at all. Guess it really was some kind of weirdness from the failing PC. 😄

Just as a random data point, the stuff cut today was some PC expansion port brackets for network cards that came without them. This was today's (in 0.75mm steel):

And when mounted (the one on the right). The one on the left was yesterday's work, done in 0.75mm aluminium:

UGS seems to be working pretty well for this already. 😄

breiler · 2018-07-19T18:29:02Z

This is awesome, thanks for showing this! 😃

justinclift · 2018-07-21T16:22:30Z

Todays ones, using the same UGS build. This looks to be a decent way to create full height brackets for cheapo Mellanox 10GbE network cards from Ebay that come with half height brackets only. 😄

breiler added 3 commits July 10, 2018 10:02

Added homing command with gcode "G28.2 X0 Y0 Z0"

9456dfa

Added support for parsing gcode states from the responses. Added supp…

ee1756d

…ort for changing work coordinates.

Merge branch 'master' of https://github.com/winder/Universal-G-Code-S…

23815b4

…ender into feature/tinyg Conflicts: ugs-core/src/com/willwinder/universalgcodesender/TinyGController.java

winder reviewed Jul 11, 2018

View reviewed changes

winder requested changes Jul 11, 2018

View reviewed changes

breiler added 4 commits July 12, 2018 07:35

Now updates the gcode state using the gcode parser

347b222

Removed duplicate code after code review

077b1c1

I'm a stupid monkey

a0fc521

Fixed codacy warnings

ae2c4d6

winder reviewed Jul 12, 2018

View reviewed changes

winder approved these changes Jul 12, 2018

View reviewed changes

breiler added 2 commits July 12, 2018 09:22

Attempt to fix codacy warning about NPath complexity

b77ec1c

Fixed codacy warning

c244cb7

Revert "Attempt to fix codacy warning about NPath complexity"

67b6cc6

This reverts commit b77ec1c.

Reverted to the previous status result parsing after code review (ign…

c7baa09

…oring codacy warnings).

Use the code .toString() instead of .name() to get the right gcode. F…

c8bbfd7

…ixes problem with parsing of gcode state for arc-distance mode.

Fixed issue where numeric fields can be returned with true/false caus…

61e5858

…ing error messages

justinclift mentioned this pull request Jul 16, 2018

Improve support for TinyG / g2core #1071

Closed

7 tasks

justinclift reviewed Jul 16, 2018

View reviewed changes

Fixed bad merge after code review

9395392

breiler merged commit e159183 into winder:master Jul 16, 2018

justinclift mentioned this pull request Jul 24, 2018

ARM discussion gnea/grbl#67

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for gcode states and work coordinates #1078

Added support for gcode states and work coordinates #1078

breiler commented Jul 11, 2018

codecov-io commented Jul 11, 2018 •

edited

Loading

winder Jul 11, 2018

winder Jul 11, 2018

winder Jul 11, 2018

winder Jul 11, 2018

winder left a comment

breiler commented Jul 12, 2018

winder left a comment

breiler commented Jul 12, 2018

winder commented Jul 12, 2018

breiler commented Jul 12, 2018

breiler commented Jul 13, 2018

justinclift Jul 16, 2018

breiler Jul 16, 2018

justinclift Jul 16, 2018

winder commented Jul 16, 2018

justinclift commented Jul 17, 2018

breiler commented Jul 17, 2018

justinclift commented Jul 17, 2018

justinclift commented Jul 17, 2018

justinclift commented Jul 17, 2018 •

edited

Loading

justinclift commented Jul 17, 2018

breiler commented Jul 18, 2018

justinclift commented Jul 18, 2018

justinclift commented Jul 19, 2018

breiler commented Jul 19, 2018

justinclift commented Jul 21, 2018

Added support for gcode states and work coordinates #1078

Added support for gcode states and work coordinates #1078

Conversation

breiler commented Jul 11, 2018

codecov-io commented Jul 11, 2018 • edited Loading

Codecov Report

winder Jul 11, 2018

Choose a reason for hiding this comment

winder Jul 11, 2018

Choose a reason for hiding this comment

winder Jul 11, 2018

Choose a reason for hiding this comment

winder Jul 11, 2018

Choose a reason for hiding this comment

winder left a comment

Choose a reason for hiding this comment

breiler commented Jul 12, 2018

winder left a comment

Choose a reason for hiding this comment

breiler commented Jul 12, 2018

winder commented Jul 12, 2018

breiler commented Jul 12, 2018

breiler commented Jul 13, 2018

justinclift Jul 16, 2018

Choose a reason for hiding this comment

breiler Jul 16, 2018

Choose a reason for hiding this comment

justinclift Jul 16, 2018

Choose a reason for hiding this comment

winder commented Jul 16, 2018

justinclift commented Jul 17, 2018

breiler commented Jul 17, 2018

justinclift commented Jul 17, 2018

justinclift commented Jul 17, 2018

justinclift commented Jul 17, 2018 • edited Loading

justinclift commented Jul 17, 2018

breiler commented Jul 18, 2018

justinclift commented Jul 18, 2018

justinclift commented Jul 19, 2018

breiler commented Jul 19, 2018

justinclift commented Jul 21, 2018

codecov-io commented Jul 11, 2018 •

edited

Loading

justinclift commented Jul 17, 2018 •

edited

Loading