[BUG] 2.0.8+ breaks MAX31865 hardware SPI on BTT GTR 1.0

[truglodite]

Did you test the latest bugfix-2.0.x code?

Yes, and the problem still exists.

Bug Description

I have successfully used my BTT GTR 1.0 board with a MAX31865 and 3-wire PT100, with marlin 2.0.7.2. Doing so required some minor changes to the configs, pins, and temperature.cpp. My attempts to port these changes to release 2.0.8 through 2.0.x-bugfix (4f8191b) have failed. I always get "Err: MAXTEMP: E1" after boot.

Bug Timeline

2.0.8+

Expected behavior

Marlin boots up and stays happy.

Actual behavior

Max temp errors on T0 after bootup every time.

Steps to Reproduce

1. Compile with my changes to configs, pins, and temperature.cpp
2. Wire up max31865 to hardware SPI using PI9 for CS
3. Boot-up, and watch the error

Version of Marlin Firmware

2.0.x-bugfix

Printer model

A8

Electronics

BTT GTR 1.0

Add-ons

MAX31865 PT-100 breakout

Your Slicer

Cura

Host Software

SD Card (headless)

Additional information & file uploads

I posted my configuration files to go with this. Note that I am using hardware SPI for the MAX31865. My understanding is this will use the SD card SPI lines (whether ONBOARD or LCD), and only a max6755_cs_pin is required for this to work. Since the GTR 1.0 uses software SPI for the built in MAX31855, those lines have to be commented in the pins file so the Adafruit lib will use the correct hardware SPI pins. Lastly, my change to temperature.cpp includes only one line (2wire to 3wire), because I use a 3 wire probe. Again, the 2.0.7.2 files are working on real hardware for me. It would be nice to have this also work with 2.0.9 as I want meatpack and a few other goodies.

Configs.zip

[slowbro]

Can you try with this branch? I went through and fixed up a bunch last night: https://github.com/slowbro/Marlin/tree/redundant-temp-fixups-2

[truglodite]

Thank you very much, that seems to fix my problem!

I diffed the configs saw that 2wire/3wire/4wire selection is in the configuration, not temperature.cpp... thank you! The sanity check also told me to add MAX31865_CS_PIN to configuration; maybe those will end up in configuration later. I defined a '65 pin in configuration, removed my cs pin definition from the pins file, and I left the MAX6675_XX_PIN lines commented out (I don't use the onboard '55 chip, and I figure if it is now letting me define the '65 cs pin without errors then I don't want the 6675 is '65 stuff causing problems). Your fork is working well with hardware SPI on my GTR board, with either onboard and lcd sd card. I also saw no errors in terminal.

[truglodite]

Just a quick update, I tested with no changes to the Gtr pins file and it won’t boot. I think the Gtr pinfile just needs a simple ifdef to toggle the 4 lines I mentioned above.

Perhaps this is also out of the scope of the changes you made. There are many ways to configure a max’65, and not sure they all are or should be supported.

[slowbro]

Hmm; Can you paste a diff of your changes to the pins file that makes it work? I'll see what needs doing. Likely it's being caused by this...

Marlin/Marlin/src/module/temperature.cpp

Lines 2013 to 2016 in 5326fdc

	#if THERMO_SEPARATE_SPI
	template<uint8_t MisoPin, uint8_t MosiPin, uint8_t SckPin> SoftSPI<MisoPin, MosiPin, SckPin> SPIclass<MisoPin, MosiPin, SckPin>::softSPI;
	SPIclass<MAX6675_DO_PIN, SD_MOSI_PIN, MAX6675_SCK_PIN> max_tc_spi;
	#endif

RE: MAX6675_CS_PIN specifically.. that was how pretty much all things were referred to (6675, '55, and '65); I'm hoping to simplify that. Since MAX TC are only supported on TEMP_SENSOR_0/1, it boils down to MAXTC_CS_PIN for 0 and MAXTC_CS2_PIN for 1.

[truglodite]

Here you go line 305 in the Gtr v1.0 pins file:

#define THERMO_SCK_PIN PI1 // SCK
#define THERMO_DO_PIN PI2 // MISO
#define THERMO_CS1_PIN PH9 // GTR K-TEMP
#define THERMO_CS2_PIN PH2 // M5 K-TEMP

