Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt to minimize steam deck display manager restart risk #237

Merged

Conversation

Hoverbear
Copy link
Contributor

Description

This attempts to address #235 with the information at hand. My best suspicion is that this is the display manager restarting.

Checklist
  • Formatted with cargo fmt
  • Built with nix build
  • Ran flake checks with nix flake check
  • Added or updated relevant tests (leave unchecked if not applicable)
  • Added or updated relevant documentation (leave unchecked if not applicable)
  • Linked to related issues (leave unchecked if not applicable)
Validating with install.determinate.systems

If a maintainer has added the upload to s3 label to this PR, it will become available for installation via install.determinate.systems:

curl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix/pr/$PR_NUMBER | sh -s -- install

@Hoverbear Hoverbear added bug Something isn't working upload to s3 The labeled PR is allowed to upload its artifacts to S3 for easy testing Steam Deck labels Feb 6, 2023
@Hoverbear Hoverbear self-assigned this Feb 6, 2023
@Hoverbear
Copy link
Contributor Author

Interesting, I experienced the bug this time when I was testing my install on the deck.

@Hoverbear
Copy link
Contributor Author

Hoverbear commented Feb 6, 2023

I seem to be no longer able to reproduce it.

@@ -188,7 +189,7 @@ impl Planner for SteamDeck {
Type=oneshot\n\
RemainAfterExit=yes\n\
ExecStart=/usr/bin/systemctl daemon-reload\n\
ExecStart=/usr/bin/systemctl restart --no-block sockets.target timers.target multi-user.target\n\
ExecStart=/usr/bin/systemctl start --no-block sockets.target timers.target multi-user.target\n\
Copy link
Contributor Author

@Hoverbear Hoverbear Feb 6, 2023

Choose a reason for hiding this comment

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

This is the load bearing line of the fix, the above line is is more assurance than a fix.

@Hoverbear
Copy link
Contributor Author

So, changing from restart to start allows us to better avoid having other units (like the display manager) restart unexpectedly, creating this "Restart like experience" for users.

@Hoverbear Hoverbear added this to the v0.2.1 milestone Feb 6, 2023
@Hoverbear Hoverbear requested a review from cole-h February 8, 2023 18:00
Copy link
Member

@cole-h cole-h left a comment

Choose a reason for hiding this comment

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

I am curious why we don't directly start the services we've placed, but this is certainly an improvement.

src/planner/linux/steam_deck.rs Outdated Show resolved Hide resolved
@Hoverbear Hoverbear requested a review from cole-h February 8, 2023 19:09
Copy link
Member

@cole-h cole-h left a comment

Choose a reason for hiding this comment

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

Nice, looks much better!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Steam Deck upload to s3 The labeled PR is allowed to upload its artifacts to S3 for easy testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants