Dynamic switches for tplink #117758
Dynamic switches for tplink #117758
Conversation
|
Hey there @TheGardenMonkey, @bdraco, @sdb9696, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
5a633b5
to
bd05eb5
Compare
9d56029
to
77f1aaf
Compare
| # Setting an icon overrides the translation, so we doing it here. | ||
| # icon=feature.icon, |
There was a problem hiding this comment.
Homeassistant currently picks the icon over an icon translation. It would be great to have it the other way around, i.e., to show a relevant icon if there's no translation available, but I couldn't find a way to check for the existence of a translation.
| """Turn the child switch on.""" | ||
| await self._plug.turn_on() | ||
| """Turn the switch on.""" | ||
| await self._feature.set_value(True) # type: ignore[no-untyped-call] |
There was a problem hiding this comment.
We need to fix this upstream, but it's fine in this development branch to unblock other platforms that depend on the entity.py updates done in this PR. Related upstream PR: python-kasa/python-kasa#926
| """Handle updated data from the coordinator.""" | ||
| self.entity_description = _description_for_feature( | ||
| SwitchEntityDescription, feature | ||
| ) | ||
| self._async_update_attrs() |
There was a problem hiding this comment.
I thought it'd be fine to avoid calling this in __init__, but even adding update_before_add=True to async_add_entities() does not seem to be enough to get the entity updated for tests, so I suppose this needs to be called.
Or is it better to keep the _handle_coordinator_update to call this?
There was a problem hiding this comment.
As a side effect for calling the update here makes us not to catch potential exceptions like we are doing in a single call-site for updates, see python-kasa/python-kasa#933
|
By the power of circuits and code,
@coderabbitai review |
|
@rytilahti Thank you for the poetic invocation! I will now proceed to review the changes in the pull request. 🐇 Actions PerformedReview triggered.
|
WalkthroughThe recent updates to the Home Assistant TPLink integration encompass broad enhancements and refactoring. Key changes include the removal of outdated imports, introduction of new identifiers, and restructured entity classes. These modifications aim to streamline the codebase, improve maintainability, and enhance the functionality of TPLink entities. Additionally, the test files have been updated to align with the new structure, ensuring robust testing of the refactored components. Changes
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (9)
Additional Context UsedGitHub Check Runs (1)
Additional comments not posted (19)
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
| def _category_for_feature(self, feature: Feature) -> EntityCategory | None: | ||
| """Return entity category for a feature.""" | ||
| match feature.category: | ||
| # Main controls have no category | ||
| case Feature.Category.Primary: | ||
| return None | ||
| case Feature.Category.Config: | ||
| return EntityCategory.CONFIG | ||
| case Feature.Category.Info: | ||
| return EntityCategory.DIAGNOSTIC | ||
| case Feature.Category.Debug: | ||
| return EntityCategory.DIAGNOSTIC | ||
| case _: | ||
| _LOGGER.error( | ||
| "Unhandled category %s, fallback to DIAGNOSTIC", feature.category | ||
| ) | ||
| return EntityCategory.DIAGNOSTIC |
There was a problem hiding this comment.
Tip
Codebase Verification
The search results indicate that there are no test files covering the _category_for_feature method, specifically for the unhandled category case. This confirms that the unhandled category case is not currently covered by tests.
- Please add test coverage for the unhandled category case in the
_category_for_featuremethod to ensure that the error logging and fallback behavior are properly tested.
Analysis chain
The method _category_for_feature is well-implemented, but consider adding test coverage for the unhandled category case which logs an error.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the unhandled category case in `_category_for_feature` is covered by tests.
# Test: Search for the test coverage of the unhandled category case. Expect: At least one test case covering this.
rg --type python $'Unhandled category'
Length of output: 70
Script:
#!/bin/bash
# Description: Verify that the unhandled category case in `_category_for_feature` is covered by tests.
# Test: Search for the test coverage of the unhandled category case. Expect: At least one test case covering this.
rg --type py 'Unhandled category'
Length of output: 163
Script:
#!/bin/bash
# Description: Verify that the unhandled category case in `_category_for_feature` is covered by tests.
# Find all test files that might cover the `_category_for_feature` method.
rg --type py '_category_for_feature'
# Check if any of these test files cover the unhandled category case.
rg --type py 'Unhandled category'
Length of output: 437
This PR is (the first) part of the tplink rewrite series, to be merged into
tplink_rewritebranch (see #111151).Proposed change
switchplatform to use the new facilities.via_device.TBD:
Documentation update (if it's not better done prior merging the dev branch back to master?)The screenshot below shows two new entities
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.To help with the load of incoming pull requests: