search

zabuldon / teslajsonpy

Apache License 2.0
57 stars 62 forks source link

New features needed for Home Assistant improvements - will they be accepted here? #14

Closed OverloadUT closed 6 years ago

OverloadUT commented 6 years ago

Hello @zabuldon!

I am planning on making some pretty big improvements to the Tesla component in Home Assistant. In order to do them, I need to make several changes to this library. I am going to make these changes regardless, but I wanted to check to see if you're still active and would accept these changes to your library. If not, I'll fork and publish a new lib, but it would be overall better to keep it all here!

Proposed changes:

  1. Make the library's polling interval management optional (Hass will take over responsibility of requesting when to poll, so that complex "smart polling" logic can be implemented and maintained there.
  2. Split out polling to be possible per-vehicle rather than per-account.
  3. Allow fetching updates from the owner API that don't require waking up the vehicle
  4. Expose a unique ID for each vehicle that can be used as a Hass unique ID, that is NOT the VIN, to avoid unnecessary PII exposure (not sure what this will be quiet yet; perhaps just a piece of the VIN)

I haven't written this yet; I am just putting this here to give you time to see it and let me know how you'd like to proceed before I get to the point where I'm merging it in to Hass :)

zabuldon commented 6 years ago

Hello, thanks for your questions:

  1. The problem is Tesla do ban your IP address in case if you're sending request too often.
  2. Could you please clarify? As I'm not sure what do you mean.
  3. Do you have documentation of it?
  4. I'm ok with it. Just used VIN because it was simpliest way for.

On Thu, 2 Aug 2018, 01:20 Greg Laabs, notifications@github.com wrote:

Hello @zabuldon https://github.com/zabuldon!

I am planning on making some pretty big improvements to the Tesla component in Home Assistant. In order to do them, I need to make several changes to this library. I am going to make these changes regardless, but I wanted to check to see if you're still active and would accept these changes to your library. If not, I'll fork and publish a new lib, but it would be overall better to keep it all here!

Proposed changes:

  1. Make the library's polling interval management optional (Hass will take over responsibility of requesting when to poll, so that complex "smart polling" logic can be implemented and maintained there.
  2. Split out polling to be possible per-vehicle rather than per-account.
  3. Allow fetching updates from the owner API that don't require waking up the vehicle
  4. Expose a unique ID for each vehicle that can be used as a Hass unique ID, that is NOT the VIN, to avoid unnecessary PII exposure (not sure what this will be quiet yet; perhaps just a piece of the VIN)

I haven't written this yet; I am just putting this here to give you time to see it and let me know how you'd like to proceed before I get to the point where I'm merging it in to Hass :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/zabuldon/teslajsonpy/issues/14, or mute the thread https://github.com/notifications/unsubscribe-auth/ABOKkG5k52yDLS_qPZwMUdFVplyJLSf0ks5uMim_gaJpZM4VrViq .

OverloadUT commented 6 years ago

Excellent; I'm glad you're still active!

  1. I understand that. What I want is the ability to have Hass manage "Smart Polling" to be much friendlier to the car battery: While the vehicle is driving, polling will be fairly rapid, but when the car is parked and not charging, polling will step down gradually to be quite infrequent, to preserve the battery. And if the battery gets to a critical state, such as 20% or less, polling will step down even further. Another objective here is that I will be exposing Hass services that can force a vehicle update, and another that will force and update and reset the step-down timer. The latter can be used in automations to ensure we get a state change to "driving" closer to when it actually starts. (Example: At 8am, reset the stepdown timer, because I expect I'll be driving very soon. More complex example: When my front door opens between 7am and 9am, reset the stepdown timer because I am probably leaving the house now)

  2. Right now your code polls all vehicles on the account at once - this is incompatible with my "state-based" polling intervals I propose above. We need to be able to poll a single vehicle at a time.

  3. I admit that I am inexperienced in this one, but from my read of the API docs, it looks like some of the properties of the vehicle can be fetched from Tesla's servers without needing to wake up the car. I may be completely wrong about this - I'll do more experimenting with the API in this regard.

zabuldon commented 6 years ago

  1. I'm comfortable with it, I've planned to do something like this, but if you send a pr with this it would be nice

  2. Sounds good. I already have this code, but it isn't stable. I can push it repo in a few days and you can make your changes.

  3. Basically you can get all the sensors, but their values will be frozen if you don't send wake up. For example temperature sensor: if you don't send wake up it respona always the same value or with empty value. If you see possible resolution of this without waking up, I m ready to discuss/accept patches.

On Thu, 2 Aug 2018, 01:39 Greg Laabs, notifications@github.com wrote:

Excellent; I'm glad you're still active!

1.

I understand that. What I want is the ability to have Hass manage "Smart Polling" to be much friendlier to the car battery: While the vehicle is driving, polling will be fairly rapid, but when the car is parked and not charging, polling will step down gradually to be quite infrequent, to preserve the battery. And if the battery gets to a critical state, such as 20% or less, polling will step down even further. Another objective here is that I will be exposing Hass services that can force a vehicle update, and another that will force and update and reset the step-down timer. The latter can be used in automations to ensure we get a state change to "driving" closer to when it actually starts. (Example: At 8am, reset the stepdown timer, because I expect I'll be driving very soon. More complex example: When my front door opens between 7am and 9am, reset the stepdown timer because I am probably leaving the house now) 2.

Right now your code polls all vehicles on the account at once - this is incompatible with my "state-based" polling intervals I propose above. We need to be able to poll a single vehicle at a time. 3.

I admit that I am inexperienced in this one, but from my read of the API docs, it looks like some of the properties of the vehicle can be fetched from Tesla's servers without needing to wake up the car. I may be completely wrong about this - I'll do more experimenting with the API in this regard.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/zabuldon/teslajsonpy/issues/14#issuecomment-409748022, or mute the thread https://github.com/notifications/unsubscribe-auth/ABOKkIhanlua7gOoxW0ytgoVdCxmaB51ks5uMi4VgaJpZM4VrViq .

alandtse commented 6 years ago

@OverloadUT

  1. I've addressed 1 for preserving batteries using automations in HA which disable update polling per vehicle after 24 hours without a charge. Of course, I have 2 implemented (see below). If you're talking about long term parking, you really should stop any polling as Tesla apparently needs to stop receiving polls to enter deep sleep. Once you expose 2, than another automation can kick in for driving, but if you get more aggressive than the default polling, you risk Tesla banning. There's probably a slightly more aggressive setting that could be fine though. I just got sick of losing connectivity while testing.
  2. I've had a per vehicle polling switch pr in for a while that hasn't been merged and also in HA. @zabuldon if you see some stability issues, let me know. I've been running it since I submitted the PR and haven't seen any issues.
  3. I think @zabuldon addressed the current limitation.
  4. I think the last 5 digits on the VIN would work. I've debated doing it for a while but never got around to it when PRs stopped getting merged in.