MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.28k stars 19.24k forks source link

Skew Correction doesn't work with UBL #8632

Closed PolloPequeno closed 6 years ago

PolloPequeno commented 6 years ago

I've been testing out the Skew correction update and cannot get any noticeable correction when I use the M852 SX.XX command. The Z correction is turned off. I tried it a bunch of different ways without any meaningful change.

On a side note, the big fixes since the beginning of Nov made a noticeable improvement to how my printer is running. Thanks!


Bug Report

For my testing I used the thingiverse file, mentioned in the config file. After trying it out with my calculated skew value of -.01 and not seeing any change, I switched to -.05 for further testing. Playing around with the inputs, a correction factor of -.05 is roughly a correction factor for +8mm length on the BD diagonal compared to the AC one. Certainly a large enough "correction" to detect a change. My printer is naturally skewed as the graphic in the config illustrates.

All the tests below show no meaningful change compared to the test print prior to using M852.

Let me know if there are any additional variations you would like me to test.

Thank you!

Configuration.zip

thinkyhead commented 6 years ago

The problem is that this feature isn't compatible with Unified Bed Leveling in its standard mode. When we move more of UBL's leveling logic into the Planner with the other leveling code then it will work.

But there is a workaround. In the latest bugfix-1.1.x configuration files there is an option SEGMENT_LEVELED_MOVES that should make UBL call the Planner for leveling. Add this to your configuration and skew should start to work.

  #if ENABLED(MESH_BED_LEVELING) || ENABLED(AUTO_BED_LEVELING_BILINEAR) || ENABLED(AUTO_BED_LEVELING_UBL)
    // Gradually reduce leveling correction until a set height is reached,
    // at which point movement will be level to the machine's XY plane.
    // The height can be set with M420 Z<height>
    #define ENABLE_LEVELING_FADE_HEIGHT

+   // For Cartesian machines, instead of dividing moves on mesh boundaries,
+   // split up moves into short segments like a Delta.
+   #define SEGMENT_LEVELED_MOVES
+   #define LEVELED_SEGMENT_LENGTH 5.0 // (mm) Length of all segments (except the last one)
+
    /**
     * Enable the G26 Mesh Validation Pattern tool.
     */
    //#define G26_MESH_VALIDATION   // Enable G26 mesh validation
    #if ENABLED(G26_MESH_VALIDATION)
      #define MESH_TEST_NOZZLE_SIZE     0.4   // (mm) Diameter of primary nozzle.
      #define MESH_TEST_LAYER_HEIGHT    0.2   // (mm) Default layer height for the G26 Mesh Validation Tool.
      #define MESH_TEST_HOTEND_TEMP   205.0   // (°C) Default nozzle temperature for the G26 Mesh Validation Tool.
      #define MESH_TEST_BED_TEMP       60.0   // (°C) Default bed temperature for the G26 Mesh Validation Tool.
    #endif

  #endif
PolloPequeno commented 6 years ago

Something very weird is happening now. Please see #8638.

On a side note, I did try using #define SEGMENT_LEVELED_MOVES with the code I pulled from Dec 2 and the Skew correction did not appear to work, although I only got one test print in before I tried the Dec 3rd updates and had the problem I described in the linked issue.

Let me know if there is anything I can do to help out with this.

PolloPequeno commented 6 years ago

I did some more testing and Skew doesn't work even if SEGMENT_LEVELED_MOVES is enabled. I looked back through the commits and it didn't look like the split moves revert should have effected this, as I see it various files. But I could be wrong about that. Please let me know.

Thank you and good luck with those split move gremlins!

thinkyhead commented 6 years ago

I think we might have gotten those gremlins. So we should take another look at the skew behavior.

So, you don't see any skew compensation at all, even with leveling off? Even with leveling off the Planner::apply_leveling function should do this much:

#if ENABLED(SKEW_CORRECTION)
  if (WITHIN(rx, X_MIN_POS + 1, X_MAX_POS) && WITHIN(ry, Y_MIN_POS + 1, Y_MAX_POS)) {
    const float tempry = ry - (rz * planner.yz_skew_factor),
                temprx = rx - (ry * planner.xy_skew_factor) - (rz * (planner.xz_skew_factor - (planner.xy_skew_factor * planner.yz_skew_factor)));
    if (WITHIN(temprx, X_MIN_POS, X_MAX_POS) && WITHIN(tempry, Y_MIN_POS, Y_MAX_POS)) {
      rx = temprx;
      ry = tempry;
    }
    else
      SERIAL_ECHOLN(MSG_SKEW_WARN);
  }
#endif

You might try sticking an #error "Hey!" line after #if ENABLED(SKEW_CORRECTION) to make sure that code is definitely being included. Meanwhile, I will test with your configurations. Are they still the same as when you first posted?

PolloPequeno commented 6 years ago

I think we might have gotten those gremlins.

Happy to hear to hear that

I haven't tried it with UBL turned off. Let me fire up the printer and I'll get back to you. Thanks!

The configs I posted haven't changed, except for the segment line option.

PolloPequeno commented 6 years ago

Confirmed that code is being included via #error "Hey!"

PolloPequeno commented 6 years ago

Marlin won't compile with Skew turned on and UBL off, so I used M420 S0 to turn it off and did some test prints.

Unfortunately, no change. I did notice that if I gave a large skew value (e.g. -.1) the whole print was shifted on the x axis, but the actual print itself measured no difference compared to skew being off. Maybe there's an issue with it not reading the Y position?

Let me know if there are other print variations I should try.

thinkyhead commented 6 years ago

I've fixed Marlin so it can compile skew without any form of leveling enabled. I'll do some testing tonight and see what's going on.

PolloPequeno commented 6 years ago

I did some testing with leveling disabled and I'm happy to report skew correction is working.

thinkyhead commented 6 years ago

Good to hear it! I thought it might be working today, as my latest tests showed it to be so.

PolloPequeno commented 6 years ago

I did some more testing and I cannot get it to work with UBL. Based on the sanitychecks, I believe that the expectation is that it should work with UBL. Let me know if I'm mistaken.

For my troubleshooting I tried to trace what the segment lines option was doing. My thought being either the skew correction is being overwritten or it's not being applied. One of the things that jumped out to me was this code block in marlin_main

#if HAS_MESH
      if (planner.leveling_active && planner.leveling_active_at_z(destination[Z_AXIS])) {
        #if ENABLED(AUTO_BED_LEVELING_UBL)
          ubl.line_to_destination_cartesian(MMS_SCALED(feedrate_mm_s), active_extruder);  // UBL's motion routine needs to know about
          return true;                                                                    // all moves, including Z-only moves.
        #elif ENABLED(SEGMENT_LEVELED_MOVES)
          segmented_line_to_destination(MMS_SCALED(feedrate_mm_s));
          return false;

as it will never go into the ENABLED(SEGMENT_LEVELED_MOVES).

I also tried using the Debug feature, but it appears that it only shows me what is being sent to Marlin. Is it possible to see what is being sent to the printer after Marlin applies it's magic.

Thank you for your help with this, I think this feature is going to be a game changer for home built printers where it's very difficult to get the printer frame and motion system perfectly square and level.

thinkyhead commented 6 years ago

It seems that for UBL there need to be some extra hooks added to handle skew. I should have a branch you can test soon. It will be in this branch: bf1_sort_out_leveling.

PolloPequeno commented 6 years ago

Thanks! Looking toward to testing it

PolloPequeno commented 6 years ago

I might be jumping the gun here, so if I am ignore this.

I did some testing with the bf1_sort_out_leveling branch and found an issue. Skew is having an effect however it's causing the X axis to do a back and forth motion whenever the Y axis is moved. Here's a first layer pic of the thingiverse file listed in the config for XY skew correction.

photo dec 09 3 12 41 pm

thinkyhead commented 6 years ago

There might have been typos in an earlier version. See if the current code in that branch behaves any better now.

PolloPequeno commented 6 years ago

Did some testing, same issue.

thinkyhead commented 6 years ago

I can't see anything particularly wrong with the skew code. Can you do some more testing to see how much different values for skew affect the unusual motion behavior? For example, if you set it to zero does the effect go away? Does it start to show up with a low skew like 0.005, or does it only show up with larger values? I will do some testing of my own shortly.

PolloPequeno commented 6 years ago

Definitely, I'll let you know what I find.

Quick question: if I give M852 S0.001 will Marlin use .001 even though it reports back as S0.00?

PolloPequeno commented 6 years ago

Here are the results: 0 - no issue .001 - no issue, but I don't think skew was doing anything, I didn't see the X stepper move .01 and -.01, much less than the pic above (which was at .05), but still noticeable, particularly when changing directions.

Here's a first layer pic at -.01 (please ignore the general crappiness of it, I haven't recalibrated it since the nozzle crash)

photo dec 10 8 18 53 pm

Let me know what you find in your testing.

thinkyhead commented 6 years ago

I figured out the cause. Forgot to skew the starting segment position. Should work better now!

PolloPequeno commented 6 years ago

Did some testing; it's working!

Thank you for working on this, I'll get the documentation out soon.

github-actions[bot] commented 3 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.