0.1.12 Release Ready #27
Conversation
…ean tested for rand. Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
…unction with C based from mcvrender. Signed-off-by: Sandro Cantarella <sandro@509dc541-f930-4e46-87da-4482e9d9a7bb.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@13de464c-e6f5-4665-b8d7-d6acf4207b24.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@13de464c-e6f5-4665-b8d7-d6acf4207b24.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@03829bc9-740a-4ae1-9848-e7d8baa7c8ba.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@cabf9afe-642b-4ef6-b6d4-34d9c877830b.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@cabf9afe-642b-4ef6-b6d4-34d9c877830b.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@1b7a9c0c-b720-40e5-a6c3-7b3a28793b2e.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
…lable Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@79f3d049-9006-400d-9954-2e5dcd250fa9.fritz.box>
|
Caution Review failedThe pull request is closed. WalkthroughRefactors many internals: centralizes constants into Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant B as BaseHandler
participant P as RRMapParser
participant H as HypferHandler/ReImageHandler
participant Cst as ColorsManagement
participant Img as ImageConversion
C->>B: async_get_image(request)
B->>B: _backup_last_image()
B->>B: _generate_new_image(m_json, destinations)
alt RRMap format
B->>P: parse_rrm_data(...)
P->>P: _parse_path_data()/ _process_image_pixels()
P-->>B: parsed_map_data
else Hypfer/Rand handler
B->>H: async_get_image_from_json(parsed_map_data)
H->>H: _prepare_data_tasks() / _draw_base_layers()
H->>Cst: set_initial_colours(device_info)
H->>H: _draw_dynamic_elements() / _draw_robot_if_enabled()
end
B->>B: _process_new_image(new_image, destinations, bytes_format)
B->>B: _add_status_text(new_image)
B->>Img: _convert_to_binary(new_image, bytes_format)
B-->>C: image + metadata
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–75 minutes Areas needing focused review:
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
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: 2
🧹 Nitpick comments (2)
SCR/valetudo_map_parser/hypfer_draw.py (1)
379-388: Code duplication detected across handlers.The
_create_robot_position_dictmethod is duplicated in bothhypfer_draw.pyandrand256_handler.py(lines 404-413). Consider extracting this to a shared utility module to reduce duplication and ensure consistency.Example refactor: move to
config/utils.py:def create_robot_position_dict( robot_x: int, robot_y: int, angle: float, room_name: str | None ) -> RobotPosition: """Create a robot position dictionary.""" return { "x": robot_x, "y": robot_y, "angle": angle, "in_room": room_name, }SCR/valetudo_map_parser/const.py (1)
202-209: Consider using regular comments instead of triple-quoted strings for inline documentation.Lines 202, 210, 221, 239, 266, and 284 use triple-quoted strings as comments. While Python treats these as valid, they're actually string literals that create unnamed string objects. For inline documentation that isn't a docstring, regular comments (
#) are more idiomatic and slightly more efficient.Apply this pattern:
-"""App Constants. Not in use, and dummy values""" +# App Constants. Not in use, and dummy values IDLE_SCAN_INTERVAL = 120-"""Base Colours RGB""" +# Base Colours RGB COLOR_CHARGER = "color_charger"-"""Rooms Colours RGB""" +# Rooms Colours RGB COLOR_ROOM_0 = "color_room_0"-"""Alpha for RGBA Colours""" +# Alpha for RGBA Colours ALPHA_CHARGER = "alpha_charger"-""" Constants for the attribute keys """ +# Constants for the attribute keys ATTR_FRIENDLY_NAME = "friendly_name"-# Status text cost +# Status text constants charge_level = "\u03de" # unicode Koppa symbolAlso applies to: 210-220, 221-238, 239-265, 266-283, 284-288
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
SCR/valetudo_map_parser/__init__.py(3 hunks)SCR/valetudo_map_parser/config/async_utils.py(1 hunks)SCR/valetudo_map_parser/config/colors.py(2 hunks)SCR/valetudo_map_parser/config/drawable.py(2 hunks)SCR/valetudo_map_parser/config/rand256_parser.py(5 hunks)SCR/valetudo_map_parser/config/shared.py(5 hunks)SCR/valetudo_map_parser/config/status_text/__init__.py(1 hunks)SCR/valetudo_map_parser/config/status_text/status_text.py(3 hunks)SCR/valetudo_map_parser/config/types.py(6 hunks)SCR/valetudo_map_parser/config/utils.py(5 hunks)SCR/valetudo_map_parser/const.py(1 hunks)SCR/valetudo_map_parser/hypfer_draw.py(1 hunks)SCR/valetudo_map_parser/hypfer_handler.py(5 hunks)SCR/valetudo_map_parser/map_data.py(6 hunks)SCR/valetudo_map_parser/rand256_handler.py(6 hunks)SCR/valetudo_map_parser/rooms_handler.py(3 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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/config/colors.py
🧬 Code graph analysis (9)
SCR/valetudo_map_parser/hypfer_draw.py (2)
SCR/valetudo_map_parser/rand256_handler.py (2)
_create_robot_position_dict(405-414)async_get_robot_in_room(479-522)SCR/valetudo_map_parser/config/utils.py (1)
point_in_polygon(803-845)
SCR/valetudo_map_parser/config/shared.py (2)
SCR/valetudo_map_parser/config/types.py (3)
CameraModes(266-275)FloorData(338-354)TrimsData(280-334)SCR/valetudo_map_parser/config/utils.py (1)
pil_size_rotation(727-735)
SCR/valetudo_map_parser/hypfer_handler.py (5)
SCR/valetudo_map_parser/config/drawable_elements.py (3)
DrawableElement(21-52)is_enabled(171-173)get_property(182-188)SCR/valetudo_map_parser/hypfer_draw.py (8)
async_draw_base_layer(41-85)async_draw_virtual_walls(300-314)async_draw_charger(235-257)async_draw_obstacle(225-233)async_get_robot_in_room(464-513)async_draw_zones(259-298)draw_go_to_flag(27-39)async_draw_paths(316-353)SCR/valetudo_map_parser/map_data.py (1)
get_obstacles(194-232)SCR/valetudo_map_parser/config/drawable.py (1)
robot(398-487)SCR/valetudo_map_parser/config/async_utils.py (2)
AsyncPIL(38-66)async_fromarray(42-44)
SCR/valetudo_map_parser/config/types.py (3)
SCR/valetudo_map_parser/config/drawable.py (1)
zones(315-395)tests/manage_rooms.py (6)
RoomProperty(22-27)RoomStore(32-62)get_rooms_count(54-58)get_rooms(48-49)set_rooms(51-52)get_all_instances(61-62)SCR/valetudo_map_parser/map_data.py (1)
Size(132-136)
SCR/valetudo_map_parser/__init__.py (1)
SCR/valetudo_map_parser/config/utils.py (2)
ResizeParams(32-41)async_resize_image(689-724)
SCR/valetudo_map_parser/config/status_text/status_text.py (2)
SCR/valetudo_map_parser/config/shared.py (1)
vacuum_bat_charged(130-142)SCR/valetudo_map_parser/config/drawable.py (1)
status_text(568-612)
SCR/valetudo_map_parser/rand256_handler.py (6)
SCR/valetudo_map_parser/config/drawable_elements.py (3)
DrawableElement(21-52)is_enabled(171-173)get_property(182-188)SCR/valetudo_map_parser/reimg_draw.py (1)
async_draw_base_layer(70-118)SCR/valetudo_map_parser/config/drawable.py (1)
create_empty_image(44-49)SCR/valetudo_map_parser/config/utils.py (2)
async_copy_array(506-508)point_in_polygon(803-845)SCR/valetudo_map_parser/hypfer_handler.py (1)
async_extract_room_properties(70-89)SCR/valetudo_map_parser/rooms_handler.py (2)
async_extract_room_properties(202-226)async_extract_room_properties(387-467)
SCR/valetudo_map_parser/config/utils.py (5)
SCR/valetudo_map_parser/config/types.py (7)
TrimsData(280-334)Destinations(43-49)to_dict(72-79)to_dict(310-312)to_dict(352-354)from_list(96-103)from_list(315-325)SCR/valetudo_map_parser/rand256_handler.py (1)
get_image_from_rrm(125-197)SCR/valetudo_map_parser/map_data.py (3)
HyperMapData(745-849)async_from_valetudo_json(761-797)to_dict(799-801)SCR/valetudo_map_parser/config/shared.py (1)
to_dict(237-247)SCR/valetudo_map_parser/config/status_text/status_text.py (1)
get_status_text(108-123)
SCR/valetudo_map_parser/config/status_text/__init__.py (2)
SCR/valetudo_map_parser/config/drawable.py (1)
status_text(568-612)SCR/valetudo_map_parser/config/status_text/status_text.py (1)
StatusText(19-123)
🔇 Additional comments (19)
pyproject.toml (1)
3-3: LGTM! Version bump aligns with 0.12.0 release.The version increment from 0.1.11b1 to 0.1.12 is appropriate for this release.
SCR/valetudo_map_parser/hypfer_draw.py (2)
390-462: Good modularization of robot-in-room detection logic.The refactoring into focused helper methods (
_check_cached_room_outline,_check_cached_room_bbox,_check_room_with_outline,_check_room_with_corners) significantly improves code readability and maintainability compared to the previous monolithic approach.
479-479: Map_boundary values are intentional and correct—dismiss this review.The 20000 vs 50000 difference reflects distinct coordinate systems for each vacuum format:
- Hypfer format (hypfer_draw.py line 479): Uses raw robot coordinates with a 20000 boundary threshold
- Rand256 format (rand256_handler.py line 494): Scales robot coordinates by 10x before room detection, using a 50000 boundary threshold
The boundaries are calibrated appropriately for each format's native coordinate ranges and any applied scaling. No changes needed.
SCR/valetudo_map_parser/const.py (1)
1-288: Excellent centralization of constants.Creating a dedicated constants module is a strong architectural improvement that:
- Reduces duplication across the codebase
- Provides a single source of truth for configuration values
- Improves maintainability
The constants are well-organized by category (attributes, configuration, colors, defaults, etc.) and follow consistent naming conventions.
SCR/valetudo_map_parser/config/async_utils.py (1)
52-52: Good update to modern Pillow API.Switching from
Image.LANCZOStoImage.Resampling.LANCZOSaligns with the current Pillow API. The older constant is deprecated, so this change improves forward compatibility.SCR/valetudo_map_parser/config/status_text/__init__.py (1)
1-7: Clean module initialization following best practices.The barrel export pattern with explicit
__all__definition provides a clear public API for the status_text package.SCR/valetudo_map_parser/config/drawable.py (3)
355-360: Improved code formatting in array construction.The multi-line array comprehensions for
xsandysimprove readability without changing functionality.
366-366: Cleaner boolean mask extraction.Using
mask_rgba[:, :, 0] > 0directly is more concise and equally clear.
15-22: Verification complete: Removed functions have proper replacements.The
point_in_polygonfunction is defined inSCR/valetudo_map_parser/config/utils.pyat line 803, and it's actively imported and used throughout the codebase in bothhypfer_draw.pyandrand256_handler.py. For_polygon_outline, the code now usesmvcrender'spolygon_u8function, which is imported in drawable.py. All usages in the production code have been properly migrated to these replacements, with no orphaned function calls remaining.SCR/valetudo_map_parser/rooms_handler.py (5)
14-14: Appropriate pylint directive for scipy import.The
pylint: disable=no-name-in-modulecomment is justified forscipy.spatial.ConvexHull, which can trigger false positives due to scipy's dynamic module structure.
86-86: Modern f-string syntax improvement.Replacing
.format()with an f-string is more idiomatic and readable.
299-300: Better exception handling specificity.Narrowing from generic
ExceptiontoValueErrorandRuntimeErrorimproves error handling precision and makes debugging easier.
106-109: Helpful debug logging added.The debug log message aids troubleshooting when segment ID mapping fails, replacing the no-op
passstatement.
387-467: Enhanced room naming with destinations support.The addition of the
destinationsparameter enables richer room property extraction by incorporating user-defined room names from the destinations JSON. The implementation correctly falls back to default names when destination data is unavailable.SCR/valetudo_map_parser/config/shared.py (4)
13-51: Import refactoring aligns with constants centralization.The imports now correctly reference the new centralized
constmodule and updatedtypesmodule, supporting the PR's objective to split constants from types.
124-125: New floor and trims tracking attributes.The addition of
floors_trims: FloorDataandcurrent_floor: strsupports the new floor and trims save functionality mentioned in the PR objectives.
145-167: Useful helper method for obstacle link composition.The
_compose_obstacle_linksstatic method cleanly encapsulates the logic for building obstacle image URLs, improving code organization.
133-142: Code formatting improvements.The multi-line conditional formatting and trailing comma additions improve readability without altering behavior.
Also applies to: 232-234, 243-243
SCR/valetudo_map_parser/config/types.py (1)
348-354: Handle legacy list payloads when loading floorsIn 0.11.x we persisted trims via
TrimCropData.to_list(), which emits a plain list. Passing that straight intoTrimsData.from_dict(...)will raise aTypeErrorduring upgrade. Please add a guard that detects list inputs and funnels them throughTrimsData.from_list(...)(mind the index ordering) so existing saves continue to load.One way to harden this:
- return cls( - trims=TrimsData.from_dict(data.get("trims", {})), - map_name=data.get("map_name", ""), - ) + trims_payload = data.get("trims", {}) + if isinstance(trims_payload, list): + trims = TrimsData.from_list(trims_payload, floor=data.get("map_name")) + elif isinstance(trims_payload, dict): + trims = TrimsData.from_dict(trims_payload) + elif isinstance(trims_payload, TrimsData): + trims = trims_payload + else: + trims = TrimsData() + return cls(trims=trims, map_name=data.get("map_name", ""))
| trim_up=crop_area[0], | ||
| trim_left=crop_area[1], | ||
| trim_down=crop_area[2], | ||
| trim_right=crop_area[3], | ||
| floor=floor, | ||
| ) |
There was a problem hiding this comment.
Keep TrimsData.floor as a string
Calling TrimsData.from_list(crop_area) without the optional floor argument now writes None into TrimsData.floor, despite the dataclass (and the rest of the codebase) treating that field as a string defaulting to "". This will leak null into persisted payloads and can trigger attribute errors anywhere that expects string methods (e.g. .strip(), .lower()). Please normalize the fallback to an empty string (or update the dataclass typing and audit every consumer).
Apply this diff:
return cls(
trim_up=crop_area[0],
trim_left=crop_area[1],
trim_down=crop_area[2],
trim_right=crop_area[3],
- floor=floor,
+ floor=floor or "",
)📝 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.
| trim_up=crop_area[0], | |
| trim_left=crop_area[1], | |
| trim_down=crop_area[2], | |
| trim_right=crop_area[3], | |
| floor=floor, | |
| ) | |
| trim_up=crop_area[0], | |
| trim_left=crop_area[1], | |
| trim_down=crop_area[2], | |
| trim_right=crop_area[3], | |
| floor=floor or "", | |
| ) |
🤖 Prompt for AI Agents
In SCR/valetudo_map_parser/config/types.py around lines 320 to 325,
TrimsData.floor is being set to the passed-in floor which can be None when
TrimsData.from_list(crop_area) is called; update the call to normalize None to
an empty string so floor remains a str (e.g. use floor or "" / coalesce before
constructing TrimsData) or update the from_list helper to default missing/None
floor to "" so persisted payloads and consumers always receive a string.
| def _convert_to_binary(self, new_image: PilPNG, bytes_format: bool): | ||
| """Convert image to binary PNG bytes.""" | ||
| if bytes_format: | ||
| self.shared.binary_image = pil_to_png_bytes(new_image) | ||
| else: | ||
| self.shared.binary_image = pil_to_png_bytes(self.shared.last_image) | ||
|
|
There was a problem hiding this comment.
Guard _convert_to_binary when no previous image exists
On the initial frame self.shared.last_image is still None, so calling pil_to_png_bytes(self.shared.last_image) raises an AttributeError, aborting the whole image-generation flow. Please skip serialization when no prior frame exists (or only serialize the new frame when explicitly requested).
if bytes_format:
self.shared.binary_image = pil_to_png_bytes(new_image)
- else:
- self.shared.binary_image = pil_to_png_bytes(self.shared.last_image)
+ else:
+ last_image = getattr(self.shared, "last_image", None)
+ self.shared.binary_image = (
+ pil_to_png_bytes(last_image) if last_image is not None else None
+ )🤖 Prompt for AI Agents
In SCR/valetudo_map_parser/config/utils.py around lines 214 to 220,
_convert_to_binary assumes self.shared.last_image exists when bytes_format is
False; guard against None by checking if self.shared.last_image is not None
before serializing it — if bytes_format is True serialize new_image as before,
else if last_image exists serialize last_image, otherwise set
self.shared.binary_image = None (i.e., skip serialization) and return; keep
behavior explicit and update the docstring if needed.
Include:
Summary by CodeRabbit
New Features
Bug Fixes
Chores