MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.24k stars 19.22k forks source link

read_max6675() improvements #169

Closed JTrantow closed 11 years ago

JTrantow commented 12 years ago

I'm not up to speed on using Git to feed improvements, but the following code should replace what's in temperature.cpp. The biggest change is the delay(1) between the slave select low and starting the SCLK has been removed. The MAX6675 only requires 100nsec and the atmel takes 610usec to put out the first clock so no additional delay is required. This knocks 1msec off the max time to execute this function (which is called within an isr.

define HEAT_INTERVAL 250

ifdef HEATER_0_USES_MAX6675

static long max6675_previous_millis = -HEAT_INTERVAL; // Initialize so first call will update the reading. int max6675_temp = 2000;

/*! Read the MAX6675 thermocouple using the SPI bus.

The MAX6675 performs cold-junction compensation and digitizes the signal from a type-K thermocouple. The data is output in a 12-bit resolution, SPI™-compatible, read-only format. This converter resolves temperatures to 0.25°C, allows readings as high as +1024°C, and exhibits thermocouple accuracy of 8LSBs for temperatures ranging from 0°C to +700°C.

This function is called every 8msec by the timer isr.

Relevant MAX6675 timing: CSB Fall to SCK Rise tCSS CL = 10pF 100 ns minimum. CSB Fall to Output Enable tDV CL = 10pF 100 ns maximum. CSB Rise to Output Disable tTR CL = 10pF 100 ns SCK Fall to Output Data Valid tDO CL = 10pF 100 ns _/ int readmax6675() { / See if it's time to update the temperature reading. _/ if (millis() - max6675_previous_millis < HEAT_INTERVAL) { return max6675temp; // Don't change the value. Just return. }else { / Time to take the reading. Reset the previousmillis. / max6675_previousmillis = millis(); } / The Power Reduction SPI bit, PRSPI must be written to zero to enable the SPI module. */

ifdef PRR

PRR &= ~(1<<PRSPI);

elif defined PRR0

PRR0 &= ~(1<<PRSPI);

endif

/* Initialize the /
SPCR = (1<<MSTR) | (1<<SPE) | (1<<SPR0); /
The SPI Master initiates the communication cycle when pulling low the Slave Select SS pin of the desired Slave. _/ WRITE(MAX6675_SS, 0); // Enable TT_MAX6675 Slave Select.

if (0)

/_
  I measured 610usec of delay between SS and SCLK without any delay on a 16Mhz Mega running a 1Mhz SPI.
  No delay should be needed to achieve the 100nsec delay required by the MAX6675.  But if you want delay use delayMicroseconds() rather than the original msec delay().
_/
delayMicroseconds(1);    // Ensure 100ns delay after slave select before clock starts. 

endif

/_ First bit from MAX is available within 100nsec of slave select going low and clock starting. All bits will be clocked in within 16sclk+100nsec. The SPI system is double buffered in the receive direction. This means that the first character must be read from the SPI Data Register(SPDR) before the next character has been completely shifted in. Otherwise, the first byte is lost. /
// Read MSB. SPDR = 0; // Starts the SPI clock generator. for (;(SPSR & (1<<SPIF)) == 0;); max6675_temp = SPDR<<8;

// Read LSB, we don't need to read the SPDR. SPDR = 0; for (;(SPSR & (1<<SPIF)) == 0;); max6675_temp |= SPDR;

WRITE(MAX6675_SS, 1); // Disable TT_MAX6675 slave select. / MAX6675 gets off the SPI bus within 100nsec of slave select going high. /

/ Check for Open Thermocouple condition. Bit D2 is normally low and goes high if the thermocouple input is open. In order to allow the operation of the open thermocouple detector, T- must be grounded. _/ if (max6675_temp & 4) { max6675temp = 2000; // thermocouple open }else { / Bottom three bits are open indicator, device id, three state bits.
Shift these bits out of the returned value.
/ max6675_temp >>=3; }

return max6675_temp; } // read_max6675().

endif

JTrantow commented 12 years ago

This may fix other problems with Marlin (using a TC with MAX6675)!!! I had several files that would lock up Marlin part way through the print. With this fix in place, I have printed 4/4 on prints that usually locked up.

korpx commented 12 years ago

The suggested patch solved my issues with silent lockups during printing as well. Every other print used to lock up but after applying the fix I have now printed a number of multi hour objects with no sign of lock up. Excellent! Please get this into the official code!

JTrantow commented 12 years ago

This solved my print problems and no side effects have shown up in many prints over the last two months. What needs to be done to get this into the trunk?

korpx commented 12 years ago

I have another MAX6675-issue. Don't know if it is related;

I use Marlin with PID autotuned.

I issue "M109 S230" (set extruder temperature to 230C and wait for it to be reached). In approximately 1 out of 5 times the target temperature will be overshot/undershot for a a while longer than normal (maybe 1-2 minutes of being close to target temp).

Suddenly the read temperature will drop 20-30 degrees in an instant, and the electronics start to compensate to reach target temp again. Target temp is reached and print begins. A couple of seconds into the print maxtemp is triggered by a temp of 20-30C higher than target and the print is halted.

JTrantow commented 12 years ago

I have seen temperature measurement changes of 20-30 C degrees which I attribute to noise picked up by the TC. I often see the temperature measurement go down when the extruder turns on. This occasionally shuts me down on min temp the first print of the day, but the short blip of extruder heating is enough to get past the drop the next time I hit print. Try monitoring the temp while you power up different parts of your system (especially the extruder). If you see an instantaneous change in temperature it is likely noise getting to the TC. Moving the extruder and TC wires may help. You can find app notes with suggestions on minimizing the noise.

korpx commented 12 years ago

I think you are correct. My temperature drop problem problem seems unrelated to the issue fixed by your posted patch. Without any other changes to my setup simply wrapping the extruder motor wires (motor end) around a ferrite ring, and the same with the hotend wires (ramps end) allowed me to print all of last night without any temperature drops.

(But then the firmware, and note that I am not a programmer, must somehow present the temperature reading in relation to the previous readings since the temperature slowly recovered after the sudden drop, rather than recovering instantly.)

Back to issue #169 though. Any news on when it will be merged with the official code?

JTrantow commented 12 years ago

There might be some filtering going on which would cause temp to slowly recover or maybe that's what's really happening. I don't know when my changes will get merged into the official code. I think I gave them everything they need to fix this and another problem with Toshiba stepper drivers. I'm not sure what else needs to be done to get this into the official release.

JTrantow commented 12 years ago

This fix has still not been applied. What is needed to get this into the releases.

JTrantow commented 11 years ago

Latest code has two nops (which are not required) but this is much better. For the record, without any nops or delays with a 16Mhz 328 there is 610ns which >> 100ns required.

github-actions[bot] commented 2 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.