Add Support for Heated Steering Wheel and Seats #188

Megabytemb · 2022-04-09T13:28:21Z

Add Heated Steering Wheel Switch

Allows users to toggle the Heated Steering Wheel from Home Assistant.

This has a silent restriction where it only works when the Climate is Enabled within a car (Similar to the Heated Seats).

If climate is not enabled, then the switch silently fails.

Heated Seats are off if climate control is off. get seat heat level returns None if climate is off.

alandtse · 2022-04-09T22:55:00Z

Thank you. How does the app handle attempts to change the heaters when the climate is off? Does it automatically enable climate? I think we should try to match it.

Megabytemb · 2022-04-10T00:54:18Z

The App automatically turns on the climate.

alandtse · 2022-04-10T01:48:26Z

Yes. Please try to match the app behavior as a default.

alandtse · 2022-04-19T23:01:08Z

Just to make sure, this is just waiting for you to let me know it matches the app's behavior by enabling the climate. Are you planning to do that?

Megabytemb · 2022-04-19T23:52:47Z

Yeah sorry, i've been snowed under with other stuff at the moment.

Its in my plate to get this PR updated so it matches the app.

shocklateboy92 · 2022-04-23T03:21:57Z

Okay, I tried to be patient, but it didn't work.
I'll take a crack at finishing this tonight if @Megabytemb doesn't have time.

@alandtse would you like me to:

dig into the underlying tesla_device and directly try turning the climate on?
try getting an instance of the TeslaThermostat instance and calling a method on it to turn on?
do the programmatic equivalent of a home assistant "service call"?

Megabytemb · 2022-04-23T03:40:53Z

Oh i'm sorry! i didn't know anyone else was waiting on this apart from me!!

i was going to work on it today anyway, so stand by

alandtse · 2022-04-23T03:44:08Z

I'm indifferent to the implementation. Most correct would probably be the tesla_device that comes from teslajsonpy. There's a set of HA specific stuff down in the API library.

We need to wait for HVAC as the heater cannot be turned on while HVAC is off.

shocklateboy92 · 2022-04-23T04:48:48Z

Oh i'm sorry! i didn't know anyone else was waiting on this apart from me!!

i was going to work on it today anyway, so stand by

No worries man! I was trying to be facetious.
This is an open source project you're contributing to out of the goodness of your heart - you don't owe anyone anything. 😊

It's awesome that you have time today. If not, I'll do it tomorrow.

Megabytemb · 2022-04-23T04:49:16Z

Ok, so I've put some new code in here to get the climate device, and use that to enable HVAC - it will wait for up to 30 seconds before it gives up.

But I've came up against an interesting bug. Climate Data doesn't appear to be shared across all devices. So if I have the HVAC and heater steering wheel on, then turn off the HVAC in Home Assistant, it can take awhile for the heated steering wheel to be reflected (as an off state).

Any ideas on how i could get a shared "climate" state? The climate device doesn't store the raw climate data in the class anywhere, so i can't access it.

alandtse · 2022-04-23T05:05:39Z

Hmmm. You may have to do some message passing so other entities are aware of linked changes. I don't think there's a specific mechanism otherwise to share state between entities.

shocklateboy92 · 2022-04-23T05:41:23Z

custom_components/tesla_custom/switch.py

+                    _LOGGER.debug("HVAC Enabled")
+                    return True
+                else:
+                    _LOGGER.info("Enabing Climate to activate Heated Steering Wheel")


Does this mean it will repeatedly send the climate on command every second for up to 30s?
Wouldn't you rather send it once and wait? Or is it sometimes flaky, and the 30 retries be potentially useful?

Yes, this is probably not the best way to implement because it will spam the api. Thinking about this more, it's probably much cleaner to have a single api command that sends the hvac start command, checks ever few seconds to see if it's been enabled, and then issues the second command.

This really feels like the wakeup decorator. Instead of waking up the car, it instead is enabling the hvac.

…eated-Steering-Wheel. Heated Seats and Heated Steering Wheel will now share logic

Megabytemb · 2022-04-23T14:15:42Z

Ok, did some more work on this.

There's some code in there that i'm kinda not proud of - it feels dirty, but as explained in code comments, its the only way I could force other devices to update based on the climate device. I'm fully aware that setting a double underscored class variable externally to the class is really not good python code. haha.

