discoverNode() should inform listeners if new node is not exactly the same as previously discovered node with same IP

[alamaral]

In this code:

public void discoverNode(ArtPollReplyPacket reply) {
    InetAddress nodeIP = reply.getIPAddress();
    ArtNetNode node = discoveredNodes.get(nodeIP);
    if (node == null) {
        logger.info("discovered new node: " + nodeIP);
        node = reply.getNodeStyle().createNode();
        node.extractConfig(reply);
        discoveredNodes.put(nodeIP, node);
        for (ArtNetDiscoveryListener l : listeners) {
            l.discoveredNewNode(node);
        }
    } else {
        // What happens here if the node info in the reply
        // doesn't exactly match the info that we got from
        // discoveredNodes?  It seems like this should be
        // considered as a new node, and l.discoveredNewNode()
        // should be called for each listener, no?
        node.extractConfig(reply);
    }
    lastDiscovered.add(node);
}

In the if clause the code creates a new ArtNetNode, which is populated with the contents of the reply, added
to discoveredNodes, and discoveredNewNode() is called on each listener. However, in the else clause, when
a node with a matching IP address is found, the contents of the reply is copied into the existing node, overwriting
the previous contents. The potential problem that I see is that if the contents of the packet that is copied into
the ArtNetNode from the reply has changed at all from what it was previously, for example if the node was
reconfigured on the fly to change the number of ports, then the listeners do not get notified of the change to the node.

To fix this there needs to be a proper set of equals (and probably hashCode) functions for the ArtNetNode and any
objects it contains, so the code can check for any changes. The default equals function does not work in this case
from what I can see and return false when the data is otherwise the same. Once these functions are implemented
then the code can do something like this in the else clause:

        ArtNetNode newNode = reply.getNodeStyle().createNode();
        newNode.extractConfig(reply);

        if (!newNode.equals(node)) {
            node = newNode;
            discoveredNodes.put(nodeIP, node);
            for (ArtNetDiscoveryListener l : listeners) {
                l.discoveredNewNode(node);
            }
        }

[cansik]

hey @alamaral, thank you very much for the issues you have opened and the problems you have described. This library is a fork of the old ArtNet4J project which once was hosted on google code, adding just some more functions. There are (as you experienced) a lot of issues within library, if you dig a bit deeper. That is why others have started to re-implement the artnet library with a better structure and less buggy behaviour. For example @schw4rzlicht has started to implement a library some years ago which seems to be pretty mature now deltaeight/LibArtNet.

To be honest, I am not sure if I am going to work on these issues any time soon. But the community and I would of course be happy if you would have the time to add a pull request (as you already supplied some of the code which would solve the issue). I would be happy to merge them and release a new version.

[alamaral]

Thanks. I had originally found a different fork of artnet4j about 4 years ago, and used that in our product, but it hasn't been touched for ages. I came across your fork which had some new functionality we needed, so started testing it. We recently got a new DMX controller (Swisson XND-4) which had brought these bugs to light and I figured it wouldn't hurt to file the bugs.

I did see LibArtNet, but it too hasn't been touched in 2 years, and I didn't want to have to port to a new library with different bugs, so I stayed with artnet4j, as it worked well enough for our purposes. I'll take another look at LibArtNet to see just how mature it is. If it doesn't work out I may end up creating a new fork for artnet4j and trying to maintain it myself.

Thanks again for your work.