feat: keep the computer awake while knitting #698 #699
Conversation
WalkthroughThe recent updates enhance user experience by implementing a feature that keeps the screen awake during knitting operations in the GUI, utilizing the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GUI
participant Engine
participant Device
User->>GUI: Initiate knitting operation
GUI->>Engine: Start operation
Engine->>wakepy: Keep screen awake
Engine->>Device: Communicate with device
Device-->>Engine: Return status
Engine->>GUI: Update status
GUI-->>User: Display feedback
Engine->>wakepy: Release screen control on completion
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- CHANGELOG.md (1 hunks)
- requirements.build.txt (1 hunks)
- src/main/python/main/ayab/engine/engine.py (2 hunks)
Additional comments not posted (5)
requirements.build.txt (1)
12-12: Dependency Added: Verify CompatibilityThe addition of
wakepy==0.9.1seems appropriate for managing wake timers. Ensure compatibility with existing dependencies and the project's environment.src/main/python/main/ayab/engine/engine.py (3)
170-183: Integration ofwakepy: Proper Use of Context ManagerThe use of
with keep.presenting(on_fail="pass")ensures that the screen stays awake during the operation. This is a good practice for long-running tasks. Verify that theon_failparameter is set appropriately for your use case.
28-28: Import Added: Ensure Proper UsageThe
wakepylibrary is imported. Ensure that it is used correctly and that any potential exceptions it might raise are handled properly.
185-201: Ensure Proper Logging and NotificationsThe logging and notification logic is well-structured. Ensure that all possible states and exceptions are covered, especially when handling cancellations and completions.
CHANGELOG.md (1)
43-43: Changelog Updated: Ensure ConsistencyThe changelog entry for keeping the screen awake during knitting is clear and informative. Ensure that it is consistent with the actual implementation and follows the project's formatting conventions.
Ok, I´ll try to add it. In the preference menu ? |
|
|
I added the option in the preferences. It tested manually, it is working. I think I´ll need to add a translation for the message ? |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/main/python/main/ayab/engine/engine.py (3 hunks)
- src/main/python/main/ayab/preferences.py (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/main/python/main/ayab/engine/engine.py
Additional comments not posted (3)
src/main/python/main/ayab/preferences.py (3)
70-70: LGTM!The addition of
"keep_awake"toPreferencesDictBoolKeysis consistent with the existing pattern.
94-94: LGTM!The addition of
"keep_awake": type[bool]toPreferencesDictis correctly implemented.
125-125: LGTM!The addition of
"keep_awake": boolto thevariablesdictionary in thePreferencesclass is appropriate and ensures proper management of the new preference.
Are we sure about this needing to be a setting? Who actually wants their computer to go to sleep while knitting? Personally I'd rather not have the UI cluttered with a toggle no-one is reasonably going to use. Also, it makes the code uglier because the context manager can't be used anymore. On another note, we just froze features for |
| keep_mode = None | ||
| if self.preferences.value("keep_awake"): | ||
| self.__logger.debug("Keeping screen awake activated") | ||
| # Activate the keep mode only if the preference is activated | ||
| keep_mode = keep.presenting(on_fail="pass") | ||
| keep_mode.__enter__() | ||
|
|
||
| try: |
There was a problem hiding this comment.
I still think it shouldn't be a preference, but if it has to be one, at least it can be made a bit more palatable with nullcontext that is designed for such cases:
| keep_mode = None | |
| if self.preferences.value("keep_awake"): | |
| self.__logger.debug("Keeping screen awake activated") | |
| # Activate the keep mode only if the preference is activated | |
| keep_mode = keep.presenting(on_fail="pass") | |
| keep_mode.__enter__() | |
| try: | |
| from contextlib import nullcontext # at the top | |
| … | |
| with keep.presenting(on_fail="pass") if self.preferences.value("keep_awake") else nullcontext(): |
Then you can get rid of the try…finally block.
There was a problem hiding this comment.
If it is okay for others, I'd prefer removing the settings ...
But if not yes that's a nicer solution.
Summary by CodeRabbit
New Features
Dependencies
wakepy==0.9.1, to improve functionality related to managing the application's operational state.Improvements
Preferences