Add renault integration #39605
Add renault integration #39605
Conversation
|
Please do not squash the commits, it makes it very hard to review/track progress. We will squash the PR into a single commit on merge. |
I've removed the extra sensors, and the fetching of the API keys. |
|
Thanks @Quentame. I'll work on those changes - but it looks like more work is required on the underlying PyZE library... |
|
Thanks @Quentame, I've worked through your review. |
|
Rebased due to conflicting files:
|
frenck
added
almost-done
second-opinion-wanted
| def extra_state_attributes(self) -> dict[str, Any]: | ||
| """Return the state attributes of this entity.""" | ||
| last_update = self.data.timestamp if self.data else None | ||
| return {ATTR_LAST_UPDATE: last_update} |
There was a problem hiding this comment.
Why is this necessary? States already have last_changed and last_updated attributes
There was a problem hiding this comment.
@balloob this is the last time that the Renault vehicle talked to the Renault servers. It is part of the response from the Renault API.
It serves to highlight issues where the HA component is working normally, the Renault servers are working correctly, but the car itself has stopped sending updates.
balloob
removed
the
second-opinion-wanted
|
|
||
| def __init__(self, hass: HomeAssistant, locale: str) -> None: | ||
| """Initialise proxy.""" | ||
| LOGGER.debug("Creating RenaultHub") |
There was a problem hiding this comment.
We don't want side effects in init methods. Please move or remove the logging.
| # Generate vehicle proxy | ||
| vehicle = RenaultVehicleProxy( | ||
| hass=self._hass, | ||
| vehicle=await self._account.get_api_vehicle(vehicle_link.vin), |
There was a problem hiding this comment.
Does the vehicle initialization need to be done in sequence or can we use asyncio.gather to do it concurrently?
| coordinator = self.coordinators[key] | ||
| if coordinator.not_supported: | ||
| # Remove endpoint as it is not supported for this vehicle. | ||
| LOGGER.error( |
There was a problem hiding this comment.
Looks like warning may be more appropriate?
| return entities | ||
|
|
||
|
|
||
| async def get_vehicle_entities(vehicle: RenaultVehicleProxy) -> list[RenaultDataEntity]: |
There was a problem hiding this comment.
This doesn't need to be a coroutine function since we don't await inside.
There was a problem hiding this comment.
If I move up the list, I still end up with async def async_setup_entry
Is there a non-async version of this method?
There was a problem hiding this comment.
No, that function is part of the API, so we can't change that one.
| return self.data.batteryLevel if self.data else None | ||
|
|
||
| @property | ||
| def icon(self) -> str: |
There was a problem hiding this comment.
This isn't needed when setting device class. It's already handled by the frontend. Please remove it.
| # them to be. | ||
| assert await async_setup_entry(hass, config_entry) | ||
| assert DOMAIN in hass.data and config_entry.unique_id in hass.data[DOMAIN] | ||
| assert isinstance(hass.data[DOMAIN][config_entry.unique_id], RenaultHub) |
There was a problem hiding this comment.
We want to avoid asserting integration details like the contents of hass.data. Assert the state of the config entry instead.
| assert DOMAIN in hass.data and config_entry.unique_id in hass.data[DOMAIN] | ||
| assert isinstance(hass.data[DOMAIN][config_entry.unique_id], RenaultHub) | ||
|
|
||
| renault_hub: RenaultHub = hass.data[DOMAIN][config_entry.unique_id] |
There was a problem hiding this comment.
Please don't access integration details in the tests. Patch the library and assert the calls made to the mock.
| ) | ||
|
|
||
| # Unload the entry and verify that the data has been removed | ||
| assert await async_unload_entry(hass, config_entry) |
There was a problem hiding this comment.
Please use hass.config_entries.async_unload instead.
| with patch( | ||
| "renault_api.renault_session.RenaultSession.login", | ||
| side_effect=aiohttp.ClientConnectionError, | ||
| ), pytest.raises(ConfigEntryNotReady): |
There was a problem hiding this comment.
Assert the state of the config entry instead.
| vehicle_proxy = await create_vehicle_proxy(hass, vehicle_type) | ||
|
|
||
| with patch( | ||
| "homeassistant.components.renault.RenaultHub.vehicles", |
There was a problem hiding this comment.
Please patch the library instead.
|
I will now start work on the comments from @MartinHjelmare |
Proposed change
Add integration for monitoring Renault vehicles, to replace custom_component.
Initial PR implements
config_flow, withsensorplatform.Follow-up PRs will include device_tracker and services
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: