Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
1 task done
mrmazakblu opened this issue Jun 20, 2023 · 19 comments
Closed
1 task done

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

mrmazakblu opened this issue Jun 20, 2023 · 19 comments

Comments

@mrmazakblu
Copy link

mrmazakblu commented Jun 20, 2023

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

  • A ZIP file containing your Configuration.h and Configuration_adv.h.

Additional information & file uploads

No response

@DerAndere1
Copy link
Contributor

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

@mrmazakblu
Copy link
Author

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
Copy link
Member

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
Copy link
Author

I read it backwards.

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

that line didn't mean what i thought?

@thisiskeithb
Copy link
Member

[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
Copy link
Contributor

DerAndere1 commented Jun 21, 2023

  • 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)

@mrmazakblu
Copy link
Author

  • 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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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

@DerAndere1
Copy link
Contributor

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
Copy link
Contributor

EvilGremlin commented Jun 22, 2023

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
Copy link
Contributor

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
Copy link
Contributor

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

@mrmazakblu
Copy link
Author

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
Copy link
Contributor

DerAndere1 commented Jun 22, 2023

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
Copy link

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.

Copy link

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.

@github-actions github-actions bot locked and limited conversation to collaborators Oct 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants