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

Loading preprocessors #1461

Merged
merged 11 commits into from
Dec 2, 2020
Merged

Loading preprocessors #1461

merged 11 commits into from
Dec 2, 2020

Conversation

breiler
Copy link
Collaborator

@breiler breiler commented Oct 31, 2020

@winder you often mention the preprocessor as a solution to many problems and I now see why. It can be really powerful way to manipulate the gcode.

The "Run from"-feature is great, but I feel that it is a bit limited as it creates a new program and loads it. If the user changes her mind the original program needs to be reloaded. So I have a suggestion that we enable some of these preprocessors through services that alters the preprocessors states (row number to start from). I have also made an attempt to create a rotation feature as shown in the video below.

Screen Recording 2020-10-31 at 20 18 14 2020-10-31 20_29_28

I'm not sure if this is the best approach, when you get the time I would appreciate if you browsed through my changes.

  • When applying a preprocessor it will be applied to the currently loaded program instead of creating a new one
  • Created services that manipulate instances of a rotate- and run from preprocessor.
  • Services also provides access to the preprocessor settings, so the "run from"-line could now be higlighted in the editor for better user experience.
  • Added a new menu "Program" that will be used for manipulating the currently loaded gcode program
  • Added four new actions for rotating the gcode program

@breiler breiler requested a review from winder October 31, 2020 19:44
@codecov-io
Copy link

codecov-io commented Oct 31, 2020

Codecov Report

Merging #1461 into master will decrease coverage by 0.26%.
The diff coverage is 56.50%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1461      +/-   ##
============================================
- Coverage     37.11%   36.84%   -0.27%     
- Complexity     1200     1218      +18     
============================================
  Files           160      164       +4     
  Lines          9097     9273     +176     
  Branches        890      900      +10     
============================================
+ Hits           3376     3417      +41     
- Misses         5406     5535     +129     
- Partials        315      321       +6     
Impacted Files Coverage Δ Complexity Δ
...inder/universalgcodesender/AbstractController.java 63.18% <ø> (ø) 98.00 <0.00> (ø)
...odesender/gcode/processors/TranslateProcessor.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
.../universalgcodesender/services/RunFromService.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...llwinder/universalgcodesender/utils/MathUtils.java 36.36% <0.00%> (-37.72%) 10.00 <0.00> (ø)
...niversalgcodesender/visualizer/GcodeViewParse.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...lwinder/universalgcodesender/model/GUIBackend.java 46.28% <33.33%> (-0.78%) 55.00 <2.00> (+1.00) ⬇️
...gcodesender/gcode/processors/RunFromProcessor.java 76.92% <50.00%> (-18.32%) 4.00 <3.00> (ø)
...illwinder/universalgcodesender/model/Position.java 50.00% <50.00%> (-3.34%) 12.00 <1.00> (+1.00) ⬇️
...winder/universalgcodesender/gcode/GcodeParser.java 91.17% <60.00%> (+0.65%) 13.00 <2.00> (-47.00) ⬆️
...lgcodesender/gcode/processors/RotateProcessor.java 66.00% <66.00%> (ø) 11.00 <11.00> (?)
... and 18 more

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 72d6a70...4c577f7. Read the comment docs.

@winder
Copy link
Owner

winder commented Nov 1, 2020

Wow, this is really cool. I'll need to schedule time time to properly review it.

I definitely agree that it's a bit awkward to rewrite the file each time and the UX is not great. The one nice thing about it is that it allows the user to see exactly what the processor is doing. For example run-from has some special code to rewrite the state init section. Does the export button still export the preprocessed code?

@breiler
Copy link
Collaborator Author

breiler commented Nov 3, 2020

Yes, the export button should still export it but I have not tested it yet. We could create a separate editor view in the platform edition to view the preprocessed code and add a export button there?

I ran into another problem when experimenting further. I want a preprocessor that can align the model to xy zero. I created on that kind of works, but if I want to combine both rotate and translate I would need the state of the model after the rotation has been applied. This is needed to get the new lower left corner of the rotated model to know how to translate the model.

I am now using the preprocessed file in RotateAction and TranslateAction to figure out the lower left corner or the middle of the model but I don't think that this will scale well. If we have many chained preprocessors, their state is based on the previous processor, and if the previous processor is changed or removed the state of the succeeding processor is no longer valid.

In this example I rotate the image 90 degrees, translate to zero, and rotate back to 0 degrees:
Screen Recording 2020-11-03 at 21 00 19 2020-11-03 21_04_24

It is maybe good enough, but I want it to be perfect! 😬

@breiler
Copy link
Collaborator Author

breiler commented Nov 4, 2020

Ok so I've changed my mind and are going for the more "keep it simple" direction.

Now I'm just stacking the operations on top of each other which works a lot better:
Screen Recording 2020-11-04 at 20 51 57 2020-11-04 20_58_58

Of course this doesn't prevent a user from doing stupid stuff like rotating multiple times on a big gcode model. 😅

Copy link
Owner

@winder winder left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, and this wasn't the most thorough review. I do think there might be times when it's useful to see the processed code, mainly during development/debugging of "invasive" processors. But this is nice progress!

…Sender into feature/processor

# Conflicts:
#	ugs-core/src/com/willwinder/universalgcodesender/gcode/GcodeParser.java
#	ugs-core/src/com/willwinder/universalgcodesender/visualizer/GcodeViewParse.java
@breiler breiler merged commit 055901e into winder:master Dec 2, 2020
@breiler breiler deleted the feature/processor branch December 3, 2020 17:17
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