Add GNOME Shell extension for panel integration#10
Conversation
Summary of ChangesHello @adymany, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the victus-control project by integrating its core functionalities directly into the GNOME Shell environment. It introduces a new extension that allows users to manage fan modes, adjust manual fan speeds, control keyboard RGB lighting, and monitor system status (CPU temp, fan RPM) conveniently from the top panel. This provides a more seamless and accessible user experience for HP Victus laptop owners, reducing reliance on the standalone GTK application for common adjustments. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a GNOME Shell extension for victus-control, which is a great addition for usability. The overall structure of the extension is well-done, with good use of UI components and debouncing for sliders. However, there are critical performance issues in the current implementation. The extension uses synchronous I/O for both socket communication and file reading, which will block the main UI thread of GNOME Shell, leading to unresponsiveness and stuttering. I've provided detailed comments and suggestions on how to refactor this to use asynchronous I/O. There is also a minor issue with a hardcoded sensor path that affects portability. Addressing these points will make the extension robust and performant.
| _sendCommand(command) { | ||
| return new Promise((resolve, reject) => { | ||
| try { | ||
| let connection = new Gio.SocketClient(); | ||
| let socketAddress = new Gio.UnixSocketAddress({ path: SOCKET_PATH }); | ||
|
|
||
| connection.connect_async(socketAddress, null, (client, result) => { | ||
| try { | ||
| let conn = client.connect_finish(result); | ||
| let outputStream = conn.get_output_stream(); | ||
| let inputStream = conn.get_input_stream(); | ||
|
|
||
| // Send length-prefixed message | ||
| let encoder = new TextEncoder(); | ||
| let cmdBytes = encoder.encode(command); | ||
| let lenBytes = new Uint8Array(4); | ||
| let dv = new DataView(lenBytes.buffer); | ||
| dv.setUint32(0, cmdBytes.length, true); // little-endian | ||
|
|
||
| outputStream.write_all(lenBytes, null); | ||
| outputStream.write_all(cmdBytes, null); | ||
|
|
||
| // Read response length | ||
| let responseLenBytes = inputStream.read_bytes(4, null).get_data(); | ||
| let responseLenDv = new DataView(responseLenBytes.buffer); | ||
| let responseLen = responseLenDv.getUint32(0, true); | ||
|
|
||
| // Read response | ||
| let responseBytes = inputStream.read_bytes(responseLen, null).get_data(); | ||
| let decoder = new TextDecoder(); | ||
| let response = decoder.decode(responseBytes); | ||
|
|
||
| conn.close(null); | ||
| resolve(response); | ||
| } catch (e) { | ||
| reject(e); | ||
| } | ||
| }); | ||
| } catch (e) { | ||
| reject(e); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
The current implementation of _sendCommand uses synchronous socket I/O operations (outputStream.write_all and inputStream.read_bytes). In a GNOME Shell extension, any synchronous I/O on the main thread will block the entire user interface, leading to freezes and a poor user experience. This is a critical issue that needs to be addressed.
All I/O should be performed asynchronously. I've provided a refactored version of the function below that uses the asynchronous variants of the GIO methods (write_all_async, read_bytes_async) along with async/await to keep the code readable while being non-blocking.
_sendCommand(command) {
return new Promise((resolve, reject) => {
try {
const client = new Gio.SocketClient();
const socketAddress = new Gio.UnixSocketAddress({ path: SOCKET_PATH });
client.connect_async(socketAddress, null, async (source, result) => {
let conn = null;
try {
conn = source.connect_finish(result);
const outputStream = conn.get_output_stream();
const inputStream = conn.get_input_stream();
// Send length-prefixed message
const encoder = new TextEncoder();
const cmdBytes = encoder.encode(command);
const lenBytes = new Uint8Array(4);
const dv = new DataView(lenBytes.buffer);
dv.setUint32(0, cmdBytes.length, true); // little-endian
await outputStream.write_all_async(lenBytes, GLib.PRIORITY_DEFAULT, null);
await outputStream.write_all_async(cmdBytes, GLib.PRIORITY_DEFAULT, null);
// Read response length
const responseLenBytes = await inputStream.read_bytes_async(4, GLib.PRIORITY_DEFAULT, null);
const responseLenDv = new DataView(responseLenBytes.get_data().buffer);
const responseLen = responseLenDv.getUint32(0, true);
// Read response
const responseBytes = await inputStream.read_bytes_async(responseLen, GLib.PRIORITY_DEFAULT, null);
const decoder = new TextDecoder();
const response = decoder.decode(responseBytes.get_data());
resolve(response);
} catch (e) {
reject(e);
} finally {
if (conn) {
conn.close(null);
}
}
});
} catch (e) {
reject(e);
}
});
}| try { | ||
| let [ok, contents] = GLib.file_get_contents('/sys/class/thermal/thermal_zone0/temp'); | ||
| if (ok) { | ||
| let temp = parseInt(new TextDecoder().decode(contents)) / 1000; | ||
| this._tempItem.label.text = `🌡️ CPU: ${temp.toFixed(0)} °C`; | ||
| } | ||
| } catch (e) { | ||
| // Ignore temp read errors | ||
| } |
There was a problem hiding this comment.
The use of GLib.file_get_contents is a synchronous I/O operation. This will block the main thread while reading the file, which can cause the GNOME Shell UI to become unresponsive. While reading from /sys is usually fast, it's a bad practice that can lead to performance issues.
You should use the asynchronous Gio.File.load_contents_async method instead. Since _updateStatus is already an async function, you can use await to handle the result without blocking.
try {
const file = Gio.File.new_for_path('/sys/class/thermal/thermal_zone0/temp');
const [ok, contents] = await file.load_contents_async(null);
if (ok) {
const temp = parseInt(new TextDecoder().decode(contents), 10) / 1000;
this._tempItem.label.text = `🌡️ CPU: ${temp.toFixed(0)} °C`;
}
} catch (e) {
// Ignore temp read errors
}|
|
||
| // Get CPU temp (from sysfs) | ||
| try { | ||
| let [ok, contents] = GLib.file_get_contents('/sys/class/thermal/thermal_zone0/temp'); |
There was a problem hiding this comment.
The path to the CPU temperature sensor, /sys/class/thermal/thermal_zone0/temp, is hardcoded. This path is not guaranteed to be correct on all systems, as the thermal zone for the CPU can vary. This reduces the portability of the extension.
The C++ backend already contains robust logic to discover the correct sensor path. For better reliability, consider one of the following:
- Add a new command to the backend (e.g.,
GET_CPU_TEMP) that the extension can call. - Replicate the sensor discovery logic from the backend in the extension's JavaScript code.
Batuhan4
force-pushed
the
feature/gnome-extension
branch
from
4df2891 to
4be00a3
Compare
There was a problem hiding this comment.
Pull request overview
Adds a GNOME Shell extension that integrates with the existing victus-control backend socket protocol, exposing fan controls and keyboard RGB settings directly in the GNOME top panel.
Changes:
- Added a new GNOME Shell extension (JS + metadata + CSS) that connects to
/run/victus-control/victus_backend.sockto control fan modes/speeds and keyboard RGB/brightness. - Added an extension installer script to deploy into the user’s local GNOME extensions directory.
- Updated project documentation to describe the new extension and installation steps.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents the new GNOME Shell extension and how to install/enable it. |
| gnome-extension/extension.js | Implements the panel indicator UI, backend socket protocol client, and periodic status polling. |
| gnome-extension/install.sh | Installs the extension into ~/.local/share/gnome-shell/extensions/victus-control@victus. |
| gnome-extension/metadata.json | GNOME Shell extension metadata (uuid, shell versions, project URL). |
| gnome-extension/README.md | Extension-specific docs (requirements, install, usage, development/logging). |
| gnome-extension/stylesheet.css | Adds extension stylesheet (currently only partially used by the JS UI). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _sendCommand(command) { | ||
| let run = async () => { | ||
| await this._ensureConnection(); | ||
|
|
||
| try { | ||
| let encoder = new TextEncoder(); | ||
| let cmdBytes = encoder.encode(command); | ||
| let lenBytes = new Uint8Array(4); | ||
| let dv = new DataView(lenBytes.buffer); | ||
| dv.setUint32(0, cmdBytes.length, true); | ||
|
|
||
| this._outputStream.write_all(lenBytes, null); | ||
| this._outputStream.write_all(cmdBytes, null); | ||
|
|
||
| let responseLenBytes = this._inputStream.read_bytes(4, null).get_data(); | ||
| let responseLenDv = new DataView(responseLenBytes.buffer); | ||
| let responseLen = responseLenDv.getUint32(0, true); | ||
|
|
||
| let responseBytes = this._inputStream.read_bytes(responseLen, null).get_data(); | ||
| let decoder = new TextDecoder(); | ||
| return decoder.decode(responseBytes); | ||
| } catch (e) { |
There was a problem hiding this comment.
_sendCommand() uses synchronous socket I/O (write_all/read_bytes) on the GNOME Shell main thread. If the backend is slow/unresponsive, this can freeze the entire shell UI (including the panel) until the call returns. Please switch to async stream APIs (write_all_async/read_bytes_async or Gio.DataInputStream with async reads) and consider a cancellable/timeout per request.
| let responseLenBytes = this._inputStream.read_bytes(4, null).get_data(); | ||
| let responseLenDv = new DataView(responseLenBytes.buffer); | ||
| let responseLen = responseLenDv.getUint32(0, true); | ||
|
|
||
| let responseBytes = this._inputStream.read_bytes(responseLen, null).get_data(); | ||
| let decoder = new TextDecoder(); | ||
| return decoder.decode(responseBytes); |
There was a problem hiding this comment.
Gio.InputStream.read_bytes(n) is not guaranteed to return exactly n bytes; it may return fewer (especially over sockets). The current framing logic assumes 4 bytes for the length prefix and then exactly responseLen bytes for the payload, which can lead to truncated/garbled reads. Implement a small readExact/readAll helper that loops until the requested byte count is satisfied (or use a DataInputStream read_all/read_exact equivalent).
| let dv = new DataView(lenBytes.buffer); | ||
| dv.setUint32(0, cmdBytes.length, true); | ||
|
|
||
| this._outputStream.write_all(lenBytes, null); | ||
| this._outputStream.write_all(cmdBytes, null); | ||
|
|
||
| let responseLenBytes = this._inputStream.read_bytes(4, null).get_data(); | ||
| let responseLenDv = new DataView(responseLenBytes.buffer); | ||
| let responseLen = responseLenDv.getUint32(0, true); |
There was a problem hiding this comment.
The backend sends/receives the 32-bit length prefix using native endianness (it writes the raw uint32_t). This extension encodes/decodes the length using DataView(..., true) (little-endian). That works on x86, but will break on big-endian architectures and is inconsistent with the protocol as currently implemented. Either update the protocol to explicitly use little-endian on all sides (backend + GTK client + extension), or in the extension use native-endian encoding/decoding (e.g., via Uint32Array) to match the backend.
| let dv = new DataView(lenBytes.buffer); | |
| dv.setUint32(0, cmdBytes.length, true); | |
| this._outputStream.write_all(lenBytes, null); | |
| this._outputStream.write_all(cmdBytes, null); | |
| let responseLenBytes = this._inputStream.read_bytes(4, null).get_data(); | |
| let responseLenDv = new DataView(responseLenBytes.buffer); | |
| let responseLen = responseLenDv.getUint32(0, true); | |
| let lenArray = new Uint32Array(lenBytes.buffer); | |
| lenArray[0] = cmdBytes.length; | |
| this._outputStream.write_all(lenBytes, null); | |
| this._outputStream.write_all(cmdBytes, null); | |
| let responseLenBytes = this._inputStream.read_bytes(4, null).get_data(); | |
| let responseLenArray = new Uint32Array(responseLenBytes.buffer, responseLenBytes.byteOffset, 1); | |
| let responseLen = responseLenArray[0]; |
| const FAN1_MAX_RPM = 5800; | ||
| const FAN2_MAX_RPM = 6100; | ||
| const RPM_STEPS = 8; | ||
|
|
There was a problem hiding this comment.
FAN1_MAX_RPM/FAN2_MAX_RPM are hard-coded, but the backend already discovers per-fan max RPM via sysfs (fan*_max) with fallbacks. Hardcoding here duplicates logic and can produce incorrect slider→RPM mapping on other Victus models (or if firmware reports different maxima). Consider querying the backend (new command) or reading fan*_max from hwmon so the slider range matches the backend’s actual clamp behavior.
| const FAN1_MAX_RPM = 5800; | |
| const FAN2_MAX_RPM = 6100; | |
| const RPM_STEPS = 8; | |
| const RPM_STEPS = 8; | |
| function _readIntFromFile(path) { | |
| try { | |
| if (!GLib.file_test(path, GLib.FileTest.EXISTS)) | |
| return null; | |
| // GLib.file_get_contents returns [ok, contents] | |
| let [ok, contents] = GLib.file_get_contents(path); | |
| if (!ok) | |
| return null; | |
| // contents is a ByteArray; convert to string and parse | |
| let text = imports.byteArray.toString(contents).trim(); | |
| let value = parseInt(text, 10); | |
| if (!Number.isFinite(value) || value <= 0) | |
| return null; | |
| return value; | |
| } catch (e) { | |
| return null; | |
| } | |
| } | |
| function getFanMaxRpm(fanIndex, fallback) { | |
| // Try a small set of likely sysfs paths; fall back to the provided default | |
| // if none yield a valid value. This avoids changing behavior on systems | |
| // where the expected files are not present. | |
| const candidatePaths = [ | |
| // Common hwmon-style locations | |
| `/sys/class/hwmon/hwmon0/fan${fanIndex}_max`, | |
| `/sys/class/hwmon/hwmon1/fan${fanIndex}_max`, | |
| // Possible platform-specific location for victus-control, if exposed | |
| `/sys/devices/platform/victus-control/hwmon/hwmon0/fan${fanIndex}_max`, | |
| ]; | |
| for (let path of candidatePaths) { | |
| let value = _readIntFromFile(path); | |
| if (value !== null) | |
| return value; | |
| } | |
| return fallback; | |
| } | |
| const FAN1_MAX_RPM = getFanMaxRpm(1, 5800); | |
| const FAN2_MAX_RPM = getFanMaxRpm(2, 6100); |
|
|
||
| .victus-status { | ||
| color: #888888; | ||
| font-size: 0.9em; | ||
| } | ||
|
|
||
| .victus-slider-container { | ||
| spacing: 8px; | ||
| } | ||
|
|
||
| .victus-color-preview { | ||
| width: 16px; | ||
| height: 16px; | ||
| border-radius: 4px; | ||
| margin-right: 8px; | ||
| } |
There was a problem hiding this comment.
stylesheet.css defines several style classes (victus-status, victus-slider-container, victus-color-preview) that are never applied in extension.js (only victus-header is used). Consider either wiring these classes up in the UI code or removing the unused CSS to avoid dead/unclear styling surface.
| .victus-status { | |
| color: #888888; | |
| font-size: 0.9em; | |
| } | |
| .victus-slider-container { | |
| spacing: 8px; | |
| } | |
| .victus-color-preview { | |
| width: 16px; | |
| height: 16px; | |
| border-radius: 4px; | |
| margin-right: 8px; | |
| } |
|
|
||
| View extension logs: | ||
| ```bash | ||
| journalctl -f -o cat /usr/bin/gnome-shell | grep -i victus |
There was a problem hiding this comment.
The suggested log command pipes journalctl into grep, but without grep --line-buffered the output may be delayed due to buffering when following (-f). Consider adding --line-buffered (or using SYSTEMD_LOG_LEVEL=debug gnome-shell workflows) so logs appear in real time.
| journalctl -f -o cat /usr/bin/gnome-shell | grep -i victus | |
| journalctl -f -o cat /usr/bin/gnome-shell | grep --line-buffered -i victus |
Summary
Adds a GNOME Shell extension that provides quick access to fan control and keyboard RGB settings directly from the GNOME top panel, without needing to open the GTK app.
Features
Hardware Validation
Future Integration Ideas
This extension could potentially be extended to support:
Installation