wez / govee2mqtt

Govee2MQTT: Connect Govee lights and devices to Home Assistant
MIT License
490 stars 32 forks source link

H7131 Target temp call should include autoStop parameter #84

Open phurth opened 9 months ago

phurth commented 9 months ago

Govee Device SKU

H7131

Govee2MQTT Version

2024.01.18-b83b7d15

Describe the issue

After all of the changes, the add-on is working almost perfectly. One thing I noticed is that the heater has a selection called auto stop which controls whether the heater turns off or stays on low after the target temp is reached. The default in the API seems to be 0 (maintain or stay on) and changing the target temp from HA always resets this to 0. When using the auto mode with a target temp, I believe this is undesirable - once the target temp is reached the device should stop heating.

Would it be possible to send the autoStop parameter along with the commend to change the target temp? I played a bit in Postman and the only way to set autoStop is to include it with an update that also includes the temp, in other words it can't be set on its own. Not setting this has the net effect of always setting it to 0 any time the temp is adjusted.

Startup Diagnostics

Not the startup log, but this is the JSON with the temperature setting object:

{
        "alarmType": null,
        "eventState": null,
        "instance": "targetTemperature",
        "parameters": {
          "dataType": "STRUCT",
          "fields": [
            {
              "dataType": "ENUM",
              "defaultValue": 0,
              "fieldName": "autoStop",
              "options": [
                {
                  "name": "Auto Stop",
                  "value": 1
                },
                {
                  "name": "Maintain",
                  "value": 0
                }
              ],
              "required": false
            },
            {
              "dataType": "INTEGER",
              "defaultValue": null,
              "fieldName": "temperature",
              "range": {
                "max": 30,
                "min": 5,
                "precision": 1
              },
              "required": true,
              "unit": null
            },
            {
              "dataType": "ENUM",
              "defaultValue": "Celsius",
              "fieldName": "unit",
              "options": [
                {
                  "name": "Celsius",
                  "value": "Celsius"
                },
                {
                  "name": "Fahrenheit",
                  "value": "Fahrenheit"
                }
              ],
              "required": true
            }
          ]
        },
        "type": "devices.capabilities.temperature_setting"
      }

Additional Logs

No response

Home Assistant Logs

No response

Anything else?

No response

wez commented 9 months ago

This is a bit awkward to map to HASS for a couple of reasons:

phurth commented 9 months ago

This is a bit awkward to map to HASS for a couple of reasons:

  • The entity we are presenting as is a simple number slider so it can only present a command to change that one parameter
  • You'd think that we could "simply" expose the current value of autoStop as its own entity, but it isn't reported by the device
  • We could synthesize an autoStop entity of our own and know to pass it along with requests like this, but since govee2mqtt is stateless and has no persistent memory, that value would revert to whatever default we chose on each restart of the addon, which almost puts you back to the current state of things. You could potentially deal with that by having your automation make a point of explicitly setting that entity before you change the target temperature?

I was afraid this might be harder than it appears. I tried but can't set autoStop without also sending a temp and unit. The API throws an error if you send the autoStop parameter alone.

fjrichman commented 9 months ago

mqtt could function as semi-persistent storage here couldn't it?

Using Retained Messages the setting for autostop could be stored and then the container would just subscribe to the topic to read the setting upon reboot.

If the topic doesn't exist for whatever reason it would get created by the container.

wez commented 9 months ago

the hard part isn't persisting the data, it's accommodating the idea that we need to track that state for devices with metadata that has this shape, because we don't really know (or want to know) the gory details of how this device or any other device works, we just want the metadata to inform us what we should do.

Roughly what I think would be needed for this is:

there's kinda of a lot of work there.

Another approach which I think is feasible only when this sort of parameter is present as a boolean is to essentially fork the HA entity into two separate sliders: Target Temperature (autoStop=true) and Target Temperature (autoStop=false).

This wouldn't help with control if/when we add a proper climate entity for this device, but it might be the simplest and most expedient way to expose this.

Ardera-Sempre commented 8 months ago

I see this is labeled an enhancement, and I would just like to comment, and add some weight to this request, that I would like to see this as a feature for the H7130 as well.

matthewcbyington commented 2 weeks ago

I would echo the above comment. Architecturally I understand the problem, but as a user it's an important feature. Thank you for consideration!