Conversation
- Add CongaMapImageHandler: scales Conga JSON (pixelSize, canvas size, entity coordinates) by ×5 before the Hypfer render pipeline, producing solid filled rooms and correctly positioned entities without post-upscale - Add CongaImageDraw: thin subclass of HypferImageDraw; coordinate scaling handled entirely in the handler - Add ImageData.is_conga_map() for detection via congaPixels field presence - Export CongaMapImageHandler and ImageData in public API (__init__.py) - Add tests/test_conga.py: detection, image generation, room extraction, calibration data (4-point vacuum↔map pairs), room data structure - Fix ALLOWED_IMAGE_FORMAT dict formatting in const.py - Bump version 0.2.9 → 0.3.0b0
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Conga vacuum support: new Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant CongaHandler as CongaMapImageHandler
participant MapData as ImageData
participant BaseHandler as HypferMapImageHandler
participant Draw as CongaImageDraw
Caller->>CongaHandler: async_get_image_from_json(m_json)
CongaHandler->>MapData: is_conga_map(m_json)?
alt conga detected
CongaHandler->>CongaHandler: _scale_conga_json(m_json, CONGA_SCALE)
CongaHandler->>BaseHandler: async_get_image_from_json(scaled_json)
else not conga
CongaHandler->>BaseHandler: async_get_image_from_json(m_json)
end
BaseHandler->>Draw: render layers/entities
Draw-->>BaseHandler: Image
BaseHandler-->>CongaHandler: Image
CongaHandler-->>Caller: Image
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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🧪 Generate unit tests (beta)
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 |
sca075
temporarily deployed
to
valetudo_map_parser_pypi
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@SCR/valetudo_map_parser/__init__.py`:
- Around line 1-2: Update the module docstring in __init__.py so the advertised
version matches pyproject.toml: change the version text in the top-level module
string (the triple-quoted docstring in SCR/valetudo_map_parser/__init__.py) from
"0.2.6" to "0.3.0b0" ensuring the exact new version token is used so docs and
quick source inspection reflect the current release.
In `@SCR/valetudo_map_parser/config/shared.py`:
- Around line 154-159: The set_content_type method currently only accepts keys
from ALLOWED_IMAGE_FORMAT (e.g., "pil","png","jpeg") but defaults to
"image/pil", so calling get_content_type() (which returns MIME values) and
passing that back into set_content_type resets formats to PIL; update
set_content_type to accept either a key or a MIME value: if new_image_format is
in ALLOWED_IMAGE_FORMAT keys, set self._image_format =
ALLOWED_IMAGE_FORMAT[new_image_format]; else if new_image_format is in
ALLOWED_IMAGE_FORMAT.values(), set self._image_format = new_image_format;
otherwise fall back to a sensible default (e.g., ALLOWED_IMAGE_FORMAT["pil"]) to
avoid the unexpected reset; reference function set_content_type,
ALLOWED_IMAGE_FORMAT and get_content_type when making the change.
In `@SCR/valetudo_map_parser/config/utils.py`:
- Around line 83-98: Add a factory function that centralizes handler selection
by calling ImageData.detect_vacuum_type(json_data) and branching on its return
value to instantiate the correct handler: when it returns "conga" return a
CongaMapImageHandler(shared) instance, when "rand256" return
ReImageHandler(shared) (or the existing Re/Rand handler), otherwise return
HypferMapImageHandler(shared); replace existing direct instantiations (e.g.,
places creating HypferMapImageHandler(shared) directly) to use this new factory
so CongaMapImageHandler can be reached via the detector.
In `@SCR/valetudo_map_parser/map_data.py`:
- Around line 300-303: The code only falls back to congaPixels when
compressed_pixels is exactly None, which misses upstream payloads that provide
an empty list; change the condition in the ImageData parsing so empty
compressed_pixels is treated the same as missing data (e.g., replace "if
compressed_pixels is None:" with a falsy check like "if not
compressed_pixels:"), then build pixels from json_obj.get("pixels") or
json_obj.get("congaPixels", []) and call
ImageData._convert_pixels_to_compressed(pixels) as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5e71a0d0-95da-49f4-85fb-baa104afb09d
📒 Files selected for processing (7)
SCR/valetudo_map_parser/__init__.pySCR/valetudo_map_parser/config/shared.pySCR/valetudo_map_parser/config/utils.pySCR/valetudo_map_parser/const.pySCR/valetudo_map_parser/map_data.pySCR/valetudo_map_parser/reimg_draw.pypyproject.toml
- Add CongaMapImageHandler: scales Conga JSON (pixelSize, canvas size, entity coordinates) by ×5 before the Hypfer render pipeline, producing solid filled rooms and correctly positioned entities without post-upscale - Add CongaImageDraw: thin subclass of HypferImageDraw; coordinate scaling handled entirely in the handler - Add ImageData.is_conga_map() for detection via congaPixels field presence - Export CongaMapImageHandler and ImageData in public API (__init__.py) - Add tests/test_conga.py: detection, image generation, room extraction, calibration data (4-point vacuum↔map pairs), room data structure - Fix ALLOWED_IMAGE_FORMAT dict formatting in const.py - Bump version 0.2.9 → 0.3.0b0
There was a problem hiding this comment.
🧹 Nitpick comments (1)
SCR/valetudo_map_parser/conga_handler.py (1)
46-52: ScalepixelSizefrom the payload, not from an assumption.Line 47 hardcodes
pixelSizetoscale, while the rest of the method multiplies source values. That stays correct only while every Conga map remainspixelSize == 1; using the incoming value keeps the transform self-consistent and avoids future skew if a payload variant changes.♻️ Proposed fix
scaled = copy.deepcopy(m_json) - scaled["pixelSize"] = scale + scaled["pixelSize"] = int(scaled.get("pixelSize", 1)) * scale size = scaled.get("size", {}) scaled["size"] = { "x": int(size.get("x", 800)) * scale, "y": int(size.get("y", 800)) * scale, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@SCR/valetudo_map_parser/conga_handler.py` around lines 46 - 52, The code currently sets scaled["pixelSize"] = scale which assumes the incoming payload's pixelSize is 1; instead read the incoming value from m_json and multiply it by scale so the transform stays self-consistent. Change the assignment to compute original_pixel = int(m_json.get("pixelSize", 1)) and set scaled["pixelSize"] = original_pixel * scale (leaving the existing size scaling logic as-is), referencing the variables scaled, m_json, pixelSize, scale, and size to locate the change in conga_handler.py.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@SCR/valetudo_map_parser/conga_handler.py`:
- Around line 46-52: The code currently sets scaled["pixelSize"] = scale which
assumes the incoming payload's pixelSize is 1; instead read the incoming value
from m_json and multiply it by scale so the transform stays self-consistent.
Change the assignment to compute original_pixel = int(m_json.get("pixelSize",
1)) and set scaled["pixelSize"] = original_pixel * scale (leaving the existing
size scaling logic as-is), referencing the variables scaled, m_json, pixelSize,
scale, and size to locate the change in conga_handler.py.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c3eb75cf-f6af-47d6-8a49-582c4a23fec0
📒 Files selected for processing (2)
SCR/valetudo_map_parser/conga_draw.pySCR/valetudo_map_parser/conga_handler.py
sca075
temporarily deployed
to
valetudo_map_parser_pypi
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pyproject.toml`:
- Line 3: The pyproject.toml version field is set to "0.3.0b1" but the PR states
the intended prerelease is "v0.3.0b0"; update the version string in
pyproject.toml (version = "0.3.0b1") to the correct prerelease identifier to
match the release plan and ensure package metadata stays in sync with the
package __init__.py (e.g., the __version__ or VERSION constant) so tags and
published artifacts use the intended prerelease number.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 82684260-a66d-4f6e-9f56-5425a6c4b490
📒 Files selected for processing (2)
SCR/valetudo_map_parser/__init__.pypyproject.toml
- Add CongaMapImageHandler subclassing HypferMapImageHandler with JSON coordinate scaling (pixelSize=1 → 5) via async_get_conga_from_json - Add CongaImageDraw subclassing HypferImageDraw as Conga draw engine - Add is_conga: bool = False to CameraShared for vacuum type detection - Route Conga in BaseHandler._generate_new_image via hasattr duck-typing - Remove is_conga_map() from ImageData and detect_vacuum_type() from BaseHandler — detection is mqtt_vacuum_camera's responsibility - Export CongaMapImageHandler and ImageData from public API
sca075
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
SCR/valetudo_map_parser/map_data.py (1)
288-291:⚠️ Potential issue | 🟡 MinorTreat empty
compressedPixelsas missing data too.The Conga fallback still only runs when
compressedPixels is None. If an upstream payload sendscompressedPixels: [], this path never readscongaPixelsand the layer is stored empty.Proposed fix
- compressed_pixels = json_obj.get("compressedPixels") - if compressed_pixels is None: + compressed_pixels = json_obj.get("compressedPixels") + if not compressed_pixels: pixels = json_obj.get("pixels") or json_obj.get("congaPixels", []) compressed_pixels = ImageData._convert_pixels_to_compressed(pixels)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@SCR/valetudo_map_parser/map_data.py` around lines 288 - 291, The code only falls back to congaPixels when compressed_pixels is None, so an upstream empty array compressedPixels: [] is treated as present and prevents using congaPixels; update the condition in the map parsing logic to treat empty lists as missing (e.g., check for falsy or len==0) so that when compressed_pixels is None or empty you retrieve pixels = json_obj.get("pixels") or json_obj.get("congaPixels", []) and call ImageData._convert_pixels_to_compressed(pixels) to populate compressed_pixels.SCR/valetudo_map_parser/config/shared.py (1)
155-160:⚠️ Potential issue | 🟡 MinorAccept MIME values here or change the default.
get_content_type()returns MIME values, but this setter still only accepts the short keys fromALLOWED_IMAGE_FORMAT. Passing the current content type back intoset_content_type()will silently reset PNG/JPEG outputs to PIL.Proposed fix
- def set_content_type(self, new_image_format: str = "image/pil") -> None: - """Set image format / content type""" - if new_image_format not in ALLOWED_IMAGE_FORMAT.keys(): - self._image_format = "image/pil" - return - self._image_format = ALLOWED_IMAGE_FORMAT.get(new_image_format) + def set_content_type(self, new_image_format: str = "pil") -> None: + """Set image format / content type.""" + if new_image_format in ALLOWED_IMAGE_FORMAT: + self._image_format = ALLOWED_IMAGE_FORMAT[new_image_format] + return + if new_image_format in ALLOWED_IMAGE_FORMAT.values(): + self._image_format = new_image_format + return + self._image_format = ALLOWED_IMAGE_FORMAT["pil"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@SCR/valetudo_map_parser/config/shared.py` around lines 155 - 160, set_content_type currently only accepts short keys from ALLOWED_IMAGE_FORMAT and resets when passed a MIME string; update it to accept either the mapping keys or the MIME values returned by get_content_type. In set_content_type (and using ALLOWED_IMAGE_FORMAT and _image_format), check: if new_image_format is a key => set _image_format to ALLOWED_IMAGE_FORMAT[new_image_format]; else if new_image_format is one of ALLOWED_IMAGE_FORMAT.values() => set _image_format to new_image_format; otherwise fall back to the default ("image/pil" or the chosen canonical default) and return. Ensure the stored _image_format is always the MIME string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@SCR/valetudo_map_parser/conga_handler.py`:
- Around line 32-35: The Conga handler constructor does not set the shared flag;
in the __init__ method of the Conga handler (the def __init__(self, shared_data:
CameraShared) -> None: constructor that currently calls
super().__init__(shared_data) and creates CongaImageDraw(self)), set
shared_data.is_conga = True to keep CameraShared.is_conga synchronized with the
active handler; update that constructor to assign the flag immediately after
calling super().__init__ (and consider leaving a brief comment noting the state
synchronization).
---
Duplicate comments:
In `@SCR/valetudo_map_parser/config/shared.py`:
- Around line 155-160: set_content_type currently only accepts short keys from
ALLOWED_IMAGE_FORMAT and resets when passed a MIME string; update it to accept
either the mapping keys or the MIME values returned by get_content_type. In
set_content_type (and using ALLOWED_IMAGE_FORMAT and _image_format), check: if
new_image_format is a key => set _image_format to
ALLOWED_IMAGE_FORMAT[new_image_format]; else if new_image_format is one of
ALLOWED_IMAGE_FORMAT.values() => set _image_format to new_image_format;
otherwise fall back to the default ("image/pil" or the chosen canonical default)
and return. Ensure the stored _image_format is always the MIME string.
In `@SCR/valetudo_map_parser/map_data.py`:
- Around line 288-291: The code only falls back to congaPixels when
compressed_pixels is None, so an upstream empty array compressedPixels: [] is
treated as present and prevents using congaPixels; update the condition in the
map parsing logic to treat empty lists as missing (e.g., check for falsy or
len==0) so that when compressed_pixels is None or empty you retrieve pixels =
json_obj.get("pixels") or json_obj.get("congaPixels", []) and call
ImageData._convert_pixels_to_compressed(pixels) to populate compressed_pixels.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a237c16e-24d2-4243-bbce-35410d38c7b7
📒 Files selected for processing (4)
SCR/valetudo_map_parser/config/shared.pySCR/valetudo_map_parser/config/utils.pySCR/valetudo_map_parser/conga_handler.pySCR/valetudo_map_parser/map_data.py
🚧 Files skipped from review as they are similar to previous changes (1)
- SCR/valetudo_map_parser/config/utils.py
Comment on lines
+32
to
+35
| def __init__(self, shared_data: CameraShared) -> None: | ||
| """Initialise the Conga image handler.""" | ||
| super().__init__(shared_data) | ||
| self.imd = CongaImageDraw(self) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Assignments and reads of is_conga:\n'
rg -n -C2 '\bis_conga\b'
printf '\nConga handler construction sites:\n'
rg -n -C2 '\bCongaMapImageHandler\s*\('Repository: sca075/Python-package-valetudo-map-parser
Length of output: 925
Set shared_data.is_conga = True in the Conga handler constructor to keep state synchronized.
CameraShared.is_conga is initialized to False and is never set to True when the Conga handler is active. While the flag is not currently read elsewhere in the codebase, keeping shared state synchronized with the active handler is a valid code hygiene practice.
Suggested fix
def __init__(self, shared_data: CameraShared) -> None:
"""Initialise the Conga image handler."""
super().__init__(shared_data)
+ self.shared.is_conga = True
self.imd = CongaImageDraw(self)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@SCR/valetudo_map_parser/conga_handler.py` around lines 32 - 35, The Conga
handler constructor does not set the shared flag; in the __init__ method of the
Conga handler (the def __init__(self, shared_data: CameraShared) -> None:
constructor that currently calls super().__init__(shared_data) and creates
CongaImageDraw(self)), set shared_data.is_conga = True to keep
CameraShared.is_conga synchronized with the active handler; update that
constructor to assign the flag immediately after calling super().__init__ (and
consider leaving a brief comment noting the state synchronization).
There was a problem hiding this comment.
♻️ Duplicate comments (1)
SCR/valetudo_map_parser/__init__.py (1)
2-2:⚠️ Potential issue | 🟡 MinorAlign the module docstring version with the release metadata.
Line 2 advertises
0.3.0b2, but this PR’s stated release target is0.3.0b0. Please keep the header in sync with the package version so source inspection and generated docs do not report a different build.#!/bin/bash # Verify that package metadata and source-advertised versions agree. # Expected result: the same version token appears in package metadata and this module docstring. fd -HI 'pyproject.toml|setup.cfg|setup.py' . rg -n '(^version\s*=)|(__version__\s*=)|(^Version: )' pyproject.toml setup.cfg setup.py SCR 2>/dev/null🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@SCR/valetudo_map_parser/__init__.py` at line 2, Update the module docstring version string in SCR/valetudo_map_parser/__init__.py (the "Version: 0.3.0b2" header) to match the release metadata token (0.3.0b0) so source-advertised version aligns with package metadata; check the package metadata file (pyproject.toml / setup.cfg / setup.py) for the canonical version and change the Version header in __init__.py to that exact token, and re-run the provided verification commands to confirm all version tokens match.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@SCR/valetudo_map_parser/__init__.py`:
- Line 2: Update the module docstring version string in
SCR/valetudo_map_parser/__init__.py (the "Version: 0.3.0b2" header) to match the
release metadata token (0.3.0b0) so source-advertised version aligns with
package metadata; check the package metadata file (pyproject.toml / setup.cfg /
setup.py) for the canonical version and change the Version header in __init__.py
to that exact token, and re-run the provided verification commands to confirm
all version tokens match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 17467792-e2da-4d2c-b9af-628238b35f33
📒 Files selected for processing (2)
SCR/valetudo_map_parser/__init__.pypyproject.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
- Fix set_content_type() to accept both short keys ('pil','png','jpeg')
and MIME values ('image/pil','image/png','image/jpeg'); corrected
default parameter from 'image/pil' to 'pil' — round-trip
set→get→set now preserves the format correctly
- Fix HyperMapData.async_from_valetudo_json: replace bare
json_data["pixelSize"] and json_data["layers"] with safe .get()
calls (defaults: 5 and []) to avoid KeyError on malformed JSON
- Add TestSetGetContentType (10 pytest cases) covering keys, MIME
values, round-trips, invalid input fallback, and default parameter
sca075
temporarily deployed
to
valetudo_map_parser_pypi
GitHub Actions
Inactive
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.
feat: add Conga vacuum support — bump to v0.3.0b2
entity coordinates) by ×5 before the Hypfer render pipeline, producing
solid filled rooms and correctly positioned entities without post-upscale
handled entirely in the handler
calibration data (4-point vacuum↔map pairs), room data structure
Summary by CodeRabbit
New Features
Chores