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

🩹 Fix LINEAR_ADVANCE/LIN_ADVANCE typo #27110

Merged
merged 1 commit into from
Oct 8, 2024

Conversation

vovodroid
Copy link
Contributor

Fix usage of wrong LINEAR_ADVANCE define, it's LIN_ADVANCE in all Marlin code.

@thisiskeithb thisiskeithb changed the title Use LIN_ADVANCE, not LINEAR_ADVANCE 🩹 Fix LINEAR_ADVANCE/LIN_ADVANCE typo May 20, 2024
@ellensp
Copy link
Contributor

ellensp commented May 27, 2024

The associated error message doesn't quite make sense. (I know you didn't change this, it was changed in 23e2cb0)

The original error was #error "LINEAR_ADVANCE currently requires NUM_AXES <= 3." which makes a lot more sense than "LIN_ADVANCE does not currently support the inclusion of an I axis."

The test for HAS_I_AXIS seems to have been used just to see if NUM_AXES is > 3. I being the 4th axis

Perhaps this could be updated to be clearer with the following

#elif ENABLED(LIN_ADVANCE) && NUM_AXES > XYZ
  #error "LIN_ADVANCE currently requires NUM_AXES <= 3."

But this raises the question why? From what I know (and I'm not a expert in LIN_ADVANCE) it only effects the E axis,
Ie it is rapid forward and back moves on E to reduce nozzle pressure... So why would that care if there a 3 or 9 axis?

@DerAndere1 Could you comment here? Since you added this check back in #23112

@ellensp ellensp added the Needs: More Data We need more data in order to proceed label May 27, 2024
@DerAndere1
Copy link
Contributor

The problem is that I dit not extened the LIN_ADVANCE code in function planner._poulate_block to more axes. The offending section starts here:

float e_D_ratio = (target_float.e - position_float.e) /

I do not know what the code should do (are calculations supposed to be done in cartesian space or in joints-space AKA stepper space? i.e. do we look for distances along axes in a cartesian space or are the steps of each joint/motor relevant?) As long as this is unclear, the fix of the sanity check as proposed in this PR looks good to me.

@ellensp ellensp removed the Needs: More Data We need more data in order to proceed label May 29, 2024
@thinkyhead thinkyhead force-pushed the bugfix-2.1.x branch 3 times, most recently from 37d77d6 to aa44542 Compare September 28, 2024 01:10
@thinkyhead thinkyhead merged commit cebed34 into MarlinFirmware:bugfix-2.1.x Oct 8, 2024
62 checks passed
@vovodroid vovodroid deleted the lin_advince_pr branch October 10, 2024 06:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants