zivillian / ism7mqtt

GNU General Public License v3.0
56 stars 10 forks source link

Add option to change polling interval #23

Closed b3nn0 closed 2 years ago

b3nn0 commented 2 years ago

Hi, Really love your work. Thanks a lot! Would it be possible to get an option for changing the polling interval to something lower or higher than one minute? Or is this not practical due to the amount of data/performance of the ISM?

Thanks!

zivillian commented 2 years ago

I'm not sure if it actually has any consequence. I've chosen 1 minute, because it was my default interval for visualizing the data. As only changed properties are reported there should be no difference in load or data volume, but I think the next step is to try it out. If you want to compile your own version you must only change this constant:

https://github.com/zivillian/ism7mqtt/blob/3b9e275b3b8cb39d197c6eb85863d9d8d289df1a/src/ism7mqtt/ISM7/Ism7Client.cs#L216

Do you have a specific reason for a different interval? Should it be higher or lower?

b3nn0 commented 2 years ago

In fact, I did already try that today after writing the request :-) Took me a while to get it to compile properly on Linux - I'm not really a C# guy, but worked in the end. I reduced it to 30 and 10s for testing, and it does seem to make a bit of a difference when values change quickly (e.g. burner turns on or something), but it's really not that much of a difference - for 99% of use cases the one minute is probably fine.

Reason I wanted reporting to be as quick as possible: I use the data returned from Wolf to guesstimate my LPG usage/consumption (Modulationsgrad / 100 peak power output energy density or something). So having little systematic error due to wrong timing is desirable. It already worked fairly well with home assistant's Wolf Portal Scraping implementation, but the online services seem to be quite unreliable, so this project is a very welcome alternative.

Feel free to discard this request - I'll just use my self compiled one.

FYI: I also marked all published MQTT topics to be retained, so that Home Assistant will immediately pick up the current values after a restart. Might be worth considering, too.

zivillian commented 2 years ago

I've added an option to specify the retain flag - can you test one of the builds from https://github.com/zivillian/ism7mqtt/actions/runs/2430130612?

b3nn0 commented 2 years ago

Seems to do the job.

zivillian commented 2 years ago

it does seem to make a bit of a difference when values change quickly (e.g. burner turns on or something), but it's really not that much of a difference

can you say how big the difference is between 60 and 30 or 10s? Do you see an increase for 120s or larger?

b3nn0 commented 2 years ago

Well, heating season is over, so it's a bit harder to get comparable data.. I tried: Graphs are "Vorlauftemperatur" this morning.

  1. 20s interval: 20s

Observation: 20s does not really have any effect, but there are certainly steps < 1 minute. The smallest one I found was a 31s interval. Confusing to me is, that even at location where I would have expected small intervals due to rapid increase in temperature, the step size was 44s in that case.

Uneducated guess: Maybe the link from the heater to the ISM7 is not the fastest? And when the burner turns on, there is quite a lot of other data to be received as well, so the link can't keep up fast intervals? Later, during cooling down, there is of course less data generated, so intervals become smaller. Never reaching the configured 60s though.

  1. 60s and 120s interval On the longer-interval end however, the configuration does get respected: image

However, it doesn't seem to be precise. When setting 60s, the gaps are usually 55-65s. When setting 120s, it's usually 110-130s in reality.

Conclusion: The Interval parameter is nothing precise, but does make a difference. Anything below 30s doesn't seem to change anything.

zivillian commented 2 years ago

I've added an option to specify the interval - can you please test the build from https://github.com/zivillian/ism7mqtt/actions/runs/2435874979?

b3nn0 commented 2 years ago

Seems to work well, thanks! Now I don't need to manually compile any more :-)