kovapatrik / homebridge-midea-platform

Homebridge plugin for Midea devices
https://www.npmjs.com/package/homebridge-midea-platform
Apache License 2.0
26 stars 2 forks source link

Remove Swing and Child Lock options from Dehumidifier (never used) #47

Closed simplytoast1 closed 6 months ago

simplytoast1 commented 10 months ago

Is your feature request related to a problem? Please describe:

I am nit sure if this is a HomeKit limitation but I was wondering if it would be possible to hide the humidify option on a dehumidifier.

Describe the solution you'd like:

Just to present options the dehumidifier has.

Describe alternatives you've considered:

Ignore it.

Additional context:

This is no means a bug and likely is a HomeKit issue but was wondering with code if you could hide or instruct HomeKit that the dehumidifier has no humidity option.

kovapatrik commented 10 months ago

@dkerr64

I think it's possible because of lines https://github.com/kovapatrik/homebridge-midea-platform/blob/main/src/accessory/DehumidifierAccessory.ts#L55-L57. The question is, how we want to seperate humidify and dehumidify? Is you dehumidifer capable of doing both?

simplytoast1 commented 10 months ago

Mine is not. Perhaps it can be under advanced settings that you can set that feature flag.

kovapatrik commented 10 months ago

Yeah I thought so because you opened the issue:D I meant to ask @dkerr64

dkerr64 commented 10 months ago

I see the same thing, I will try @kovapatrik suggestion and see if that removes the humidifier options from HomeKit.

dkerr64 commented 10 months ago

The "valid values" for TargetHumidifierDehumidifierState do not have any impact on how Homebridge or Apple Home apps show the dehumidifier. Both continue to show Auto, Humidify and Dehumidify in their user interface. At least with the Apple Home app if you try and select anything other than Dehumidify the user interface "resets" the selection to Dehumidify.

I also tested the Eve app and the Controller for HomeKit app. The Eve app behaves just the same as Apple Home app. The Controller app just shows "dehumidifying" for target state, but does not allow you to change that... whether valid values is set to all three, or just dehumidify.

There is an advantage to changing the lines that @kovapatrik pointed out. As currently coded the plugin permits the apps to send a "auto" or "humidify" target state, and that results in this error to log...

[11/17/2023, 8:00:42 PM] [homebridge-midea-platform] This plugin threw an error from the characteristic 'Target Humidifier-Dehumidifier State': Unhandled error thrown inside write handler for characteristic: Device net_a1_363A (150633093209019) can only be a DeHumidifier, illegal value: 0. See https://homebridge.io/w/JtMGR for more info.

So I think it would be best to change the valid values to only "dehumidifier" but don't expect this to change the user interface in the Apple Home app.

kovapatrik commented 10 months ago

@simplytoast1 With the latest version you should experience the same usage as @dkerr64 wrote above. Can you confirm this? If yes, can this issues be closed?

simplytoast1 commented 9 months ago

I can confirm in iOS 17.2 the “humidity” characteristics are hidden.

Is there a way to potentially hide oscillate as well?

dkerr64 commented 9 months ago

Good to hear that this is fixed by Apple in 17.2. I have not installed it as I am unable to install betas.

Yes we can hide swing/oscillate too. I'm wondering whether to just remove it completely or make it optional. Are there any dehumidifiers with this feature? I doubt it?

This also raises the question of child lock. That similarly does nothing for me, though the API accepts it, and immediately resets it to off.

@kovapatrik thoughts?

simplytoast1 commented 9 months ago

To chime in from a user perspective:

re: Child Lock - I would hide it as well.

re: Oscillate - There are no models that I can find that support that option

I defer to you both for the design choices.

kovapatrik commented 9 months ago

Good to hear that this is fixed by Apple in 17.2. I have not installed it as I am unable to install betas.

Yes we can hide swing/oscillate too. I'm wondering whether to just remove it completely or make it optional. Are there any dehumidifiers with this feature? I doubt it?

This also raises the question of child lock. That similarly does nothing for me, though the API accepts it, and immediately resets it to off.

@kovapatrik thoughts?

From what you and @simplytoast1 have experienced about these attributes, I would just simply remove them.

dkerr64 commented 9 months ago

I will implement that this evening.

dkerr64 commented 9 months ago

PR #54 ready for review.

Please note comments in that PR about what I had to do to make the swing/lock characteristics go away for existing users that have cached accessory.

@simplytoast1 when you first restart this plugin you should see a message to log...

New dehumidifier service version, replacing cached version.

It should only ever appear once, the very first time you restart this plugin and then never again (well, until I make another breaking change).

Also, FYI, iOS 17.2 was released today and the options to select auto/humidifier are gone for me too.

simplytoast1 commented 9 months ago

Great news all around. Waiting to see the new version of the plugin published. You all should get this verified. This is a great plugin!

dkerr64 commented 9 months ago

@kovapatrik I noticed when I restarted this in my production host that the info log message reporting

[12/14/2023, 6:06:21 PM] [homebridge-midea-platform] [Dehumidifier] New dehumidifier service version.
          Upgrade from vundefined to v1.

is now split across two lines. That wasn't the intent... is caused by line-to-long prettier here.

No urgency, but next time we're touching the code I'd like to put that back to a single line.

kovapatrik commented 9 months ago

I thought about this when I wrote that part. I didn't know if it will be a new line or no, but yeah, this is the correct behaviour I guess. I will do an issue for this so we won't forget about it.

kovapatrik commented 7 months ago

@dkerr64 Can I close this issue?

dkerr64 commented 7 months ago

@dkerr64 Can I close this issue?

Yes can close. Thanks.