#define MAX6675_SS_PIN THERMO_CS1_PIN
#define MAX6675_SS2_PIN THERMO_CS2_PIN
#define MAX6675_SCK_PIN THERMO_SCK_PIN
#define MAX6675_DO_PIN THERMO_DO_PIN
‘’’

That doesn’t boot. Changing to this it works:

‘’’
//#define THERMO_SCK_PIN PI1 // SCK
//#define THERMO_DO_PIN PI2 // MISO
//#define THERMO_CS1_PIN PH9 // GTR K-TEMP
//#define THERMO_CS2_PIN PH2 // M5 K-TEMP

//#define MAX6675_SS_PIN THERMO_CS1_PIN
//#define MAX6675_SS2_PIN THERMO_CS2_PIN
//#define MAX6675_SCK_PIN THERMO_SCK_PIN
//#define MAX6675_DO_PIN THERMO_DO_PIN
‘’’

[truglodite]

@slowbro, this may be related to the work you are doing...

I did some testing of the onboard max31855 chip on the GTR, and cannot get that to compile with your branch. Here's the error out:

In file included from Marlin\src\module\temperature.cpp:95:
.pio\libdeps\BIGTREE_GTR_V1_0\Adafruit MAX31855 library/Adafruit_MAX31855.h:50:11: warning: 'boolean' is deprecated [-Wdeprecated-declarations]
   50 |   boolean initialized;
      |           ^~~~~~~~~~~
In file included from C:\Users\Kevin\.platformio\packages\framework-arduinoststm32\cores\arduino/wiring.h:34,
                 from C:\Users\Kevin\.platformio\packages\framework-arduinoststm32\cores\arduino/Arduino.h:36,
                 from Marlin\src\module\../inc/../HAL/./STM32/../shared/Marduino.h:36,
                 from Marlin\src\module\../inc/../HAL/./STM32/HAL.h:28,
                 from Marlin\src\module\../inc/../HAL/HAL.h:30,
                 from Marlin\src\module\../inc/MarlinConfig.h:31,
                 from Marlin\src\module\../MarlinCore.h:24,
                 from Marlin\src\module\temperature.cpp:30:
C:\Users\Kevin\.platformio\packages\framework-arduinoststm32\cores\arduino/wiring_constants.h:110:14: note: declared here
  110 | typedef bool boolean __attribute__((deprecated));
      |              ^~~~~~~
In file included from Marlin\src\module\temperature.cpp:95:
.pio\libdeps\BIGTREE_GTR_V1_0\Adafruit MAX31855 library/Adafruit_MAX31855.h:49:32: warning: passing NULL to non-pointer argument 1 of 'Adafruit_SPIDevice::Adafruit_SPIDevice(int8_t, uint32_t, BitOrder, uint8_t, SP
IClass*)' [-Wconversion-null]
   49 |   Adafruit_SPIDevice spi_dev = NULL;
      |                                ^~~~
In file included from .pio\libdeps\BIGTREE_GTR_V1_0\Adafruit MAX31855 library/Adafruit_MAX31855.h:30,
                 from Marlin\src\module\temperature.cpp:95:
