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.17k stars 19.21k forks source link

Circular Nozzle clean #8604

Closed Core3dTech closed 6 years ago

Core3dTech commented 6 years ago

Bug Report

code used to create circle currently:

float x, y;
    for (uint8_t s = 0; s < strokes; s++) {
      for (uint8_t i = 0; i < NOZZLE_CLEAN_CIRCLE_FN; i++) {
        x = middle.x + sin((M_2_PI / NOZZLE_CLEAN_CIRCLE_FN) * i) * radius;
        y = middle.y + cos((M_2_PI / NOZZLE_CLEAN_CIRCLE_FN) * i) * radius;

        do_blocking_move_to_xy(x, y);
      }
   } 

Assuming NOZZLE_CLEAN_CIRCLE_FN represents a slice of the circle, the M_2_PI should be *2M_PI**

    float x, y;
    for (uint8_t s = 0; s < strokes; s++) {
      for (uint8_t i = 0; i < NOZZLE_CLEAN_CIRCLE_FN; i++) {
        x = middle.x + sin((2* M_PI / NOZZLE_CLEAN_CIRCLE_FN) * i) * radius;
        y = middle.y + cos((2* M_PI / NOZZLE_CLEAN_CIRCLE_FN) * i) * radius;

        do_blocking_move_to_xy(x, y);
      }
    }
bjarchi commented 6 years ago

Confirmed - it looks like the M_2_PI define in Arduino/Math.h is for 2/pi, not 2*pi as someone might expect:

#define M_2_PI      0.63661977236758134308

Later on there's a M_TWOPI define that's correct but it's guarded as follows:

#ifndef __STRICT_ANSI__

#define M_TWOPI         (M_PI * 2.0)
#define M_3PI_4     2.3561944901923448370E0
... 

So I'm not sure if that's available for use or not. Fixing this would be better achieved by using (or creating) a 2pi constant, rather than doing the extra 2 M_PI multiplication in each loop iteration IMO.

@thinkyhead This is a bug I could actually fix, but I'm curious about whether that M_TWOPI define is available (and guaranteed to be available across boards/Arduino versions)

bjarchi commented 6 years ago

@mvl03 This should be fixed now in latest bugfix-1.1.x branch (and bugfix-2.0.x). I don't use the nozzle clean feature, so I could only verify that it compiles correctly and that the intent of the code is correct. Can you give it a try on your hardware, and close the issue if it is indeed fixed?

bjarchi commented 6 years ago

@thinkyhead Realized that I can test this since the config gives full control over positioning and Z height.

Tested on bugfix-1.1.x... G12 P2 S3 - Nozzles goes to the specified point, does a cute little circular dance (3x) and returns to where it started.

Core3dTech commented 6 years ago

excellent. thank you. I created some custom code of my own but I'll have a look at the new code when I get a chance.

Core3dTech commented 6 years ago

Tested the new circular code on my printer and it worked. Thx

bjarchi commented 6 years ago

Thanks for testing!

Are you on bugfix-1.1.x or bugfix-2.0.x? I've also tested on 1.1.x, would be nice to see a test on 2.0.x (although the code change is identical)

Core3dTech commented 6 years ago

To be honest I tested your code snippet from bugfix-1.1.x . I'm not quite ready right now to overwrite my entire installation. I haven't looked at 2.0.x release yet.

bjarchi commented 6 years ago

That's fine - I'm in the same boat in terms of changing to bugfix-2.0.x. I was hoping you might already be there :)

I'll probably be switching to track that version soon, so I'll test when I do if nobody else gets there first.

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.