-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[Midea] Update follow me action with Fahrenheit support #4437
base: next
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for esphome ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
WalkthroughThe changes introduced in this pull request primarily involve updates to documentation files and configuration settings across various components in the ESPHome project. Key modifications include updating version numbers in configuration files, enhancing documentation clarity, adding new features, and modifying existing functionalities for several components. Notable updates include new sensor documentation, improved error handling in HTTP requests, and expanded configuration options for various components. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (42)
components/i2c_device.rst (3)
9-16
: Consider documenting thescan
parameter.While the example configuration is clear, it includes a
scan: True
parameter without explaining its purpose. Consider adding a brief explanation about what this parameter does and when users should enable it.i2c: sda: 4 scl: 5 - scan: True + # Enable I²C bus scanning at startup to detect available devices + scan: True
18-25
: Enhance the lambda example with a complete implementation.The current example contains a TODO comment and doesn't explain the purpose of the read/write operations. Consider:
- Completing the example with actual implementation
- Adding comments to explain what each operation does
- Including error handling for the read operation
on...: then: - lambda: !lambda |- + // Write 0x12 to register 0x00 id(i2cdev).write_byte(0x00, 0x12); + // Read from register 0x01 and handle the result if (auto b = id(i2cdev).read_byte(0x01)) { - // TODO + // Process the read value + ESP_LOGI("i2c_device", "Read value: 0x%02X", *b); + } else { + ESP_LOGE("i2c_device", "Failed to read from device"); }
27-30
: Consider documenting additional configuration options.The configuration section only documents the required
address
parameter. Consider adding:
- Common optional parameters (if any)
- The valid range for the address parameter
- Examples of typical addresses for common I²C devices
Configuration variables: ------------------------ - **address** (*Required*, int): I²C address of the device. + Valid addresses are between 0x00 and 0x7F. + Common addresses include 0x20-0x27 for PCF8574, 0x38-0x3F for PCF8574A, etc. + +- **id** (*Optional*, :ref:`config-id`): Manually specify the ID used for code generation. +- **update_interval** (*Optional*, :ref:`config-time`): The interval to check the device. Defaults to 60s.components/touchscreen/axs15231.rst (3)
4-8
: Clarify the hardware relationship in SEO metadataThe image reference
esp32_s3_box_3.png
suggests this touchscreen is used in ESP32-S3-BOX-3. Consider adding this information to the description and keywords if this is a primary use case... seo:: - :description: Instructions for setting up AXS15231 touch screen controller with ESPHome + :description: Instructions for setting up AXS15231 touch screen controller with ESPHome, commonly used in ESP32-S3-BOX-3 :image: esp32_s3_box_3.png - :keywords: AXS15231 + :keywords: AXS15231, ESP32-S3-BOX-3, touchscreen
9-10
: Add technical specifications and requirementsThe description should include key technical details such as:
- I²C address
- Number of supported touch points
- Resolution capabilities
- Power requirements
33-37
: Expand the See Also sectionConsider adding links to:
- Other supported touchscreen components for comparison
- Troubleshooting guide for common issues
- Example projects using this touchscreen
Makefile (1)
2-2
: Consider adding a comment explaining beta version usageSince you're using a beta version (2024.11.0b1), it would be helpful to add a comment explaining why this specific beta version is required for the Fahrenheit support documentation.
-ESPHOME_REF = 2024.11.0b1 +# Using beta version that includes Fahrenheit support changes (esphome/esphome#7762) +ESPHOME_REF = 2024.11.0b1components/touchscreen/xpt2046.rst (2)
35-36
: Add explanation for the transform section.The purpose and effect of
mirror_x: true
should be documented to help users understand when to use this setting.Consider adding a brief comment above the transform section:
threshold: 400 + # Mirror the X coordinates if your touch input is reversed horizontally transform: mirror_x: true
38-41
: Add context for calibration values.The example shows specific calibration values without explaining that these are example values and users need to determine their own values through calibration.
Consider adding a comment:
+ # These are example values - you need to calibrate your specific device calibration: x_min: 280 x_max: 3860 y_min: 340 y_max: 3860
components/speaker/index.rst (3)
16-20
: Enhance the YAML example with a concrete platform implementation.The current example only shows a placeholder. Consider expanding it with a real platform example to make it more helpful for users.
speaker: - - platform: ... + - platform: i2s_audio # or another actual platform + dac_type: external + i2s_dout_pin: GPIO25
76-76
: Fix grammar in emphasized text.The emphasis on "is" seems unnecessary and grammatically awkward.
-This action will stop playing audio data from the speaker after all data **is** played. +This action will stop playing audio data from the speaker after all data is played.
111-124
: Add language specification to the code block.The code block should specify yaml as the language for proper syntax highlighting.
-.. code-block:: +.. code-block:: yamlcomponents/sensor/max17043.rst (4)
36-47
: Enhance the configuration exampleConsider enhancing the example configuration with:
- Default values for optional parameters
- Update interval configuration
- Unit of measurement for voltage and percentage
# Example configuration entry sensor: - platform: max17043 id: max17043_id i2c_id: i2c_max17043 + # Default update interval: 60s + update_interval: 60s battery_voltage: name: "Battery Voltage" + unit_of_measurement: "V" battery_level: name: "Battery" + unit_of_measurement: "%"
51-56
: Add value ranges and accuracy specificationsConsider enhancing the sensor documentation with:
- Typical value ranges (e.g., 2.5V to 4.2V for battery voltage, 0-100% for battery level)
- Resolution and accuracy specifications from the datasheet
86-86
: Specify a concrete trigger exampleReplace the placeholder
on_....:
with a concrete example trigger (e.g.,on_time:
).- on_...: + on_time: + - seconds: 0 + minutes: /30
101-106
: Enhance the See Also sectionConsider adding more relevant links such as:
- Power management components
- Battery monitoring components
- ADC sensor documentation for comparison
components/display/qspi_dbi.rst (2)
Line range hint
59-86
: Improve clarity of configuration parametersTwo suggestions to enhance the documentation:
- The
init_sequence
parameter description is ambiguous. It's marked as optional but described as required for the CUSTOM model. Consider rephrasing to:-- **init_sequence** (*Optional*, A list of byte arrays): Specifies the init sequence for the display. This is required when using the ``CUSTOM`` model - but may be empty. If specified for other models this data will be sent after the pre-configured sequence. ++ **init_sequence** (*Optional*, A list of byte arrays): Specifies the init sequence for the display. Required and must not be empty when using the ``CUSTOM`` model. For other models, if specified, this data will be sent after the pre-configured sequence.
- The
draw_from_origin
parameter would benefit from more context about its performance implications and use cases.
181-227
: Consider enhancing the JC4832W535 example configurationA few suggestions to improve the example:
The
init_sequence
parameter is included but empty. Either:
- Remove it if not needed
- Add the actual sequence if required
- Add a comment explaining why it's empty
Consider adding a note about the pin configuration, particularly since
ignore_strapping_warning: true
is used for the CS pin.The rotation note could be expanded to explain why landscape mode is recommended for this specific display.
components/sensor/mopeka_pro_check.rst (2)
60-65
: Consider adding explanatory comments to the example configuration.While the new configuration options are correctly shown, adding brief inline comments would help users understand their purpose immediately in the example.
signal_quality: - name: "Propane test read quality" + name: "Propane test read quality" # Reports sensor reading confidence level ignored_reads: - name: "Propane test ignored reads" + name: "Propane test ignored reads" # Tracks consecutive readings below quality threshold # Report sensor distance/level data for all equal or greater than -minimum_signal_quality: "LOW" +minimum_signal_quality: "LOW" # Accepts: HIGH, MEDIUM, LOW, ZERO
129-141
: Add default value information for minimum_signal_quality.While the documentation comprehensively explains the quality threshold feature and its values, it should explicitly state the default value. Line 138 implies "MEDIUM" is the default, but this should be clearly stated.
Acceptable Values: - - "HIGH": High Quality - - "MEDIUM": Medium Quality (default value) - - "LOW": Low Quality - - "ZERO": Zero Quality + - "HIGH": High Quality + - "MEDIUM": Medium Quality + - "LOW": Low Quality + - "ZERO": Zero Quality + +Default value: "MEDIUM"guides/getting_started_hassio.rst (1)
25-25
: Consider clarifying the alternative installation reference.The sentence "take a look at the other installation methods here" appears to be missing a proper link or reference to where users can find these alternative methods.
Consider updating to include a specific link:
-If you run a Home Assistant installation that does not have access to add-ons, take a look at the other installation methods here. +If you run a Home Assistant installation that does not have access to add-ons, take a look at the other installation methods in :doc:`getting_started_command_line`.components/ethernet.rst (2)
99-101
: Consider adding example configuration with polling_intervalThe documentation for the new
polling_interval
configuration is clear and well-structured. To make it even more helpful, consider adding an example configuration that demonstrates its usage.Example addition:
ethernet: type: W5500 clk_pin: GPIOXX mosi_pin: GPIOXX miso_pin: GPIOXX cs_pin: GPIOXX + # Example using polling instead of interrupt + polling_interval: 15ms reset_pin: GPIOXX
103-116
: Consider restructuring framework compatibility sectionThe framework compatibility information is comprehensive and valuable. To improve readability, consider:
- Moving this section to a dedicated "Framework Compatibility" subsection
- Using a table format for the framework versions
Example restructuring:
Framework Compatibility ^^^^^^^^^^^^^^^^^^^^^ The following table shows framework support for SPI polling mode (no interrupt pin): .. list-table:: :header-rows: 1 * - Framework - Minimum Version - Notes * - ESP-IDF - 5.3 - Full support * - ESP-IDF 5.2.x - 5.2.1 - Full support * - ESP-IDF 5.1.x - 5.1.4 - Full support * - Arduino-ESP32 - 3.0.0 - Not supported in PlatformIO Configuration Rules: * When using supported frameworks: * Either ``interrupt_pin`` or ``polling_interval`` can be set * Setting both will result in an error * When using unsupported frameworks: * ``interrupt_pin`` is required * ``polling_interval`` cannot be usedcomponents/climate/midea.rst (1)
164-165
: Clarify the temperature handling when fahrenheit is true.The documentation could be more precise about the temperature handling. Consider revising the description to be more explicit:
-- **fahrenheit** (*Optional*, boolean, :ref:`templatable <config-templatable>`): Specifies if the temperature configuration variable value is in Fahrenheit. When set to True, the temperature is parsed and sent in Fahrenheit. +- **fahrenheit** (*Optional*, boolean, :ref:`templatable <config-templatable>`): Specifies if the temperature configuration variable value is in Fahrenheit. When set to True, the provided temperature value is treated as Fahrenheit and automatically converted to Celsius for internal use.components/http_request.rst (4)
170-174
: LGTM: Important note about response validationThe note effectively explains the importance of checking status codes. Consider enhancing it with a specific code example showing proper error handling for different status codes.
Add a brief code example like:
- if: condition: lambda: return response->status_code == 200; then: - logger.log: "Success: Processing response body" else: - logger.log: format: "Error: Server returned %d" args: [response->status_code]
190-193
: Add more context about error conditionsWhile the example is clear, it would be helpful to add a comment explaining what conditions trigger the
on_error
handler (e.g., network failures, DNS resolution failures, etc.).
195-203
: LGTM: Clear distinction between errors and status codesThe documentation clearly explains when the
on_error
trigger fires and its limitations. Consider adding a cross-reference to the status code handling section to help users implement comprehensive error handling.Add a cross-reference like:
See the note about :ref:`status code handling <http_request-on_response>` for handling server-side errors.
Line range hint
269-303
: LGTM: Comprehensive JSON handling exampleThe example demonstrates best practices:
- Status code validation before processing
- JSON key existence check
- Meaningful error logging
Consider adding a note about potential JSON parsing errors and how to handle them.
Add a note about JSON parsing errors:
.. note:: The ``json::parse_json`` function may fail if the response body is not valid JSON. Consider adding error handling: .. code-block:: yaml - lambda: |- if (!json::parse_json(body, [](JsonObject root) { // ... handling code ... })) { ESP_LOGW(TAG, "Failed to parse JSON"); }changelog/2024.11.0.rst (2)
10-19
: Remove extra blank line after the component table.There's an unnecessary double blank line after the component table. In RST, a single blank line is sufficient for separation between sections.
TC74, components/sensor/tc74, tc74.jpg -
26-26
: Fix typo in the introduction text.There's a spelling error in the word "isntall".
-Because there are now more devices you can buy and do not have to isntall ESPHome onto yourself, we have made some +Because there are now more devices you can buy and do not have to install ESPHome onto yourself, we have made somecomponents/opentherm.rst (1)
1-8
: Add version information and compatibility notesPlease enhance the documentation header with:
- Minimum ESPHome version requirement
- Changelog entry reference
- Migration guide for users upgrading from previous versions
components/mqtt.rst (1)
767-767
: Consider expanding the dynamic broker example.The note about dynamic broker negotiation could benefit from additional context about when this scenario typically occurs.
Consider adding a brief explanation like:
"For example, in environments where the broker address is obtained through DHCP, network discovery, or external configuration systems."components/modbus_controller.rst (3)
67-68
: Documentation formatting needs improvementThe anchor tag for
offline_skip_updates
should be on its own line for better readability and maintainability.-.. _modbus_controller-offline_skip_updates: - +.. _modbus_controller-offline_skip_updates:
757-773
: Improve documentation structure and formattingThe documentation for the
on_online
automation trigger has inconsistent spacing and formatting. The section header markers should be consistent in length with the title.-.. _modbus_controller-on_online: - -``on_online`` -******************* +.. _modbus_controller-on_online: + +``on_online`` +************
774-790
: Enhance documentation clarity for offline triggerThe documentation for the
on_offline
automation trigger should include more details about when exactly the controller is considered offline and reference the configuration that controls this behavior.Add more context about the offline state:
This automation will be triggered when a `modbus_controller` goes ``offline`` (See :ref:`offline_skip_updates <modbus_controller-offline_skip_updates>`). In :ref:`Lambdas <config-lambda>` you can get the function code in ``function_code`` and the register address in ``address``. + +A controller is considered offline when it fails to respond to a command. The number of updates to skip while offline +can be configured using the ``offline_skip_updates`` option. This helps reduce bus congestion when multiple slaves +are present and some are not responding.components/light/index.rst (5)
8-11
: Enhance Home Assistant integration details.Consider adding a note about specific Home Assistant features supported (e.g., brightness control, color support, effects) to provide more comprehensive guidance.
53-57
: Clarify initial_state and restore_mode interaction.The documentation should explicitly state the precedence between
initial_state
andrestore_mode
to avoid confusion. Consider adding an example showing how these settings interact.
105-123
: Add visual examples for color modes.Consider adding diagrams or example configurations showing the practical differences between color modes, especially for complex modes like RGB_COLD_WARM_WHITE.
467-471
: Add Home Assistant UI screenshots.Consider adding screenshots showing how effects appear in the Home Assistant frontend to improve user understanding.
Line range hint
1011-1138
: Add network requirements section.Consider adding a dedicated section about network requirements, including:
- Required ports and protocols
- Firewall considerations
- Network bandwidth requirements
- Multicast configuration needs
components/lvgl/index.rst (2)
170-190
: Fix heading style for consistency.The new section about multiple LVGL configurations is well-documented with clear examples. However, the heading style uses asterisks which is inconsistent with other similar headings in the file.
Apply this diff to fix the heading style:
-Multiple LVGL configurations -**************************** +Multiple LVGL configurations +---------------------------
863-876
: Fix spacing between trigger sections.The new trigger documentation is clear and well-structured, but there are inconsistent line spacings between sections. Remove extra blank lines for consistency with other trigger documentation.
Apply this diff to fix the spacing:
-.. _lvgl_on_pause_trigger: - - -``on_pause`` -************ - +.. _lvgl_on_pause_trigger: + +``on_pause`` +************ + +This :ref:`trigger <lvgl-automation-triggers>` is triggered when LVGL is paused... - -This :ref:`trigger <lvgl-automation-triggers>` is triggered when LVGL is paused... -.. _lvgl_on_resume_trigger: - - -``on_resume`` -************* - +.. _lvgl_on_resume_trigger: + +``on_resume`` +************* + +This :ref:`trigger <lvgl-automation-triggers>` is triggered when LVGL is resumed... - -This :ref:`trigger <lvgl-automation-triggers>` is triggered when LVGL is resumed...
🛑 Comments failed to post (4)
components/touchscreen/axs15231.rst (1)
16-22: 🛠️ Refactor suggestion
Enhance the configuration example
The current example is minimal. Consider adding a more comprehensive example that demonstrates:
- Interrupt pin configuration
- Reset pin configuration
- Any additional touchscreen options that are commonly used
# Example configuration example +i2c: + sda: GPIO21 + scl: GPIO22 + touchscreen: platform: axs15231 id: my_touchscreen + interrupt_pin: GPIO5 + reset_pin: GPIO4 + # Add other common touchscreen options here📝 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... code-block:: yaml # Example configuration example i2c: sda: GPIO21 scl: GPIO22 touchscreen: platform: axs15231 id: my_touchscreen interrupt_pin: GPIO5 reset_pin: GPIO4 # Add other common touchscreen options here
components/speaker/index.rst (1)
127-127: 🛠️ Refactor suggestion
Enhance volume configuration documentation.
The volume configuration lacks important details about valid ranges and format.
-**volume** (**Required**, percentage): The volume to set the speaker to. +**volume** (**Required**, percentage): The volume to set the speaker to. Valid values are between 0% and 100%, or a float value between 0.0 and 1.0 when using lambda.📝 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.**volume** (**Required**, percentage): The volume to set the speaker to. Valid values are between 0% and 100%, or a float value between 0.0 and 1.0 when using lambda.
components/opentherm.rst (2)
127-130:
⚠️ Potential issueFix YAML indentation
The
t_set
key is misaligned. This could cause YAML parsing issues.output: - platform: opentherm - t_set: + t_set: id: setpointCommittable suggestion skipped: line range outside the PR's diff.
346-424: 🛠️ Refactor suggestion
Enhance PID thermostat example with temperature unit handling
The PID thermostat example could be improved by:
- Adding comments specifying the temperature unit system in use
- Demonstrating how to configure Fahrenheit support
- Including temperature range validation
climate: - platform: pid name: "Central heating" heat_output: t_set default_target_temperature: 20 + # Temperature values are in Celsius. For Fahrenheit, set unit_of_measurement: "°F" + unit_of_measurement: "°C" sensor: ch_room_temperature control_parameters: kp: 0.4 ki: 0.004 + # Add temperature range validation + min_temperature: 10 + max_temperature: 30📝 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.Basic PID thermostat ******************** .. code-block:: yaml # A basic thremostat for a boiler with a single central heating circuit and # domestic hot water. It reports the flame, CH and DHW status, similar to what # you would expect to see on a thermostat and also reports the internal boiler # temperatures and the current modulation level. The temperature is regulated # through a PID Climate controller and the current room temperature is retrieved # from a sensor in Home Asisstant. # This configuration should meet most needs and is the recommended starting # point if you just want a thermostat with an external temperature sensor. opentherm: in_pin: GPIOXX out_pin: GPIOXX dhw_enable: true # Note that when we specify an input in hub config with a static value, it can't be # changed without uploading new firmware. If you want to be able to turn things on or off, # use a switch (see the ch_enable switch below). # Also note that when we define an input as a switch (or use other platform), we don't need # to set it at hub level. output: - platform: opentherm t_set: id: t_set min_value: 20 max_value: 65 zero_means_zero: true sensor: - platform: opentherm rel_mod_level: name: "Boiler Relative modulation level" t_boiler: name: "Boiler water temperature" t_ret: name: "Boiler Return water temperature" - platform: homeassistant id: ch_room_temperature entity_id: sensor.temperature filters: # Push room temperature every second to update PID parameters - heartbeat: 1s binary_sensor: - platform: opentherm ch_active: name: "Boiler Central Heating active" dhw_active: name: "Boiler Domestic Hot Water active" flame_on: name: "Boiler Flame on" fault_indication: name: "Boiler Fault indication" entity_category: diagnostic diagnostic_indication: name: "Boiler Diagnostic event" entity_category: diagnostic switch: - platform: opentherm ch_enable: name: "Boiler Central Heating enabled" restore_mode: RESTORE_DEFAULT_ON climate: - platform: pid name: "Central heating" heat_output: t_set default_target_temperature: 20 # Temperature values are in Celsius. For Fahrenheit, set unit_of_measurement: "°F" unit_of_measurement: "°C" sensor: ch_room_temperature control_parameters: kp: 0.4 ki: 0.004 # Add temperature range validation min_temperature: 10 max_temperature: 30
Description:
Update follow me action documentation to support Fahrenheit optional variable bool
Related issue (if applicable): fixes
Pull request in esphome with YAML changes (if applicable): esphome/esphome#7762
Checklist:
I am merging into
next
because this is new documentation that has a matching pull-request in esphome as linked above.or
I am merging into
current
because this is a fix, change and/or adjustment in the current documentation and is not for a new component or feature.Link added in
/index.rst
when creating new documents for new components or cookbook.