Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🩹 Fix MKS Gen-L V1 pins, allow more RAMPS overrides #26974

Merged
merged 10 commits into from
Aug 11, 2024
Merged

🩹 Fix MKS Gen-L V1 pins, allow more RAMPS overrides #26974

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
d3281b2
🩹 Fix MKS Gen-L V1 pins, allow more RAMPS overrides
thisiskeithb Apr 16, 2024
4e7b5fb
Rely on MOSFET_*_PIN defaults from pins_RAMPS.h
thisiskeithb Apr 16, 2024
856f46e
Update Marlin/src/pins/ramps/pins_MKS_GEN_L.h
thisiskeithb Apr 29, 2024
7f6cfa6
Update Marlin/src/pins/ramps/pins_MKS_GEN_L.h
thisiskeithb Apr 29, 2024
46088ae
Update Marlin/src/pins/ramps/pins_MKS_GEN_L.h
thisiskeithb Apr 29, 2024
5ec84e0
Update Marlin/src/pins/ramps/pins_MKS_GEN_L.h
thisiskeithb Apr 29, 2024
f710b48
use FET_ORDER_*
thinkyhead Aug 9, 2024
0a7e3da
Merge branch 'bugfix-2.1.x' into pr/26974
thinkyhead Aug 9, 2024
b2e12ed
use ENABLED()
thinkyhead Aug 9, 2024
8bb329d
maybe this…
thinkyhead Aug 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Marlin/src/pins/pins.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@
#elif MB(MKS_GEN_13)
#include "ramps/pins_MKS_GEN_13.h" // ATmega2560, ATmega1280 env:mega2560 env:mega1280
#elif MB(MKS_GEN_L)
#include "ramps/pins_MKS_GEN_L.h" // ATmega2560, ATmega1280 env:mega2560 env:mega1280
#include "ramps/pins_MKS_GEN_L.h" // ATmega2560 env:mega2560
#elif MB(KFB_2)
#include "ramps/pins_BIQU_KFB_2.h" // ATmega2560 env:mega2560
#elif MB(ZRIB_V20)
Expand Down
19 changes: 14 additions & 5 deletions Marlin/src/pins/ramps/pins_MKS_GEN_L.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
/**
* MKS GEN L – Arduino Mega2560 with RAMPS v1.4 pin assignments
* Schematic: https://github.com/makerbase-mks/MKS-GEN_L/blob/master/hardware/MKS%20Gen_L%20V1.0_008/MKS%20Gen_L%20V1.0_008%20SCH.pdf
thisiskeithb marked this conversation as resolved.
Show resolved Hide resolved
* ATmega2560, ATmega1280
* Pin Diagram: https://github.com/makerbase-mks/MKS-GEN_L/blob/master/hardware/MKS%20Gen_L%20V1.0_008/MKS%20Gen_L%20V1.0_008%20PIN.pdf
* Note: Schematic contains pin & connector errors, correct pin mappings are found on the pin diagram
* ATmega2560
*/

#if HOTENDS > 2 || E_STEPPERS > 2
Expand All @@ -37,10 +39,17 @@
// Heaters / Fans
//

#define MOSFET_A_PIN 10 // HE0
#define MOSFET_B_PIN 7 // HE1 or FAN Hotend Cooling
#define MOSFET_C_PIN 8 // HBED
#define FAN0_PIN 9 // FAN Part Cooling
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either the MOSFET pin scheme needs to be retained, or this file needs to do the handling of the flags *_EFB, *_EEB, etc., for cases where the Extruder, Fan, Bed arrangement may change. These flags may already be set for the HAS_MULTI_HOTEND and/or HEATERS_PARALLEL and should be preferred over referring to these settings flags directly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reminder that the rule for Extruder, Fan, Bed pins is not to always define them according to their silkscreen label, but according to the configuration requirements. The board may say something like "H1, Fan, Bed" which would correspond to the common default FET_ORDER_EFB. But when two extruders are defined, the FET_ORDER_EEB flag gets set, so the pins should then be defined as "H1, H2, Bed" with no fan.

If the FET_ORDER_* flags aren't coming out right for this board according to those flags, either these FETs are just not defined in the correct sequence, or the general FET assignment in pins_RAMPS.h is wonky.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I wonder if it work just using the order 10, 9, 8, 7… But we have to look at the machines these ship in also and see if they are using non-standard ordering on shipping units.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either the MOSFET pin scheme needs to be retained, or this file needs to do the handling of the flags *_EFB, *_EEB, etc.,

I really don't follow all that FET_ORDER stuff since we don't do that on non-AVR boards, but I have physically traced the pins on the MCU and verified that they now match real hardware.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I wonder if it work just using the order 10, 9, 8, 7… But we have to look at the machines these ship in also and see if they are using non-standard ordering on shipping units.

We host several Artillery configs and the older/non-32-bit printers use a standard Gen-L: https://github.com/MarlinFirmware/Configurations/tree/import-2.1.x/config/examples/Artillery

// The following are included for documentation purposes only. They are actually defined in pins_RAMPS.h.
//#define HEATER_BED_PIN 8 // H-BED / "MOSFET_C_PIN"
sjasonsmith marked this conversation as resolved.
Show resolved Hide resolved
//#define HEATER_0_PIN 10 // HE0 / "MOSFET_A_PIN"
sjasonsmith marked this conversation as resolved.
Show resolved Hide resolved
thisiskeithb marked this conversation as resolved.
Show resolved Hide resolved
#if ANY(HAS_MULTI_HOTEND, HEATERS_PARALLEL)
#define HEATER_1_PIN 7 // HE1
#else
#define FAN1_PIN 7 // HE1
#endif

// The following is included for documentation purposes only. Is is actually defined in pins_RAMPS.h.
//#define FAN0_PIN 9 // FAN / "MOSFET_B_PIN"
sjasonsmith marked this conversation as resolved.
Show resolved Hide resolved
thisiskeithb marked this conversation as resolved.
Show resolved Hide resolved

//
// CS Pins wired to avoid conflict with the LCD
Expand Down
20 changes: 15 additions & 5 deletions Marlin/src/pins/ramps/pins_RAMPS.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,16 +231,22 @@
#define MOSFET_D_PIN -1
#endif

#define HEATER_0_PIN MOSFET_A_PIN
#ifndef HEATER_0_PIN
#define HEATER_0_PIN MOSFET_A_PIN
#endif

#if FET_ORDER_EFB // Hotend, Fan, Bed
#ifndef HEATER_BED_PIN
#define HEATER_BED_PIN MOSFET_C_PIN
#endif
#elif FET_ORDER_EEF // Hotend, Hotend, Fan
#define HEATER_1_PIN MOSFET_B_PIN
#ifndef HEATER_1_PIN
#define HEATER_1_PIN MOSFET_B_PIN
#endif
#elif FET_ORDER_EEB // Hotend, Hotend, Bed
#define HEATER_1_PIN MOSFET_B_PIN
#ifndef HEATER_1_PIN
#define HEATER_1_PIN MOSFET_B_PIN
#endif
#ifndef HEATER_BED_PIN
#define HEATER_BED_PIN MOSFET_C_PIN
#endif
Expand All @@ -253,9 +259,13 @@
#define HEATER_BED_PIN MOSFET_C_PIN
#endif
#if ANY(HAS_MULTI_HOTEND, HEATERS_PARALLEL)
#define HEATER_1_PIN MOSFET_D_PIN
#ifndef HEATER_1_PIN
#define HEATER_1_PIN MOSFET_D_PIN
#endif
#else
#define FAN1_PIN MOSFET_D_PIN
#ifndef FAN1_PIN
#define FAN1_PIN MOSFET_D_PIN
#endif
#endif
#endif

Expand Down