MPCTEMP improvements

[StevilKnevil]

Description

I've broken down the large MPCTEMP auto-tuning function into a simpler function containing the maths logic and a helper class MPC_autotuner that performs and hold the results of all the measurements.

It may be hard to read this pull request due to the restructuring of the code, however to summarise the code locations on bugfix_2.1.x:

• The housekeeping lambda (ln 935) and OnExit handler (ln 936) have been moved into MPC_autotuner class`
• The determination of ambient temperature (line 1009) has been moved into MPC_autotuner::measure_ambient_temp()
• The measurement of heatup (line 1037) has been moved into MPC_autotuner::measure_heatup()
• The measurement of heat transfer (line 1092) has been moved into MPC_autotuner::measure_transfer()

This branch also adds the functionality to allow MPC autotuning to use an alternative method to establish model parameters from the rate of change of heating.

The tuning algorithm falls back to this method if the original (asymptotic) method fails to produce sensible values for the sensor_responsiveness and block_capacity

Details

MPC_autotuner::housekeeping() and MPC_autotuner::init_timers() handles the management of timers by keeping track of current time and next report times.

Literals (e.g. 10000UL) have been converted to const variables (e.g. const millis_t test_interval_ms = 1000UL;) to improve code readability

Added enum return type:

enum MeasurementState {
  CANCELLED,
  FAILED,
  SUCCESS
};

to improve readability of return from the housekeeping function. Used to be a bool with true meaning the test has been cancelled by M108, now just returns CANCELLED.

Brought consistency to all variables by following the convention of using the _ms postfix to indicate times in milliseconds

Added a MS_TO_SEC_PRECISE(N) macro to turn a time in milliseconds into time in (fractions of) seconds.

Used MS_TO_SEC_PRECISE and SEC_TO_MS macros to make conversions more readable (rather than using literals).

'M306 T' Command has been extended to allow 'M306 TA' and 'M306 TD' to force asymptotic and differential tuning. Omitting the A or D will attempt asymptotic first and fall back to differential. See: MarlinFirmware/MarlinDocumentation#491

Requirements

None

Benefits

The code is clearer to reason about. Each measurement step is isolated in it's own function separated from the business logic of using the measurements to calculate the parameters of MPC temp.

This makes the MPC Temperature auto tuning more reliable in situations where the original method fails.

Gives a solid basis for me to be able to implement: #25496

Configurations

None - works in all configs.

Related Issues

Gives a solid basis for me to be able to implement: #25496
Related documentation changes: MarlinFirmware/MarlinDocumentation#491

[StevilKnevil]

@StevilKnevil I was trying to test this because of the failure of current autotuning (revo has PTC heater) but I see that temp array is defined but never initialized.

Edit: I manually cherry-picked this PR, maybe I missed something. Edit2: I replaced it with temp_samples[1]

Ooof - good catch, and good fix :)

[GMagician]

I need more checks to see if my "manual" cherry-pick is ok but it seems not to cool down correctly (M360 T S0). It started to cool down but stopped at 110C°

[GMagician]

in method measure_ambient_temp shouldn't the variable ambient_temp be initialized after current_temp?

[GMagician]

I changed to: ambient_temp = current_temp = degHotend(e);

[StevilKnevil]

measure_ambient_temp

Yes, you're quite right!

[thinkyhead]

By changing some of the general-purpose elements of the MPC_autotuner class to static I was able to reduce the build size pretty significantly. For AVR with a single extruder the build size went from 60824 / 2512 to 59170 / 2591, a savings of 1654 bytes of Flash, but permanently using up 79 bytes of SRAM. This makes it only 576 bytes larger than the current MPC_AUTOTUNE while using 103 more bytes of SRAM, which isn't too bad.

I have (for the moment) preserved both implementations, although that now seems less important, especially considering that MPCTEMP already uses up 3494 more bytes than PIDTEMP when auto-tuning is enabled — even though MPCTEMP alone is actually quite a bit smaller than PIDTEMP. If we can reduce the general size of auto-tuning, that would be helpful, but at the moment it seems reasonably optimal.

[StevilKnevil]

By changing some of the general-purpose elements of the MPC_autotuner class to static I was able to reduce the build size pretty significantly.

Oh wow - good work! I was pondering whether there would be value in having a statically allocated global "scratchpad" of memory allocated that could be used by systems as working memory rather than each system having it's own workspace. For example you won't be running Temperature Tuning and Bed Levelling at the same time, so if there was a way of allowing those two systems to share their working memory that may be of benefit.