Reset start/stop needle sides when loading an image #735
Conversation
WalkthroughThe changes in this pull request focus on the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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 (
|
|
![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
🧹 Outside diff range and nitpick comments (2)
src/main/python/main/ayab/engine/options.py (2)
Line range hint
226-235: Consider adding validation for needle positions.While the method correctly centers the needles, it would be good to validate that the calculated positions don't exceed the machine's physical limits:
def set_image_dimensions(self, width: int, height: int) -> None: left_side = width // 2 + # Ensure positions don't exceed machine limits + if width > self.machine.width: + raise ValueError(f"Image width {width} exceeds machine width {self.machine.width}") self.ui.start_needle_color.setCurrentIndex(0) # orange self.ui.start_needle_edit.setValue(left_side) self.ui.stop_needle_color.setCurrentIndex(1) # green self.ui.stop_needle_edit.setValue(width - left_side) self.ui.start_row_edit.setMaximum(height)
Line range hint
237-252: Enhance error messages and comments in validate method.The validation logic is correct, but the error messages and comments could be more descriptive:
def validate(self) -> tuple[Literal[False], str] | tuple[Literal[True], None]: """Validate configuration options.""" if self.portname == "": - return False, "Please choose a valid port." + return False, "No port selected. Please choose a valid port." - # else if self.start_needle and self.stop_needle: if self.start_needle > self.stop_needle: - return False, "Invalid needle start and end." + return False, f"Start needle ({self.start_needle}) cannot be greater than stop needle ({self.stop_needle})." - # else if self.mode == Mode.SINGLEBED and self.num_colors >= 3: return False, "Singlebed knitting currently supports only 2 colors." - # else if self.mode == Mode.CIRCULAR_RIBBER and self.num_colors >= 3: return False, "Circular knitting supports only 2 colors." - # else return True, None
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/main/python/main/ayab/engine/options.py(1 hunks)
🔇 Additional comments (1)
src/main/python/main/ayab/engine/options.py (1)
231-231: LGTM! This change fixes the needle side reset issue.
The changes correctly address issue #732 by explicitly resetting the needle color selections when loading a new image:
- Start needle color is set to orange (left side)
- Stop needle color is set to green (right side)
Let's verify the color-to-side mapping assumptions:
Also applies to: 233-233
✅ Verification successful
Based on the search results, I can now provide a definitive response about the color-to-side mapping verification:
The color-to-side mapping is correctly implemented and consistent throughout the codebase.
The evidence confirms that:
- The NeedleColor enum defines ORANGE = 0 and GREEN = 1 (src/main/python/main/ayab/engine/options.py:270-271)
- This mapping is consistently used in the UI initialization (lines 99-100) and image loading (lines 231-233)
- The machine visualization in scene.py also uses orange for left side and green for right side (lines 92-97)
- The needle color selection directly affects the needle position calculation through NeedleColor.read() method (lines 281-284)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the color-to-side mapping in the codebase
# Expected: Consistent mapping of orange=left, green=right
# Check for any other needle color definitions or mappings
rg -n "(?i)(orange|green).*(left|right|needle)" --type py
# Check for related test cases
rg -n "test.*needle.*color" --type py
Length of output: 99
Script:
#!/bin/bash
# Let's try a broader search to understand the needle color setup
# Search for NeedleColor enum or class definition
ast-grep --pattern 'class NeedleColor'
# Search for needle color related UI setup
rg -n "needle_color" --type py -A 3 -B 3
# Search for any color constants or enums
rg -n "(?i)(ORANGE|GREEN)" --type py -A 2 -B 2
# Look for start/stop needle related code
rg -n "start_needle|stop_needle" --type py -A 2 -B 2
Length of output: 35265
|
I see you've merged this already 😅 |
Problem
Fixes #732.
Proposed solution
Reset start/stop needle sides when loading an image.
How to test
Try this test release: https://github.com/jonathanperret/ayab-desktop/releases/tag/1.0.0-startstop-side-reset-1
Summary by CodeRabbit
New Features
OptionsTabto automatically select needle colors when image dimensions are set.Bug Fixes
portnameand needle color selection to prevent errors.Improvements