Conversation
Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@79f3d049-9006-400d-9954-2e5dcd250fa9.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
Signed-off-by: SCA075 <82227818+sca075@users.noreply.github.com>
…it bump version in pyproject.toml added to __init__.py Trims and Floor Data Signed-off-by: SCA075 <82227818+sca075@users.noreply.github.com>
Signed-off-by: SCA075 <82227818+sca075@users.noreply.github.com>
Signed-off-by: SCA075 <82227818+sca075@users.noreply.github.com>
Signed-off-by: SCA075 <82227818+sca075@users.noreply.github.com>
…ses old key trims_data Signed-off-by: SCA075 <82227818+sca075@users.noreply.github.com>
Signed-off-by: SCA075 <82227818+sca075@users.noreply.github.com>
- Add CARPET drawable element with configurable color and alpha - Implement carpet zone parsing from Valetudo JSON (PolygonMapEntity) - Add carpet rendering with customizable color (default: 50% of room_0) - Support disable_carpets flag to toggle carpet visibility - Fix COLORS list order to match base_color_keys mapping - Fix floor rendering to use correct MAP_BACKGROUND color - Add carpet color configuration to device_info Fixes color index mismatch that affected carpet and floor color updates Signed-off-by: SCA075 <82227818+sca075@users.noreply.github.com>
📝 WalkthroughWalkthroughAdds carpet as a drawable map element (constants, enum, defaults, drawing), threads floor info into trim data, adds polygon entity support, derives compressedPixels when missing, and bumps package/version and dependency. Changes
Sequence DiagramsequenceDiagram
participant Handler as HypferHandler
participant Draw as ImageDraw
participant Rooms as RoomStore
participant Canvas as NumpyArray
Handler->>Draw: async_draw_zones(m_json, np_array, color_zone_clean, color_no_go, color_carpet)
Draw->>Draw: identify carpet_zones from m_json
alt carpet_zones present and color_carpet provided and carpets enabled
loop for each carpet polygon/segment
Draw->>Rooms: lookup room for polygon segment
Rooms-->>Draw: room id / room color
Draw->>Draw: compute carpet color (50% room brightness, alpha 255)
Draw->>Canvas: rasterize carpet polygon onto array
end
end
Draw-->>Handler: np_array with carpets rendered
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
SCR/valetudo_map_parser/config/types.py (1)
84-92:from_dictwill raiseKeyErrorfor existing data withoutfloorkey.Using
data["floor"]directly throws aKeyErrorif the key is missing from legacy serialized data. Consider usingdata.get("floor", "floor_0")for backward compatibility with existing configurations.🐛 Proposed fix for backward compatibility
@staticmethod def from_dict(data: dict): """Create dataclass from dictionary.""" return TrimCropData( - floor=data["floor"], + floor=data.get("floor", "floor_0"), trim_left=data["trim_left"], trim_up=data["trim_up"], trim_right=data["trim_right"], trim_down=data["trim_down"], )SCR/valetudo_map_parser/config/drawable_elements.py (1)
207-219: Critical: Missing CARPET in element_color_mapping breaks device_info color configuration.The
element_color_mappinginupdate_from_device_infois missing theDrawableElement.CARPETentry, while the identical mapping in_set_default_properties(lines 86-99) includes it. This means user-providedcolor_carpetandalpha_carpetfromdevice_infowill be ignored, breaking the feature described in the PR objectives.Additionally, this mapping is duplicated across two methods, violating DRY principles and creating maintenance risks (as demonstrated by this bug).
🐛 Proposed fix: Add CARPET to mapping and eliminate duplication
Fix 1: Add the missing CARPET entry
element_color_mapping = { DrawableElement.FLOOR: SupportedColor.MAP_BACKGROUND, DrawableElement.WALL: SupportedColor.WALLS, DrawableElement.ROBOT: SupportedColor.ROBOT, DrawableElement.CHARGER: SupportedColor.CHARGER, DrawableElement.VIRTUAL_WALL: SupportedColor.NO_GO, DrawableElement.RESTRICTED_AREA: SupportedColor.NO_GO, DrawableElement.PATH: SupportedColor.PATH, DrawableElement.PREDICTED_PATH: SupportedColor.PREDICTED_PATH, DrawableElement.GO_TO_TARGET: SupportedColor.GO_TO, DrawableElement.NO_MOP_AREA: SupportedColor.NO_GO, DrawableElement.OBSTACLE: SupportedColor.NO_GO, + DrawableElement.CARPET: SupportedColor.CARPET, }Fix 2 (Recommended): Extract to class constant to eliminate duplication
Add a class-level constant after line 18:
# Map DrawableElement to SupportedColor for non-room elements _ELEMENT_COLOR_MAPPING = { DrawableElement.FLOOR: SupportedColor.MAP_BACKGROUND, DrawableElement.WALL: SupportedColor.WALLS, DrawableElement.ROBOT: SupportedColor.ROBOT, DrawableElement.CHARGER: SupportedColor.CHARGER, DrawableElement.VIRTUAL_WALL: SupportedColor.NO_GO, DrawableElement.RESTRICTED_AREA: SupportedColor.NO_GO, DrawableElement.PATH: SupportedColor.PATH, DrawableElement.PREDICTED_PATH: SupportedColor.PREDICTED_PATH, DrawableElement.GO_TO_TARGET: SupportedColor.GO_TO, DrawableElement.NO_MOP_AREA: SupportedColor.NO_GO, DrawableElement.OBSTACLE: SupportedColor.NO_GO, DrawableElement.CARPET: SupportedColor.CARPET, }Then replace both local definitions (lines 86-99 and 207-219) with references to this constant.
🧹 Nitpick comments (2)
SCR/valetudo_map_parser/map_data.py (1)
277-293: Edge case: odd-length pixels array silently drops the last element.The conversion logic correctly handles the case where
compressedPixelsis missing by deriving it frompixels. However, ifpixelshas an odd length (malformed data), the last element is silently dropped. Consider logging a warning for debugging purposes.🔧 Optional: Add warning for malformed data
pixels = json_obj.get("pixels", []) if pixels: + if len(pixels) % 2 != 0: + # Log warning for malformed pixel data + pass # or use appropriate logger # Convert pairs to triplets by adding count=1 for each pixel compressed_pixels = [] for i in range(0, len(pixels), 2):SCR/valetudo_map_parser/hypfer_draw.py (1)
315-364: Dead code:_draw_carpetsmethod is never called.This method appears to be leftover development code. The actual carpet drawing is handled at lines 309-311 using
self.img_h.draw.zonesdirectly. Additionally, the importfrom .config.room_store import RoomStoreon line 319 is incorrect—RoomStoreis already imported from.config.typesat line 12.Consider removing this unused method to avoid confusion and maintenance burden.
♻️ Remove dead code
- async def _draw_carpets( - self, np_array: NumpyArray, carpet_zones: list - ) -> NumpyArray: - """Draw carpet zones with 50% of room color and alpha 255.""" - from .config.room_store import RoomStore - - # Get room store to map segment IDs to room indices - room_store = RoomStore(self.file_name) - room_keys = list(room_store.get_rooms().keys()) - - for carpet in carpet_zones: - try: - # Get the segment ID from carpet metadata - segment_id = carpet.get("metaData", {}).get("id") - if not segment_id: - continue - - # Find the room index for this segment ID - if segment_id in room_keys: - room_index = room_keys.index(segment_id) - else: - # Default to room 0 if segment ID not found - room_index = 0 - - # Get the room color - if room_index < len(self.img_h.shared.rooms_colors): - room_color = self.img_h.shared.rooms_colors[room_index] - else: - room_color = self.img_h.shared.rooms_colors[0] - - # Calculate carpet color: 50% of room color with alpha 255 - carpet_color = ( - room_color[0] // 2, - room_color[1] // 2, - room_color[2] // 2, - 255, # Full opacity - ) - - # Draw the carpet zone - np_array = await self.img_h.draw.zones( - np_array, [carpet], carpet_color - ) - - except (KeyError, TypeError, IndexError) as e: - _LOGGER.warning( - "%s: Error drawing carpet: %s", self.file_name, str(e) - ) - continue - - return np_array
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
SCR/valetudo_map_parser/config/colors.pySCR/valetudo_map_parser/config/drawable_elements.pySCR/valetudo_map_parser/config/shared.pySCR/valetudo_map_parser/config/types.pySCR/valetudo_map_parser/config/utils.pySCR/valetudo_map_parser/const.pySCR/valetudo_map_parser/hypfer_draw.pySCR/valetudo_map_parser/hypfer_handler.pySCR/valetudo_map_parser/map_data.pypyproject.toml
💤 Files with no reviewable changes (1)
- SCR/valetudo_map_parser/config/shared.py
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-19T10:03:55.490Z
Learnt from: sca075
Repo: sca075/Python-package-valetudo-map-parser PR: 29
File: SCR/valetudo_map_parser/hypfer_draw.py:438-462
Timestamp: 2025-12-19T10:03:55.490Z
Learning: In SCR/valetudo_map_parser/hypfer_draw.py, the `_check_room_with_corners` method correctly sets `self.img_h.robot_in_room` before validation. This state mutation pattern during room iteration is intentional and works correctly with the coordinate system used by the Hypfer handler, similar to the Rand256 handler pattern.
Applied to files:
SCR/valetudo_map_parser/hypfer_draw.pySCR/valetudo_map_parser/hypfer_handler.py
📚 Learning: 2025-10-11T14:02:20.278Z
Learnt from: sca075
Repo: sca075/Python-package-valetudo-map-parser PR: 21
File: SCR/valetudo_map_parser/config/drawable.py:56-56
Timestamp: 2025-10-11T14:02:20.278Z
Learning: In the valetudo-map-parser codebase, the Color type definition (`Union[Tuple[int, int, int], Tuple[int, int, int, int]]`) allows both RGB and RGBA to accommodate Home Assistant's RGB selector during configuration (which doesn't accept alpha channels). However, alpha is added during config processing, so by the time colors reach the drawing/processing code in files like drawable.py, they are always RGBA (4-tuple). Therefore, direct access to color[3] is safe in all drawing and processing code, and guards are not needed.
Applied to files:
SCR/valetudo_map_parser/const.pySCR/valetudo_map_parser/config/colors.py
🧬 Code graph analysis (2)
SCR/valetudo_map_parser/config/utils.py (1)
SCR/valetudo_map_parser/config/types.py (3)
TrimsData(293-347)from_list(99-107)from_list(328-338)
SCR/valetudo_map_parser/config/drawable_elements.py (1)
SCR/valetudo_map_parser/config/colors.py (1)
SupportedColor(144-163)
🔇 Additional comments (19)
pyproject.toml (2)
3-3: LGTM: Version bump is appropriate.The patch version increment from 0.1.14 to 0.1.15 is reasonable for the carpet zone feature addition in a pre-1.0 release.
21-21: mvcrender version 0.0.7 is available on PyPI with no known security vulnerabilities.The dependency update to
==0.0.7is valid and verified.SCR/valetudo_map_parser/hypfer_handler.py (2)
133-133: LGTM!The comment clarification accurately documents the existing behavior that floor layers are always drawn when present, improving code readability.
232-235: LGTM!The integration of carpet color into the zone drawing pipeline is clean. Using
colors.get("carpet")safely returnsNonewhen the color isn't configured, and the callee (async_draw_zones) handles this gracefully with its default parametercolor_carpet=None.SCR/valetudo_map_parser/map_data.py (1)
120-135: LGTM!The new
PolygonMetaandPolygonMapEntityTypedDicts follow the established patterns for entity types, and theEntityunion is properly extended to include the new polygon type.SCR/valetudo_map_parser/config/types.py (2)
66-67: LGTM!The new
floorfield is properly added to theTrimCropDatadataclass.
98-107: LGTM!The
from_listmethod correctly accepts an optionalfloorparameter with a sensible default value of"floor_0".SCR/valetudo_map_parser/const.py (3)
36-47: LGTM!The
COLORSlist is correctly extended with "carpet" and "text" entries. The ordering aligns with thebase_color_keysincolors.py, ensuring correct index-based color mapping throughout the codebase.
88-88: LGTM!The
trims_datadefault now includes thefloorfield with a sensible default value of"floor_0", aligning with theTrimCropDatadataclass changes.
97-97: LGTM!The carpet color and alpha constants are properly defined with consistent naming conventions. The default carpet color
[67, 103, 125](a muted blue-gray) provides reasonable visual distinction from floor/room colors.Also applies to: 107-107, 236-236, 266-266
SCR/valetudo_map_parser/hypfer_draw.py (2)
105-122: LGTM!The floor layer handling is improved:
- Floor layers consistently use
rooms_colors[0]for visual uniformity across vacuum modelsroom_idis only incremented for segment layers, preventing index drift when floor layers are processedThis ensures correct room color assignment regardless of layer ordering in the JSON data.
262-313: LGTM!The carpet zone drawing integration is clean:
- The
color_carpetparameter withNonedefault maintains backward compatibility- Carpet rendering is properly gated by three conditions: zones exist, color is provided, and the
CARPETelement is enabled- Follows the established pattern for zone drawing
SCR/valetudo_map_parser/config/colors.py (3)
13-13: LGTM!The carpet color constants are properly imported and defined. The
color_carpetRGBA value(67, 103, 125, 255)correctly combines the RGB fromconst.pywith full opacity.Also applies to: 39-39, 69-69
155-155: LGTM!The
SupportedColor.CARPETenum member and its corresponding RGB entry inCOLORS_RGBare correctly added. The comment accurately notes that(67, 103, 125)represents 50% of the default room_0 color(135, 206, 250).Also applies to: 178-178
302-313: LGTM!The
COLOR_CARPETentry inbase_color_keysis positioned correctly at index 6, matching the position of "carpet" in theCOLORSlist inconst.py. This ensures consistent color mapping across the codebase.SCR/valetudo_map_parser/config/utils.py (1)
276-280: The code change is correct and safe. Theself.shared.current_floorattribute is always initialized to"floor_0"when the shared data object is created (line 124 inshared.py), and it's guaranteed to be set in thefrom_dictmethod regardless of whether device info contains floors data or falls back to defaults (lines 321, 326). NoAttributeErrorrisk exists.SCR/valetudo_map_parser/config/drawable_elements.py (3)
36-36: LGTM!The CARPET enum value is correctly defined and follows the sequential numbering pattern.
86-115: LGTM!The CARPET element is correctly integrated into the default properties:
- Mapped to
SupportedColor.CARPETfor color configuration- Assigned z-index 15, placing it above floor and below walls as intended
263-276: LGTM!The
disable_carpetsflag is correctly mapped toDrawableElement.CARPET, following the same pattern as other element disable flags.
from_list Signed-off-by: SCA075 <82227818+sca075@users.noreply.github.com>
Fixes carpet color/alpha from device_info not being applied due to missing DrawableElement.CARPET entry in update_from_device_info(). Signed-off-by: SCA075 <82227818+sca075@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
SCR/valetudo_map_parser/config/drawable_elements.py (2)
101-115: Avoid ambiguous layering ties (CARPET and OBSTACLE both z=15).
If both are enabled, draw order will effectively depend on enum iteration order whenz_indexties—verify this is the intended stacking (do you want obstacles above carpets?).Possible tweak (make ordering explicit by breaking the tie)
z_indices = { @@ - DrawableElement.CARPET: 15, # Draw carpets above floor but below walls - DrawableElement.OBSTACLE: 15, + DrawableElement.CARPET: 15, # Draw carpets above floor but below walls + DrawableElement.OBSTACLE: 16, # Keep obstacles above carpets (if desired) @@ }
204-261: Device-info color/alpha update for CARPET is correctly hooked up; consider using.valuefor alpha keys.
Todayalpha_key = f"alpha_{color_key}"relies onStrEnumformatting;alpha_{color_key.value}is clearer and avoids surprises if the enum implementation changes.Proposed clarity-only change
- alpha_key = f"alpha_{color_key}" + alpha_key = f"alpha_{color_key.value}" alpha = device_info.get(alpha_key, 255.0)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
SCR/valetudo_map_parser/config/drawable_elements.py
🧰 Additional context used
🧬 Code graph analysis (1)
SCR/valetudo_map_parser/config/drawable_elements.py (1)
SCR/valetudo_map_parser/config/colors.py (1)
SupportedColor(144-163)
🔇 Additional comments (3)
SCR/valetudo_map_parser/config/drawable_elements.py (3)
85-100: Default mapping for CARPET looks correct.
WiringDrawableElement.CARPET→SupportedColor.CARPETensures default color/opacity init runs for carpets as intended.
262-277: The key namedisable_carpets(plural) is correct and consistent with the codebase. It follows the same pattern as other multi-element flags likedisable_virtual_walls,disable_restricted_areas, anddisable_obstacles, which all map to singular enum names. The enumDrawableElement.CARPETis properly defined and actively used throughout the codebase for color mapping and rendering checks.
21-37: No actionable issues identified. The CARPET enum addition is fully integrated with consistent color, alpha, z-index, and disable flag mappings. The implementation handles all integration points correctly.
…tial implementation attempt and is not needed since we're using the simpler approach of passing color_carpet directly from device_info to the zones() drawing method. Signed-off-by: SCA075 <82227818+sca075@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @SCR/valetudo_map_parser/hypfer_draw.py:
- Around line 302-311: The carpet zone drawing block may raise AttributeError by
calling self.img_h.drawing_config.is_enabled without ensuring drawing_config
exists; add the same defensive check pattern used elsewhere: verify
hasattr(self.img_h, "drawing_config") and that self.img_h.drawing_config is not
None before calling is_enabled(DrawableElement.CARPET), and only call
self.img_h.draw.zones(np_array, carpet_zones, color_carpet) when carpet_zones,
color_carpet and the defensive drawing_config check all pass.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
SCR/valetudo_map_parser/hypfer_draw.pypyproject.toml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-19T10:03:55.490Z
Learnt from: sca075
Repo: sca075/Python-package-valetudo-map-parser PR: 29
File: SCR/valetudo_map_parser/hypfer_draw.py:438-462
Timestamp: 2025-12-19T10:03:55.490Z
Learning: In SCR/valetudo_map_parser/hypfer_draw.py, the `_check_room_with_corners` method correctly sets `self.img_h.robot_in_room` before validation. This state mutation pattern during room iteration is intentional and works correctly with the coordinate system used by the Hypfer handler, similar to the Rand256 handler pattern.
Applied to files:
SCR/valetudo_map_parser/hypfer_draw.py
🧬 Code graph analysis (1)
SCR/valetudo_map_parser/hypfer_draw.py (3)
SCR/valetudo_map_parser/config/drawable_elements.py (2)
is_enabled(174-176)DrawableElement(21-53)SCR/valetudo_map_parser/config/drawable.py (1)
zones(315-395)tests/refactored.py (1)
zones(429-448)
🔇 Additional comments (4)
pyproject.toml (2)
3-3: Verify the version skip from 0.1.14 to 0.1.16.The version increment skips 0.1.15. While this can be intentional, please confirm whether version 0.1.15 was used elsewhere or if this skip is intentional.
21-21: The mvcrender version 0.0.7 is legitimate and available on PyPI.Version 0.0.7 exists and is published by the mvcrender maintainer (sca075). The package is a well-maintained C extension library for image processing used by the Valetudo Map Parser project. No security vulnerabilities are reported, and the release includes proper wheel distributions for supported platforms. The dependency is safe to use.
SCR/valetudo_map_parser/hypfer_draw.py (2)
105-122: LGTM: Floor layer handling ensures consistent background color.The logic correctly ensures floor layers always use
rooms_colors[0]while segment layers retain the active room color behavior. The conditional room_id increment prevents room color cycling for floor layers, which is the intended behavior.
268-268: LGTM: Backward-compatible API extension.The optional
color_carpetparameter with a default value ofNonemaintains backward compatibility while enabling carpet zone rendering.
1 participant
This PR adds full support for carpet zone detection and rendering in the Valetudo map parser, and fixes floor rendering to use the correct background color.
✨ What's New
Added CARPET to DrawableElement enum (value: 12, z-index: 15)
Carpets are rendered above floor but below walls
Carpets are detected from Valetudo JSON as PolygonMapEntity with type="carpet"
color_carpet: RGB color for carpet zones (default: [67, 103, 125] - 50% of room_0 color)
alpha_carpet: Alpha transparency (default: 255.0 - full opacity)
disable_carpets: Boolean flag to hide/show carpets (default: false)
Added PolygonMeta TypedDict for polygon metadata
Added PolygonMapEntity TypedDict for polygon-based map entities
Updated Entity union type to include PolygonMapEntity
Summary by CodeRabbit
New Features
Enhancements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.