Fixed race condition in Generic Thermostat

[aronsky]

Description:

Generic Thermostat has a race condition in its keepalive feature when the switching of the heater and the keepalive event happen at the same time. Here's a basic outline of the problem:

1. Keepalive event begins executing, and checks which command to repeat (keepalive), based on the current status
2. Context switches to the event of switching the heater. The event completes, and the heater is switched to the opposite state.
3. Context returns to the keepalive event - which now retransmits the command of the old state, reverting the switch that just happened.

On top of the almost instantaneous turning on and off of the compressor (which can't be very healthy for it), the heater gets stuck in the old state for another min_cycle_duration, causing the temperature to drift further away from the target.

The issue is solved in this pull request by introducing a lock. Due to the async nature of the lock, the _async_control_heating is rewritten as async. Consequently, all methods that it calls are made async, and all the methods that call those methods, and so on. Currently, there's a mix of async and regular methods throughout the code - it's messy, but will be addressed in a future pull request, to limit the number of reviewable changes in this PR.

Additionally, _async_control_heating was refactored to be more concise, and accommodate the old keepalive functionality (the old async_keep_alive is now obsolete and, therefore, removed).

Checklist:

• The code change is tested and works locally.
• Local tests pass with tox. Your PR cannot be merged unless tests pass

[aronsky]

@amelchio @balloob @MartinHjelmare
It's a smaller PR than the previous one, with minimal changes only relevant to the fix itself. What do you think?

[amelchio]

Looks good to me.

I enjoyed your thorough description, it makes reading the diff that much easier.

[amelchio]

This looks a bit odd, maybe add a comment or named variable that tells the reader that this is the keepalive case?

[aronsky]

You're right, I'll add a comment.

[amelchio]

You have removed the logging that mentions AC and I think that is okay because the configuration for the entity_id is indeed named heater, even in ac_mode.

[aronsky]

Yup. I hope we can come up with a better, more neutral naming convention (as heater is very, well, heating-oriented :) ), and when we do, I'll update the logging accordingly.