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

Fix "setup Connected Services Error" due to latest API Changes #240

Merged
merged 10 commits into from
Nov 12, 2023

Conversation

CM000n
Copy link
Collaborator

@CM000n CM000n commented Nov 9, 2023

Possible fix for DurgNomis-drol/ha_toyota#164

It seems that the previous API endpoint will continue to be used for the basic car information even after the switch to the new MyToyota app (until now).
However, the structure of the returned data has apparently changed, which is why the check whether Connected Services are activated fails.
In the case of my Toyota Corolla, the information about the Connected Service now looks like this:

{'alias': 'Corolla',
 'claimedBy': 'CUSTOMER',
 'connected': False,
 'connectedService': {'devices': [{'brand': 'TOYOTA',
                                   'countryOfUsage': 'DE',
                                   'customerId': 'my-secret-customer-id',
                                   'deviceId': 'my-secret-device-id',
                                   'linkedWithCustomer': True,
                                   'privacy': False,
                                   'provider': 'DCM19',
                                   'state': 'ACTIVE',
                                   'subState': 'ACTIVE',
                                   'vin': 'my-secret-vin'}],
                      'registrationPreReqs': [],
                      'registrationTerms': [],
                      'status': 'DISABLED',
                      'wifiBoxData': None},
 'entitledBy': 'CUSTOMER',
 'entitledOn': '2023-10-31T16:56:02.541Z',
 'fud': False,
 'hasAutomaticMMRegistration': False,
 'id': my-secret-id,
 'isEntitled': True,
 'isNC': False,
 'isOneAppMigrated': True,
 'isRentalVehicle': False,
 'owner': True,
 'ownerFlag': True,
 'source': 'NMSC',
 'startDate': '2023-10-31T16:56:02.541Z',
 'vehicleAddedOn': '2020-11-19T17:36:19.469Z',
 'vin': 'my-secret-vin'}

This change requires that the query for activated connected services be adjusted to determine whether there is a connected service device with an activated status whose Vin matches the vehicle's Vin.

To be discussed: Perhaps checking for an activated device with a suitable Vin is not sufficient? Information from other users would be desirable. Maybe it also makes sense to skip checking whether connected services are activated entirely?

DurgNomis-drol
DurgNomis-drol previously approved these changes Nov 9, 2023
Copy link
Owner

@DurgNomis-drol DurgNomis-drol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Can you confirm this works? 🚀

@CM000n
Copy link
Collaborator Author

CM000n commented Nov 9, 2023

LGTM! Can you confirm this works? 🚀

yes, that works (in my case). But as already written, I can't really validate whether this is a sufficient check, whether the connected services are set up, or whether it doesn't check something completely different now.

Copy link

codecov bot commented Nov 10, 2023

Codecov Report

Merging #240 (c4c8a1a) into master (5245d94) will decrease coverage by 0.24%.
Report is 1 commits behind head on master.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #240      +/-   ##
==========================================
- Coverage   94.64%   94.41%   -0.24%     
==========================================
  Files          30       30              
  Lines        1607     1612       +5     
==========================================
+ Hits         1521     1522       +1     
- Misses         86       90       +4     
Flag Coverage Δ
unittests 94.41% <100.00%> (-0.24%) ⬇️

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

Files Coverage Δ
mytoyota/models/vehicle.py 96.47% <100.00%> (-2.28%) ⬇️
tests/test_hvac.py 100.00% <ø> (ø)
tests/test_myt.py 100.00% <ø> (ø)
tests/test_myt_online.py 100.00% <ø> (ø)
tests/test_parking_location.py 100.00% <ø> (ø)
tests/test_sensors.py 100.00% <ø> (ø)
tests/test_statistics.py 100.00% <ø> (ø)
tests/test_utils.py 100.00% <ø> (ø)
tests/test_utils_logs.py 100.00% <ø> (ø)
tests/test_vehicle.py 100.00% <ø> (ø)

... and 1 file with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@CM000n
Copy link
Collaborator Author

CM000n commented Nov 10, 2023

Hy @DurgNomis-drol I would also like to suggest cancelling python3.7 support, since pylint < 3.0.0 is probably incompatible with python >= 3.11, but pylint >= 3.0.0 no longer supports python < 3.8 versions.

@DurgNomis-drol
Copy link
Owner

@CM000n That would be great, but @joro75 dependes on it for his integration for Domoticz. I have asked him if it is possible for him to upgrade his integration to support the newest python version

@joro75
Copy link
Collaborator

joro75 commented Nov 11, 2023

@CM000n and @DurgNomis-drol I also fixed some other Pull requests today, and I observed the same. Python < 3.8 is not supported anymore by a lot of dependencies that we are using. Please see PR #237, where I also suggested to require Python >= 3.8.1.

For me it is OK to go to Python >= 3.8

@joro75
Copy link
Collaborator

joro75 commented Nov 11, 2023

Regarding the actual change on the improved check of the connected services: I will try to check it tomorrow on my Toyota.

@joro75
Copy link
Collaborator

joro75 commented Nov 12, 2023

I'm still using the 'old' MyT application, as I have not been converted yet to the 'MyToyota' app.
I have checked the modifications, and can confirm that they are still working for my situation. I did add an additional check to prevent TypeErrors when the vin of the car and the vin of the devices are not matching.

joro75
joro75 previously approved these changes Nov 12, 2023
@joro75 joro75 merged commit e2faa70 into DurgNomis-drol:master Nov 12, 2023
18 checks passed
@CM000n CM000n self-assigned this Nov 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants