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

Update DDFs for Hue motion sensors #7247

Merged
merged 7 commits into from Sep 25, 2023
Merged

Conversation

ebaauw
Copy link
Collaborator

@ebaauw ebaauw commented Sep 17, 2023

This PR depends on #7252 and on #7253 (to show the floating point values).

Add state/measured_value and corresponding capabilities to the ZHATemperature sensor.
Needs https://github.com/dresden-elektronik/deconz-rest-plugin for the C++ changes and the generic item definition for cap/measured_value/quantity.

  • Add state/measured_value, as float value in °C;
  • Add corresponding cap/measured_value items. min and max are reported by the sensor; quantity, substance, and unit are defined in the DDF;
  • Deprecate state/temperature (integer value in 0.01 °C);
  • Not sure what to do with config/offset (also integer value in 0.01 °C). I do apply it to state/measured_value, but ultimately, we want this as a float values as well. Maybe introduce config/measured_value/offset and deprecate config/offset?
{
  "capabilities": {
    "measured_value": {
      "max": 327.67,
      "min": -273.15,
      "quantity": "temperature",
      "substance": "air",
      "unit": "degC"
    }
  },
  "config": {
    "alert": "none",
    "battery": 100,
    "offset": 0,
    "on": true,
    "reachable": true
  },
  "ep": 2,
  "etag": "dab5aba3b1d450f8ff3285e99d6229ce",
  "lastannounced": null,
  "lastseen": "2023-09-23T14:17Z",
  "manufacturername": "Signify Netherlands B.V.",
  "modelid": "SML004",
  "name": "Hue outdoor motion sensor 2022",
  "productname": "Hue outdoor temp. sensor",
  "state": {
    "lastupdated": "2023-09-23T14:16:55.944",
    "measured_value": 23.09,
    "temperature": 2309
  },
  "swversion": "2.53.6",
  "type": "ZHATemperature",
  "uniqueid": "00:17:88:01:0b:d1:08:d7-02-0402"
}

Add `state/measured_value` and associated `capabilities` to ZHATemperature.
Add `state/measured_value` and associated `capabilities` to ZHATemperature.
Add `state/measured_value` and associated `capabilities` to ZHATemperature.
Add `state/measured_value` and associated `capabilities` to ZHATemperature.
@ebaauw ebaauw added the Device Improvement Additional tag to attach to a existing issue. label Sep 17, 2023
@ebaauw ebaauw added this to the v2.24.1-beta milestone Sep 17, 2023
@ebaauw ebaauw requested a review from manup September 17, 2023 12:44
@manup
Copy link
Member

manup commented Sep 21, 2023

{
          "name": "state/temperature",
          "deprecated": "2023-09-17"
}

If I understand this right the simplification, without parse function, here still works since the same is defined in the generic state/temperature item?

I'm still concerned with cap/measured_value/unit expressed as UTF-8 string like "°C" since this may break in funny ways as we have seen recently with Ikea firmware. On the other hand at least on our side we can catch valid UTF-8 strings with the validator. Fun fact even with valid UTF-8 if (someVal.unit == "°C") may not be what one thinks since comparing Unicode strings is not as simple as comparing ASCII strings.

@SwoopX
Copy link
Collaborator

SwoopX commented Sep 21, 2023

I'm irritated, why is state/temperature deprecated, please?

@ebaauw
Copy link
Collaborator Author

ebaauw commented Sep 21, 2023

I'm irritated, why is state/temperature deprecated, please?

I thought we wanted to replace it by the floating point value in state/measured_value?

If I understand this right the simplification, without parse function, here still works since the same is defined in the generic state/temperature item?

Yes, the generic item contains the parse, and state/temperature is still updated.

I'm still concerned with cap/measured_value/unit expressed as UTF-8 string like "°C" since this may break in funny ways as we have seen recently with Ikea firmware. On the other hand at least on our side we can catch valid UTF-8 strings with the validator. Fun fact even with valid UTF-8 if (someVal.unit == "°C") may not be what one thinks since comparing Unicode strings is not as simple as comparing ASCII strings.

JSON defines:

A string is a sequence of zero or more Unicode characters, wrapped in double quotes, using backslash escapes. A character is represented as a single character string. A string is very much like a C or Java string.

I've been using °C (and µg/m³) in JavaScript and HomeKit for ages without any issue. I cannot talk about other API clients, of course.

Note that the generic item for state/temperature has a description of "The current temperature in °C × 100.".

@manup
Copy link
Member

manup commented Sep 21, 2023

And here starts the fun part of Unicode, the same text can be represented in multiple ways which are often not visually different:
From your above example I guess you typed in ° and C?

Copy paste the following in node and press enter :)

"°C" == "℃"

Here you might even see a visual difference depending on the font and rendering. In any case comparing such stuff safely would require proper extra Unicode compare functions which do all kinds of black magic behind the scenes.

@ebaauw
Copy link
Collaborator Author

ebaauw commented Sep 21, 2023

Yes, two characters, degree (option-shift-8 or option-* on macOS; hold the 0 on iPadOS) followed by capital C.

Now that you mention it, I've seen the single character for degrees-celsius before, but kinda forgot about it. Looking from my iPad, it's beautiful (even more than the two characters), looking from my Mac (rendered in a fix-space font) is uglier than the latest IKEA switches (which is probably why I suppressed my memory of that character).

We could of course prevent the variation, by adding the units to constants.json ;-)

But seriously, what do you want to do: write out degrees Celsius, degrees C, or simply C?
And microgram per cubic meter, microgram per cubic metre or ug/m3.

Incidentally, I only use the units as string literal to display these in HomeKit - no logic based on the string value.

@manup
Copy link
Member

manup commented Sep 21, 2023

Yeah and there a dozens more ways to create a °C string. It gets even more tricky since text editors can (and do) change your input representation as well as JSON parsers can (and do) this "arbitrarily". In Javascript you can use String.normalze to counter against this but it still doesn't solve all cases.

My wish for unit would be to have it working without any Unicode magic which is basically required. From a client side there can be use cases which need to compare against the unit string and do something, or to be able to make unit conversions, and math stuff ... the value is not just for pretty printing, in which case it would be fine to be "random" Unicode.

So from my perspective this should be ASCII only like ug/m^3 — leaving the pretty printing to the client / UI — I wrote about it in #6673 (comment)

... inspired by GNU Units https://www.gnu.org/software/units/manual/html_node/Operators.html#Operators

@ebaauw
Copy link
Collaborator Author

ebaauw commented Sep 21, 2023

Changed to degC (which seems to be used by Units) and to ug/m^3 (in other PRs).

Are we OK with deprecating state/temperature, for do we want to keep that next to state/measured_value indefinitely? I don't expect we'll be removing it anytime soon, but I also don't think we should keep it in API v2.

@manup
Copy link
Member

manup commented Sep 24, 2023

Changed to degC (which seems to be used by Units) and to ug/m^3 (in other PRs).

Cool thanks

Are we OK with deprecating state/temperature, for do we want to keep that next to state/measured_value indefinitely? I don't expect we'll be removing it anytime soon, but I also don't think we should keep it in API v2.

In the long run l hope yes. But this needs to be prepared and discussed before marked for deprecation. Perhaps in the next developer session, other client developers need to be on board and understand the whole change. We also should find a coarse date when the actual removal of the old way happens (e.g. 1-2 years time to upgrade). This also means we should add proper documentation, which items are affected and what needs to be done to replace them.

@ebaauw
Copy link
Collaborator Author

ebaauw commented Sep 24, 2023

Removed the marks.

@manup manup merged commit 64ceb5b into dresden-elektronik:master Sep 25, 2023
1 check failed
@ebaauw ebaauw deleted the hue-motion branch September 25, 2023 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Device Improvement Additional tag to attach to a existing issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants