Add better connection management for Idasen Desk #102135
Conversation
|
The failing tests don't seem related to this change. |
| ) -> None: | ||
| """Initialize an Idasen Desk cover.""" | ||
| super().__init__(coordinator) | ||
| self._desk = desk | ||
| self._attr_name = device_info[ATTR_NAME] | ||
| self._desk: Desk = coordinator.desk |
There was a problem hiding this comment.
Is this type hint needed? If the library is missing a py.typed file per https://peps.python.org/pep-0561/ it would be better to type the Desk object in the coordinator instead
| self._desk = desk | ||
| self._attr_name = device_info[ATTR_NAME] | ||
| self._desk: Desk = coordinator.desk | ||
| self._attr_name = device_info.get(ATTR_NAME) |
There was a problem hiding this comment.
Is the name now not always going to be there?
There was a problem hiding this comment.
It is still always there. I am now using pyright and it complains with:
Pyright: Could not access item in TypedDict
"name" is not a required key in "DeviceInfo", so access may result in runtime exception [reportTypedDictNotRequiredAccess]
Should I revert it back?
(I can't use pylance since I am not using vscode)
There was a problem hiding this comment.
vscode doesn't complain about it.
We know its there because you set it in async_setup_entry so it would be a bug if it wasn't. Please revert it back. We want it to blow up if future refactoring would have it be missing so it doesn't generate a hard to find bug
There was a problem hiding this comment.
Looks good to merge once this outstanding review comment is addresesed
There was a problem hiding this comment.
Oops! I reverted it in 5c507e0#diff-1b01d895b3a26b42bfc8a33f512b33e8fa163c2b19c8db761bdde72385c4ee03R128 but forgot about this one.
|
look like 3.11 5 failed for an unrelated reason. I restarted the CI runner |
bdraco
added
the
smash
|
@bdraco the diff codecov is failing. The line that is missing coverage will only be used on the other PR that I'll update as soon as this one is in. Is it ok to merge this as-is? Otherwise I'll just delete the 3 lines. |
|
It's fine to leave them since they aren't critical and I already have a good idea what is coming. We are tight on ci resources at the moment anyways due to having two supported python versions so best not to do another run just to remove them when you are going to add them right back anyways. |
| hass, | ||
| _LOGGER, | ||
| name=entry.title, | ||
| coordinator: IdasenDeskCoordinator = IdasenDeskCoordinator( |
There was a problem hiding this comment.
We're missing a type annotation for the data type of the coordinator. The coordinator class is generic. See other integrations that implement a custom coordinator class for examples.
Proposed change
This adds better connection management, by automatically reconnecting in case of a
disconnect. It is a split from a bigger PR: #100830
Diff for the supporting lib: abmantis/idasen-ha@1.4.1...2.3
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.To help with the load of incoming pull requests: