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.

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

add CONTROLLER_FAN check to PID autotune #26652

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
34 changes: 19 additions & 15 deletions Marlin/src/feature/controllerfan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,17 +55,19 @@ void ControllerFan::set_fan_speed(const uint8_t s) {
}

void ControllerFan::update() {
static millis_t lastMotorOn = 0, // Last time a motor was turned on
nextMotorCheck = 0; // Last time the state was checked
static millis_t lastComponentOn = 0, // Last time a stepper, heater, etc. was turned on
nextFanCheck = 0; // Last time the state was checked
const millis_t ms = millis();
if (ELAPSED(ms, nextMotorCheck)) {
nextMotorCheck = ms + 2500UL; // Not a time critical function, so only check every 2.5s

// If any triggers for the controller fan are true...
// - At least one stepper driver is enabled
// - The heated bed is enabled
// - TEMP_SENSOR_BOARD is reporting >= CONTROLLER_FAN_MIN_BOARD_TEMP
// - TEMP_SENSOR_SOC is reporting >= CONTROLLER_FAN_MIN_SOC_TEMP
if (ELAPSED(ms, nextFanCheck)) {
nextFanCheck = ms + 2500UL; // Not a time critical function, so only check every 2.5s

/**
* If any triggers for the controller fan are true...
* - At least one stepper driver is enabled
* - The heated bed (MOSFET) is enabled
* - TEMP_SENSOR_BOARD is reporting >= CONTROLLER_FAN_MIN_BOARD_TEMP
* - TEMP_SENSOR_SOC is reporting >= CONTROLLER_FAN_MIN_SOC_TEMP
*/
const ena_mask_t axis_mask = TERN(CONTROLLER_FAN_USE_Z_ONLY, _BV(Z_AXIS), (ena_mask_t)~TERN0(CONTROLLER_FAN_IGNORE_Z, _BV(Z_AXIS)));
if ( (stepper.axis_enabled.bits & axis_mask)
|| TERN0(HAS_HEATED_BED, thermalManager.temp_bed.soft_pwm_amount > 0)
Expand All @@ -75,13 +77,15 @@ void ControllerFan::update() {
#ifdef CONTROLLER_FAN_MIN_SOC_TEMP
|| thermalManager.wholeDegSoc() >= CONTROLLER_FAN_MIN_SOC_TEMP
#endif
) lastMotorOn = ms; //... set time to NOW so the fan will turn on
) lastComponentOn = ms; //... set time to NOW so the fan will turn on

// Fan Settings. Set fan > 0:
// - If AutoMode is on and steppers have been enabled for CONTROLLERFAN_IDLE_TIME seconds.
// - If System is on idle and idle fan speed settings is activated.
/**
* Fan Settings. Set fan > 0:
* - If AutoMode is on and hot components have been powered for CONTROLLERFAN_IDLE_TIME seconds.
* - If System is on idle and idle fan speed settings is activated.
*/
set_fan_speed(
settings.auto_mode && lastMotorOn && PENDING(ms, lastMotorOn + SEC_TO_MS(settings.duration))
settings.auto_mode && lastComponentOn && PENDING(ms, lastComponentOn + SEC_TO_MS(settings.duration))
? settings.active_speed : settings.idle_speed
);

Expand Down
30 changes: 23 additions & 7 deletions Marlin/src/module/temperature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,25 @@ volatile bool Temperature::raw_temps_ready = false;
* Class and Instance Methods
*/

#if ANY(HAS_PID_HEATING, MPC_AUTOTUNE)

/**
* Run the minimal required activities during a tuning loop.
* TODO: Allow tuning routines to call idle() for more complete keepalive.
*/
inline void tuning_manage_activity() {
// Run HAL idle tasks
hal.idletask();

// Run Controller Fan check (normally handled by manage_inactivity)
TERN_(USE_CONTROLLER_FAN, controllerFan.update());

// Run UI update
TERN(DWIN_CREALITY_LCD, dwinUpdate(), ui.update());
Copy link
Member

Choose a reason for hiding this comment

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

Aha, there is a general UI update… thought so. We can still poke through manage_inactivity to see if any other relevant items would be useful here, and we can still explore whether it would be okey-dokey to call idle() during PID/MPC tuning without having any negative impact on the tuning process. I don't think that's ever been attempted.

}

#endif

#if HAS_PID_HEATING

inline void say_default_() { SERIAL_ECHOPGM("#define DEFAULT_"); }
Expand Down Expand Up @@ -890,11 +909,8 @@ volatile bool Temperature::raw_temps_ready = false;
goto EXIT_M303;
}

// Run HAL idle tasks
hal.idletask();

// Run UI update
TERN(DWIN_CREALITY_LCD, dwinUpdate(), ui.update());
// Run minimal necessary machine tasks
tuning_manage_activity();
}
wait_for_heatup = false;

Expand Down Expand Up @@ -1157,8 +1173,8 @@ volatile bool Temperature::raw_temps_ready = false;
SERIAL_EOL();
}

hal.idletask();
TERN(DWIN_CREALITY_LCD, dwinUpdate(), ui.update());
// Run minimal necessary machine tasks
tuning_manage_activity();

if (!wait_for_heatup) {
SERIAL_ECHOLNPGM(STR_MPC_AUTOTUNE_INTERRUPTED);
Expand Down