-
-
Notifications
You must be signed in to change notification settings - Fork 766
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
Updates & fixes for Auto Leveler plugin #2022
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2022 +/- ##
============================================
+ Coverage 22.51% 22.54% +0.02%
Complexity 1997 1997
============================================
Files 577 577
Lines 24244 24252 +8
Branches 1922 1924 +2
============================================
+ Hits 5459 5468 +9
+ Misses 18314 18308 -6
- Partials 471 476 +5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
// If we're in relative mode there is no adjustment needed | ||
// TODO: This won't be true if we start in relative mode, in which case we need to ensure the first command | ||
// is to move down by the offset at (0, 0) | ||
if (!state.inAbsoluteMode) { | ||
return Collections.singletonList(commandString); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really true? If there is a difference in the mesh heights between the current point and the previous one doesn't it need to be calculated and applied?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in other words, the current zPointOffset
minus the previous zPointOffset
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. The previous code was wrong, but so is this. I'll send out an updated patch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. I was previously testing with gcode that only had relative z axis-only moves. This time I tested with the gcode below. It does still have an issue when the first move is relative, but I think it requires a broader change to be able properly detect and deal with this situation.
G91
G1 Z-1F300
G1 X10Z2F1000
G1 Y10Z2
G1 X-10Z-2
G1 Y-10Z-2
@@ -599,26 +599,32 @@ private void dataViewerActionPerformed(java.awt.event.ActionEvent evt) {//GEN-FI | |||
}//GEN-LAST:event_dataViewerActionPerformed | |||
|
|||
private void applyToGcodeActionPerformed(java.awt.event.ActionEvent evt) {//GEN-FIRST:event_applyToGcodeActionPerformed | |||
GcodeParser gcp = new GcodeParser(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was something broken with this approach? Without replacing the gcode parser you could have other processors loaded which might effect the final generated program.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that applyGcodeParser
was deprecated and I assumed that having all processors that loaded run would be the expected behavior...wouldn't one expect those other processors to effect the final generated program?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember deprecating this method as I thought it got unintuitive when you applied a processor the previous one got replaced. For instance this prevented the user to apply "Mirror" and "Run from" at the same time.
It is still a bit unintuitive as the changes aren't reflected in the GCode which might have made more sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to reflecting the changes in the GCode in some way. I considered adding a (read-only) view of what will actually be sent out when I was debugging this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default processors would remove comments and I always tried to maintain those in the editor view, but I can see how it's misleading to display different gcode than what gets sent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it not be best to apply all processors in a serial fashion, rather than just the last? The order could be pre-determined based on what made the most sense.
…start AutoLeveler apply to gcode will now remove old processors and replace instead of disabling button Fixes for MeshLeveler relative mode (G91)
eb10afa
to
bd57fd7
Compare
I would be happy to test this new auto-level function if you would like. Just let me know what you need me to do. 👍 |
FWIW, I've engraved several PCBs using this code with a 0.05mm depth and it has worked great for me. I do often forget to hit the apply button first though, so I may look into adding a reminder if you have scan data loaded but haven't applied it to your gcode 😉 |
I hate to be 'that guy', but are there directions somewhere on how to build this version into something I can run on my Windows machine, please? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, fantastic. Nice work Nick and Joacim.
With the recent Github build script addition, does this mean that downloading the Nightly will include this patch?
EDIT: I downloaded the latest nightly (titlebar says 2022-12-28), but I still get the 'Autoleveler doesn't work' warning on startup. @nickmayer indicated here that it should only warn when autoleveling is applied to gcode, so I'm left wondering if this nightly is actually up to date.
@daxliniere I haven't merged it yet as I have not been able to test it (my workshop is temporarily used for storage due to some renovations so there is very limited access). You are more than welcome to test it and report any findings. It should be available in the nightly build as soon as this finishes: |
Will do Joacim, but won't be back at the workshop for at least another week, I'd say. |
Just extracted it and there's a problem. I got the usual issue that requires cache to be deleted. I deleted it in both locations and on re-loading I got 'module needed, but not found' errors. I clicked Exit, then on re-load again, it loads without error message, but looks like this: There are no cache folders left. |
@daxliniere this is so weird that this happens to you. I upgrade/downgrade UGS all the time without this ever happening to me. Anyway, since version 2.0.13 the cache folder is now located under: Sorry for the inconvenience... =( |
Yup, I have deleted both cache dirs, but I still have this problem. Any idea if I can get my layout back? |
How would I perform a clean installation? Maybe that will prevent this happening? |
When the error occurred it hopefully just broke the window locations. But I am afraid that those settings are lost. As I have never been able to reproduce the error it would have been interesting to see your log file when this happens. You should be able to restore the window positions by using the menu
A clean install would be to download and unzip UGS:
|
AutoLeveler now warns about possible bugs on applying to gcode instead of spamming on startup
AutoLeveler apply to gcode will now remove old processors and replace instead of disabling button making it possible to use a different heightmap
Fix for MeshLeveler relative mode (G91)