PrusaOwners / Marlin

Optimized firmware for RepRap 3D printers based on the Arduino platform.
http://www.marlinfw.org/
GNU General Public License v3.0
15 stars 4 forks source link

Motocoder/pinda temp2 #5

Closed matthew-humphrey closed 5 years ago

matthew-humphrey commented 5 years ago
stahlfabrik commented 5 years ago

I reviewed the code in the Github web view. Looks great.

A minor issue is indentation at the first if statement (seenval("S")).

I do not have the chance to compile and run the code for the next 8 hours at least. So if you want, you can merge the request - I am confident it works fine.

I really have to digest your filtering code:-)

One question: I am unsure about the idle() call - do you also think it is the right thing to do? We could also call "manage_heaters()" and other things manually. But if idle() does all we want (and not unwanted stuff) we are fine.

matthew-humphrey commented 5 years ago

A minor issue is indentation at the first if statement (seenval("S")).

I just checked this, and it looks correct to me.

I do not have the chance to compile and run the code for the next 8 hours at least. So if you want, you can merge the request - I am confident it works fine.

I tested warm-up, cool-down, and timeout, as well as the filtering. But please also test when you get a chance.

One question: I am unsure about the idle() call - do you also think it is the right thing to do? We could also call "manage_heaters()" and other things manually. But if idle() does all we want (and not unwanted stuff) we are fine.

Yes, I went through gcode_M109 in detail and looked at what it was doing to make sure we weren't missing anything. This is how I found that we were missing the call to reset_stepper_timeout(). We're doing now the same thing that M109 is doing, namely calling idle() followed by reset_stepper_timeout() inside our while loop.