search

rvdbreemen / OTGW-firmware

A ESP8266 devkit firmware for the Nodoshop version of the Opentherm Gateway (OTGW)
MIT License
145 stars 34 forks source link

selective mqtt config for Home Assistant #103

Closed rlagerweij closed 2 years ago

rlagerweij commented 2 years ago

This pull request changes the mqtt discovery mechanism to only send configs for messages it actually encounters in the opentherm protocol traffic.

This should cut down the number of sensors creates in Home Assistant to only those which are actually used by the heating system.

A further selection mechanism can be built on top of this.

As an aside, there are still a number of values which the OTGW can decode for which there is no line in the mqttha.cfg file.

rvdbreemen commented 2 years ago

@rlagerweij working on your pull request... just wondering about a few things you can answer me:

  1. why use a bitmap? and not the array of epochs?
  2. did you take into account that we need to reconfigure all sensors when HA goes offline and comes back online?

I will continue to integrate this pull request because I think it's an improvement over the original concept of setting it up on the start, this will configure only what is "used" by the thermostat and boiler. Even though it might take a while for the sensors to be discovered.

edit: By looking at the code... Ad.1 bitmap nice solution separate from the epoch, making it independent, clearer for later understanding Ad2. you reconfigure from scratch, it will mean it will rebuild the sensors after a reconnect

rvdbreemen commented 2 years ago

The PR does raise a new question, should we start completing the whole mqttha.cfg for all message id's. The reason I did not do it before was simply driven by the fact that the sensors are already a lot and they were added at the start. However if we now have a way to add them to auto discovery on the fly, we could also complete the whole list and make it work for all users that use my firmware.

So based on that conclusion, I will release this PR as part of v0.9.2 once I validated it. And then in a next release I will start adding all other messageid's.

rlagerweij commented 2 years ago

I think even in this selective form there are still too many sensors and the original related issue remains valid. This would only be a true solution if we allow users to select which data gets forwarded. One way to make is user configurable is to have them edit the mqttha.cfg file to comment out config lines that they don't want to receive. It was with this in mind that I kept the config file in place despite your suggestion to move it into a datastructure in progmem. The other solution is to build a ui for selecting which of the detected sensors needs to be processed, which would be cleaner.

In any case I think otgw firmware should ship with reasonable defaults and we should probably not forward masterconfigid, opentherm version, etc to home assistant.

I do have a small update to this PR which reimplements the doautoconfig() since there is some lost functionality (in restapi.ino if I remember correctly) without it. I won't get to upstreaming that until Wednesday night or Thursday sadly.

I m happy to work on further improving this concept for inclusion though. It was not meant as to thrown across the schutting for you to figure out. Although you are of course free to take it on as you see fit.

Get Outlook for Androidhttps://aka.ms/AAb9ysg

From: Robert van den Breemen @.> Sent: Monday, January 3, 2022 11:21:52 PM To: rvdbreemen/OTGW-firmware @.> Cc: rlagerweij @.>; Mention @.> Subject: Re: [rvdbreemen/OTGW-firmware] selective mqtt config for Home Assistant (PR #103)

The PR does raise a new question, should we start completing the whole mqttha.cfg for all message id's. The reason I did not do it before was simply driven by the fact that the sensors are already a lot and they were added at the start. However if we now have a way to add them to auto discovery on the fly, we could also complete the whole list and make it work for all users that use my firmware.

So based on that conclusion, I will release this PR as part of v0.9.2 once I validated it. And then in a next release I will start adding all other messageid's.

— Reply to this email directly, view it on GitHubhttps://github.com/rvdbreemen/OTGW-firmware/pull/103#issuecomment-1004398024, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAFAYYF3SZDN5MDJK5JRU23UUIOQBANCNFSM5K4ZSTOA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you were mentioned.Message ID: @.***>

rvdbreemen commented 2 years ago

@rlagerweij I worked on integrating it last night, would you be so kind to look at my branch: https://github.com/rvdbreemen/OTGW-firmware/tree/dev-selective-mqtt-discovery

I am going to test it probably this weekend... hoping to release the feature this weekend.