This repository has been archived by the owner on Jun 23, 2022. It is now read-only.
Allow setting climate mode & set target temperature min/max/step from hass data #178
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Current temperature does not need min/max values, it has a default already.
|
@andersonshatch, thanks for your PR! By analyzing the history of the files in this pull request, we identified @robbiet480 and @Minims to be potential reviewers. |
houndci-bot
reviewed
Jun 25, 2017
accessories/climate.js
Outdated
|
|
||
| var that = this; | ||
|
|
||
| this.client.callService(this.domain, 'set_operation_mode', serviceData, function(data) { |
There was a problem hiding this comment.
Missing space before function parentheses space-before-function-paren
| serviceData.entity_id = this.entity_id; | ||
|
|
||
| var mode = ''; | ||
| switch (value) { |
There was a problem hiding this comment.
Expected a default case default-case
| } else { | ||
| callback(communicationError); | ||
| } | ||
| }); | ||
| }, | ||
|
|
||
| setTargetHeatingCoolingState: function (value, callback, context) { |
There was a problem hiding this comment.
'context' is defined but never used no-unused-vars
accessories/climate.js
Outdated
| state = Characteristic.TargetHeatingCoolingState.OFF; | ||
| break; | ||
| } | ||
| callback(null, state); |
There was a problem hiding this comment.
Unexpected tab character no-tabs
Mixed spaces and tabs no-mixed-spaces-and-tabs
|
Thanks! 🐬 🍪 💯 |
This was referenced Jul 19, 2017
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR includes 2 enhancements to the climate accessory:
The ability to set the operation mode of the climate device.
Currently, the 4 HomeKit thermostat modes are Off, Heat, Cool & Auto, these are respectively mapped to the home-assistant climate modes of idle, heat, cool & auto.
In addition, the HK mode is now properly set based on the home-assistant operation mode.
(in future it will be nice to customise this mapping in the homebridge config as climate devices in hass may have different modes (e.g. 'dry', 'fan_only'), but this is a good first step)
The target temperature minimum temperature, maximum temperature and temperature step are now retrieved from the home-assistant climate device's attributes, rather than being hard coded in the climate accessory.
I removed the min/max values which were previously set on the current temperature attribute, as it's possible that those values will be outside the min/max target temperatures of the climate device, and the current temperature attribute itself seems to have sensible defaults.