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

Delay on populate new bed target temperature #1543

Closed
derpicknicker1 opened this issue Oct 11, 2016 · 2 comments

Comments

Projects
None yet
2 participants
@derpicknicker1
Copy link
Contributor

commented Oct 11, 2016

What were you doing?

I'm working on a plugin, where i set the bed temperature by calling
self._printer.set_temperature('bed',70)
Immediately after that i grab the temperatures like this:
temps = self._printer.get_current_temperatures()

What did you expect to happen?

I expected that temps['bed']['target'] will have a value of 70 immediately after calling self._printer.set_temperature()

What happened instead?

self._printer.get_current_temperatures() will return the old value (value before set to 70) of ['bed]['target'] for about 2.5 seconds. After this time, the new value will be returned right.
Temperatures of the tools will have the correct target temperature immediately after setting them.

Branch & Commit or Version of OctoPrint

1.2.16 (master branch) on rPi 2 with octopi

Printer model & used firmware incl. version

Marlin 1.0.2 Printer and Virtual Printer (Virtual Printer also on fresh VM installation of 1.2.16 master)

Link to octoprint.log

https://gist.github.com/derpicknicker1/bb488bead50a15ee2cb04d673c8e824f

I have read the FAQ.

foosel added a commit that referenced this issue Oct 14, 2016

Fix target temperature propagation from comm layer
When setting the tracked target temperature from a sent temperature
command, the changes in tracked temperature were not propagated
from the comm layer to registered callbacks.

But since the standard printer also didn't make a copy of the mutable
dict of tool temperatures, those were in fact updated even without
propagation in the printer implementation when the values in the
comm layer got updated, whereas the bed temperature - an immutable
tupel - was not.

Two wrongs sometimes do in fact make a right. In this case that led
to target temperature changes on the tools immediately reflecting
in printer.get_current_temperatures after the command was sent,
but changes to the bed target taking until the next M105 response
to propagate.

Decoupling the data structures and adding propagation commands
to the comm layer solves this issue.

Fixes #1543
@foosel

This comment has been minimized.

Copy link
Owner

commented Oct 14, 2016

That was a prime example of two wrongs sometimes actually making a right.

As things were, when sending a temperature command to the printer and updating the tracked target internally, those changes were actually not forwarded to the printer instance until the next M105 response was parsed. The fact that the tool target temperature changes were reflected anyhow was caused by not properly isolating the data handed over from the communication layer - tool temperatures are in a (mutable) dictionary, one (actual, target) tuple per tool indexed by tool identifier, bed temperature is an (immutable) tuple. Updating the tool temperatures from the comm layer also updated the reference in the printer instance.

This has now been fixed by a) making a copy of the data handed over from the client instead of just using it as is in the printer and b) triggering a temperature propagation after adjusting target values from sent commands.

I've tested this using this minimal plugin based on your provided example.

Note that the target temperature still will not updated immediately but only once the update command has been sent to the printer.

Fix is merged on both maintenance and devel and will be part of any of the following releases and release candidates.

@foosel

This comment has been minimized.

Copy link
Owner

commented Nov 25, 2016

1.2.18rc1 is out

@foosel foosel closed this Nov 25, 2016

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.