-
Notifications
You must be signed in to change notification settings - Fork 0
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Multiple cars info combined into one device #4
Comments
Excellent info, thanks for reporting. I did try to make sure they would be associated with the correct devices, but I only have the one account, and it appears that maybe didn't work as expected. I'll see what I can do to address that. Hopefully it's something obvious. If not I might ask for your help with some debug logs. To clarify, are all of the entities working as expected, with data for the correct car, and only the device is mixing them together? And if you open the more info dialog for each entity, does it have the correct name for each car, as well as in the entity id? For example, |
Thanks for getting back about this. |
Great, thanks for confirming. In that case the fix should be pretty simple 🤔 |
Are you comfortable viewing your Look for 2 entries containing:
It's possible that there might only be one entry with that URL, since that would seem to line up with what you're seeing in the UI (one device, conflating the two physical cars as one). Specifically, I'm curious about what the I used to get ids for those:
I think a fix for this would be to change how we're constructing However I'm not sure what effect that will have on the actual device registry and how those are currently being associated. That is: will it leave the old conflated device as an orphan; or will it split them apart as expected and keep them associated correctly. Or both. I may need to do some testing to see if I need to add a config entry version bump in order to do a one-time migration that removes the old device entry and adds the 2 new device entries. I'll start by first switching a single device identifier, using |
So, I thought we would be able to easily set up a way to test this before releasing, but it looks like HACS does not have that capability. I've opened a feature request to consider ways to facilitate end-user testing before a PR is merged. Anyway, I'll let you know when there's an easier way to test. |
Sorry for the delay - time zones... May be too late now, but have discovered how to view that file - below. It seems to be as you suspected.
|
SO grateful for all your work with this. Re-downloaded the pr/5 as per, and I think it's almost what you're expecting. I suspect that it would now all be cleaned up and work brilliantly by removing the devices and re-adding them, but I haven't done that yet in case there's any information which you'd like before I do... Now shows as 2 devices, but with the entities unevenly split: The original device ID is now unavailable, I don't know if that's the same as empty: and the device_registry now has 4 search matches for 'TeslaFi' - was 2 before - but one of them has 2 VINs and the "config entries" are the same for both. The sensors which are now showing as available have got a bit confused as well - the sensor.miss_daisy_car_state is now reporting Ticky's status. As I say I strongly suspect that this would / will be sorted out by removing and re-adding, but that perhaps doesn't help the rest of the user base! Conversely, with any luck the upgrade only goes wonky when there are 2 TeslaFi accounts linked - which is undoubtedly a much smaller user base! I have a test RPi at work which I'm using to try things out on (new to HA but enthusiastic...) so I can try setting up the new integration with 2 TeslaFi accounts from scratch on a 'clean' system tomorrow. Let me know if there's anything you'd like from the 'wonky' system - and thanks again! :) |
OK, that is unexpected. It looks like it doesn't dedupe the VIN entry. I'll work on a config entry migration that will clean up the bad entries. Normally this would be done by mapping one-to-one config entries to devices, because all that's changing is the identifiers for known unique devices already, but in this case the device entries are known to be not (properly) unique. That is, the devices that have 2 I guess the easiest thing to do would be to (during the config flow migration, so it only happens once) delete all device entries for each config entry, and let HA recreate new devices on reload. But that will result in a new device.id. Doesn't actually matter here, because we don't have device actions/triggers/conditions, so it is still an option and the simplest option at this point. If we want to keep the existing device ids, we would need to add a migration that, scans the device entries for each config entry, and tries to "normalize" them by:
That's a lot of effort to go through, and probably not worth it. I think for now I'll just make it delete all device entries on migration, and let HA recreate them. |
Agreed - given that the Venn diagram overlap between 'Uses HA' and 'Is interested in 2 different TeslaFi accounts' is probably pretty slim it feels like it wouldn't be worth the considerable effort involved! |
OK, can you try redownloading the temp tag again? In HACS, first click It should:
You should see a warning entry in the System > Logs, explaining that it is removing a corrupted device (in your case you may see it log 4x). From there, it should proceed with setting up the integration with the correct device identifiers, and you should then finally have 2 devices with 40 entities each. |
Sadly not - it now ends up with 40 entities (all .miss_daisy) - for instance sensor.miss_daisy_battery, but split between the two devices (Ticky and Miss Daisy). Let me know if you'd like any other info, but it feels like that part has taken up enough of your time. |
Does that mean that the other 40 for Ticky just disappeared? In the device registry file, other than the "HACS" entry for the integration repo, how many other devices are there with the domain "teslafi"? And how many identifiers for each? There may also be some that got moved to the deleted/orphaned section towards the end of the file, which is what I expected to happen: I expected it to move all of the associated devices to deleted, and then when loading the integration, it should have created 40 and 40, each with their respective (new) device. What might have happened though, since I was only testing the migration with my one config entry because I don't have a second API key, is it might have gone through the whole process twice: load car one, migrate car one (which should have deleted any matching devices), add entities and new device for car one; and then do the same steps for car 2. But the migration for car one should have cleared all of the related entities, identifiers, and config entries for car one, which should have severed any connections to car two before it even got to that point... I'm worried about the fact that you lost all of the car two entities. That's the most troubling part. |
Yes the others seem to have vanished. 2 instances of 'teslafi' in the core.device_registry file as of now: { |
Ok, and again the double config entry is unexpected. That means that both entries are association themselves with the one device. Also odd that it lists both config entries, but you don't have the second car's entities? I'll run some more tests and see if I can figure out what's happening |
Okey dokey. I won't do that unless you say though - in case you want to try something else from where my system currently is. However, because my system has done the two different versions of pr/5 I thought maybe mine is now in a weird state! |
No worries, I don't need your token. Anyway, given that you're probably one out of a tiny handful who may be in this situation (I don't even know how many people have found the integration at all, and it was never advertised in the forum or anything), and you're seeing that it comes up correctly on a clean installation with the pr/5 version, I think I'm going to call it good enough for now, and just call it out in the release notes. So, just one last test: let's try removing the integration on the broken one, redownload pr/5 from HACS, and then add back the integration (x2). Hopefully it will come up like it did on your fresh install, and if so, I'll release it like that. If not, and it's in a broken/half state, we may need to do some more work to clean up the broken device entries. |
Yep absolutely. Have just done that and it appears to be perfect (thanks again). Device registry is now as below - and everything appears to be brilliant. Your work on this is hugely appreciated! { |
OK great, thanks for checking. And just to confirm - this was on the initial broken install, not the sandbox/pi clean system that you tested here? I'm just going to do one more round of cleanup, because I think I can make the device entry migration smarter (so it doesn't have to delete a device if all it needs to do is rearrange the identifiers). But since you've already applied the migration now (config flow v2), once it does get re-released, it won't affect you because your config version will match, and it won't try to migrate you again. |
Yes, this was on my main install which had been 'wonky' before. |
OK, confirmed that migrating config entry v1 to v2 works correctly and does not have to delete the device (which can be problematic since it also resets entity overrides - such as if you've enabled one of the disabled-by-default entities). Now going from v1 to (future) v2, as long as it's not in a broken state like yours was, it will just rewrite the identifiers and not delete anything else. I'm going to close this with the #5 PR |
Also, side note, I did speak with the maintainer of HACS, and he says he is planning to make it possible to install an arbitrary version in the future, using the |
Hello again :) Removed both TeslaFi accounts. Linked first TeslaFi account (Ticky) to HA. All added fine. core.device_registry entry now: re-started HA. All still working and registry unchanged. Linked the second (Miss Daisy) account to HA. Linked fine, both show up (and show the correct information) in the Overview. core.device_registry entry now: Also no 'TeslaFi' entries in the log. All is well. Then restarted HA again... Ticky has now become 'unavailable' and there are several entries in the log about unique IDs and the config_entries in the registry file have got 'mixed' As before, if this is something that can be fixed without taking up lots of your time then that would be great, but I am already very grateful for your work on this and don't expect anything! |
It sounds like the Did you still have that 2nd "sandbox" server you were toying with previously? And are both cars still connected and working fine there, including after a restart? If the 2nd server is still working correctly, I think that means that the current integration is correct, but the devices/entities (and unique ids) from the broken version a few weeks ago are still lingering somewhere and causing problems. But I don't have a good way to test this scenario, since I only have one car and one TeslaFi account. |
Makes sense. I do still have the sandbox server and the behaviour is the same on that one (breaks after a restart), but that may also have some IDs left over from the previous iteration. There's nothing on that server that I need day to day so I'll take a backup of it, wipe it clean and then start from a properly clean install to see where we're at :) Have now done that, and sadly things are the same. Totally clean flash of HA, added Terminal, HACS & TeslaFi integration then the cars one at a time (with restart after the first car). Same situation, so both cars showed up fine after the second install, but one of them goes unavailable after the restart. I have disabled the API commands on my Ticky account so let me know how I should send the API token to you - thus you'd have a second account to test with (if you'd like - I totally understand if you don't have the time). Then once all is well I can change my token :) |
Also seeing this behavior with two Tesla vehicles. Tested via clean instance of HA, works until rebooting core and then the second Tesla goes unavailable. Any thoughts on what can be causing this. Would be happy to provide a token to help troubleshoot and get this great integration to support >1 vehicle. Thanks! |
If you're willing to provide a teslafi token, that would help me with testing. I may also be able to mock a 2nd device, in which case I may not need a real teslafi token. But I'm transitioning to a new workstation right now and I'm not quite at a point where I can work on it, and it wouldn't make sense to provide me access until I can actually sit down and work on it. I might have some time later in the week or this weekend, but I'll give you a heads up so that you can prepare a temporary token for testing; and a timeframe so you can know when it's clear to revoke/regenerate. |
Happy to provide @jhansche when you're ready to troubleshoot. Really appreciate it. Happy to support in anyway as this is a very useful repo, something I had been using template sensors and JSON parsing prior. |
Sorry to bother, but have you had any luck figuring it out? Would provide a second token if you needed. Thank you |
Just as a data point, I tried manually installing the integration to a different folder in custom_components and changing the domain information in manifest.json and const.py. Then that second 'copy' of the integration had the second API key. It sort-of worked as in each integration retained 40 entities after a restart, but a range of sensors (battery, range, cabin and outside temperatures) became unavailable on both cars / accounts. |
We were using a class attribute for saving API response data, which causes corruption when there are more than one config entries set up. Move that to an instance attribute to avoid the collision. This fixes #4
I finally got some time to look into this. I was able to reproduce the issue on my own, by mocking a 2nd vehicle - but thank you for the offer to use your token @BDubz20. The issue ended up being a memory leak caused by using shared state between the two config entries, leading to data corruption between the two entries: entities from one device start bleeding over into the other device, and vice versa. This also led to finding some other issues: for example, we weren't defining a proper Thanks for your patience, both of you. I'll let you know when the fix has been released. |
We were using a class attribute for saving API response data, which causes corruption when there are more than one config entries set up. Move that to an instance attribute to avoid the collision. This fixes #4
For this particular issue -- and I would recommend checking here first before you update, just so you can see what has changed -- when you go Settings > Integrations > TeslaFi (http://hass.local:8123/config/integrations/integration/teslafi), you will likely see something like this: that is, this is what we want to see, and the first time you add the 2nd API key, this is what you should see. Even after reloading config entries, this continues to look correct. Specifically, each entry should have But as soon as you restart Home Assistant, things get weird. For one or both physical device entries (cars), some of the entities will get mixed in with the other physical device. For one or both config entries, you'll see After the update, you'll know that it is fixed, if you see I do not know what kind of side effects you might experience if you leave it like this - but you're welcome to try leaving it if you want to see what happens. If it bothers you (like it would me), you can clear that count by deleting the old device(s) that have gotten mixed up. The above screen would tell you if a config entry is associated with more than one device ( Once that's done, you'll want to go back to the Integration page, and reload both config entries. Once it's reloaded, you'll see just |
The update is available now: |
Amazing @jhansche. Thanks for all the hard work and for fixing the bug. Looking forward to getting the automations running again. How can I support you and this repo? Happy to buy you a coffee :) |
Pay it forward 😁☕ |
Just to add my thanks again for the work on this - seems to be working beautifully on two accounts now. Now it's not just my wife that can have a car warmed up by an automation in the mornings :) |
Very many thanks for your work on this - I trust the TeslaFi service to not keep the car 'awake' so prefer not to connect directly.
I have access to two different TeslaFi accounts and would like to add them both to my HA. When I add the second API token it recognises it successfully and asks which area to add this second car to. However, it then combines both cars into one 'device', so instead of 2 devices with 40 entities each I then have 1 device with 80 entities, all in the second car's area.
In the dashboard there are then two different entries, named the same thing (all named as the second account) but actually referring to two different vehicles. In my case the cars are called 'Ticky' and 'Miss Daisy'. Screenshot below of the two cars showing in the integration entries, but only one device. Also further below showing a dashboard for 'Car Miss Daisy' containing two different cabin temperatures for two different cars - but one is actually for Ticky...
Let me know if there is anything I can do to assist in troubleshooting this snag. Many thanks, Dave
The text was updated successfully, but these errors were encountered: