[api] Fix missing ifdef guards for field_ifdef fields in protobuf base classes - #9693
Conversation
|
To use the changes from this PR as an external component, add the following to your ESPHome configuration YAML file: external_components:
- source: github://pr#9693
components: [api]
refresh: 1h(Added by the PR bot) |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #9693 +/- ##
=======================================
Coverage 59.94% 59.94%
=======================================
Files 50 50
Lines
8000
10293 10293
Branches 1381 1381
=======================================
Hits 6170 6170
Misses 3762 3762
Partials 361 361 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Also for a followup, we still generate But it should have an ifdef I can't fix it without conflicting with other PRs though so I'll leave it for later |
|
👋 Hi there! I've automatically requested reviews from codeowners based on the files changed in this PR. @OttoWinter - You've been requested to review this PR as codeowner(s) of 2 file(s) that were modified. Thanks for your time! 🙏 |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the protobuf code generation script to properly wrap fields with ifdef guards when they are extracted into base classes. Previously, fields marked with field_ifdef options (like USE_DEVICES for device_id and USE_ENTITY_ICON for icon) were losing their conditional compilation guards when moved to generated base classes.
- Adds a helper function to detect consistent
field_ifdefoptions across messages sharing common fields - Updates base class generation to preserve ifdef guards for fields when all derived classes agree on the same ifdef condition
- Fixes API connection code to properly wrap icon field access with the corresponding ifdef guard
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| script/api_protobuf/api_protobuf.py | Adds logic to preserve field_ifdef guards in generated base classes |
| esphome/components/api/api_pb2.h | Generated protobuf headers with proper ifdef guards around conditional fields |
| esphome/components/api/api_connection.h | Wraps icon field access with USE_ENTITY_ICON ifdef guard |
Not for backport. can wait for 2025.8.x as it just uses more memory than needed but not a lot.
What does this implement/fix?
This PR fixes the protobuf code generation script to properly wrap fields with their corresponding ifdef guards in base classes. Previously, fields marked with
field_ifdefoptions in the proto file (such as[(field_ifdef) = "USE_DEVICES"]for device_id and[(field_ifdef) = "USE_ENTITY_ICON"]for icon) were not getting ifdef guards when they appeared in generated base classes likeInfoResponseProtoMessage,StateResponseProtoMessage, andCommandProtoMessage.The fix ensures that when common fields are extracted into base classes, their field_ifdef options are preserved if all derived classes have the same ifdef value.
Types of changes
Related issue or feature (if applicable):
Pull request in esphome-docs with documentation (if applicable):
Test Environment
Example entry for
config.yaml:# No configuration changes needed - this is an internal code generation fixChecklist:
tests/folder).If user exposed functionality or configuration variables are added/changed:
Additional details
The change includes two fixes:
Protobuf generation fix: Adds a
get_common_field_ifdef()helper function that checks if all messages sharing a common field have the samefield_ifdefoption. If they do, the base class field declaration is wrapped with the appropriate ifdef guard.API connection fix: Wraps the
iconfield access inapi_connection.hwith#ifdef USE_ENTITY_ICONto match the generated protobuf headers.This reduces memory usage by excluding fields when their features are disabled:
device_idfield (4 bytes + padding) whenUSE_DEVICESis not definediconfield (string object overhead) whenUSE_ENTITY_ICONis not definedThis is particularly important for memory-constrained devices where every byte counts.