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

[BFW-5923] Make serial printing screen a menu option (Implements #4070) #4071

Merged

Conversation

PinkPandaKatie
Copy link
Contributor

This adds a toggle menu option under User Interface to enable/disable the serial printing screen.

Copy link
Member

@danopernis danopernis left a comment

Choose a reason for hiding this comment

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

Hi @PinkPandaKatie thanks for the contribution.

Since we recently changed the master branch, I have taken a liberty of rebasing your PR and resolving conflicts.

Overall I quite like the change and find it useful for expert users. IIRC the serial printing screen exists to prevent interaction with the printer via the LCD. Interactions may lead to failed prints, akin to inserting random G-codes to the instruction stream. I would be happy to merge this if you moved the menu item to the "Experimental Settings" menu which is more difficult for regular users to access. This way I wouldn't need to go through the product department to get their approval. What do you think?

Rant: if only there was a "Start printing" and "End printing" G-code which would enable us to clearly differentiate between various printer states instead of using heuristics like "Is it a movement G-code?"

@PinkPandaKatie
Copy link
Contributor Author

I definitely agree with needing "start" and "end" G-codes - I went looking for them before even starting on this PR. I am fine moving it to Experimental settings.

I am curious about one thing, though - Is there a way to enable the experimental / dev settings (i.e. the green menu items) on stock firmware?

@PinkPandaKatie PinkPandaKatie force-pushed the serial-printing-menu-option branch from 5ccb767 to 66b9c3f Compare August 14, 2024 23:43
@PinkPandaKatie
Copy link
Contributor Author

Ok, I think I got it moved. I did do a local build, but I haven't had a chance to test it because I'm in the middle of a long print right now.

@danopernis danopernis changed the title Make serial printing screen a menu option (Implements #4070) [BFW-5923] Make serial printing screen a menu option (Implements #4070) Aug 15, 2024
Copy link
Member

@danopernis danopernis left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Regarding your question about development (green) menu items: no, you can't enable them on the production firmware. There is a CMake option DEVELOPMENT_ITEMS_ENABLED which translates to a compile-time constant GuiDefaults::ShowDevelopmentTools which in turn controls if the menu item is displayed.

@danopernis danopernis merged commit 132592a into prusa3d:master Aug 15, 2024
1 check passed
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