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

Introduce Timed Intervals for MQTT Messages #37

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

redahb
Copy link

@redahb redahb commented Oct 4, 2024

This PR introduces timers for all MQTT topics (Energy, Main, HMI, and Stats), ensuring data is sent only at the configured intervals rather than immediately after a value change (e.g., water temperature).

Full updates and Error updates remain unaffected: Full updates are already configurable, and Error messages should still be relayed ASAP for critical notifications.

I initiated this change because my Atlantic Explorer V4 was sending temperature values approximately every 0.5 seconds, which overwhelmed the Home Assistant recorder. Since AquaMQTT controls heat pumps, real-time temperature data isn’t essential. Configurable intervals, such as 1 or 5 minutes, provide a more practical solution.

Enable individual timers for each MQTT topic.
@tspopp tspopp self-requested a review October 6, 2024 19:18
Copy link
Owner

@tspopp tspopp left a comment

Choose a reason for hiding this comment

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

Hey @redahb, thanks for the contribution! There are similarities to the request in #19. It seems the amount of data published is really an issue.

The changes are quite good, but I am missing a switch in the configuration to keep the original behavior, for example constexpr bool MQTT_PUBLISH_IMMEDIATELY_ON_CHANGE = true and then adapt the check a litte, for example:

if (((notify & 1 << 8) != 0) && MQTT_PUBLISH_IMMEDIATELY_ON_CHANGE || hmiUpdate || fullUpdate)

Another way of solving this issue is adding additional filters before publishing as requested in #19.

If you add the suggested additional configuration option to keep the original behavior, I'd love to merge this. Great work! Thanks :)

@tspopp
Copy link
Owner

tspopp commented Oct 14, 2024

@redahb please have a look at the latest version which adds temperature value filters using kalman. I am curious, if you still need these requested change since it dramatically reduces the amount of values emitted to the broker.

Cheers!

@henrykuijpers
Copy link

henrykuijpers commented Oct 28, 2024

@tspopp I notice the same thing with my boiler (with the latest main branch):
image

It seems it publishes a new time update every second!
It also seems that the amount of messages that are going through the HMI is quite large, even though it seems the boiler isn't doing so much. So next to that time update every second, there is also an update every few seconds when a new message was processed.
image

@tspopp
Copy link
Owner

tspopp commented Oct 28, 2024

I will increase the default configuration interval for the stats topic in the next version to a higher value. This will reduce the updates for the debug message values.

For the time value I would propose to remove the seconds from the MQTT payload. So this would lead to an update every minute.

Does this sound feasible? :)

@henrykuijpers
Copy link

Yes, that sounds good already! That saves 59 updates per minute. :P Maybe even hourly would be useful.

It would also be nice to be able to just turn this off, as it would not be very useful for a lot of users. If you can turn it on via Mqtt, then you can use it temporarily for debugging.

@henrykuijpers
Copy link

For the HMI & main, messages sent and handled, it would also be good to find a good threshold, as it currently spams a lot. Normally you would be interested in the functioning of the controller. Maybe a messages handled / sent per minute or something could be useful?

I'm definitely not interested in receiving that +1 each and everytime, for 4 different stats.

I just want to know that my controller is working correctly.

@tspopp
Copy link
Owner

tspopp commented Oct 28, 2024

Well, you might also disable MQTT discovery and write your own configuration file in home assistant to filter the not wanted attributes. I am willing to do a few optimizations here and there, but since this is a non profit and free open source project, I put my free time where it makes sense to me.

You may always come up with a PR, but I might propose changes to not break existing functionality like within this PR, which I do happily merge as soon as @redahb pushes changes.

Sometimes I feel like, I sold you guys a product. But this is open source, happy to get your comments but at some point just use it as it is 🤷‍♂️

@henrykuijpers
Copy link

@tspopp I'm sorry I made you feel this way? That was not the intention. I was thinking out loud, hoping to get some feedback (because maybe my thoughts are incorrect and the information is actually good to receive (I'm afraid it is a lot of traffic on the mqtt server)).

I can also come up with some separate PRs to tackle the other issues, but I was hoping we could have a little discussion on how to approach these fixes, as that would point into the right direction of making a good fix.

And know that I am super grateful for this product, into which you put a lot of time and effort and is working very well! Thank you!

@tspopp
Copy link
Owner

tspopp commented Oct 28, 2024

No worries, maybe some misinterpretation due to translation. But yeah, this is not a product.

The only values which change frequently are the temperature values and they should be silenced using kalman filters. So rule of thumb, if there is a change, it is a change you also want to see within homeassistant. If you do filtering time-based, like one value every 5 minutes, you will lose resolution and there won't be any neat graphs in home assistant. Therefore, I suggested going with kalman filters. You will get more mqtt messages when the heat-pump is heating up (since the value is actually changing a lot!), and less mqtt messages if it is idle (less change). If somebody takes water out of your heat-pump, there will be more messages. But none of the message is sent twice for the same value. For me, this solution is perfect and I don't really see the need for something to be added.

kalman

Regarding debugging stats, we can disable them entirely if the current stats timer configuration is not enough. The reason I have this is if people using AquaMQTT with newer heat pumps or if there are connectivity issues due to hardware issues. I always expected CRC fails or even dropped bytes if there is something wrong. If somebody has troubles in first place, I can always ask for these metrics, in order to get a direction to debug.

In your case, everything already works fine which is great. So if you're not interested in these values which I understand, we can just add a switch to disable them entirely.

Time and Date is also something which should be disabled. If you're playing around with RTC or using another NTP time zone or server than default, it is of interest which time AquaMQTT is actually using. But since there are currently no known bugs I guess we can also disable them by default.

At which else attributes do you see hmi and main messages spamming?

@redahb
Copy link
Author

redahb commented Nov 6, 2024

@redahb please have a look at the latest version which adds temperature value filters using kalman. I am curious, if you still need these requested change since it dramatically reduces the amount of values emitted to the broker.

Cheers!

Hello @tspopp,

Sorry for the late reply but yes, for me I still need my changes after the kalman filters. I think we differ in opinion in what is interesting information. I am eternally grateful for the hardware and software you developed, so don't feel bad about it :).

I only really want to know 2 things from my boiler: Power draw and water temperature. Perhaps possibly inlet/outlet air temperature, but in reality that's it. Everything else, while it might be interesting for others, is 'noise' to me. I don't use it at all :)

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.

3 participants