awawa-dev / HyperHDR

Highly optimized open source ambient lighting implementation based on modern digital video and audio stream analysis for Windows, macOS and Linux (x86 and Raspberry Pi / ARM).
http://www.hyperhdr.eu/
MIT License
1.05k stars 110 forks source link

Yeelight music mode server binding to wrong IP address (is more than one interface exists) #580

Closed CheerpipeTecnova closed 1 year ago

CheerpipeTecnova commented 1 year ago

Bug report, debug log and your config file (FULL LOGS ARE MANDATORY)

Steps to reproduce

Add a working yeelight bulb/stripe

image

What is expected?

Light works using music mode.

What is actually happening?

Light is failing to connect to HyperHDR because the wrong IP address is being send to the light. Important to note that HyperHDR is reaching the buld the identify button is working.

This is the important log line:

2023-06-01T15:45:41.221Z [LEDDEVICE_YEELIGHT] (LedDeviceYeelight.cpp:958) 2| 192.168.1.27| writeCommand() : {"id":12,"method":"set_music","params":[1,"172.17.0.1",44775]}

Thing is, i am using it in a docker (using host mode) so i have more than one IP and the yeelight component is sending the wrong IP address to the bulb (172.17.0.1 is the internal container IP address. This IP should be something like 192.168.1.x). One possible and general solution would be to allow yeelight componente to specify the ip address to bind to the music mode server. I am very confident this is the problem and should be solves just sending the right IP address

System

awawa-dev commented 1 year ago

The ip is correct because that's the network interface the app can see from inside the container. Use bridge connection then it should work.

CheerpipeTecnova commented 1 year ago

It also fails with bridge. In fact, it may be worse because music mode server port is random so it can't opened using a single port.

Docker container sees the host network interface but it is picking ¿the first one? that is the docker (in fact, as container is in host mode it should not be using docker0 network interface).

I think this happens because you are looking for the first non loopback network interface:

        if (_musicModeServerAddress.isNull())
        {
            for (const auto& address : QNetworkInterface::allAddresses())
            {
                // is valid when, no loopback, IPv4
                if (!address.isLoopback() && address.protocol() == QAbstractSocket::IPv4Protocol)
                {
                    _musicModeServerAddress = address;
                    break;
                }
            }
        }

But what if you have more than one? In my case, the correct network interface is the second non loopback network interface.

I may be fixes letting the user choose the network interface manually (maybe a list?) This problem may occur in any setup with a server with more than one network interface.

I would love to help with a PR but C is not my friend (i will try to do my best anyway cause love this project and really want to get it working with yeelight lights...WLED works like a charm btw).

awawa-dev commented 1 year ago

I completely dont see any reason why it should fail in the bridge mode. Long time ago I tested in VBox and I really dont want to rework it for such specific scenario. With bridge-only networking you can have only one IP, because the container joins the host network. If you want to add new field for your IP you must modify yeelight scheme in the LED drivers folder, in the C++ file that you've already mentioned: add to config loading section reading its value provided by users and finally add new condition to the loop (comparing to the user IP if provided). But such corresponding to user IP interface must exists in the docker.

CheerpipeTecnova commented 1 year ago

I did some similar. I just added some aditional filters to avoid docker network interfaces and it is working now

                _interface.type() != QNetworkInterface::InterfaceType::Wifi) ||
                _interface.humanReadableName().indexOf("virtual", 0, Qt::CaseInsensitive) >= 0 ||
                _interface.humanReadableName().indexOf("br-", 0, Qt::CaseInsensitive) >= 0 ||           // This
                _interface.humanReadableName().indexOf("docker", 0, Qt::CaseInsensitive) >= 0 ||     // This
                _interface.humanReadableName().indexOf("veth", 0, Qt::CaseInsensitive) >= 0 ||         // And this
                !_interface.flags().testFlag(QNetworkInterface::InterfaceFlag::IsRunning) ||
                !_interface.flags().testFlag(QNetworkInterface::InterfaceFlag::IsUp))

With this bulbs are binded with my host ip

[{"command":"leddevice","ledDeviceType":"yeelight","params":{"hostname":"192.168.1.27","port":55443},"subcommand":"identify","tan":1}]

Won't PR because as you said it appear to be a border case that just happens with my specific network configuration.

awawa-dev commented 1 year ago

Good to hear that your workaround fixes the problem. If new rules work, then probably it's also enough to add virtual prefix to the network interface that you dont want to use e.g. docker network create --opt com.docker.network.bridge.name=virtual_<old_name> (it will be skipped in the loop).

DrConflict commented 4 months ago

Got it working by deleting a docker that was using a bridge network instead of the host.