search

Achronite / mqtt-energenie-ener314rt

MQTT interface for Energenie ENER314-RT add-on board for the Raspberry Pi, designed for use by Home Assistant.
MIT License
13 stars 5 forks source link

Added setting to config file for controlling MQTT discovery HA 'disabled entities' for Monitor Plug, and Smart Plug+ #55

Closed genestealer closed 9 months ago

genestealer commented 11 months ago

Added setting to config file for controlling MQTT discovery HA 'disabled entities' for Monitor Plug, and Smart Plug+

Address: https://github.com/Achronite/mqtt-energenie-ener314rt/issues/54 "Add option to config to enable or disable voltage and frequency reporting"

genestealer commented 11 months ago

@Achronite Please review and merge/edit, before merging https://github.com/Achronite/mqtt-energenie-ener314rt/pull/53

Achronite commented 11 months ago

Sorry, but I really don't like this change you are proposing. It adds device specific code to the discovery section; something I tried hard not to do in the first place by externalizing the device config.

Please refer back to my initial comment on this when you first raised this as an issue.

genestealer commented 10 months ago

Sorry, but I really don't like this change you are proposing. It adds device specific code to the discovery section; something I tried hard not to do in the first place by externalizing the device config.

Please refer back to my initial comment on this when you first raised this as an issue.

@Achronite Hey, sorry missed this until now.

Yes it's adding specific code to the discovery section, but I could not see any other way for our two views to co-exist. Technically it's not device specific, rather class specific as any device which reports frequency and mains voltage are included.

Thoughts please?

Achronite commented 10 months ago

Thoughts please?

1) It is standard practice to have data disabled by default for a device in HA. If a user (such as yourself) wants to have the values tracked they simply have to go into the device config and click ENABLE to re-enable them. This is far easier than faffing around in config files. image

2) I don't want code that could disable VOLTAGE. For example the whole house monitor uses VOLTAGE to represent the device battery voltage; even though it is monitoring mains electricity! Energenie are inconsistent in how they use the OpenThings parameters; this is why I removed VOLTAGE in the first place, as I thought it was confusing!