-
-
Notifications
You must be signed in to change notification settings - Fork 3.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
[epsonprojector] Fix discovery service to prevent erroneous inbox items #12586
Conversation
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
logger.trace("Multicast listener parsing announcement packet: {}", beacon); | ||
|
||
clearProperties(); | ||
if (beacon.contains("EPSON") && beacon.contains("VideoProjector")) { |
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.
Did you purposely remove the toUpperCase
calls? I thought those were there in case a future projector changed the case of those items (which admittedly seems unlikely).
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.
This was something that I had on the back burner for a while. There was a problem with another binding where similar code had issues on systems that were using a non-english language locale. It seems a little safer to match the actual value. I might revisit this.
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.
Is it ok now?
Why no more using toUpperCase for VideoProjector?
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.
It is not necessary. The string 'VideoProjector' is the given SDKClass name and will always be capital case as defined by the whitepaper I found on the AMX beacon spec. The few other examples that I found for a projector beacon supported this. Here is the whitepaper, the device class names are on the last page: https://trade.amx.com/assets/whitePapers/AMX.Dynamic.Device.Discovery.White.Paper.pdf
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.
Other than my one question, this looks like a good solution.
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
...r/src/main/java/org/openhab/binding/epsonprojector/internal/discovery/MulticastListener.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
…ms (openhab#12586) * Fix discovery service to prevent erroneous inbox items * improve EPSON string matching Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com> Signed-off-by: Nick Waterton <n.waterton@outlook.com>
…ms (openhab#12586) * Fix discovery service to prevent erroneous inbox items * improve EPSON string matching Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
…ms (openhab#12586) * Fix discovery service to prevent erroneous inbox items * improve EPSON string matching Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com> Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>
…ms (openhab#12586) * Fix discovery service to prevent erroneous inbox items * improve EPSON string matching Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
Fix an issue with the Epson binding creating erroneous inbox items when AMX discovery beacon packets from other devices (GlobalCache, etc) are received. The code was improved to ignore non Epson discovery beacon packets and will handle multiple Epson projectors on the network properly (thread safe).
First discovered in this post: https://community.openhab.org/t/is-the-epson-projector-binding-abandoned/107769/82