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

Reprint after print not possible and ends in a runtime error, see Issue #1421 #1454

Merged
merged 5 commits into from
Oct 15, 2024

Conversation

DivingDuck
Copy link
Collaborator

@DivingDuck DivingDuck commented Oct 12, 2024

For reference pls. see issue #1421

I found in my firmware test 3 different behaviors to newer Marlin versions. So we have now 3 variants:

  • Prusa firmware >3.12.x wants M110 N-1 for resetting the gcode line number in the printer
  • Smoothieware wants N-1 M110
  • Newer Marlin firmware accept both, N-1 M110 and M110 N-1
  • Old Marlin firmware 1.0.0 ... x wants N-1 M110 + line checksum

My solution for this is adding back the old behavior and combine this with the actual implemented Prusa modification:
self._send("M110 N-1", -1, True). This ends in sending N-1 M110 N-1*125 and both, the old firmware from Marlin and for Smoothieware are happy with the gcode.

Up to now there is no tester available for this solution with an actual Pusa firmware.

The pull request includes in addition 2 little gcode test files:
quicktest.gcode and quicktest.dfx for testing repeated prints
arc_test.gcode for testing the printer capability of arc movements in xyz axis
Both gcode files needs only a half minute for testing and are located in the folder testfiles. Let me know if I should delete them

@DivingDuck DivingDuck changed the title Reprint after print not possible and ends in a runtime error, see Issue 1421 Reprint after print not possible and ends in a runtime error, see Issue #1421 Oct 12, 2024
Copy link
Collaborator

@rockstorm101 rockstorm101 left a comment

Choose a reason for hiding this comment

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

Hi @DivingDuck, I'm happy with the new test files and the code change but for the comment below. Are you willing to introduce the change now or should we wait to test it?

@@ -416,7 +416,16 @@ def startprint(self, gcode, startindex = 0):
return True

self.clear = False
self._send("M110 N-1")

'''Reset Gcode line number for printer, pls. see issue #1421.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This information if very useful but I would not clutter the code with it. I believe this kind of explanation belongs to the commit message that introduces the code change, not in the code itself. If you know what I mean. Plus you wouldn't need to repeat it twice. I'm happy with the change otherwise.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No problem, I removed the comments.

@rockstorm101
Copy link
Collaborator

@DivingDuck let me know if you think we should wait for the solution to be tested on Prusa's firmware. I expect it to work well but you never know.

@DivingDuck
Copy link
Collaborator Author

I wish I have a Prusa for testing on my hand, but I don't.
Let us wait this week whether we get a response on our request in #1421. We should merge it end of the next week w/o further tests if not.

@DivingDuck
Copy link
Collaborator Author

@rockstorm101,
this one is ready to merge. See latest comments of #1421

Maybe we should think about a new release that incorporate all updates we did since 2.1.0?

@rockstorm101
Copy link
Collaborator

Maybe we should think about a new release that incorporate all updates we did since 2.1.0?

Yeah, absolutely, since this change fixes a regression I think we must release at least the fix. I'd say we do a minor release with all updates (i.e. go for a 2.2.0 release). What do you think?

@DivingDuck
Copy link
Collaborator Author

We have a plan - sounds good to me 🙂

@rockstorm101 rockstorm101 merged commit 991fa18 into kliment:master Oct 15, 2024
17 checks 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