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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rename set_wifi_led_* to set_led_* for xiaomi_miio #68629
Rename set_wifi_led_* to set_led_* for xiaomi_miio #68629
Conversation
This pull request needs to be manually signed off by @home-assistant/core before it can get merged. |
Hey there @syssi, @starkillerOG, @bieniu, mind taking a look at this pull request as it has been labeled with an integration ( |
if self._device_features & FEATURE_SET_WIFI_LED == 1 and state.wifi_led: | ||
self._state_attrs[ATTR_WIFI_LED] = state.wifi_led | ||
if self._device_features & FEATURE_SET_LED == 1 and state.led: | ||
self._state_attrs[ATTR_LED] = state.led |
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.
An LED shouldn't be an attribute and have their own services for control. It should just be a light entity with entity category "config".
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.
The new name is a bit misleading. The LED is a "WiFi connection indicator" and can be disabled to not annoy the owner. Do you agree to use a switch entity using the entity category "config" in this case?
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.
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.
Agreed, I created this PR just to fix the deprecated warning so I will close this now. The real solution is to improve the backend library to allow creating entities based on what is supported by the given device, and remove all custom services altogether.
I put some thoughts on this topic in #67058 (comment) but I'm afraid I won't personally have any free time in the future to pursue this.
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
If someone is reading this issue and thinks about working on it, see #67058 (comment) for ideas how to do it properly. |
Breaking change
To consolidate the services to allow further extensions, the following changes have been made:
switch_set_wifi_led_on
is renamed toswitch_set_led_on
switch_set_wifi_led_off
is renamed toswitch_set_led_off
wifi_led
is renamed toled
Proposed change
The upstream is moving towards having common APIs which involves some renaming some existing properties and methods,
causing deprecation warnings at homeassistant's code.
This PR converts the switch platform to use the new names, and also takes the opportunity to consolidate the naming scheme to be consistent with upstream.
Type of change
Additional information
Checklist
black --fast 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
.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: