muart-group / esphome

ESPHome fork for development of the mitsubishi_uart component. Check out the documentation for more info:
https://muart-group.github.io/
4 stars 0 forks source link

UX improvements #34

Open paravoid opened 1 month ago

paravoid commented 1 month ago

Copying from Discord to not forget :)

paravoid commented 2 weeks ago
* [ ]  Per [ESPHome docs](https://esphome.io/components/sensor/index.html), "If you have a [friendly_name](https://esphome.io/components/esphome#esphome-configuration-variables) set for your device and you want the sensor to use that name, you can set name: None.". Unfortunately, this didn't work for me, the Climate entity was named as "none" instead.

I don't fully understand this yet, but this fixes it (on top of the recent mitsubishi_itp changes):

--- a/esphome/components/mitsubishi_itp/climate.py
+++ b/esphome/components/mitsubishi_itp/climate.py
@@ -105,14 +105,11 @@ INTERNAL_TEMPERATURE_SOURCE_OPTIONS = [
 validate_custom_fan_modes = cv.enum(CUSTOM_FAN_MODES, upper=True)

 BASE_SCHEMA = (
-    cv.polling_component_schema(DEFAULT_POLLING_INTERVAL)
-    .extend(climate.CLIMATE_SCHEMA)
-    .extend(
+    climate.CLIMATE_SCHEMA.extend(
         {
             cv.GenerateID(CONF_ID): cv.declare_id(MitsubishiUART),
             cv.Required(CONF_UART_HEATPUMP): cv.use_id(uart.UARTComponent),
             cv.Optional(CONF_UART_THERMOSTAT): cv.use_id(uart.UARTComponent),
-            cv.Optional(CONF_NAME, default="Climate"): cv.string,
             cv.Optional(
                 CONF_SUPPORTED_MODES, default=DEFAULT_CLIMATE_MODES
             ): cv.ensure_list(climate.validate_climate_mode),
@@ -135,6 +132,7 @@ BASE_SCHEMA = (
             ),
         }
     )
+    .extend(cv.polling_component_schema(DEFAULT_POLLING_INTERVAL))
 )

 # TODO Storing the registration function here seems weird, but I can't figure out how to determine schema type later
Sammy1Am commented 2 weeks ago

So just to clarify some of the intent here with the naming (to be clear, this is definitely user-setup-dependent):

The reason I've added "Climate" as the default rather than allowing the device friendly name is that in my ESPHome config I've named the device basement_hp with a friendly name of "Basement Heat Pump" because that's what it's controlling. That means if I name a sensor e.g. "Defrost", then in Home Assistant the sensor has id binary_sensor.basement_hp_defrost and is named "Basement Heat Pump Defrost" (both of which seem fine to me).

If I didn't manually specify a separate name for the Climate component, then its id became climate.basement_hp_basement_heat_pump and its name was "Basement Heat Pump Basement Heat Pump" which was long and redundant. Defaulting to "Climate" means that if I do nothing in the config, the Home Assistant device is named just "Basement Heat Pump Climate".

paravoid commented 2 weeks ago

I think we may be talking about the same thing :) I'd like the Climate entity to be named (say) "Basement Heat Pump". (If I set name: Basement Heat Pump, the name becomes Basement Heat Pump Basement Heat Pump, as you correctly point out.)

The standard, documented, way to accomplish what I'd like to accomplish (and I think you too!) this is to set name: None. See the note under "name" in https://esphome.io/components/sensor/index.html.

With the code as it is today, setting name: None, actually names the climate entity... "None", so this doesn't work.

With the patch I provided above, you can set name: None and then everything works as intended, I think?

Sammy1Am commented 2 weeks ago

June Improvements Branch updated with a fix to restore name: None functionality. (I branched off the 0x09 fix, so you can test this one if you'd like but I'll still be adding new stuff so it might break at some point before I do an actual PR).

I feel a little weird checking off tasks in the part of the issue you created (never worked an issue with tasks in it like that), but here's where we are currently:

So it looks like basically swing stuff at this point. I'll either tackle some of these tonight, or just move the info here into #14 and track it there.

Sammy1Am commented 2 weeks ago

I've copied some information from this issue into the Swing Support issue, so we can continue to track that particular enhancement there.