Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pre-release last adjustments #293

Merged
merged 3 commits into from
Dec 3, 2024
Merged

Pre-release last adjustments #293

merged 3 commits into from
Dec 3, 2024

Conversation

sca075
Copy link
Owner

@sca075 sca075 commented Dec 3, 2024

Reformat and corrections of several files added also docs.

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced "Obstacle Detection and Image Processing" feature for visualizing obstacles detected by vacuums.
    • Enhanced service options for the MQTT Vacuum Camera integration, including new services like turn_on, turn_off, and snapshot.
  • Bug Fixes

    • Improved error handling for various file operations and service configurations.
  • Documentation

    • Added detailed documentation for the new obstacle detection feature.
  • Chores

    • Updated version number from beta to stable in the manifest file.

@sca075 sca075 self-assigned this Dec 3, 2024
@sca075 sca075 added documentation Improvements or additions to documentation enhancement New feature or request labels Dec 3, 2024
Copy link

coderabbitai bot commented Dec 3, 2024

Walkthrough

The changes in this pull request involve multiple updates across various files in the mqtt_vacuum_camera component. Key modifications include restructuring import statements, enhancing service management, improving error handling, and refining logging for better clarity. New services related to obstacle detection and image processing have been added, alongside documentation for the new features. Overall, the updates focus on improving code organization, readability, and maintainability without altering the core functionalities.

Changes

File Change Summary
custom_components/mqtt_vacuum_camera/__init__.py Reorganized imports; removed async_clean_up_all_auto_crop_files from imports; clarified service registration in async_setup_entry; updated async_unload_entry to ensure safe service removal.
custom_components/mqtt_vacuum_camera/camera.py Adjusted import statements; refactored handle_obstacle_view for readability; updated logging in download_image; added warning log in _process_parsed_json.
custom_components/mqtt_vacuum_camera/common.py Reformatted function signatures for readability; modified get_entity_id logic to move return statement inside loop; improved exception handling in update_options.
custom_components/mqtt_vacuum_camera/config_flow.py Reorganized imports; enhanced error handling in async_step_user and async_step_init; updated OptionsFlowHandler for better options handling.
custom_components/mqtt_vacuum_camera/const.py Added version comment "Version v2024.12.0"; modified comment formatting for CameraModes.
custom_components/mqtt_vacuum_camera/services.yaml Added descriptions and requirements for services; defined new services: reload, turn_off, turn_on, snapshot, reset_trims, obstacle_view, vacuum_go_to, vacuum_clean_zone, vacuum_clean_segment, vacuum_map_save, vacuum_map_load.
custom_components/mqtt_vacuum_camera/snapshots/log_files.py Updated module docstring; improved error handling in async_get_filtered_logs; maintained existing logic in other functions.
custom_components/mqtt_vacuum_camera/utils/auto_crop.py Reformatted _async_save_auto_crop_data call for readability; maintained existing error handling.
custom_components/mqtt_vacuum_camera/utils/camera/camera_services.py Adjusted imports; updated reset_trims to call async_clean_up_all_auto_crop_files; refined logging in reload_camera_config and obstacle_view.
custom_components/mqtt_vacuum_camera/utils/camera/camera_shared.py Moved CameraModes import position; initialized attributes in CameraShared and CameraSharedManager classes.
custom_components/mqtt_vacuum_camera/utils/drawable.py Added ERROR_OUTLINE and ERROR_COLOR constants; modified conditional check in status_text.
custom_components/mqtt_vacuum_camera/utils/files_operations.py Added error handling in JSON file operations; introduced async_clean_up_all_auto_crop_files function; updated async_rename_room_description and async_reset_map_trims for better error management.
custom_components/mqtt_vacuum_camera/utils/img_data.py Changed conditional check in get_rrm_goto_target for clarity.
custom_components/mqtt_vacuum_camera/utils/status_text.py Updated Unicode character handling in StatusText class.
custom_components/mqtt_vacuum_camera/valetudo/MQTT/connector.py Modified IP address handling in async_message_received method to prioritize IPv4 addresses.
custom_components/mqtt_vacuum_camera/valetudo/hypfer/image_handler.py Restructured variable assignments for clarity; enhanced error handling in async_map_coordinates_offset.
custom_components/mqtt_vacuum_camera/valetudo/rand256/image_handler.py Reformatted assignments for readability; maintained existing error handling.
custom_components/mqtt_vacuum_camera/valetudo/rand256/reimg_draw.py Reformatted assignments for clarity; refined exception handling in several methods.
docs/obstacles_detection.md Added documentation for the "Obstacle Detection and Image Processing" feature, detailing its functionality and configuration.

Possibly related PRs

Suggested labels

dependencies


📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between abc9edb and 3b52280.

📒 Files selected for processing (1)
  • custom_components/mqtt_vacuum_camera/manifest.json (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • custom_components/mqtt_vacuum_camera/manifest.json

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Outside diff range and nitpick comments (16)
custom_components/mqtt_vacuum_camera/utils/camera/camera_services.py (2)

Line range hint 27-60: Consider making the timeout configurable 🔧

The reload function is rock solid, but that hardcoded 10-second timeout might be worth making configurable for different environments.

+from ...const import RELOAD_TIMEOUT_SECONDS  # Add to const.py with default of 10
-            with async_timeout.timeout(10):
+            with async_timeout.timeout(RELOAD_TIMEOUT_SECONDS):

62-85: Add coordinate validation for extra robustness 💪

The obstacle view function is clean, but let's add some validation for those coordinates to make it bulletproof.

     coordinates_x = call.data.get("coordinates_x")
     coordinates_y = call.data.get("coordinates_y")
+    if not isinstance(coordinates_x, (int, float)) or not isinstance(coordinates_y, (int, float)):
+        raise ValueError("Coordinates must be numeric values")
+    if coordinates_x < 0 or coordinates_y < 0:
+        raise ValueError("Coordinates must be positive values")
docs/obstacles_detection.md (4)

12-13: Fix typo in Xiaomi name 🔍

There's a small typo in "Xaiomi" - should be "Xiaomi".

🧰 Tools
🪛 LanguageTool

[style] ~13-~13: Consider a shorter alternative to avoid wordiness.
Context: ...lovelace-xiaomi-vacuum-map-card). - In order to select the obstacle that is highlight i...

(IN_ORDER_TO_PREMIUM)


[uncategorized] ~13-~13: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...In order to select the obstacle that is highlight in a red dot drawn on the map, it is ne...

(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)

🪛 Markdownlint (0.35.0)

12-12: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


13-13: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


47-48: Clean up the sentence structure 📝

The sentence needs better punctuation and structure.

-    - The system locates the nearest obstacle to the given coordinates it isn't necessary to point directly on it.
+    - The system locates the nearest obstacle to the given coordinates; it isn't necessary to point directly at it.
🧰 Tools
🪛 LanguageTool

[uncategorized] ~47-~47: A punctuation mark might be missing here.
Context: ...earest obstacle to the given coordinates it isn't necessary to point directly on it...

(AI_EN_LECTOR_MISSING_PUNCTUATION)


[uncategorized] ~47-~47: The preposition “at” seems more likely in this position.
Context: ...es it isn't necessary to point directly on it. - 3. **Image Download and Proc...

(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)


57-57: Fix typo in "any ware" 🎯

Small typo needs fixing.

-    - Clicking any ware obstacle image switches back to `Map View`.
+    - Clicking anywhere on the obstacle image switches back to `Map View`.

65-66: Fix grammar in vacuum support section 📚

The grammar needs some tweaking in this section.

-If the vacuum do not support the `ObstacleImagesCapability`, the Camera will simply display the obstacles with a Red Dot on the map, when the Vacuum support Obstacle Detections and has no oboard camera.
+If the vacuum does not support the `ObstacleImagesCapability`, the Camera will simply display the obstacles with a Red Dot on the map when the Vacuum supports Obstacle Detection and has no onboard camera.
🧰 Tools
🪛 LanguageTool

[grammar] ~65-~65: “Vacuum” is a singular noun. It appears that the verb form is incorrect.
Context: ...tes ### Supported Vacuums If the vacuum do not support the `ObstacleImagesCapabili...

