Skip to content

update temperature unit to work in HA#768

Merged
1technophile merged 3 commits into1technophile:developmentfrom
orrpan:degree_fix
Oct 4, 2020
Merged

update temperature unit to work in HA#768
1technophile merged 3 commits into1technophile:developmentfrom
orrpan:degree_fix

Conversation

@orrpan
Copy link
Contributor

@orrpan orrpan commented Sep 27, 2020

from "C" and "F" to "°C and "°F"

@orrpan
Copy link
Contributor Author

orrpan commented Sep 27, 2020

I'm not 100% this works with F, have to test a little. (I don't use F but I see that my HA is thinking it also is and C, maybe it only needs one of them and then sets it by language?)

Edit: This works, but it seems like home assistant fixes correct unit regardless if it is Celsius or Fahrenheit so tem, tempf and tempc could be only one.

This would work for both Celsius and Fahrenheit

  char* MiJiasensor[MiJiaparametersCount][8] = {
      {"sensor", "MiJia-batt", mac, "battery", jsonBatt, "", "", "%"},
      {"sensor", "MiJia-temp", mac, "temperature", jsonTemp, "", "", "°C"},
      {"sensor", "MiJia-hum", mac, "humidity", jsonHum, "", "", "%"}
      //component type,name,availability topic,device class,value template,payload on, payload off, unit of measurement
  };

Tests

MQTT

tem

{
    "stat_t": "home/OpenMQTTGateway/BTtoMQTT/582D3432D799",
    "name": "MiJia-tem",
    "uniq_id": "582D3432D799-MiJia-tem",
    "dev_cla": "temperature",
    "val_tpl": "{{ value_json.tempc | is_defined }}",
    "unit_of_meas": "°C"
}

tempc

{
    "stat_t": "home/OpenMQTTGateway/BTtoMQTT/582D3432D799",
    "name": "MiJia-tempc",
    "uniq_id": "582D3432D799-MiJia-tempc",
    "dev_cla": "temperature",
    "val_tpl": "{{ value_json.tempc | is_defined }}",
    "unit_of_meas": "°C"
}

tempf

{
    "stat_t": "home/OpenMQTTGateway/BTtoMQTT/582D3432D799",
    "name": "MiJia-tempf",
    "uniq_id": "582D3432D799-MiJia-tempf",
    "dev_cla": "temperature",
    "val_tpl": "{{ value_json.tempf | is_defined }}",
    "unit_of_meas": "°F"
}

In Home Assistant

Metric system configured in Home Assistant

metric

Imperial system configured in Home Assistant

imperial

@1technophile
Copy link
Owner

1technophile commented Sep 27, 2020

Thanks for the PR.

@jmw6773 as you already worked on this. Could you give your point of view about this PR ?

@jmw6773
Copy link
Contributor

jmw6773 commented Sep 28, 2020

@1technophile,
I'm sorry. My wife just had our third child, so I don't know when I'll be able to test this fully.

I made the recommendation to remove the ° in PR #703 as Home Assistant was ignoring the unit_of_measurement when the symbol was included and converting the temperature readings to a different unit. Upon removal of the symbol, the tempc and tempf sensor values displayed the transmitted unit without any conversions.

From my understanding, which could be wrong, Home Assistant converts temperature values to what the user has set in the configuration.yaml file for the temperature_unit (C or F), or if that isn't defined, what it set for unit_system (metric or imperial).

In my Home Assistant setup I have the following set in the basic setup.

#  unit_system: metric
#  temperature_unit: C

This is an image from my OMG running current dev branch (just compiled) with an AHT10 sensor.
HA_Discovery

Below is an image of the same device recompiled only adding the ° in the AHTx0 mqttDiscovery definitions. (And a custom OMG version number.)
HA_Discovery2

If we want, we could remove the tempc and tempf sensor definitions from mqttDiscovery and just transmit one temperature value with the appropriate unit_of_measurement, with the ° symbol. The user would then rely on Home Assistant to do the conversions to what they have set in their config files. Both C and F values would still be posted to MQTT topics for other applications, but only one included in mqttDiscovery for Home Assistant.

@orrpan
Copy link
Contributor Author

orrpan commented Sep 28, 2020

Home assistant seems to kind of understand without the ° symbol. I notice the problem when I used the homekit integration, then home assistant couldn't pass any value to the sensor.

If we want, we could remove the tempc and tempf sensor definitions from mqttDiscovery and just transmit one temperature value with the appropriate unit_of_measurement, with the ° symbol. The user would then rely on Home Assistant to do the conversions to what they have set in their config files. Both C and F values would still be posted to MQTT topics for other applications, but only one included in mqttDiscovery for Home Assistant.

This sounds good, should I then close this PR, create a new PR or juste remove all tempC and tempF and keep temp with "°C"?
Btw should it be "temp" or "tem", seems to differ in a few places.

@1technophile
Copy link
Owner

My wife just had our third child

Congratulations for you and your family !!!

This sounds good, should I then close this PR, create a new PR or just remove all tempC and tempF and keep temp with "°C"?

Just remove tem and tempf from the discovery (keep °C) on this PR or open a new one as you prefer.

tem will be removed in all the code later on.

@jmw6773
Copy link
Contributor

jmw6773 commented Oct 3, 2020

I tested your branch last night and it is crashing at startup. It's a remote device, so I couldn't get any logs.

However I believe it's because the count of parameters for each of the sensors needs to be updated to match. For example, with the MiFlora sensor, the MiFloraparametersCount should be 4 instead of the original 6.

# define MiFloraparametersCount 4

# define MiFloraparametersCount 6
Log.trace(F("MiFloraDiscovery" CR));
char* MiFlorasensor[MiFloraparametersCount][8] = {

@orrpan
Copy link
Contributor Author

orrpan commented Oct 4, 2020

I tested your branch last night and it is crashing at startup. It's a remote device, so I couldn't get any logs.

However I believe it's because the count of parameters for each of the sensors needs to be updated to match. For example, with the MiFlora sensor, the MiFloraparametersCount should be 4 instead of the original 6.

# define MiFloraparametersCount 4

# define MiFloraparametersCount 6
Log.trace(F("MiFloraDiscovery" CR));
char* MiFlorasensor[MiFloraparametersCount][8] = {

Strange, mine is working. But anyway it seems like a smart thing to fix!

@1technophile 1technophile added this to the v0.9.6 milestone Oct 4, 2020
@1technophile
Copy link
Owner

Thanks!

@1technophile 1technophile merged commit 03ab7f7 into 1technophile:development Oct 4, 2020
JeroenWiersma pushed a commit to JeroenWiersma/OpenMQTTGateway that referenced this pull request Oct 18, 2025
* update temperature unit to work in HA
* use only one temperature sensor for HA
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants