feat(matic): improve power management for lid close and AC power - #1140
Conversation
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
Summary of ChangesHello, 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 refines the power management behavior for the 'matic' host, specifically addressing how the system handles lid closure and AC power. The changes aim to provide a more user-friendly experience by preventing immediate suspension on lid close and ensuring the system remains active when plugged into AC power, while still maintaining idle suspension on battery. Highlights
Changelog
Activity
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
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughPower management configuration updates across two files: Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Mesa DescriptionTL;DRImproved power management on What changed?
Description generated by Mesa. Update settings |
There was a problem hiding this comment.
Code Review
This pull request improves power management on the 'matic' host by delegating lid-close actions to hypridle and preventing auto-suspension when on AC power. The changes are logical and correctly implemented. I've added a couple of suggestions for minor improvements: one to simplify the AC power check in hypridle.conf for better efficiency and robustness, and another to remove a redundant configuration line in default.nix for conciseness.
| listener { | ||
| timeout = 1800 | ||
| on-timeout = systemctl suspend | ||
| on-timeout = bash -c 'cat /sys/class/power_supply/AC*/online | grep -q 1 || systemctl suspend' |
There was a problem hiding this comment.
The command to check for AC power can be simplified by using grep directly on the file, which avoids the need for cat and a pipe. This is slightly more efficient. I've also added 2>/dev/null to suppress potential error messages if the power supply path doesn't exist, making the execution cleaner.
on-timeout = bash -c 'grep -q 1 /sys/class/power_supply/AC*/online 2>/dev/null || systemctl suspend'
| services.logind.settings.Login.HandleLidSwitch = "ignore"; | ||
| services.logind.settings.Login.HandleLidSwitchExternalPower = "ignore"; |
There was a problem hiding this comment.
The HandleLidSwitchExternalPower setting is redundant here. According to the logind.conf documentation, if HandleLidSwitchExternalPower is not set, it defaults to the value of HandleLidSwitch. Since you are setting both to ignore, you can safely remove the HandleLidSwitchExternalPower line to make the configuration more concise.
services.logind.settings.Login.HandleLidSwitch = "ignore";
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adjusts power-management behavior on the matic host so lid-close no longer triggers an immediate suspend via logind, and Hyprland’s idle daemon (hypridle) only suspends when on battery power.
Changes:
- Configure
systemd-logindto ignore lid-close events (on battery and external power) so idle timers control suspension. - Update
hypridle’s 30-minute suspend listener to conditionally suspend only when not on AC power.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| named-hosts/matic/default.nix | Sets logind lid-switch handling to ignore so lid close doesn’t force immediate suspend. |
| config/hyprland/hypridle.conf | Adds an AC-power check before running systemctl suspend for the 30-minute idle action. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| listener { | ||
| timeout = 1800 | ||
| on-timeout = systemctl suspend | ||
| on-timeout = bash -c 'cat /sys/class/power_supply/AC*/online | grep -q 1 || systemctl suspend' |
There was a problem hiding this comment.
2 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="named-hosts/matic/default.nix">
<violation number="1" location="named-hosts/matic/default.nix:136">
P2: Delegating lid-close handling entirely to `hypridle` breaks suspend at the greeter/logout path. Since `hypridle` only runs inside a logged-in Hyprland session, closing the lid before login or after logout will now be ignored and the laptop stays awake.</violation>
</file>
<file name="config/hyprland/hypridle.conf">
<violation number="1" location="config/hyprland/hypridle.conf:31">
P2: This AC-power check only matches `online=1`, but the kernel ABI also allows `online=2` for programmable external supplies. On those chargers, the idle action will still suspend while plugged in.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| # Power button behavior - lock screen instead of shutdown | ||
| services.logind.settings.Login.HandlePowerKey = "lock"; | ||
| # Ignore lid close — let hypridle's 30-min idle timer handle suspension | ||
| services.logind.settings.Login.HandleLidSwitch = "ignore"; |
There was a problem hiding this comment.
P2: Delegating lid-close handling entirely to hypridle breaks suspend at the greeter/logout path. Since hypridle only runs inside a logged-in Hyprland session, closing the lid before login or after logout will now be ignored and the laptop stays awake.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At named-hosts/matic/default.nix, line 136:
<comment>Delegating lid-close handling entirely to `hypridle` breaks suspend at the greeter/logout path. Since `hypridle` only runs inside a logged-in Hyprland session, closing the lid before login or after logout will now be ignored and the laptop stays awake.</comment>
<file context>
@@ -132,6 +132,9 @@ inputs.nixpkgs.lib.nixosSystem {
# Power button behavior - lock screen instead of shutdown
services.logind.settings.Login.HandlePowerKey = "lock";
+ # Ignore lid close — let hypridle's 30-min idle timer handle suspension
+ services.logind.settings.Login.HandleLidSwitch = "ignore";
+ services.logind.settings.Login.HandleLidSwitchExternalPower = "ignore";
</file context>
| listener { | ||
| timeout = 1800 | ||
| on-timeout = systemctl suspend | ||
| on-timeout = bash -c 'cat /sys/class/power_supply/AC*/online | grep -q 1 || systemctl suspend' |
There was a problem hiding this comment.
P2: This AC-power check only matches online=1, but the kernel ABI also allows online=2 for programmable external supplies. On those chargers, the idle action will still suspend while plugged in.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/hyprland/hypridle.conf, line 31:
<comment>This AC-power check only matches `online=1`, but the kernel ABI also allows `online=2` for programmable external supplies. On those chargers, the idle action will still suspend while plugged in.</comment>
<file context>
@@ -25,8 +25,8 @@ listener {
listener {
timeout = 1800
- on-timeout = systemctl suspend
+ on-timeout = bash -c 'cat /sys/class/power_supply/AC*/online | grep -q 1 || systemctl suspend'
}
</file context>
| on-timeout = bash -c 'cat /sys/class/power_supply/AC*/online | grep -q 1 || systemctl suspend' | |
| on-timeout = bash -c 'grep -Eq "^(1|2)$" /sys/class/power_supply/AC*/online || systemctl suspend' |
Summary
Test plan
🤖 Generated with Claude Code
Summary by cubic
Improve power management on matic: ignore lid close in
logindand only suspend after 30‑minute idle on battery; never auto‑suspend on AC. Also move matic NixOS docs from rootREADME.mdto@matic/readme.md.New Features
services.logind: setHandleLidSwitchandHandleLidSwitchExternalPowertoignoresohypridlecontrols suspend.hypridle: suspend after 30 minutes only whenAC*/onlineis 0; skip suspend on AC.Refactors
README.mdto@matic/readme.md.Written for commit 9e0a8f9. Summary will update on new commits.