Fix Apollo Firmware Version sensor showing unknown#59
Merged
Conversation
WalkthroughRemoved automated updates to the apollo_firmware_version sensor in report scripts and removed the lambda that populated the Core text_sensor; added on_boot actions (priority 500) across multiple PLT configs to publish the firmware version to the apollo_firmware_version text_sensor. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Per ESPHome dev feedback: the version is a compile-time constant, so publish it once on_boot rather than using a lambda with periodic updates. Removed component.update from reportAllValues scripts.
The text sensor component sets up at priority 800 (HARDWARE), so publishing at the same priority races with component init. Move to priority 600 so the component is ready when publish fires.
Adding logger.log before text_sensor.template.publish to debug why the publish isn't working at priority 500.
Add text_sensor.template.publish for apollo_firmware_version to PLT-1_Minimal, PLT-1B, PLT-1B_Minimal, PLT-1_BLE, and PLT-1B_BLE. Remove debug log from PLT-1.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Integrations/ESPHome/PLT-1B.yaml (1)
26-30: Consider centralizing the firmware-version publish.This exact
on_bootpublish block now exists across every PLT-1 variant touched by the PR. Moving it intoCore.yamlor another shared include would reduce drift the next time this sensor logic changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Integrations/ESPHome/PLT-1B.yaml` around lines 26 - 30, Duplicate on_boot firmware publish block (text_sensor.template.publish with id apollo_firmware_version and state "${version}") should be centralized: remove the repeated block from each PLT-1 variant and add the single publish definition into the shared Core.yaml (or another common include) so all variants inherit it; ensure Core.yaml is included by the variant configs and that the centralized block uses the same id (apollo_firmware_version) and state expression so behavior remains identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Integrations/ESPHome/PLT-1B.yaml`:
- Around line 26-30: Duplicate on_boot firmware publish block
(text_sensor.template.publish with id apollo_firmware_version and state
"${version}") should be centralized: remove the repeated block from each PLT-1
variant and add the single publish definition into the shared Core.yaml (or
another common include) so all variants inherit it; ensure Core.yaml is included
by the variant configs and that the centralized block uses the same id
(apollo_firmware_version) and state expression so behavior remains identical.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 165e329c-feeb-467a-8fcc-0b93f332cabe
📒 Files selected for processing (6)
Integrations/ESPHome/PLT-1.yamlIntegrations/ESPHome/PLT-1B.yamlIntegrations/ESPHome/PLT-1B_BLE.yamlIntegrations/ESPHome/PLT-1B_Minimal.yamlIntegrations/ESPHome/PLT-1_BLE.yamlIntegrations/ESPHome/PLT-1_Minimal.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- Integrations/ESPHome/PLT-1.yaml
Contributor
Author
|
confirmed working with sleep as well! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Version:
What does this implement/fix?
Fixes Apollo Firmware Version text sensor showing "unknown" in Home Assistant.
component.updatewithtext_sensor.template.publishfor the version sensor inreportAllValuesscript (Battery.yaml and NonBattery.yaml)update_interval: neverso the lambda also fires periodically (every 60s) as a fallbackThe root cause is that ESPHome's
component.updateaction silently skips execution when the component'sis_ready()check fails (seeUpdateComponentAction::play()inesphome/core/base_automation.h). Thetext_sensor.template.publishaction callspublish_state()directly without this guard.Same fix as ApolloAutomation/AIR-1#87
Types of changes
Checklist / Checklijst:
If user-visible functionality or configuration variables are added/modified:
Summary by CodeRabbit