Skip to content

Replace untilted_stepper_position with planner.unapply_leveling, use in set_current_from_steppers_for_axis#4814

Merged
thinkyhead merged 1 commit intoMarlinFirmware:RCBugFixfrom
thinkyhead:rc_marlin_reforms
Sep 22, 2016
Merged

Replace untilted_stepper_position with planner.unapply_leveling, use in set_current_from_steppers_for_axis#4814
thinkyhead merged 1 commit intoMarlinFirmware:RCBugFixfrom
thinkyhead:rc_marlin_reforms

Conversation

@thinkyhead
Copy link
Member

@thinkyhead thinkyhead commented Sep 15, 2016

Like the title says…

Copy link

@jouirau jouirau left a comment

Choose a reason for hiding this comment

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

I am currently working on a SCARA robot that is not Morgan based (ie no Theta + Psi on the 'Y' arm)

I would argue that there are three types of arms in the SCARA family: Morgan, LeftArm, RightArm. The kinematics are almost identical - just the theta + psi change, and the direction of the stepper drives.

My comment on the latest update is that the older IS_SCARA test may be more representative of the family than ENABLED(MORGAN_SCARA) . Nearly all of the code works fine for all SCARA types with a few exceptions.

For example with setting the scaling factor in lcd_control_motion_menu() in ultralcd.cpp I think the IS_SCARA is better than ENABLED(MORGAN_SCARA), as the scaling factor applies to all the SCARA types not just the Morgan.

One place where it makes sense to me to check for MORGAN_SCARA is the Theta + Psi in the procedure inverse_kinematics():

#if MORGAN_SCARA
delta[B_AXIS] = DEGREES(THETA + PSI); // equal to sub arm angle (inverted motor)
#else // all other 'normal' SCARA
delta[B_AXIS] = DEGREES(PSI); // equal to sub arm angle (inverted motor)
#endif

Overall I am really liking the SCARA support in this firmware. Marlin has been working superbly on my SCARA machine (with a few minor tweaks) G2 & G3 also seem to work fine (more testing required). Your current efforts to make more sense of the logical versus physical position stuff are highly welcomed. Thank you!

@thinkyhead
Copy link
Member Author

thinkyhead commented Sep 15, 2016

@jouirau Can you describe what axis_scaling is all about? I'm writing the firmware for the MakerArm, and I haven't been planning to use it.

I do however have it working with both left and right arm kinematics, with the ability to switch between them.

@thinkyhead thinkyhead force-pushed the rc_marlin_reforms branch 2 times, most recently from fe05315 to 0b3713b Compare September 16, 2016 00:05
@jouirau
Copy link

jouirau commented Sep 16, 2016

@thinkyhead I have not used the scale factor yet. I assume that it is a way of tweaking the ratio of the stepper motor steps to the number of degrees moved on the fly. It does not seem to be that different in effect to tweaking the XStepsMM/YStepsMM. My immediate question is whether this is a logical or physical tweak. IE is it the scaling for the A and B arm steppers or is it the logical scale of the x and y axis? Do you know the intent of the original inclusion of these scale options?

At some point more descriptive menu items might be needed for SCARA. Thing like 'A steps/degree' instead of 'X steps/mm' and 'AScaleFactor' instead of 'XScaleFactor' to move away from the slightly misleading use of X and Y in the SCARA situation. (unless the XScale factor is actually meant to be the logical scale...)

The point I was trying to make was if there is a need for a 'scaling factor' then it should probably apply to all types of SCARA not just the Morgan.

More generally I am arguing that the IS_SCARA test or ENABLED(SCARA) should be used for most of the SCARA related stuff instead of ENABLED(MORGAN_SCARA) to reflect the overall similarities of the SCARA family. And then use ENABLED(MORGAN_SCARA) for the few very specific Morgan related differences.

@thinkyhead thinkyhead force-pushed the rc_marlin_reforms branch 3 times, most recently from bcf482c to d3cb864 Compare September 16, 2016 02:54
@thinkyhead
Copy link
Member Author

thinkyhead commented Sep 16, 2016

The axis_scaling factors are unnecessary, obsolete, and the idea is bankrupt. According to the source code comments, these exist to adjust the "build size." Well, you can't do that! If one wants to build something at a different size, one must scale it in the modeling software or in the slicer. Otherwise you just create gaps between the extruded lines.

The firmware cannot make something bigger or smaller merely by scaling the XY (cartesian) coordinates, which is what these factors attempt to do. If someone's custom build needs tweaking due to being scaled up from the Morgan (or other SCARA) build plans, it must be done through configuration.

If the idea is to scale for the sake of using a pen or paintbrush, well, again, that is not limited to SCARA. And again, it should be done in the slicer or Inkscape plugin export feature, not in the firmware.

@thinkyhead
Copy link
Member Author

thinkyhead commented Sep 16, 2016

More generally I am arguing that the IS_SCARA test or ENABLED(SCARA) should be used

The items enabled specifically for MORGAN_SCARA are things which do not apply to the only other current option, which is MAKERARM_SCARA.

I suppose it could be #if IS_SCARA && DISABLED(MAKERARM_SCARA) but for now I'm keeping it cleaner.

You'll note that I haven't added the kinematics and other MakerArm-specific changes. It's just hinting at things to come. I'm not entirely free to release the code just yet because the machine is still in development, but no one has this robot yet anyway.

@jouirau
Copy link

jouirau commented Sep 16, 2016

Good points! So in fact the two scale menu items can be deleted along with
the two related msg defines in language_en and any related scaling code.
Will you add this to your current cleanup, or would you like me to do it?

PS - impressed that you have both leftArm and rightArm working! I am
waiting on some hardware to arrive before I attempt that.

@thinkyhead
Copy link
Member Author

thinkyhead commented Sep 16, 2016

impressed that you have both leftArm and rightArm working!

If you have (or can download) OpenSCAD, then you should download my gist demonstrating Ambidextrous SCARA Kinematics. I note that the equations are similar —but not identical— to the kinematics from the SCARA code. I did a lot of messing around to arrive at the functions that worked best. Use the OpenSCAD "Animation" panel to see it in all of its circular glory.

@thinkyhead
Copy link
Member Author

thinkyhead commented Sep 16, 2016

the two scale menu items can be deleted

I'll do that in a separate PR after this one, which seems finally to be in good shape.
At least, it's passing Travis.

@thinkyhead thinkyhead force-pushed the rc_marlin_reforms branch 6 times, most recently from 55a19f3 to d70abeb Compare September 16, 2016 20:24
@thinkyhead thinkyhead mentioned this pull request Sep 17, 2016
@thinkyhead thinkyhead force-pushed the rc_marlin_reforms branch 4 times, most recently from 71dba72 to 83b45fb Compare September 18, 2016 16:13
@thinkyhead thinkyhead force-pushed the rc_marlin_reforms branch 8 times, most recently from c4f893a to 9060a4a Compare September 20, 2016 23:39
@thinkyhead thinkyhead changed the title Various reforms, cleanups to Marlin Replace untilted_stepper_position with planner.unapply_leveling, use in set_current_from_steppers_for_axis Sep 21, 2016
@thinkyhead thinkyhead merged commit cd75b89 into MarlinFirmware:RCBugFix Sep 22, 2016
@thinkyhead thinkyhead deleted the rc_marlin_reforms branch September 22, 2016 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants