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

UX improvements #34

Open
7 of 8 tasks
paravoid opened this issue Jun 5, 2024 · 6 comments · Fixed by #42
Open
7 of 8 tasks

UX improvements #34

paravoid opened this issue Jun 5, 2024 · 6 comments · Fixed by #42

Comments

@paravoid
Copy link

paravoid commented Jun 5, 2024

Copying from Discord to not forget :)

  • inspired by Intuitive vane mappings echavet/MitsubishiCN105ESPHome#73 by @SixFive7, vane namings can use Unicode: AUTO", "↑↑", "↑", "—", "↓", "↓↓", "SWING" and "←←", "←", "|", "→", "→→", "←→", "SWING" , which are arguably nicer than <<, < | <> etc.
  • there are 13 sensors and knobs reported, but -at least in my case- only 3-4 are useful. Perhaps these should be minimized, and/or made less confusing?
    • I had no idea what "Active Mode" was until I looked at the code. That's useful mostly for debugging, AIUI? Is there a legitimate reason to turn it off otherwise? (see issue Disable Active Mode Switch when no Thermostat Defined #35)
    • Shouldn't Compressor Frequency and Error Code be marked as Diagnostic? Perhaps Actual Fan Speed as well? Not sure about Service Filter. (see issue Sensor categorization #43)
    • My unit does not support packet 0x09 ([MSZ-GE35VA] Timeout waiting for response to 42 packet. #23), so some of the sensors never receive any value (Service Filter, Defrost, HotAdjust, Standby, Actual Fan Speed). Whatever solution is devised for that issue (new config option?), perhaps it should entail disabling those sensors as well?
    • I have no other temperature sensor configured, so the "Temperature Source" selector with only "Internal" as a choice isn't super useful 😆 I'd guess this would be the most common config too.
  • Per ESPHome docs, "If you have a friendly_name 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.
  • Swing modes don't seem to be available in the Lovelace card. Perhaps this is an artifact of the fact that vanes show up under the Configuration section, rather than under the Controls section in the HA Device? (see issue Vane Swing Support #14)
@paravoid
Copy link
Author

* [ ]  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
Copy link

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
Copy link
Author

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
Copy link

Sammy1Am commented Jun 16, 2024

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:

  • Swing mode labels
    • Looking into it
  • Lots of sensors
  • Active mode switch removed
    • Replaced with disable_active_mode config option
  • Shouldn't Compressor Frequency and Error Code be marked as Diagnostic?
    • Diagnostic is currently being reserved for non-sensor, non-climate stuff (like RSSI, Uptime, Loop Duration, etc.). I understand the argument that some of these are uncommonly used, but arguably all of the sensors are less commonly used than the values built into the Controls area, so I'm not sure there'd be any sensors left in Sensors, and there'd be nowhere to put the aforementioned diagnostic stuff.
    • Going to check this off for now so this issue doesn't get held up just on discussion, but feel free to open a separate issue to track discussion of how best to categorize sensors.
  • My unit does not support packet 0x09
    • Code will stop sending 0x09 packets if responses are not received
  • Temperature source select hidden if less than 2 options
  • name: None functionality restored
  • Swing modes
    • This is related to the fact that Mitsubishi units don't have a separate "Swing On" bit, but track swinging in the same byte being used for vane direction (i.e. "Swing" is just one of the available directions). So turning swing on from the Lovelace card would be no problem, but turning it off puts the vanes into an undefined direction.
    • The solution is to store the last vane direction when swing is turned on, default to Auto (probably) if no direction is stored, and turn off swing mode anytime a direction is selected from the select menu.
    • Unclear if "Swing" will stay as a dropdown option in the select menu, but I'd say it probably should so that it's more clear to the user that selecting a direction will disable swing.
    • It's also easy for users to disable e.g. the horizontal swing select menu in Home Assistant, but slightly more complicated to disable the swing options in the Lovelace card.

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.

This was referenced Jun 16, 2024
@Sammy1Am Sammy1Am linked a pull request Jun 16, 2024 that will close this issue
@Sammy1Am
Copy link

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

@paravoid
Copy link
Author

I added a reference to #14 to the top, and marked the task as "done". (It's not really, but we can track that in #14).

I think the Unicode vane namings is the only thing left in this task that isn't tracked elsewhere.

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 a pull request may close this issue.

2 participants