Drive the purifier fans through stock [fan_generic] objects - #10
Open
justinh-rahb wants to merge 3 commits into
Open
Drive the purifier fans through stock [fan_generic] objects#10justinh-rahb wants to merge 3 commits into
justinh-rahb wants to merge 3 commits into
Conversation
Builds on @paxx12's approach in paxx12-snapmaker-u1/SnapmakerU1-Extended- Firmware#670: the exhaust and inner fans move out of [purifier] into stock [fan_generic exhaust_fan] and [fan_generic circulation_fan] sections, and Purifier looks them up with load_object() instead of building PurifierFan from raw pins. PurifierFan and PurifierFanTachometer are removed - their PWM, kick-start and tachometer handling is what fan.Fan already does. That alone leaves a second writer to the fans that Purifier cannot see. [fan_generic] registers its own SET_FAN_SPEED and control/generic_fan handlers, which drive fan.Fan directly, so the power enable pin, the fan state machine, the delay-off timers, the presence gate and the inner fan work time accounting all go stale. Measured on hardware: a circulation fan started with SET_FAN_SPEED keeps spinning at 4652 rpm through both SET_PURIFIER FAN=inner SPEED=0 and SET_PURIFIER_MODE MODE=0, because set_inner_fan_delay_turn_off() early-returns on FAN_STATE_TURN_OFF. PurifierFanRouter wraps the fan.Fan held by each [fan_generic] object and overrides only set_speed_from_command(), delegating everything else through __getattr__. Both generic entry points funnel through that one method - as does the M106/M107 fan id mapping in extras/fan.py - so every path lands in Purifier.set_*_fan_speed() alongside SET_PURIFIER. Purifier itself calls set_speed(), which delegates, so there is no recursion. Routing rather than reconciling means nothing can desync, so the periodic _sync_power_enable() tick is not needed and is left out. SET_PURIFIER, GET_PURIFIER, SET_PURIFIER_MODE, WAIT_CHAMBER_TEMP and the control/purifier endpoint are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-Authored-By: paxx12 <245230251+paxx12@users.noreply.github.com>
With PurifierFanRouter there is a single write path to each fan, so _exhaust_fan_state/_inner_fan_state no longer carry anything that is not already in fan.Fan.last_fan_value: state == FAN_STATE_TURN_OFF <-> last_fan_value == 0 state != FAN_STATE_TURN_OFF <-> last_fan_value != 0 state == FAN_STATE_TURN_ON <-> clamped speed != 0 FAN_STATE_TURNING_OFF only ever meant "not off" - the fan is still spinning through the delay-off window - so the derived checks give the same answer. The clamp block is kept, which is what makes the accrual trigger equivalent. Follows @paxx12's simplification in paxx12-snapmaker-u1/SnapmakerU1-Extended- Firmware#670 once the router landed there. Re-ran on hardware, unchanged from the previous build: SET_FAN_SPEED starts the circulation fan at 4645 rpm and SET_PURIFIER FAN=inner SPEED=0 stops it; DELAY_OFF=5 against a SET_FAN_SPEED-started fan arms and fires at ~5s; the control/generic_fan webhook at S=70 runs 0.7 and a purifier stop clears it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-Authored-By: paxx12 <245230251+paxx12@users.noreply.github.com>
extras/fan.py already carries an exhaust_fan/exhaust_fan_id pair under a
comment reading "exhaust fan / purifier fan", and Snapmaker's own factory
configs use it (factory2.cfg sets exhaust_fan: purifier, exhaust_fan_id: 3).
It was unusable from printer.cfg because the purifier's fans were not
[fan_generic] objects and fan.py resolves the mapping with
lookup_object("fan_generic <name>").
Now that they are, point it at the exhaust fan so slicer-emitted M106/M107
can drive it.
This is the path that made the router necessary rather than optional:
cmd_M106/cmd_M107 call fan_obj.fan.set_speed_from_command() directly
(fan.py:192, fan.py:206), never touching the SET_FAN_SPEED mux command, so a
dispatch-level interception would not have covered it.
Verified on hardware:
M106 P3 S128 -> speed 0.502 (128/255)
M106 P3 S255 -> speed 1.0
SET_PURIFIER FAN=exhaust SPEED=0 -> speed 0 (M106 went through
set_exhaust_fan_speed, so the
purifier can stop what it started)
M106 P3 S200 then M107 P3 -> 0.784 then 0
The purifier status object mirrors the same values throughout.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: paxx12 <245230251+paxx12@users.noreply.github.com>
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
Moves the purifier's exhaust and inner fans onto stock Klipper
[fan_generic]objects, so they are addressable like every other fan on the machine —SET_FAN_SPEED FAN=exhaust_fan,M106 P3, thecontrol/generic_fanendpoint,printer.fan_generic circulation_fanin the object query API, and the Fluidd/Mainsail fan panels.Today both fans are driven by
PurifierFan/PurifierFanTachometer, private classes insidepurifier.pythat reimplement whatfan.Fanalready does (PWM setup, kick-start, tachometer). Nothing outsideSET_PURIFIERand thecontrol/purifierwebhook can see or drive them.Net effect on
klippy/extras/purifier.py: -114 lines (1197 → 1083).Approach
[purifier]no longer takes raw fan pins. The pin/PWM/tachometer config moves into[fan_generic exhaust_fan]and[fan_generic circulation_fan], and[purifier]references them by name viaexhaust_fan_name/inner_fan_name.Purifier.__init__resolves them withload_object(config, 'fan_generic ' + name).PurifierFanandPurifierFanTachometerare deleted.PurifierFanRouterwraps thefan.Faneach[fan_generic]object holds and overrides onlyset_speed_from_command(), delegating everything else through__getattr__.Step 3 is what makes step 1 safe, and is the part worth reviewing.
Why the router is required
[fan_generic]registers its ownSET_FAN_SPEEDmux command andcontrol/generic_fanendpoint, andextras/fan.pyreaches throughfan_obj.fan.set_speed_from_command()forM106/M107fan-id mapping (fan.py:192,fan.py:206) and for power-loss resume (resume_all_fan_speed(),fan.py:178).All of those drive
fan.Fandirectly, bypassingset_exhaust_fan_speed()/set_inner_fan_speed()and everything they own:power_enable_pin— the fan power rail. Never asserted, so the fan gets a PWM duty cycle with no power._power_detectedpresence gateinner_work_time, which tracks filter lifeEvery one of those entry points converges on
set_speed_from_command(), so routing that single method covers all of them, with no changes tofan_generic.py, no dependency on config section order, and no reliance ongcode/webhooksinternals.Purifier.set_*_fan_speed()calls the real fan'sset_speed(), which the router does not intercept, so there is no recursion.Measured without the router, on hardware: a circulation fan started with
SET_FAN_SPEEDkeeps spinning at 4652 rpm through bothSET_PURIFIER FAN=inner SPEED=0andSET_PURIFIER_MODE MODE=0, becauseset_inner_fan_delay_turn_off()returns early onFAN_STATE_TURN_OFF.Fan state enum removed
With one write path per fan,
_exhaust_fan_state/_inner_fan_stateno longer carry anything not already infan.Fan.last_fan_value:state == FAN_STATE_TURN_OFFlast_fan_value == 0state != FAN_STATE_TURN_OFFlast_fan_value != 0state == FAN_STATE_TURN_ONspeed != 0FAN_STATE_TURNING_OFFonly ever meant "not off" — the fan is still spinning through the delay-off window — so the derived checks give the same answer. The speed clamp is kept, which is what keeps the accrual trigger equivalent.Unchanged
SET_PURIFIER,GET_PURIFIER,SET_PURIFIER_MODE,WAIT_CHAMBER_TEMPand thecontrol/purifierwebhook are untouched, as are the chamber modes, dynamic cooling ramp, RPM fault detection, presence detection and thepurifierstatus object.Config change
[purifier]sections carrying raw pins will no longer load —exhaust_fan_name/inner_fan_nameare required.lava/printer.cfgis updated to match; see the diff for the exact split.The module and the config ship together in a firmware image, so the two always move as a pair and there is no migration path to support. The required-with-no-default choice is deliberate on that basis: a
[purifier]missing either key is a packaging error worth failing loudly on, not a machine that should come up with a silently absent fan.M106/M107 fan-id mapping
extras/fan.pyalready carries anexhaust_fan/exhaust_fan_idpair under a comment reading "exhaust fan / purifier fan", and Snapmaker's own factory configs use it (factory2.cfgsetsexhaust_fan: purifier,exhaust_fan_id: 3). It was unusable fromprinter.cfgbecause the purifier's fans were not[fan_generic]objects andfan.pyresolves the mapping withlookup_object("fan_generic <name>").lava/printer.cfgnow points it at the exhaust fan, so slicer-emittedM106 P3/M107 P3drive it.This is also the case that makes the router necessary rather than merely tidy:
cmd_M106/cmd_M107callfan_obj.fan.set_speed_from_command()directly (fan.py:192,fan.py:206) and never touch theSET_FAN_SPEEDmux command, so intercepting at the dispatch layer would not have covered them.Testing
On a U1 with a purifier attached, Klipper
1.6.0.267_20260815150420:SET_FAN_SPEED FAN=circulation_fan SPEED=0.6SET_PURIFIER FAN=inner SPEED=0after the aboveSET_FAN_SPEED FAN=circulation_fan SPEED=1.0DELAY_OFF=5against aSET_FAN_SPEED-started fancontrol/generic_fanS=70, thenSET_PURIFIER ... SPEED=0SET_FAN_SPEED FAN=exhaust_fan SPEED=0.5purifierreportsexhaust_fan.speed: 0.5SET_PURIFIER FAN=inner SPEED=0.3SET_FAN_SPEEDrefused whilepower_detectedis falseM106 P3 S128M106 P3 S255SET_PURIFIER FAN=exhaust SPEED=0afterM106 P3M106startedM106 P3 S200thenM107 P3The delay-off row is the meaningful one for the state removal: delay-off only arms when the fan reads non-zero, so a timer firing on a fan that
SET_FAN_SPEEDstarted shows the derived check matches the old enum.Credit
The
[fan_generic]migration and thePurifierFanremoval are @paxx12's design, from paxx12-snapmaker-u1/SnapmakerU1-Extended-Firmware#670. This PR carries the same approach against a stock 1.6.0 tree, adds the router, and drops the state enum. The two implementations converged independently and are functionally equivalent.Offering it upstream so the community firmware and mainline do not diverge on this, and so the fix lands where the
exhaust_fan_idhook already lives.🤖 Generated with Claude Code