Skip to content

Conversation

@RafaeruAybu
Copy link

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 new in PR culture, so any suggestion are welcome.

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.
@RafaeruAybu
Copy link
Author

RafaeruAybu commented Oct 27, 2025

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.

@AdnanHodzic AdnanHodzic requested a review from Copilot October 29, 2025 08:40
Copy link

Copilot AI left a 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 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_path from the incorrect /sys/devices/system/cpu/intel_pstate to 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_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.
    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.

Comment on lines +155 to +156
except:
return None
Copy link

Copilot AI Oct 29, 2025

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 like except (FileNotFoundError, OSError, IOError) to handle expected file operation errors.

Copilot uses AI. Check for mistakes.
Comment on lines +180 to +181
except:
return None
Copy link

Copilot AI Oct 29, 2025

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 like except (FileNotFoundError, OSError, ValueError, IOError) to handle expected file operation and conversion errors.

Copilot uses AI. Check for mistakes.
Copy link
Owner

Choose a reason for hiding this comment

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

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.

Copy link
Owner

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 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:
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
@AdnanHodzic
Copy link
Owner

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:

Previously, the stats output was showing values from the config file instead of actual system values. This fixes the issue by ...

Could you please tell me how do your stats output differ before and with this PR? As besides seeing EPP setting: performance and Setting to use: "balance_performance" EPB it looks the same to me as before.

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.

2 participants