(PCT_SINGULAR_NOUN_PLURAL_VERB_AGREEMENT)


[uncategorized] ~65-~65: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...h a Red Dot on the map, when the Vacuum support Obstacle Detections and has no oboard c...

(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)

custom_components/mqtt_vacuum_camera/services.yaml (1)

45-51: Align indentation with coordinates_x for consistency 🎨

The indentation of the coordinates_y section should match coordinates_x for better readability.

     coordinates_y:
       name: y
       description: Coordinate y for the obstacle view.
       required: true
       selector:
-          number:
-              min: 0
-              max: 90000
+        number:
+          min: 0
+          max: 90000
custom_components/mqtt_vacuum_camera/utils/status_text.py (1)

91-91: Consider using string constants for Unicode characters

The Unicode characters are scattered throughout the code. Consider defining them as constants at the module level for better maintainability and reusability.

+ # At module level
+ UNICODE_KOPPA = "\u03de"  # Koppa symbol
+ UNICODE_CHARGING = "\u2211"  # Charging symbol
+ UNICODE_MIDDLE_DOT = "\u00b7"  # Middle dot

- charge_level = "\u03de"  # unicode Koppa symbol
+ charge_level = UNICODE_KOPPA

Also applies to: 109-109, 114-114, 119-119

custom_components/mqtt_vacuum_camera/snapshots/log_files.py (3)

1-4: Fix docstring formatting

The docstring should have consistent spacing and formatting.

-"""Logs and files colloection
-MQTT Vacuum Camera component for Home Assistant
-Version: v2024.10.0"""
+"""
+Logs and files collection.
+
+MQTT Vacuum Camera component for Home Assistant.
+Version: v2024.10.0
+"""

Line range hint 24-41: Enhance file operation security

The file operations could benefit from additional security measures:

  1. Path traversal prevention
  2. File size limits for log files
  3. Secure file permissions

Consider adding these safety checks:

 async def async_get_filtered_logs(base_path, directory_path: str, file_name):
+    # Prevent path traversal
+    safe_file_name = os.path.basename(file_name)
+    if not os.path.normpath(directory_path).startswith(base_path):
+        raise ValueError("Invalid directory path")
+
     log_file_path = os.path.join(base_path, "home-assistant.log")
-    tmp_log_file_path = os.path.join(directory_path, f"{file_name}.tmp")
+    tmp_log_file_path = os.path.join(directory_path, f"{safe_file_name}.tmp")
+
+    # Add file size check
+    if os.path.exists(log_file_path) and os.path.getsize(log_file_path) > 50_000_000:  # 50MB
+        raise ValueError("Log file too large")

Line range hint 156-167: Consider using asyncio.create_task instead of ThreadPoolExecutor

For I/O-bound operations like file handling, asyncio.create_task might be more efficient than using a thread pool.

-    with concurrent.futures.ThreadPoolExecutor(
-        max_workers=1, thread_name_prefix=f"{file_name}_LogsSave"
-    ) as executor:
-        tasks = [
-            loop.run_in_executor(
-                executor,
-                process_logs,
-                hass,
-                file_name,
-            )
-        ]
-        logs_save = await gather(*tasks)
+    task = asyncio.create_task(async_logs_store(hass, file_name))
+    logs_save = await task
custom_components/mqtt_vacuum_camera/valetudo/MQTT/connector.py (1)

409-412: Add validation for IP address format and enhance logging

The IP address handling could be more robust with format validation and detailed logging.

-            vacuum_host_ip = await self.async_decode_mqtt_payload(msg)
-            # When IPV4 and IPV6 are available, use IPV4
-            if vacuum_host_ip.split(",").__len__() > 1:
-                self._shared.vacuum_ips = vacuum_host_ip.split(",")[0]
+            vacuum_host_ip = await self.async_decode_mqtt_payload(msg)
+            ip_addresses = vacuum_host_ip.split(",")
+            # Log all available IP addresses
+            _LOGGER.debug(f"Available vacuum IPs: {ip_addresses}")
+            # When IPV4 and IPV6 are available, use IPV4
+            if len(ip_addresses) > 1:
+                # Basic IPv4 format validation
+                ipv4 = ip_addresses[0].strip()
+                if all(part.isdigit() and 0 <= int(part) <= 255 for part in ipv4.split(".")):
+                    self._shared.vacuum_ips = ipv4
+                    _LOGGER.debug(f"Selected IPv4 address: {ipv4}")
+                else:
+                    _LOGGER.warning(f"Invalid IPv4 format: {ipv4}, using original value")
+                    self._shared.vacuum_ips = vacuum_host_ip
custom_components/mqtt_vacuum_camera/utils/drawable.py (1)

