-
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
Add useful template examples for both text and numerical sensors #4445
base: current
Are you sure you want to change the base?
Add useful template examples for both text and numerical sensors #4445
Conversation
✅ Deploy Preview for esphome ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for esphome ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
WalkthroughThe changes involve the addition of new template sensors in two documentation files: Changes
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.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
components/sensor/template.rst (1)
82-86
: Consider broadening the section introductionThe current introduction focuses specifically on Bluetooth proxies, which might limit the section's perceived scope. Consider making it more generic to accommodate future template sensor examples.
-Here are some useful sensors for debugging and tracking Bluetooth proxies. +Here are some useful template sensor examples for various debugging and monitoring purposes. The first examples focus on Bluetooth proxy functionality.components/text_sensor/template.rst (3)
92-94
: Consider optimizing update intervalsThe 600s (10 minute) update interval seems frequent for project version and name information that typically only changes during firmware updates. Consider using a longer interval or the
never
option with manual triggers on boot.Also applies to: 101-103, 110-112
93-94
: Document the expected output format and available variablesThe lambda expressions use variables (
ESPHOME_PROJECT_VERSION
,ESPHOME_PROJECT_NAME
) that aren't documented elsewhere. Please add:
- Documentation for these built-in variables
- Example outputs to show the expected format
- For the detailed version, explain the format of
App.get_compilation_time()
Example documentation format:
The following variables are available: - ``ESPHOME_PROJECT_VERSION``: The version of your ESPHome project (e.g., "1.0.0") - ``ESPHOME_PROJECT_NAME``: The name defined in your ESPHome configuration (e.g., "living_room") - ``App.get_compilation_time()``: Returns the firmware compilation timestampAlso applies to: 102-103, 111-112
85-113
: Consider adding error handling examplesThe examples could benefit from demonstrating proper error handling for cases where the project variables might be undefined.
Example with error handling:
lambda: |- if (ESPHOME_PROJECT_VERSION == nullptr) { return { "Unknown" }; } return { ESPHOME_PROJECT_VERSION };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
components/sensor/template.rst
(1 hunks)components/text_sensor/template.rst
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
components/sensor/template.rst (1)
Pattern **
: - Do not generate or add any sequence diagrams
components/text_sensor/template.rst (1)
Pattern **
: - Do not generate or add any sequence diagrams
🔇 Additional comments (3)
components/sensor/template.rst (1)
82-112
: Verify coverage of common template sensor use cases
The new section provides useful examples for Bluetooth proxy monitoring. Let's verify if there are other common template sensor use cases that should be included in this section.
✅ Verification successful
Template sensor examples are well covered across documentation
The new Bluetooth proxy template sensors fit well within the existing documentation structure. After reviewing the search results:
- The main template sensor documentation (components/sensor/template.rst) already covers the core concepts and basic usage
- Specific use cases are appropriately distributed across various component docs where they are most relevant (like BME680, ATM90E32)
- The new Bluetooth proxy examples follow the established pattern of documenting domain-specific template sensors within their relevant sections
The current addition of Bluetooth proxy monitoring template sensors is appropriate and doesn't require additional examples in this section.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for template sensor usage in issues and discussions to identify common use cases
gh search issues "template sensor example" --repo esphome/esphome --repo esphome/esphome-docs --json title,body -q '.[].title' | sort -u
# Search for existing template sensor examples in the codebase
rg -l "platform: template" --type yaml
Length of output: 179
Script:
#!/bin/bash
# Search for template sensor examples in documentation
fd -e rst -e md | xargs rg -l "platform: template" --type rst --type md
# Search for template sensor examples in code
fd -e yaml | xargs rg -l "platform: template" --type yaml
# Look for template sensor related content in documentation
rg -A 5 "template.*sensor" components/sensor/
Length of output: 4423
components/text_sensor/template.rst (2)
78-82
: LGTM! Clear section introduction
The new section is well-structured and clearly states its purpose for debugging and project tracking.
78-113
: Documentation aligns well with PR objectives
The added template examples effectively fulfill the PR's goal of providing useful documentation for text sensors. The examples are practical for debugging and tracking project information.
.. code-block:: yaml | ||
|
||
# Example configuration entry | ||
sensor: | ||
- platform: template | ||
name: "Bluetooth Proxy Connections Limit" | ||
id: bluetooth_proxy_connections_limit | ||
icon: "mdi:bluetooth-settings" | ||
update_interval: 30s | ||
entity_category: "diagnostic" | ||
lambda: |- | ||
int limit = bluetooth_proxy::global_bluetooth_proxy->get_bluetooth_connections_limit(); | ||
ESP_LOGD("bluetooth_proxy_sensor", "Current connections limit => %d", limit); | ||
return limit; | ||
|
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.
🛠️ Refactor suggestion
Enhance error handling and logging in the template sensor
While the configuration is well-structured, consider these improvements:
- Add error handling for cases where bluetooth_proxy module is not available
- Enhance the log message with more context
- Consider adding a note about prerequisites
lambda: |-
+ if (bluetooth_proxy::global_bluetooth_proxy == nullptr) {
+ ESP_LOGW("bluetooth_proxy_sensor", "Bluetooth proxy not available");
+ return {};
+ }
int limit = bluetooth_proxy::global_bluetooth_proxy->get_bluetooth_connections_limit();
- ESP_LOGD("bluetooth_proxy_sensor", "Current connections limit => %d", limit);
+ ESP_LOGD("bluetooth_proxy_sensor", "Bluetooth proxy: maximum concurrent connections allowed = %d", limit);
return limit;
Also, consider adding a note about prerequisites:
.. note::
This sensor requires the bluetooth_proxy component to be configured in your device.
- platform: template | ||
name: "Bluetooth Proxy Connections Free" | ||
id: bluetooth_proxy_connections_free | ||
icon: "mdi:bluetooth-settings" | ||
update_interval: 30s | ||
entity_category: "diagnostic" | ||
lambda: |- | ||
int free = bluetooth_proxy::global_bluetooth_proxy->get_bluetooth_connections_free(); | ||
ESP_LOGD("bluetooth_proxy_sensor", "Current connections free => %d", free); | ||
return free; | ||
|
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.
🛠️ Refactor suggestion
Improve documentation structure and error handling
The second sensor has similar concerns to the first one regarding error handling. Additionally, since both sensors are closely related, consider restructuring the documentation to show their relationship.
- Add error handling similar to the first sensor:
lambda: |-
+ if (bluetooth_proxy::global_bluetooth_proxy == nullptr) {
+ ESP_LOGW("bluetooth_proxy_sensor", "Bluetooth proxy not available");
+ return {};
+ }
int free = bluetooth_proxy::global_bluetooth_proxy->get_bluetooth_connections_free();
- ESP_LOGD("bluetooth_proxy_sensor", "Current connections free => %d", free);
+ ESP_LOGD("bluetooth_proxy_sensor", "Bluetooth proxy: number of available connection slots = %d", free);
return free;
- Consider adding a description before the YAML to explain how these sensors work together:
The following example creates two complementary sensors for monitoring Bluetooth proxy connections:
- ``bluetooth_proxy_connections_limit``: Shows the maximum number of concurrent connections allowed
- ``bluetooth_proxy_connections_free``: Shows the number of available connection slots
Description:
Closing prs for dedicated sensors for this in favor of documentation (closed PR here esphome/esphome#7707 )
Related issue (if applicable): fixes
Pull request in esphome with YAML changes (if applicable): esphome/esphome#
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.