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

Revert "Keep UTC time zone attached to datetime objects when API retu… #109

Merged
merged 1 commit into from
Dec 11, 2023

Conversation

dvx76
Copy link
Member

@dvx76 dvx76 commented Dec 11, 2023

@badrpc FYI - have to revert this so we can safely release a fix for the user-agent

This reverts commit 02c9c84.

Throws the following exception on Enyaq for Webspider:

Traceback (most recent call last):
  File "/home/nivo/dev/skodaconnect/example/me.py", line 358, in <module>
    loop.run_until_complete(main())
  File "/usr/lib/python3.10/asyncio/base_events.py", line 649, in run_until_complete
    return future.result()
  File "/home/nivo/dev/skodaconnect/example/me.py", line 247, in main
    print(f'\tstr_state: {instrument.str_state} - state: {instrument.state}')
  File "/home/nivo/dev/skodaconnect/skodaconnect/dashboard.py", line 123, in str_state
    return f'{self.state}'
  File "/home/nivo/dev/skodaconnect/skodaconnect/dashboard.py", line 127, in state
    val = super().state
  File "/home/nivo/dev/skodaconnect/skodaconnect/dashboard.py", line 58, in state
    if hasattr(self.vehicle, self.attr):
  File "/home/nivo/dev/skodaconnect/skodaconnect/vehicle.py", line 1542, in last_connected
    last_connected = datetime.fromisoformat(last_connected_utc)
ValueError: Invalid isoformat string: '2023-12-11T15:34:24.425Z'

…rns timestamps in UTC."

This reverts commit 02c9c84.

Throws the following exception on Enyaq for Webspider:

```
Traceback (most recent call last):
  File "/home/nivo/dev/skodaconnect/example/me.py", line 358, in <module>
    loop.run_until_complete(main())
  File "/usr/lib/python3.10/asyncio/base_events.py", line 649, in run_until_complete
    return future.result()
  File "/home/nivo/dev/skodaconnect/example/me.py", line 247, in main
    print(f'\tstr_state: {instrument.str_state} - state: {instrument.state}')
  File "/home/nivo/dev/skodaconnect/skodaconnect/dashboard.py", line 123, in str_state
    return f'{self.state}'
  File "/home/nivo/dev/skodaconnect/skodaconnect/dashboard.py", line 127, in state
    val = super().state
  File "/home/nivo/dev/skodaconnect/skodaconnect/dashboard.py", line 58, in state
    if hasattr(self.vehicle, self.attr):
  File "/home/nivo/dev/skodaconnect/skodaconnect/vehicle.py", line 1542, in last_connected
    last_connected = datetime.fromisoformat(last_connected_utc)
ValueError: Invalid isoformat string: '2023-12-11T15:34:24.425Z'
```
@dvx76 dvx76 merged commit ae87d8e into skodaconnect:main Dec 11, 2023
@badrpc
Copy link
Contributor

badrpc commented Dec 12, 2023

Oh, I'm really sorry for the troubles.

However I must say that I have asked about this on the original pull request:

Important: this will only work on Python 3.11 and higher because prior to 3.11 this method only supported formats that could be emitted by date.isoformat() or datetime.isoformat(). In Python 3.9 for example this code will fail to parse the timestamp returned by the API. I can replace this with datetime.strptime(last_connected_utc,'%Y-%m-%dT%H:%M:%SZ').astimezone(timezone.utc) which works in older versions. I'm not sure what is the expectation of Python version for this library.

I'm quite certain that problem is not with a model but with python version:

Python 3.9:

