fix: display actual EPP and EPB values in stats output#894
fix: display actual EPP and EPB values in stats output#894RafaeruAybu wants to merge 1 commit intoAdnanHodzic:masterfrom
Conversation
Previously, the stats output was showing values from the config file instead of actual system values. This fixes the issue by: 1. Reading EPP directly from /sys/devices/system/cpu/cpu0/cpufreq/energy_performance_preference 2. Reading EPB from /sys/devices/system/cpu/cpu0/power/energy_perf_bias and converting numeric values to string representations This ensures that the displayed values reflect the actual current system settings rather than the configured values.
|
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.
Pull Request Overview
This PR fixes the current_epp and current_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: | ||
| return None |
There was a problem hiding this comment.
Bare except clause catches all exceptions including system-exiting exceptions like KeyboardInterrupt and SystemExit. Replace with a specific exception type like except (FileNotFoundError, OSError, IOError) to handle expected file operation errors.
| except: | ||
| return None |
There was a problem hiding this comment.
Bare except clause catches all exceptions including system-exiting exceptions like KeyboardInterrupt and SystemExit. Replace with a specific exception type like except (FileNotFoundError, OSError, ValueError, IOError) to handle expected file operation and conversion errors.
There was a problem hiding this comment.
I'm bit bothered by cpu0 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 have cpu0 (crazy, but not impossible). User will get an error.
There was a problem hiding this comment.
I'm running on Intel and using pstate driver, but replacing epb_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 depending on which driver user is using.
| return None | ||
|
|
||
| @staticmethod | ||
| def current_epb(is_ac_plugged: bool) -> str | None: |
There was a problem hiding this comment.
The is_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.
|
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 also asked 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.