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

Initial support for Toyota Supra #565

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

vanshg
Copy link

@vanshg vanshg commented Sep 14, 2023

Proposed change

This is a first attempt to add support for Toyota Supra, for which the Supra Connect service uses the same underlying APIs.

Type of change

  • New feature (which adds functionality to this library)

Additional information

The API expects the value toyota in the request, however, it returns DRITTKUNDE as the response (German for "third party vendor"). To address this, the fallback parsing was updated to default to CarBrand.Toyota if DRITTKUNDE is returned. However, for all I know there may be other brands that also use DRITTKUNDE...(Rolls Royce infotainments are also iDrive based)

I have not yet had a chance to test this. Here is the fingerprint:
toyota_supra_fingerprint.zip

Checklist

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works.

@rikroe
Copy link
Member

rikroe commented Sep 14, 2023

Nice, thanks! Didn't know that there are even more cars and brands using NBT systems. Is it only the Supra? And only in the US? Or also globally/other cars?

Regarding the CarBrands issue: I would rather only have TOYOTA and DRITTKUNDE as valid options and no parsing from DRITTKUNDE to TOYOTA in the enum.
When initializing a vehicle, the brand could be added as additional parameter. Then we could check in MyBMWVehicle.brand property if the value in JSON is DRITTKUNDE and replace it with the init parameter?

@rikroe rikroe self-assigned this Sep 14, 2023
@rikroe rikroe added the enhancement 🆕 New feature or request label Sep 14, 2023
@vanshg
Copy link
Author

vanshg commented Sep 14, 2023

For Toyota, yeah only the Supra. For other brands it's only Rolls Royce as far as I can tell (see the last paragraph in the intro here: https://en.wikipedia.org/wiki/BMW_iDrive). The Supra is available in at least Europe but no clue about China.

I would rather only have TOYOTA and DRITTKUNDE as valid options and no parsing from DRITTKUNDE to TOYOTA in the enum

The problem with this is that drittkunde is not a valid value in API requests. So e.g. if we're iterating through all CarBrands, where DRITTKUNDE has been included as a value, the API will return a 400 Bad Request.

One approach might still be to include DRITTKUNDE as an enum, but explicitly filter it out when iterating over all brands -- maybe a CarBrands.validBrands() helper?

@rikroe
Copy link
Member

rikroe commented Sep 15, 2023

Interesting!

I could imagine something like this:

    @classmethod
    @property
    def api_brands(cls) -> list[str]:
        """Return members for API calls."""
        return (cls._member_map_[name] for name in cls._member_names_ if name != "DRITTKUNDE")

@vanshg
Copy link
Author

vanshg commented Nov 13, 2023

IMO, I think that implementation would become a footgun, as it would break the normal/expected Enum iteration (i.e. you would have to know to do for brand in CarBrands.api_brands when making requests as opposed to for brand in CarBrands)

@vanshg
Copy link
Author

vanshg commented Nov 28, 2023

@rikroe Thoughts?

@rikroe
Copy link
Member

rikroe commented Nov 30, 2023

I see your point. But to be honest, I also have no other idea.
Could you please rebase so that test are green? Then we should be able to merge

@vanshg
Copy link
Author

vanshg commented Dec 15, 2023

Sorry for the delay @rikroe. Updated

@rikroe
Copy link
Member

rikroe commented Feb 11, 2024

With #591 merged, could you please have another look? The URLs and return values do differ for the vehicle list, and I'm especially curious if there is any change regarding the returned brands.

Also, I have updated the tests so it is hopefully easier to understand where to adjust them.

@sslobodyan98
Copy link

sslobodyan98 commented Mar 10, 2024

@rikroe @vanshg Hey everyone, I was looking for an API to interact with my Supra and stumbled upon this over some Home Assistant forums. For my use case I just want a local project to mess around and don't need all the bulk of HA. I installed this package and tried to use the CLI tool to login, there are no obvious errors but it returns vehicles as 0 so assume it can't find any cars on my profile. Any ideas?

Update: after running bimmerconnected vehiclefinder
I got another failure, reran: bimmerconnected status
And got back:
Unable to update charging_profile - (TypeError) argument of type 'NoneType' is not iterable
Found 1 vehicles: SPX 40i
Along with all the JSON associated with the car, but mileage isn't accurate as of yet. Will continue to explore it's capabilities :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🆕 New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants