celogeek / OctoPrint-ProgressBasedOnTime

Replace file based progression by time based progression
3 stars 0 forks source link

When enabling this plugin the Gcode-viewer looses sync #1

Open dajosco opened 5 years ago

dajosco commented 5 years ago

When I enable this plugin the Gcode-viewer seems to be synchronized by time and not by the actual layer or Gcode instruction and therefore goes faster than what the printer is actually doing (up to 5 layers ahead!). I don't know if this is a bug from this plugin or most likely from the G-Code viewer itself. Just wanted to bring this up. When I had this issue, installed four plugins at the same time, so I started to disable one by one to find which one was causing the issue as before this the G-Code viewer was working great. Once I found this plugin was the cause, I enabled all the plugins and confirmed by enabling and disabling only this one again. My setup is:

Let me know if you need any other info. I open to any suggestions.

Thanks!

celogeek commented 5 years ago

I have also seen some issues with the filament manager. I need to figure out how they use the "completion" information. For the OctoPrint part, I may also include a fix for the viewer.

Thanks for the report.

celogeek commented 5 years ago

Ok found it. It uses the file progression by using the "completion" key. This one is fixed by using time progress with my module. that's why it is out of sync.

It is not very easy to fix it.

eyal0 commented 4 years ago

Maybe what you can do is instead of changing the completion here:

https://github.com/celogeek/OctoPrint-ProgressBasedOnTime/blob/master/octoprint_ProgressBasedOnTime/__init__.py#L64

To use time-based completion, just return the regular completion but also return another time-based completion. And then modify the web interface to use the time-based completion by writing some javascript to replace the part of the interface that saves the completion:

https://github.com/foosel/OctoPrint/blob/8409fdaa48ff692bbb050086e8d7c09940b58c59/src/octoprint/static/js/app/viewmodels/printerstate.js#L291

You would need to add a new ko.observable, maybe called time_progress. Then you adjust the display to use the new observable:

https://github.com/foosel/OctoPrint/blob/8409fdaa48ff692bbb050086e8d7c09940b58c59/src/octoprint/static/js/app/viewmodels/printerstate.js#L155

You need to add the printer state view model here:

https://github.com/foosel/OctoPrint/blob/8409fdaa48ff692bbb050086e8d7c09940b58c59/src/octoprint/static/js/app/viewmodels/printerstate.js#L155

And then put a line like this:

self.printerStateViewModel = parameters[0];

here:

https://github.com/celogeek/OctoPrint-ProgressBasedOnTime/blob/master/octoprint_ProgressBasedOnTime/static/js/ProgressBasedOnTime.js#L12

Then you put:

self.original_processProgressData = self.printerStateViewModel._processProgressData // Save the old one.
self.printerStateViewModel._processProgressData = function(data) {
      self.original_processProgressData(data); // Call the old one then adjust.
      if (data.timeBasedProgress) {
        self.timeBasedProgress(data.timeBasedProgress);
      }
    };

Does that make sense? The goal is to not replace the original progress but to add a new member in the dict and then modify the javascript to use that new one. The downside is that other tools that using the progress will still get the file-based one, so tools that, for example, look at the progress and send things to display on the screen of the printer, will use the other progress. But it would fix the issue here.

eyal0 commented 4 years ago

Also, in general, I think that it's a nice idea to add to the dict instead of modifying it. In fact, it would be nice if the new _updateProgressDataCallback called the original one and then modified it, rather than replacing it. That way if foosel modifies the source you'll get those changes. And if there are two plugins that are both modifying that same function, they will stack up and run in order instead of just the most recent one overriding the changes the the previous plugins made.

celogeek commented 4 years ago

The goal of this plugin is to change the completion value in order to have every other module who use this to display a progress based on time left. More specially the iOS or Android apps.

PrintTimeGenius already change the progress time bar in the interface to match this.

Also I can't apply change after the original function because I need to change the behavior of the function at different point in it.

The only change I may try is to ack the gcode viewer.

A best way to do this would be to add a patch to OctoPrint directly by adding a time_completion field and inform the developers to use it for time left result. And use completion for file left result.

This plugin unfortunately may break some app/plugin who use completion to sync gcode rendering.

I try to solve it by changing the inner behavior and call hook with file completion information ( use by most of the app).

Some of then use the result like gcide viewer instead and then add issues.

I’m not sure it is possible to patch this without breaking anything and achieve the expected result.

eyal0 commented 4 years ago

I think that it should still be possible. What if you wrote this in the startup:

def newUpdateProgressDataCallback(old_callback):
      def return_function():
        old_result = dict(old_callback()) # Call the original
        old_result["progress"] = (old_result["printTime"] or 0) / ((old_result["printTime"] or 0) + (old_result["printTimeLeft"] or 0)) # Modify it.
        return old_result # Done.
      return return_function

    def on_startup(self, host, port):
        self._printer._stateMonitor._on_get_progress = types.MethodType(newUpdateProgressDataCallback(
        self._printer._stateMonitor._on_get_progress), self._printer._stateMonitor)

This way the new self._printer._stateMonitor._on_get_progress calls the original one. And then it modifies the output of the original one. Maybe there are some more cases in there to deal with like 0 or None progress but you get the idea...

I ask because I am about to clobber the same function and soon we will have incompatible plugins, where each one is trying to rewrite the same function. If it is written this way, however, each function will call the previous one so they will be additive.

What do you think?

celogeek commented 4 years ago

Yeah, that would be safer to do it this way. I need to dig in to test it. Would it be possible to do a PR about this?

celogeek commented 4 years ago

I have update the plugin for python3, with your less intrusive suggestion.

Now I have to fix gcodeviewer to use event.data.file_completion instead of event.data.completion

If you have a patch of it, it would be wonderful. I will try to see how to change the gcodeviewer behavior about this on init.

codingcatgirl commented 4 years ago

Hi,i would greatly appreciate if this issue was fixed because it sadly makes the plugin unusable for me, especially in combination with the cancel objects plugin. :)

celogeek commented 4 years ago

Hi,

Sorry for the terrible delay.

Ok, I take a look and I try a patch.

Can you validate that works fine now?

The plugin PrintTimeGenius already change it to time based on the progress bar.

Test the master branch directly, I haven't released it yet. Just want to be sure it is ok first.

https://github.com/celogeek/OctoPrint-ProgressBasedOnTime/archive/master.zip

codingcatgirl commented 4 years ago

Hey, sorry for the late response. Do i see correctly that the current version contains this patch already, becasue i saw an update.

Sadly, i enabled the plugin again and i'm afraid the gcode viewer still shows tons of the first layer printed while the printer is only heating =(

celogeek commented 4 years ago

Does it happen only during the heating and then it works?

Or is it still out of sync?

Do you have any errors in the console of your browser when you open the interface? When you start printing?

codingcatgirl commented 4 years ago

I only tried it during the heating. But it basically already assumed i was way further into the print than it actually was.

But i am kinda starting to see the actual problem here. I keep comparing your plugin to the progress bar on my printer… but that's not how it works. The time progress/remaining is calculated (in my case) from M73 commands. Those provide a percentage AND a time remaining. The former is used for the progress bar on the printer.

Thinking about this, i might also have a solution for the problem though? And i apologize in advance if this is already the approach you're using. Would it be possible that you calculate the percentage not as time_passed/total_time but instead as 100-(time_remaining/total_time). In most cases the time remaining and the total time will be calculated by the same module, meaning that in this way you will get the true percentage and even if the printer keeps heating forever, the progress would still stay at 0 until it makes progress.

celogeek commented 4 years ago

My change here was to restore the original progressData for javascript context. So if you disable the plugin and try like that, it should work the same way for gcode viewer.

Some plugin like printTimeGenius change this behavior on the fly for the progress bar.

In another context (like an ios/android app that pulls the changes through the API), the "complete" progress data is replaced by the time base context. This time base is computed using what we have in the remaining time, calculated by the plugin or not you have that does this job.

let me checks the formula I have put there.

celogeek commented 4 years ago

The formula I use is:

time_completion = float(result['printTime']) * 100.00 / float(result['printTime'] + result['printTimeLeft'])

So the printTime elapse compared to the total time estimated (printTime + printTimeLeft).

I think this formula is correct. right?

celogeek commented 4 years ago

Let's try to compare the formula.

100-(time_remaining/total_time) vs printTime*100/(printTime+printTimeLeft)

Let say the job should take 100min, and you are at 5min of heating.

So: You formula should give: 100 - (100/(100 + 5)) = 99.04 My formula give: 5 * 100 / (100 + 5) = 4,76

is it correct?

codingcatgirl commented 4 years ago

Sounds pretty much right to me. And yeah, duh, your solution of restoring the value for Javascript sounds much better, my bad. I wonder why it doesn't work, cause it sounds like it should?

celogeek commented 4 years ago

I don't know. Can you capture what happened during the process? Can you see if anything wrong on the console of your browser? Can you try only with this plugin on to be sure they are no conflict with other plugins?

celogeek commented 3 years ago

I have seen in my test that a bunch of gcode is sent during the heating phase. It is due to the size of the buffer the printer can handle.

And in that case, the completion goes on. So gviewer doesn't show exactly what is actually printed but more that has been sent to the printer already.

does it make sense?

codingcatgirl commented 3 years ago

Btw, i just set up octoprint again and the problem sadly still persists =(

luizbgomide commented 2 years ago

Is that fixed?

celogeek commented 2 years ago

I don't know. I was expecting some logs to figure out.

I haven't use Octoprint for a while.

Do you have issue with gcode viewer and this plugin? Does it works now?

Can you try it?

luizbgomide commented 2 years ago

I have tried and the Gcode viewer was very out of sync, I had to disable this plugin since I use the gcode viewer a lot to monitor my prints.