-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
feat(inputs.knx_listener): Add support for string data type #15169
feat(inputs.knx_listener): Add support for string data type #15169
Conversation
KNX has string data types like DPT 16.000 (ASCII text) which can transfer up to 14 bytes of ASCII text. knx_listener didn't support these yet and therefore couldn't log telegrams of this type; this PR adds the required support.
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
Thanks for taking the time to put this up! I think the only thing missing would be a test. In
@srebhan is that the right address and DPT to use? |
The PR should contain a test...? It displays for me:
|
🤦 how did I miss that in a 5 line diff |
Also a side note about the test that I forgot to mention: when creating the PR, I used
When the feature is implemented, the test works as expected:
I assume that the strange failing behavior is an existing issue in the test code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution @martinvonwittich!
Summary
KNX has string data types like DPT 16.000 (ASCII text) which can transfer up to 14 bytes of ASCII text. knx_listener didn't support these yet and therefore couldn't log telegrams of this type; this PR adds the required support.
Please review this carefully; I've never written Go before.
Checklist
Related issues
resolves #15158