Open
Conversation
added 8 commits
January 23, 2026 16:35
…caling - Calculate panel_width from chain_length in manager - Convert all hardcoded pixel values to percentages based on panel width (64px) and display height (32px) - Preserve original 32x64 display spacing while scaling properly on larger multi-panel displays - Update logo positioning, text gaps, chart dimensions, and spacing to use percentage-based calculations - Maintains consistent visual proportions across any display size
- Increase logo left margin from 3.125% to 5% of panel width - Move text column further right (38% with chart, 48% without chart) - Increase chart margin from 6.25% to 12% of panel width for better text-chart separation - Increase stock gap from 16.67% to 22% to prevent inter-stock overlap - Increase element gap from 12.5% to 15% for better element separation Resolves overlapping issues between logo, text, and chart elements.
- Calculate actual logo, text, and chart dimensions before positioning - Position elements dynamically based on their actual sizes - Use minimum spacing (8% of panel width) between elements - Ensure chart doesn't overlap with text by calculating text end position - Adjust chart width if needed to fit within image bounds This prevents overlaps by calculating expected sizes rather than using fixed percentages.
- Chart width increased from 40% to 50% of scrolling image width - Provides better visibility of price trends while maintaining proper spacing
- Chart width now based on 50% of panel_width instead of stock display width - Prevents charts from varying in size based on text length - All charts will now be the same width regardless of symbol/price text length - Only reduces chart width if it would exceed bounds, never increases it Fixes issue where shorter text (like VOO) resulted in much wider charts.
- Increase stock display width from 2.0 to 2.5 panel widths when chart enabled - Provides sufficient space for logo, text, spacing, and 50% panel_width chart - Update clamping logic to maintain at least 40% chart width (was 20%) - Add warning log if chart needs to be reduced (shouldn't happen with proper sizing) Fixes issue where charts were being clamped down to ~20% due to insufficient space.
- Add chart_size property to display section in config schema (0.2-0.8, default 0.5) - Chart size is now configurable as percentage of panel width - Update config_manager to read and validate chart_size - Update display_renderer to use chart_size from config instead of hardcoded 0.5 - Maintains backward compatibility with default 0.5 (50%) Users can now configure chart width via display.chart_size in config.
- Always use 2.5 panel widths for stock display width (not 1.5 when chart disabled) - Always use same text gap (6.25% of display height) regardless of chart toggle - Always position text in same location (after logo with minimum spacing) - Chart space is simply left empty when disabled, no blank chart area Layout now looks exactly the same whether chart is enabled or disabled, just without the chart when disabled.
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant Plugin as StockTickerPlugin
participant Config as StockConfigManager
participant Manager as DisplayManager
participant Renderer as StockDisplayRenderer
Plugin->>Config: Initialize and load config
activate Config
Config->>Config: Read display.chart_size or legacy chart_size\nClamp to [0.2,0.8]
Config-->>Plugin: Return chart_size
deactivate Config
Plugin->>Manager: Request hardware/display settings
activate Manager
Manager->>Manager: Read chain_length, clamp/validate
Manager-->>Plugin: Return panel_width
deactivate Manager
Plugin->>Renderer: Init renderer with panel_width and chart_size
activate Renderer
Renderer->>Renderer: Calculate layout proportions based on panel_width
Renderer->>Renderer: Compute chart_width from chart_size
Renderer->>Renderer: Position logo, text, and chart relative to panel_width
Renderer-->>Plugin: Renderer ready
deactivate Renderer
Plugin->>Renderer: Render frame
Renderer-->>Plugin: Frame rendered
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@manager.py`:
- Around line 41-56: The retrieved chain_length from
plugin_manager/display_manager may be non-int or huge; validate and normalize it
before computing panel_width: when reading chain_length from hardware_config in
the try block (where chain_length is set), attempt to coerce to int (handling
ValueError/TypeError), fall back to the default (2) on failure, then enforce a
minimum of 1 and cap it at self.display_width (or another sensible max) so
panel_width = self.display_width // chain_length never becomes zero or negative;
update the logic around hardware_config, chain_length, and panel_width to
perform this validation/clamping.
🧹 Nitpick comments (2)
config_manager.py (1)
90-109: Coercechart_sizeto float before clamping.
This avoidsTypeErrorduring min/max if config supplies a string and prevents falling back to full defaults.♻️ Proposed fix
# Clamp scroll_speed to valid range (0.5-5.0) per schema self.scroll_speed = max(0.5, min(5.0, self.scroll_speed)) # Clamp chart_size to valid range (0.2-0.8) per schema - self.chart_size = max(0.2, min(0.8, self.chart_size)) + try: + self.chart_size = float(self.chart_size) + except (TypeError, ValueError): + self.chart_size = 0.5 + self.chart_size = max(0.2, min(0.8, self.chart_size))display_renderer.py (1)
521-535: Trim unused locals/params in mini‑chart helpers.
Removes lint noise without behavior change.♻️ Proposed fix
chart_width = int(self.panel_width * self.chart_size) chart_height = int(self.display_height * 0.6667) chart_margin = int(self.panel_width * 0.12) chart_x = int(width - chart_width - chart_margin) - chart_y = int((height - chart_height) / 2) self._draw_mini_chart_at_position(draw, price_history, chart_x, width, height, color, chart_width) - def _draw_mini_chart_at_position(self, draw: ImageDraw.Draw, price_history: List[Dict], - chart_x: int, width: int, height: int, + def _draw_mini_chart_at_position(self, draw: ImageDraw.Draw, price_history: List[Dict], + chart_x: int, _width: int, height: int, color: Tuple[int, int, int], chart_width: int) -> None:
…lculation - Coerce chain_length to int, handling ValueError/TypeError with fallback to default - Enforce minimum of 1 (at least one panel) - Cap at min(display_width, 32) to ensure panel_width >= 1 and prevent absurdly small panels - Add final safety check to ensure panel_width is at least 1 - Add debug logging for panel width calculation - Add warning logs for invalid/clamped values Prevents division by zero, negative panel_width, and handles edge cases gracefully.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@manager.py`:
- Around line 68-79: Guard against zero/negative display_width by ensuring
max_chain_length is at least 1 before clamping chain_length: change the
computation around max_chain_length (which currently uses self.display_width and
32) to enforce max_chain_length = max(1, min(self.display_width, 32)) (or
equivalent) so chain_length never becomes 0, and keep the existing clamp that
logs via self.logger.warning and then compute panel_width = self.display_width
// chain_length after these safeguards.
- Around line 52-56: The conversion of chain_length_raw to int using
int(float(chain_length_raw)) can raise OverflowError for extremely large values
(e.g., "1e309"); update the exception handling in the chain_length coercion (the
try/except around chain_length = int(float(chain_length_raw))) to catch
OverflowError in addition to ValueError and TypeError, and on catching it use
the same fallback behavior (log via self.logger.warning and use the default
chain length).
Comment on lines
+52
to
+56
| # Coerce to int, handling ValueError/TypeError | ||
| try: | ||
| chain_length = int(float(chain_length_raw)) # Handle both int and float strings | ||
| except (ValueError, TypeError): | ||
| self.logger.warning("Invalid chain_length value '%s', using default 2", chain_length_raw) |
There was a problem hiding this comment.
Catch overflow when coercing chain_length.
int(float(chain_length_raw)) can raise OverflowError for huge values (e.g., "1e309"), which currently escapes the inner try and will crash init. Please include it in the handled exceptions.
🐛 Proposed fix
- except (ValueError, TypeError):
+ except (ValueError, TypeError, OverflowError):
self.logger.warning("Invalid chain_length value '%s', using default 2", chain_length_raw)
chain_length = 2📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Coerce to int, handling ValueError/TypeError | |
| try: | |
| chain_length = int(float(chain_length_raw)) # Handle both int and float strings | |
| except (ValueError, TypeError): | |
| self.logger.warning("Invalid chain_length value '%s', using default 2", chain_length_raw) | |
| # Coerce to int, handling ValueError/TypeError | |
| try: | |
| chain_length = int(float(chain_length_raw)) # Handle both int and float strings | |
| except (ValueError, TypeError, OverflowError): | |
| self.logger.warning("Invalid chain_length value '%s', using default 2", chain_length_raw) | |
| chain_length = 2 |
🤖 Prompt for AI Agents
In `@manager.py` around lines 52 - 56, The conversion of chain_length_raw to int
using int(float(chain_length_raw)) can raise OverflowError for extremely large
values (e.g., "1e309"); update the exception handling in the chain_length
coercion (the try/except around chain_length = int(float(chain_length_raw))) to
catch OverflowError in addition to ValueError and TypeError, and on catching it
use the same fallback behavior (log via self.logger.warning and use the default
chain length).
Comment on lines
+68
to
+79
| # Cap at display_width to ensure panel_width is at least 1 pixel | ||
| # Also cap at reasonable maximum (e.g., 32) to prevent absurdly small panels | ||
| max_chain_length = min(self.display_width, 32) | ||
| if chain_length > max_chain_length: | ||
| self.logger.warning("chain_length %d exceeds maximum %d, clamping to %d", | ||
| chain_length, max_chain_length, max_chain_length) | ||
| chain_length = max_chain_length | ||
|
|
||
| # Calculate panel width (width of a single panel) | ||
| # This is now safe: chain_length is guaranteed to be 1 <= chain_length <= max_chain_length | ||
| panel_width = self.display_width // chain_length | ||
|
|
There was a problem hiding this comment.
Guard against display_width <= 0 to avoid division by zero.
When self.display_width is 0 (or negative), max_chain_length becomes 0, which can clamp chain_length to 0 and cause a divide-by-zero at Line 78. Add a minimum clamp to keep max_chain_length >= 1.
🐛 Proposed fix
- max_chain_length = min(self.display_width, 32)
+ max_chain_length = max(1, min(self.display_width, 32))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Cap at display_width to ensure panel_width is at least 1 pixel | |
| # Also cap at reasonable maximum (e.g., 32) to prevent absurdly small panels | |
| max_chain_length = min(self.display_width, 32) | |
| if chain_length > max_chain_length: | |
| self.logger.warning("chain_length %d exceeds maximum %d, clamping to %d", | |
| chain_length, max_chain_length, max_chain_length) | |
| chain_length = max_chain_length | |
| # Calculate panel width (width of a single panel) | |
| # This is now safe: chain_length is guaranteed to be 1 <= chain_length <= max_chain_length | |
| panel_width = self.display_width // chain_length | |
| # Cap at display_width to ensure panel_width is at least 1 pixel | |
| # Also cap at reasonable maximum (e.g., 32) to prevent absurdly small panels | |
| max_chain_length = max(1, min(self.display_width, 32)) | |
| if chain_length > max_chain_length: | |
| self.logger.warning("chain_length %d exceeds maximum %d, clamping to %d", | |
| chain_length, max_chain_length, max_chain_length) | |
| chain_length = max_chain_length | |
| # Calculate panel width (width of a single panel) | |
| # This is now safe: chain_length is guaranteed to be 1 <= chain_length <= max_chain_length | |
| panel_width = self.display_width // chain_length |
🤖 Prompt for AI Agents
In `@manager.py` around lines 68 - 79, Guard against zero/negative display_width
by ensuring max_chain_length is at least 1 before clamping chain_length: change
the computation around max_chain_length (which currently uses self.display_width
and 32) to enforce max_chain_length = max(1, min(self.display_width, 32)) (or
equivalent) so chain_length never becomes 0, and keep the existing clamp that
logs via self.logger.warning and then compute panel_width = self.display_width
// chain_length after these safeguards.
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
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.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.