[API] Sub devices #1146
[API] Sub devices #1146
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1146 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 19 19
Lines 2779 2795 +16
=========================================
+ Hits 2779 2795 +16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #1146 will not alter performanceComparing Summary
|
WalkthroughThis pull request modifies the CI workflow to provide clearer debugging output when generated protobuf files differ, by adding informative echo statements. It also extends the API and data model by introducing a new sub-device information structure. A new message type and corresponding model class for sub-device info have been added, and existing messages and classes have been updated to include sub-device lists and device identifiers. These changes support configurations with multiple devices in one setup. Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as GitHub Actions Runner
participant Git as Git CLI
Runner->>Git: Execute "git diff --quiet"
alt Differences Exist
Runner->>Runner: echo "The diff is:"
Runner->>Git: Execute "git diff"
Runner->>Runner: Exit with status 1
end
sequenceDiagram
participant Converter as SubDeviceInfo.convert_list
participant Item as List Item
participant Instance as SubDeviceInfo Instance
Converter->>Item: Iterate over input list
alt Item is dict
Item->>Converter: Process using from_dict
else Item is not dict
Item->>Converter: Process using from_pb
end
Converter->>Instance: Create SubDeviceInfo instance
Assessment against linked issues
Suggested reviewers
✨ Finishing Touches
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. 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
aioesphomeapi/model.py (1)
220-221: Consider implementing device_id at the EntityInfo base class level.The commented-out code suggests a plan to add device_id to the base EntityInfo class. This would be a more maintainable approach than adding it to each entity type individually.
- # # Is it ok to ad for the generic device info before all are added? - # device_id: str = "" + device_id: str = ""Since this is marked as a comment/question, please clarify if there's a specific reason to delay adding this field to the base class. Adding it now would simplify future implementations across all entity types.
aioesphomeapi/api.proto (1)
387-387: Consider implementing device_id for all entity types consistently.Many entity types have commented-out device_id fields. Consider whether these should be implemented now for consistency, or if there's a specific reason to add them incrementally.
A phased approach is reasonable, but ensure there's a clear plan for adding support to all entity types in subsequent PRs. Document this plan if there are specific prerequisites or dependencies that would benefit from a staged implementation.
Also applies to: 456-457, 546-547, 577-578, 613-614, 777-778, 879-880, 961-962, 1000-1001, 1100-1101, 1136-1137, 1192-1193, 1690-1691, 1734-1735, 1773-1774, 1815-1816, 1860-1861, 1890-1891, 1935-1936, 1974-1975
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/ci.yml(1 hunks)aioesphomeapi/api.proto(24 hunks)aioesphomeapi/model.py(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
aioesphomeapi/model.pyaioesphomeapi/api.proto
🪛 GitHub Check: codecov/patch
aioesphomeapi/model.py
[warning] 145-146: aioesphomeapi/model.py#L145-L146
Added lines #L145 - L146 were not covered by tests
[warning] 148-148: aioesphomeapi/model.py#L148
Added line #L148 was not covered by tests
🔇 Additional comments (9)
.github/workflows/ci.yml (1)
125-131: Improved error handling output for protobuf file differences.The added echo statements enhance developer experience by providing clearer debugging information when generated protobuf files differ from expected versions.
aioesphomeapi/model.py (4)
172-174: Added sub_devices list to DeviceInfo class.This addition enables the representation of multiple virtual devices within a single physical device, which aligns well with the PR objectives.
234-234: Added device_id to BinarySensorInfo.This enables linking binary sensors to specific sub-devices.
251-251: Added device_id to CoverInfo.This enables linking covers to specific sub-devices.
136-150:❓ Verification inconclusive
New SubDeviceInfo class added to support virtual device grouping.
This class implements the data model for sub-devices with identification, name, and location information. Good implementation of the convert_list method for handling different input formats.
🏁 Script executed:
#!/bin/bash # Check if there are any unit tests for the new SubDeviceInfo class echo "Checking for unit tests covering SubDeviceInfo class..." rg "SubDeviceInfo" tests/Length of output: 146
Attention: Confirm Unit Test Coverage for SubDeviceInfo Class
The implementation of the new
SubDeviceInfoclass—with its clear data model and the robustconvert_listmethod handling both dictionary and protobuf inputs—is well done. However, our search in the tests directory did not locate any unit tests for this class. Please manually verify whether tests exist in another location or consider adding unit tests to cover its functionality (especially for bothfrom_dictandfrom_pbconversion paths).🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 145-146: aioesphomeapi/model.py#L145-L146
Added lines #L145 - L146 were not covered by tests
[warning] 148-148: aioesphomeapi/model.py#L148
Added line #L148 was not covered by testsaioesphomeapi/api.proto (4)
185-189: New SubDeviceInfo message type added.This protocol buffer message provides the structure for transmitting sub-device information with fields for id, name, and suggested_area.
237-237: Added sub_devices field to DeviceInfoResponse message.This allows the API to report multiple sub-devices associated with a single physical device.
281-281: Implemented device_id field for binary sensors and covers.These are the first entity types to fully implement the sub-device association. This implementation correctly links these entity types to the new sub-device structure.
Also applies to: 315-315
1-2007:❓ Verification inconclusive
Verify compatibility with the linked esphome repository PR.
The PR objectives mention a linked PR to the main esphome repository (esphome/esphome#8544). Ensure these API changes are compatible with the changes in that PR.
🌐 Web query:
What's the current status of esphome/esphome PR #8544?💡 Result:
The current search results do not provide specific information about the status of esphome/esphome PR #8544. The latest mentions in the provided data refer to releases up to 2025.3.3 and changes in 2024.8.x, but no details about PR #8544 are included.
For the most accurate and up-to-date status, you would need to check the esphome/esphome GitHub repository directly. The provided results do not indicate whether this PR has been merged, closed, or is still under review[3][6].
Citations:
- 1: https://esphome.io/changelog/v1.15.0.html
- 2: https://static.xtremeownage.com/blog/2023/pioneer-mini-split-esphome/
- 3: https://github.com/esphome/esphome/releases
- 4: https://www.sudo.is/docs/esphome/boards/esp32nodemcu/
- 5: https://esphome.io/components/sensor/index.html
- 6: https://esphome.io/changelog/2024.8.0.html
- 7: https://www.sudo.is/docs/esphome/boards/esp8266d1mini/
- 8: https://www.sudo.is/docs/esphome/components/pcd8544/
- 9: https://esphome.io/guides/faq.html
- 10: https://esphome.io/components/
Action: Manually verify compatibility with esphome/esphome#8544
The API changes in this file (aioesphomeapi/api.proto, lines 1–2007) should be checked against the upstream modifications introduced in esphome/esphome PR #8544. Currently, web searches did not return specific status details for that PR. Please ensure that:
- The updated service definitions and RPC options are fully aligned with the upstream changes.
- Any removed or replaced code in this API file is compatible with the handling in [API] Sub devices esphome#8544.
- You cross-check the most recent state of the upstream PR directly on GitHub to confirm compatibility.
🧰 Tools
🪛 Buf (1.47.2)
3-3: import "api_options.proto": file does not exist
(COMPILE)
bdraco
added
the
waiting-for-esphome
What does this implement/fix?
Adds the possibility to group entities on one physical device into virtual devices (in Home Assistant), handy when one ESP serves several purposes.
Read more in related ESP Home PR
Types of changes
Related issue or feature (if applicable):
Pull request in esphome (if applicable):
Checklist:
tests/folder).