-
-
Notifications
You must be signed in to change notification settings - Fork 7.4k
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
Add field temperature_unit
in climates set_temperature
action call
#36625
Add field temperature_unit
in climates set_temperature
action call
#36625
Conversation
✅ Deploy Preview for home-assistant-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
📝 WalkthroughWalkthroughThe pull request updates the Home Assistant climate integration documentation to introduce a new optional Changes
Sequence DiagramsequenceDiagram
participant User
participant HomeAssistant
participant ClimateDevice
User->>HomeAssistant: climate.set_temperature
Note over User: Specify temperature (77°F)
Note over User: Optional temperature_unit: 'fahrenheit'
HomeAssistant->>ClimateDevice: Convert temperature if needed
Note over HomeAssistant: Converts 77°F to 25°C
ClimateDevice->>HomeAssistant: Apply temperature setting
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🪛 LanguageToolsource/_integrations/climate.markdown[grammar] ~102-~102: Did you mean “these”? (THIS_NNS_VB) 🪛 Markdownlint (0.37.0)source/_integrations/climate.markdown102-102: Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe (MD055, table-pipe-style) 🔇 Additional comments (1)source/_integrations/climate.markdown (1)
The example effectively demonstrates the new 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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
source/_integrations/climate.markdown (1)
102-102
: Enhance parameter documentation clarity and fix formatting.
- Fix grammar and add trailing pipe in the table row:
-| `temperature_unit` | yes | The unit in which one of the above target temperatures should be set. This defaults based the selected unit system of your HA instance. +| `temperature_unit` | yes | The unit in which one of the above target temperatures should be set. This defaults based on the selected unit system of your HA instance. |
- Consider enhancing the description by specifying the accepted values:
-| `temperature_unit` | yes | The unit in which one of the above target temperatures should be set. This defaults based on the selected unit system of your HA instance. | +| `temperature_unit` | yes | The unit in which one of the above target temperatures should be set (`°C` or `°F`). This defaults based on the selected unit system of your HA instance. |🧰 Tools
🪛 LanguageTool
[grammar] ~102-~102: Did you mean “these”?
Context: ...bove target temperatures should be set. This defaults based the selected unit system...(THIS_NNS_VB)
🪛 Markdownlint (0.37.0)
102-102: Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
Table pipe style(MD055, table-pipe-style)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
source/_integrations/climate.markdown
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
source/_integrations/climate.markdown
[grammar] ~102-~102: Did you mean “these”?
Context: ...bove target temperatures should be set. This defaults based the selected unit system...
(THIS_NNS_VB)
🪛 Markdownlint (0.37.0)
source/_integrations/climate.markdown
102-102: Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
Table pipe style
(MD055, table-pipe-style)
🔇 Additional comments (2)
source/_integrations/climate.markdown (2)
138-152
: LGTM! Clear and helpful example.
The example effectively demonstrates:
- Usage of the new
temperature_unit
parameter - Temperature unit conversion behavior
- Proper YAML formatting
Line range hint 102-152
: Verify alignment with core implementation.
Since this documentation change is linked to core PR #134316, let's verify the implementation details.
✅ Verification successful
Documentation matches core implementation for temperature_unit parameter
The documentation accurately reflects the core implementation changes from PR #134316:
- The parameter name
temperature_unit
matches exactly with the core constantATTR_TEMPERATURE_UNIT
- The implementation supports both °C and °F units as shown in the selector options
- The behavior aligns with the documentation where it's used to specify the unit for target temperature values
- The test cases demonstrate the same usage pattern as shown in the documentation examples
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify parameter name and usage in core implementation
# Check the climate integration file in core PR for the parameter name
gh api repos/home-assistant/core/pulls/134316/files | \
jq -r '.[] | select(.filename | contains("climate")) | .patch' | \
grep -A 5 "temperature_unit"
Length of output: 1439
bca4d8c
to
edc1a9a
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
source/_integrations/climate.markdown (1)
102-102
: Enhance parameter documentation and fix table formatting.
- Add trailing pipe to maintain consistent table formatting
- Consider listing supported temperature units explicitly
-| `temperature_unit` | yes | The unit in which the above target temperatures should be set. This defaults based on the selected unit system of your HA instance. +| `temperature_unit` | yes | The unit in which the above target temperatures should be set. This defaults based on the selected unit system of your HA instance. Supported values: `°F`, `°C`. |🧰 Tools
🪛 LanguageTool
[grammar] ~102-~102: Did you mean “these”?
Context: ...bove target temperatures should be set. This defaults based on the selected unit sys...(THIS_NNS_VB)
🪛 Markdownlint (0.37.0)
102-102: Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
Table pipe style(MD055, table-pipe-style)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
source/_integrations/climate.markdown
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
source/_integrations/climate.markdown
[grammar] ~102-~102: Did you mean “these”?
Context: ...bove target temperatures should be set. This defaults based on the selected unit sys...
(THIS_NNS_VB)
🪛 Markdownlint (0.37.0)
source/_integrations/climate.markdown
102-102: Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
Table pipe style
(MD055, table-pipe-style)
🔇 Additional comments (2)
source/_integrations/climate.markdown (2)
138-152
: LGTM! Clear and helpful example.
The example effectively demonstrates temperature unit conversion while maintaining consistency with other examples in the document.
Line range hint 102-152
: Verify documentation matches core implementation.
Since this documentation change is linked to core PR #134316, let's verify the implementation details match.
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.
✅ Approved. Can be merged as soon as the parent PR gets merged.
Going to close this, since the architectural discussion needs to be done first and most properly it will result in a different, more generic solution |
Proposed change
SSIA
Type of change
current
branch).current
branch).next
branch).next
branch).Additional information
Checklist
current
branch.next
branch.Summary by CodeRabbit
temperature_unit
parameter to climate temperature setting.