547-547: Consider using string constants for special characters

The unicode character comparison could be more maintainable using named constants.

+    # Special unicode characters
+    SIGMA_CHAR = "\u2211"
+    DELTA_CHAR = "\u03de"
+
     @staticmethod
     def status_text(...):
-            if "\u2211" in text or "\u03de" in text:
+            if cls.SIGMA_CHAR in text or cls.DELTA_CHAR in text:
custom_components/mqtt_vacuum_camera/camera.py (1)

Line range hint 254-288: Simplify nested conditionals in obstacle view handling

The nested conditionals could be simplified for better readability and maintainability.

-        if (
-            self._shared.obstacles_data
-            and self._shared.camera_mode == CameraModes.MAP_VIEW
-        ):
+        if not self._shared.obstacles_data:
+            _LOGGER.debug("No obstacles data available.")
+            self._should_poll = True
+            return
+
+        if self._shared.camera_mode != CameraModes.MAP_VIEW:
+            return
+
+        _LOGGER.debug(f"Received event: {event.event_type}, Data: {event.data}")
+        if event.data.get("entity_id") != self.entity_id:
+            return
custom_components/mqtt_vacuum_camera/config_flow.py (2)

Line range hint 871-884: Enhance error handling in options storage

The error handling in the options storage could be improved for better debugging and user experience.

Consider applying these improvements:

 _LOGGER.info(
     f"Storing Updated Camera ({self.camera_config.unique_id}) Options."
 )
 try:
     _, vacuum_device = get_vacuum_device_info(
         self.camera_config.data.get(CONF_VACUUM_CONFIG_ENTRY_ID), self.hass
     )
     opt_update = await update_options(self.bk_options, self.camera_options)
     _LOGGER.debug(f"updated options:{dict(opt_update)}")
     return self.async_create_entry(
         title="",
         data=opt_update,
     )
-except Exception as e:
-    _LOGGER.error(f"Error in storing the options: {e}")
-    return self.async_abort(reason="error")
+except ValueError as e:
+    _LOGGER.error("Invalid value in options: %s", e)
+    return self.async_abort(reason="invalid_options")
+except KeyError as e:
+    _LOGGER.error("Missing required option: %s", e)
+    return self.async_abort(reason="missing_options")
+except Exception as e:
+    _LOGGER.exception("Unexpected error storing options: %s", e)
+    return self.async_abort(reason="unknown_error")

Line range hint 352-355: Enhance rooms validation in async_step_init

The rooms validation could be more robust by providing specific error messages for different validation failures.

Consider applying this improvement:

 if (
     not isinstance(self.number_of_rooms, int)
     or self.number_of_rooms < DEFAULT_ROOMS
 ):
-    errors["base"] = "no_rooms"
-    _LOGGER.error("No rooms found in the configuration. Aborting.")
+    if not isinstance(self.number_of_rooms, int):
+        errors["base"] = "invalid_rooms_type"
+        _LOGGER.error("Invalid rooms count type: %s. Expected integer.", type(self.number_of_rooms))
+    else:
+        errors["base"] = "no_rooms"
+        _LOGGER.error("Invalid rooms count: %d. Expected at least %d.", self.number_of_rooms, DEFAULT_ROOMS)
     return self.async_abort(reason="no_rooms")
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between a59996c and abc9edb.

📒 Files selected for processing (19)
  • custom_components/mqtt_vacuum_camera/__init__.py (1 hunks)
  • custom_components/mqtt_vacuum_camera/camera.py (8 hunks)
  • custom_components/mqtt_vacuum_camera/common.py (2 hunks)
  • custom_components/mqtt_vacuum_camera/config_flow.py (2 hunks)
  • custom_components/mqtt_vacuum_camera/const.py (2 hunks)
  • custom_components/mqtt_vacuum_camera/services.yaml (1 hunks)
  • custom_components/mqtt_vacuum_camera/snapshots/log_files.py (1 hunks)
  • custom_components/mqtt_vacuum_camera/utils/auto_crop.py (1 hunks)
  • custom_components/mqtt_vacuum_camera/utils/camera/camera_services.py (2 hunks)
  • custom_components/mqtt_vacuum_camera/utils/camera/camera_shared.py (1 hunks)
  • custom_components/mqtt_vacuum_camera/utils/drawable.py (2 hunks)
  • custom_components/mqtt_vacuum_camera/utils/files_operations.py (1 hunks)
  • custom_components/mqtt_vacuum_camera/utils/img_data.py (1 hunks)
  • custom_components/mqtt_vacuum_camera/utils/status_text.py (2 hunks)
  • custom_components/mqtt_vacuum_camera/valetudo/MQTT/connector.py (1 hunks)
  • custom_components/mqtt_vacuum_camera/valetudo/hypfer/image_handler.py (3 hunks)
  • custom_components/mqtt_vacuum_camera/valetudo/rand256/image_handler.py (3 hunks)
  • custom_components/mqtt_vacuum_camera/valetudo/rand256/reimg_draw.py (1 hunks)
  • docs/obstacles_detection.md (1 hunks)
✅ Files skipped from review due to trivial changes (3)
  • custom_components/mqtt_vacuum_camera/const.py
  • custom_components/mqtt_vacuum_camera/utils/camera/camera_shared.py
  • custom_components/mqtt_vacuum_camera/utils/auto_crop.py
🧰 Additional context used
📓 Learnings (2)
custom_components/mqtt_vacuum_camera/valetudo/MQTT/connector.py (3)
Learnt from: sca075
PR: sca075/mqtt_vacuum_camera#253
File: custom_components/mqtt_vacuum_camera/valetudo/MQTT/connector.py:347-349
Timestamp: 2024-11-12T05:06:42.185Z
Learning: In the `ValetudoConnector`, when handling the MQTT command topic in `async_message_received`, `msg.payload` can be used directly without decoding, as `async_decode_mqtt_payload` immediately returns the payload without encoding for this attribute.
Learnt from: sca075
PR: sca075/mqtt_vacuum_camera#260
File: custom_components/mqtt_vacuum_camera/valetudo/MQTT/connector.py:153-157
Timestamp: 2024-11-12T05:06:42.185Z
Learning: Always use `self.async_decode_mqtt_payload(msg)` when handling MQTT messages in `async_message_received` in `connector.py` to ensure payloads are correctly decoded, as the payload format may change in the future.
Learnt from: sca075
PR: sca075/mqtt_vacuum_camera#256
File: custom_components/mqtt_vacuum_camera/valetudo/MQTT/connector.py:396-427
Timestamp: 2024-11-12T05:06:42.185Z
Learning: In the `ValetudoConnector.async_decode_mqtt_payload` method, the developer prefers using `string_payload.isdigit()` to check if the payload is numeric, rather than relying on try-except blocks during type conversion.
custom_components/mqtt_vacuum_camera/config_flow.py (4)
Learnt from: sca075
PR: sca075/mqtt_vacuum_camera#196
File: custom_components/mqtt_vacuum_camera/config_flow.py:352-355
Timestamp: 2024-11-12T05:06:42.185Z
Learning: Changes in `config_flow.py` now include handling for `floor_only` and `alpha_floor` steps based on the number of rooms, ensuring proper configuration and error handling for unexpected values of `number_of_rooms`.
Learnt from: sca075
PR: sca075/mqtt_vacuum_camera#196
File: custom_components/mqtt_vacuum_camera/config_flow.py:352-355
Timestamp: 2024-11-12T05:06:42.185Z
Learning: Ensure that `number_of_rooms` has a default case to handle unexpected values in the `async_step_init` method of `OptionsFlowHandler` class in `config_flow.py`.
Learnt from: sca075
PR: sca075/mqtt_vacuum_camera#289
File: custom_components/mqtt_vacuum_camera/config_flow.py:871-884
Timestamp: 2024-11-30T23:02:20.628Z
Learning: In `custom_components/mqtt_vacuum_camera/config_flow.py`, avoid adding multiple exception handlers or making the code unnecessarily long; the user prefers concise and clean code.
Learnt from: sca075
PR: sca075/mqtt_vacuum_camera#196
File: custom_components/mqtt_vacuum_camera/translations/en.json:132-140
Timestamp: 2024-11-12T05:06:42.185Z
Learning: Changes in `config_flow.py` related to adding steps for `floor_only` and `alpha_floor` configurations resolve the issue with the `floor_only` section in the translation files.
🪛 LanguageTool
docs/obstacles_detection.md

