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

[hue] Add channels for time of last sensor update (API v2) #15552

Merged
merged 7 commits into from
Oct 26, 2023

Conversation

jlaur
Copy link
Contributor

@jlaur jlaur commented Sep 5, 2023

This introduces new channels:

  • motion-last-updated for API v2 which is equivalent to channel last_updated for API v1 thing type 0107 (motion sensor).
  • temperature-last-updated for API v2 which is equivalent to channel last_updated for API v1 thing type 0302 (temperature sensor).
  • light-level-last-updated for API v2 which is equivalent to channel last_updated for API v1 thing type 0106 (light sensor).
  • button-last-updated for API v2.
  • rotary-steps-last-updated for API v2.

Additionally, the motion / temperature / light-level / button-last-event / rotary-steps channels have been refactored to use new DTO fields with fallback to the now deprecated fields.

Resolves #15546
Resolves #15599

Relevant DTO documentation

GET /resource/motion/{id} response

motion: required(object)

  • motion: required(boolean)
    Deprecated. Moved to motion_report/motion.
  • motion_valid: required(boolean)
    Deprecated. Motion is valid when motion_report property is present, invalid when absent.
  • motion_report: (object)
    • changed: required(datetime)
      last time the value of this property is changed.
    • motion: required(boolean)
      true if motion is detected

See https://developers.meethue.com/develop/hue-api-v2/api-reference/#resource_motion__id__get for full response.

GET /resource/temperature/{id} response

temperature: required(object)

  • temperature: required(number)
    Deprecated. Moved to Temperature_report/temperature
  • temperature_valid: required(boolean)
    Deprecated. Indication whether the value presented in temperature is valid
  • temperature_report: (object)
    • changed: required(datetime)
      last time the value of this property is changed.
    • temperature: required(number)
      Temperature in 1.00 degrees Celsius

See https://developers.meethue.com/develop/hue-api-v2/api-reference/#resource_temperature__id__get for full response.

GET /resource/light_level/{id} response

light: required(object)

  • light_level: required(integer)
    Deprecated. Moved to light_level_report/light_level
  • light_level_valid: required(boolean)
    Deprecated. Indication whether the value presented in light_level is valid
  • light_level_report: (object)
    • changed: required(datetime)
      last time the value of this property is changed.
    • light_level: required(integer)
      Light level in 10000*log10(lux) +1 measured by sensor. Logarithmic scale used because the human eye adjusts to light levels and small changes at low lux levels are more noticeable than at high lux levels. This allows use of linear scale configuration sliders.

See https://developers.meethue.com/develop/hue-api-v2/api-reference/#resource_light_level__id__get for full response.

GET /resource/button/{id} response

button: required(object)

  • last_event: (one of initial_press, repeat, short_release, long_release, double_short_release, long_press)
    Deprecated. Move to button_report/event
  • button_report: (object)
    • updated: required(datetime)
      last time the value of this property is updated.
    • event: required(one of initial_press, repeat, short_release, long_release, double_short_release, long_press)
      event which can be send by a button control
  • repeat_interval: (integer)
    Duration of a light transition or timed effects in ms.
  • event_values: (array of ButtonEvent)
    list of all button events that this device supports

See https://developers.meethue.com/develop/hue-api-v2/api-reference/#resource_button__id__get for full response.

GET /resource/relative_rotary/{id} response

relative_rotary: required(object)

  • last_event: (object)
    Deprecated. Renamed to RelativeRotaryReport. Indicate which type of rotary event is received
  • action: required(one of start, repeat)
    Indicate which type of rotary event is received
  • rotation: required(object)
    • direction: required(one of clock_wise, counter_clock_wise)
      A rotation opposite to the previous rotation, will always start with new start command.
    • steps: required(integer – minimum: 0 – maximum: 32767)
      amount of rotation since previous event in case of repeat, amount of rotation since start in case of a start_event. Resolution = 1000 steps / 360 degree rotation.
    • duration: required(integer – minimum: 0 – maximum: 65534)
      duration of rotation since previous event in case of repeat, amount of rotation since start in case of a start_event. duration is specified in miliseconds.
  • rotary_report: (object)
    • updated: required(datetime)
      last time the value of this property is updated.
    • action: required(one of start, repeat)
      Indicate which type of rotary event is received
    • rotation: required(object)
      • direction: required(one of clock_wise, counter_clock_wise)
        A rotation opposite to the previous rotation, will always start with new start command.
      • steps: required(integer – minimum: 0 – maximum: 32767)
        amount of rotation since previous event in case of repeat, amount of rotation since start in case of a start_event. Resolution = 1000 steps / 360 degree rotation.
      • duration: required(integer – minimum: 0 – maximum: 65534)
        duration of rotation since previous event in case of repeat, amount of rotation since start in case of a start_event. duration is specified in miliseconds.

See https://developers.meethue.com/develop/hue-api-v2/api-reference/#resource_relative_rotary__id__get for full response.

@jlaur jlaur added the enhancement An enhancement or new feature for an existing add-on label Sep 5, 2023
@jlaur jlaur force-pushed the 15546-hue-motion-last-updated branch 2 times, most recently from 67919f7 to c1598f6 Compare September 6, 2023 18:55
@jlaur jlaur changed the title [hue] Add channel for last motion sensor update (API v2) [hue] Add channels for last motion/temperature sensor update (API v2) Sep 6, 2023
@jlaur jlaur marked this pull request as ready for review September 6, 2023 19:01
@jlaur
Copy link
Contributor Author

jlaur commented Sep 6, 2023

Small note (to self / @andrewfg): Consider trying to figure out when motion_report/temperature_report were introduced. If the old fields were already deprecated before first non-beta CLIP 2 firmware, there is probably no need to maintain backwards compatibility.

@andrewfg
Copy link
Contributor

andrewfg commented Sep 8, 2023

@jlaur I will study this in more detail over the weekend, but I already have some quick suggestions..

  • It is inconsistent to implement this xxx-report model only for motion and temperature sensors. You should implement it for light-level, button, and relative-rotary sensor channels as well.
  • Last week Philips Signify introduced their range of home security products, and consequently they added new resource types for camera-motion, contact and tamper security sensors. These also use the xxx-report model. The full support for those new resource / things should not be added in this PR, but please consider if there is anything necessary in this PR in order prepare the foundations for that subsequent PR to come.
  • So I suggest the DTOs for motion-report, temperature-report, button-report, light-level-report, relative-rotary-report, tamper-report, contact-report, camera-motion-report etc. should all descend from a common base-sensor-report class that implements the updated resp. changed JSON field. Note: I don't know why some sensor DTO's use updated and others use changed field names, but apparently they are synonyms, so the base class could deserialize both these alternate field names as one actual class field.

EDIT: @jlaur as this PR has increased in scope from your original concept, I am happy to take over the coding, if you prefer. Please let me know.


EDIT 2: probably the 'camera-motion' sensor could be coded in OH as the same channel as current 'motion' sensor; the advantage of doing that would be that we could reduce the total number of channels in OH things. WDYT?

@jlaur jlaur force-pushed the 15546-hue-motion-last-updated branch from 41cac0f to db9457a Compare September 10, 2023 18:21
Copy link
Contributor

@andrewfg andrewfg left a comment

Choose a reason for hiding this comment

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

  • It is inconsistent to implement this xxx-report model only for motion and temperature sensors. You should implement it for light-level, button, and relative-rotary sensor channels as well.
  • Last week Philips Signify introduced their range of home security products, and consequently they added new resource types for camera-motion, contact and tamper security sensors. These also use the xxx-report model. The full support for those new resource / things should not be added in this PR, but please consider if there is anything necessary in this PR in order prepare the foundations for that subsequent PR to come.
  • So I suggest the DTOs for motion-report, temperature-report, button-report, light-level-report, relative-rotary-report, tamper-report, contact-report, camera-motion-report etc. should all descend from a common base-sensor-report class that implements the updated resp. changed JSON field. Note: I don't know why some sensor DTO's use updated and others use changed field names, but apparently they are synonyms, so the base class could deserialize both these alternate field names as one actual class field.

@jlaur jlaur changed the title [hue] Add channels for last motion/temperature sensor update (API v2) [hue] Add channels for time of last sensor update (API v2) Sep 11, 2023
@andrewfg
Copy link
Contributor

Consider trying to figure out when motion_report/temperature_report were introduced.

@jlaur see the following post from March this year..

https://hueblog.com/2023/03/14/hue-bridge-update-brings-back-popular-feature/

Copy link
Contributor

@andrewfg andrewfg left a comment

Choose a reason for hiding this comment

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

I have just created #15601 to add support for security devices. And #15602 to fix the deprecated DTOs for button and relative_rotary sensors. They both depend on some underlying infrastructure that I am expecting to be introduced in this PR (e.g. the BaseReport class).

@jlaur can you please look at #15601 and #15602 to give you an idea what I am counting on?

@jlaur
Copy link
Contributor Author

jlaur commented Sep 17, 2023

It is inconsistent to implement this xxx-report model only for motion and temperature sensors. You should implement it for light-level, button, and relative-rotary sensor channels as well.

Thanks, I was unaware of these. I managed to refactor light-level as well before leaving on a short holiday this week. I will add button and relative-rotary as well for consistency. I don't have any relative-rotary devices to test with, but since it will be similar to the others, I guess chances are good that it will work too.

Completion of this PR for Motion, Temperature, LightLevel sensors => @jlaur => keep scope as is, or include 2. as well?

See above.

Make the same analogous changes for existing Button and RelativeRotary sensors => @jlaur include in this PR, or @andrewfg create a new PR?

See above. I see you already created a draft PR. I will have a look at that also to make sure I get it right. I already noticed that button is a bit different since there is one report per individual button.

Implement the new home security sensors, (including the same analogous code) as above => @andrewfg

Agreed.

@andrewfg
Copy link
Contributor

will add button and relative-rotary as well for consistency. I don't have any relative-rotary devices to test with, but since it will be similar to the others, I guess chances are good that it will work too.

I have added the button and relative rotary in #15602 and the home security with #15601

@jlaur jlaur force-pushed the 15546-hue-motion-last-updated branch from a6b0ffc to e05503e Compare September 18, 2023 20:24
@jlaur jlaur force-pushed the 15546-hue-motion-last-updated branch from e05503e to 12b5118 Compare September 24, 2023 18:39
@jlaur jlaur requested a review from andrewfg October 5, 2023 16:43
@andrewfg
Copy link
Contributor

andrewfg commented Oct 5, 2023

@jlaur I will only be able to look at this finally next week

@jlaur jlaur force-pushed the 15546-hue-motion-last-updated branch from 12b5118 to f3cc301 Compare October 7, 2023 22:33
@jlaur jlaur force-pushed the 15546-hue-motion-last-updated branch from f3cc301 to e6a4b0b Compare October 10, 2023 08:45
@jlaur jlaur force-pushed the 15546-hue-motion-last-updated branch from e6a4b0b to 541f435 Compare October 10, 2023 08:48
@andrewfg
Copy link
Contributor

@jlaur a few points..

  • in the interests of having an easy life, I accept your argument not to inherit the xxxReport classes from a common base class. Even though it means that I have to modify my own PR for home security to match you.
  • apart from the comment about button last updated above, I reviewed you other code again, and I did not find any new points
  • nevertheless I think there are still some open points from my prior review still to be resolved

@jlaur
Copy link
Contributor Author

jlaur commented Oct 10, 2023

  • nevertheless I think there are still some open points from my prior review still to be resolved

I believe I responded to all comments, can you check?

andrewfg added a commit to andrewfg/openhab-addons that referenced this pull request Oct 10, 2023
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@jlaur jlaur force-pushed the 15546-hue-motion-last-updated branch from df61a95 to 52bf2d7 Compare October 10, 2023 17:33
@jlaur jlaur requested a review from a team October 10, 2023 19:38
@jlaur jlaur force-pushed the 15546-hue-motion-last-updated branch 2 times, most recently from 2c31252 to 95b4724 Compare October 14, 2023 17:37
jlaur added 7 commits October 17, 2023 14:37
Resolves openhab#15546

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur jlaur force-pushed the 15546-hue-motion-last-updated branch from 85cd948 to 551a916 Compare October 17, 2023 12:38
@jlaur
Copy link
Contributor Author

jlaur commented Oct 17, 2023

@openhab/add-ons-maintainers - a review would be much appreciated, since there's currently a moderate activity level for this binding, and additional issues waiting in line to be tackled as well. Thanks in advance. 😉

@andrewfg
Copy link
Contributor

a review would be much appreciated, since there's currently a moderate activity level for this binding, and additional issues waiting in line to be tackled as well

@openhab/add-ons-maintainers there is also a new PR #15601 to support new Hue security products that depends on this PR being merged :)

Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

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

lgtm, many thanks @jlaur for the enhancement and @andrewfg for the review!

@kaikreuzer kaikreuzer added this to the 4.1 milestone Oct 26, 2023
@kaikreuzer kaikreuzer merged commit 1702631 into openhab:main Oct 26, 2023
2 checks passed
andrewfg added a commit to andrewfg/openhab-addons that referenced this pull request Oct 27, 2023
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
…5552)

* Add channels for last motion/temperature sensor update (API v2)
* Add channel for last light level sensor update (API v2)
* Add channel for last rotary steps update (API v2)
* Add channel for last button update (API v2)

Resolves openhab#15546

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
@jlaur jlaur deleted the 15546-hue-motion-last-updated branch April 30, 2024 19:27
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.

[hue] deprecated DTOs for Button, RelativeRotary [hue] Add channel for last motion (API v2)
3 participants