Skip to content

Conversation

@MayorBug
Copy link

@MayorBug MayorBug commented Oct 18, 2025

This PR fixes incorrect Orbitool O2S fan pin definitions and enables the 4-pin dedicated fan option in the configurator.

Changes:

  • Exposed “Digital PWM on 4-pin port” in the configurator
  • Fixed fan pin aliases to match actual hardware
  • Remove unnecessary 4-pin tachometer pin definitions. Previously, the configurator added tachometer fields even for 2-pin fans, which caused a 0 RPM readout on non-tacho setups.

Changes tested on dev-deployment branch and real hardware:

  • 5015 24 V 2-pin fan (Input Voltage PWM) ✅
  • Arctic S4028-15K 12 V 4-pin fan (Digital PWM on 4-pin port) ✅

Summary by CodeRabbit

  • New Features
    • Support for up to 2 four‑pin fan connectors.
    • Added configuration for a Sherpa Micro extruder.
  • Chores
    • Updated fan and cooling pin mappings and board pin inversion mapping.
  • Bug Fixes
    • Removed tachometer configuration fields (tachometer/ppr/poll) from fan and heater‑fan configurations.

Updated board-definition.json for ldo-orbitool-o2s
Updated toolboard fan pin aliases to match real Orbitool O2s hardware
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 18, 2025

Walkthrough

Change board configuration for ldo-orbitool-o2s: swap inverted pin logic PB0 → PA10, add fourPinFanConnectorCount: 2, remove tachometer-related fields from fan and heater_fan parameters, reorganize fan / 4‑pin fan pin aliases in the toolboard config, and add a new Sherpa Micro extruder config.

Changes

Cohort / File(s) Summary
Board definition
configuration/boards/ldo-orbitool-o2s/board-definition.json
Updated invertPinLogic from ["PB0","PB15"] to ["PA10","PB15"]; added fourPinFanConnectorCount: 2; removed tachometer fields under fan.parameters and heater_fan.parameters (tachometer_pin, tachometer_ppr, tachometer_poll_interval).
Toolboard config / pin aliases
configuration/boards/ldo-orbitool-o2s/toolboard-config.cfg
Remapped fan aliases: fan_part_cooling_pin moved from PB0 to PA10, fan_toolhead_cooling_pin set to PB15; reorganized 4‑pin fan alias lines and tach pin assignments (4p_fan_part_cooling_pin=PB0 with 4p_fan_part_cooling_tach_pin=PA14, 4p_toolhead_cooling_pin=PB15 with 4p_toolhead_cooling_tach_pin=PB12).
Extruder config (new)
configuration/extruders/sherpa-micro.cfg
Added Sherpa Micro extruder configuration: rotation_distance, gear_ratio, microsteps, max_extrude_only_* limits, filament_diameter, firmware_retraction settings, and includes external stepper config reference.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "Fix Orbitool O2S fan pin definitions and add 4-pin fan support" directly and accurately reflects the main changes in the changeset. The title captures both primary modifications: correcting fan pin definitions (invertPinLogic update from ["PB0","PB15"] to ["PA10","PB15"], fan_part_cooling_pin change from PB0 to PA10) and adding 4-pin fan support (fourPinFanConnectorCount field addition and 4-pin fan alias reorganization). The title is concise, clear, and specific enough for a teammate to understand the primary intent of the PR without additional context. While the changeset includes a new Sherpa Micro extruder configuration, the evaluation criteria acknowledge that comprehensive coverage is not required, and the title appropriately prioritizes the most significant changes.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
configuration/boards/ldo-orbitool-o2s/toolboard-config.cfg (1)

17-17: Consider trailing comma consistency.

The trailing comma added here is stylistically inconsistent with other non-fan pin entries (lines 11-15), though it's functionally fine and aids future edits.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bdffe02 and af8bff3.

📒 Files selected for processing (2)
  • configuration/boards/ldo-orbitool-o2s/board-definition.json (1 hunks)
  • configuration/boards/ldo-orbitool-o2s/toolboard-config.cfg (1 hunks)
🔇 Additional comments (4)
configuration/boards/ldo-orbitool-o2s/board-definition.json (1)

16-17: LGTM! Pin logic and 4-pin fan configuration correctly updated.

The invertPinLogic change from PB0 to PA10 aligns with the fan pin reassignment in toolboard-config.cfg, and fourPinFanConnectorCount: 2 correctly reflects the two 4-pin fan connectors (part cooling and toolhead cooling) exposed in the pin aliases.

configuration/boards/ldo-orbitool-o2s/toolboard-config.cfg (3)

19-20: LGTM! Fan pin reassignments correctly implemented.

The pin changes align with the board-definition.json updates and match the PR's hardware-tested configuration. The regular fan part cooling pin moved from PB0 to PA10, freeing PB0 for the 4-pin fan connector.


22-23: Verify pin sharing between 2-pin and 4-pin toolhead cooling fans.

Both fan_toolhead_cooling_pin (line 20) and 4p_toolhead_cooling_pin (line 23) are assigned to PB15. If this represents the same physical connector with alternative configurations (2-pin vs 4-pin), this is correct. However, if these are meant to be independent connectors, this creates a pin conflict.

Please confirm that:

  1. The toolhead cooling connector can be used in either 2-pin or 4-pin mode (mutually exclusive)
  2. Users won't attempt to configure both simultaneously

If these are separate physical connectors, the pin assignments need correction.


22-23: Excellent addition of 4-pin fan support with tachometer pins.

The 4-pin fan definitions correctly expose PWM and tachometer pins for both part cooling and toolhead cooling fans. The tachometer pin assignments (PA14, PB12) match the customSections in board-definition.json, and the feature was validated with real hardware testing.

Remove unnecessary 4-pin tachometer pin definitions.
Previously, the configurator added tachometer fields even for 2-pin fans, which caused a 0 RPM readout on non-tacho setups.
@MayorBug MayorBug changed the title Fix Orbitool O2s fan pin definitions and add 4-pin fan support Fix Orbitool O2S fan pin definitions and add 4-pin fan support Oct 19, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e3b3668 and f992532.

📒 Files selected for processing (1)
  • configuration/extruders/sherpa-micro.cfg (1 hunks)
🔇 Additional comments (2)
configuration/extruders/sherpa-micro.cfg (2)

18-18: Verify the high max_extrude_only_distance value.

A max_extrude_only_distance of 1400mm is unusually high. While this might be intentional for loading/unloading operations with long Bowden setups, please confirm this value is correct for the Sherpa Micro's typical use case.


15-15: Rotation_distance value verified as correct.

The rotation_distance value of 22.67895 matches the standard Klipper starting value for the Sherpa Micro with 5mm Bondtech-style drive gears and is confirmed by official Annex Engineering and community configurations. For non-standard configurations (different motor pinion tooth counts or alternative drive gears), users should calibrate by marking 120mm, extruding 100mm, and adjusting the value if needed.


[extruder]
rotation_distance: 22.67895
gear_ratio: 50:8 #for sherpa mini 8t motor
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify naming: "Sherpa Micro" vs "sherpa mini".

The metadata at line 8 identifies this as "Annex Engineering Sherpa Micro," but this comment refers to "sherpa mini 8t motor." Please confirm whether these terms are interchangeable or if there's a naming inconsistency that should be corrected.


🏁 Script executed:

#!/bin/bash
# Search for other references to "sherpa" variants in the codebase to check naming conventions
rg -i -C2 'sherpa.*(mini|micro)'

Length of output: 1832



---

🌐 Web query:

```web
Annex Engineering Sherpa Micro extruder motor specifications 8t

💡 Result:

Brief specs for the Sherpa Micro "8T" extruder motor (commonly sold for Annex Engineering Sherpa Micro):

  • Motor type / model family: NEMA14 stepper — LDO 36STH series (often listed as LDO‑36STH20‑1004AHG / 0354AHG with G8T). [1][2]
  • Integrated gear: 8‑tooth pinion (8T) sized to mate with Sherpa Micro drivetrain (gives the Sherpa ~50:8 effective gearing). [2][3]
  • Body length / package: 36 mm / round NEMA14 with ~20 mm body length variant commonly sold for Sherpa (20 mm motor length). [3]
  • Intended use / features: high‑temperature rated extruder motor for Sherpa Mini/Micro and similar compact extruders. Market listings state improved pushing force for high‑performance extrusion. [1][2]
  • Typical seller claims: effective extrusion push up to ~10 kg (vendor spec). Use manufacturer datasheet for exact torque/current. [2]

Sources / vendor pages: Annex Engineering product listing (Sherpa Micro), LDO 36STH 8T product listings. [1][2][3]

If you want exact electrical specs (holding torque, step angle, rated current, inductance) I can fetch the LDO 36STH20‑1004AHG datasheet and paste the precise values.


Update comment on line 16 to reflect correct extruder variant.

The comment refers to "sherpa mini" but this configuration file is for the Sherpa Micro (as specified in the metadata on line 8 and the stepper model LDO-36STH20-1004AHG). The Sherpa Micro does use an 8T motor, so update the comment from #for sherpa mini 8t motor to #for sherpa micro 8t motor.

🤖 Prompt for AI Agents
In configuration/extruders/sherpa-micro.cfg around line 16, the inline comment
incorrectly references "sherpa mini" even though this file is for the Sherpa
Micro; edit the comment text on that line to read "#for sherpa micro 8t motor"
so it correctly matches the file metadata and hardware variant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant