chrisstaite / ArduinoArtNet

ArtNet library for Arduino
GNU General Public License v3.0
17 stars 4 forks source link

I think I found some protocol violations. #3

Open bzuidgeest opened 7 years ago

bzuidgeest commented 7 years ago

I believe found a little mistake in your library. In enum ArtNetTalkToMeTag the fourth bit is 1 if unicast should be used for diagnostics instead of broadcast. using your setup causes sendpol to send to the serverip by default which the spec says is illegal (page 14 of artnet spec 4).

typedef enum ArtNetTalkToMeTag { ARTNET_DIAGNOSTIC_BROADCAST = (1 << 3), // Set if ArtNetPollReply should broadcast ARTNET_DIAGNOSTIC_SEND = (1 << 2), // Set if diagnostic messages should be sent ARTNET_DIAGNOSTIC_ALWAYS = (1 << 1) // Set if ArtNetPollReply should be sent whenever changes occur }

to

typedef enum ArtNetTalkToMeTag { ARTNET_DIAGNOSTIC_UNICAST = (1 << 3), // Set if ArtNetPollReply should unicast ARTNET_DIAGNOSTIC_SEND = (1 << 2), // Set if diagnostic messages should be sent ARTNET_DIAGNOSTIC_ALWAYS = (1 << 1) // Set if ArtNetPollReply should be sent whenever changes occur }

then adjust both the constructor and sendpol.

Also I see this in artnet.h:

// Port to reply on

define UDP_PORT_ARTNET_REPLY (UDP_PORT_ARTNET + 1)

The spec clearly say the protocol uses only one port and I could not get it to work without removing this define.

chrisstaite commented 7 years ago

The first part is likely to be correct. I'd fix it, but if you'd prefer to have your commit in there please send a pull request with your changes.

On the second part, the reply port to a UDP packet is undefined. On hosted systems it is usually incremental from a pool in the high range 10000+. However, being embedded and these not being tracked sessions this port could be anything. We don't want the reply port to be the same as the sending port, otherwise things can get confused between stream directions, especially if cheap switching or routing gear is involved, so picking a port that isn't the ArtNet port is pragmatic. We can guarantee that (the port + 1) is not going to equal (the port).

bzuidgeest commented 7 years ago

The reply port in an udp packet is undefined, but the ArtNet spec version 4 explicitly state that all traffic is supposed to move over a single port. for example http://www.lightjams.com/artnetominator/ wont recognize a reply on the wrong port.

http://www.artisticlicence.com/WebSiteMaster/User%20Guides/art-net.pdf

Under port it says: Port: Actual data transmission on Art-Net uses the UDP protocol that operates ‘on top of’ the TCP/IP protocol. UDP data transfer operates by transferring data from a specific IP:Port on a Node or Controller to a second specific IP:Port on a second Node or Controller. Art-Net uses only one port of 0x1936.

I would send a pull request but I made some other major changes too and I'm not sure how to single that out. I've been working to factor out the dependence on the eeprom library to store config and the link to the enc... ethernet library, so I can use it on multiple platforms. I'm getting there, but not done yet. I should really do a fork so you can check it over if you want to.