-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Update time and temperature cookbook #4783
base: current
Are you sure you want to change the base?
Update time and temperature cookbook #4783
Conversation
Use a temperature that comes with the default Home Assistant setup - weather Show how to use google fonts so a user doesn't have to setup font files to get something running.
I'm a new ESPHome user and definitely really love the cookbooks, but I wanted to get something up and running as fast as possible to see what I could do, and so I thought a few changes to this cookbook would make it simpler for a new user who may not have a temperature sensor set up, but could get something showing up immediately. |
WalkthroughThe update modifies the sensor configuration and font definitions in the documentation. It changes the temperature sensor setup to import a single weather forecast (providing both temperature and unit details) instead of two separate sensors, and adds a new text sensor for the temperature unit. Additionally, generic font identifiers have been updated to descriptive names, and new Google font entries have been introduced with specified file paths and sizes. The display output text has also been updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant HA as Home Assistant
participant Sensor as Sensor Module
participant Display as OLED Display Logic
HA->>Sensor: Provide weather.forecast_home data
Sensor->>Display: Relay temperature and unit data
Display->>Display: Format data using updated fonts ("medium" for inside, descriptive font for outside)
Display->>User: Render "Time and Temperature" output
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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)
cookbook/display_time_temp_oled.rst (1)
98-113
: Add Alternative Google Fonts Configuration
A new code block introduces font definitions using thegfonts://
scheme, offering an alternative to local TrueType fonts. It is recommended to clarify within the documentation that this block is optional and should not be active simultaneously with the local font definitions to avoid potential conflicts inid
usage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cookbook/display_time_temp_oled.rst
(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**`: - Do not generate or add any sequence diagrams
**
: - Do not generate or add any sequence diagrams
cookbook/display_time_temp_oled.rst
🔇 Additional comments (7)
cookbook/display_time_temp_oled.rst (7)
48-48
: Clarify Temperature Sensor Introduction
The updated sentence now indicates that a single temperature sensor along with the complete weather forecast data will be imported from Home Assistant. This change clearly sets the stage for the configuration that follows.
64-67
: Update Outside Temperature Sensor Configuration
The sensor configuration now usesentity_id: weather.forecast_home
with the specifiedattribute: temperature
and marks it as internal. This change streamlines the setup by relying on the weather forecast entity instead of a separate sensor. Ensure that the Home Assistant entityweather.forecast_home
reliably provides thetemperature
attribute.
68-72
: Introduce Text Sensor for Temperature Unit
A new text sensor has been added to retrieve the temperature unit via the attributetemperature_unit
from the same weather entity. This clearly separates the temperature value from its unit, enhancing readability and display precision. Please verify that Home Assistant supports this attribute for the given entity.
87-97
: Adopt Descriptive Font Identifiers
The font definitions have been updated from generic identifiers (font1
,font2
,font3
) to descriptive names (small
,medium
,large
). This improves clarity in the documentation and ensures that references in the lambda code are self-explanatory.
138-142
: Update Display Header Text in Lambda
The lambda section now prints "Time and" and "Temperature" using the updatedsmall
andlarge
fonts respectively. This change aligns the display header with the revised sensor configuration and font naming conventions.
146-151
: Refine Sensor Value Display in Lambda
The lambda now prints the inside temperature using themedium
font and displays the outside temperature concatenated with its unit by converting the text sensor state with.c_str()
. This integration ensures consistency in presentation. Please ensure that the temperature unit value is formatted safely and is non-empty under all conditions.
164-164
: Provide Alternative Alarm State Example
The additional example replacing the top display with the Home Assistant alarm state adds valuable flexibility for users. The updated explanatory text is clear, but consider highlighting that this configuration is an alternative to the time and temperature display to avoid confusion.
✅ Deploy Preview for esphome ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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
🔭 Outside diff range comments (1)
cookbook/display_time_temp_oled.rst (1)
174-182
: 💡 Verification agent🧩 Analysis chain
Inconsistent Font Identifier in Alarm Display
The lambda for displaying the alarm state still referencesid(font1)
even though the font identifiers have been updated tosmall
,medium
, andlarge
. This inconsistency might lead to runtime issues or unintended styling. Consider updating the reference (for example, replacingid(font1)
withid(small)
) to ensure consistency across the documentation.Suggested diff:
- it.printf(64, 0, id(font1), TextAlign::TOP_CENTER, "Alarm State: %s", id(alarm_state).state.c_str()); + it.printf(64, 0, id(small), TextAlign::TOP_CENTER, "Alarm State: %s", id(alarm_state).state.c_str());
Critical Update Required: Resolve Inconsistent Font Identifier
The lambda for displaying the alarm state in
cookbook/display_time_temp_oled.rst
(lines 174-182) is still referencingid(font1)
instead of one of the updated font identifiers (small
,medium
, orlarge
). This discrepancy could lead to runtime issues or inconsistent display styling.
- Action Required: Replace
id(font1)
with the appropriate updated identifier (e.g.,id(small)
).Suggested diff:
- it.printf(64, 0, id(font1), TextAlign::TOP_CENTER, "Alarm State: %s", id(alarm_state).state.c_str()); + it.printf(64, 0, id(small), TextAlign::TOP_CENTER, "Alarm State: %s", id(alarm_state).state.c_str());
🏁 Script executed:
#!/bin/bash # Check for the outdated font reference "id(font1)" in the cookbook file. rg "id\(font1\)" cookbook/display_time_temp_oled.rst # Check for updated font identifier usages to validate the expected naming. rg "id\(small\)" -n . rg "id\(medium\)" -n . rg "id\(large\)" -n .Length of output: 1074
Action Required: Update Inconsistent Font Identifier in OLED Alarm Display
The lambda block in
cookbook/display_time_temp_oled.rst
(lines 174-182) still usesid(font1)
, which is inconsistent with the updated font identifiers (small
,medium
,large
). The search results confirm thatid(small)
is used elsewhere in the file, so updating this reference will help avoid potential runtime issues or styling errors.
- File to update:
cookbook/display_time_temp_oled.rst
(lines 174-182)- Change needed: Replace
id(font1)
withid(small)
(or another appropriate updated font identifier)Suggested diff:
- it.printf(64, 0, id(font1), TextAlign::TOP_CENTER, "Alarm State: %s", id(alarm_state).state.c_str()); + it.printf(64, 0, id(small), TextAlign::TOP_CENTER, "Alarm State: %s", id(alarm_state).state.c_str());
🧹 Nitpick comments (3)
cookbook/display_time_temp_oled.rst (3)
98-113
: Enhance Google Fonts Section
The optional Google Fonts section adds value by offering an alternative to including local font files. Ensure that users are aware of any differences in rendering between these fonts and the locally provided ones, especially given the differences in font sizes.
129-130
: YAML Comment Style Suggestion
The comments in the i2c configuration use//
, which is not standard for YAML (typically using#
). If these are meant to be active comments, consider updating them to the YAML comment style to avoid any parsing issues.
149-152
: Outside Temperature Display with Unit
The lambda prints the outside temperature along with its unit using themedium
font. This approach neatly combines two data points. It may be worth considering a fallback or simple error handling in case theoutside_temperature_unit
state is empty or invalid.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cookbook/display_time_temp_oled.rst
(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**`: - Do not generate or add any sequence diagrams
**
: - Do not generate or add any sequence diagrams
cookbook/display_time_temp_oled.rst
🔇 Additional comments (9)
cookbook/display_time_temp_oled.rst (9)
48-48
: Clarify Temperature Sensor Configuration
The update now explains that a single temperature sensor is obtained from Home Assistant’s weather forecast. For extra clarity, you might consider explicitly noting that both the temperature value and its unit are derived from the sameweather.forecast_home
entity.
62-67
: Update Outside Temperature Sensor
The sensor configuration now correctly references the Home Assistant weather forecast by specifyingentity_id: weather.forecast_home
along with theattribute: temperature
. This aligns well with the PR objectives.
68-73
: Add Temperature Unit Text Sensor
Introducing a text sensor to extract the temperature unit fromweather.forecast_home
is a smart move. Please verify that the chosen weather entity indeed provides thetemperature_unit
attribute, so the display can render the correct unit.
85-97
: Update Font Identifiers for Clarity
The font configuration block has been updated to use descriptive identifiers (small
,medium
,large
) instead of generic names. This improves readability and consistency. The associated file names and sizes appear intentional.
138-139
: Updated Display Text for Time and Temperature
The lambda now prints "Time and" and "Temperature" using the updatedsmall
font identifier. This change clearly reflects the intended updates. Just verify that the text alignment and font size work optimally on the OLED display.
142-142
: Time Formatting Check
Time is rendered using%H:%M
with thelarge
font, which is appropriate. Confirm that this format and font size meet the layout requirements for the display.
145-147
: Inside Temperature Display
The inside temperature is displayed with themedium
font, ensuring legible output. Double-check that the numerical formatting ("%.1f"
) is consistent with the sensor’s data type and expected display.
164-164
: Clear Explanation for Alarm Example
The description introducing the alarm state example is clear and concise. It successfully communicates the transition from the default time and temperature display to an alternative alarm status display.
168-173
: Alarm State Text Sensor Configuration
The alarm state text sensor is configured correctly and should integrate seamlessly with the subsequent display configuration.
fc00254
to
0376336
Compare
Description:
Use a temperature that comes with the default Home Assistant setup - weather
Show how to use google fonts so a user doesn't have to setup font files to get something running.
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
/components/index.rst
when creating new documents for new components or cookbook.