[style] ~13-~13: Consider a shorter alternative to avoid wordiness.
Context: ...lovelace-xiaomi-vacuum-map-card). - In order to select the obstacle that is highlight i...

(IN_ORDER_TO_PREMIUM)


[uncategorized] ~13-~13: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...In order to select the obstacle that is highlight in a red dot drawn on the map, it is ne...

(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)


[uncategorized] ~41-~41: Loose punctuation mark.
Context: ...dle the request. - coordinates: The map coordinates of the clicked poin...

(UNLIKELY_OPENING_PUNCTUATION)


[uncategorized] ~47-~47: A punctuation mark might be missing here.
Context: ...earest obstacle to the given coordinates it isn't necessary to point directly on it...

(AI_EN_LECTOR_MISSING_PUNCTUATION)


[uncategorized] ~47-~47: The preposition “at” seems more likely in this position.
Context: ...es it isn't necessary to point directly on it. - 3. **Image Download and Proc...

(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)


[grammar] ~65-~65: “Vacuum” is a singular noun. It appears that the verb form is incorrect.
Context: ...tes ### Supported Vacuums If the vacuum do not support the `ObstacleImagesCapabili...

(PCT_SINGULAR_NOUN_PLURAL_VERB_AGREEMENT)


[uncategorized] ~65-~65: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...h a Red Dot on the map, when the Vacuum support Obstacle Detections and has no oboard c...

(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)

🪛 Markdownlint (0.35.0)
docs/obstacles_detection.md

8-8: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


9-9: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


12-12: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


13-13: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


44-44: null
Bare URL used

(MD034, no-bare-urls)

🔇 Additional comments (16)
custom_components/mqtt_vacuum_camera/utils/camera/camera_services.py (2)

Line range hint 1-14: Sweet imports setup! 🎯

The imports are nicely organized and the new async_clean_up_all_auto_crop_files import fits perfectly with the cleanup functionality.


Line range hint 17-24: Solid reset_trims implementation! 🚀

The function handles errors like a pro and the new auto crop files cleanup is a nice touch. Clean and effective implementation.

custom_components/mqtt_vacuum_camera/__init__.py (1)

30-34: LGTM! Clean import organization

The imports are well-organized and grouped logically.

custom_components/mqtt_vacuum_camera/snapshots/log_files.py (1)

Line range hint 89-93: Verify ZIP file creation security

The ZIP file creation could be vulnerable to ZIP bombs. Consider adding size limits and validation.

custom_components/mqtt_vacuum_camera/common.py (1)

201-204: LGTM! Clean signature formatting.

The multi-line parameter formatting improves readability while maintaining the same functionality.

custom_components/mqtt_vacuum_camera/valetudo/rand256/reimg_draw.py (1)

65-69: LGTM! Clean tuple unpacking.

The multi-line tuple unpacking improves readability and follows Python best practices.

custom_components/mqtt_vacuum_camera/utils/files_operations.py (1)

314-316: LGTM! Clean string formatting.

The multi-line string formatting improves readability while maintaining the same functionality.

custom_components/mqtt_vacuum_camera/valetudo/hypfer/image_handler.py (3)

97-102: Clean formatting for room coordinates unpacking! 🎯

The multi-line tuple unpacking improves readability while maintaining functionality.


