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

MAX6675 HEAT_INTERVAL not defined. #867

Closed JTrantow closed 9 years ago

JTrantow commented 10 years ago

1) Code will not compile with MAX6675 selected as temperature sensor. Looks like someone changed HEAT_INTERVAL to MAX6675_HEAT_INTERVAL but forgot to use it in the initializer.

2) read_max6675() still has completely unnecessary delays inserted. There is no reason to add unrequired delay to a function called within an interrupt.

3) read_max6675() isn't checking status bits that can be used to detect whether the device is present.

I have attached code which addresses these problems along with relevant comments. Please replace the existing function with this improvement. Issues 2) and 3) had been covered a long time ago but never made it into the code.

if defined(HEATER_0_USES_MAX6675)

define MAX6675_HEAT_INTERVAL 250 //!< Time to read MAX6675 [msec].

static long max6675_previous_millis = -MAX6675_HEAT_INTERVAL; // Initialize so first call will update the reading. static int max6675_temp = (4*500); //!< Four times the temperature in degrees C. (0.25 degrees per count)

/*! 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 IS REQUIRED AND ADDING UNREQUIRED DELAY IN AN INTERRUPT IS UNEXCUSABLE.
_/
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. /

/ If the MAX6675 isn't connected it could clock in all ones(MISO high).
Check that the high bit and device present bits are zeros indicating the MAX6675 is present. _/ if (0 == (max6675temp & 0x8002)) { / 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. _/ max6675temp >>=3; } }else { / We did NOT get a valid reading from the MAX6675.
/ max6675_temp = 2000; // MAX6675 not present. } return(max6675_temp); } // read_max6675().

endif

JTrantow commented 10 years ago

4) temperature.cpp tp_init() needs to initialize MAX_SCK_PIN, MAX_MOSI_PIN, and MAX_ISO_PIN in case SD has not initialized these pins.

if defined(HEATER_0_USES_MAX6675)

  SET_OUTPUT(MAX_SCK_PIN);
  WRITE(MAX_SCK_PIN,0);
  SET_OUTPUT(MAX_MOSI_PIN);
  WRITE(MAX_MOSI_PIN,1);

  SET_INPUT(MAX_MISO_PIN);
  WRITE(MAX_MISO_PIN,1);

SET_OUTPUT(MAX6675_SS);
WRITE(MAX6675_SS,1);

endif

nothinman commented 10 years ago

@JTrantow can you open pull request please?

JTrantow commented 10 years ago

Sorry, I'm a SVN user and the Marlin Git organization is still a mystery to me.

On Wed, Apr 2, 2014 at 12:24 PM, nothinman notifications@github.com wrote:

@JTrantow https://github.com/JTrantow can you open pull request please?

Reply to this email directly or view it on GitHubhttps://github.com/ErikZalm/Marlin/issues/867#issuecomment-39358458 .

boelle commented 9 years ago

a pull request would be nice yes.... but maybe the issue has been fixed allready??

boelle commented 9 years ago

please open a new issue if this is still present in the current bug fixing branch

https://github.com/ErikZalm/Marlin/tree/Marlin-v1-bug-fixing

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.