Fix PWM10+ discovery and sort order in Common.php - #10
Open
mrjacobarussell wants to merge 1 commit into
Open
mrjacobarussell wants to merge 1 commit into
mrjacobarussell wants to merge 1 commit into
Conversation
Two bugs, both hit by any hwmon device with 10+ PWM channels
(e.g. ARCTIC Fan Controller, 10 channels):
1. glob("pwm[0-9]") and find -iname 'pwm[0-9]' match exactly one
digit, so pwm10 (and above) are silently dropped from both
build_pwm_map() and list_pwm(). Fixed by globbing broadly
(pwm*) and filtering with a strict ^pwm\d+$ regex, which also
avoids matching auxiliary attributes like pwm1_enable,
pwm1_auto_point1_pwm, etc.
2. list_pwm()'s usort() used strcmp(), which sorts alphabetically
("pwm10" < "pwm2" as strings) rather than numerically. Fixed
by switching to strnatcmp().
Verified against a live 10-channel ARCTIC Fan Controller on Unraid:
before the fix, channel 10 was missing from the plugin UI entirely
and pwm10 sorted between pwm1 and pwm2; after, all 10 channels
appear in correct numeric order.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 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. Comment |
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
1 participant
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.
What's changed
Two bugs, both hit by any hwmon device with 10+ PWM channels (found via a real ARCTIC Fan Controller, which has 10 channels):
glob("pwm[0-9]")andfind -iname 'pwm[0-9]'only match a single digit, sopwm10and above are silently dropped from bothbuild_pwm_map()andlist_pwm(). Fixed by globbing broadly (pwm*) and filtering with a strict^pwm\d+$regex — this also avoids accidentally matching auxiliary attributes likepwm1_enableorpwm1_auto_point1_pwmthat a looserpwm[0-9]*pattern would pull in.list_pwm()'susort()usedstrcmp(), which sorts alphabetically ("pwm10" < "pwm2"as strings) instead of numerically. Switched tostrnatcmp().Verification
Tested against a live 10-channel ARCTIC Fan Controller on Unraid. Before the fix, channel 10 was missing from the plugin UI entirely, and once naively patched to just match
pwm10, it sorted betweenpwm1andpwm2(and a looser glob pattern pulled in every per-channel attribute, not just the base PWM control file). After this fix, all 10 channels appear, in correct numeric order, with nothing extraneous.Upgrade compatibility
No config/migration impact — this only affects which hwmon files get discovered and how they're ordered for display.