This really highlighted the need to remove any home assistant stuff from the underlying library, so we can have all the code that influences home assistant logic in a single library.

I'd be really interested in knowing your thoughts here.

shocklateboy92 · 2022-04-23T17:34:31Z

custom_components/tesla_custom/climate.py

+        # This is really gross, and i kinda hate it.
+        # but its the only way i could figure out how to force an update on the underlying device
+        # thats in the teslajsonpy library.
+        # This could be fixed by doing a pr in the underlying library,


By adding a public method to allow resetting the manual update time?
Perhaps it's worth filing an issue on the teslajsonpy repo, so this doesn't get forgotten.

Yes, let's open an issue. Basically what you're saying we need is a way to force a device to reread its info from cache.

I'm ok with the current approach for a 1.0 type release, but it makes sense to resolve it more cleanly in the future.

alandtse · 2022-04-23T19:04:24Z

custom_components/tesla_custom/helpers.py

+                # So it could eat into our timeout, and this is fine.
+                # We'll try to turn the set the status, and check again
+                try:
+                    await climate_device.set_status(True)


How often on your system does this hit the Tesla endpoint? The issue I'm concerned about is that each api call has a wake call which includes 5 separate retries. Since this loop in theory can hit 15 times and if there's a wakeup that quickly goes to 75 api hits. Are we slamming the endpoint unnecessarily?

alandtse · 2022-04-23T19:06:33Z

custom_components/tesla_custom/climate.py

+        # This is really gross, and i kinda hate it.
+        # but its the only way i could figure out how to force an update on the underlying device
+        # thats in the teslajsonpy library.
+        # This could be fixed by doing a pr in the underlying library,


Yes, let's open an issue. Basically what you're saying we need is a way to force a device to reread its info from cache.

I'm ok with the current approach for a 1.0 type release, but it makes sense to resolve it more cleanly in the future.

Megabytemb · 2022-04-23T23:19:09Z

Ok, combining your feedback and more testing, I've more tightly controlled when we go to the Tesla API for an update, and forcing other devices to read from cache.

We're actually a little behind on feature parody with tesla's app, and i this this is due to the need to update 2 libraries to implement a new feature.
Want me to open up a separate issue on this library so we can discuss the idea of moving all home assistant logic into this library, and purely using teslajsonpy as the transport layer?
That way we can implement things like a shared car state across entities, and develop with more speed. There's a bunch in the API that we should probably look to implement for feature completeness.

alandtse · 2022-04-24T00:33:05Z

Great. This looks better. Can you rebase off dev so I can then merge this in please?

We already have that issue to do the separation. It's just been a lack of motivation to be honest. zabuldon/teslajsonpy#24

custom_components/tesla_custom/climate.py

Heated Seats are off if climate control is off. get seat heat level returns None if climate is off.

We need to wait for HVAC as the heater cannot be turned on while HVAC is off.

…nto Heated-Steering-Wheel

Megabytemb · 2022-04-24T12:10:31Z

I think we're in a good place now for a merge into dev.

alandtse · 2022-04-24T20:46:58Z

FYI @Megabytemb you need to use a dict.get(). See #199

Megabytemb · 2022-04-24T22:44:48Z

Created
zabuldon/teslajsonpy#308
in the teslajsonpy library, where the code is located.

Megabytemb added 3 commits April 9, 2022 13:07

Add Support for Select and Heated Seats

1c83a90

Match Tesla Functionality.

fd1db45

Heated Seats are off if climate control is off. get seat heat level returns None if climate is off.

Add Support for Heated Steering Wheel

223e74f

Wait for HVAC to turn on.

7942e5a

We need to wait for HVAC as the heater cannot be turned on while HVAC is off.

shocklateboy92 reviewed Apr 23, 2022

View reviewed changes

Megabytemb added 2 commits April 23, 2022 13:00

Merge branch 'add-heated-seats' of github.com:megabytemb/tesla into H…

4dd0668

…eated-Steering-Wheel. Heated Seats and Heated Steering Wheel will now share logic

Sizeable refactor to snable shared climate states.

f0d3552

Megabytemb mentioned this pull request Apr 23, 2022

Add Heated Seats support via the Select entity #187

Closed

Megabytemb changed the title ~~Add Support for Heated Steering Wheel~~ Add Support for Heated Steering Wheel and Seats Apr 23, 2022

shocklateboy92 approved these changes Apr 23, 2022

View reviewed changes

alandtse reviewed Apr 23, 2022

View reviewed changes

tigher controls over climate Refresh hack

8300989

shocklateboy92 reviewed Apr 24, 2022

View reviewed changes

custom_components/tesla_custom/climate.py Outdated Show resolved Hide resolved

Megabytemb added 3 commits April 24, 2022 22:01

Add Support for Select and Heated Seats

7e23a21

Match Tesla Functionality.

e8dbc3d

Heated Seats are off if climate control is off. get seat heat level returns None if climate is off.

Add Support for Heated Steering Wheel

1e9867d

Megabytemb added 3 commits April 24, 2022 22:01

Wait for HVAC to turn on.

c2324e7

We need to wait for HVAC as the heater cannot be turned on while HVAC is off.

Sizeable refactor to snable shared climate states.

4a4bba6

tigher controls over climate Refresh hack

bfd8cb5

Megabytemb force-pushed the Heated-Steering-Wheel branch from 8300989 to bfd8cb5 Compare April 24, 2022 12:01

Megabytemb added 3 commits April 24, 2022 22:04

Merge branch 'Heated-Steering-Wheel' of github.com:megabytemb/tesla i…

98313e0

…nto Heated-Steering-Wheel

remove unneeded library from testing

c6124d2

add check_for_reauth to be consistent.

f300afd

alandtse merged commit c052539 into alandtse:dev Apr 24, 2022

alandtse mentioned this pull request Oct 22, 2022

Steering wheel heater switch #283

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Support for Heated Steering Wheel and Seats #188

Add Support for Heated Steering Wheel and Seats #188

Megabytemb commented Apr 9, 2022

alandtse commented Apr 9, 2022

Megabytemb commented Apr 10, 2022

alandtse commented Apr 10, 2022

alandtse commented Apr 19, 2022

Megabytemb commented Apr 19, 2022

shocklateboy92 commented Apr 23, 2022

Megabytemb commented Apr 23, 2022

alandtse commented Apr 23, 2022

shocklateboy92 commented Apr 23, 2022

Megabytemb commented Apr 23, 2022

alandtse commented Apr 23, 2022

shocklateboy92 Apr 23, 2022

alandtse Apr 23, 2022

alandtse Apr 23, 2022

Megabytemb commented Apr 23, 2022

shocklateboy92 Apr 23, 2022

alandtse Apr 23, 2022

alandtse Apr 23, 2022

alandtse Apr 23, 2022

Megabytemb commented Apr 23, 2022

alandtse commented Apr 24, 2022

Megabytemb commented Apr 24, 2022

alandtse commented Apr 24, 2022

Megabytemb commented Apr 24, 2022

Add Support for Heated Steering Wheel and Seats #188

Add Support for Heated Steering Wheel and Seats #188

Conversation

Megabytemb commented Apr 9, 2022

Add Heated Steering Wheel Switch

alandtse commented Apr 9, 2022

Megabytemb commented Apr 10, 2022

alandtse commented Apr 10, 2022

alandtse commented Apr 19, 2022

Megabytemb commented Apr 19, 2022

shocklateboy92 commented Apr 23, 2022

Megabytemb commented Apr 23, 2022

alandtse commented Apr 23, 2022

shocklateboy92 commented Apr 23, 2022

Megabytemb commented Apr 23, 2022

alandtse commented Apr 23, 2022

shocklateboy92 Apr 23, 2022

Choose a reason for hiding this comment

alandtse Apr 23, 2022

Choose a reason for hiding this comment

alandtse Apr 23, 2022

Choose a reason for hiding this comment

Megabytemb commented Apr 23, 2022

shocklateboy92 Apr 23, 2022

Choose a reason for hiding this comment

alandtse Apr 23, 2022

Choose a reason for hiding this comment

alandtse Apr 23, 2022

Choose a reason for hiding this comment

alandtse Apr 23, 2022

Choose a reason for hiding this comment

Megabytemb commented Apr 23, 2022

alandtse commented Apr 24, 2022

Megabytemb commented Apr 24, 2022

alandtse commented Apr 24, 2022

Megabytemb commented Apr 24, 2022