Skip to content

logind: Mark LidClosed property as "emits change"#30706

Merged
bluca merged 1 commit intosystemd:mainfrom
garnacho:logind-power-emit
Jan 10, 2024
Merged

logind: Mark LidClosed property as "emits change"#30706
bluca merged 1 commit intosystemd:mainfrom
garnacho:logind-power-emit

Conversation

@garnacho
Copy link
Copy Markdown
Contributor

@garnacho garnacho commented Jan 3, 2024

It may be useful for DEs to follow changes on these properties, esp. now that recent UPower has removed its own lid handling code.

Related: https://gitlab.freedesktop.org/upower/upower/-/commit/07565ef6a1aa4a115f8ce51e259e408edbaed4cc

@github-actions github-actions bot added documentation please-review PR is ready for (re-)review by a maintainer labels Jan 3, 2024
@poettering poettering added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed please-review PR is ready for (re-)review by a maintainer labels Jan 3, 2024
@poettering
Copy link
Copy Markdown
Member

lgtm

@YHNdnzj
Copy link
Copy Markdown
Member

YHNdnzj commented Jan 3, 2024

Hmm, is this sufficient? Nothing is actually emitting those signals currently IIUC.

@poettering
Copy link
Copy Markdown
Member

uh true. we do not generate those signals indeed.

@poettering poettering added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed labels Jan 3, 2024
@YHNdnzj YHNdnzj added the logind label Jan 3, 2024
@garnacho garnacho changed the title logind: Mark Docked/LidClosed/OnExternalPower properties as "emits change" logind: Mark Docked/LidClosed properties as "emits change" Jan 3, 2024
@garnacho
Copy link
Copy Markdown
Contributor Author

garnacho commented Jan 3, 2024

Eek, good catch, and silly me. I updated the patch adding notifications, and giving up OnExternalPower ATM. I notice that this does not handle yet changes in the number of outputs for Docked, I'll look into that.

@github-actions github-actions bot added please-review PR is ready for (re-)review by a maintainer and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Jan 3, 2024
@bluca bluca added ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR and removed please-review PR is ready for (re-)review by a maintainer labels Jan 3, 2024
It may be useful for DEs to follow changes on this property, esp. now that
recent UPower has removed its own lid handling code.

Related: https://gitlab.freedesktop.org/upower/upower/-/commit/07565ef6a1aa4a115f8ce51e259e408edbaed4cc
@garnacho
Copy link
Copy Markdown
Contributor Author

garnacho commented Jan 3, 2024

Toned it down further to handling only LidClosed ATM. I'm having a hard time testing this patch over Silverblue, and at least on the GNOME side there's no immediate use for notifications on the Docked D-Bus property, I will save the hairier udev state tracking for future PRs.

@garnacho garnacho changed the title logind: Mark Docked/LidClosed properties as "emits change" logind: Mark LidClosed property as "emits change" Jan 3, 2024
@github-actions github-actions bot added please-review PR is ready for (re-)review by a maintainer and removed ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR labels Jan 3, 2024
Copy link
Copy Markdown
Contributor

@whot whot left a comment

Choose a reason for hiding this comment

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

This LGTM, thanks

@bluca bluca merged commit 501d8b8 into systemd:main Jan 10, 2024
@garnacho garnacho deleted the logind-power-emit branch January 13, 2024 10:49
erikinkinen pushed a commit to droidian/upower that referenced this pull request Feb 15, 2024
This reverts commit 07565ef.

In the current systemd stable release 255 org.freedesktop.login1 does
not emit a LidisClosed event, this has added in systemd `main` and will
be availble in the next release.

As GNOME control panel still uses UPower's `LidIsclosed` property and
many other DE's such as Xfce/LXQt/Deepin as well revert this until the
systemd changes are available in all Distributions.

systemd/systemd#30706

Resolve: https://gitlab.freedesktop.org/upower/upower/-/issues/260
@bluca bluca removed the please-review PR is ready for (re-)review by a maintainer label Mar 18, 2024
@mrc0mmand mrc0mmand added login and removed logind labels Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

7 participants