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

remove hardcoded SHAPING_FREQ_X from SHAPING_MENU #27499

Open
wants to merge 2 commits into
base: bugfix-2.1.x
Choose a base branch
from

Conversation

ellensp
Copy link
Contributor

@ellensp ellensp commented Oct 26, 2024

Description

It was noticed in #27475 (comment)
That enabling SHAPING_MENU without INPUT_SHAPING_X (but with another axes enabled) causes a compile error

Marlin/src/lcd/menu/menu_advanced.cpp:571:92: error: 'SHAPING_FREQ_X' was not declared in this scope; did you mean 'SHAPING_FREQ_Y'?
  571 |           ACTION_ITEM_N(AXIS, MSG_SHAPING_ENABLE, []{ stepper.set_shaping_frequency(AXIS, (SHAPING_FREQ_X) ?: (SHAPING_MIN_FREQ)); ui.refresh(); });

This is due to the hardcoded use of SHAPING_FREQ_X in the macro SHAPING_MENU_FOR_AXIS

This macro is used to generate the shaping menu items for X,Y and Z
When shaping for the axis is enabled the menu shows the settings for SHAPING_FREQ and SHAPING_ZETA
You can also disable each axis.

This issue is when you enable a disabled axis it needs a default SHAPING_FREQ, it is currently hardcoded to use SHAPING_FREQ_X

This breaks if you don't have INPUT_SHAPING_X enabled.

I updated the Macro SHAPING_MENU_FOR_AXIS to use the correct SHAPING_FREQ_{X|Y|Z} for each axis, so SHAPING_FREQ_X is only used if you have INPUT_SHAPING_X

Requirements

SHAPING_MENU
with any combination of
INPUT_SHAPING_X
INPUT_SHAPING_Y
INPUT_SHAPING_Z

Benefits

Builds as expected

Configurations

Configuration that triggers the issue.zip

Related Issues

  • Marlin/issues/27475#issuecomment-2439653573

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

    1 participant