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

[MiHome] Add a Bridge property for the network interface to be used for multicast traffic #9619

Merged
merged 3 commits into from
Mar 27, 2021

Conversation

ratza
Copy link
Contributor

@ratza ratza commented Dec 31, 2020

[MiHome] Add a Bridge property for the network interface to be used for multicast traffic

This MR adds a property on the Bridge Thing to allow explicit configuration of the network interface name to which the MulticastSocket is bound.

When the computer running OpenHAB has more than one network interface configured (typically, a VLAN for your segregated IoT devices, and the other for your regular traffic like internet, OpenHAB panel access, etc), it could be that OpenHAB will attempt to listen for Multicast traffic of the Gateway on the wrong network interface. Depending on the OS the network interface chosen for binding the Multicast traffic depends on routing rules, default gateway or plain interface order (don't quote me on that). Couldn't find a specific document that describes the actual behavior, mostly because multiple networks configured for multicast is not considered the norm, hence has an undefined behavior. The solution is to bind to a specific network interface. Since there's no option to do it globally (for all Bindings) within OpenHAB and I read, somewhere on the forum, that it is expected for each binding to implement support separately, I've added this for the MiHome binding. I've also added a succinct description to the documentation.

Provides a graceful property to solve the issue described in #3239.

I've tested this in my own environment (Ubuntu, 2 VLANs), to be working correctly.

Signed-off-by: Ion Marusic ion.marusic@gmail.com

…or multicast traffic

Signed-off-by: Ion Marusic <ion.marusic@gmail.com>
@ratza ratza force-pushed the feature/multicast-interface-configuration branch from 99801a6 to e15a831 Compare December 31, 2020 18:30
@fwolter
Copy link
Member

fwolter commented Jan 3, 2021

@pboos can you take a look?

@@ -110,13 +110,17 @@ Just follow the instructions in ["Connecting devices to the gateway"](#connectin

- The binding requires port `9898` to not be used by any other service on the system.
- Make sure multicast traffic is correctly routed between the gateway and your openHAB instance
- To correctly receive multicast traffic, when your OpenHAB machine is using multiple network interfaces, you might need to configure the optional `interface` property on the `Bridge` Thing, like so:
Copy link
Member

Choose a reason for hiding this comment

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

Please check all.

Suggested change
- To correctly receive multicast traffic, when your OpenHAB machine is using multiple network interfaces, you might need to configure the optional `interface` property on the `Bridge` Thing, like so:
- To correctly receive multicast traffic, when your openHAB machine is using multiple network interfaces, you might need to configure the optional `interface` property on the `Bridge` Thing, like so:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reviewed the whole file and updated all relevant instances. Also fixed some other obvious typos.

@@ -36,6 +36,14 @@
<advanced>true</advanced>
</parameter>

<parameter name="interface" type="text">
<context>network-address</context>
Copy link
Member

Choose a reason for hiding this comment

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

The network-address context validates hostnames and IP adresses. AFAIK, there is no context for interface names. So, this should be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed. Unfortunately, I'm not accustomed with the effect of the "contexts", so I blindly replicated based on the other fields.

@fwolter
Copy link
Member

fwolter commented Mar 27, 2021

@ratza What's the state of this PR?

Signed-off-by: Ion Marusic <ion.marusic@gmail.com>
Signed-off-by: Ion Marusic <ion.marusic@gmail.com>
@ratza
Copy link
Contributor Author

ratza commented Mar 27, 2021

@ratza What's the state of this PR?

My apologies, life took over. Just pushed the relevant changes.

Copy link
Member

@fwolter fwolter left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM

@fwolter fwolter merged commit 767cbb1 into openhab:main Mar 27, 2021
@fwolter fwolter added this to the 3.1 milestone Mar 27, 2021
themillhousegroup pushed a commit to themillhousegroup/openhab2-addons that referenced this pull request May 10, 2021
…or multicast traffic (openhab#9619)

Signed-off-by: Ion Marusic <ion.marusic@gmail.com>
Signed-off-by: John Marshall <john.marshall.au@gmail.com>
computergeek1507 pushed a commit to computergeek1507/openhab-addons that referenced this pull request Jul 13, 2021
…or multicast traffic (openhab#9619)

Signed-off-by: Ion Marusic <ion.marusic@gmail.com>
thinkingstone pushed a commit to thinkingstone/openhab-addons that referenced this pull request Nov 7, 2021
…or multicast traffic (openhab#9619)

Signed-off-by: Ion Marusic <ion.marusic@gmail.com>
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022
…or multicast traffic (openhab#9619)

Signed-off-by: Ion Marusic <ion.marusic@gmail.com>
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