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

[gree] Allow wider temperature range #14217

Merged
merged 3 commits into from
Jan 14, 2023
Merged

Conversation

lsiepel
Copy link
Contributor

@lsiepel lsiepel commented Jan 14, 2023

  • Fix checkstyle
  • Allow wider range of temperatures

Previously the binding limits the setpoint temperature from 16-30. As devices have different operating range, it would be better to have a wider allowed range and adjust it with sitemap minValue/ maxValue according to your specific device.

Tested and confirmed. Devices gracefully ignore temperatures outside of the operating range. So regressions or device mallfunction is not to be expected.

Fixes: #14207

Signed-off-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: lsiepel <leosiepel@gmail.com>
@lsiepel lsiepel added the bug An unexpected problem or unintended behavior of an add-on label Jan 14, 2023
@lsiepel lsiepel requested a review from markus7017 as a code owner January 14, 2023 11:01
Signed-off-by: lsiepel <leosiepel@gmail.com>
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

LGTM, thank you

@lolodomo lolodomo merged commit b9bec52 into openhab:main Jan 14, 2023
@lolodomo lolodomo added this to the 4.0 milestone Jan 14, 2023
@lsiepel lsiepel deleted the free-temp-range branch January 14, 2023 11:14
@jlaur
Copy link
Contributor

jlaur commented Jan 14, 2023

@lsiepel - does it work correctly when having this definition where minimum is still 16?

<channel-type id="targetTemperature">
<item-type>Number:Temperature</item-type>
<label>@text/channel-type.gree.targettemperature.label</label>
<description>@text/channel-type.gree.targettemperature.description</description>
<state min="16" max="86" step="1" pattern="%d %unit%"></state>
</channel-type>

@lolodomo
Copy link
Contributor

I probably merged too fast, sorry !

@lsiepel
Copy link
Contributor Author

lsiepel commented Jan 14, 2023

The user that tested was able to send a 7 celsius command, guess he used textual config as the quoted code would only affect Graphic config right? I'll make a PR fast :-)

@lsiepel
Copy link
Contributor Author

lsiepel commented Jan 14, 2023

I probably merged too fast, sorry !

Sorry, i also missed it.

nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Feb 28, 2023
* Fix checkstyle
* Allow wider temp range
* Remaining checkstyle

Signed-off-by: lsiepel <leosiepel@gmail.com>
renescherer pushed a commit to renescherer/openhab-addons that referenced this pull request Mar 23, 2023
* Fix checkstyle
* Allow wider temp range
* Remaining checkstyle

Signed-off-by: lsiepel <leosiepel@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of an add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[gree] binding does not allow target temperatures below 16°C
3 participants