fix(inputs.knx_listener): Ignore GroupValueRead requests

[farmio]

Summary

Ignore KNX GroupValueRead requests as they don't contain any value. Yet DPT 1 typed requests are treated as telegrams with 0 value.

These are my first lines of Go, so please be patient with me 😬

Checklist

• No AI generated code was used in this PR

Related issues

resolves #15006

[telegraf-tiger]

Thanks so much for the pull request!
🤝 ✒️ Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla

[farmio]

!signed-cla

[userwithoutpassword]

I tested some more DPTs. Yes only DPT1.xxx have this problem

[farmio]

Yes, that's because unpacking GroupValueRead data for other DPTs fail at

telegraf/plugins/inputs/knx_listener/knx_listener.go

Line 147 in 19ac5e8

err := target.datapoint.Unpack(msg.Data)

[inputs.knx_listener] Unpacking data failed: given application data has invalid length

so they have never get to be forwarded anyway.

[userwithoutpassword]

Yes, that's because unpacking GroupValueRead data for other DPTs fail at

telegraf/plugins/inputs/knx_listener/knx_listener.go

Line 147 in 19ac5e8

err := target.datapoint.Unpack(msg.Data)

[inputs.knx_listener] Unpacking data failed: given application data has invalid length

so they have never get to be forwarded anyway.

Interesting

But anyway there is no need for saving GroupValue_Read Messages because there is no valid data in this messages.

What did u change? Does it now ignore all Read messages?

[farmio]

What did u change? Does it now ignore all Read messages?

Yes, as it says in the PR description.

[srebhan]

@farmio thanks for your contribution! Your code looks good but I do have two comments/concerns:

1. Your current code will spam the log-file as you generate an entry every time a group-read arrives. Please change the logic to only log each address once? Some lines below you can see an example for how to do it...
2. For some applications it might be advantageous to also log DPT1.xxx events. Could we just log a fixed value if the datagram does not contain any? Like a zero value or similar?

[farmio]

@srebhan Hi 👋!

Ad 1: Depending on the log-level, current implementation will reduce logs, as the new GroupValueRead-log is debug, but a read request for a non-DPT-1-Telegram did produce an error log (see #15007 (comment))
The example you mentioned would filter out (except for the first occurrence) logs for unconfigured GAs (addresses). Is this in your interest?
Or do you want to filter all GAs? I guess we may just remove the log in that case.

Ad 2: DPT 1.x (Boolean value) write and response telegrams are still logged like they have been before. Only the read telegrams are filtered out. I can not think of an application needing to log a read telegram as constant value (without the possibility to distinguish it from writes/responses - which would be possible by eg. adding an additional tag).

[farmio]

I moved the match for the message command behind the test for the group address.
Now the GroupValueRead debug log only applies for configured (known) group addresses.
Please let me know if this is what you meant.

[srebhan]

@farmio thanks for your explanation! So just to be sure I got it right, your PR filters the group-read-request but not the response!?!? If this is correct, I would not log anything and just go on. Please also add a small comment to the code on why this is required...

If you really want to log a debug message, you should really only log it once for a GA as some people run Telegraf in debug mode in production environments and we would log this every single time we see a group-read request otherwise.

[farmio]

You are exactly right. We ignore the read request (that doesn't carry valuable data), but still process the responses (that do carry data). And of course we still process the normal write requests.

Removed the logging and added a comment explaining why GroupValueRead requests are ignored.

[telegraf-tiger]

Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip.
Downloads for additional architectures and packages are available below.

👍 This pull request doesn't change the Telegraf binary size

📦 Click here to get additional PR build artifacts

Artifact URLs

DEB	RPM	TAR GZ	ZIP
amd64.deb	aarch64.rpm	darwin_amd64.tar.gz	windows_amd64.zip
arm64.deb	armel.rpm	darwin_arm64.tar.gz	windows_arm64.zip
armel.deb	armv6hl.rpm	freebsd_amd64.tar.gz	windows_i386.zip
armhf.deb	i386.rpm	freebsd_armv7.tar.gz
i386.deb	ppc64le.rpm	freebsd_i386.tar.gz
mips.deb	riscv64.rpm	linux_amd64.tar.gz
mipsel.deb	s390x.rpm	linux_arm64.tar.gz
ppc64el.deb	x86_64.rpm	linux_armel.tar.gz
riscv64.deb		linux_armhf.tar.gz
s390x.deb		linux_i386.tar.gz
		linux_mips.tar.gz
		linux_mipsel.tar.gz
		linux_ppc64le.tar.gz
		linux_riscv64.tar.gz
		linux_s390x.tar.gz

[srebhan]

Awesome. Thanks a lot @farmio for the fix and your explanation!