mytoyota icon indicating copy to clipboard operation
mytoyota copied to clipboard

WIP: Lexus Work

Open GitOldGrumpy opened this issue 1 year ago • 15 comments

@CM000n and @ellvisss branch for working through getting a Lexus vehicle to work.

Currently contains all fixes we have found and a fix for the first pydantic error.

If we run and post the log files on errors, unless of course you can see the fix, then fix away.

GitOldGrumpy avatar Jan 24 '24 21:01 GitOldGrumpy

After modifing to imei: Optional[str] = None, I am receiving more information now (ServiceHistoryResponseModel, NotificationResponseModel, TelemetryResponseModel, LocationResponseModel, VehicleHealthResponseModel), After TripsResponseModel, it is showing only the last trip and right after I get this:

Traceback (most recent call last): File "C:\Temp\mytoyota-master\simple_client_example.py", line 88, in asyncio.run(get_information()) File "C:\Users\UserName\AppData\Local\Programs\Python\Python311\Lib\asyncio\runners.py", line 190, in run return runner.run(main) ^^^^^^^^^^^^^^^^ File "C:\Users\UserName\AppData\Local\Programs\Python\Python311\Lib\asyncio\runners.py", line 118, in run return self._loop.run_until_complete(task) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "C:\Users\UserName\AppData\Local\Programs\Python\Python311\Lib\asyncio\base_events.py", line 653, in run_until_complete return future.result() ^^^^^^^^^^^^^^^ File "C:\Temp\mytoyota-master\simple_client_example.py", line 79, in get_information Trips NameError: name 'Trips' is not defined

ellvisss avatar Jan 24 '24 21:01 ellvisss

Now my local time is after midnight, and when I tried again, I got this error for the Trips:

mytoyota.exceptions.ToyotaApiError: Request Failed. 400, {"status":{"messages":[{"responseCode":"400 BAD_REQUEST","description":"BAD_REQUEST","detailedDescription":"Invalid to date 2024-01-25. It should be in yyyy-MM-dd format and not a future date"}]}}.

ellvisss avatar Jan 24 '24 22:01 ellvisss

Now my local time is after midnight, and when I tried again, I got this error for the Trips:

mytoyota.exceptions.ToyotaApiError: Request Failed. 400, {"status":{"messages":[{"responseCode":"400 BAD_REQUEST","description":"BAD_REQUEST","detailedDescription":"Invalid to date 2024-01-25. It should be in yyyy-MM-dd format and not a future date"}]}}.

I think I understand this one, probably something to do with different time zones and not being able to request a "future" date. Will need looking into.

GitOldGrumpy avatar Jan 24 '24 22:01 GitOldGrumpy

After modifing to imei: Optional[str] = None, I am receiving more information now (ServiceHistoryResponseModel, NotificationResponseModel, TelemetryResponseModel, LocationResponseModel, VehicleHealthResponseModel), After TripsResponseModel, it is showing only the last trip and right after I get this:

Traceback (most recent call last): File "C:\Temp\mytoyota-master\simple_client_example.py", line 88, in asyncio.run(get_information()) File "C:\Users\UserName\AppData\Local\Programs\Python\Python311\Lib\asyncio\runners.py", line 190, in run return runner.run(main) ^^^^^^^^^^^^^^^^ File "C:\Users\UserName\AppData\Local\Programs\Python\Python311\Lib\asyncio\runners.py", line 118, in run return self._loop.run_until_complete(task) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "C:\Users\UserName\AppData\Local\Programs\Python\Python311\Lib\asyncio\base_events.py", line 653, in run_until_complete return future.result() ^^^^^^^^^^^^^^^ File "C:\Temp\mytoyota-master\simple_client_example.py", line 79, in get_information Trips NameError: name 'Trips' is not defined

This seems odd. Have you un-commented any lines. Line 79 is a comment line that should be # Trips?

GitOldGrumpy avatar Jan 24 '24 22:01 GitOldGrumpy

Codecov Report

Attention: Patch coverage is 63.63636% with 8 lines in your changes missing coverage. Please review.

Project coverage is 70.09%. Comparing base (43c8189) to head (d7e4b2d). Report is 31 commits behind head on master.

Files Patch % Lines
mytoyota/controller.py 12.50% 7 Missing :warning:
mytoyota/client.py 50.00% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #316      +/-   ##
==========================================
- Coverage   70.35%   70.09%   -0.26%     
==========================================
  Files          32       32              
  Lines        1511     1518       +7     
==========================================
+ Hits         1063     1064       +1     
- Misses        448      454       +6     
Flag Coverage Δ
unittests 70.09% <63.63%> (-0.26%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jan 25 '24 10:01 codecov[bot]

@CM000n what's the thinking for adding the brand to each of the endpoint calls? I don't think it can change once you are connected as the vehicles you get back will only be of the specific brand. Is this not something we could put on the initialiser of the API class?

GitOldGrumpy avatar Jan 25 '24 11:01 GitOldGrumpy

@CM000n what's the thinking for adding the brand to each of the endpoint calls? I don't think it can change once you are connected as the vehicles you get back will only be of the specific brand. Is this not something we could put on the initialiser of the API class?

Good point @GitOldGrumpy. I thought this was also required in the headers of the respective API calls? If we really only need it for the initial call of the vehicle, the whole thing could of course be simplified. But I think we should at least parameterise it so that the desired brand can be passed when calling get_vehicles().

CM000n avatar Jan 25 '24 14:01 CM000n

Its needed for all calls to any endpoint but only needs to be set the once so it could be set when the API class is constructed as opposed to on each API call? This simplifies all the endpoint calls.

There is one downside if a user has both Toyota's and Lexi then they will need too construct two API classes one for each brand. This will work as the first will login and the second will use the cached credentials. I think this is still preferable to having to specifying the brand in each API endpoint call as for all but this case you will always being passing the same value.

GitOldGrumpy avatar Jan 25 '24 14:01 GitOldGrumpy

Its needed for all calls to any endpoint but only needs to be set the once so it could be set when the API class is constructed as opposed to on each API call? This simplifies all the endpoint calls.

There is one downside if a user has both Toyota's and Lexi then they will need too construct two API classes one for each brand. This will work as the first will login and the second will use the cached credentials. I think this is still preferable to having to specifying the brand in each API endpoint call as for all but this case you will always being passing the same value.

That gives me an idea. The current way of caching (we store it as toyota_credentials_cache_contains_secrets file in the home directory) could possibly become a problem if the usere uses different accesses for his Toyota and Lexus vehicles, right?

CM000n avatar Jan 25 '24 18:01 CM000n

Its needed for all calls to any endpoint but only needs to be set the once so it could be set when the API class is constructed as opposed to on each API call? This simplifies all the endpoint calls. There is one downside if a user has both Toyota's and Lexi then they will need too construct two API classes one for each brand. This will work as the first will login and the second will use the cached credentials. I think this is still preferable to having to specifying the brand in each API endpoint call as for all but this case you will always being passing the same value.

That gives me an idea. The current way of caching (we store it as toyota_credentials_cache_contains_secrets file in the home directory) could possibly become a problem if the usere uses different accesses for his Toyota and Lexus vehicles, right?

Yes you're correct if they have different email addresses across the apps then they will get different tokens. In my test case I was using the same details across both.

GitOldGrumpy avatar Jan 25 '24 18:01 GitOldGrumpy

After modifing to imei: Optional[str] = None, I am receiving more information now (ServiceHistoryResponseModel, NotificationResponseModel, TelemetryResponseModel, LocationResponseModel, VehicleHealthResponseModel), After TripsResponseModel, it is showing only the last trip and right after I get this:

Traceback (most recent call last): File "C:\Temp\mytoyota-master\simple_client_example.py", line 88, in asyncio.run(get_information()) File "C:\Users\UserName\AppData\Local\Programs\Python\Python311\Lib\asyncio\runners.py", line 190, in run return runner.run(main) ^^^^^^^^^^^^^^^^ File "C:\Users\UserName\AppData\Local\Programs\Python\Python311\Lib\asyncio\runners.py", line 118, in run return self._loop.run_until_complete(task) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "C:\Users\UserName\AppData\Local\Programs\Python\Python311\Lib\asyncio\base_events.py", line 653, in run_until_complete return future.result() ^^^^^^^^^^^^^^^ File "C:\Temp\mytoyota-master\simple_client_example.py", line 79, in get_information Trips NameError: name 'Trips' is not defined

This seems odd. Have you un-commented any lines. Line 79 is a comment line that should be # Trips?

Yes, sorry, I think I un-commented it by mistake. Now it works with no error.

ellvisss avatar Jan 26 '24 16:01 ellvisss

Yes you're correct if they have different email addresses across the apps then they will get different tokens. In my test case I was using the same details across both.

I'm really excited about how we want to design it. 😊 Maybe we should write it down/record it?

My idea would be that a user does not have to specify the brand, but we use the get_vehicles_endpoint(self) endpoint to determine the vehicles for both Toyota and Lexus and combine them in the returned model first. We then decide which endpoint headers to use for further requests, depending on the brand present in the VehiclesResponseModel.

Do you already have an idea of how we want to deal with multi-accounting and caching? In the issues of the Home Assistant integration there are also issues from people who have authorization problems when the lib tries to create a cache folder + file in the home directory of the process user.

CM000n avatar Jan 27 '24 11:01 CM000n

My thoughts around brands:

  • Do we want to combine them? It seems that users with both Lexi & Toyota's will be a fairly niche use case. Is it worth the extra complications to support given users can easily create two API instances? (Once we sort the issue with cache file). How does the effect home assistant? I think at the moment it only supports 1 car?
  • For the user to not specify the brand we would have to hit the endpoint twice, first with the brand set to "T" then to "L" the apps would never do this I have a small concern this could get us noticed as we are using the API not how its intended. The other issue is for users with only one brand we are always making the call twice even though its not required.
  • For users that do have both brands how do we deal with the username/password when they are different? They will need to pass us both with the brand information, which sort of negates the auto determination? (We could still make it work without the brand information but it would mean more calls to the get_vehicle_endpoint.)

My thoughts are credential caching:

  • It would appear we need to cache per brand, though we don't know the brand(unless told) until after the call to get_vehicle_endpoint.
  • On the HA issue its not bad if it fails to cache as the API is only constructed the once? If this is the case we could have a flag for DONT cache or just log if the cache file could not be created.
  • Does HA have a cache location for such things? Can we write into the custom_component/??? folder. We could always cache to /tmp?

It seems to me that it is easier to keep the API single branded which we are told, although easier isn't always correct.

GitOldGrumpy avatar Jan 27 '24 13:01 GitOldGrumpy

My thoughts around brands:

  • Do we want to combine them? It seems that users with both Lexi & Toyota's will be a fairly niche use case. Is it worth the extra complications to support given users can easily create two API instances? (Once we sort the issue with cache file). How does the effect home assistant? I think at the moment it only supports 1 car?

You're right, it's usually better to make the information explicit rather than implicit. After thinking about it again I think most people would expect the same behaviour as when using two different apps. With regard to Home Assitant, it should already be possible to specify several accounts. For each account, all associated vehicles should then actually be created as a device with their respective entities (sensors). Unfortunately, I haven't been able to test it yet, as I only have one vehicle myself and writing tests for Home Assistant custom components is a PITA

To be honest, I don't quite understand how you want to use the specified brand once when constructing the API class, instead of adding it to the endpoint calls. But I'm happy to be persuaded. 😊

  • For the user to not specify the brand we would have to hit the endpoint twice, first with the brand set to "T" then to "L" the apps would never do this I have a small concern this could get us noticed as we are using the API not how its intended. The other issue is for users with only one brand we are always making the call twice even though its not required.

I don't think that makes any difference or makes the situation worse. 😉 If anyone at Toyota takes a closer look, we are already conspicuous. Currently, the Home Assistant integration queries the API every 5 minutes. I don't think the apps do that with such regularity.

  • For users that do have both brands how do we deal with the username/password when they are different? They will need to pass us both with the brand information, which sort of negates the auto determination? (We could still make it work without the brand information but it would mean more calls to the get_vehicle_endpoint.)

As written under point 1, you are probably right. People would expect similar behaviour here as with different apps.

My thoughts are credential caching:

  • It would appear we need to cache per brand, though we don't know the brand(unless told) until after the call to get_vehicle_endpoint.

Wouldn't it be easiest to create a cache file with a name based on the user's email address? This would provide a clear distinction. Or is it possible to log in to Lexus and Toyota with the same e-mail address and then get different accounts?

  • On the HA issue its not bad if it fails to cache as the API is only constructed the once? If this is the case we could have a flag for DONT cache or just log if the cache file could not be created.

That would be a possible emergency solution

  • Does HA have a cache location for such things? Can we write into the custom_component/??? folder. We could always cache to /tmp?

To be honest, I don't know. The diverse installation options of HA make it difficult to determine how the respective user system is currently behaving.

It seems to me that it is easier to keep the API single branded which we are told, although easier isn't always correct.

👍🏻

CM000n avatar Jan 27 '24 21:01 CM000n

guys, i'd like to support, e.g. with testing. i have lexus rx. feel free to come back to me

nrpetonr avatar Apr 21 '24 16:04 nrpetonr