Skip to content

Refactor - #4

Open
MiguVT wants to merge 9 commits into
mainfrom
dev
Open

MiguVT wants to merge 9 commits into
mainfrom
dev

Conversation

@MiguVT

@MiguVT MiguVT commented May 5, 2026

Copy link
Copy Markdown
Owner

This pull request refactors the Valve Index HID reboot fix to use systemd services and hooks instead of ad-hoc shell commands in udev rules. The new approach improves maintainability, reliability, and cross-distro compatibility. It introduces dedicated scripts and service files, and updates the installer to handle these files. The most important changes are:

Migration to systemd-based reboot logic:

  • The udev rule (99-valve-index-reboot.rules) now triggers a systemd service (valve-index-hid-reboot@.service) on device add, replacing the previous inline shell command.
  • Added a systemd unit file (valve-index-hid-reboot@.service) that runs the reboot script safely, only if needed.
  • Introduced a dedicated reboot script (valve-index-hid-reboot.sh) to encapsulate the device reset logic, improving clarity and testability.
  • Added a systemd sleep hook (valve-index-hid-reboot) to ensure the device is reset after resume from sleep.

Installer improvements:

  • The installer (install.sh) now installs all required files (udev rule, script, service, and sleep hook), downloading them as needed, and reloads both systemd and udev to apply changes. [1] [2] [3] [4]
  • Improved file lookup and download logic in the installer, making it more robust and flexible. [1] [2]

Documentation and messaging:

  • Updated user messages to clarify that multiple files are now installed, and that the fix will apply both on device reconnect and resume from sleep. [1] [2] [3]

Copilot AI review requested due to automatic review settings May 5, 2026 11:19
@MiguVT MiguVT added the enhancement New feature or request label May 5, 2026

Copilot AI left a comment

Copy link
Copy Markdown

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 refactors the Valve Index HID reboot workaround to be driven by systemd (service + system-sleep hook) instead of running an inline shell command from the udev rule, and updates the installer to deploy the additional artifacts.

Changes:

  • Replace udev RUN+=... logic with a systemd-triggered templated unit on hidraw device add.
  • Add a dedicated reboot script and a systemd sleep hook to re-apply the reset after resume.
  • Extend install.sh to install all required files and reload systemd/udev.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/99-valve-index-reboot.rules Switches to SYSTEMD_WANTS to start a systemd service on device add.
src/valve-index-hid-reboot@.service Adds a oneshot systemd unit that runs the reboot script (guarded by a flag).
src/valve-index-hid-reboot.sh Introduces a standalone script to locate the device and issue the HID reset.
src/valve-index-hid-reboot Adds a systemd sleep hook to reset after resume (and reset the flag).
src/install.sh Installs the rule, script, unit, and sleep hook; reloads systemd and udev.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/valve-index-hid-reboot.sh Outdated
Comment thread src/valve-index-hid-reboot@.service Outdated
Comment thread src/install.sh
Comment on lines +78 to +91
warn "File $filename not found locally, downloading from GitHub…" >&2
local tmp_file
tmp_file="$(mktemp "/tmp/${filename}.XXXXXX")"

if command -v curl &>/dev/null; then
curl -fsSL "$RULE_RAW_URL" -o "$tmp_rule" || die "Failed to download rule file from GitHub."
curl -fsSL "$url" -o "$tmp_file" || die "Failed to download $filename."
elif command -v wget &>/dev/null; then
wget -qO "$tmp_rule" "$RULE_RAW_URL" || die "Failed to download rule file from GitHub."
wget -qO "$tmp_file" "$url" || die "Failed to download $filename."
else
die "Rule file not found locally and neither curl nor wget is available."
die "Neither curl nor wget is available to download $filename."
fi

[[ -s "$tmp_rule" ]] || die "Downloaded rule file is empty."
printf '%s\n' "$tmp_rule"
[[ -s "$tmp_file" ]] || die "Downloaded $filename is empty."
printf '%s\n' "$tmp_file"

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

cleanup tmp files with trap

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Done in commit 15febe9. Added a _TMP_FILES array that records each temp path immediately after mktemp, and an EXIT trap calling _cleanup_tmp to rm -f all tracked files when the script exits (normally or via die).

Comment thread src/install.sh
Comment on lines +213 to 216
step "Reloading systemd and udev…"
$SUDO systemctl daemon-reload
$SUDO udevadm control --reload-rules
$SUDO udevadm trigger --action=add --subsystem-match=hidraw

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

  • Detect systemctl and show a clear error if not present
  • Skip systemd steps when systemctl is missing

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Done in commit ffefdba. Added a systemctl check at the top of install_manual() — if systemctl is not found, the script dies early with a clear error message explaining that systemd is required (due to SYSTEMD_WANTS in the udev rule), which also skips all systemd-dependent steps.

MiguVT and others added 2 commits May 5, 2026 13:25
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot finished work on behalf of MiguVT May 5, 2026 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Doesn't trigger after suspending fixvr-git broke after an update (working now)

3 participants