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

IFNDEF more code that is releated to Filament Autoload #1

Conversation

3d-gussner
Copy link

No description provided.

Firmware/ultralcd.cpp Outdated Show resolved Hide resolved
Copy link

@gudnimg gudnimg left a comment

Choose a reason for hiding this comment

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

We shouldn't be adding ifdefs around setAutoLoadEnabled and getAutoLoadEnabled because that will affect more than just the Autoload menu item we are looking to disable. The user should still be able to perform autoload by pushing filament into the fsensor.

Comment on lines +49 to +52
#ifndef REMOVE_AUTOLOAD_FILAMENT_MENU_ENTRY
void setAutoLoadEnabled(bool state, bool updateEEPROM = false);
bool getAutoLoadEnabled() const { return autoLoadEnabled; }
#endif //NOT REMOVE_AUTOLOAD_FILAMENT_MENU_ENTRY
Copy link

Choose a reason for hiding this comment

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

I don't think this is the correct approach, you're disabling too much. These functions are not just used for the Autoload menu item. It's used by triggerFilamentInserted() too for normal autoload functionality.

Copy link
Author

Choose a reason for hiding this comment

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

Actually these shouldn't be used when autoload is disabled. There in the 3rd commit I removed even more code and tested it with MK404

  • MK3S
  • MK3S + MMU
  • MK2.5S

@3d-gussner
Copy link
Author

Nevertheless I will close this PR some of the autoload features are actually nice to have even when not active.

Gonna keep the branch.

@3d-gussner 3d-gussner closed this Dec 31, 2024
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.

2 participants