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

V2 #166

Merged
merged 43 commits into from
Nov 16, 2023
Merged

V2 #166

merged 43 commits into from
Nov 16, 2023

Conversation

CM000n
Copy link
Collaborator

@CM000n CM000n commented Nov 9, 2023

No description provided.

@DurgNomis-drol
Copy link
Owner

You are updating github actions in this and introducing a labeler?

@CM000n
Copy link
Collaborator Author

CM000n commented Nov 10, 2023

You are updating github actions in this and introducing a labeler?

Yes, most of the actions used in the workflows were quite outdated. And from the repos I've worked with so far, I'm used to the labels being predefined
But I agree with you, that doesn't really belong in this PR. I can also undo it and leave it out if it bothers you.

@DurgNomis-drol
Copy link
Owner

I think it is a great idea, can you add this in a separat PR? Just so it will be easier for us to review this? 🚀 Also great job!

@CM000n CM000n marked this pull request as draft November 10, 2023 21:22
@CM000n
Copy link
Collaborator Author

CM000n commented Nov 10, 2023

I'll take a break for today and maybe have another look tomorrow.
Looks like we still have a problem setting up the sensor platform because it tries to pass a nonetype object to the callable.
File "/config/custom_components/toyota/sensor.py", line 301, in extra_state_attributes self.entity_description.attributes_fn(self.vehicle) TypeError: 'NoneType' object is not callable

I hate this damn async stuff 😆

@CM000n
Copy link
Collaborator Author

CM000n commented Nov 14, 2023

Perhaps a question about a general opinion. Especially @DurgNomis-drol.
It is currently good practice for Home Assistant development not to put any information that changes frequently in the extra attributes of a sensor.
Hence the question of whether we want to provide the additional information with the pre-aggregated data via the extra attributes of the statistics sensors, as is currently the case, or make separate sensors out of them?
With just under 18 pieces of information per statistics sensor, we would then generate 72 sensors in one go. 🤯

@DurgNomis-drol
Copy link
Owner

@CM000n I always like to conform to the HA way of doing things. But I also don't think that exposing 72 sensors is good UX. A lot have changed in HA since I implemented this. So I think we need to have discussion about what is the best way to implement it going forward.

I don't want to add 72 sensors, so I will not accept this change, hope you understand.

We could return the data from a service call instead maybe, if this make sense for peoples use case. I'm open for better solutions.

@CM000n
Copy link
Collaborator Author

CM000n commented Nov 14, 2023

I agree with you @DurgNomis-drol. I also think that 72 sensors, some of them redundant, will clutter up the UI. I just wanted to bring it up to discussion.

I think for now it's good with the 4 statistic sensors and their extra attributes. The attributes don't change every second, so the risk of an extremely large database should be low.

If someone wants to use the information from the extra attributes, this can already be done easily using templates.

@DurgNomis-drol
Copy link
Owner

V2 works for me too

image

@HuffYk
Copy link

HuffYk commented Nov 14, 2023

Hi @CM000n

day stats still doesn't work for me. It is because my json for both cars looks like this for day (there is no "PERIODE_START" but "date"):

        "statistics": {
            "day": [
                {
                    "bucket": {
                        "year": 2023,
                        "dayOfYear": 318,
                        "unit": "metric",
                        "date": "2023-11-14"
                    },
                    "data": {
                        xxx
                    }
                },
                {
                    "bucket": {
                        "year": 2023,
                        "dayOfYear": 317,
                        "unit": "metric",
                        "date": "2023-11-13"
                    },
                    "data": {
                        xxx
                    }
                }
            ],

so after fixing it in sensor.py:

changing

"From": data[BUCKET][PERIODE_START] if BUCKET in data else from_dt,
to
"From": data[BUCKET]["date"] if BUCKET in data else from_dt,

I successfully got day data and the only error left in log is this:

File "/config/custom_components/toyota/entity.py", line 30, in __init__
self.vehicle: Vehicle = coordinator.data[self.index]["data"]
~~~~~~~~~~~~~~~~^^^^^^^^^^^^
TypeError: list indices must be integers or slices, not Vehicle

but not sure why it worked for you so please check. I see also in @DurgNomis-drol 's screenshot that day sensor is hidden so maybe it didn't work also for him ?

Regarding vehicle info... these JSON parts came through to UI:

_vehicle_info: 
odometer: 
_status:

what is missing:

_status_legacy:  a lot of interesting info there... not everything from here was in old HA integration 

"_status_legacy": {
    "VehicleInfo": {
        "AcquisitionDatetime": "2023-11-14T12:30:50Z",
        "RemoteHvacInfo": {
            "InsideTemperature": 24,
            "RemoteHvacMode": 0,
            "RemoteHvacProhibitionSignal": 1,
            "SettingTemperature": 20.5,
            "BlowerStatus": 0,
            "FrontDefoggerStatus": 0,
            "RearDefoggerStatus": 0,
            "LatestAcStartTime": "2023-11-09T05:39:15Z",
            "TemperatureDisplayFlag": 1,
            "Temperaturelevel": 31
        },
        "ChargeInfo": {
            "GasolineTravelableDistanceUnit": 1,
            "GasolineTravelableDistance": 484,
            "EvDistanceInKm": 0.0,
            "PlugStatus": 12,
            "PlugInHistory": 41,
            "RemainingChargeTime": 65535,
            "EvTravelableDistance": 0.0,
            "EvTravelableDistanceSubtractionRate": 4,
            "ChargeRemainingAmount": 31,
            "SettingChangeAcceptanceStatus": 0,
            "ChargeType": 15,
            "ChargeWeek": 0,
            "ChargeStartTime": "42:35",
            "ChargeEndTime": "42:35",
            "ConnectorStatus": 2,
            "BatteryPowerSupplyPossibleTime": 16383,
            "ChargingStatus": "none",
            "EvDistanceWithAirCoInKm": 0.0
        }
    }

_connected_services: here I don't think anything is needed for now in UI... a lot of private data there like device IDs, keys, customer IDs, IMEI etc... for new car (Yaris Cross 2023) there is not just one device with VIN code (which is checked if connected services are on) but there are also other 2 devices:

"provider": "AVMAP_DCM600"
"provider": "TAS600"

each this device has also HTTP links, but none of them works with SSOMS token:
e.g.

                  "links": [
                        {
                            "rel": "self",
                            "href": "http://devicemaster.toyota-europe.com/services/devices/xxx"
                        },
                        {
                            "rel": "profile",
                            "href": "http://devicemaster.toyota-europe.com/services/devices/xxx/profile"
                        },
                        {
                            "rel": "config",
                            "href": "http://devicemaster.toyota-europe.com/services/devices/xxx/config"
                        }
                    ]

But in MyToyota app these new cars lost lock and light information which was part of "_status": so we will need some new API to get this info again...

@CM000n
Copy link
Collaborator Author

CM000n commented Nov 14, 2023

Hi @HuffYk , I forgot to push earlier 😆
Sry

@HuffYk
Copy link

HuffYk commented Nov 14, 2023

Hi @HuffYk , I forgot to push earlier 😆 Sry

np, works now 👍

@HuffYk
Copy link

HuffYk commented Nov 14, 2023

Perhaps a question about a general opinion. Especially @DurgNomis-drol. It is currently good practice for Home Assistant development not to put any information that changes frequently in the extra attributes of a sensor. Hence the question of whether we want to provide the additional information with the pre-aggregated data via the extra attributes of the statistics sensors, as is currently the case, or make separate sensors out of them? With just under 18 pieces of information per statistics sensor, we would then generate 72 sensors in one go. 🤯

I think it's more clean to have stats as we have them now. However some info from _status_legacy would be great to have as sensors. Not sure if HA supports some kind of grouping as I like grouping done in API results (car -> HVAC; car -> Charging... possibly also car -> Lights and car -> Locks)... I suppose HA doesn't support nesting devices (or one device having sub-devices and sensors linked to sub-devices) ?

@DurgNomis-drol
Copy link
Owner

DurgNomis-drol commented Nov 14, 2023

@CM000n I have entity translations ready, but will submit them in new PR when this is merge 🚀 I think we are getting close to getting this ready to get merge into master. The rest of the data available in mytoyota we can add in PR's after this one. What do you think or am I missing something?

@HuffYk Great find!

@lkucytowski
Copy link

Guys, let me if I can somehow participate in getting PHEV sensors to work in v2. So far I'm missing two crucial ones - battery status and range. Not sure what else is exposed in the new API that could be useful.

@CM000n
Copy link
Collaborator Author

CM000n commented Nov 14, 2023

@CM000n I have entity translations ready, but will submit them in new PR when this is merge 🚀 I think we are getting close to getting this ready to get merge into master. The rest of the data available in mytoyota we can add in PR's after this one. What do you think or am I missing something?

@HuffYk Great find!

I think that fits. Let's create a basis first. We can then gradually add additional sensors and information and deal with them separately.

There may also be changes regarding the new API endpoints in the mytoyota Lib. This will also make it easier if we don't have to take too much into account in one go.

@CM000n
Copy link
Collaborator Author

CM000n commented Nov 14, 2023

Guys, let me if I can somehow participate in getting PHEV sensors to work in v2. So far I'm missing two crucial ones - battery status and range. Not sure what else is exposed in the new API that could be useful.

We are not yet using any new API endpoints. The changes here are still using the old endpoints, but are already using a newer version of the mytoyota lib, which will (hopefully) make future changes easier.
For the new endpoints, a PR with the necessary adjustments in the mytoyota repository is required first.

@DurgNomis-drol
Copy link
Owner

DurgNomis-drol commented Nov 14, 2023

@CM000n That was my thought too. It will probably be a couple of months before we know what API endpoints they will turn off and we have everything moved over to the new platform (Meaning new Toyota App).

I believe with this refactor it will be much easier to handle changes in the future and conform to HA standards.

Just ping me when you want me to review this for a final time! For me everything is working, but I don't have many of the sensors that are possible, so I am waiting for you guys to confirm that

@DurgNomis-drol DurgNomis-drol merged commit 561c25a into master Nov 16, 2023
3 checks passed
@CM000n CM000n deleted the v2 branch November 16, 2023 11:55
@oskarhood
Copy link

I still get this error message in the logs: 2023-11-20 05:17:54.018 ERROR (MainThread) [mytoyota] Please setup Connected Services if you want live data from the car. (JTM**************). RAV4 2020, Sweden. Only Numberplate working after changing to new Toyota app. Anyone else with the same problem?

@CM000n
Copy link
Collaborator Author

CM000n commented Nov 20, 2023

Should bei fixes with V2:

I still get this error message in the logs: 2023-11-20 05:17:54.018 ERROR (MainThread) [mytoyota] Please setup Connected Services if you want live data from the car. (JTM**************). RAV4 2020, Sweden. Only Numberplate working after changing to new Toyota app. Anyone else with the same problem?

Should be fixed with V2 of the Custom Component which ist not released yet.
Pleased read the discussion here in this issuee.

@oskarhood
Copy link

Ok, @CM000n thanks for the info! Good work, looking forward to V2!

@DurgNomis-drol
Copy link
Owner

@oskarhood If you use HACS, you can choose master when selecting a version to install, this way you can try it out

@oskarhood
Copy link

@DurgNomis-drol , master version working perfect! As mentioned above, the sensor for PHEV battery charge percentage is still missing. I hope someone can find it!

@clews
Copy link

clews commented Nov 21, 2023

Finally i also managed to test the master version with the "dirty" patch from #164 .

Got the trip data, fuel level, odometer, parking_location and the basic "number plate" data (without number plate in AT)

Got two errors, the one already mentioned:

File "/config/custom_components/toyota/entity.py", line 30, in __init__
self.vehicle: Vehicle = coordinator.data[self.index]["data"]
~~~~~~~~~~~~~~~~^^^^^^^^^^^^
TypeError: list indices must be integers or slices, not Vehicle

and one about the fuel_level sensor:

2023-11-21 08:55:59.065 ERROR (MainThread) [homeassistant.components.binary_sensor] Error while setting up toyota platform for binary_sensor
  File "/config/custom_components/toyota/binary_sensor.py", line 307, in async_setup_entry
    ToyotaBinarySensor(
  File "/config/custom_components/toyota/entity.py", line 32, in __init__
2023-11-21 08:55:59.076 WARNING (MainThread) [homeassistant.components.sensor] Entity sensor.prius_5_fuel_level (<class 'custom_components.toyota.sensor.ToyotaSensor'>) is using native unit of measurement '%' which is not a valid unit for the device class ('volume_storage') it is using; expected one of ['L', 'fl. oz.', 'mL', 'm³', 'ft³', 'gal', 'CCF']; Please update your configuration if your entity is manually configured, otherwise create a bug report at https://github.com/DurgNomis-drol/ha_toyota/issues

A lot of info is missing, no EV data of the PRIUS 2023 (except on the trips)... Guess I should test the mytoyotalib to see the data that can be gathered right now?

Repository owner locked as resolved and limited conversation to collaborators Nov 21, 2023
@CM000n
Copy link
Collaborator Author

CM000n commented Nov 21, 2023

Hi @clews,
Thx for testing but this hast already been merged.
Will Take a Look at your error with the native unit For the fuel level sensor later

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error «Please setup Connected Services» after Migration to new MyToyota App/API
7 participants