Add new Volvo integration #142994
Add new Volvo integration #142994
Conversation
home-assistant
bot
added
cla-signed
has-tests
new-integration
integration: volvo
quality-scale
labels
|
Test is failing on an unrelated component: |
| if self.source in (SOURCE_REAUTH, SOURCE_RECONFIGURE): | ||
| # Don't let users change the VIN. The entry should be | ||
| # recreated if they want to change the VIN. | ||
| return await self._async_create_or_update() |
There was a problem hiding this comment.
Ideally I would argue that we create 1 entry for the account and just create the cars on the fly. It might even use sub entries
There was a problem hiding this comment.
The thing is that, once all platforms have been added, a single coordinator update cycle will consume about a dozen of API requests. There is a limit on the number of API requests a user can make per API key. So I wouldn't add all vehicles on the fly, and try to keep an API key per vehicle. I can look into sub entries if that is still feasible then.
There was a problem hiding this comment.
I've spent a few hours wrapping my head around sub entries (most of the time to already refactoring code so I could use the same flow in both root entry and sub entry, but probably was not the best choice in hindsight 😄). I'm at the point now where I can create a sub entry, but it seems that the integration init only runs for the root config entry.
I've searched in the source code for examples, and could only find kitchen_sink and mqtt, but it is still unclear to me how to assign entities to the sub entry, or let the coordinator do its work.
How can I now effectively add entities to a sub entry? I thought HA would init all sub entries automatically, as if they were a first class entry. That would take some load off of the implementors and simplify things from a implementor's perspective. Or if that is not possible, then maybe sub entries are not the best fit for this scenario?
Please advise.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
| # | ||
| # Raising ConfigEntryAuthFailed will cancel future updates | ||
| # and start a config flow with SOURCE_REAUTH (async_step_reauth) | ||
| _LOGGER.exception( |
There was a problem hiding this comment.
Is this a rightful usage of Exception? Shouldn't this be _LOGGER.error?
There was a problem hiding this comment.
I'm not sure, tbh. Don't we want to know what exactly happened?
There was a problem hiding this comment.
As I contributed in some part of this checks here is my comments about this part:
this _LOGGER.exception is very usefull to understand some problems that we got from Volvo API. And that VolvoAuthCondition is one that we must understand what are the real cause.
If the Volvo servers are really trustable I agree to use only error, but we still receive strange messages from Volvo servers and this helps to understand. As an example there is an API entrypoint that sometimes return correct result and sometimes 404.
I suggest to keep this for sometime until Volvo servers return to be stable, and we dont have to understand what is happening on some of the API calls.
There was a problem hiding this comment.
Do we also know why the Volvo servers are unstable?
Generally speaking the exception is used when something truly breaks and can't continue. Is that also the case here?
There was a problem hiding this comment.
Do we also know why the Volvo servers are unstable?
No, not really.
Generally speaking the exception is used when something truly breaks and can't continue. Is that also the case here?
If a VolvoAuthException is raised during the coordinator update cycle, then it means that the token refresh has failed and no data will be retrieved.
There was a problem hiding this comment.
@erwindouna some have tried to contact them without responses. Apparently they have many servers and depends what server responds to your request or what route you take. One time that I got timeouts using my fiber connection I was able to get responses using my starlink. For me the problem with 404 is related to some kind of reverse proxy and backend that is not correctly configured.
|
I've marked it ready for review again, while knowing there are still two open questions/remarks:
|
|
Pulled latest from |
|
Fantastic work @thomasddn! Just a question regarding car models supported. But in the API documentation it says: All cars with Volvo On Call add-on from model year 2010-2024 Or am I looking in the wrong place? |
|
@bohmandan Good catch! Apparently, they have done some changes to their API and documentation recently, and they now support more vehicles. See a previous version of that page on the wayback machine. I will adapt the docs. Thanks for bringing this to my attention! |
Proposed change
This new integration allows you to integrate your Volvo vehicle in Home Assistant.
There was already a Volvo integration, but the API used was deprecated and the integration is no longer maintained. I've chosen to make a completely new integration instead of rebuilding the existing one mainly because the deprecated integration uses the
volvooncalldomain. "On Call" is a reference to the marketing name of the deprecated Volvo API. This new integration usesvolvowhich is agnostic of any API marketing name.Ideally, the old
volvooncallintegration is removed.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: