openhab / openhab-core

Core framework of openHAB
https://www.openhab.org/
Eclipse Public License 2.0
907 stars 420 forks source link

Add alternative utility class for apache subnetutils #3965

Open lsiepel opened 7 months ago

lsiepel commented 7 months ago

We try to get rid of dependencies to apache libs per https://github.com/openhab/openhab-addons/issues/7722

we made great progress with https://github.com/openhab/openhab-core/pull/3738 Now there are 5 bindings that make use of subnetutils. I suggest a similar approach here, I can create an alternative util class in core to prevent code duplication. will be based on https://github.com/openhab/openhab-addons/pull/14430#issuecomment-1869707020 where the implementation was confirmed working as expected.

@openhab/core-maintainers any thoughts before I start working on it?

J-N-K commented 7 months ago

I don't think that's necessary. We need org.apache.commons.net anyway, so we can also use it in add-ons.

lsiepel commented 7 months ago

Oké, in that case I think https://github.com/openhab/openhab-addons/issues/7722 can be closed, as these are the last dependencies in addons repo. Full list is here https://github.com/openhab/openhab-addons/issues/7722#issuecomment-1432112689

@wborn wdyt?

wborn commented 7 months ago

Maybe you can use NetUtil / NetworkAddressService instead? It has a getAllInterfaceAddresses method. Add-ons should also respect the openHAB "Network Configuration" settings like not using IPv6 if it is disabled. Currently we cannot remove commons-net because it is a dependency required for rfc2217 connections using nrjavaserial.

lsiepel commented 7 months ago

This issue is not about removing commons-net at this moment, its scope is much smaller. It is about reducing some dependencies to commons-net. I know we still depend on commons-net due to nrjavaserial, but why not reduce it as much as possible?

I also see this as an opportunity to reduce and improve code that is used in multiple bindings that have about the same functionality. For instance, when i do a quick scan, it looks like none of the 5 involved bindings respect the openHAB "Network Configuration" settings.

wborn commented 7 months ago

I also see this as an opportunity to reduce and improve code that is used in multiple bindings that have about the same functionality. For instance, when i do a quick scan, it looks like none of the 5 involved bindings respect the openHAB "Network Configuration" settings.

Yes that might be a nice improvement to fix multiple issues at once. :+1: