Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork332
fix: display actual EPP and EPB values in stats output#894
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Conversation
Previously, the stats output was showing values from the config fileinstead of actual system values. This fixes the issue by:1. Reading EPP directly from /sys/devices/system/cpu/cpu0/cpufreq/energy_performance_preference2. Reading EPB from /sys/devices/system/cpu/cpu0/power/energy_perf_bias and converting numeric values to string representationsThis ensures that the displayed values reflect the actual current system settingsrather than the configured values.
RafaeruAybu commentedOct 27, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I am still not sure, if this can be applied to all systems with different cpus and OS. I noticed that GTK version works good. It works by reading auto_cpufreq_stats_file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
This PR fixes thecurrent_epp andcurrent_epb methods to read actual system values instead of configuration values. The key changes include:
- Changed both methods from reading config values to reading actual hardware values from sysfs
- Corrected the
epb_pathfrom the incorrect/sys/devices/system/cpu/intel_pstateto the proper/sys/devices/system/cpu/cpu0/power/energy_perf_bias - Added numeric-to-string conversion logic for EPB values (0→"performance", 4→"balance_performance", 6→"default", 8→"balance_power", 15→"power")
Comments suppressed due to low confidence (3)
auto_cpufreq/modules/system_info.py:147
- The
is_ac_pluggedparameter is no longer used in the method implementation. This parameter should be removed to avoid confusion and maintain a clean API, as the method now reads the actual system value rather than configuration-based values.
def current_epp(is_ac_plugged: bool) -> str | None:auto_cpufreq/modules/system_info.py:155
- Except block directly handles BaseException.
except:auto_cpufreq/modules/system_info.py:180
- Except block directly handles BaseException.
except:💡Add Copilot custom instructions for smarter, more guided reviews.Learn how to get started.
| except: | ||
| returnNone |
CopilotAIOct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Bare except clause catches all exceptions including system-exiting exceptions like KeyboardInterrupt and SystemExit. Replace with a specific exception type likeexcept (FileNotFoundError, OSError, IOError) to handle expected file operation errors.
| except: | ||
| returnNone |
CopilotAIOct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Bare except clause catches all exceptions including system-exiting exceptions like KeyboardInterrupt and SystemExit. Replace with a specific exception type likeexcept (FileNotFoundError, OSError, ValueError, IOError) to handle expected file operation and conversion errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm bit bothered bycpu0 in/sys/devices/system/cpu/cpu0/power/energy_perf_bias line. Would love it could somehow be made more generic, as in some weird situation someone doesn't havecpu0 (crazy, but not impossible). User will get an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm running on Intel and using pstate driver, but replacingepb_path = "/sys/devices/system/cpu/intel_pstate" doesn't seem to affect me.
Would also be nice if these changes would be made made in more generic way dependingon which driver user is using.
| returnNone | ||
| @staticmethod | ||
| defcurrent_epb(is_ac_plugged:bool)->str|None: |
CopilotAIOct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Theis_ac_plugged parameter is no longer used in the method implementation. This parameter should be removed to avoid confusion and maintain a clean API, as the method now reads the actual system value rather than configuration-based values.
AdnanHodzic commentedOct 29, 2025
Thank you for PR, welcome to open source and let's work on getting these changes merged :) I left a couple of comments and I alsoasked on auto-cpufreq discord community for others to test these changes to make sure they work every architecture. Rest of things were raised by Copilot, nothing major but would be good if they were addressed. Also, to resolve any ambiguity:
Could you please tell me how do your stats output differ before and with this PR? As besides seeing |
Previously, the stats output was showing values from the config file instead of actual system values. This fixes the issue by:
This ensures that the displayed values reflect the actual current system settings rather than the configured values.
I am new in PR culture, so any suggestion are welcome.