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

Display Info on LCD #15866

Merged
merged 9 commits into from Nov 24, 2023
Merged

Display Info on LCD #15866

merged 9 commits into from Nov 24, 2023

Conversation

GregValiant
Copy link
Collaborator

@GregValiant GregValiant commented Jun 22, 2023

Description

DisplayInfoOnLCD combines DisplayFilenameAndLayerOnLCD with DisplayProgress. Those two post processors become options within the single post processor and the two previous files become obsolete. There is an option to add an M118 line as well as M117 so the info would get bounced back to a print server.
There is another pull request #11846 that involves "Display Progress on LCD" and BTREE firmware.
The Aug 12 commit adds "Countdown to Pauses" option when "Display Progress" is selected.
The Sep 8 commit adds an "Estimated Print Finish Time" message as an option per request #16677. #6019
Oct 20 commit adds a comment below the TIME: line and converts "seconds" to Housr:Minutes:Seconds with a pause and/or filament change count.
;TIME:14071
;Filament used: 13.3238m
;Layer height: 0.2
;Cura Time Estimate: 14071sec = 3hr 54min 31sec with 1 pause(s) and 1 filament change(s)

Type of change

  • [ X] Updated feature

How Has This Been Tested?

  • [ X] Test A - Cura 4.x and 5.x

Test Configuration:

  • Operating System: Win10 Home and Marlin

Checklist:

  • [ X] My code follows the style guidelines of this project as described in UltiMaker Meta and Cura QML best practices
  • [X ] I have read the Contribution guide
  • [X ] I have commented my code, particularly in hard-to-understand areas
  • [ X] I have uploaded any files required to test this change

@github-actions github-actions bot added the PR: Community Contribution 👑 Community Contribution PR's label Jun 22, 2023
These two files become obsolete if DisplayInfoOnLCD is accepted.
Remove code that added the PP name to the gcode.

Create DisplayInfoOnLCD.py

This post processor combines DisplayLayerAndFilename with DisplayProgress.  Those two post processors would be obsolete.
Added TimeToPause to DisplayProgress
Get the current print time estimate, adjust it by the Fudge Factor, and add it to the current time +10 minutes (to actually start the print.)

Update DisplayInfoOnLCD.py

Change message format for Finish Time Estimate.
Changed "name" variable to "file_name" to avoid any conficts.
Add a line below the ";TIME:" line to convert "seconds" to Hours:Minutes
@GregValiant
Copy link
Collaborator Author

I was researching something else and came across a fact I had completely missed - RepRap can use M566 for Jerk in mm/min.
This PR adds support for M566.

Copy link
Member

@rburema rburema left a comment

Choose a reason for hiding this comment

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

Hi Greg!

The big problem I have with this is that when you save a project, it saves which (post-processing) scripts where used as well. If it can't find them, it just ignores them -- which means that if the (now) removed scripts are added to a project, they'll no longer work, nor do they use the new script.

There's a number of ways to handle this.

  • Writing an upgrade-script would be the most proper, but also the hardest (and I'm not sure if we can do that for upgrade scripts -- though it should be possible). Maybe a bit overkill?
  • Leaving the scripts in, but messaging the user that the older scripts are deprecated.
  • Only leaving shells of the scripts, and only leave functionality in there to message the user.
  • Rewriting the scripts so that they inherit from the new one.
  • Not do anything (so reinstate them) and just have them live side-by-side until the next major version of Cura.
  • ... or I can ask the rest of the team if it's ok that we just rip these out and just communicate the change well.

plugins/PostProcessingPlugin/scripts/DisplayInfoOnLCD.py Outdated Show resolved Hide resolved
plugins/PostProcessingPlugin/scripts/DisplayInfoOnLCD.py Outdated Show resolved Hide resolved
@GregValiant
Copy link
Collaborator Author

GregValiant commented Oct 11, 2023

The project file situation is something I hadn't thought of.
The new script carries over "Display Filename and Layer on LCD" complete and I don't recall making changes to that script.
The second half of "Display Progress on LCD" is somewhat different with the time fudge factor, and Finish Time message, etc.

As a user I think I would prefer a quick message to inform me to move to the new post processor. If the script is left in then there is no incentive to move on and people will keep using it.

I've made changes to the two original scripts and turned them into "dummies" that just give messages. What is your preference in handling this?

In addition, I made some additions and made another commit. It will have the two changes you requested. It does not have the other two files in it.

Added Print Finish Time option.  Changed a couple of if statements to "line.startswith".
@GregValiant
Copy link
Collaborator Author

Remco,
I added the messages and exit code to the original scripts (DisplayFilename and DisplayProgress) and the messages point the user to DisplayInfo.

Add pause count notification

Update DisplayInfoOnLCD.py

Moved some line insertions to accommodate newer Creality firmware.

Change DisplayFIlename and DIsplayProgress

Add messages to use DIsplay Info and add exit code.

Update DisplayInfoOnLCD.py

Some changes
Copy link
Member

@rburema rburema left a comment

Choose a reason for hiding this comment

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

Hi Greg,
Sorry for the wait (again... it's been quite busy here).

I think we can accept this now. I have noticed that this doesn't 100% replace the other two scripts (the possibility to set M73 instead of M117/8 is missing, and the update-frequency can only be once per layer), but I think this adds enough (and the consolidation itself is also a good thing), that it's still a good improvement. -- So let's just wait to see if anyone complains about those little thing.

Thanks again for the contribution, and I'll try to take another look at the other PRs again next week. (That big list was/is useful.)

@rburema rburema merged commit 4f0840d into Ultimaker:main Nov 24, 2023
2 of 3 checks passed
@GregValiant GregValiant deleted the DisplayInfoOnLCD branch November 24, 2023 16:35
@lethuer lethuer mentioned this pull request Mar 30, 2024
@lethuer
Copy link

lethuer commented Mar 30, 2024

I've observed some bugs in the new plugin: DisplayInfoOnLCD.py
and noted my obervations here: #18766

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Community Contribution 👑 Community Contribution PR's
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants