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

[homekit] add support for min/max values for temperature #7782

Merged
merged 2 commits into from
May 27, 2020

Conversation

yfre
Copy link
Contributor

@yfre yfre commented May 25, 2020

HomeKit specification supports custom min/max values as well as the min step for some characteristics, including temperature. For instance, an outside temperature sensor can have a different temperature range than an inside temperature sensor.

Java-HAP library added support for custom min/max values recently.

This PR add supports for min/max/step to HomeKit binding.

changes:

  • update to latest Java-HAP library (minor update)
  • add support for configurable min/max/step values at item level for temperatures.
  • reduce level of one logging warning which is rather common and harmless

Closes #3400

Signed-off-by: Eugen Freiter freiter@gmx.de

Signed-off-by: Eugen Freiter <freiter@gmx.de>
@yfre yfre added the enhancement An enhancement or new feature for an existing add-on label May 25, 2020
@TravisBuddy
Copy link

Travis tests were successful

Hey @yfre,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

Signed-off-by: Eugen Freiter <freiter@gmx.de>
@TravisBuddy
Copy link

Hey @yfre,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: c20296f0-9f6e-11ea-a34f-dd9ab62511aa

@cpmeister
Copy link
Contributor

cpmeister commented May 27, 2020

Just closing and reopening to see if I can get the DCO error to go away.

@cpmeister cpmeister closed this May 27, 2020
@cpmeister cpmeister reopened this May 27, 2020
@cpmeister cpmeister merged commit cd3f520 into openhab:2.5.x May 27, 2020
@cpmeister cpmeister added this to the 2.5.6 milestone May 27, 2020
@TravisBuddy
Copy link

Hey @yfre,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: 6260a8a0-a049-11ea-aa49-9d3926c79f60

Dries-Vandenneucker added a commit to Dries-Vandenneucker/openhab-addons that referenced this pull request May 31, 2020
* 2.5.x: (174 commits)
  [hpprinter] Add additional data points and refactoring (openhab#7805)
  [neohub] new/legacy API; null annotations; enhancements; bugs; logging (openhab#7767)
  [meteoalerte] Initial contribution (openhab#7200)
  [lgwebos] Console command to show the access key (openhab#7801)
  [hue] Refactored state handling and fix polling after command (openhab#7518)
  [telegram] add attachment URL (openhab#7816)
  [siemensrds] readme adjusted to match openhab#7814 (openhab#7819)
  [lametrictime] correctly parse response (openhab#7818)
  [Seneye] Bug fix for using Pond or Home sensors. (openhab#7797)
  [siemensrds] apply UoM quantityType percent for relative humidity (openhab#7814)
  [alarmdecoder] Add vzone thing for virtual zone control (openhab#7800)
  [hue] Channel alert added for groups (openhab#7810)
  [hue] Keep compatibility with hue emulation for groups (openhab#7809)
  [dscalarm] Bridge/things management refactored (openhab#7748)
  [avmfritz] Add link to Fensterkontakt (magnetisch) to docs (openhab#7806)
  [deconz] add light/blind support and additional sensors (openhab#7608)
  [homekit] add support for min/max values for temperature (openhab#7782)
  [tesla] Use CXF JAX-RS client builder, if available (openhab#7804)
  [mqtt.homie] Improve Homie discovery time (openhab#7760)
  [siemensrds] null annotations; JUnit; UoM; enhancements; bug; refactoring; logging (openhab#7769)
  ...
@yfre yfre deleted the min_max_values branch June 20, 2020 17:31
@Hobohome
Copy link

I think there is an issue with the [minValue=10.5, maxValue=27] function in that it does not accept a negative value.
If I use a value [minValue=-25, maxValue=30] (for a freezer for example) the item file is rejected with a message ...
Configuration model 'custom.items' has errors, therefore ignoring it: [311,124]: no viable alternative at input '-'

Number Coolroom_Temp "Coolroom [%.1f°C ]" (gSQLpersist_everyChange)
{ channel="mqtt:topic:pi1_mqtt:Coolroom_Sensor:Coolroom_Temp",homekit = "TemperatureSensor.CurrentTemperature" [minValue=-25, maxValue=30],alexa="TemperatureSensor.temperature" }

@yfre
Copy link
Contributor Author

yfre commented Jun 30, 2020

@Hobohome thank you for reporting.
i have discovered this as well. but this is an issue in openHab core (it cannot parse negative number)
issue: openhab/openhab-core#1501
fix: openhab/openhab-core#1517

but the fix will be first available with openhab 3.0. we cannot do much until then.
i will add this to the documentation as "known limitations".

@Hobohome
Copy link

Hay @yfre and thanks for the reply. Do you know if it is possible still to over-ride the global default min/max in the Homekit config? I read that adding two custom parameters in PaperUI (maximumTemperature and minimumTemperature) this could be done.
I have tried this and still get an error when the value returned from the sensor is less than 10 degC. Perhaps I have not done this correctly?

@yfre
Copy link
Contributor Author

yfre commented Jul 1, 2020

@Hobohome you have done all correctly. it is documentation that is not up-to-date. the global defaults are not supported anymore. they were dropped in favour of local, item level, settings.

but if "-" is not supported at item level, maybe we should add global level back.. or support both, check local, if not there, take global. let me see if this can be easily done

as of now, there is no way to define "-" for temperatures. sorry for that.

@Hobohome
Copy link

Hobohome commented Jul 2, 2020

Thanks @yfre . I think having both global and item level would be great. Allow the item level value (when specified) to override the global values.

J-N-K pushed a commit to J-N-K/openhab-addons that referenced this pull request Jul 14, 2020
* add support for min/max values for temperature
* run spotless

Signed-off-by: Eugen Freiter <freiter@gmx.de>
CSchlipp pushed a commit to CSchlipp/openhab-addons that referenced this pull request Jul 26, 2020
* add support for min/max values for temperature
* run spotless

Signed-off-by: Eugen Freiter <freiter@gmx.de>
Signed-off-by: CSchlipp <christian@schlipp.de>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
* add support for min/max values for temperature
* run spotless

Signed-off-by: Eugen Freiter <freiter@gmx.de>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
* add support for min/max values for temperature
* run spotless

Signed-off-by: Eugen Freiter <freiter@gmx.de>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
* add support for min/max values for temperature
* run spotless

Signed-off-by: Eugen Freiter <freiter@gmx.de>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
* add support for min/max values for temperature
* run spotless

Signed-off-by: Eugen Freiter <freiter@gmx.de>
DaanMeijer pushed a commit to DaanMeijer/openhab-addons that referenced this pull request Sep 1, 2020
* add support for min/max values for temperature
* run spotless

Signed-off-by: Eugen Freiter <freiter@gmx.de>
Signed-off-by: Daan Meijer <daan@studioseptember.nl>
markus7017 pushed a commit to markus7017/openhab-addons that referenced this pull request Sep 19, 2020
* add support for min/max values for temperature
* run spotless

Signed-off-by: Eugen Freiter <freiter@gmx.de>
@grzegorz914
Copy link

grzegorz914 commented Nov 8, 2020

Hi,

minValue and maxValue for Thermostat and Temperature Sensor don’t work in OH 3.0.
Also negative minValue are not acceptable in config.

item:
Number temperature_sensor "Temperature Sensor [%.1f C]" {homekit="TemperatureSensor" [minValue=0, maxValue=100] }

2020-11-08 09:11:19.336 [WARN ] [istics.impl.base.FloatCharacteristic] - Detected value out of range 4.8. Returning min value instead. Characteristic io.github.hapjava.characteristics.impl.thermostat.CurrentTemperatureCharacteristic@16061a5

@pug304
Copy link

pug304 commented Dec 13, 2020

Hi,

minValue and maxValue for Thermostat and Temperature Sensor don’t work in OH 3.0.
Also negative minValue are not acceptable in config.

item:
Number temperature_sensor "Temperature Sensor [%.1f C]" {homekit="TemperatureSensor" [minValue=0, maxValue=100] }

2020-11-08 09:11:19.336 [WARN ] [istics.impl.base.FloatCharacteristic] - Detected value out of range 4.8. Returning min value instead. Characteristic io.github.hapjava.characteristics.impl.thermostat.CurrentTemperatureCharacteristic@16061a5

it seems issue stil is existing in most recent version unstable 3.0.0~S2072-1

@yfre
Copy link
Contributor Author

yfre commented Dec 13, 2020

@pug304
hm. it should work. i recently tested it with OH3 using items and using UI.
what you see in karaf if you typ

metadata list
or
metadata list <item_name>

@pug304
Copy link

pug304 commented Dec 13, 2020

Metadata [key=homekit:TemperaturGarage_1_ACTUAL_TEMPERATURE, value=TemperatureSensor.CurrentTemperature, configuration=[]]
Metadata [key=stateDescription:TemperaturGarage_1_ACTUAL_TEMPERATURE, value= , configuration=[step=0.1, min=-30, max=40]]
Metadata [key=semantics:TemperaturGarage_1_ACTUAL_TEMPERATURE, value=Point, configuration=[isPointOf=TemperaturGarage]]

due to out of limits oh reports 10° to homekit, i've tested with min=0 and different steps

edit: same behavior on two different sensors, have deleted and readded one of them

@yfre
Copy link
Contributor Author

yfre commented Dec 13, 2020

can you try to close home app and keep it closed for 10-20 seconds and open again?
or add sensor with different name?

home app request the complete list of accessories only at very beginning or at new start after longer time. once it has it, it only asks for actual values and not for meta data.

@pug304
Copy link

pug304 commented Dec 13, 2020

i will give this a try tomorrow :)

@grzegorz914
Copy link

Hi,
minValue and maxValue working OK but step not.

@pug304
Copy link

pug304 commented Dec 14, 2020

have doen this steps:

  • close all Homekit apps, shut down Homekit relay device

  • restart Homekit on one device, no change - and stopped the app after

  • removed one temperature sensor and re-added with another name

  • set minValue/maxValue/Step

  • set Homekit metadata

  • also no change :(

error: 18:17:29.892 [WARN ] [ristics.impl.base.FloatCharacteristic] - Detected value out of range 41.0. Returning max value instead. Characteristic io.github.hapjava.characteristics.impl.thermostat.TargetTemperatureCharacteristic@2853999d

may be the error is because of setting a step value

@grzegorz914
Copy link

After U set metadata with minValue/maxValue/step stop and start OH with:

sudo /bin/systemctl stop openhab.service
sudo /bin/systemctl start openhab.service

@pug304
Copy link

pug304 commented Dec 14, 2020

  • close all Homekit apps, shut down Homekit relay device
  • deleted model, things & items
  • sudo /bin/systemctl restart openhab.service
  • recreated everything
  • set minValue/maxValue/Step
  • sudo /bin/systemctl restart openhab.service
  • set Homekit metadata
  • sudo /bin/systemctl restart openhab.service

without success

@yfre
Copy link
Contributor Author

yfre commented Dec 14, 2020

thank you for trying all this.
strange it is not working. the logic in the code is pretty simple, probably 10 lines in total.
can you share you exact configuration for one item, which is not working and how exactly you do the configuration.
this will help to reproduce and fix the bug

@grzegorz914
Copy link

grzegorz914 commented Dec 14, 2020

@pug304
I think U set minVaule and maxValue in wrong place, U need to set this in HomeKit metadata config file not in the item config itself.

@pug304
Copy link

pug304 commented Dec 14, 2020

@pug304
I think U set minVaule and maxValue in wrong place, U need to set this in HomeKit metadata config file not in the item config itself.

IMHO this changed in OH3, you have to set the values in the item definition

@grzegorz914
Copy link

8E6036A4-A82C-460A-8115-8E9D77119BA1
6F2B018F-A068-4AF7-B230-FF55EE14674D

@pug304
Copy link

pug304 commented Dec 14, 2020

thank you for trying all this.
strange it is not working. the logic in the code is pretty simple, probably 10 lines in total.
can you share you exact configuration for one item, which is not working and how exactly you do the configuration.
this will help to reproduce and fix the bug

for sure, no secret ;)

thing code


UID: homematic:HmIP-STHO-A:3014F711A0001F98A9AA0C5E:0010DA499476AE
label: MyTemperaturGarage
thingTypeUID: homematic:HmIP-STHO-A
configuration:
HMP_0_CYCLIC_INFO_MSG_DIS_UNCHANGED: 0
HMP_2_COND_TX_CYCLIC_BELOW: false
HMP_3_EVENT_DELAY_VALUE: 3
HMP_3_EVENT_RANDOMTIME_VALUE: 1
HMP_2_EVENT_RANDOMTIME_UNIT: S
HMP_2_COND_TX_RISING: false
HMP_3_COND_TX_FALLING: false
HMP_0_ENABLE_ROUTING: true
HMP_2_COND_TX_CYCLIC_ABOVE: false
HMP_3_EVENT_DELAY_UNIT: S
HMP_0_LOW_BAT_LIMIT: 2.4
HMP_1_TEMPERATURE_OFFSET: 0
HMP_2_COND_TX_FALLING: false
HMP_3_COND_TX_CYCLIC_ABOVE: false
HMP_2_EVENT_RANDOMTIME_VALUE: 1
HMP_3_COND_TX_DECISION_ABOVE: 200
HMP_0_CYCLIC_INFO_MSG_OVERDUE_THRESHOLD: 2
HMP_2_EVENT_DELAY_UNIT: S
HMP_2_COND_TX_DECISION_BELOW: 0
HMP_2_COND_TX_THRESHOLD_LO: 26
HMP_2_EVENT_DELAY_VALUE: 3
HMP_0_CYCLIC_INFO_MSG_DIS: 0
HMP_2_COND_TX_THRESHOLD_HI: 27
HMP_0_LOCAL_RESET_DISABLED: false
HMP_0_ARR_TIMEOUT: 10
HMP_3_COND_TX_THRESHOLD_LO: 70
HMP_0_DUTYCYCLE_LIMIT: 180
HMP_3_COND_TX_THRESHOLD_HI: 80
HMP_3_COND_TX_DECISION_BELOW: 0
HMP_3_EVENT_RANDOMTIME_UNIT: S
HMP_3_COND_TX_CYCLIC_BELOW: false
HMP_2_COND_TX_DECISION_ABOVE: 200
HMP_0_CYCLIC_INFO_MSG: 1
HMP_3_COND_TX_RISING: false
bridgeUID: homematic:bridge:3014F711A0001F98A9AA0C5E


Item/Link

grafik


Model

grafik


medadata

grafik

grafik

TemperatureSensor tag is set in regarding model

other then in OH2 (all definitions inside .thing ans .item files) i've created all in the UI

@pug304
Copy link

pug304 commented Dec 14, 2020

8E6036A4-A82C-460A-8115-8E9D77119BA1
6F2B018F-A068-4AF7-B230-FF55EE14674D

ah i see....

@grzegorz914
Copy link

..... but Step not working

@pug304
Copy link

pug304 commented Dec 14, 2020

great! indeed this is working this way! Thanks for your explanation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[homekit] Make thermostat temperature limits configurable
6 participants