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);
}
}
In this code:
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: