1mckenna / esp32_iGrill

ESP32 iGrill BLE Client
MIT License
46 stars 14 forks source link

Update Metric <-> Imperial Unit Conversion #21

Closed LukasQ closed 2 years ago

LukasQ commented 2 years ago

Hi all

One could update from imperial to metric in the UI and the units change. Unfortunately the numbers stay the same. This leads to 80°C in my room, which I would rate as slightly high.

Cheers, Lukas

kneip68 commented 2 years ago

Shouldn't the recalculation be conditional? I can see in the raw MQTT data "unit_of_measurement: °C" and actually my dashboard shows 25°C for my room, as I have set Celsius in the app.... Or do I miss something?

LukasQ commented 2 years ago

Ah. Well, the unit editing happens in your case in the iGrill app. If you don't use the app at all, a conversion needs to be applied. Thus, i added

if (USE_METRIC_DEGREES)
        t=(t-32)/1.8; // if metrical, convert to celsius

To have all the configuration in the same location. But, indeed, we should after the unit switch, send an update through BLE, instead of calculating new.

1mckenna commented 2 years ago

The current use imperial units option in the configuration is only to tell the MQTT topics what unit the temperature is being sent in as.

I did this for a couple reasons:

  1. If you change the units in the iGrill app to C that change will persist on the iGrill device even when not using the iGrill app and temperatures being sent from the iGrill to the esp32 will also be in C. If the wrong units are sent in via the MQTT topic, Home Assistant will automatically convert the temperatures to the unit your dashboard uses which causes the probe readings to be incorrect.
  2. I assume most people setup the iGrill using the phone app before looking to use this with their Home Automation System so I assumed the correct units were selected then

I will do some checking on what is being done when the units are toggled in the iGrill app to see if that is something we can set via BLE to match the temperature units selected in the config UI.

kneip68 commented 2 years ago

Ah OK I see, so actually whether the temperature in MQTT is Celsius or Fahrenheit is set just by the Flag in the web interface, set in lines if(USE_METRIC_DEGREES) probe1JSON["unit_of_measurement"] = "°C"; else probe1JSON["unit_of_measurement"] = "°F"; and not actually coming from the device, isn't it? In that case I would say the workflow is "Set the web interface to whatever has been set in the app (default Fahrenheit), and do the conversion at the receiver of the MQTT message", correct? In that case I would say we do not need the conversion, but maybe just clarify in the README.

1mckenna commented 2 years ago

I took some time tonight to reverse the Android app and have added in the functionality to now set the temperature units on the iGrill device. Now whatever option is selected in the Configuration Portal will get set on the iGrill device during its initial setup. I tested this out on my iGrillv2 device and its working alright. Can I get someone else to pull down the new tempUnits branch and see if it works for them as well before I merge it into main.

beed2112 commented 2 years ago

Looks good to me.

Flashed the tempUnits branch to my esp32 -

to force the config portal modified this line

bool initialConfig = false; //default false

to

bool initialConfig = true; //default false

Set Units to metric in the config portal.

Values were coming in from MQTT as Celsius

Values were displayed in HA with the correct temperature in Fahrenheit. #win

To get the values displayed in Celsius, needed to change the Settings -> System -> General -> Unit System to Celsius/kilograms

Values were now displayed in HA with the correct temperature in Celsius. #win

1mckenna commented 2 years ago

The tempUnits branch has now been merged into main, so now whatever temperature units is selected in the config portal is what will get set on the iGrill device.