162-166: Neat unpacking of robot position data! 🤖

The multi-line formatting enhances code clarity while preserving the logic.


292-295: Sweet tuple unpacking for crop dimensions! 📏

The multi-line formatting maintains consistency with other similar changes.

custom_components/mqtt_vacuum_camera/valetudo/rand256/image_handler.py (3)

82-86: Slick formatting for segment data unpacking! 🎨

The multi-line tuple unpacking improves readability while maintaining the same functionality.


176-180: Clean robot position unpacking! 🤖

The multi-line formatting enhances code clarity and maintains consistency with other files.


276-280: Nice crop dimensions unpacking! 📐

The multi-line formatting follows the established pattern, improving overall code readability.

custom_components/mqtt_vacuum_camera/utils/drawable.py (1)

25-28: LGTM! Well-defined error color constants

The error color constants are clearly defined with appropriate opacity levels for different use cases.

custom_components/mqtt_vacuum_camera/camera.py (1)

128-130: LGTM! Clean formatting of async_listen call

The multi-line formatting improves readability.

custom_components/mqtt_vacuum_camera/config_flow.py (1)

Line range hint 92-157: LGTM! Solid configuration flow implementation

The configuration flow implementation is well-structured with proper error handling, validation of the vacuum entity and MQTT topic, and appropriate storage path creation.

Comment on lines +165 to +167
def from_device_ids_to_entity_ids(
device_ids: str, hass: HomeAssistant, domain: str = "vacuum"
) -> str:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix early return in device ID processing loop.

The current implementation only processes the first device ID and ignores the rest. This could lead to missing entities when multiple device IDs are provided.

Here's the fix:

 def from_device_ids_to_entity_ids(
     device_ids: str, hass: HomeAssistant, domain: str = "vacuum"
 ) -> str:
     dev_reg = dr.async_get(hass)
     entity_reg = er.async_get(hass)
     resolved_entity_ids = []
     
     for device_id in device_ids:
         device = dev_reg.async_get(device_id)
         if device:
             for entry in entity_reg.entities.values():
                 if entry.device_id == device_id and entry.domain == domain:
                     resolved_entity_ids.append(entry.entity_id)
-            return resolved_entity_ids
+    return resolved_entity_ids

Committable suggestion skipped: line range outside the PR's diff.

@@ -332,7 +332,7 @@ def get_rrm_goto_target(json_data: JsonType) -> list or None:
except KeyError:
return None
else:
if path_data is not []:
if path_data != []:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fixed list comparison operator! 🔧

Changed from is not [] to != [] which is the correct way to check if a list is not empty. The is operator checks for identity, while != checks for inequality, which is what we want here.

Comment on lines +577 to +581
self.camera_image(self._image_w, self._image_h)
_LOGGER.warning(
f"{self._file_name}: No JSON data available. Camera Suspended."
)
self._should_pull = False
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix typo in should_poll assignment

There's a typo in the variable name that could cause issues.

-            self._should_pull = False
+            self._should_poll = False
📝 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
self.camera_image(self._image_w, self._image_h)
_LOGGER.warning(
f"{self._file_name}: No JSON data available. Camera Suspended."
)
self._should_pull = False
self.camera_image(self._image_w, self._image_h)
_LOGGER.warning(
f"{self._file_name}: No JSON data available. Camera Suspended."
)
self._should_poll = False

Comment on lines +374 to +381
_LOGGER.debug(
f"Image downloaded successfully: {obstacle_file}"
)
return obstacle_file
else:
_LOGGER.warning(f"Failed to download image: {response.status}")
_LOGGER.warning(
f"Failed to download image: {response.status}"
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add timeout handling for image downloads

The download operation should have a timeout to prevent hanging.

             try:
-                async with aiohttp.ClientSession() as session:
+                timeout = aiohttp.ClientTimeout(total=30)  # 30 seconds timeout
+                async with aiohttp.ClientSession(timeout=timeout) as session:
                     async with session.get(url) as response:
                         if response.status == 200:

Committable suggestion skipped: line range outside the PR's diff.

@sca075 sca075 merged commit f79f589 into main Dec 3, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant