A few nspanel fixes #63
Conversation
| @@ -180,8 +180,8 @@ void NSPanel::send_time_() { | |||
|
|
|||
| void NSPanel::send_temperature_(float temperature) { | |||
| std::string json_str = json::build_json([this, temperature](JsonObject &root) { | |||
| root["temperature"] = temperature; | |||
| root["tempUnit"] = this->temperature_celcius_ ? 0 : 1; | |||
| root["temperature"] = this->temperature_celsius_ ? temperature : temperature * 9.0 / 5.0 + 32; | |||
There was a problem hiding this comment.
I don't think this should be done here if a user is setting to be F then they should supply a temp in F already
There was a problem hiding this comment.
As I understand it, everything in ESPHome uses Celsius internally, and it's (intentionally?) annoying to convert between the two.
IMO, either temperature_unit_celsius shouldn't exist and we should look at the unit of measurement to choose whether to display C/F, or we should do the conversion from C ourselves since we're the final ones to handle the data.
There was a problem hiding this comment.
Yes, but you only need to add a filter to a sensor to convert it in yaml
There was a problem hiding this comment.
I actually like the idea of removing the config option altogether and looking at the UOM of the source sensor to determine C or F.
There was a problem hiding this comment.
I actually like the idea of removing the config option altogether and looking at the UOM of the source sensor to determine C or F.
Ok, sounds good. I'll try to scrounge some time to implement this today or tomorrow.
Some typo fixes (celcius -> celsius, sreen -> screen), plus a fix for Fahrenheit temperature display to do the conversion in esphome (it looks like the tempUnit bool only affects whether the text is °C or °F).