alandtse / tesla

Tesla custom integration for Home Assistant. This requires a refresh token be generated by third-party apps to login.
Apache License 2.0
599 stars 99 forks source link

v3.0 missing old state sensors #294

Closed FalconUK closed 1 year ago

FalconUK commented 2 years ago

I used to extract tyre pressures from the attributes of

'vehicle_state_data_sensor'

This sensor appears to be missing.

There was also a similar sensor 'drive_state_data_sensor', or something like that?

Are these sensors coming back?

alandtse commented 2 years ago

@shred86 I didn't realize we took them away. Any thoughts on whether we bring them back? These were the raw json sensors for people to template on

shred86 commented 2 years ago

I'm not too familiar with the previous version as I honestly only started looking into this integration with the rewrite version. Was it just a sensor with a bunch of attributes from the Tesla API vehicle_data endpoint? Seems like it would be a very long list of attributes... My thought is we should just add the entities people are using so they don't have to create template sensors. Tire pressures should be an easy one to create sensors for, but we would have to add it on the teslajsonpy side as I don't think I made those into properties.

Edit: I'm looking at the old teslajsonpy code where the HA integration was. I see the data sensors and it looks like they were just pulling from the old get_drive_params, get_gui_params, etc. and setting them as attributes.

@FalconUK besides the tire pressure data, what other attributes were you using? If it's just a few entities, I think it would be worth just adding them as actual entities.

FalconUK commented 2 years ago

@shred86, @alandtse, I was using tyre pressures to nurse a slow puncture through until a replacement tyre arrived. I declared all four, example below:

    - sensor:
        - name: "*** tyre pressure front left"
          unique_id: sensor.***_tyre_pressure_front_left
          state_class: measurement
          state: "{{(state_attr('sensor.***_vehicle_state_data_sensor', 'tpms_pressure_fl')|float * 14.5038)|round(2)}}"
          unit_of_measurement: "PSI"

This raises a similar localisation question as with the units for the odometer sensor - km/miles, bar/PSI

It's fair to say that some attributes are definitely more useful than others. I think it would be in line with the current philosophy to expose the majority of sensors as entities, perhaps with some of them disabled by default?

alandtse commented 2 years ago

Yes, the intent of the rewrite was to make it much easier to edit the ha component for contributors. If it's an entity that is important, it should be created. Ideally we can templatize it so it's just data driven.

craigsbits commented 2 years ago

The state sensors were useful the other day when the battery state of charge disappeared! Personally I would like to see them back especially the tyre (tire) pressures back. Support the odometer units mention above. The heated seats in the official app now have an "auto" setting if that could be found and added. I will open a new thread for that.

bkr1969 commented 2 years ago

So is the "old" version completely out the window? It seems poor practice to rewrite an integration that was working just fine to one that doesn't work at all.

TheDK commented 2 years ago

Another option could be to have a "vehicle raw data" entity with everything as attributes or even raw json and disable it by default. Then whenever someone needs something this could be enabled and templated.

alandtse commented 2 years ago

So is the "old" version completely out the window? It seems poor practice to rewrite an integration that was working just fine to one that doesn't work at all.

The 2.x series is still available for download. Source code is also available if you'd like to fix or change things. However, please check your attitude.

It's poor practice to:

  1. Download a breaking change without reading and then complaining about it.
  2. Post duplicate issues without bothering to search.
  3. Cross post your issue on unrelated issues particular when it's unuseful info like "everything is broken because I didn't read breaking notes".

Please note, this is your one warning so please stop. After that, you will be banned from the repo which may mean you can't download from it anymore.

@shred86 please be aware of people like this now that you're helping maintain. If they harass you or escalate, please let me know and we will ban them without issuing a warning.

alandtse commented 2 years ago

Another option could be to have a "vehicle raw data" entity with everything as attributes or even raw json and disable it by default. Then whenever someone needs something this could be enabled and templated.

I think this makes sense. The only downside I see is once someone figures out a template they tend not to share it and also they don't have any incentive to submit a PR to officially add the entity so others can benefit. @shred86 your thoughts on this?

bkr1969 commented 2 years ago

I'm not trying to have an attitude, but it would be nice if the release notes for an update gave some sort of notice as to "breaking changes" or re-named entities or that the integration had to be completely removed and reinstalled to work at all after updating. I have several automations that rely on some of these sensors and my code needed to be completely re-vamped. If this information was out there, I apologize, but I certainly didn't see it.

alandtse commented 2 years ago

I'm not trying to have an attitude, but it would be nice if the release notes for an update gave some sort of notice as to "breaking changes" or re-named entities or that the integration had to be completely removed and reinstalled to work at all after updating. I have several automations that rely on some of these sensors and my code needed to be completely re-vamped. If this information was out there, I apologize, but I certainly didn't see it.

https://github.com/alandtse/tesla/releases/tag/v3.0.0

Once the major version changes, that's a breaking change so you need to read the relevant update.

shred86 commented 2 years ago

Another option could be to have a "vehicle raw data" entity with everything as attributes or even raw json and disable it by default. Then whenever someone needs something this could be enabled and templated.

I think this makes sense. The only downside I see is once someone figures out a template they tend not to share it and also they don't have any incentive to submit a PR to officially add the entity so others can benefit. @shred86 your thoughts on this?

The only thing I’m seeing requested so far is tire pressure, which we could easily add as separate entities. I personally rather just add the entities people need rather than making an entity with a long list of attributes. The main reason for this is the HA dev docs mentions a “danger” note:

Entities that generate a significant amount of state changes can quickly increase the size of the database when the extra_state_attributes also change frequently. Minimize the number of extra_state_attributes for these entities by removing non-critical attributes or creating additional sensor entities.

Besides tire pressure, what other attributes do folks want or need exposed as entities?

FalconUK commented 2 years ago

Is there a concise list of what the attribute list used to have, or some simple instructions? I don't mind doing some digging but I'm not familiar with the code.

From memory and referring to TeslaFi:

Happy to go through the attribute list for vehicle_state and drive_state if someone can show me where they are.

TheDK commented 2 years ago

I personally rather just add the entities people need rather than making an entity with a long list of attributes. The main reason for this is the HA dev docs mentions a “danger” note:

Entities that generate a significant amount of state changes can quickly increase the size of the database when the extra_state_attributes also change frequently. Minimize the number of extra_state_attributes for these entities by removing non-critical attributes or creating additional sensor entities.

You're righ. I thought there was a recent change that attribute changes would not be written to the DB if the state of the entity didn't change, but I can't find anything about that, so I probably misremember. If that's the case I'd say adding the entities mentioned by @FalconUK together with tire state and maybe the UserID who's driving should be covering mosts cases.

shred86 commented 2 years ago

Should be able to add these.

FalconUK commented 2 years ago

Out of curiosity @FalconUK is there something specific you're trying to do with shift state?

I have security lighting/flood lighting over the front of my property. It activates when I drive in, which is fine. I reverse in to my parking spot in front of the garage and the flood lights blind the view in my mirrors. It hasn't been a problem over the summer so I've forgotten about it. Now it's dark in the evenings, it's a reminder that I should turn off the flood lights if I've just arrived home, and have selected reverse, within 5 minutes of each other.

sanegaming commented 2 years ago

I also miss the states, my dashboard had "Update Available, charging sensor, online sensor, interior and exterior temp, charge rate, etc." there is a couple more but those are my big ones. I will be attempting to revert until this is brought back. Example: image

shred86 commented 2 years ago

I also miss the states, my dashboard had "Update Available, charging sensor, online sensor, interior and exterior temp, charge rate, etc." there is a couple more but those are my big ones. I will be attempting to revert until this is brought back. Example: image

All of those you mentioned are already implemented. The entity naming and unique IDs changed with this rewrite so it’s best to just remove the integration and re-add it. You’ll have to update your dashboard to reflect the new entity names.

shred86 commented 2 years ago

Out of curiosity @FalconUK is there something specific you're trying to do with shift state?

I have security lighting/flood lighting over the front of my property. It activates when I drive in, which is fine. I reverse in to my parking spot in front of the garage and the flood lights blind the view in my mirrors. It hasn't been a problem over the summer so I've forgotten about it. Now it's dark in the evenings, it's a reminder that I should turn off the flood lights if I've just arrived home, and have selected reverse, within 5 minutes of each other.

Ah, nice. I’m thinking if we implement a shift state sensor (e.g. drive, reverse, park), I’m not sure if we really need the parking brake binary sensor too. I’d probably use the same logic where if the vehicle is asleep, just report Park.

alandtse commented 2 years ago

Ah, nice. I’m thinking if we implement a shift state sensor (e.g. drive, reverse, park), I’m not sure if we really need the parking brake binary sensor too. I’d probably use the same logic where if the vehicle is asleep, just report Park.

Let's leave the binary sensor intact to avoid a breaking change. We can warn people it's deprecated and remove it when we really need to break something.

tasranson commented 2 years ago

I was using the following from vehicle state:

Was also using the steering wheel heater switch which seems to be missing but off topic (so will raise as seperate).

FalconUK commented 2 years ago

Ah, nice. I’m thinking if we implement a shift state sensor (e.g. drive, reverse, park), I’m not sure if we really need the parking brake binary sensor too. I’d probably use the same logic where if the vehicle is asleep, just report Park.

Let's leave the binary sensor intact to avoid a breaking change. We can warn people it's deprecated and remove it when we really need to break something.

This sounds like a structured approach.

FalconUK commented 2 years ago

I was using the following from vehicle state:

  • Tire pressure x 4 From Drive State:
  • Heading (Direction) - I can get this from vehicle tracker
  • Speed - I can get this from vehicle tracker
  • Power
  • Shift State From Online Sensor:
  • update interval

Was also using the steering wheel heater switch which seems to be missing but off topic (so will raise as seperate).

I would also be happy to see power, more for interest than actual use.

freshfieldx commented 2 years ago

I'm using:

Speed from location tracker Charge amps from charging state data Charge energy added from charging rate.

shred86 commented 2 years ago

Given Home Assistant doesn't actually have a car class, I think the best way to add some of these requested entities are:

Power is a bit of an odd one. We could just add it as a new sensor that just passes the raw JSON value (appears to be an integer) which I'm not sure what the unit of measurement is (watts?). I don't really think it would be smart to add this as an attribute to an entity since it's a value that is constantly changing when driving a vehicle (database concerns).

InTheDaylight14 commented 2 years ago
  • Door and window status added as attributes to the door lock sensor.

Windows could be added as a cover to vent and close them when parked. I had a cover template working with the custom_api service and detailed vehicle state data. Though the cover template would still work with the window state in door lock attributes.

cover:
  - platform: template
    covers:
      <vehicle_name>_windows:
        device_class: window
        friendly_name: "<Vehicle Name> Windows"
        value_template: >-
          {% if state_attr('sensor.<vehicle_name>_vehicle_state_data_sensor','fd_window') %}
          true
          {%- else -%}
          false
          {%- endif %}
        open_cover:
          - service: tesla_custom.api
            data:
              command: WINDOW_CONTROL
              parameters:
                path_vars:
                  vehicle_id: >-
                    {{ state_attr('binary_sensor.<vehicle_name>_online_sensor', 'id') }}
                command: vent
                lat: 0
                long: 0
                wake_if_asleep: true
        close_cover:
          - service: tesla_custom.api
            data:
              command: WINDOW_CONTROL
              parameters:
                path_vars:
                  vehicle_id: >-
                    {{ state_attr('binary_sensor.<vehicle_name>_online_sensor', 'id') }}
                command: close
                lat: 0
                long: 0
                wake_if_asleep: true
shred86 commented 2 years ago

That would make sense. A single cover entity for all windows and the open cover command would call the vent feature. We could then just add each window state as an attribute.

Are you actually able to close the windows? I can't on my 2015 Model S via the app, says it's unavailable to close remotely but might just be some weird limitation on the Model S or older models.

Edit: I suppose we could do the same for the doors. Any Model X owners able to share how you're able to open the doors with your Tesla App? Is there control for each door? Can you close each door with the app or just the falcon wing doors?

FalconUK commented 2 years ago

Vent and close works fine for me via the app. 2020 model 3 LR.

pmrocha26 commented 2 years ago

I was just starting to use the "is_user_present" attribute but, to be honest, I am not really sure if that means what it appears to mean - I updated the integration before really testing it out. Assuming it means "someone is inside the car", I think it could come in handy. I was just sending a notification when the car was left unlocked for some time - weirdly, the car does not lock when we move away in some specific places (no idea why and yes, the tesla app now does the same)... I could also use it in my own "Cabin Overheat Protection" - the Tesla one does not work when sentry mode is on and the alarm tilt sensor is on... And the tilt sensor keeps resetting to on, so I can not turn the COP on from the app (late 2021 model 3 LR).

FalconUK commented 1 year ago

Any update on this? There's been a few releases without any mention of these.

alandtse commented 1 year ago

I'm going to close this. The reasoning for why raw_json isn't exposed anymore is basically here. We can revisit this in the future but right now I am liking the contributions we're getting now (which is a lot more than we used to). Thanks to @shred86 @InTheDaylight14 @carleeno and probably others I'm forgetting about.

If you want something specific, please just put in a new issue and hopefully a PR will come in to close it.