Implement synchronization for rainbow effects #64
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added synchronization features for rainbow effects in title bar and border.
Reviewer's GuideThis PR introduces a centralized synchronization mechanism for rainbow effects by spawning a single, invisible master thread that cycles a global color and exposes it via synchronized_rainbow, while modifying title_bar and border classes to run as slaves fetching and applying that color in their own timer threads. Sequence diagram for starting synchronized rainbow effect on title barsequenceDiagram
actor User
participant "rainbow_title_bar"
participant "synchronized_rainbow"
participant "title_bar_color"
User->>"rainbow_title_bar": start_sync(window, interval, color_stops)
"rainbow_title_bar"->>"synchronized_rainbow": start(interval, color_stops)
"rainbow_title_bar"->>"synchronized_rainbow": get_current_color()
"rainbow_title_bar"->>"title_bar_color": set(hwnd, rgb)
Note over "rainbow_title_bar": Repeats periodically in timer thread
Sequence diagram for stopping synchronized rainbow effect on bordersequenceDiagram
actor User
participant "rainbow_border"
participant "border_color"
participant "synchronized_rainbow"
User->>"rainbow_border": stop_sync(window)
"rainbow_border"->>"border_color": reset(window)
"rainbow_border"->>"synchronized_rainbow": stop_sync_if_last()
"synchronized_rainbow"->>"synchronized_rainbow": stop() (if last window)
Class diagram for synchronized rainbow effect synchronizationclassDiagram
class synchronized_rainbow {
+_color_changer_task(interval, color_stops)
+get_current_color()
+start(interval, color_stops)
+stop()
}
class rainbow_title_bar {
+current_color
+_sync_timer_threads
+start(window, interval, color_stops)
+get_current_color()
+stop(window)
+start_sync(window, interval, color_stops)
+stop_sync(window)
}
class rainbow_border {
+current_color
+_sync_timer_threads
+start(window, interval, color_stops)
+get_current_color()
+stop(window)
+start_sync(window, interval, color_stops)
+stop_sync(window)
}
synchronized_rainbow <.. rainbow_title_bar : "sync color fetch"
synchronized_rainbow <.. rainbow_border : "sync color fetch"
rainbow_title_bar o-- _sync_timer_threads
rainbow_border o-- _sync_timer_threads
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `hPyT/hPyT.py:165` </location>
<code_context>
+ if color is None:
+ return (0, 0, 0)
+
+ # Convert integer color (BGR format) back to RGB tuple
+ b = (color >> 16) & 0xFF
+ g = (color >> 8) & 0xFF
</code_context>
<issue_to_address>
**issue (bug_risk):** Color conversion appears to use BGR order, but the packing uses RGB.
The unpacking in get_current_color expects BGR, but packing uses RGB. Please ensure both use the same channel order to avoid incorrect color values.
</issue_to_address>
### Comment 2
<location> `hPyT/hPyT.py:195` </location>
<code_context>
+ if sync_is_running:
+ sync_is_running = False
+ # Wait for the thread to finish cleanly
+ if threading.current_thread() is not sync_color_thread:
+ sync_color_thread.join(timeout=0.1)
+ sync_color_thread = None
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Joining the sync_color_thread with a timeout may not guarantee clean thread termination.
Consider increasing the timeout or implementing a loop to wait for thread termination, and ensure the thread responds promptly when sync_is_running is set to False.
Suggested implementation:
```python
# Wait for the thread to finish cleanly
if threading.current_thread() is not sync_color_thread:
max_wait = 2.0 # seconds
waited = 0.0
interval = 0.1
while sync_color_thread.is_alive() and waited < max_wait:
sync_color_thread.join(timeout=interval)
waited += interval
sync_color_thread = None
```
- Ensure that the thread running in `sync_color_thread` checks the `sync_is_running` flag frequently and exits promptly when it is set to `False`. If this is not already implemented in the thread's target function, you should add such a check in its loop.
</issue_to_address>
### Comment 3
<location> `hPyT/hPyT.py:972` </location>
<code_context>
+ It uses the synchronized_rainbow master thread for color data.
+ """
+ hwnd: int = module_find(window)
+ if hwnd in cls._sync_timer_threads:
+ raise RuntimeError("Title bar is already running in synchronization mode.")
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** No explicit cleanup for threads in _sync_timer_threads after stop_sync.
Ensure that threads are properly joined or signaled to terminate when removed from _sync_timer_threads to prevent orphaned threads.
Suggested implementation:
```python
# Dictionary to store stop events for each thread
_sync_timer_stop_events = {}
@classmethod
def start_sync(cls, window: Any, interval: int = 5, color_stops: int = 5) -> None:
"""
Starts the Title Bar in synchronization mode (as a slave).
It uses the synchronized_rainbow master thread for color data.
"""
hwnd: int = module_find(window)
```
```python
WINDOWS_VERSION = float(platform.version().split(".")[0])
def _sync_timer_thread_func(hwnd, stop_event, interval, color_stops):
while not stop_event.is_set():
# Thread logic here, e.g. update color from master
# ...
time.sleep(interval)
# Cleanup if needed
```
```python
def stop_sync_if_last():
"""Checks if any element is still running in sync mode and stops the master if none are."""
def stop_sync(cls, window: Any) -> None:
"""
Stops the Title Bar synchronization mode for the given window.
Ensures thread is signaled to terminate and joined.
"""
hwnd: int = module_find(window)
thread = cls._sync_timer_threads.pop(hwnd, None)
stop_event = cls._sync_timer_stop_events.pop(hwnd, None)
if stop_event is not None:
stop_event.set()
if thread is not None:
thread.join()
```
- You will need to update the code where threads are created for sync mode to use `_sync_timer_thread_func` and pass a `threading.Event` as the stop signal.
- Ensure that `_sync_timer_threads` and `_sync_timer_stop_events` are properly managed (added/removed) in both `start_sync` and `stop_sync`.
- If your thread logic is elsewhere, adapt `_sync_timer_thread_func` accordingly.
- You may need to add the `stop_sync` method to your class if it does not already exist, and ensure it is called when synchronization is stopped.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if color is None: | ||
| return (0, 0, 0) | ||
|
|
||
| # Convert integer color (BGR format) back to RGB tuple |
There was a problem hiding this comment.
issue (bug_risk): Color conversion appears to use BGR order, but the packing uses RGB.
The unpacking in get_current_color expects BGR, but packing uses RGB. Please ensure both use the same channel order to avoid incorrect color values.
| if sync_is_running: | ||
| sync_is_running = False | ||
| # Wait for the thread to finish cleanly | ||
| if threading.current_thread() is not sync_color_thread: |
There was a problem hiding this comment.
suggestion (bug_risk): Joining the sync_color_thread with a timeout may not guarantee clean thread termination.
Consider increasing the timeout or implementing a loop to wait for thread termination, and ensure the thread responds promptly when sync_is_running is set to False.
Suggested implementation:
# Wait for the thread to finish cleanly
if threading.current_thread() is not sync_color_thread:
max_wait = 2.0 # seconds
waited = 0.0
interval = 0.1
while sync_color_thread.is_alive() and waited < max_wait:
sync_color_thread.join(timeout=interval)
waited += interval
sync_color_thread = None- Ensure that the thread running in
sync_color_threadchecks thesync_is_runningflag frequently and exits promptly when it is set toFalse. If this is not already implemented in the thread's target function, you should add such a check in its loop.
| It uses the synchronized_rainbow master thread for color data. | ||
| """ | ||
| hwnd: int = module_find(window) | ||
| if hwnd in cls._sync_timer_threads: |
There was a problem hiding this comment.
suggestion (bug_risk): No explicit cleanup for threads in _sync_timer_threads after stop_sync.
Ensure that threads are properly joined or signaled to terminate when removed from _sync_timer_threads to prevent orphaned threads.
Suggested implementation:
# Dictionary to store stop events for each thread
_sync_timer_stop_events = {}
@classmethod
def start_sync(cls, window: Any, interval: int = 5, color_stops: int = 5) -> None:
"""
Starts the Title Bar in synchronization mode (as a slave).
It uses the synchronized_rainbow master thread for color data.
"""
hwnd: int = module_find(window)WINDOWS_VERSION = float(platform.version().split(".")[0])
def _sync_timer_thread_func(hwnd, stop_event, interval, color_stops):
while not stop_event.is_set():
# Thread logic here, e.g. update color from master
# ...
time.sleep(interval)
# Cleanup if needed
def stop_sync_if_last():
"""Checks if any element is still running in sync mode and stops the master if none are."""
def stop_sync(cls, window: Any) -> None:
"""
Stops the Title Bar synchronization mode for the given window.
Ensures thread is signaled to terminate and joined.
"""
hwnd: int = module_find(window)
thread = cls._sync_timer_threads.pop(hwnd, None)
stop_event = cls._sync_timer_stop_events.pop(hwnd, None)
if stop_event is not None:
stop_event.set()
if thread is not None:
thread.join()- You will need to update the code where threads are created for sync mode to use
_sync_timer_thread_funcand pass athreading.Eventas the stop signal. - Ensure that
_sync_timer_threadsand_sync_timer_stop_eventsare properly managed (added/removed) in bothstart_syncandstop_sync. - If your thread logic is elsewhere, adapt
_sync_timer_thread_funcaccordingly. - You may need to add the
stop_syncmethod to your class if it does not already exist, and ensure it is called when synchronization is stopped.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added synchronization feature for rainbow effects of title bar and border.
2025-10-18.01-02-09.mp4
Summary by Sourcery
Implement a synchronization feature for rainbow title bar and border effects using a shared master thread and individual slave threads for each window.
New Features:
Enhancements: