skodaconnect / homeassistant-myskoda

Homeassistant integration for MySkoda.
4 stars 2 forks source link

Consider limitting outstanding requests to 1 #5

Open dvx76 opened 5 days ago

dvx76 commented 5 days ago

Currently requests, e.g. to change the charge limit, will just queue and end up all executing. The 'problem' is that these requests can take a lot of time (10's of seconds, used to be more like minutes in the past).

E.g. if you change the charge limit to 90 and then to 80, you'll get something like this:

2024-09-15 13:44:33.669 INFO (MainThread) [myskoda.rest_api] Setting charge limit to 90.0
2024-09-15 13:44:48.539 INFO (MainThread) [myskoda.rest_api] Setting charge limit to 80.0
2024-09-15 13:44:50.141 DEBUG (MainThread) [custom_components.myskoda] Finished fetching myskoda data in 1.082 seconds (success: True)
2024-09-15 13:45:07.408 DEBUG (MainThread) [custom_components.myskoda] Finished fetching myskoda data in 2.264 seconds (success: True)
2024-09-15 13:45:08.801 DEBUG (MainThread) [custom_components.myskoda] Finished fetching myskoda data in 5.156 seconds (success: True)
2024-09-15 13:45:25.169 DEBUG (MainThread) [custom_components.myskoda] Finished fetching myskoda data in 2.758 seconds (success: True)
2024-09-15 13:45:26.552 DEBUG (MainThread) [custom_components.myskoda] Finished fetching myskoda data in 2.748 seconds (success: True)
2024-09-15 13:45:43.151 DEBUG (MainThread) [custom_components.myskoda] Finished fetching myskoda data in 2.979 seconds (success: True)
2024-09-15 13:45:44.485 DEBUG (MainThread) [custom_components.myskoda] Finished fetching myskoda data in 2.931 seconds (success: True)
2024-09-15 13:45:58.155 DEBUG (MainThread) [custom_components.myskoda.number] Changed charge limit to 90.0.
2024-09-15 13:46:02.314 DEBUG (MainThread) [custom_components.myskoda] Finished fetching myskoda data in 2.827 seconds (success: True)
2024-09-15 13:46:18.186 DEBUG (MainThread) [custom_components.myskoda] Finished fetching myskoda data in 0.868 seconds (success: True)
2024-09-15 13:46:35.144 DEBUG (MainThread) [custom_components.myskoda] Finished fetching myskoda data in 1.955 seconds (success: True)
2024-09-15 13:46:51.299 DEBUG (MainThread) [custom_components.myskoda] Finished fetching myskoda data in 1.153 seconds (success: True)
2024-09-15 13:47:08.281 DEBUG (MainThread) [custom_components.myskoda] Finished fetching myskoda data in 1.979 seconds (success: True)
2024-09-15 13:47:27.105 DEBUG (MainThread) [custom_components.myskoda] Finished fetching myskoda data in 3.819 seconds (success: True)
2024-09-15 13:47:47.610 DEBUG (MainThread) [custom_components.myskoda] Finished fetching myskoda data in 5.503 seconds (success: True)
2024-09-15 13:47:47.611 DEBUG (MainThread) [custom_components.myskoda.number] Changed charge limit to 80.0.

The old integration would cap the number of parallel requests to 1 I believe. Will try to find a reference.

We may also want to relax the poll frequency in the set_ methods (currently 15s) to avoid getting rate limited. Also wondering if we could update just the entity being (instead of everything, which I understand is what coordinator.async_refresh() will do.

I'm also curious what the best-practice is for HA when updating number entities where the execution of the update request is high asynchronous like here.

dvx76 commented 5 days ago

https://github.com/skodaconnect/skodaconnect/blob/75088af7795aa049dafb8442bf9dc0b5d817cdc7/skodaconnect/vehicle.py#L443C23-L443C54

        if self._requests['batterycharge'].get('id', False):
            timestamp = self._requests.get('batterycharge', {}).get('timestamp', datetime.now())
            expired = datetime.now() - timedelta(minutes=3)
            if expired > timestamp:
                self._requests.get('batterycharge', {}).pop('id')
            else:
                raise SkodaRequestInProgressException('Charging action already in progress')

If I'm not mistaken the Vehicle class keeps a map of outstanding requests by 'type' (e.g. batterycharge here) and if a previous requests is outstanding, the new request is rejected.

Prior99 commented 5 days ago

I will integrate the MQTT server that Skoda is using to push updates to the app in the next days, and then we don't need to poll anymore at all :)

On Sun, Sep 15, 2024, 14:45 Fabrice Devaux @.***> wrote:

https://github.com/skodaconnect/skodaconnect/blob/75088af7795aa049dafb8442bf9dc0b5d817cdc7/skodaconnect/vehicle.py#L443C23-L443C54

    if self._requests['batterycharge'].get('id', False):
        timestamp = self._requests.get('batterycharge', {}).get('timestamp', datetime.now())
        expired = datetime.now() - timedelta(minutes=3)
        if expired > timestamp:
            self._requests.get('batterycharge', {}).pop('id')
        else:
            raise SkodaRequestInProgressException('Charging action already in progress')

If I'm not mistaken the Vehicle class keeps a map of outstanding requests by 'type' (e.g. batterycharge here) and if a previous requests is outstanding, the new request is rejected.

— Reply to this email directly, view it on GitHub https://github.com/skodaconnect/homeassistant-myskoda/issues/5#issuecomment-2351577373, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALME5RYZTPVPC6KSC3P5ATZWV6M5AVCNFSM6AAAAABOHZB6BOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJRGU3TOMZXGM . You are receiving this because you are subscribed to this thread.Message ID: @.***>