feat: speed-controlled fans #8188
Conversation
This implements converters for speed-controlled fans. These fans are implemented as level-controlled devices on the ZigBee side (having a onOff and levelCtrl cluster), thus no speed translation is necessary and their state can be handled by the normal on_off converters.
| zigbeeModel: ['FanBee1', 'Fanbox2'], | ||
| model: 'FanBee', | ||
| vendor: 'Lorenz Brun', | ||
| description: 'Fan with valve', |
There was a problem hiding this comment.
Can you provide a link to this device? Note to self; after this merge Koenkk/zigbee2mqtt#24483
There was a problem hiding this comment.
This is a custom device built by me. I'll eventually open-source the design files and firmware, but right now there is no public docs or anything for it.
There was a problem hiding this comment.
For those type of devices, you can use the external converters (there is no point of adding it to the list of officially supported devices if nobody can get it)
There was a problem hiding this comment.
I know, but for the tests we need a device which exercises this behavior. There is an Inovelli fan which looks like it could also use this fan control model but as I do not own one of them I didn't feel comfortable changing it over to the new model (also this is a breaking change for anyone using the MQTT API).
I do intend on making this device publicly available.
There was a problem hiding this comment.
I do intend on making this device publicly available.
Good, then I will merge it once this is done.
There was a problem hiding this comment.
|
This pull request is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 30 days |
| } | ||
|
|
||
| withModes(modes: string[], access = a.ALL) { | ||
| this.addFeature(new Binary('state', access, 'ON', 'OFF').withDescription('On/off state of this fan').withProperty('fan_state')); |
There was a problem hiding this comment.
This should not be here, add a withState() and update all existing e.fan() to call .withState()
EDIT: Since it is also in withSpeed, why not leave it in the constructor?
There was a problem hiding this comment.
The withProperty at the end is different, and that difference is significant.
There was a problem hiding this comment.
Although I agree fan_state is not the nicest name, I think we should keep it consistent at least. Is there a strong reason to have state instead of fan_state?
There was a problem hiding this comment.
As far as I understand the codebase the on_off from/toZigbee converters want the property to be called state, right?
There was a problem hiding this comment.
Yes, but we could add another key to the list (fan_state) to also support that.
There was a problem hiding this comment.
IMO the solution with state is a cleaner both from a code perspective (no vaguely-related keys in generic converters) and from an MQTT API perspective as any system which knows how to power on/off a device can now do that to a fan too without needing specific knowledge, similar to how this is designed with the the Zigbee clusters. I would prefer to convert the remaining fan_state fields to state as well for similar reasons but that's a breaking API change so probably needs to wait until the next major release.
There was a problem hiding this comment.
I would prefer to convert the remaining fan_state fields to state as well for similar reasons but that's a breaking API change so probably needs to wait until the next major release.
That cannot be done I think, there are devices which have both a state (for e.g. a light) and fan_state (for the fan). I think the cleanest solution is to have a withState() where the property name is specified
There was a problem hiding this comment.
But shouldn't that be two different endpoints / entities?
There was a problem hiding this comment.
I don't remember/can find the device anymore, but that could be indeed, they are definitely different entities.
This is a custom self-built device for ventilating spaces. It contains a valve and a speed-controlled fan, the valve is automatically opened when the fan runs. The Zigbee standard fan cluster is unsuitable for this type of device, so instead a level-controlled device type is used, allowing for fine speed control over the fan. I'll publish the design files and firmware for this eventually, but we need one device to test the speed-controlled fan device model in Z2M so we need it in there already.
This implements converters for speed-controlled fans. These fans are
implemented as level-controlled devices on the ZigBee side (having a
onOff and levelCtrl cluster), thus no speed translation is necessary and
their state can be handled by the normal on_off converters.
Also adds my custom FanBee device as we need one device to run tests
against.
Co-dependent with Koenkk/zigbee2mqtt#24483