Fix chamber target temperature for K2 Base via Moonraker fallback - #57
Conversation
Summary of ChangesHello @PavelStoyan0v, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Creality Home Assistant integration by improving the accuracy and control of chamber target temperatures, specifically for K2 Base printers. It introduces a robust fallback to Moonraker for missing telemetry data and refines the temperature setting process with optimistic updates and model-specific logic adjustments. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a fix for the chamber target temperature on K2 Base printers by implementing a fallback mechanism to poll Moonraker. It also improves the user experience by adding optimistic updates to the UI for temperature controls. The changes are logical and address the issue. My review includes suggestions to improve performance by caching the printer model detection result, which is currently re-evaluated frequently. I've also pointed out an opportunity to make the error handling in the new Moonraker polling logic more robust and a minor code style issue.
| if (payload.get("targetBoxTemp") == 0) and ModelDetection(self.data).is_k2_base: | ||
| payload.pop("targetBoxTemp") |
There was a problem hiding this comment.
The ModelDetection class is instantiated on every WebSocket message here and again on line 316. This is inefficient as the printer model is static and messages can arrive frequently. Consider detecting the model once and caching the result. For example, you could add a _is_k2_base attribute to the coordinator, initialize it to None, and then compute it once when model information becomes available in self.data.
| except Exception: | ||
| # Moonraker might be disabled or port 7125 blocked; fail silently | ||
| pass |
There was a problem hiding this comment.
The except Exception: block is too broad and will catch and silently pass any exception. This could hide bugs, for example a KeyError if the response format from Moonraker changes, or other unexpected issues. It's better to catch specific exceptions related to HTTP requests, or at a minimum, log the caught exception at a debug level to aid in troubleshooting.
| except Exception: | |
| # Moonraker might be disabled or port 7125 blocked; fail silently | |
| pass | |
| except Exception as e: | |
| # Moonraker might be disabled or port 7125 blocked; fail silently | |
| _LOGGER.debug("Failed to poll Moonraker for extras: %s", e) |
| # Chamber heater only activates when > 40°C (K2 Plus behavior) | ||
| v = 0 if value <= 40 else int(round(value)) | ||
| # Bypass this restriction for the Base K2. | ||
| if ModelDetection(self.coordinator.data).is_k2_base: |
There was a problem hiding this comment.
The ModelDetection class is instantiated every time async_set_native_value is called. While less frequent than in the coordinator's message handler, this is still inefficient as the model is static. Consider using a cached model detection result from the coordinator, as suggested for coordinator.py.
| # Optimistic update | ||
| self.coordinator.data["targetBoxTemp"] = v | ||
| self.coordinator.async_update_listeners() | ||
|
|
No description provided.