Skip to content
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

Poll Nissan servers for battery updates #44826

Merged
merged 8 commits into from Jul 12, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
47 changes: 36 additions & 11 deletions homeassistant/components/nissan_leaf/__init__.py
Expand Up @@ -46,7 +46,7 @@
RESTRICTED_BATTERY = 2
RESTRICTED_INTERVAL = timedelta(hours=12)

MAX_RESPONSE_ATTEMPTS = 10
MAX_RESPONSE_ATTEMPTS = 3

PYCARWINGS2_SLEEP = 30

Expand Down Expand Up @@ -195,6 +195,14 @@ def setup_leaf(car_config):
return True


def _extract_start_date(battery_info):
"""Extract the server date from the battery response."""
try:
return battery_info.answer["BatteryStatusRecords"]["OperationDateAndTime"]
except KeyError:
return None


class LeafDataStore:
"""Nissan Leaf Data Store."""

Expand Down Expand Up @@ -326,20 +334,25 @@ async def async_refresh_data(self, now):
self.request_in_progress = False
async_dispatcher_send(self.hass, SIGNAL_UPDATE_LEAF)

@staticmethod
def _extract_start_date(battery_info):
"""Extract the server date from the battery response."""
try:
return battery_info.answer["BatteryStatusRecords"]["OperationDateAndTime"]
except KeyError:
return None

async def async_get_battery(self):
"""Request battery update from Nissan servers."""

try:
# Request battery update from the car
_LOGGER.debug("Requesting battery update, %s", self.leaf.vin)
start_date = None
try:
start_server_info = await self.hass.async_add_executor_job(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we store this between updates so we don't have to fetch it each time?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. In my testing, if this call isn't made (with the following 1 second sleep) then the API just never returns anything, we retry ten times and then give up. I believe doing this closely mimics the behaviour of the official app.

self.leaf.get_latest_battery_status
)
except TypeError: # pycarwings2 can fail if Nissan returns nothing
_LOGGER.debug("Battery status check returned nothing")
else:
if not start_server_info:
_LOGGER.debug("Battery status check failed")
else:
start_date = _extract_start_date(start_server_info)
await asyncio.sleep(1) # Critical sleep
request = await self.hass.async_add_executor_job(self.leaf.request_update)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't polling too frequently so if we stored the last value, we could request the update and then schedule a check 60 seconds later to get the new data and check to see if its changed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain a bit more what you mean? Is not polling too frequently a good thing?

I am open to scheduling checks and waiting for them in other methods, but my intention here is to make the code work without diverging too much from @filcole 's creation.

if not request:
_LOGGER.error("Battery update request failed")
Expand Down Expand Up @@ -367,7 +380,19 @@ async def async_get_battery(self):
server_info = await self.hass.async_add_executor_job(
self.leaf.get_latest_battery_status
)
return server_info
if not start_date or (
server_info and start_date != _extract_start_date(server_info)
):
return server_info
# get_status_from_update returned {"resultFlag": "1"}
# but the data didn't change, make a fresh request.
await asyncio.sleep(1) # Critical sleep
request = await self.hass.async_add_executor_job(
self.leaf.request_update
)
if not request:
_LOGGER.error("Battery update request failed")
return None

_LOGGER.debug(
"%s attempts exceeded return latest data from server",
Expand All @@ -382,7 +407,7 @@ async def async_get_battery(self):
except CarwingsError:
_LOGGER.error("An error occurred getting battery status")
return None
except KeyError:
except (KeyError, TypeError):
_LOGGER.error("An error occurred parsing response from server")
return None

Expand Down