-
-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add fullscreen troubleshooting directions #3
base: dev
Are you sure you want to change the base?
Conversation
WalkthroughThe changes in this pull request primarily involve updates to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
README.md (2)
142-144
: Excellent addition addressing the fullscreen issue.This new section directly addresses the PR objectives by providing a solution for the fullscreen mode issue on Linux systems using Wayland. The information is relevant and actionable.
To make this even more helpful, consider expanding the instructions with the following details:
- Exact file name of the systemd unit file (e.g.,
ovos-gui-app.service
).- Step-by-step instructions on how to edit the file and restart the service.
- A note on potential side effects of changing this variable.
For example:
### Fullscreen Mode for ovos-gui-app On some Linux OSs using Wayland as the Window Manager, the `ovos-gui-app` doesn't natively run in fullscreen mode. Here's a solution: 1. Locate the systemd unit file: `/home/<user>/.config/systemd/user/ovos-gui-app.service` 2. Edit the file (use sudo if necessary) and find the `Environment` line. 3. Change the `QT_QPA_PLATFORM` variable from `wayland;xcb` to `eglfs`:Environment=QT_QPA_PLATFORM=eglfs
4. Save the file and restart the service:
systemctl --user daemon-reload
systemctl --user restart ovos-gui-app.serviceNote: Changing to `eglfs` might affect other Qt applications. If you experience issues, revert the change.
🧰 Tools
🪛 Markdownlint
142-142: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
142-142
: Consider project's markdown style preferences for headings.The static analysis tool suggests using setext-style headings (underlined with
===
or---
) instead of the current atx-style (#
) for consistency. However, both styles are widely supported and the current style doesn't impact readability.Unless there's a specific project style guide that mandates setext-style headings, it's reasonable to ignore this suggestion. If you decide to change it, ensure consistency throughout the document.
🧰 Tools
🪛 Markdownlint
142-142: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- README.md (1 hunks)
🧰 Additional context used
🪛 Markdownlint
README.md
142-142: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
🔇 Additional comments (1)
README.md (1)
Line range hint
1-144
: Overall, great improvements to the README!The changes effectively address the PR objectives by adding clear instructions for troubleshooting fullscreen mode issues. The document maintains good structure and readability. With the suggested minor enhancements, this update will significantly improve the user experience for those encountering similar issues.
🧰 Tools
🪛 Markdownlint
142-142: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
145-145: Expected: setext; Actual: atx_closed
Heading style(MD003, heading-style)
145-145: null
Multiple headings with the same content(MD024, no-duplicate-heading)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- README.md (1 hunks)
🧰 Additional context used
🪛 Markdownlint
README.md
142-142: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
🔇 Additional comments (2)
README.md (2)
142-142
: Maintain consistent heading styleThe static analysis tool suggests changing the heading style, but I recommend keeping it as is. The current atx-style heading (using
#
) is consistent with the rest of the document. Changing only this heading would introduce inconsistency.🧰 Tools
🪛 Markdownlint
142-142: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
Line range hint
1-144
: Overall assessment: Valuable addition to documentationThe changes in this PR successfully address the stated objective of providing troubleshooting directions for fullscreen display issues. The new section is well-integrated into the existing document structure and provides helpful information for users experiencing the described problem.
The modifications maintain the overall integrity and formatting of the README, which is commendable. With the suggested enhancements to the new section, this PR will significantly improve the user experience for those encountering fullscreen mode issues.
🧰 Tools
🪛 Markdownlint
142-142: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
145-145: Expected: setext; Actual: atx_closed
Heading style(MD003, heading-style)
145-145: null
Multiple headings with the same content(MD024, no-duplicate-heading)
### Fullscreen Mode for ovos-gui-app | ||
On some Linux OSs using Wayland as the Window Manager, the `ovos-gui-app` doesn't natively run in fullscreen mode. One possible solution is to change the `QT_QPA_PLATFORM` variable from something like `wayland;xcb` to `eglfs` in the systemd unit file located in `/home/<user>/.config/systemd/user`. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance the fullscreen troubleshooting instructions
The new section addressing fullscreen mode issues is a valuable addition that directly addresses the PR objective. To make it even more helpful, consider the following improvements:
- Provide step-by-step instructions for modifying the systemd unit file.
- Include a command to locate the correct systemd unit file.
- Add a note about potential risks of changing system configurations.
- Suggest reverting the changes if they don't solve the issue.
Here's a suggested expansion of the section:
### Fullscreen Mode for ovos-gui-app
On some Linux OSs using Wayland as the Window Manager, the `ovos-gui-app` doesn't natively run in fullscreen mode. Here's a potential workaround:
1. Locate the systemd unit file:
systemctl --user cat ovos-gui-app.service
If this command doesn't work, the file might be in `/home/<user>/.config/systemd/user/ovos-gui-app.service`.
2. Edit the file using your preferred text editor (e.g., nano):
nano /home//.config/systemd/user/ovos-gui-app.service
3. Find the `Environment` line and change the `QT_QPA_PLATFORM` variable from `wayland;xcb` to `eglfs`:
Environment=QT_QPA_PLATFORM=eglfs
4. Save the file and exit the editor.
5. Reload the systemd daemon:
systemctl --user daemon-reload
6. Restart the ovos-gui-app service:
systemctl --user restart ovos-gui-app.service
Note: Modifying system configurations can have unintended consequences. If this change doesn't resolve the issue or causes other problems, revert the changes and seek further assistance.
This expanded section provides more detailed guidance while maintaining the original information.
🧰 Tools
🪛 Markdownlint
142-142: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
I struggled with this on my Pi4 running Raspbian after installing with the ovos-installer in using the virtualenv option. The ovos-gui-app displayed as a "window in window" with 2 sets of window controls.
Summary by CodeRabbit
New Features
Documentation