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

Add dimmer configuration for ubisys D1(-R) #1984

Merged
merged 10 commits into from
Jan 1, 2021
Merged

Conversation

guydriesen
Copy link
Contributor

No description provided.

@guydriesen
Copy link
Contributor Author

@@ -2528,6 +2528,32 @@ const converters = {
], manufacturerOptions.ubisys));
},
},
ubisys_dimmer_setup: {
key: ['ubisys_dimmer_setup'],
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose to chnage this to ubisys_dimmer_mode so we don't have a nested object. This allows it to add it to exposes, (as enum, see example here:

e.smoke(), e.battery_low(), e.tamper(), e.battery(), exposes.enum('sensitivity', exposes.access.ALL, ['low', 'medium', 'high']),
)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, will try it (new to javascript ;-))

meta.logger.warn(
`ubisys: Dimmer setup read for '${meta.options.friendlyName}': ${JSON.stringify(utils.toSnakeCase(dataRead))}`);
};
log(await entity.read('manuSpecificUbisysDimmerSetup', ['capabilities']));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of logging, just read here and add a fromZigbee converter to translate it to something user friendly. (https://github.com/Koenkk/zigbee2mqtt.io/pull/528/files#r549327509)

@guydriesen
Copy link
Contributor Author

Added convertor for the different attributes and removed the nested object to support the exposes. Works nicely in the ui.

@guydriesen
Copy link
Contributor Author

Not sure if this the best option to have the dimmer phase control mode available for configuration in exposes. This is basically a one time setup, so it might be better to have it in the devices.yaml file. That's also why it was initially coded like the ballast_config.
What do you think? And can you give me some clues how to implement?
Thanks

@Koenkk
Copy link
Owner

Koenkk commented Dec 31, 2020

so it might be better to have it in the devices.yaml file.

I don't agree on this. These options are stored on the device side, not on the Z2M side. Allowing these options to be put in devices.yaml would add a lot of code on the Z2M side to sync the current selected options (there is also currently no mechanism for this).

PR looks good to me, let me know if it can be merged.

@guydriesen
Copy link
Contributor Author

Ok, I see. I was wondering if there's some sort of device configuration available to place configuration options like this. On/Off, brightness,... are used on a daily basis, but things like the dimmer setup, ballast configuration are typically only configured during installation and it might not be bad to prevent accidental change (in case of the dimmer setup this could even destroy the device!).

If it's fine for you, it can be merged.
Thanks for the support!

@Koenkk
Copy link
Owner

Koenkk commented Jan 1, 2021

Thanks!

@Koenkk Koenkk merged commit 93db602 into Koenkk:master Jan 1, 2021
@guydriesen guydriesen deleted the master branch January 5, 2021 21:08
@guydriesen guydriesen restored the master branch January 5, 2021 21:09
@guydriesen guydriesen deleted the master branch January 5, 2021 21:13
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 this pull request may close these issues.

2 participants