Rewrite to use models instead

[DurgNomis-drol]

Sick with corona and I was extremely bored 😜 so I rewrote nearly all the data models. This should now fix a lot of problems, but this will also mean that this is a breaking change. Hopefully this makes it a lot easier in the future to manage this module.

@joro75 If you are up for the task, you are more then welcome to review it. But I do understand if you don't (as it is quite large) 😃

I also rewrote the tests to match the new structure

[joro75]

@DurgNomis-drol I hope you haven't been to ill.

It indeed is a large change, but it shouldn't be a problem to review this. Please give me a few days, so I can do a decent review. Will come back to you soon!

[DurgNomis-drol]

@joro75 Thanks, no hurry, I am still missing some minor things in this PR. I need to add docstrings and so on. Will try to get it done to night.

Fortunately i'm not that ill from it

[DurgNomis-drol]

A couple of things have been removed in this also. This now means that you can not get the vehicle status in json. If this is still something we want, we can implement it. But I don't think it is need.

The structure of the objects have changed also to make more sense and i have also add a couple of new attributes and name things more appropriately.

There are probably things that can be done better, this is just a "draft" to get us started.

[joro75]

I was first confused by the different dictionary key names that are used, but I discovered that this seems to be correct.
I do like the proposed changes, and I do agree that this would be an improvement for the usability of the library.

There are 2 suggestions:

1. Is there way that we can determine the version number of the library so any code that is using the library can check if the correct version is installed or not? For example a client.MyT.version property could return '0.8.0' to indicate that the new model structure is being used.

2. A lot of the member functions of the model now return a data-type or None.
   For example the 'hybrid' function now returns a boolean or 'None' if the 'hybrid' key is not present. Wouldn't it be more logical to always only return a boolean? If the 'hybrid' key is not present, we could just return 'False', as the vehicle then very probably is not a hybrid one.
   Some other suggestions for these are:

• 'alias' will return "" if not present
• 'vin' will return "" if not present
• 'warning' will return False if not present
• 'closed' will return True if not present
• 'locked' will return True if not present
  This will remove a lot of 'Null'-checking in the program that is using the mytoyota library. The only disadvantage would be that the program is not able anymore to determine if it is actual data or 'default' data. But we could add an is_actual_data function that returns True or False for that object.

[DurgNomis-drol]

Thanks for the feedback.

Suggestion 1:
I just pin the version I am using in the Home Assistant integration to make sure it is always the correct one that is being installed. I am unsure if this is possible for yours. We can implement a boolean that returns true if the new data models is to be used, so the user can see what format the data will return in?

Suggestion 2:
You are right about the hybrid property, it now defaults to False instead of None.

The reason I have chosen to use None to present the absence of a value is because it is the pythonic way to do it. And I don't want to lose the ability to know if information is available or not. It could be missing even though it should be there, and there for it is nice to know that.

We could introduce a default for alias instead of returning an empty string.

But what about a function that returns what features is available for a given car? This would make it easy to know what to expect. I could also use this feature in my Home Assistant integration.

When looking into adding a function that tells you what features are supported, I realised, it is just easier to check if the parent object is None (dashboard, parkinglocation, sensors, hvac). A parent object will always be None if it is unsupported, so you can just check for that. and if the object is present, then so will the data.

[joro75]

Ok.
Regarding the suggestions we are discussing:

1. The pinning is not possible for me. This is however a separate topic, and not related to these code changes. I will investigate it and open a seperate issue or PR.
2. Ok. Let's keep it pythonic using the null responses.

There are a few checks I still would like to do before approving the PR. Will come back later, after I checked it.

[DurgNomis-drol]

Yes, open a separate issue so we can find a solution 😃

Again, thanks for reviewing and giving feedback

[joro75]

See my PR #128.
It will update the example code from the README.md.
The changes you propose in this PR (#126) , are ok for me. I do have to adjust my plugin, but that is not a problem. What I need is still possible with your proposed changes.

Do you want me to approve this PR (after you approved PR #128)? It will then of course become the actual 'master' code, causing code breaking changes.

[DurgNomis-drol]

I just approved yours. And when you are ready, you can approve this, and then i will merge both in to master.

I won't release a new version just yet, I want to do some code clean up and simplification (Or at least try to do that) but none of that should be a breaking change.

I also believe we should find a solution to your version problem, so we don't break your integration before you have moved it, before releasing a new version.

I won't immediately change my Home Assistant integration, because I am thinking rewriting it anyways.

[joro75]

Is certainly a large code changes, but it is actually a good change in my opinion, as it will simplify the using of the library. 👍👍