Update powerwall for tesla_powerwall 0.5.0 which is async #107164

bubonicbob · 2024-01-04T18:49:32Z

Proposed change

Updates the tesla-powerwall library to 0.5.0 and therefore requires a few async updates to the component.

changelog: jrester/tesla_powerwall@v0.4.0...v0.5.0

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue: PowerWall integration does not support more than one gateway connected to the same set of power walls #100065
Link to documentation pull request: PowerWall integration does not support more than one gateway connected to the same set of power walls #100065 (comment)

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
I have followed the perfect PR recommendations
The code has been formatted using Ruff (ruff format homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

home-assistant

Hi @bubonicbob

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

home-assistant · 2024-01-04T18:50:24Z

Hey there @bdraco, @jrester, @daniel-simpson, mind taking a look at this pull request as it has been labeled with an integration (powerwall) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of powerwall can trigger bot actions by commenting:

@home-assistant close Closes the pull request.
@home-assistant rename Awesome new title Renames the pull request.
@home-assistant reopen Reopen the pull request.
@home-assistant unassign powerwall Removes the current integration label and assignees on the pull request, add the integration domain after the command.
@home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
@home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

bubonicbob · 2024-01-04T19:30:47Z

Since I don't have a powerwall, I need some help to update the fixture data so that the tests will pass. I saw these samples from a similar project but still the powerwall firmware is a few years old.

Also accepting donations to get my own powerwall(s)! 💰

bdraco · 2024-01-04T19:43:15Z

I have power walls and I have access to 5 powerwall sites as well so I will test this throughly before merging

homeassistant/components/powerwall/config_flow.py

homeassistant/components/powerwall/__init__.py

bdraco · 2024-01-07T20:28:34Z

I fixed the cookie authentication and improve the error reporting a bit

Everything seems to be working now

bdraco · 2024-01-07T20:39:39Z

Pushing to a production system for additional testing

bdraco · 2024-01-07T20:43:03Z

One system went well

The other one had the order of the serial numbers change so it generated a new unique id and all new entities

bdraco · 2024-01-07T20:50:06Z

I don't think there is anything we can do about the unique id order without doing the migration first

homeassistant/components/powerwall/__init__.py

bubonicbob · 2024-01-10T18:46:08Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

In the future, is there an official way to request that a CI be started? According to the process, I should wait to click the "ready for review" until after CI has passed.

bdraco · 2024-01-10T18:55:55Z

Once we accept a PR and merge, you won't have to ask for the CI to be started anymore

Stale

bdraco · 2024-01-10T19:26:48Z

Need to check if that config flow line is covered, config flows are supposed to have 100% coverage before merging, but if it's existing we can override it

bdraco · 2024-01-10T19:38:17Z

Missing coverage is new on the config flow so we do need to fix that

bdraco · 2024-01-10T19:38:53Z

before

---------- coverage: platform darwin, python 3.12.1-final-0 ----------
Name                                                  Stmts   Miss  Cover   Missing
-----------------------------------------------------------------------------------
homeassistant/components/powerwall/binary_sensor.py      59      0   100%
homeassistant/components/powerwall/config_flow.py       122      0   100%
homeassistant/components/powerwall/const.py              14      0   100%
homeassistant/components/powerwall/entity.py             17      0   100%
homeassistant/components/powerwall/models.py             28      0   100%
homeassistant/components/powerwall/sensor.py            103      1    99%   196
homeassistant/components/powerwall/switch.py             37      0   100%
-----------------------------------------------------------------------------------
TOTAL                                                   380      1    99%

after

---------- coverage: platform darwin, python 3.12.1-final-0 ----------
Name                                                  Stmts   Miss  Cover   Missing
-----------------------------------------------------------------------------------
homeassistant/components/powerwall/binary_sensor.py      62      1    98%   139
homeassistant/components/powerwall/config_flow.py       134      1    99%   56
homeassistant/components/powerwall/const.py              13      0   100%
homeassistant/components/powerwall/entity.py             17      0   100%
homeassistant/components/powerwall/models.py             26      0   100%
homeassistant/components/powerwall/sensor.py            110      4    96%   180, 200, 256, 276
homeassistant/components/powerwall/switch.py             37      0   100%
-----------------------------------------------------------------------------------
TOTAL                                                   399      6    98%

bubonicbob · 2024-01-10T19:43:44Z

Need to check if that config flow line is covered, config flows are supposed to have 100% coverage before merging, but if it's existing we can override it

Hmm, all but one are technically new uncovered lines of code but I'd have to think about how to even test them. They all have to do when a sensor meter isn't available. Perhaps the code should make more usage of Entity.avaliable to guard against the runtime case where the meter is None? Even so it wouldn't show up in codecov.

bdraco · 2024-01-10T20:14:49Z

I'll take care of the sensor tests

bdraco · 2024-01-10T20:25:35Z

I took care of the config flow test as well. I didn't think it was fair to ask you to do that since the coverage wasn't great for that in the first place

bdraco · 2024-01-10T20:26:29Z

Also if you like, you should add yourself to codeowners in manifiest.json

bdraco · 2024-01-10T20:28:08Z

I am updating the other sites now

cd /config
curl -o- -sSL https://gist.githubusercontent.com/bdraco/43f8043cb04b9838383fd71353e99b18/raw/core_integration_pr | bash /dev/stdin -d powerwall -p 107164

bubonicbob · 2024-01-10T20:30:31Z

I took care of the config flow test as well. I didn't think it was fair to ask you to do that since the coverage wasn't great for that in the first place

Oh thank you! My partner is giving me sideways glances how much time I'm spending on this anyhow. 👀

bubonicbob · 2024-01-10T20:32:23Z

Also if you like, you should add yourself to codeowners in manifiest.json

I might have to hold off on that until the day I get my own powerwall. It was a big productivity limitation for me on this and made it much harder to understand the HA portion of the code.

bdraco · 2024-01-10T20:34:09Z

Looks like we have an exception trap missing somewhere

2024-01-10 10:32:50.421 ERROR (MainThread) [homeassistant.config_entries] Error setting up entry Koston for powerwall
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/config_entries.py", line 406, in async_setup
    result = await component.async_setup_entry(hass, self)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/custom_components/powerwall/__init__.py", line 132, in async_setup_entry
    base_info = await _login_and_fetch_base_info(
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/custom_components/powerwall/__init__.py", line 191, in _login_and_fetch_base_info
    await power_wall.login(password)
  File "/usr/local/lib/python3.11/site-packages/tesla_powerwall/powerwall.py", line 58, in login
    return await self.login_as(User.CUSTOMER, password, email, force_sm_off)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/tesla_powerwall/powerwall.py", line 49, in login_as
    response = await self._api.login(user, email, password, force_sm_off)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/tesla_powerwall/api.py", line 172, in login
    return await self.post(
           ^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/tesla_powerwall/api.py", line 146, in post
    response = await self._http_session.post(
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/aiohttp/client.py", line 601, in _request
    await resp.start(conn)
  File "/usr/local/lib/python3.11/site-packages/aiohttp/client_reqrep.py", line 960, in start
    with self._timer:
  File "/usr/local/lib/python3.11/site-packages/aiohttp/helpers.py", line 735, in __exit__
    raise asyncio.TimeoutError from None
TimeoutError

bdraco · 2024-01-10T20:37:32Z

bdraco · 2024-01-10T20:39:32Z

Also if you like, you should add yourself to codeowners in manifiest.json

I might have to hold off on that until the day I get my own powerwall. It was a big productivity limitation for me on this and made it much harder to understand the HA portion of the code.

No worries. I'll take care of any issues that pop up from the change.

Great first contribution by the way. Most people usually start off working on a device they own 🚀

bdraco

Thanks @bubonicbob

bubonicbob · 2024-01-10T20:47:32Z

Thanks @bubonicbob

All you integration owners deserve thanks for maintaining this awesome HA system. 👏

bdraco · 2024-01-10T20:58:13Z

Everything working well on all 5 sites.

Will merge is CI gives us a 👍

Update powerwall component to use latest tesla-powerwall dependency

90ec3b8

home-assistant bot added cla-needed dependency has-tests integration: powerwall labels Jan 4, 2024

home-assistant bot previously requested changes Jan 4, 2024

View reviewed changes

home-assistant bot assigned bdraco Jan 4, 2024

home-assistant bot added the Quality Scale: No score label Jan 4, 2024

bubonicbob mentioned this pull request Jan 4, 2024

Asyncify the API jrester/tesla_powerwall#55

Merged

home-assistant bot added cla-recheck cla-signed and removed cla-recheck cla-needed labels Jan 4, 2024

bdraco reviewed Jan 4, 2024

View reviewed changes

homeassistant/components/powerwall/config_flow.py Outdated Show resolved Hide resolved

bdraco reviewed Jan 4, 2024

View reviewed changes

homeassistant/components/powerwall/__init__.py Outdated Show resolved Hide resolved

bubonicbob and others added 2 commits January 5, 2024 09:54

Use hass client session

e2e7d02

Merge branch 'dev' into powerwall_asyncify

1ee841b

This comment was marked as outdated.

Sign in to view

fix cookies and error reporting

1ff517b

bdraco mentioned this pull request Jan 7, 2024

PowerWall integration does not support more than one gateway connected to the same set of power walls #100065

Closed

bdraco reviewed Jan 8, 2024

View reviewed changes

homeassistant/components/powerwall/__init__.py Outdated Show resolved Hide resolved

bubonicbob commented Jan 8, 2024

View reviewed changes

homeassistant/components/powerwall/__init__.py Show resolved Hide resolved

Fix unique id changing

1c7694e

bubonicbob marked this pull request as ready for review January 10, 2024 19:15

home-assistant bot requested a review from bdraco January 10, 2024 19:15

Merge branch 'dev' into powerwall_asyncify

78f024c

bdraco added 3 commits January 10, 2024 10:15

add missing coverage

53b4ee3

fix typing

5c8dc7d

cover missing config flow line

bf424ff

handle timeout

891550c

bdraco approved these changes Jan 10, 2024

View reviewed changes

bdraco changed the title ~~Update powerwall component to use latest tesla-powerwall dependency~~ Update powerwall for tesla_powerwall 0.5.0 which is async Jan 10, 2024

bdraco added the smash Indicator this PR is close to finish for merging or closing label Jan 10, 2024

bdraco merged commit c74bef2 into home-assistant:dev Jan 10, 2024
53 checks passed

github-actions bot locked and limited conversation to collaborators Jan 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update powerwall for tesla_powerwall 0.5.0 which is async #107164

Update powerwall for tesla_powerwall 0.5.0 which is async #107164

bubonicbob commented Jan 4, 2024 •

edited by bdraco

Loading

home-assistant bot left a comment

home-assistant bot commented Jan 4, 2024

bubonicbob commented Jan 4, 2024

bdraco commented Jan 4, 2024 •

edited

Loading

This comment was marked as outdated.

bdraco commented Jan 7, 2024

bdraco commented Jan 7, 2024

bdraco commented Jan 7, 2024

bdraco commented Jan 7, 2024

bubonicbob commented Jan 10, 2024

bdraco commented Jan 10, 2024

bdraco commented Jan 10, 2024

bdraco commented Jan 10, 2024

bdraco commented Jan 10, 2024

bubonicbob commented Jan 10, 2024

bdraco commented Jan 10, 2024

bdraco commented Jan 10, 2024

bdraco commented Jan 10, 2024

bdraco commented Jan 10, 2024

bubonicbob commented Jan 10, 2024

bubonicbob commented Jan 10, 2024

bdraco commented Jan 10, 2024

bdraco commented Jan 10, 2024 •

edited

Loading

bdraco commented Jan 10, 2024

bdraco left a comment

bubonicbob commented Jan 10, 2024

bdraco commented Jan 10, 2024

Update powerwall for tesla_powerwall 0.5.0 which is async #107164

Update powerwall for tesla_powerwall 0.5.0 which is async #107164

Conversation

bubonicbob commented Jan 4, 2024 • edited by bdraco Loading

Proposed change

Type of change

Additional information

Checklist

home-assistant bot left a comment

Choose a reason for hiding this comment

home-assistant bot commented Jan 4, 2024

bubonicbob commented Jan 4, 2024

bdraco commented Jan 4, 2024 • edited Loading

This comment was marked as outdated.

bdraco commented Jan 7, 2024

bdraco commented Jan 7, 2024

bdraco commented Jan 7, 2024

bdraco commented Jan 7, 2024

bubonicbob commented Jan 10, 2024

bdraco commented Jan 10, 2024

bdraco commented Jan 10, 2024

bdraco commented Jan 10, 2024

bdraco commented Jan 10, 2024

bubonicbob commented Jan 10, 2024

bdraco commented Jan 10, 2024

bdraco commented Jan 10, 2024

bdraco commented Jan 10, 2024

bdraco commented Jan 10, 2024

bubonicbob commented Jan 10, 2024

bubonicbob commented Jan 10, 2024

bdraco commented Jan 10, 2024

bdraco commented Jan 10, 2024 • edited Loading

bdraco commented Jan 10, 2024

bdraco left a comment

Choose a reason for hiding this comment

bubonicbob commented Jan 10, 2024

bdraco commented Jan 10, 2024

bubonicbob commented Jan 4, 2024 •

edited by bdraco

Loading

bdraco commented Jan 4, 2024 •

edited

Loading

bdraco commented Jan 10, 2024 •

edited

Loading