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.18k stars 19.22k forks source link

Delta : Per tower rod trim #2159

Closed BuzzBumbleBee closed 6 years ago

BuzzBumbleBee commented 9 years ago

Hello all,

I have implemented some trim process for each tower in regards to diagonal rod lengths locally, and i have a few questions.

A) Was i blind and there is already a function for this in the development branch ?

B) Would a feature like this be wanted in the main firmware ?

C) If so would a M command be needed for this or would configuration based be ok ?

The reason I decided to try and trim my diagonal rod lengths in FW was even tho I created the rods on a JIG there was still slight oval to the prints (between 0.4 and 0.8mm error over 50mm on all towers). With the trim process I have added i am now less than 0.2mm error on all towers (the rest i can put down to how the filament settles)

Cheers

Wackerbarth commented 9 years ago

@BuzzBumbleBee -- A) There is some code. But it was developed against an ancient version of Marlin. I've been tackling other issues, like getting the development branch to work in my environment., before I work on this issue. One thing that bothers me is that it seems that each developer has been tacking on his own way to address the problems. IMHO, we need to set up a design methodology for the parameterization of the printer rather than having multiple parameters which act at cross purposes.

B) I assume that we need it to handle deltabots. If it doesn't fit in the main firmware, then Marlin is abandoning a significant market segment. I don't want to see that happen.

C) ALL of the parameters that are likely to need adjusting SHOULD be settable by some M-code and also stored in Eprom.

BuzzBumbleBee commented 9 years ago

@Wackerbarth

Fwiw my method is to calculate a delta_diagonal_rod_2 value for each tower (eg delta_diagonal_rod_2_z) using the value from DELTA_DIAGONAL_ROD (or its eeprom value) plus a small trim. This value is then used in calculate_delta for each tower.

For me this allowed me to use DELTA_DIAGONAL_ROD as the JIG length (in my case 162.0) and the trims varied +/- 0.6mm to get the delta calibrated. At the moment my code lacks the m command to read over serial, but the movement of the printer is fine as far as I can tell

Regards

Wackerbarth commented 9 years ago

Yes, I understand what you have done. And I agree that a computation of that nature needs to be performed.

I see two issues: 1) The rod lengths are just part of the parameter space describing a deltabot. Assuming that the towers are exactly perpendicular to the bed, in addition to the rod lengths, there is the issue of the tower placement, and there is the issue of the placement of the homing switch on each tower. In all, that creates a 9 dimensional space. I suggest that we have compile time constants for the nominal value of each of them and then a "trim" amount that can be set by Mcode (and stored in Eprom)

2) There are multiple coordinate systems which are candidates for the calculations. I'm not sure that the cartesian system is necessarily the fastest. I can think of 3 cylindrical systems that might also work.

BuzzBumbleBee commented 9 years ago

There is already a trim option for the end-stops, you are suggesting the fw need the ability to fine trim delta radius and placement of the towers around that circle (per tower) ?

In any case ill push my changes to my github and send a PR (cant hurt) the code seems to work well and works cleanly with the dev branch

Wackerbarth commented 9 years ago

Yes, I am aware that there are some "trim options" for the end stops. What I am suggesting is that, rather than having different, ad hoc, mechanisms, we need to specify all 9 of these dimensions (and their adjustments) in a unified manner.

BuzzBumbleBee commented 9 years ago

Ah yeah, that would be handy. However the M/G codes used have to follow the structure that set out by machines predating marlin. As far as i know there is no command that allows setting delta machine trims at this complexity

Wackerbarth commented 9 years ago

... there is no command ... -- that means that we can write (or preferably just extend) one. I suggest that we try to extend the 3-axis model to have 9 axes. the "normal" 3 for the positioners, and 6 trim-able "constants" to describe the tower placement and arm lengths.

"Tower placement" is an interesting domain. Nominally, we have a fixed radius for a circle and the towers are 120 degrees apart. But, if you move one of the towers inward (for example if there is a variance in the carriage depth or rail placement), the three points still lie on a circle, but it has a larger radius, its center moves, and the angles change.

An alternate way to look at the tower placement (which I think makes more sense) is to consider instead a polar coordinate system on which the three towers are exactly 120 degrees apart. In this case, moving one of the towers changes the center and radii of each tower. However, I think that the effect is less severe.

This has the advantage that we are describing all 9 parameters as distances.

No matter how we do it, we will want to pre-compute values that feed the distance calculation and have them available in RAM. They change only when we adjust the calibration.

BuzzBumbleBee commented 9 years ago

So just to confirm im on the right track the 9 "axis" are :-

X Y Z endstops X Y Z delta rod length trim X Y Z delta radius trim

Or was the last one trim from the 120 degree layout ?

Wackerbarth commented 9 years ago

To avoid confusion, for the purpose of discussion (only), let's use X,Y,Z to describe the position of the extruder nozzle in a cartesian system. The dimensions related to each of the towers will be designates by a numerical index [1,2,3]

Now, I assume that you are familiar with the transform of the kinematics which has the three arms meeting at the point of the nozzle (We subtract the radial components of the effector and the vertical offset of the nozzle) rather than the physical distances. I refer to dimensions related to that space.

We choose a cylindrical coordinate system such that, if every thing was exactly to "factory design", The towers would be located at some radius "R", and the top of the arms would be at height "H". The nozzle would then be at a height "Z" determined by (H-Z)^2 + R^2 = L^2 where L is the length of the arm.

But, from the nominal values, we will "trim" each, replacing H with (H+h1), R with (R+r1), and L with (L+l1), and similarly for towers 2 and 3.

Note that the coordinate system is chosen such that the towers remain at EXACTLY 120 degree placements and in the "neutral" position, the nozzle is EXACTLY centered (R=0). The "trim" values for the three carriages (h1, h2, h3) are chosen to then reflect the distance from that point to the contact point of the corresponding end stop. This value depends on both the location of the switch and the variations in the other dimensions.

clefranc commented 9 years ago

@Wackerbarth Did you have time to take a look at this: https://github.com/clefranc/Marlin/commit/122c608f83d7870d14ff45ae0df75f1f88d065ee

Somewhat related to this issue.

Wackerbarth commented 9 years ago

@clefranc -- Yes, that is a small part of the code. It fails to address trimming each of the 9 variables and I don't like the implied "angularly adjusted" coordinate space.

BuzzBumbleBee commented 9 years ago

@clefranc that code will also not use the eeprom value afaik (at the very least the delta_diagonal_rod set on the fly would have no effect). Its probably cleaner (code / commit wise) to split each set of values (diagonal rod trim / delta radius trim / angular trim) into separate commits

@Wackerbarth i understand the theory of the delta printers enough, however I was unsure if you were talking about trimming the delta radius per tower AND / OR any angular trims for units that don't a perfect 120 / 120 / 120 layout. Bear in mind i am referring to current variables and config settings within marlin for ease

Wackerbarth commented 9 years ago

I am suggesting that we need only handle the model that assumes perfect 120/120/120 spacing with variable radius.

BuzzBumbleBee commented 9 years ago

OK I'm going to refactor my changes to allow changing both diagonal rod trim and delta radius per tower. Via config and M command

BuzzBumbleBee commented 9 years ago

@Wackerbarth here is my initial change

https://github.com/BuzzBumbleBee/Marlin/commit/4724995d965f8b902b7ac132f437ad8b5741861b

I haven't looked at EEPROM yet as that involved changing the layout and breaking compatibility with mainline Marlin

BuzzBumbleBee commented 9 years ago

If anyone in interested in this feature / has feedback I am implementing this here

https://github.com/BuzzBumbleBee/Marlin/tree/Delta-Trim

clefranc commented 9 years ago

@Wackerbarth

I am suggesting that we need only handle the model that assumes perfect 120/120/120 spacing with variable radius.

My tower spacing is not perfect, one of my tower is 1° off. My rod length are not perfect too, there is millimeters variance. I can't adjust the towers placement, nor the arms length and I'm not going to disassemble the printer to fix these. The only adjustment I have is the endstops height (I'm lucky).

Since there is already calculations in Marlin for tower angles, why not include them in the EEPROM?

Code from actual development branch:

#ifdef DELTA
  float delta[3] = { 0 };
  #define SIN_60 0.8660254037844386
  #define COS_60 0.5
  float endstop_adj[3] = { 0 };
  // these are the default values, can be overriden with M665
  float delta_radius = DELTA_RADIUS;
  float delta_tower1_x = -SIN_60 * delta_radius; // front left tower
  float delta_tower1_y = -COS_60 * delta_radius;     
  float delta_tower2_x =  SIN_60 * delta_radius; // front right tower
  float delta_tower2_y = -COS_60 * delta_radius;     
  float delta_tower3_x = 0;                      // back middle tower
  float delta_tower3_y = delta_radius;
  float delta_diagonal_rod = DELTA_DIAGONAL_ROD;
  float delta_diagonal_rod_2 = sq(delta_diagonal_rod);
  float delta_segments_per_second = DELTA_SEGMENTS_PER_SECOND;
  #ifdef ENABLE_AUTO_BED_LEVELING
    int delta_grid_spacing[2] = { 0, 0 };
    float bed_level[AUTO_BED_LEVELING_GRID_POINTS][AUTO_BED_LEVELING_GRID_POINTS];
  #endif
#else
  static bool home_all_axis = true;
#endif

Perhaps expand M665 or create a new command for all adjustment:

M664 T A L R E S<segments/s>

Wackerbarth commented 9 years ago

You should not provide two sets of parameters which define the same points. Although you can define a coordinate system based on a given radius and variable angles, I believe that the alternate system based on constant angles and variable radii is simpler to handle and likely to give better results. (For example, we know, to any desired precision, the exact sin and cos for 120 degrees. However, we can only approximate it for other nearby angles.

And there is absolutely nothing to be gained by allowing both the angle and the radius to be varied. To do so requires 6 parameters and 3 are sufficient to define the relative tower positions.

Just because someone, at some time, added a particular parameter is not a good reason to perpetuate it. If existing users need to convert, a simple routine can be built to do the transformations.

Wackerbarth commented 9 years ago

On May 28, 2015, at 1:12 PM, Christian Lefrançois notifications@github.com wrote:

@Wackerbarth https://github.com/Wackerbarth I am suggesting that we need only handle the model that assumes perfect 120/120/120 spacing with variable radius.

My tower spacing is not perfect, one of my tower is 1° off. My rod length are not perfect too, there is millimeters variance. I can't adjust the towers placement, nor the arms length and I'm not going to disasemble the printer to fix these. The only adjustment I have is the endstops height (I'm lucky).

I’m not suggesting that you make physical adjustments to the towers. I only assert that you can provide an alternate description of the positions which requires fewer parameters and does not involve angles.

boelle commented 9 years ago

has anyone looked at the link @BuzzBumbleBee posted? could that be used to solve this one?

BuzzBumbleBee commented 9 years ago

@boelle

My branch can currently "trim" the diagonal rod lengths to allow greater accuracy on X/Y dimensions on deltas. I will add in (when i get a free moment this week) the ability to shim the delta radius of each tower.

@Wackerbarth for angular trimming it would easy to overload the SIN_60 / COS_60 in configuration for wherever needs it.

Wackerbarth commented 9 years ago

I don't think you can simply overload the trig because that would imply a symmetry in the placement of the towers. But, if you are going to choose a placement to that keeps that symmetry, you might as well choose a placement that keeps all of the towers positioned at the proper angle.

BuzzBumbleBee commented 9 years ago

@Wackerbarth fair point ..... the more i think about it the less i like modifying those values.

BuzzBumbleBee commented 9 years ago

Rebased on current Development branch and added per tower radius trim https://github.com/BuzzBumbleBee/Marlin/tree/Delta-Trim

boelle commented 9 years ago

can you submit a PR also? it will prob last a while before we merge though

Den onsdag den 3. juni 2015 skrev BuzzBumbleBee notifications@github.com:

Rebased on current Development branch and added per tower radius trim https://github.com/BuzzBumbleBee/Marlin/tree/Delta-Trim

— Reply to this email directly or view it on GitHub https://github.com/MarlinFirmware/Marlin/issues/2159#issuecomment-108265497 .

Wackerbarth commented 9 years ago

@BuzzBumbleBee -- Please post this as a pull request against the BuzzBase Branch of my repository. We can work on it there since this repository is slow to incorporate changes of this nature.

BuzzBumbleBee commented 9 years ago

pull request sent to both :+1:

superlou commented 8 years ago

Is anyone still working towards this?

thinkyhead commented 8 years ago

@superlou Currently we have…

/**
 * M665: Set delta configurations
 *
 *    L = diagonal rod
 *    R = delta radius
 *    S = segments per second
 *    A = Alpha (Tower 1) diagonal rod trim
 *    B = Beta (Tower 2) diagonal rod trim
 *    C = Gamma (Tower 3) diagonal rod trim
 */
inline void gcode_M665() {
  if (code_seen('L')) delta_diagonal_rod = code_value();
  if (code_seen('R')) delta_radius = code_value();
  if (code_seen('S')) delta_segments_per_second = code_value();
  if (code_seen('A')) delta_diagonal_rod_trim_tower_1 = code_value();
  if (code_seen('B')) delta_diagonal_rod_trim_tower_2 = code_value();
  if (code_seen('C')) delta_diagonal_rod_trim_tower_3 = code_value();
  recalc_delta_settings(delta_radius, delta_diagonal_rod);
}
superlou commented 8 years ago

@thinkyhead That looks really helpful, but I'm not sure how to set those? The code is in Marlin/Marlin_main.cpp, but is there a way to configure those values in configuration.h? Are they only settable by sending the M665 code?

Roxy-3D commented 8 years ago

In Marlin_Main.cpp, the following delta configuration numbers are used:

  float delta_radius = DELTA_RADIUS;
  float delta_tower1_x = -SIN_60 * (delta_radius + DELTA_RADIUS_TRIM_TOWER_1); // front left tower
  float delta_tower1_y = -COS_60 * (delta_radius + DELTA_RADIUS_TRIM_TOWER_1);
  float delta_tower2_x =  SIN_60 * (delta_radius + DELTA_RADIUS_TRIM_TOWER_2); // front right tower
  float delta_tower2_y = -COS_60 * (delta_radius + DELTA_RADIUS_TRIM_TOWER_2);
  float delta_tower3_x = 0;                                                    // back middle tower
  float delta_tower3_y = (delta_radius + DELTA_RADIUS_TRIM_TOWER_3);
  float delta_diagonal_rod = DELTA_DIAGONAL_ROD;
  float delta_diagonal_rod_trim_tower_1 = DELTA_DIAGONAL_ROD_TRIM_TOWER_1;
  float delta_diagonal_rod_trim_tower_2 = DELTA_DIAGONAL_ROD_TRIM_TOWER_2;
  float delta_diagonal_rod_trim_tower_3 = DELTA_DIAGONAL_ROD_TRIM_TOWER_3;

You can set these in your source code and then you don't need to set them in M665. And in fact, by setting them in the source code, you get extra control that is not in M665 yet. You get the ability to control the DELTA_RADIUS_TRIM for each tower.

For some (bad) reason, these values are currently found in Marlin.h and not in Configuration.h

BuzzBumbleBee commented 8 years ago

Sorry for the slow response (have been away from 3D printing a while)

If you look at

https://github.com/MarlinFirmware/Marlin/blob/e8f8a46ef51dcf3045c7270f29868a6174186d18/Marlin/Marlin.h#L279

The DELTA_DIAGONAL_ROD_TRIMTOWER* definitions are defaulted there so as not to cause issues with devices that do not define them in their configuration. You can set them in your configuration file (and as long as you have no values stored on eeprom) they should work fine

#define DELTA_DIAGONAL_ROD_TRIM_TOWER_1 1.0
#define DELTA_DIAGONAL_ROD_TRIM_TOWER_2 1.0
#define DELTA_DIAGONAL_ROD_TRIM_TOWER_3 1.0
superlou commented 8 years ago

Thanks for the quick responses! I'm going to try these this evening. I'm assuming the diagonal rod trims assume that each pair of rods is at least matched? I'm going to take apart the system tonight to more accurately measure the rods and hopefully pair them up more closely.

BuzzBumbleBee commented 8 years ago

@superlou

The way i calibrated my printer was to print a 60mm circle 5mm high. Measure the actual dimension (in line with the tower) and calculate the new lengths.

For example across DELTA_DIAGONAL_ROD_TRIM_TOWER_1

Target size = 60mm
Real size = 62mm
Current rod length = 120mm

120 * (60/62) = 116.13

New rod length = 116.13 (120-3.87)

DELTA_DIAGONAL_ROD_TRIM_TOWER_1 = -3.87

Repeat for all towers, you should have pretty much all errors due to rod lengths fixed :)

superlou commented 8 years ago

That sounds like a cool trick. Will give it a shot tonight. My initial problem was leveling the bed by 3 points on a circle (one in line with each tower) and a middle point. I can get all 3 radial points equal, but when I try to correct for bowling/doming differences between the middle and the radial points, the radials would no longer match each other. I had (maybe incorrectly) assumed this had something to do with my rod inaccuracies preventing the corrections from being independent. However, I'm getting a bit off topic from the issue topic. :)

superlou commented 8 years ago

After a bit of futile attempts, I just realized, is this trim functionality only available in the RC and not the latest stable?

thinkyhead commented 8 years ago

Correct. Version 1.0.2-1 doesn't have the complete feature. We only added the ABC parameters to M665 recently.

superlou commented 8 years ago

Would it be testable using the recent RC5, or should I wait until RC6 lands to make sure I'm giving it a fair shot?

Edit: so I tried the rod trim corrections in RC5. @BuzzBumbleBee, is there any chance either the target-to-real ratio is inverted or the sign is inverted on the trim? My measurements got worse following the given equations, but better (I think, print bottom layer wasn't great) when I inverted the ratio.

superlou commented 8 years ago

@jbrazio Are you planning to remove this feature in future versions, or holding off on further development?

jbrazio commented 8 years ago

I'm not sure if I understood your question fully.. Features present in RCx/RCBugfix will be part of the next stable release (1.1.0), we have no plans to back port features into 1.0.x for instance.

Roxy-3D commented 8 years ago

And there has been talk about people needing Delta Tower Angle Trim. Some people have towers that are positioned a few degrees off of the 120 degree spacing. That would show up first on the MarlinDev side. And almost for sure it would be an addition to the M665's A,B,C and I,J,K (Delta Rod Trim and Delta Radius Trim).

superlou commented 8 years ago

@jbrazio Sounds good. I think I misinterpreted that Feature Requests milestone. Having that in the 1.1.0 release is fine.

@Roxy-3DPrintBoard I wouldn't be opposed to the trim angle, as my towers are a little sloppy. Some of the calibration wizards give corrections for tower angle, so it would be nice to be able to use those easily.

boelle commented 8 years ago

should this not be closed with remark that the feature should be submitted as a PR from the OP ?

jbrazio commented 8 years ago

Submitted as a Pull Request ? PR is for merging code.

superlou commented 8 years ago

I believe the feature's already in and working, though a comment on how to use it with respect to +/- would be helpful.

boelle commented 8 years ago

yep OP should submit a PR with the code needed for the feature

but if its allreadt implemented we can close this one and write some documentation on it instead

jbrazio commented 8 years ago

I believe it's documented on the delta example files if memory is not playing a trick on me.

boelle commented 8 years ago

great we can close this one then

jbrazio commented 8 years ago

No I was wrong, there is absolutely no documentation about this anywhere, @BuzzBumbleBee did the implementation but no documentation.

boelle commented 8 years ago

heheh then we have a new requirement for PR's... features also need to be documented :-P