Add Sensor platform to NASweb integration #133063
Conversation
| def _get_input(coordinator: NASwebCoordinator, index: int) -> NASwebInput | None: | ||
| for entry in coordinator.webio_api.inputs: | ||
| if entry.index == index: | ||
| return entry | ||
| return None |
There was a problem hiding this comment.
Instead of doing this, why don't we make the data of the coordinator a dict of {index: entry} so we can do a quick lookup?
There was a problem hiding this comment.
Since this function is used in one place, I decided to make a change to _check_entities which makes it no longer needed.
| async def async_update(self) -> None: | ||
| """Update the entity. | ||
|
|
||
| Only used by the generic entity update service. | ||
| Scheduling updates is not necessary, the coordinator takes care of updates via push notifications. | ||
| """ |
There was a problem hiding this comment.
I'm not sure if you're referring here to the docstring or the function itself. This function is abstract and I need to implement it, but it does nothing in my case, so I added a docstring explaining why it's empty.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
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. |
|
Waiting for the review |
|
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. |
|
A documentation PR is needed. As a bar minimum, add |
| data: dict[str, Any] = {} | ||
| data[KEY_OUTPUTS] = self.webio_api.outputs | ||
| data[KEY_INPUTS] = self.webio_api.inputs | ||
| data[KEY_TEMP_SENSOR] = self.webio_api.temp_sensor |
There was a problem hiding this comment.
| data: dict[str, Any] = {} | |
| data[KEY_OUTPUTS] = self.webio_api.outputs | |
| data[KEY_INPUTS] = self.webio_api.inputs | |
| data[KEY_TEMP_SENSOR] = self.webio_api.temp_sensor | |
| data: dict[str, Any] = { | |
| KEY_OUTPUTS: self.webio_api.outputs, | |
| KEY_INPUTS: self.webio_api.inputs, | |
| KEY_TEMP_SENSOR: self.webio_api.temp_sensor, | |
| } |
| data[KEY_OUTPUTS] = self.webio_api.outputs | ||
| data[KEY_INPUTS] = self.webio_api.inputs | ||
| data[KEY_TEMP_SENSOR] = self.webio_api.temp_sensor |
There was a problem hiding this comment.
It's a little bit odd to use constants from the library to index data which we construct and consume ourselves
| STATE_NORMAL, | ||
| STATE_PROBLEM, | ||
| ] | ||
| _attr_icon = "mdi:import" |
There was a problem hiding this comment.
This should be set in icons.json instead if we never modify it: https://developers.home-assistant.io/docs/core/entity?_highlight=icons.json#icon-translations
Breaking change
Proposed change
Adds Sensor platform to NASweb integration
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.To help with the load of incoming pull requests: