Skip to content

Ubuntu 24.04.4 improvements#7

Open
Koifajardo wants to merge 2 commits intosam1am:mainfrom
Koifajardo:ubuntu-24.04.4-improvements
Open

Ubuntu 24.04.4 improvements#7
Koifajardo wants to merge 2 commits intosam1am:mainfrom
Koifajardo:ubuntu-24.04.4-improvements

Conversation

@Koifajardo
Copy link

Title:
Ubuntu 24.04.4: installer, polkit, 15W/25W presets, NBFC fixes, and Build package
Description:
This PR adds improvements focused on Ubuntu 24.04.4: an automated installer, better NBFC handling, and a ready-to-use Build package so users can install without cloning the full repo.
Installer and Build package
Build/ folder with:
install-standalone.sh – One-shot installer: installs system deps (python3-venv, python3.X-venv, python3-pip, libxcb-cursor0, lm-sensors), extracts the app bundle, creates venv, installs polkit policy, desktop shortcut, and optionally ryzenadj (snap) and nbfc (apt or .deb from nbfc-linux releases). After installing nbfc, it runs nbfc update, systemctl enable nbfc_service, and tries nbfc config -s auto and nbfc start.
RyzenMasterCommander-bundle.tar.gz – App sources only (no Build/ inside).
create-bundle.sh – Regenerates the bundle from the repo.
README.md – Installation and dependency notes (English, target: Ubuntu 24.04.4).
No auto-apply of TDP on startup; user chooses profile or 15W/25W.
15W and 25W preset buttons (25W uses --max-performance); same presets in the system tray menu.
Polkit and helpers
Polkit policy updated so ryzenadj and nbfc can run via pkexec without repeated password prompts (allow_any / allow_inactive / allow_active = yes).
ryzen-master-commander-helper and polkit policy use /snap/bin/ryzenadj when the snap is installed.
install-polkit-rmc.sh – Installs the polkit policy; run-ryzen-master-commander.sh – Runs the app with correct PYTHONPATH (no pkexec at launch to avoid Qt xcb issues).
NBFC
When the user selects a config in the NBFC dialog, set_nbfc_config and start_nbfc_service are chained with callbacks so the service is started only after the config is applied.
At startup, if NBFC is not running, the app defaults to Manual fan control to avoid “Error setting automatic fan control” and QProcess warnings.
is_nbfc_configured uses the helper script; update_nbfc_configs receives parent to avoid QProcess being destroyed early.
nbfc_manager: fixed NameErrors in async callbacks (closure over callback / process).
Other
system_utils: warn only once per session when nbfc or sensors are missing (no repeated messages every 5s).
main_window: single “automatic fan control unavailable” message per session when NBFC is not running.
main.py: dark mode detection uses PyQt6 QPalette (ColorGroup/ColorRole); src/init.py: removed import main to avoid runpy warning.
TDP profiles: LenovoV15-22W.json, Por-defecto-15W.json (tctl 70°C).
Build/README.md: dependencies listed (system, optional, Python); target Ubuntu 24.04.4.
Tested on Ubuntu 24.04.4 and a Lenovo V15 G3 (5825U) with nbfc config “Lenovo ThinkPad T14 Gen2”.

fajardo added 2 commits February 26, 2026 17:00
…25W, system tray, sin auto-aplicar TDP al inicio

- run-ryzen-master-commander.sh: lanzar app con PYTHONPATH correcto (sin pkexec para evitar error xcb)
- install-polkit-rmc.sh: instalar política polkit para ryzenadj/nbfc sin pedir contraseña
- polkit: allow_any/allow_inactive/allow_active=yes; ryzenadj vía /snap/bin/ryzenadj
- bin/ryzen-master-commander-helper: usar /snap/bin/ryzenadj
- nbfc_manager: corregir NameError en callbacks; is_nbfc_configured usa helper
- src/__init__.py: quitar import main para evitar RuntimeWarning
- src/main.py: detección tema oscuro con PyQt6 QPalette (ColorGroup/ColorRole)
- profile_manager: botones 15W y 25W (25W con --max-performance); no restaurar TDP al inicio; desplegable con opción No aplicar
- Perfiles TDP: Por-defecto-15W.json, LenovoV15-22W.json (tctl 70)
- main_window: opciones 15W y 25W en el menú del system tray

Made-with: Cursor
… en errores; lm-sensors e inicio nbfc en instalador

- nbfc_manager: al elegir perfil en el diálogo, aplicar config y luego arrancar servicio (callbacks encadenados); pasar parent a update_nbfc_configs; mismo flujo para config recomendado
- main_window: comprobar nbfc al iniciar y poner Manual si no está corriendo; mensaje de error de auto fan solo una vez por sesión; arreglo check_nbfc_running
- system_utils: avisar una sola vez por sesión si nbfc o sensors fallan (no spamear cada 5s)
- Build/install-standalone.sh: instalar lm-sensors; tras instalar nbfc ejecutar nbfc update, systemctl enable nbfc_service, nbfc config -s auto y nbfc start
- Build/README: lm-sensors en dependencias
- Build/: install-standalone.sh, create-bundle.sh, README.md, RyzenMasterCommander-bundle.tar.gz

Made-with: Cursor
@sam1am
Copy link
Owner

sam1am commented Feb 26, 2026

Thank you for your contribution! This looks to add some really good improvements at the app level, but introduces some breaking changes for non-Ubuntu systems. See below for details.

Critical Issues — Linux Compatibility

1. Hardcoded /snap/bin/ryzenadj breaks non-Ubuntu distros

The PR changes bin/ryzen-master-commander-helper and the polkit policy to hardcode /snap/bin/ryzenadj instead of /usr/bin/ryzenadj.

  • On Fedora, Arch, openSUSE, etc., ryzenadj is typically built from source and installed to /usr/bin/ or /usr/local/bin/.
  • Snap is not available on many distros (Fedora, Arch) or is actively avoided.
  • This will break TDP control on any non-snap installation.

Recommendation: The helper should search for ryzenadj dynamically:

# Find ryzenadj in common locations
for p in /usr/bin/ryzenadj /usr/local/bin/ryzenadj /snap/bin/ryzenadj; do
  [ -x "$p" ] && RYZENADJ="$p" && break
done
if [ -z "$RYZENADJ" ]; then
  RYZENADJ=$(command -v ryzenadj)
fi
pkexec "$RYZENADJ" "$@"

2. Polkit policy hardcodes snap path

Same issue in polkit/com.merrythieves.ryzenadj.policy — the exec.path annotation is changed from /usr/bin/ryzenadj to /snap/bin/ryzenadj. Polkit won't recognize ryzenadj invocations from /usr/bin/ at all on non-snap systems.

3. allow_any and allow_inactive changed to yes — security concern

The polkit policy changes from auth_admin_keep to yes for both allow_any and allow_inactive. This means:

  • Any user on the system (not just sudoers) can run ryzenadj/nbfc without any password prompt
  • allow_inactive = yes means even remote/non-console sessions get passwordless root access to these tools

The original auth_admin_keep was more appropriate — it asks for a password once and caches it for the session. Changing to yes is a privilege escalation on multi-user systems.

4. Installer scripts are Ubuntu/Debian-only

install-standalone.sh uses apt-get, dpkg, and snap exclusively. While expected given the PR title, there's no guard or early warning for non-Debian systems. The script would fail confusingly on Fedora/Arch. A simple check at the top would help:

if ! command -v apt-get &>/dev/null; then
  echo "This installer is designed for Debian/Ubuntu systems. Exiting."
  exit 1
fi

Code Quality Issues

5. Spanish-language comments and UI strings mixed with English

The existing codebase is in English. This PR introduces Spanish in:

  • All installer script comments and output messages
  • Tooltip text: "Aplica 15W (boost y sostenido). Tctl 70°C."
  • Profile dropdown: "— No aplicar —"
  • Desktop shortcut comment: "Monitorizar y controlar TDP y ventilador Ryzen"
  • Profile name: "Por-defecto-15W"

These should be in English for consistency with the rest of the project.

6. is_nbfc_configured() uses relative path

In nbfc_manager.py, the change to use bin/ryzen-master-commander-helper is a relative path. This will break unless the working directory happens to be the project root. The original sudo nbfc start worked regardless of CWD.

7. setup_nbfc() now returns True prematurely

The refactored async chain in setup_nbfc() returns True immediately after starting the async operation, not after it succeeds. Callers that check the return value to determine if NBFC is actually running will get false positives.

8. Binary tarball committed to repo

Build/RyzenMasterCommander-bundle.tar.gz is a binary blob checked into the repo. This will permanently bloat the git history. Please remove.


Minor Issues

  • Missing newline at end of JSON files — Both LenovoV15-22W.json and Por-defecto-15W.json lack a trailing newline.
  • Profile name "Por-defecto-15W" is Spanish — Profiles ship with the app and should use English names.

What's Good

These changes are solid and should be kept:

  • QProcess closure fix — Passing callback/process as default args in on_finished correctly solves Python's late-binding closure issue in nbfc_manager.py.
  • "Warn once" pattern in system_utils.py — Good UX improvement; stops spamming the console every 5s when nbfc/sensors are missing.
  • Default to manual fan control when NBFC isn't running — Prevents error messages on startup.
  • PyQt5 → PyQt6 fix in dark mode detection (main.py) — Genuine bug fix using the correct ColorGroup/ColorRole API.
  • Async chaining of set_configstart_service in NBFC setup is architecturally correct.
  • Removing from . import main in __init__.py avoids circular import / runpy warning.
  • result.stderr or "" guard in check_nbfc_running prevents potential None membership test.

Suggested path forward:

  1. Split into two PRs:

    • App fixes PR: The NBFC async fixes, warn-once, default-to-manual, PyQt6 fix, preset buttons (with English strings). This can merge quickly.
    • Ubuntu installer PR: The Build/ directory and installer scripts, clearly scoped as Ubuntu-only tooling.
  2. For both PRs, address:

    • Dynamic ryzenadj path lookup instead of hardcoding /snap/bin/
    • Revert polkit allow_any/allow_inactive back to auth_admin_keep
    • Translate all user-facing strings and comments to English
    • Remove the binary tarball from git; use GitHub Releases
    • Fix the relative path in is_nbfc_configured()

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