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

check_and_force_update_vehicles not refreshing data #425

Closed
Jesus-M opened this issue Oct 13, 2023 · 7 comments
Closed

check_and_force_update_vehicles not refreshing data #425

Jesus-M opened this issue Oct 13, 2023 · 7 comments

Comments

@Jesus-M
Copy link

Jesus-M commented Oct 13, 2023

  • Hyundai / Kia Connect version: 3.10.1
  • Python version: 3.9.18
  • Operating System: Slackware Linux -current

Description

The method check_and_force_update_vehicles is not doing what expected, not refreshing the data (or I don't know how to use it).

I am trying to get an updated status of the car if the last update is over 30 minutes old, but I am getting always the old cached information.

What I Did

I built a small script to get the car status:

vm = VehicleManager(region=1, brand=2, username="xxxxx", password="xxxxx", pin="xxxx")
vm.check_and_refresh_token()
vm.check_and_force_update_vehicles(1800)
vm.update_all_vehicles_with_cached_state()

kona = vm.get_vehicle('xxxxxxxxxxxxxxxxxxxxxxx')

now = datetime.now().astimezone().strftime("%Y-%m-%d %H:%M:%S %z")

print()
print(f"Current time: {now}")
print(f"Last Updated: {kona.last_updated_at}")
print()
print(f"CarId: {kona.id}, last updated on {kona.last_updated_at}")
print(f"Car name ({kona.name}) and VIN ({kona.VIN})")

But the result is not the expected, and clearly it is over 30 minutes since last update (actually, over 90 minutes):

Current time: 2023-10-13 16:45:50 +0100
Last Updated: 2023-10-13 16:12:48+02:00

CarId: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx, last updated on 2023-10-13 16:12:48+02:00
Car name (KONA) and VIN (vvvvvvvvvvvvvvvvv)

If I try to forcing the refresh without the check, it works:

vm = VehicleManager(region=1, brand=2, username="xxxxx", password="xxxxx", pin="xxxx")
vm.check_and_refresh_token()
vm.force_refresh_all_vehicles_states()
vm.update_all_vehicles_with_cached_state()

kona = vm.get_vehicle('xxxxxxxxxxxxxxxxxxxxxxx')

now = datetime.now().astimezone().strftime("%Y-%m-%d %H:%M:%S %z")

print()
print(f"Current time: {now}")
print(f"Last Updated: {kona.last_updated_at}")
print()
print(f"CarId: {kona.id}, last updated on {kona.last_updated_at}")
print(f"Car name ({kona.name}) and VIN ({kona.VIN})")
Current time: 2023-10-13 16:47:10 +0100
Last Updated: 2023-10-13 17:47:05+02:00

CarId: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx, last updated on 2023-10-13 17:47:05+02:00
Car name (KONA) and VIN (vvvvvvvvvvvvvvvvv)

I saw at the issue #412 that check_and_force_update_vehicles doesn't refresh the VM object, that's why I am making a call to update_all_vehicles_with_cached_state after it, but still it is showing the old information.

I may be missing something, but I cannot find what.

@cdnninja
Copy link
Collaborator

Does it work in the mobile app? If not you may have used the max force calls for the day. I have heard it may only be 10 a day now.

@Jesus-M
Copy link
Author

Jesus-M commented Oct 13, 2023

Not only it works on the phone app, it also works when I use the force refresh without the check.

@Jesus-M
Copy link
Author

Jesus-M commented Oct 15, 2023

ok, I wasn't seeing anything in the debugger, so I added some extra debug lines (simple 'prints') to the code:

def check_and_force_update_vehicles(self, force_refresh_interval: int) -> None:
    # Force refresh only if current data is older than the value bassed in seconds.
    # Otherwise runs a cached update.
    started_at_utc: dt = dt.datetime.now(pytz.utc)
    print(f"*** started_at_utc: {started_at_utc}")
    for vehicle_id in self.vehicles.keys():
        vehicle = self.get_vehicle(vehicle_id)
        print(f"*** Vehicle: ({vehicle.id}) / {vehicle.last_updated_at}")
        if vehicle.last_updated_at is not None:
            _LOGGER.debug(
                f"{DOMAIN} - Time differential in seconds: {(started_at_utc - vehicle.last_updated_at).total_seconds()}"  # noqa
            )
            if (
                started_at_utc - vehicle.last_updated_at
            ).total_seconds() > force_refresh_interval:
                print(f"*** Vehicle: ({vehicle.id}) / {vehicle.last_updated_at} - updating")
                self.force_refresh_vehicle_state(vehicle_id)
            else:
                print(f"*** Vehicle: ({vehicle.id}) / {vehicle.last_updated_at} - NOT updating")
                self.update_vehicle_with_cached_state(vehicle_id)
        else:
            print(f"*** Vehicle: ({vehicle.id}) / {vehicle.last_updated_at} - NOT updating due to last_updated not defined")
            self.update_vehicle_with_cached_state(vehicle_id)

And the output was:

*** started_at_utc: 2023-10-15 08:31:18.516215+00:00
*** Vehicle: (xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx) / None
*** Vehicle: (xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx) / None - NOT updatingi due to last_updated not defined

So I realized that the update_all_vehicles_with_cached_state needs to be called BEFORE and AFTER check_and_force_update_vehicles

  • BEFORE, so we 'know' when the last update was.
  • AFTER, so we update the cache and we have the updated readings.

Ant it worked:

*** started_at_utc: 2023-10-15 08:35:03.113173+00:00
*** Vehicle: (xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx) / 2023-10-15 08:51:41+02:00
*** Vehicle: (xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx) / 2023-10-15 08:51:41+02:00 - updating

So, if I understood well the logic of check_and_force_update_vehicles:

  • if the refresh_interval has passed, it forces an update of the car info in the server side (updates the cache), but we need then to update our object info from the cache.
  • if the refresh_interval hasn't passed, it just updates the object in the client side with the cached information.

So, since we need to update from cache before using the check_and_update, we have two scenarios:

  1. refresh_interval not passed: three calls to update_all_vehicles_with_cached_state (before we use the check_and_force_update_vehicles method, IN the check_and_force_update_vehicles method, and after we called check_and_force_update_vehicles -this last one, unnecessary, but we have to because we don't know if it will be needed).
  2. refresh_interval not passed: two calls to update_all_vehicles_with_cached_state (before we use the check_and_force_update_vehicles method and after we called check_and_force_update_vehicles .

So, I wonder whether it would more efficient to make a call to update_all_vehicles_with_cached_state ALWAYS from within check_and_force_update_vehicles. At least we would be saving one of the calls in scenario 1. I would even argue that probably it would be better to also add the initial call to update_all_vehicles_with_cached_state in the check_and_force_update_vehicles, so it would be transparent to the user.

Thoughts?

@cdnninja
Copy link
Collaborator

I sat to read this a bit more. Couple things to note:

  1. You can't get data every 30 minutes you will one max your API count and two kill your 12v battery.
  2. check_and_force_update is suppose to be a safe check. I think it is working as designed. If you don't want a safe check use "force_refresh_all_vehicles_states"

The API covers multiple use cases. In some situations this library is active for months at a time and not restarted regularly. As such you don't hit the no data situation. Seems like in your case you aren't keeping it running long term. Home assistant is the example of this.

@Jesus-M
Copy link
Author

Jesus-M commented Oct 15, 2023

Hi,

I'm not and don't want to run a full refresh every 30 minutes, just wanted to be sure that when I run it and get the data, it is updated, so no older than 30 minutes.

But anyway, ok, thanks for your comments.

@cdnninja
Copy link
Collaborator

Got it. So if that's the case from your side I would have your code call a get cached call first followed by an if statement to determine if the data is too old for your requirements.

If it is call a force refresh which should update the vehicle object.

@Jesus-M
Copy link
Author

Jesus-M commented Oct 15, 2023

So basically implementing the 'check and refresh' in my code instead of using the 'standard' method.

Gotcha.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants