MPC Edit Menu compilation error when compiling for multi-hotends revisited

[tombrazier]

Description

#24049 was merged without actually fixing the compile error. This PR fixes it by converting the lambda functions to static code inserted with macros.

In the course of resolving this, the following error was encountered:

Marlin\src\lcd\menu\menu_advanced.cpp:373:1: error: macro "SUBMENU_N_P" passed 4 arguments, but takes just 3

which is resolved by moving the lambda function declaration in macro MPC_ENTRY into a separate statement. Apparently the lambda function syntax makes the preprocessor feel a bit wobbly.

Related Issues

#24049, #23984

[tombrazier]

@zeleps do you mind testing that this works for you too?

[zeleps]

Will do it tonight if I get a chance, otherwise tomorrow morning (UTC+3)

[thinkyhead]

Please provide a configuration that produces the compile error in question. Lambdas work fine in many other cases, so whatever the issue is, I am sure it may be solved without need of an intermediate MPC_EDIT_HOTEND macro.

[zeleps]

Minor fix @370 (mind the repeat macro):

        void (*fn)() = []{ MPC_EDIT_HOTEND(MenuItemBase::itemIndex); }; 
        #define MPC_ENTRY(N) SUBMENU_N(N, MSG_MPC_EDIT, fn);

This does compile fine.

@thinkyhead please see my latest comment in #24049 for a configuration that breaks compilation.

[zeleps]

One way to address this would be to overload MENU_ITEM::action() to accept capturing lambdas. Just a thought.

[tombrazier]

Minor fix @370 (mind the repeat macro):

        void (*fn)() = []{ MPC_EDIT_HOTEND(MenuItemBase::itemIndex); }; 
        #define MPC_ENTRY(N) SUBMENU_N(N, MSG_MPC_EDIT, fn);

This does compile fine.

Does it run correctly? Especially, does it give the right indices for all the hotends in the advanced menu and submenus?

[tombrazier]

Here's a Configuration.h with minimal changes that triggers the compile error on bugfix-2.0.x:

Configuration.h.zip

[zeleps]

@tombrazier your latest commit compiles and works fine with multiple hotends.

[tombrazier]

If @zeleps's suggested fix works, I think the latest commit will work. And it uses a lambda function rather than macro.

I have only one hotend but have forced the compiler to compile this code by overriding HAS_MULTI_HOTEND. It works for that case. Have not been able to test on my printer because it is in the midst of a long print.

[tombrazier]

@tombrazier your latest commit compiles and works fine with multiple hotends.

Ah. Fab.

[tombrazier]

One way to address this would be to overload MENU_ITEM::action() to accept capturing lambdas. Just a thought.

The canonical way to do this is with std::function. I just tried it and the compiler denied all knowledge of the existence of the C++ standard library. Similar functionality could perhaps be implemented in Marlin but I am pretty sure it would require virtual functions. I think those are forbidden in the coding standards.

A better approach might be to make template versions of the action function. I just had a quick bash at that and it is not a quick task. But I think it is achievable. The main thing will be to ensure that any UI elements that retain a copy of the function pointer do not have template versions. Otherwise we end up with dangling references to local instances of closures which have long since been deleted. Having said that, are there any UI elements that do not keep a copy of the pointer? I don't know.

The main thing here is that a closure (i.e. an instance on a lambda function) stores data and can be of arbitrary size. This, therefore, cannot be stored as static data in RAM. It starts off on the stack but that instance might go out of scope before the pointer/reference is used. That leaves new/delete (or malloc/free) and smart pointers as an option.

[thinkyhead]

Alas, if we could use namespace std there would be so many other things we'd patch, such as strings. But we can't always be assured of having std and we can't always afford the overhead on small boards.

[thinkyhead]

Thanks for the test configuration! Using that for testing, I was able to reduce the changes down to a minimum. Since the main issue is the inability for lambdas to capture non-static locals, simply adding static to the mpc_edit_hotend lambda makes the anonymous lambda happy.

[zeleps]

Nice one, never crossed my mind. Modern languages have softened me :)

[Sonicboom247]

loving this, one question. will MPC be extended to the Bed or do we still leave PID autotune enabled for just the Bed?

[tombrazier]

It could be extended to the bed. I have some other priorities at present but there isn't any reason why it could not be done.

One question is whether it needs to be done. @dust on discord has an interesting suggestion to use the models for timing the heating so bed and hotend get to temperature at the same time. No more cooking filament in the hotend whilst waiting for the bed. That would be a reason to implement on the bed.

Edit: so, yes, still use PID with the bed and if you want PID autotune leave it enabled.

[Sonicboom247]

fantastic, i will leave it enabled for the bed only, my start gcode uses heat bed only to tilting the mesh, then once mesh is tilted heat nozzle on the way to print the prime line and wait. figured that is the best way for me to avoid just as you said cooking the filament while saving a bit on energy usage.

[tombrazier]

I have just changed my start code to wait for 10 degrees lower than bed target then start G29 and then wait for target temperature. Saves a bit of time and 10 degrees off does not mess up probing too much. I still need to get probe temperature compensation really working properly on my machine.

[Sonicboom247]

do tell! my OCD want the printer moving and not just sitting there waiting.

[tombrazier]

I am using PrusaSlicer. My start G-code has the following:

; set temperature
M104 S[first_layer_temperature] ; set extruder temp
M140 S[first_layer_bed_temperature] ; set bed temp

; home
G28

; lift nozzle and go to safe spot for dribbling
G1 X10 Y10 Z5 F9000

; get up to temperature
M109 S[first_layer_temperature] ; wait for extruder temp
M190 S{first_layer_bed_temperature[0] - 10} ; wait for bed temp

; level bed
G29

M190 S[first_layer_bed_temperature] ; wait for bed temp

So two M190 instructions one before G29 with S{first_layer_bed_temperature[0] - 10} and one after with S[first_layer_bed_temperature].

[tombrazier]

If you are really impatient and don't mind some manual intervention, rest a sheet of slightly rumpled aluminium foil on the bed. It removes the majority of the convective and radiative losses. Whip it away before G29 starts though!

[Sonicboom247]

If you are really impatient and don't mind some manual intervention, rest a sheet of slightly rumpled aluminium foil on the bed. It removes the majority of the convective and radiative losses. Whip it away before G29 starts though!

Haha, i like that! Thank you for the script!