.pio\libdeps\BIGTREE_GTR_V1_0\Adafruit BusIO/Adafruit_SPIDevice.h:61:29: note:   declared here
   61 |   Adafruit_SPIDevice(int8_t cspin, uint32_t freq = 1000000,
      |                      ~~~~~~~^~~~~
Marlin\src\module\temperature.cpp: In static member function 'static int Temperature::read_max_tc()':
Marlin\src\module\temperature.cpp:2697:31: error: 'class Adafruit_MAX31855' has no member named 'readRaw32'
 2697 |       max_tc_temp = max855ref.readRaw32();
      |                               ^~~~~~~~~
*** [.pio\build\BIGTREE_GTR_V1_0\src\src\module\temperature.cpp.o] Error 1
========================= [FAILED] Took 114.60 seconds =========================

My configs included no changes to temperature.cpp, just -3 for sensor0 in configuration (no other max defines), and these changes to the GTR pins file (again line 305 on):

#define THERMO_SCK_PIN                      PI1   // SCK
#define THERMO_DO_PIN                       PI2   // MISO
#define THERMO_CS1_PIN                      PH9   // GTR K-TEMP
#define THERMO_CS2_PIN                      PH2   // M5 K-TEMP

#define MAX31855_CS_PIN            THERMO_CS1_PIN
#define MAX31855_CS2_PIN           THERMO_CS2_PIN
#define MAX31855_SCK_PIN           THERMO_SCK_PIN
#define MAX31855_DO_PIN             THERMO_DO_PIN

//#define MAX6675_SS_PIN            THERMO_CS1_PIN
//#define MAX6675_SS2_PIN           THERMO_CS2_PIN
//#define MAX6675_SCK_PIN           THERMO_SCK_PIN
//#define MAX6675_DO_PIN             THERMO_DO_PIN

I also tried removing the 31855 definitions, using the max6675 lines instead. This gives the same error.

I am not going to use the '55 on my printer anyways, but figured I can gather some data on this before marrying the GTR to my printer. I will be installing the GTR on my printer tomorrow... so please let me know if there's any other testing to be done before then if you get a chance.

[slowbro]

Yeah, that branch is under active development (if you pulled/compiled recently) - but I think I just ironed out the last of the changes. You can see them all in #22196 but the gist of it is the '65 has an internal library now to alleviate the LPCx issues (pending review, may not get approved), and I removed the implicit inclusion of the Adafruit_MAX31855 library - since it isn't strictly needed. Additionally, all the MAX\d+5_.*_PIN and THERMO_.*_PIN defines have been replaced with defines that directly reference the TEMP_SENSOR_n involved; so this:

#define THERMO_SCK_PIN                      PI1   // SCK
#define THERMO_DO_PIN                       PI2   // MISO
#define THERMO_CS1_PIN                      PH9   // GTR K-TEMP
#define THERMO_CS2_PIN                      PH2   // M5 K-TEMP

#define MAX31855_CS_PIN            THERMO_CS1_PIN
#define MAX31855_CS2_PIN           THERMO_CS2_PIN
#define MAX31855_SCK_PIN           THERMO_SCK_PIN
#define MAX31855_DO_PIN             THERMO_DO_PIN

will become:

#define TEMP_0_CS_PIN                       PH9   // GTR K-TEMP
#define TEMP_0_SCK_PIN                      PI1   // SCK
#define TEMP_0_MISO_PIN                     PI2   // MISO
//#define TEMP_0_MOSI_PIN                   ...   // For MAX31865

#define TEMP_1_CS_PIN                       PH2   // M5 K-TEMP
#define TEMP_1_SCK_PIN           TEMP_0_SCK_PIN
#define TEMP_1_MISO_PIN         TEMP_0_MISO_PIN
//#define TEMP_1_MOSI_PIN       TEMP_0_MOSI_PIN

The changes allow the old style (except THERMO_) and translate them for now.. I also updated all the default pins files to match the new style

If you have time to try it out with the latest commit on my branch, it would be appreciated! I don't have a GTR, but plan on doing some hardware testing with some MAX boards I ordered and my SKR 1.4, soon..

slowbro@d575e9a

EDIT: Also, you probably will need to comment out the SCK and MISO pins, since having them defined makes Marlin use Software SPI (likely the source of your boot problems).

EDIT2: Just tested compilation a bit; I had to remove the Adafruit libs from .pio/libdeps/BIGTREE_GTR_V1_0/ to stop them from being included automatically. I was able to compile OK with '55 and '65 but I obviously can't test farther :)

[truglodite]

I just tested the commit I pulled before leaving home yesterday:

https://github.com/slowbro/Marlin/tree/b3527cb856799f5ce2cc09f767964dc6794b4da9

It gives max temp errors on bootup. FWIW, I am testing the onboard '55 with a short copper shunt, as my only k tc has a proper 2-metal connector to fit my DMM, and I don't want to hack it off for this (I did try and hack some alligator clips to attach it as well). I read somewhere that using a short copper shunt here will fool the sensor into thinking it is around room temperature. If this is not the case, I would need a k probe for further testing (or perhaps kill max temp warning for test?). The bad thing is, I have never observed my onboard '55 actually working, with any code I've tried (even release 2072). If I have a bad max55... maybe there's a way to run some test code from the F4 to verify it is working?

EDIT: Also, you probably will need to comment out the SCK and MISO pins, since having them defined makes Marlin use Software SPI (likely the source of your boot problems).

I am pretty certain the onboard max55 needs soft spi to work. That is why I left those defines in for the max55 test. OTOH since I am using my external max65 on hardware SPI, they need commenting out for my real printer. I think yes, something like a macro that looks to what sensor0 and sensor1 are, and defines the pins accordingly would be best to handle that... at least if beginners are expected to not touch files outside the configs.

[slowbro]

This commit should fix the max/min temp errors.

slowbro@9ebeb83

You can also uncomment IGNORE_THERMOCOUPLE_ERRORS at the top of temperature.cpp to test things without it freaking out. Make sure to re-comment it before installing it on your printer, though :)

RE: SCK/MISO, yes, you are correct. My understanding of what was switching between soft/hardware SPI was flawed. From the looks of it, the only options are hardware SPI over the default bus (usually SD card), or software SPI over arbitrary pins - even if those pins are their own hardware SPI bus. So, you'd need the SCK/MISO pins still. I added a new section to Configuration_adv.h that has an option to 'force' hardware SPI when the pins file has these defined, which should help your situation:

/**
 * Configuration options for MAX Thermocouples (-2, -3, -5).
 *   FORCE_HW_SPI:   Ignore SCK/MOSI/MISO pins and just use the CS pin & default SPI bus.
 *   MAX31865_WIRES: Set the number of wires for the probe connected to a MAX31865 board, 2-4. Default: 2
 */
//#define TEMP_SENSOR_0_FORCE_HW_SPI
//#define TEMP_SENSOR_1_FORCE_HW_SPI
//#define MAX31865_SENSOR_WIRES_0 2
//#define MAX31865_SENSOR_WIRES_1 2

In that way, you won't have to modify your pins file - just uncomment TEMP_SENSOR_0_FORCE_HW_SPI. You'd still need to set the CS pin properly, but that can go in Configuration.h.

Thanks again for testing. I have hardware to test '65 and 6675, and plan on doing that today/this weekend.

[truglodite]

Thank you very much once again... with that commit the max55 is now working.

For the max65, the force HW spi option still requires editing the pins file for the max65 to work. If use the force hardware spi option, and define the cs pin in configuration for example #define TEMP_0_CS_PIN PI9, that pin is being redefined in the pins file to PH9 (compiler warnings say so). I have my max65 sidelined ATM while testing the max55 here. So not sure if just commenting out the cs pin in the pins file would get that working, but it compiles with no more warnings when I do. Not sure though if the sck and miso pins are also getting defined by that pins file when "force" is enabled?

I was thinking something like this in the pins file would fix it, but not experienced enough to know if it even works or there's better ways:

#if TEMP_SENSOR_IS_MAX(0, 31855)
  #define TEMP_0_CS_PIN                       PH9   // GTR K-TEMP
  #define TEMP_0_SCK_PIN                      PI1   // SCK
  #define TEMP_0_MISO_PIN                     PI2   // MISO
  //#define TEMP_0_MOSI_PIN                   ...   // For MAX31865
#endif
#if TEMP_SENSOR_IS_MAX(1, 31855)
  #define TEMP_1_CS_PIN                       PH2   // M5 K-TEMP
  #define TEMP_1_SCK_PIN           TEMP_0_SCK_PIN
  #define TEMP_1_MISO_PIN         TEMP_0_MISO_PIN
  //#define TEMP_1_MOSI_PIN       TEMP_0_MOSI_PIN
#endif

Also, I am not sure if the mosi pin even needs to be in there, since IIRC the adafruit lib always uses the default hardware SPI, unless software SPI pins are defined. That default hardware SPI will already have the right mosi pin I think. So just need to sort out which cs pin gets compiled based on configs. For the GTR using -3, it of course should just default to the onboard chip. For external boards (especially the max65 which I think we both see a future for it), maybe leaving //#define TEMP_0_CS_PIN PI9 // external max55/65 temp using hardware SPI in configuration_adv would be a good idea (less support that way I think)

[slowbro]

I think instead of the TEMP_SENSOR_IS_MAX() macro, wrapping those in #ifndef checks would be better; that way it would not override user settings elsewhere. That sould solve the redifined warnings/issue. I'm not sure if TEMP_SENSOR_IS_MAX would be defined at that point, since it's defined in temperature.cpp

Would it be possible to also test with both '55s on the GTR enabled? I want to make sure the code for 2x '55 works, but I only have the one.

For '65 SPI, MOSI is used to trigger a 'one shot' reading in the Adafruit lib, as well as other things like switching between the 50Hz/60Hz filter, enabling bias, and other configuration. When it's using hardware SPI, it likely is using the hardware-defined MOSI. For SoftSPI piggybacking on the SD card, it is using the SD MOSI as well, as seen in 2.0.8 (before my redundant-temp changes):

Marlin/Marlin/src/module/temperature.cpp

Lines 91 to 109 in d92decb

	#if HAS_MAX31865
	#include <Adafruit_MAX31865.h>
	#ifndef MAX31865_MOSI_PIN
	#define MAX31865_MOSI_PIN SD_MOSI_PIN
	#endif
	#if PIN_EXISTS(MAX31865_MISO) && PIN_EXISTS(MAX31865_SCK)
	#define MAX31865_USES_SW_SPI 1
	#endif
	#if TEMP_SENSOR_0_IS_MAX31865 && PIN_EXISTS(MAX31865_CS)
	#define HAS_MAX31865_TEMP 1
	Adafruit_MAX31865 max31865_0 = Adafruit_MAX31865(MAX31865_CS_PIN
	#if MAX31865_USES_SW_SPI && PIN_EXISTS(MAX31865_MOSI)
	, MAX31865_MOSI_PIN, MAX31865_MISO_PIN, MAX31865_SCK_PIN // For software SPI also set MOSI/MISO/SCK
	#endif
	#if ENABLED(LARGE_PINMAP)
	, HIGH
	#endif
	);
	#endif

Looking at the Adafruit lib, it is still calling writeRegister8() to set config items in Hardware SPI mode - so AFAIK, MOSI would be required. As it stands, '65 is broken on my branch, but I have a fix that will be pushed soon - testing w/ SKR V2 & Pt100.

[truglodite]

Would it be possible to also test with both '55s on the GTR enabled?

Unfortunately I also only have one 55 (I didn't buy an M1 to go with my GTR... not yet anyways).

I figured as much, about my TEMP_SENSOR_IS_MAX maybe not being defined when it is needed. I agree keeping it as an #ifdef macro is cleaner... just couldn't recall the proper code for that off the top of my head.

As far as the MOSI pin, I assumed that everyone using the F4 would have SD_CARD_ENABLED, and therefore have the SD_MOSI_PIN defined. So yeah to handle all situations, the mosi define should probably be left in (maybe add to the notes to uncomment only if not using an sd card).

Lastly, I saw you mentioned softSPI on the SD CARD... I was going off of what GadgetAngel wrote in the expansive max31865+GTR1.0 guide about the GTR's hardware SPI. Accordingly, the GTR uses hardware SPI1 as the default hardware SPI when SD_CARD_CONNECTION ONBOARD, or it will use hardware SPI2 as the default when SD_CARD_CONNECTION LCD. I assumed in both cases those were hardware SPI. Is this not correct? Does the SKR v2 use softspi for sdcard? Isn't software SPI "not as good" for printing from SD?

[slowbro]

Unfortunately I also only have one 55 (I didn't buy an M1 to go with my GTR... not yet anyways).

No worries! I'll have to grab another. Though I do have two 6675s, so that likely will suffice.

I assumed in both cases those were hardware SPI. Is this not correct? Does the SKR v2 use softspi for sdcard? Isn't software SPI "not as good" for printing from SD?

The SD Card will use Hardware SPI, but when setting up the SoftSPI class in temperature.cpp, it uses the SD_MOSI_PIN:

Marlin/Marlin/src/module/temperature.cpp

Lines 1924 to 1927 in d92decb

	#if THERMO_SEPARATE_SPI
	template<uint8_t MisoPin, uint8_t MosiPin, uint8_t SckPin> SoftSPI<MisoPin, MosiPin, SckPin> SPIclass<MisoPin, MosiPin, SckPin>::softSPI;
	SPIclass<MAX6675_DO_PIN, SD_MOSI_PIN, MAX6675_SCK_PIN> max_tc_spi;
	#endif

As of now, that SoftSPI is only used when no library is in use. I'm not sure if that actually uses the MOSI line, or if it just needs a pin defined for the template/initializer?

For the Adafruit libs, it uses the SPI class for Hardware SPI, and raw digitalWrite() and digitalRead() for software SPI. I'm uncertain how this affects SD Card reads/writes.. I have a feeling things are happening single-threaded-ly, so it likely doesn't interfere with the SD Card ops, since it's talking to another device (CS pulled low for the '65).

https://github.com/GadgetAngel/Adafruit-MAX31865-V1.1.0-Mod-M/blob/a859d02051c9031cb5d5203818bc8b38ac307c19/Adafruit_MAX31865.cpp#L637

[slowbro]

@truglodite #22196 has been merged, so this should be good to go!

[github-actions]

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.