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

examples: Add dup filtering to mqtt_relay #3018

Merged
merged 1 commit into from
Aug 9, 2024

Conversation

gdt
Copy link
Collaborator

@gdt gdt commented Aug 2, 2024

Keep information about the previous value sent. If it's been 5 seconds, or new value is different (ignorning keys like snr and frequency), then send it. Otherwise, just don't. This causes bursts of e.g. 4 transmissions to result in one MQTT message, on the theory that the 4 transmissions are not actually 4 messags, but a strategy to transmit one message more reliably.

Define a new configuration option to enable duplicate filtering, and default it to True.

Tonight, this is a draft with debug logging enabled.
But it's running on 3 systems, 1 macOS and 2 Debian, and seems to be working ok.

Obviously I should turn off debug logging, and I'm inclined to just change to debug = False, and not make it a config. I lean to understandability and defensive code over beauty.

@gdt gdt force-pushed the feature.mqtt-relay-dedup branch from 107516e to 765596b Compare August 2, 2024 23:46
@zuckschwerdt
Copy link
Collaborator

Very nice! Maybe change to logging and later just init to INFO level. S.a. https://github.com/merbanan/rtl_433/blob/master/examples/mqtt_filter.py#L179-L180

@gdt
Copy link
Collaborator Author

gdt commented Aug 3, 2024

Thanks; will steal the logging stuff.

Interesting to see two independent dedup implementations. I already had been designing it in my head, and I did see your commit, but I avoided looking. Your impl keeps track of multiple messages and mine doesn't. Mine calls things a match if they differ in rx metadata, and yours doesn't. That is probably fine if people aren't using -M.

All of this is making me think we need to move to having a real python module with installed scripts, that can use common code via import rtl_433. But that's a lot of work and not urgent.

@zuckschwerdt
Copy link
Collaborator

Don't look too hard at my code. It's just a quick proof of concept to get a basis for discussion. I haven't even tested anything…

@gdt
Copy link
Collaborator Author

gdt commented Aug 3, 2024

Don't look too hard at my code. It's just a quick proof of concept to get a basis for discussion. I haven't even tested anything…

Thanks for the warning. I am running what's in the PR and it worked overnight. I'm seeing only 1 line per sensor report (well 2 for 433 from 2 computers in different places) and my sensors are reporting (modulo a sysadmin error now fixeed).

But I do want to improve logging before merging.

@gdt gdt force-pushed the feature.mqtt-relay-dedup branch 4 times, most recently from 49a5ccd to 4ddba77 Compare August 4, 2024 12:20
Keep information about the previous value sent.  If it's been 5
seconds, or new value is different (ignoring keys like snr and
frequency), then send it.  Otherwise, just don't.  This causes bursts
of e.g. 4 transmissions to result in one MQTT message, on the theory
that the 4 transmissions are not actually 4 messags, but a strategy to
transmit one message more reliably.

Define a new configuration option to enable duplicate filtering, and
default it to True.

Steal logging config from mqtt_filter.py, and add a configuration
option DEBUG that if True results in debug logging instead of info.
@gdt gdt force-pushed the feature.mqtt-relay-dedup branch from 4ddba77 to e85083f Compare August 4, 2024 12:23
@gdt
Copy link
Collaborator Author

gdt commented Aug 4, 2024

I took the logging config from mqtt_filter.py, and made it configurable. I also moved comments into docstrings.

This is now ready for actual review; modulo recent cosmetic changes, it ran overnight.

@gdt gdt merged commit aed3b6f into merbanan:master Aug 9, 2024
8 checks passed
@gdt gdt deleted the feature.mqtt-relay-dedup branch August 9, 2024 23:03
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.

2 participants