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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GEO Location data is truncated Hyundai USA #196

Closed
rappazzo opened this issue Dec 1, 2021 · 36 comments
Closed

GEO Location data is truncated Hyundai USA #196

rappazzo opened this issue Dec 1, 2021 · 36 comments
Labels
bug Something isn't working USA

Comments

@rappazzo
Copy link
Contributor

rappazzo commented Dec 1, 2021

Please check Services, Known Bug / Issues and Troubleshooting over here first: https://github.com/fuatakgun/kia_uvo/blob/master/README.md
Region and Brand of car
USA - Hyundai 2020 Ioniq EV

Describe the bug
The Location info in the logs seems to have lost its decimal precision
Here is my car's current location (in the Atlantic Ocean!):

         'vehicleLocation': {
            'coord': {
               'alt': 24, 
               'lon': -74, 
               'type': 0, 
               'lat': 40
            }
         }, 

It looks like this is just truncated (not rounded), as my actual latitude is closer to 41

To Reproduce
Steps to reproduce the behavior:

  1. In the lovelace interface, add a map card including the device tracker location of the car
  2. Observe the imprecise location.
  3. In the logs, observe that the location lat and lon values have no decimal precision

Expected behavior
The lat and lon values need a few more decimal places for more accuracy

Screenshots
From Home Assistant
("mep" is my phone's location)

@cdnninja
Copy link
Collaborator

cdnninja commented Dec 1, 2021

Are you able to confirm the Kia app works correctly?

@rappazzo
Copy link
Contributor Author

rappazzo commented Dec 1, 2021

yeah here is a screen shot (Hyundai app):
From Hyundai App
:

@cdnninja
Copy link
Collaborator

cdnninja commented Dec 1, 2021

Is the above from the logs or home assistant data? I don't see us doing anything to the location so not sure why this would occur unless they aren't giving us the right amount of accuracy.

@rappazzo
Copy link
Contributor Author

rappazzo commented Dec 1, 2021

The above data is from the logs (after I turned on debug level logging). For what it's worth, I used to use Bluelinky with Node Red, and that did have the correct precision in the data.

@rappazzo
Copy link
Contributor Author

rappazzo commented Dec 1, 2021

Here is the full message from the log:

2021-12-01 15:06:44 DEBUG (SyncWorker_4) [custom_components.kia_uvo.HyundaiBlueLinkAPIUSA] kia_uvo - get_cached_vehicle_status response {'hataTID': '<redacted>', 'vehicleStatus': {'dateTime': '2021-12-01T20:05:12Z', 'acc': False, 'defrostStatus': 'false', 'transCond': True, 'doorLockStatus': 'false', 'doorOpen': {'frontRight': 0, 'frontLeft': 0, 'backLeft': 0, 'backRight': 0}, 'battery': {'batSoc': 87, 'batState': 0, 'sjbDeliveryMode': 0}, 'vehicleLocation': {'coord': {'alt': 24, 'lon': -74, 'type': 0, 'lat': 40}}, 'ign3': False, 'ignitionStatus': 'false', 'lowFuelLight': False, 'sideBackWindowHeat': 0, 'engine': False, 'hoodOpen': False, 'airConditionStatus': 'false', 'steerWheelHeat': 0, 'trunkOpen': False, 'doorLock': False, 'airCtrlOn': False, 'airTemp': {'unit': 1, 'hvacTempType': 0, 'value': 'LO'}, 'evStatus': {'valueDiff': -2, 'remainTime2': {'etc3': {'unit': 1, 'value': 25}, 'etc2': {'unit': 1, 'value': 0}, 'atc': {'unit': 1, 'value': 0}, 'etc1': {'unit': 1, 'value': 0}}, 'evIgnitionStatus': False, 'batteryPlugin': 0, 'timeDiff': 35703, 'batteryCharge': False, 'batteryStatus': 98, 'remainTime': [], 'drvDistance': [{'rangeByFuel': {'gasModeRange': {'unit': 3, 'value': 0.0}, 'totalAvailableRange': {'unit': 3, 'value': 200.0}, 'evModeRange': {'unit': 3, 'value': 200.0}}, 'type': 2}], 'reservChargeInfos': {'targetSOClist': [{'rangeByFuel': {'gasModeRange': {'unit': 3, 'value': 0}, 'totalAvailableRange': {'unit': 3, 'value': 200}, 'evModeRange': {'unit': 3, 'value': 200}}, 'plugType': 0, 'targetSOClevel': 127, 'type': 2}, {'rangeByFuel': {'gasModeRange': {'unit': 3, 'value': 0}, 'totalAvailableRange': {'unit': 3, 'value': 200}, 'evModeRange': {'unit': 3, 'value': 200}}, 'plugType': 1, 'targetSOClevel': 127, 'type': 2}], 'reservChargeInfo': {'dateTime': '2021/12/01T12:06:44Z', 'reservChargeInfoDetail': {'reservChargeSet': False, 'reservEndTime': '', 'reservFatcSet': {'defrost': False, 'airTemp': {'unit': 0, 'value': '00H'}, 'airCtrl': 0}, 'reservInfo': {'time': {'timeSection': 0, 'time': '1200'}, 'day': [9]}}}, 'reservFlag': 0, 'reserveChargeInfo2': {'reservChargeInfoDetail': {'reservChargeSet': False, 'reservEndTime': '', 'reservFatcSet': {'defrost': False, 'airTemp': {'unit': 0, 'value': '00H'}, 'airCtrl': 0}, 'reservInfo': {'time': {'timeSection': 0, 'time': '1200'}, 'day': [9]}}}, 'offpeakPowerInfo': {'offPeakPowerFlag': 0, 'offPeakPowerTime1': {'startTime': {'timeSection': 0, 'time': '1200'}, 'endTime': {'timeSection': 0, 'time': '1200'}}}}}, 'sleepModeCheck': True, 'defrost': False, 'tirePressureLamp': {'tirePressureWarningLampRearLeft': 0, 'tirePressureWarningLampFrontLeft': 0, 'tirePressureWarningLampFrontRight': 0, 'tirePressureWarningLampAll': 0, 'tirePressureWarningLampRearRight': 0}, 'trunkOpenStatus': 'false'}}```

@rappazzo
Copy link
Contributor Author

rappazzo commented Dec 1, 2021

I am primarily a java coder so take this with a grain of salt, but it looks like custom_components/kia_uvo/HyundaiBlueLinkAPIUSA.py doesn't implement a separate call to get the location. Having looked through the bluelinky node red code (it's in node), there is a separate call for location and odometer. Not sure if that is relevant or not

@cdnninja
Copy link
Collaborator

cdnninja commented Dec 1, 2021

get_location was used in the canada API because the data wasn't included in the first API call. It appears as though Hyundai USA includes atleast some data in the initial call.

I think you are correct in that we need to call a second API to get the more accurate data.

Today we don't have a developer actively working on Hyundai USA API, I cover canada and other regions where I can. Feel free to drop a PR if you are inclined. I can also give it the first go around if you would rather not, I would need a hand testing though.

@cdnninja cdnninja added bug Something isn't working USA labels Dec 1, 2021
@cdnninja cdnninja changed the title GEO Location data is truncated GEO Location data is truncated Hyundai USA Dec 1, 2021
@rappazzo
Copy link
Contributor Author

rappazzo commented Dec 1, 2021

I could take it up, but I don't know how to setup the testing environment. Can you point me to how to do that?

@cdnninja
Copy link
Collaborator

cdnninja commented Dec 1, 2021

I am terrible and just edit my production HA code, restarting HA all the time. I have it so I can browse my HA via SMB and edit via my editor of choice.

@fuatakgun recommended I setup a dev environment and had thoughts on a way that works well.

@fuatakgun
Copy link
Member

i have setup vs code following home assistant developer guidelines which us independent than my production environment. Restarting it is very fast, please let me know if you can do it or i can code blindly and ask you to test @rappazzo

@rappazzo
Copy link
Contributor Author

rappazzo commented Dec 2, 2021

Once I have the VSCode dev container up, I need to install HACS to it? Then, how do I point it to my copy of this repo? (I am happy to discuss on discord/etc rather than hijack the github issue if there is such a place too).

@rappazzo
Copy link
Contributor Author

rappazzo commented Dec 2, 2021

https://github.com/fuatakgun/kia_uvo/compare/master...rappazzo:hyundai-usa-location?expand=1
Still not sure how to test this. I could pull it into my HA instance I guess.

@fuatakgun
Copy link
Member

I can see your pull request and it is a good starting point and I see that it requires PIN based call, which consumes daily API call limit of the user. I am not sure if we should place this call part of update or force_update.

  • update is responsible to receive cached data
  • force_update is using PIN and forcing car to refresh the data on cloud and get refreshed data from update

@cdnninja , I remember you had some issues with limits, what do you think?

@cdnninja
Copy link
Collaborator

cdnninja commented Dec 2, 2021

What I did in CA was calling get location in get_status but only when odometer changed. It also passes the pauth to reuse it i believe. A little hacky if you look at it but works.

@fuatakgun
Copy link
Member

did you call get_location inside update or force_update with odometer condition?

@AndrewDaws
Copy link

I can confirm I am also having the same Hyundai USA location truncation issue.
Screenshot from 2021-12-02 13-45-28
As a result my vehicle is showing waaaaay off where it actually is.
Screenshot from 2021-12-02 13-46-14

@rappazzo
Copy link
Contributor Author

rappazzo commented Dec 2, 2021

I didn't add any triggers as of yet. When I had the node red bluelinky setup, I also needed to make a separate call to get the odometer reading, so @cdnninja's solution may not work for Hyundai USA. I don't see the odometer among the entity listing for this integration either, so that would be consistent.

That node red setup was on a 1 hour cadence, making 3 calls - status, location, and odometer. As far as the daily API limit, I don't think that I ever reached it.

Would it make sense to put these on a separate schedule? Is that an option?

@fuatakgun
Copy link
Member

what about last_updated in get_vehicle response? if data is updated on cloud response, we can trigger a PIN based call to fetch location.

@rappazzo
Copy link
Contributor Author

rappazzo commented Dec 2, 2021

Judging by the logs, this may work. Over the last day or so in my log with DEBUG level set, it doesn't update too often.

One thing that I am a little unclear on. This code seems to want to format the date:

        vehicle_status["vehicleStatus"]["dateTime"] = (
            vehicle_status["vehicleStatus"]["dateTime"]
            .replace("-", "")
            .replace("T", "")
            .replace(":", "")
            .replace("Z", "")
        )

But the log looks like that isn't working:

'dateTime': '2021-12-01T23:05:45Z'

Or is this log from before the alteration?

(I've updated my fork -- see the link above for changes)

@cdnninja
Copy link
Collaborator

cdnninja commented Dec 3, 2021

Logs are coming from line 148 so before the datetime change. The home assistant data entity is after the changes occur. I think your updated fork above still needs data mapping to get the value returned to Home assistant?

As to odometer that is another bug that is open that needs to be fixed. In Hyundai USA it is in a separate call, it is the only region like this.

@rappazzo
Copy link
Contributor Author

rappazzo commented Dec 3, 2021

I've updated my branch to be based on your odometer patch branch (with my fix). I (mostly) copied the "if odometer changed" code from Kia CA for the updates. Once the odometer code is merged, I will test this on my HA instance, and submit a pull request.

rappazzo pushed a commit to rappazzo/hacs-bluelink that referenced this issue Dec 3, 2021
…nect#196)

The location information provided in the cached vehicle status does not
include decimal precision.  In order to fix this, implement the get_location
method in the api for Hyundai USA.

Considering that there is a daily API limit, poll the get_location
method based on how fresh the data in the get_status response is.
@cdnninja
Copy link
Collaborator

cdnninja commented Dec 3, 2021

Good FYI on rates: https://github.com/Hacksore/bluelinky/wiki/API-Rate-Limits

Once this is working we can see if you hit the limit and how the integration handles it. I find I don't using this logic, but we don't drive often. In theory location is only hit if you moved between scans. A full day of driving may max out location calls.

@rappazzo
Copy link
Contributor Author

rappazzo commented Dec 3, 2021

In the logs there are a lot more requests than I would have expected.

2021-12-03 11:41:00 WARNING (SyncWorker_0) [homeassistant.loader] We found a custom integration kia_uvo which has not been tested by Home Assistant
2021-12-03 11:41:07 DEBUG (MainThread) [custom_components.kia_uvo] kia_uvo - async_setup_entry started 
2021-12-03 11:41:09 DEBUG (SyncWorker_5) Get Vehicles Response
2021-12-03 11:41:18 DEBUG (SyncWorker_2) Get Vehicles Response
2021-12-03 12:11:19 DEBUG (SyncWorker_2) Get Vehicles Response
2021-12-03 12:11:21 DEBUG (SyncWorker_0) Get Vehicles Response
2021-12-03 12:11:21 DEBUG (SyncWorker_4) Get Vehicles Response
2021-12-03 12:11:23 DEBUG (SyncWorker_3) Get Vehicles Response
2021-12-03 12:11:24 DEBUG (SyncWorker_5) Get Vehicles Response
2021-12-03 12:41:25 DEBUG (SyncWorker_7) Get Vehicles Response
2021-12-03 12:41:27 DEBUG (SyncWorker_7) Get Vehicles Response
2021-12-03 12:41:27 DEBUG (SyncWorker_9) Get Vehicles Response
2021-12-03 12:41:29 DEBUG (SyncWorker_4) Get Vehicles Response
2021-12-03 12:41:29 DEBUG (SyncWorker_0) Get Vehicles Response
2021-12-03 13:11:31 DEBUG (SyncWorker_9) Get Vehicles Response
2021-12-03 13:11:33 DEBUG (SyncWorker_4) Get Vehicles Response
2021-12-03 13:11:33 DEBUG (SyncWorker_3) Get Vehicles Response
2021-12-03 13:11:35 DEBUG (SyncWorker_7) Get Vehicles Response
2021-12-03 13:11:35 DEBUG (SyncWorker_9) Get Vehicles Response
2021-12-03 13:41:37 DEBUG (SyncWorker_9) Get Vehicles Response
2021-12-03 13:41:39 DEBUG (SyncWorker_2) Get Vehicles Response
2021-12-03 13:41:39 DEBUG (SyncWorker_7) Get Vehicles Response
2021-12-03 13:41:41 DEBUG (SyncWorker_1) Get Vehicles Response
2021-12-03 13:41:41 DEBUG (SyncWorker_3) Get Vehicles Response
2021-12-03 14:11:42 DEBUG (SyncWorker_3) Get Vehicles Response
2021-12-03 14:11:44 DEBUG (SyncWorker_4) Get Vehicles Response
2021-12-03 14:11:44 DEBUG (SyncWorker_2) Get Vehicles Response
2021-12-03 14:11:47 DEBUG (SyncWorker_7) Get Vehicles Response
2021-12-03 14:11:48 DEBUG (SyncWorker_5) Get Vehicles Response

I lot of different threads(?) doing the same request in clusters. (11:41 (2x), 12:11 (5x), 12:41 (5x), 13:11 (5x), 13:41 (5x), 14:11 (5x)).

Is it over subscribed? Should we try to detect a recent request + successful response?

Along those same lines, I have seen instances where some of the data returned is bad (my EV battery in the response was 0). Then in a subsequent request it seems to self correct (you can actually see an example of this in my post above with the battery graph - a quick down spike). My concern here is that if we do try to limit the number of requests, how can we assure that the response data is of quality?

@cdnninja
Copy link
Collaborator

cdnninja commented Dec 3, 2021

Well that is odd -is this debug logging or did you just truncate it? Typically it is configured to pull in the minutes not the seconds.

@rappazzo
Copy link
Contributor Author

rappazzo commented Dec 3, 2021

I trimmed the log down to only those calls and removed the response details. It is from the DEBUG level logging.

I can post the whole thing if you think that would help, but I would have to edit out the PII

@cdnninja
Copy link
Collaborator

cdnninja commented Dec 3, 2021

Is this with my PR integrated and was this happening before changes were made? Almost looks like an infinite loop of some form.

@rappazzo
Copy link
Contributor Author

rappazzo commented Dec 3, 2021

This is running the HyundaiBlueLinkAPUSA.py from your branch. Revision 5619c71.

I wouldn't say that it is an infinite loop. Just a loop of 5 iterations. Notice that each of the thread names is different

@cdnninja
Copy link
Collaborator

cdnninja commented Dec 3, 2021

Fair - It almost seems something else is missing from the logs or it is a different view. Typically other methods that call get vehicle response also have other log entries.

@rappazzo
Copy link
Contributor Author

rappazzo commented Dec 3, 2021

I totally trimmed it down. I'll post more details in a minute, but it looks like most of those calls is preceeded by get_cached_vehicle_status with the same thread name. Of course, but the code, that would make sense

@rappazzo
Copy link
Contributor Author

rappazzo commented Dec 3, 2021

Here is the full log (with all of the responses removed): kia_uvo.log

rappazzo pushed a commit to rappazzo/hacs-bluelink that referenced this issue Dec 5, 2021
…nect#196)

The location information provided in the cached vehicle status does not
include decimal precision.  In order to fix this, implement the get_location
method in the api for Hyundai USA.

Considering that there is a daily API limit, poll the get_location
method based on how fresh the data in the get_status response is.
@cdnninja
Copy link
Collaborator

cdnninja commented Dec 5, 2021

I think some of this is caused by having to call the vehicle command for both login and updating car status. Looks like the API also logs out so every time a scan occurs it has to login again.

rappazzo pushed a commit to rappazzo/hacs-bluelink that referenced this issue Dec 6, 2021
…nect#196)

The location information provided in the cached vehicle status does not
include decimal precision.  In order to fix this, implement the get_location
method in the api for Hyundai USA.

Considering that there is a daily API limit, poll the get_location
method based on whether or not the odometer changed.
rappazzo pushed a commit to rappazzo/hacs-bluelink that referenced this issue Dec 6, 2021
…nect#196)

The location information provided in the cached vehicle status does not
include decimal precision.  In order to fix this, implement the
get_location method in the api for Hyundai USA.

Considering that there is a daily API limit, poll the get_location
method based on whether or not the odometer changed.  It will also keep
track of the last time that the location was retrieved form the server,
and only fetch a new location if the last update was more than an hour
ago.
@rappazzo
Copy link
Contributor Author

rappazzo commented Dec 6, 2021

So, I have been running my get_location code for a day now, and I got rate limited (in the location query only). So I am implementing a time check to only check the location every hour in addition to the odometer check. I also added a log warning for when the rate limit response is given. I'll be testing this for a few days before submitting the pull request.

rappazzo pushed a commit to rappazzo/hacs-bluelink that referenced this issue Dec 6, 2021
…nect#196)

The location information provided in the cached vehicle status does not
include decimal precision.  In order to fix this, implement the
get_location method in the api for Hyundai USA.

Considering that there is a daily API limit, poll the get_location
method based on whether or not the odometer changed.  It will also keep
track of the last time that the location was retrieved form the server,
and only fetch a new location if the last update was more than an hour
ago.
@rappazzo
Copy link
Contributor Author

rappazzo commented Dec 7, 2021

I am looking into overfetching of the location data, and it seems like HA is either keeping multiple instances of the Hyundai/Kia component, or it is discarding and then recreating new instances (ie, instances are being garbage collected). My evidence of this is that that I see in multiple log entries that it doesn't have the previous vehicle state, nor does it have a copy of the stored timestamp of my last location update.

The odometer and timestamp checks do work in short term cases, but I think we would be better served if we could actually read the entity value history from HA....do you know how to do that?

(edit)
I moved the variables to be Class variables in the Hyundai USA implementation, and I'll see how that works out.

rappazzo pushed a commit to rappazzo/hacs-bluelink that referenced this issue Dec 8, 2021
…nect#196)

The location information provided in the cached vehicle status does not
include decimal precision.  In order to fix this, implement the
get_location method in the api for Hyundai USA.

Considering that there is a daily API limit, poll the get_location
method based on whether or not the odometer changed.  It will also keep
track of the last time that the location was retrieved form the server,
and only fetch a new location if the last update was more than an hour
ago.
rappazzo pushed a commit to rappazzo/hacs-bluelink that referenced this issue Dec 8, 2021
…nect#196)

The location information provided in the cached vehicle status does not
include decimal precision.  In order to fix this, implement the
get_location method in the api for Hyundai USA.

Considering that there is a daily API limit, poll the get_location
method based on whether or not the odometer changed.  It will also keep
track of the last time that the location was retrieved form the server,
and only fetch a new location if the last update was more than an hour
ago.
rappazzo pushed a commit to rappazzo/hacs-bluelink that referenced this issue Dec 8, 2021
…nect#196)

The location information provided in the cached vehicle status does not
include decimal precision.  In order to fix this, implement the
get_location method in the api for Hyundai USA.

Considering that there is a daily API limit, poll the get_location
method based on whether or not the odometer changed.  It will also keep
track of the last time that the location was retrieved form the server,
and only fetch a new location if the last update was more than an hour
ago.
rappazzo added a commit to rappazzo/hacs-bluelink that referenced this issue Dec 8, 2021
…nect#196)

The location information provided in the cached vehicle status does not
include decimal precision.  In order to fix this, implement the
get_location method in the api for Hyundai USA.

Considering that there is a daily API limit, poll the get_location
method based on whether or not the odometer changed.  It will also keep
track of the last time that the location was retrieved form the server,
and only fetch a new location if the last update was more than an hour
ago.
rappazzo added a commit to rappazzo/hacs-bluelink that referenced this issue Dec 8, 2021
…nect#196)

The location information provided in the cached vehicle status does not
include decimal precision.  In order to fix this, implement the
get_location method in the api for Hyundai USA.

Considering that there is a daily API limit, poll the get_location
method based on whether or not the odometer changed.  It will also keep
track of the last time that the location was retrieved form the server,
and only fetch a new location if the last update was more than an hour
ago.
@rappazzo
Copy link
Contributor Author

rappazzo commented Dec 9, 2021

OK, so the pull request is submitted for this, but there are still duplicate calls going on from (I presume) different threads.

2021-12-08 12:12:10 WARNING (SyncWorker_2) [custom_components.kia_uvo.HyundaiBlueLinkAPIUSA] kia_uvo - Unable to get vehicle location: {'errorSubCode': 'HT_533', 'systemName': 'HATA', 'functionName': 'findMyCar', 'errorSubMessage': 'HATA findMyCar service failed while performing the operation FindMyCar', 'errorMessage': 'Unable to send your request because a previous request is pending. Please wait and try again later.', 'errorCode': 502, 'serviceName': 'FindMyCar'}

Followed shortly by

2021-12-08 12:12:40 DEBUG (SyncWorker_0) [custom_components.kia_uvo.HyundaiBlueLinkAPIUSA] kia_uvo - Get Vehicle Location {'head': 46, 'coord': {'alt': 90.0, 'lon': XXX, 'type': 0, 'lat': YYY}, 'accuracy': {'pdop': 10, 'hdop': 5}, 'time': '2021-12-08T17:12:25Z', 'speed': {'unit': 1, 'value': 0}}

Would it make sense to use the double check idiom to execute the get_location code? Something like this:

if <it is OK to call get_location on the server>:
   with self.lock:
      if <it is OK to call get_location on the server>:
        do_get_location()

I would do this in a separate issue later.

@cdnninja
Copy link
Collaborator

If we have thread issues I am thinking it may be a bigger issue but this part gets over my head. I will watch my logs to see if I see any similar trends. lets open a new issue for it. We good to close this one?

@rappazzo
Copy link
Contributor Author

rappazzo commented Dec 11, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working USA
Projects
None yet
Development

No branches or pull requests

4 participants