xC0000005 / Marlin

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

Lerdge-S pins and config #10

Closed mapfde closed 1 year ago

mapfde commented 4 years ago

Description

Complete pins file and example configuration for Lerdge-S. Only remaining issue is the thermistor inputs. The Lerdge-S board has a max6675 chip onboard and can select thermistor or thermocouple use in software. I am sure the pins are correct, as I found a set of pins (on the expansion board interface) that allow marlin to read temperatures in the correct range - I touched them by accident, and for a short time the LCD shows correct values.

Benefits

Get the correct pins for the Lerdge-S board.

Related Issues

mapfde commented 4 years ago

I noted in the pins file from LoialOtter that there is a Winbond flash memory chip on the Lerdge-K. Of course, that is also on the Lerdge-S. I'll go after those pins next. ..

mapfde commented 4 years ago

After careful inspection, I found the location of the second max6675 chip. It is on the underside of the expansion board, but has not been installed on mine. Not sure if intentional, but as I have found pcb pictures of Lerdge-S with the max6675 unpopulated as well, it might be they wanted to save some money.

Curious. 2020-06-20 16 19 35

mapfde commented 4 years ago

Data and clock pins are the same, CS is pin PH0. When I enter this in the pins file, Marlin complains:

In file included from Marlin\src\gcode\config\M43.cpp:29:
c:\code\marlin\marlin\src\pins\stm32f4/pins_LERDGE_S.h:124:44: error: 'PH0' was not declared in this scope; did you mean 'PH_0'?
  124 | #define MAX6675_SS2_PIN                    PH0 //max6675 datasheet: /CS pin, found with multimeter, not tested
      |                                            ^~~
c:\code\marlin\marlin\src\pins\pinsdebug.h:88:64: note: in definition of macro '_ADD_PIN_2'
   88 | #define _ADD_PIN_2(ENTRY_NAME, NAME, IS_DIGITAL) { ENTRY_NAME, NAME, IS_DIGITAL },
      |                                                                ^~~~
c:\code\marlin\marlin\src\pins\pinsdebug.h:90:44: note: in expansion of macro '_ADD_PIN'
   90 | #define REPORT_NAME_DIGITAL(COUNTER, NAME) _ADD_PIN(NAME, COUNTER, true)
      |                                            ^~~~~~~~
c:\code\marlin\marlin\src\pins\pinsDebug_list.h:683:3: note: in expansion of macro 'REPORT_NAME_DIGITAL'
  683 |   REPORT_NAME_DIGITAL(__LINE__, MAX6675_SS2_PIN)
      |   ^~~~~~~~~~~~~~~~~~~
c:\code\marlin\marlin\src\pins\pinsDebug_list.h:683:33: note: in expansion of macro 'MAX6675_SS2_PIN'
  683 |   REPORT_NAME_DIGITAL(__LINE__, MAX6675_SS2_PIN)
      |                                 ^~~~~~~~~~~~~~
mapfde commented 4 years ago

The complaint is valid, as PH0 and PH1 are missing in the variant header file. I added them, but no luck with the max6675. No matter which of the cs pins I twiddle with M42, nothing happens.

But: If I ground the expansion board pin that is connected to the CS input of the second (non-existing) max6675 and PH0, measurement of ambient temperature works fine - for both heater thermistor inputs.

Maybe it is time for a bodge wire ;)

mapfde commented 4 years ago

I made a mistake when counting the pins. Yesterday I used M42 M43 to find the pins, which worked fine. Today I used the multimeter and counted pins - and used the wrong diagram.

No wonder nothing works...

mapfde commented 4 years ago

We did it! With a M42 P99 S0 the thermistor readings are right. I'll re-check all my multimeter readings later, I must have counted wrong.

xC0000005 commented 4 years ago

Congratulations! I pulled your commits into the main PR in Marlin - they'll be merged when it is!

mapfde commented 4 years ago

I checked the pull request into the main repository. Something is off, it has not the correct pins for the S board.

xC0000005 commented 4 years ago

I think I know what’s happened, I’ll pull each of your commits and let you know.

On Jun 22, 2020, at 2:43 PM, Magnus Pfeffer notifications@github.com wrote:

I checked the pull request into the main repository. Something is off, it has not the correct pins for the S board.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/xC0000005/Marlin/pull/10#issuecomment-647733189, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHVGS4OCZ5QT6XIUXG3I6Y3RX6X47ANCNFSM4ODK4KRA.

xC0000005 commented 4 years ago

Ok…try now. I’m pretty much awful with git, but it looks like I got the changes (I hope).

On Jun 22, 2020, at 2:43 PM, Magnus Pfeffer notifications@github.com wrote:

I checked the pull request into the main repository. Something is off, it has not the correct pins for the S board.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/xC0000005/Marlin/pull/10#issuecomment-647733189, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHVGS4OCZ5QT6XIUXG3I6Y3RX6X47ANCNFSM4ODK4KRA.

mapfde commented 4 years ago

Yes, it is now the latest version of my pins definition. We still need a clean way to handle that stupid pin that needs to be pulled LOW so the thermistor readings are correct. I'll ask on the discord for pointers.

mapfde commented 4 years ago

Ah, got an answer already. Thinkyhead says "Add OUT_WRITE(MY_PIN,LOW); to setup()".

Where is the board setup?

mapfde commented 4 years ago

ah, in MarlinCore.cpp

xC0000005 commented 4 years ago

You should do it if the thermistor type is a thermocouple. There has to be a pattern for this (I’m just not sure what it is).

On Jun 22, 2020, at 3:28 PM, Magnus Pfeffer notifications@github.com wrote:

ah, in MarlinCore.cpp

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/xC0000005/Marlin/pull/10#issuecomment-647752628, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHVGS4LW5O6OQBT3NSIHKTLRX65F7ANCNFSM4ODK4KRA.

xC0000005 commented 4 years ago

My recommendation is to do it in thermal manager::Init, something like if sensor1 == -1 (whatever it is) and PIN_EXISTS (THERMISTOR SOMEHTING) set it.

On Jun 22, 2020, at 3:28 PM, Magnus Pfeffer notifications@github.com wrote:

ah, in MarlinCore.cpp

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/xC0000005/Marlin/pull/10#issuecomment-647752628, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHVGS4LW5O6OQBT3NSIHKTLRX65F7ANCNFSM4ODK4KRA.

mapfde commented 4 years ago

Good idea. Have it working for heater0, just added heater1. Will amend this pull request in a minute.

mapfde commented 4 years ago

Now all we need is proper scaling of the gui, the right touchscreen calibration and the correct init of the lcd to have the proper colors :)

I am currently working on my 3d printer projects, I will put the board in either printer#3 or printer#4, depending on when the missing parts arrive, so I can test the board in a printer instead of on the bench.

mapfde commented 4 years ago

There is a logical error in the change you did to the code when you pushed it to the marlin github:

#if PIN_EXISTS(TEMP_0_TR_ENABLE) && HEATER_0_USES_MAX6675

has it the wrong way.

If the pin is active and we do not use a thermocouple, we pull the pin.

I am not sure what the max6675 "needs", maybe it is the same pin on HIGH, maybe it is just the CS. I do not have a TC sensor, so I cannot test that.

xC0000005 commented 4 years ago

I also broke the CI build. I will fix it in a bit. I want to use the conditionals and just add a NOT()

Sent from my iPhone

On Jun 23, 2020, at 9:08 AM, Magnus Pfeffer notifications@github.com wrote:

 There is a logical error in the change you did to the code when you pushed it to the marlin github:

if PIN_EXISTS(TEMP_0_TR_ENABLE) && HEATER_0_USES_MAX6675

has it the wrong way.

If the pin is active and we do not use a thermocouple, we pull the pin.

I am not sure what the max6675 "needs", maybe it is the same pin on HIGH, maybe it is just the CS. I do not have a TC sensor, so I cannot test that.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

xC0000005 commented 4 years ago

I’m testing a build now (build only, of course, since I”m far from home) that uses this to make sure that the pin is always in the right state (I think). I’ll update the Ledge X and K pins to have these as well.

if ANY(MB(LERDGE_S), MB(LERDGE_X), MB(LERDGE_K))

//thermistor activation by MCU pin

if PIN_EXISTS(TEMP_0_TR_ENABLE)

 OUT_WRITE(TEMP_0_TR_ENABLE_PIN, HEATER_0_USES_MAX6675);

endif

if PIN_EXISTS(TEMP_1_TR_ENABLE)

 OUT_WRITE(TEMP_1_TR_ENABLE_PIN, HEATER_1_USES_MAX6675);

endif

endif

On Jun 23, 2020, at 9:08 AM, Magnus Pfeffer notifications@github.com wrote:

There is a logical error in the change you did to the code when you pushed it to the marlin github:

if PIN_EXISTS(TEMP_0_TR_ENABLE) && HEATER_0_USES_MAX6675

has it the wrong way.

If the pin is active and we do not use a thermocouple, we pull the pin.

I am not sure what the max6675 "needs", maybe it is the same pin on HIGH, maybe it is just the CS. I do not have a TC sensor, so I cannot test that.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/xC0000005/Marlin/pull/10#issuecomment-648173849, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHVGS4PCHEPULUVWYOJUBH3RYCZM3ANCNFSM4ODK4KRA.

xC0000005 commented 4 years ago

Well, it turns out you change may be the only way it works - conditionals aren’t always defined, so while this looks pretty, it doesn’t work. I’ll revert to your commit and see how X and K compile.

On Jun 23, 2020, at 9:25 AM, JC Nelson authorjcnelson@gmail.com wrote:

I’m testing a build now (build only, of course, since I”m far from home) that uses this to make sure that the pin is always in the right state (I think). I’ll update the Ledge X and K pins to have these as well.

if ANY(MB(LERDGE_S), MB(LERDGE_X), MB(LERDGE_K))

//thermistor activation by MCU pin

if PIN_EXISTS(TEMP_0_TR_ENABLE)

 OUT_WRITE(TEMP_0_TR_ENABLE_PIN, HEATER_0_USES_MAX6675);

endif

if PIN_EXISTS(TEMP_1_TR_ENABLE)

 OUT_WRITE(TEMP_1_TR_ENABLE_PIN, HEATER_1_USES_MAX6675);

endif

endif

On Jun 23, 2020, at 9:08 AM, Magnus Pfeffer <notifications@github.com mailto:notifications@github.com> wrote:

There is a logical error in the change you did to the code when you pushed it to the marlin github:

if PIN_EXISTS(TEMP_0_TR_ENABLE) && HEATER_0_USES_MAX6675

has it the wrong way.

If the pin is active and we do not use a thermocouple, we pull the pin.

I am not sure what the max6675 "needs", maybe it is the same pin on HIGH, maybe it is just the CS. I do not have a TC sensor, so I cannot test that.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/xC0000005/Marlin/pull/10#issuecomment-648173849, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHVGS4PCHEPULUVWYOJUBH3RYCZM3ANCNFSM4ODK4KRA.

mapfde commented 4 years ago

I can test if your chosen condition still switches the pin correctly. Simply post it here in the conversation or send me a diff.

mapfde commented 4 years ago

I think the logic is still inverted after your last change

xC0000005 commented 4 years ago

Well, I was never one to do something straightforward. Let me ask it again simply, if this is true: Thermistors == pins high Thermocouple == pins low

Or is it backwards?

On Jun 23, 2020, at 12:49 PM, Magnus Pfeffer notifications@github.com wrote:

I think the logic is still inverted after your last change

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/xC0000005/Marlin/pull/10#issuecomment-648318827, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHVGS4JQEXWUDADY7MRPPODRYDTMHANCNFSM4ODK4KRA.

mapfde commented 4 years ago

thermistor needs low. I have absolutely no idea what the thermocouple needs, and actually never tested setting the pin high.

xC0000005 commented 4 years ago

What I expect is that this pin is disabling the path to ground for the divider network (since the thermocouple wouldn’t use it). I’m sorry we’re having to iterate so many times, but we’ll get there.

On Jun 23, 2020, at 12:56 PM, Magnus Pfeffer notifications@github.com wrote:

thermistor needs low. I have absolutely no idea what the thermocouple needs, and actually never tested seeing the pin high.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/xC0000005/Marlin/pull/10#issuecomment-648322152, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHVGS4JAP3RM24YHTKDTRQLRYDUE5ANCNFSM4ODK4KRA.

mapfde commented 4 years ago

you might want to change that first #if PIN_EXISTS(TEMP_1_TR_ENABLE_PIN) into #if PIN_EXISTS(TEMP_0_TR_ENABLE_PIN) ...

xC0000005 commented 4 years ago

I am going to have to squash commits just to remove proof of my idiocy from the internet. :)

On Jun 23, 2020, at 1:11 PM, Magnus Pfeffer notifications@github.com wrote:

TEMP_0_TR_ENABLE_PIN

mapfde commented 4 years ago

On another note: I just saw that functions for touch calibrations were added in the "TFT and touch" pull request. And there is code for scaling the interface to fill any TFT resolution in another PR. This is looking very promising to me :)

tpruvot commented 2 years ago

@mapfde I see you have a Lerdge S... Could you confirm me this section works for you for the I2C EEPROM : https://github.com/MarlinFirmware/Marlin/pull/22658/files

I guess the pins are wrong, seems set to Y/ZMAX Endstops, if you could find the 2 pins connected to your eeprom ... would be nice ;)

mapfde commented 2 years ago

@mapfde I see you have a Lerdge S... Could you confirm me this section works for you for the I2C EEPROM : https://github.com/MarlinFirmware/Marlin/pull/22658/files

I guess the pins are wrong, seems set to Y/ZMAX Endstops, if you could find the 2 pins connected to your eeprom ... would be nice ;)

Hi. I am currently not near my printer, and will check later this month. I had to stop working on my printer last summer but I have some free time soon and am looking forward to see if the current Marlin will work with the Lerdge-S

tpruvot commented 2 years ago

yep tx, else should take 2mn if you have a multimeter ;) see https://github.com/MarlinFirmware/Marlin/pull/22658 discussion... should be merged without I2C eeprom before that else... these cards are out of stock