% python3.9
Python 3.9.14 (main, Sep 20 2022, 01:16:05) 
[Clang 13.0.0 (git@github.com:llvm/llvm-project.git llvmorg-13.0.0-0-gd7b669b3a on freebsd13
Type "help", "copyright", "credits" or "license" for more information.
>>> from datetime import datetime
>>> datetime.fromisoformat("2023-12-11T15:34:24.425Z")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: Invalid isoformat string: '2023-12-11T15:34:24.425Z'
>>> 

Python 3.11:

% python3
Python 3.11.5 (main, Aug 29 2023, 15:31:31) [GCC 13.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from datetime import datetime
>>> datetime.fromisoformat("2023-12-11T15:34:24.425Z")
datetime.datetime(2023, 12, 11, 15, 34, 24, 425000, tzinfo=datetime.timezone.utc)
>>>

Shall I implement this in a way that's compatible with Python before 3.11?

@dvx76
Copy link
Member Author

dvx76 commented Dec 12, 2023

@WebSpider did you get an error on a Python 3.9 env?

If that's the case and it all works fine on 3.11 then I think you can just resubmit the exact same fix. HA Core depends on 3.11 now anyway (https://github.com/home-assistant/core/blob/dev/pyproject.toml#L24). Sorry we weren't quite sure yesterday and had to get a fix out for the updated user-agent.

@WebSpider
Copy link
Contributor

I will double check, I don't know what my python version in the venv was at that point.

@badrpc
Copy link
Contributor

badrpc commented Dec 12, 2023

No worries, I can totally understand it.

FYI: 3.10 is also expected to have the same error - just checked. And according to https://docs.python.org/3/library/datetime.html#datetime.datetime.fromisoformat 3.11 is the first version which support the required format.

@WebSpider
Copy link
Contributor

Yep. It was python version related. The exception is solved when using 3.11.

However, the date conversion does not seem correct to me:

DEBUG:skodaconnect.connection:Request for "https://api.connect.skoda-auto.cz/api/v2/vehicle-status/MYVIN" returned with status code [200], response: {'remote': {'capturedAt': '2023-12-12T09:22:05.583Z', 'status': {'open': 'NO', 'locked': 'YES'}, 'mileageInKm': 60000.1, 'doors': [{'name': 'BONNET', 'status': 'CLOSED'}, {'name': 'TRUNK', 'status': 'LOCKED'}, {'name': 'REAR_RIGHT', 'status': 'LOCKED'}, {'name': 'REAR_LEFT', 'status': 'LOCKED'}, {'name': 'FRONT_RIGHT', 'status': 'LOCKED'}, {'name': 'FRONT_LEFT', 'status': 'LOCKED'}], 'windows': [{'name': 'SUN_ROOF', 'status': 'CLOSED'}, {'name': 'ROOF_COVER', 'status': 'UNSUPPORTED'}, {'name': 'SUN_ROOF_REAR', 'status': 'UNSUPPORTED'}, {'name': 'FRONT_LEFT', 'status': 'CLOSED'}, {'name': 'FRONT_RIGHT', 'status': 'CLOSED'}, {'name': 'REAR_LEFT', 'status': 'CLOSED'}, {'name': 'REAR_RIGHT', 'status': 'CLOSED'}], 'lights': {'overallStatus': 'OFF', 'lightsStatus': [{'name': 'RIGHT', 'status': 'OFF'}, {'name': 'LEFT', 'status': 'OFF'}]}}, 'local': None, 'errors': []}

Results in the dashboard in:

MYVIN Request results - (request_results)
        str_state: N/A - state: N/A
        attributes: {'latest': 'N/A', 'state': 'N/A', 'departuretimer': 'N/A', 'departuretimer_timestamp': '1970-01-01T00:00:00', 'batterycharge': 'N/A', 'batterycharge_timestamp': '1970-01-01T00:00:00', 'air-conditioning': 'N/A', 'air-conditioning_timestamp': '1970-01-01T00:00:00', 'refresh': 'N/A', 'refresh_timestamp': '1970-01-01T00:00:00', 'lock': 'N/A', 'lock_timestamp': '1970-01-01T00:00:00'}

MYVIN Door locked - (door_locked)
        str_state: Locked - state: True
        attributes: {'last_result': 'N/A', 'last_timestamp': '1970-01-01T00:00:00'}

So the time for some reason is set to the beginning of the epoch, and not to the value of capturedAt?

@badrpc
Copy link
Contributor

badrpc commented Dec 12, 2023

On the surface this looks completely unrelated to my change. I only updated two specific sensors: last_connected and parking_time - these should be visible in Home Assistant as sensors, not as attributes of entities. I don't know where this attributes are coming from, is this not even something maintained internally by Home Assistant? I only have these attributes on a few entities and N/A in latest or last_result attributes makes me think that maybe integration has never provided values for these entities?

@badrpc
Copy link
Contributor

badrpc commented Dec 13, 2023

I looked a bit more into these "N/A" and "1970-01-01T00:00:00". At least some of them seem to be related to actions initiated from Home Assistant. For example I have just triggered "Force data refresh" and now for request_results I have

latest: Refresh
state: Success
batterycharge: N/A
batterycharge_timestamp: 1970-01-01T00:00:00
refresh: Success
refresh_timestamp: 2023-12-13T09:56:12
lock: N/A
lock_timestamp: 1970-01-01T00:00:00
model: Škoda Scala Ambition/2019-11-12
assumed_state: true
icon: mdi:chat-alert
friendly_name: My Skoda Request results

Before triggering "Force data refresh" I had "N/A" in latest and refresh and "1970-01-01T00:00:00" in refresh_timestamp. I'm not sure how exactly this works, but I don't think this is in any way related to last_connected and parking_time sensors I was changing in 02c9c84.

I will prepare another PR identical to 02c9c84 later today.

dvx76 added a commit to dvx76/skodaconnect that referenced this pull request Apr 13, 2024
Align with requirements.txt. Required for skodaconnect#109. Note that HA-core
already depends on 3.12.

Fixes skodaconnect#123
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

Successfully merging this pull request may close these issues.

None yet

3 participants