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

Error when connecting a printer that is printing #1805

Closed
brunoaandrade opened this issue Mar 6, 2017 · 7 comments

Comments

Projects
None yet
4 participants
@brunoaandrade
Copy link

commented Mar 6, 2017

What were you doing?

Error in statistics when i connect a printer that is already printing.
The problem is when Octoprint tries to calculate statistics, there are no information's about when the printing start.
in file "printer/standard.py" function "def on_comm_print_job_done" makes a call to function to log_print in filemanager/init.py where the print_time is passed as an argument with value None.
Next, when "_add_history" and then "_calculate_stats_from_history" in file filemanager/storage.py, the variable metadata while use the value None to calculate the "averagePrintTime" and a exception occurs.

Branch & Commit or Version of OctoPrint

I'm on Version 1.3.1 (master branch).

Link to contents of terminal tab or serial.log

http://pastebin.com/u723DVPt

@foosel

This comment has been minimized.

Copy link
Owner

commented Mar 6, 2017

So this is not a request but actually a bug report, correct?

In that case please describe what you mean with "connecting to a printer that is already printing", because I'm not entirely sure how you'd even do that. Is the printer printing from its own SD?

Also please never truncate the logs!

@brunoaandrade

This comment has been minimized.

Copy link
Author

commented Mar 6, 2017

Yes, it's a bug. I start by printing a gcode file. During the process, when it is already printing, I restart the software. The printer connects to software and no problems there. When the printing job finishes a error occurred "TypeError: unsupported operand type(s) for +: 'float' and 'NoneType'", because when trying to calculate statistics, by doing sum(former_print_times[printer_profile]), the last print has the value None ( because print_time can't be calculated, no information about when the printer job start is or can be provided)

Link to log: http://pastebin.com/ZZDESc9h

@brunoaandrade brunoaandrade changed the title [Request] Error when connecting a printer that is printing Error when connecting a printer that is printing Mar 6, 2017

foosel added a commit that referenced this issue Mar 6, 2017

@foosel

This comment has been minimized.

Copy link
Owner

commented Mar 6, 2017

Starting BEEweb 0.7.6+0.g0e08566.dirty (master branch)

This is not an official version of OctoPrint and it's impossible for me to see what version this fork was based off. When reporting bugs here, please always reproduce against an official version of OctoPrint.

During the process, when it is already printing, I restart the software.

This part does only make sense if you were printing from SD, because if you were streaming the file, restarting the OctoPrint server would interrupt the whole print job.

OctoPrint itself does not even trigger on_comm_print_done when connecting to a printer that is already printing from SD and said print finishes - this line prevents that from happening.

Your stack trace indicates that the call causing the issue is triggered from a plugin on your end which apparently takes care of the whole printer communication and fully replaces OctoPrint's own communication layer. You are signaling on_comm_print_done but apparently your custom comm layer is not providing all required information for this to be properly processed. The only way your error can trigger is if your custom comm layer returns None on the call of self._comm.getPrintTime() in the printer implementation - or did so in the past, causing invalid values to be recorded to your metadata. So I suggest you fix that.

In the meantime, I've made that particular bit in OctoPrint's file manager a bit more resilient against invalid print time values being logged by plugins, see the above commit. Pushed to maintenance, soon devel, will be part of 1.3.2.

But just to reiterate: The cause of the exception you are seeing is your (self implemented) communication layer returning invalid data in the handler for a call it triggers itself - this needs to be fixed on your end.

@foosel foosel added this to the 1.3.2 milestone Mar 6, 2017

@foosel

This comment has been minimized.

Copy link
Owner

commented Mar 6, 2017

As a follow-up, after taking another look at your stack trace, it also looks like your replacement comm layer doesn't even come from a plugin but you are running in full fork mode here. If you don't want to drown in merge conflicts down the road and encounter even more issues like the above due to replacing bits and pieces but not having them behave as expected by OctoPrint, I strongly suggest implementing your custom printer & comm layer via the available plugin adjustment points, e.g. this hook for replacing the printer instance to use.

Another reason for that recommendation is that I cannot provide support for inofficial versions - I simply have no way to see what you changed and diving through that each time in order to figure out if a bug is located in OctoPrint's official version too or introduced by your changes is simply not possible, so if you continue down this road, you are making it impossible to help you with any hurdles you encounter.

The plugin system is there to be used for changes without the above mentioned downsides. Please use it instead of modifying core code and losing maintainability and supportability. If something is missing from core OctoPrint that you need to achieve your goals, feel free to open a feature request for such an addition to OctoPrint's plugin system so we can talk through what you are missing to get around having to maintain a fork.

@ntoff

This comment has been minimized.

Copy link
Contributor

commented Mar 7, 2017

This is not an official version of OctoPrint and it's impossible for me to see what version this fork was based off. When reporting bugs here, please always reproduce against an official version of OctoPrint.

@foosel oh dear: https://github.com/beeverycreative/BEEweb what terrible issues this is going to create

@foosel

This comment has been minimized.

Copy link
Owner

commented Mar 7, 2017

@ntoff That's pretty much what I feared with regards to a maintenance nightmare. I mean, it's totally fine to fork, but if you want to stay compatible to the fork base it produces an endless stream of merge conflicts and hard to track down bugs. You loose so much time with stuff like that that could be better spent on actually maintaining and improving your own additions. It doesn't scale. Hence: plugins.

@foosel

This comment has been minimized.

Copy link
Owner

commented Mar 16, 2017

1.3.2 was just released.

@foosel foosel closed this Mar 16, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.