envy / esp-knx-ip

A KNX/IP library for the ESP8266 with Arduino
MIT License
135 stars 49 forks source link

Make MULTICAST_IP externally configurable using compiler switches #75

Closed pfisterer closed 4 years ago

pfisterer commented 4 years ago

I had to change MULTICAST_IP when using Tasmota's KNX support because a router frequently used in Germany (AVM's Fritz!Box) does not correctly implement multicast support. This, however, seems to be a frequent issue with many routers.

As these faulty implementations forward Apple's Bonjour traffic, an option is to set the default multicast address to 224, 0, 0, 251. Using Tasmota, this requires to modify esp-knx-ip in the lib/ folder - which should be avoided.

Therefore, it could make sense to change esp-knx-ip-0.5.2/esp-knx-ip.h as follows:

Current version:

#define MULTICAST_IP              IPAddress(224, 0, 23, 12) // [Default IPAddress(224, 0, 23, 12)]

Updated version:

#ifndef
#define MULTICAST_IP              IPAddress(224, 0, 23, 12) // [Default IPAddress(224, 0, 23, 12)]
#endif

This would allow setting the MULTICAST_IP using compiler switches (e.g., -DMULTICAST_IP=IPAddress(224, 0, 0, 251)).

With this setting, I'm able to get a working setup with KNX/IP to KNX/TP forwarding. So, I'm able to switch a MagicHome LED controller running Tasmota with a KNX/TP switch.

pfisterer commented 4 years ago

@ascillato This is in the same project which we discussed yesterday in Tasmota's support chat (https://github.com/arendst/Tasmota/issues/7161#issuecomment-563392401).

Maybe it would be helpful, to mention this in Tasmota's manual? It took me a while to find out that my wifi router was not correctly implementing multicast support. I've documented my workaround here.

ascillato commented 4 years ago

@pfisterer In the meantime, you can make a PR to Tasmota in order to add that to the library.

ascillato commented 4 years ago

Did you test that approach? not always a key can be defined for a library.

pfisterer commented 4 years ago

@ascillato: A PR would only affect the documentation (see below). However, this makes IMHO only sense after modifiying this lib.

Yes. To verify this setup, I've changed lib/esp-knx-ip-0.5.2/esp-knx-ip.h as follows:

#ifndef MULTICAST_IP
#define MULTICAST_IP IPAddress(224, 0, 23, 12)  // [Default IPAddress(224, 0, 23, 12)]
#else
#warning NOT USING DEFAULT SETTING, MULTICAST_IP has been defined before
#endif

Afterwards, I've modified my platformio_override.ini of my Tasmota build and modified build_flags as follows:

build_flags = ${core_active.build_flags} '-DMULTICAST_IP=IPAddress(224, 0, 0, 251)'.

Compilation output (excerpt):

In file included from tasmota/tasmota_post.h:37:0,
                 from /Users/dennis/Documents/dev/Arduino/tasmota/Sonoff-Tasmota-MagicHome/tasmota/tasmota.ino:36:
lib/esp-knx-ip-0.5.2/esp-knx-ip.h:39:2: warning: #warning NOT USING DEFAULT SETTING, MULTICAST_IP has been defined before [-Wcpp]
 #warning NOT USING DEFAULT SETTING, MULTICAST_IP has been defined before
  ^

In file included from lib/esp-knx-ip-0.5.2/esp-knx-ip-config.cpp:7:0:
lib/esp-knx-ip-0.5.2/esp-knx-ip.h:39:2: warning: #warning NOT USING DEFAULT SETTING, MULTICAST_IP has been defined before [-Wcpp]
 #warning NOT USING DEFAULT SETTING, MULTICAST_IP has been defined before
  ^

Compiling .pioenvs/tasmota-knx/lib56f/esp-knx-ip-0.5.2/esp-knx-ip-send.cpp.o
In file included from lib/esp-knx-ip-0.5.2/esp-knx-ip-conversion.cpp:7:0:
lib/esp-knx-ip-0.5.2/esp-knx-ip.h:39:2: warning: #warning NOT USING DEFAULT SETTING, MULTICAST_IP has been defined before [-Wcpp]
 #warning NOT USING DEFAULT SETTING, MULTICAST_IP has been defined before
  ^
Compiling .pioenvs/tasmota-knx/lib56f/esp-knx-ip-0.5.2/esp-knx-ip-webserver.cpp.o
In file included from lib/esp-knx-ip-0.5.2/esp-knx-ip-send.cpp:7:0:
lib/esp-knx-ip-0.5.2/esp-knx-ip.h:39:2: warning: #warning NOT USING DEFAULT SETTING, MULTICAST_IP has been defined before [-Wcpp]
 #warning NOT USING DEFAULT SETTING, MULTICAST_IP has been defined before
  ^
Compiling .pioenvs/tasmota-knx/lib56f/esp-knx-ip-0.5.2/esp-knx-ip.cpp.o
In file included from lib/esp-knx-ip-0.5.2/esp-knx-ip-webserver.cpp:7:0:
lib/esp-knx-ip-0.5.2/esp-knx-ip.h:39:2: warning: #warning NOT USING DEFAULT SETTING, MULTICAST_IP has been defined before [-Wcpp]
 #warning NOT USING DEFAULT SETTING, MULTICAST_IP has been defined before
  ^
In file included from lib/esp-knx-ip-0.5.2/esp-knx-ip.cpp:7:0:
lib/esp-knx-ip-0.5.2/esp-knx-ip.h:39:2: warning: #warning NOT USING DEFAULT SETTING, MULTICAST_IP has been defined before [-Wcpp]
 #warning NOT USING DEFAULT SETTING, MULTICAST_IP has been defined before
ascillato commented 4 years ago

A PR would only affect the documentation

I meant, until @envy check this, we can modify the actual library in /lib folder in Tasmota repository for allowing this now.

ascillato commented 4 years ago

Anyway, don't worry. I'm adding this now in the esp-knx-ip library in Tasmota.

envy commented 4 years ago

Hi,

I added #ifndef guards around the IP and port. I also had fun with multicast with my DLink switch. I had to completely disable IGMP snooping, otherwise it would not pass the KNX multicast traffic.

pfisterer commented 4 years ago

Great, thank you guys!