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

Support Climate Devices v1 #64

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rdehuyss
Copy link

This adds support for climate devices.

  • It works the same as the cover domain.
  • It waits 3 seconds after the latest change by the user before doing a service call (climate.set_temperature)

This version only shows the thermostat, not the temperature.

This adds support for climate devices. 
- It works the same as the cover domain.
- It waits 3 seconds after the latest change by the user before doing a service call (climate.set_temperature)

This version only shows the thermostat, not the temperature.
Copy link
Owner

@nervetattoo nervetattoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work fine but please look at the specific review points

@@ -381,6 +383,45 @@ class BannerCard extends LitElement {
</div>
`;
}

renderDomainClimate(options) {
let entity_id = options.entity.split(".")[1];
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please camelCase variables

};
let onThermostatArrowClick = (diff) => {
clearTimeout(this._updateThermostateTemperatureTimeout);
options.attributes.temperature += diff;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modifying the passed state object is not a good idea. A pure HA state object has a single source or truth, but when modifying it it gets confusing. I get that waiting for state changes to come back from HA takes too long for climate devices though. Perhaps keeping a this.stagedChanges = { [entityId]: { temperature: newTemperature } } or something similar could work. It would have to for example use a timestamp that is used in set hass (){} to clear the staged changes once a newer value comes from HA.

role="button"
@click=${onThermostatArrowUpClick}
></paper-icon-button>
<span class="${entity_id}_value">${thermostatTemperature.toFixed(1)}</span>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value + the heading should be clickable to bring up the native more info dialog like it works for for example sensors.

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

Successfully merging this pull request may close these issues.

2 participants