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

Rotary axis option will not compile[BUG] (bug summary) #26002

Closed mrmazakblu closed 1 year ago

mrmazakblu commented 1 year ago

Did you test the latest bugfix-2.1.x code?

Yes, and the problem still exists.

Bug Description

Tried to build rotary axis config in the latest (as of 6/20/23) bugfix 2.1.*, receive build errors. 3 standard Cartesian axis X,Y,Z plus 1 rotary axis.(I) -- (A)

error states that J,K,U,V,W are not defined.

I can build with additional axis if the extra axis is linear, but if enabled "axis_rotates" then the build still looks for all 9 possible axis to be named and defined.

In motion.cpp the additional axis section for linear axis is written with conditional define statements, but the rotary section is written so that is any exist then they all need to be defines.

Bug Timeline

No response

Expected behavior

I built with the 2.1.2.1 release and rotary worked, but for other reasons(probe_target build error) I needed to use bugfix.

Actual behavior

compile time error prevents successful build.

Steps to Reproduce

  1. download from marlin source
  2. download config examples
  3. load the config/examples/linear_axes/RAMPS 5 LINEAR_AXES config files
  4. Line 194 should have this

    ifdef I_DRIVER_TYPE

    define AXIS4_NAME 'U' // :['A', 'B', 'C', 'U', 'V', 'W']

    endif

    ifdef J_DRIVER_TYPE

    define AXIS5_NAME 'V' // :['A', 'B', 'C', 'U', 'V', 'W']

    endif

    ifdef K_DRIVER_TYPE

    define AXIS6_NAME 'C' // :['A', 'B', 'C', 'U', 'V', 'W']

    endif

Change it to this

ifdef I_DRIVER_TYPE

define AXIS4_NAME 'A' // :['A', 'B', 'C', 'U', 'V', 'W']

define AXIS4_ROTATES // THIS BREAKS THE BUILD

endif

ifdef J_DRIVER_TYPE

define AXIS5_NAME 'B' // :['A', 'B', 'C', 'U', 'V', 'W']

define AXIS5_ROTATES // THIS BREAKS THE BUILD

endif

ifdef K_DRIVER_TYPE

define AXIS6_NAME 'C' // :['A', 'B', 'C', 'U', 'V', 'W']

endif

Version of Marlin Firmware

Marlin-bugfix-2.1.x

Printer model

No response

Electronics

No response

Add-ons

No response

Bed Leveling

None

Your Slicer

None

Host Software

None

Don't forget to include

Additional information & file uploads

No response

DerAndere1 commented 1 year ago

This will be fixed once https://github.com/MarlinFirmware/Marlin/pull/24334 is merged. As that pull request changes a lot, it may never be merged.

I uploaded a simpler solution for testing at https://github.com/DerAndere1/Marlin/tree/rotational_axis_fix

The changes I applied there can be viewed here: https://github.com/MarlinFirmware/Marlin/compare/bugfix-2.1.x...DerAndere1:Marlin:rotational_axis_fix

Let me know if it works

mrmazakblu commented 1 year ago

trying now, but there is comment that shows 24334 is already merged in 3 weeks ago. ...... It did build with rotary axis. now ned to merge my actual machine specs in . make sure i dont have other issues.

So your alternate fix worked and the merge with 24334 did not., I had an issue with g38_probe_target not building, that was solved in bugfix_2.1.x(with 24334 merged in)

thisiskeithb commented 1 year ago

trying now, but there is comment that shows 24334 is already merged in 3 weeks ago.

PR #24334 is still open/not merged:

PR #24334
mrmazakblu commented 1 year ago

I read it backwards.

[Merge branch 'bugfix-2.1.x' into pr/24334]

that line didn't mean what i thought?

thisiskeithb commented 1 year ago

[Merge branch 'bugfix-2.1.x' into pr/24334]

This just means that bugfix-2.1.x is being merged into PR #24334 to keep it up to date until it is merged.

DerAndere1 commented 1 year ago
mrmazakblu commented 1 year ago
  • I confirm that my alternative solution compiles with AXIS4_ROTATES and G38_PROBE_TARGET both enabled
  • Please note that probing (G29, G38 etc) and any resulting compensation (bed leveling) is not supported while the extra axes are tilted. The user is responsible for ensuring that all extra axes are in neutral (zero) position so that the table is parallel to the XY-plane and the nozzle is parallel to the Z axis. I am open to discussion on how to improve that situation. I only have code for inverse kinematics for 5 axis machines. I guess we would also need forward kinematics but we cannot copy kinematics from LinuxCNC because of (LinuxCNC issue 2435)

Is this line missing a "+" before diff.i ?

https://github.com/DerAndere1/Marlin/blob/6cfa95af1f741c17bdd7ebf56a5a514f2a0ea4a0/Marlin/src/module/motion.cpp#LL1162C47-L1162C47

I noticed it in both your bugfix and main bugfix, but it is over my head to know if it is a typo or not. each of the lines for linear axis has a + , but diff.i for rotational does not.

DerAndere1 commented 1 year ago

AFAIK, it should be fine as is:

distance_sqr = ROTATIONAL_AXIS_GANG(sq(diff.i), + sq(diff.j), + sq(diff.k), + sq(diff.u), + sq(diff.v), + sq(diff.w));

is the same as

distance_sqr = ROTATIONAL_AXIS_GANG(+ sq(diff.i), + sq(diff.j), + sq(diff.k), + sq(diff.u), + sq(diff.v), + sq(diff.w));

The intention is documented in this comment: https://github.com/DerAndere1/Marlin/blob/6cfa95af1f741c17bdd7ebf56a5a514f2a0ea4a0/Marlin/src/module/motion.cpp#L1137-L1138

If you need different behaviour, try to enable option ARTICULATED_ROBOT_ARM. More info about this can be found in my Wiki at https://github.com/DerAndere1/Marlin/wiki or in this pull request: https://github.com/MarlinFirmware/MarlinDocumentation/pull/446/files#diff-e0ea0f997f69ea31cb3a6211432f1ae6fe10f5a46c12bd38770cc84081cf960d

mrmazakblu commented 1 year ago

This will be fixed once #24334 is merged. As that pull request changes a lot, it may never be merged.

I uploaded a simpler solution for testing at https://github.com/DerAndere1/Marlin/tree/rotational_axis_fix

The changes I applied there can be viewed here: bugfix-2.1.x...DerAndere1:Marlin:rotational_axis_fix

Let me know if it works

My sample builds worked. But when added the rest of my config variables it created new error. I am not able to enable an lcd display. build fails with error.

Compiling .pio\build\mega2560\src\src\lcd\menu\menu_spindle_laser.cpp.o Marlin\src\lcd\menu\menu_motion.cpp: In function 'void lcd_move_axis(AxisEnum)': Marlin\src\lcd\menu\menu_motion.cpp:86:16: error: 'class GCodeParser' has no member named 'axis_unit_factor'; did you mean 'axisunitsval'? if (parser.axis_unit_factor(axis) != 1.0f) { ^~~~ axisunitsval if (parser.axis_is_rotational(axis)) { ^~~~~~ *** [.pio\build\mega2560\src\src\lcd\menu\menu_motion.cpp.o] Error 1 ========================================================== [FAILED] Took 68.12 seconds ==========================================================

bugfix_rotation_fix-config.zip

DerAndere1 commented 1 year ago

I quickly fixed that in https://github.com/DerAndere1/Marlin/tree/rotational_axis_fix . I will not have the time to test the move menu for now. In case you want to try moving axes from lcd, be prepared for too long/fast/short/slow moves.

mrmazakblu commented 1 year ago

seemed to be correct on first run. will do more testing later. ty

DerAndere1 commented 1 year ago

The fun begins when moving rotational axes in inch mode. For testing that, you would have to enable INCH_MODE_SUPPORT and change to inch mode with G-code G20. In that case, distances and feedrate are expressed in inches for linear axes and in angular degrees for rotational axes. In fact I managed to perform a quick hardware test and it seems to work.

EvilGremlin commented 1 year ago

OMG why would you do inch mode? Can anyone name one valid reason for any printer firmware to support it at all? it's not like inch mode operate in true fractions, it's all exact same decimals.

DerAndere1 commented 1 year ago

I love the SI unit system too. especially since we got rid of the kilogram prototype in 2019 and all SI units are now based on 7 constants. but then there are the traditionalists using imperial units in machining

EvilGremlin commented 1 year ago

In metal machning it makes sense, but not in anything CNC-related. Just convert to metric in slicer/CAM.

mrmazakblu commented 1 year ago

I maybe missed something in the reply. In order to get rotary axis movements in degrees, I need to also be in inch mode?

Or is rotary displayed degrees either way

DerAndere1 commented 1 year ago

rotational axes are always in degrees, regardless of "inch mode" or "mm mode". It was just more difficult for me to make it work in inch mode, so more testing in inch mode is required before my solution can be merged into official Marlin.

github-actions[bot] commented 1 year ago

This issue has had no activity in the last 60 days. Please add a reply if you want to keep this issue active, otherwise it will be automatically closed within 10 days.

github-actions[bot] commented 10 months 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.