Question: Generic Fan 0 & 1 and gcode_macro M106 & M107

[Agash]

Sovol defined the part cooling fans as two separate generic fans and sets them correctly using the gcode_macro. I don't think that is necessary, as both fans aren't individually controlled, hence a virtual pin would suffice. It would also rid the sovol_macros.cfg of some lines of code.

Hence, in the config, instead of:

[fan_generic fan0] # back model cooling fan
pin: extra_mcu:PA7
max_power: 1.0

[fan_generic fan1] # front model cooling fan
pin: extra_mcu:PB1
max_power: 1.0

We'd have

[multi_pin print_cooling_fan_pins]
pins: extra_mcu:PA7, extra_mcu:PB1

# print/part cooling fan
[fan]
pin: multi_pin:print_cooling_fan_pins
max_power: 1.0

Which would enable us to get rid of following lines in the sovol-macros.cfg:

[gcode_macro M106]
gcode:
    {% set fan = 'fan' + (params.P|int if params.P is defined else 0)|string %}
    {% set speed = (params.S|float / 255 if params.S is defined else 1.0) %}
    {% if fan == 'fan3'%}
            SET_FAN_SPEED FAN={fan} SPEED={speed}
    {% else %}
        SET_FAN_SPEED FAN={'fan0'} SPEED={speed}
        SET_FAN_SPEED FAN={'fan1'} SPEED={speed}
    {% endif %}

[gcode_macro M107]
gcode:
    {% set fan = 'fan' + (params.P|int if params.P is defined else 0)|string %}
    {% if fan == 'fan3'%}
            SET_FAN_SPEED FAN={fan} SPEED=0
    {% else %}
        SET_FAN_SPEED FAN={'fan0'} SPEED=0
        SET_FAN_SPEED FAN={'fan1'} SPEED=0
    {% endif %}

As I currently don't have the SV08 yet, I can't really test this, but it seems to check out config wise. What is the general consensus on this? I would have liked to discuss this in Discussions, but it isn't enabled in this repo, hence the Issue.

[Rappetor]

Yeah, disabled (or rather not enabled) the discussion because most of all this is discussed on discord.

While I like the idea of a shorter cfg (and not overriding default commands like M106 and M107). Maybe these changes are better for people to do on their own? Trying to keep things somewhat as stock as possible. And even though the way they solved this makes me want to cry, it does work and doesn't cause any issues 😆

[Agash]

I was just thinking since the functionality as with these changes would be "stock" still, just more aligned with the whole mainline klipper theme, it would fit quite well. I'm eager to hear more opinions thought.

Which Discord btw, the official one or is there another?

[Rappetor]

Yeah appears there are more, the unofficial one; https://discord.gg/zzu564CKCe

[mon5termatt]

Started working on this in #31

[Rappetor]

Merged! And I already replied on discord, but I'll post here as well. If we put the part cooling config under just [fan] in the printer config then maybe we don't need to M106 and M107 overrides anymore? Or do they do something special with that 'fan3' in there? (like some gcode they send when people have the enclosure?) edit, but like Agash mentioned above. Since we are changing it anyway I guess we can just as well do it the 'proper' way with [fan] 😄

[mon5termatt]

I haven't had the time to do the change to my printer for just using [fan] and therefore don't want to change it on the main repo yet. Once I can test that, I can make the change here.

[Agash]

@Rappetor the only real place I see where another change is necessary is in the LCD config. It uses the modified M106 and M107 gcodes to set the exhaust fan speed but we can easily change that to SET_FAN_SPEED gcode, right next to the LED changes by @mon5termatt from the last PR.

[mon5termatt]

Tested just [fan] last night. Seens to work just fine.

[Agash]

Changed it over to [fan], fixed some minor bugs with fan0 still being referenced and added some minor QoL improvement to print_start (uncommented), print_end (uncommented) and the LCD.

Would someone be down to test this real quick? My SV08 is still in transit and probably will be for a while since it seems to move to the wrong direction lol

[Rappetor]

Hope to test this soon (today or tomorrow). But it all looks ok to me.

[Agash]

then I'll wait with closing this issue until further testing even though the PR is merged.

[Rappetor]

Just gave it a quick test, the part cooling fans works as expected 👍 And also the fans from the stock display menu work as they should! 👍

[Agash]

Neat, thank you for testing. I'll close the issue then.