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 specific driver for Vimar Touch Thermostats of type 02952.b #29

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

petero-dk
Copy link
Contributor

And before going into details on this implementation, I think it is worth to think about a future where all devices might not share the .homeycompose/drivers/pair/select_groupaddresses/index.html

Maybe there is a way to make it dynamic based on the settings object?

From a feature point of view this device is functioning quite well (running it now) but I am unsure about how you see the future of specific devices.

I specifically made this because it is a device that absolutely does not fit into the existing thermostat driver, and it is so specific that I don't think it would make sense to create a generic setpoint shift type thermostat.

What do you think? Because I don't see a way (new to Homey) that we could make a system like Zigbee/BLE/ZWave where other apps could be developed for specific devices? They all have to be in this repository, right?

@ttherbrink
Copy link
Contributor

Excuse the delay, We are discussing this internally

@petero-dk
Copy link
Contributor Author

Excuse the delay, We are discussing this internally

No problem, no rush for me.

Copy link
Contributor

@ttherbrink ttherbrink left a comment

Choose a reason for hiding this comment

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

Don't forget to translate the setting labels/hints and the flow labels to dutch if you are able to.
Also it would be nice to have a custom icon and images for this device. You can request the icon here: https://github.com/athombv/homey-vectors-public

this.registerCapabilityListener('vimar_thermostat_mode', this.onCapabilityMode.bind(this));

// Maybe this can be placed better during pairing?
if (!this.settings.ga_summerwinter_state && this.hasCapability('summer_winter')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a bug here. If you dont fill in all the adresses during pairing you still see these capabilities

super.onInit();

this.homey.flow.getActionCard('set-window-switch').registerRunListener(async (args, state) => {
return args.device.knxInterface.writeKNXGroupAddress(args.device.settings.ga_window_switch, args.open, 'DPT1')
Copy link
Contributor

Choose a reason for hiding this comment

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

Check if group adress is available

});
});

this.homey.flow.getActionCard('reset_to_basesetpoint').registerRunListener(async (args, state) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would someone want to set the target temperature to 0?

</div>
</div>
</fieldset>
<fieldset id="vimar-thermostat-02952-b-fieldset" style="display: none">
Copy link
Contributor

Choose a reason for hiding this comment

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

you might want to mention that some adresses are optional

"getable": true,
"setable": false,
"uiComponent": "sensor"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The sensors probably need a capability icon as well, otherwise they will appear with an empty placeholder in the UI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants