1

I'm doing some changes the ArtNet repository. I want to extend it a little but to do that efficiently I had a couple of question where I don't know the answer on. Please consider the piece of code bellow. As you can see I have three structs:

  • ArtNetNode: this contains all node identification information. IP and short/long name can off course change during the program.
  • ArtPollReplyPack: this struct has the packed argument attached to it. I use this struct later on so send the udp packet. It really is the entire package as specified in the artnet protocol specifications.
  • ArtDmxPack: a similar to point two, but is there to prove the point that data from the node struct is used in multiple packets.

Current implemantation keeps a lot of coppies from specifiic information, I refer to IP address, mac, short and long name. If one of these parameters changes i need to change it on all three locations. Which is not very efficient. My question is how can I make this efficient, so I only have to change the node.ip instead of all three location. Searches led me to pointers, which really make sense. However, I this this will mess up the structure packing since ArtPollReplyPack and ArtDmxPack are really the full data set of the package. Can any help me to overcome this? Maybe by example.

Thank you for your time!

FYI: i'm implementing with Teensyduino on a Teensy 3.2 board with Wiz8xx Ethernet port.

#include <Ethernet.h>
#include <EthernetUdp.h>

EthernetUDP Udp;

byte mac[] = {0x04, 0xE9, 0xE5, 0x00, 0x69, 0xEC};
IPAddress controllerIP(192, 168, 0, 2);

struct ArtNetNode {
  IPAddress  ip;
  uint8_t  mac[6];
  uint8_t  oemH;            
  uint8_t  oemL;
  uint8_t  shortname[18];
  uint8_t  longname[64];
};

struct ArtPollReplyPack {
  uint8_t  id[8];
  uint16_t opCode;          
  uint8_t  ip[4];
  uint8_t  mac[6];           
  uint16_t port;
  uint8_t  oemH;          
  uint8_t  oemL;
  uint8_t  shortname[18];   
  uint8_t  longname[64];
}__attribute__((packed));

struct ArtDmxPack {
  uint8_t  id[8];
  uint16_t opCode;          
  uint8_t  ip[4];           
  uint16_t port;
  uint8_t  DmxData[512];                 
}__attribute__((packed));

struct ArtNetNode node;
struct ArtPollReplyPack ArtPollReply;
struct ArtDmxPack ArtDmx;

void setup() {
  ArtPollReply.oemH = node.oemH;
  ArtPollReply.oemL = node.oemL;

  memcpy(ArtPollReply.shortname, node.shortname, sizeof(node.shortname));
  memcpy(ArtPollReply.longname, node.longname, sizeof(node.longname));
  memcpy(node.mac, mac, sizeof(mac));
  memcpy(ArtPollReply.mac, node.mac, sizeof(node.mac));

  Ethernet.begin(mac);
  Udp.begin(6454);
}

void loop() {
  switch(Ethernet.maintain()) {
    case 1:
    case 3:
      // rebind / renew failed.
      break;

    case 2:
    case 4:
      // update the node IP address.
      memcpy(node.ip, Ethernet.localIP(), 4);
    case 0:
    default: 
      break;
  }

  Udp.beginPacket(controllerIP, 6454);
  Udp.write((uint8_t *)&ArtPollReply, sizeof(ArtPollReply));
  Udp.endPacket();
}
Thieu
  • 33
  • 1
  • 4
  • 2
    Arduino is using C++ as programming language. That means you could use classes to wrap the structures. And those classes could contain members that are pointers to the linked structures. – Some programmer dude Jan 16 '20 at 11:12
  • @Someprogrammerdude Admittedly without specific domain knowledge I'd assume that the structures correspond to data fields in a layout expected by hardware or other components (note the `packed` attribute), so they are not really subject to arbitrary changes. One *could* of course set up a redundancy-free logical data holder class of some kind which can on demand *produce* the binary data in the specific layout needed. That will surely be less efficient in terms of memory and run time. – Peter - Reinstate Monica Jan 16 '20 at 11:17
  • 1
    @Peter-ReinstateMonica Wrapping the structures and having suitable conversion operators and constructors will not automatically lead to less "efficiency". Compilers these days are very good at optimizations so could probably use the wrapped structures directly when needed (especially if the "wrapping" is done by inheritance). And the only extra memory needed would be the extra pointer in the class for the link. – Some programmer dude Jan 16 '20 at 11:26
  • @Someprogrammerdude the __PACKED__ is an importent aspect since the packets are defines in the struct. For instance, if an pointer is to be added instead of the real opCode this would cause and extra byte to the packet to store the pointer (teensy has 16bit addresses). And therefor the message is no longer according specification. – Thieu Jan 16 '20 at 13:45
  • 1
    @Thieu I don't suggest that you change the current structures, but rather either inherit from them, or include them inside another class (as in `struct ArtNetNodeWrapper { ArtNetNode packet; /* Possible other needed extra data */ };`) – Some programmer dude Jan 16 '20 at 13:49

2 Answers2

1

There's no real way to set all three IP addresses to the same value without setting all three IP addresses to the same value.

However, you don't have to use memcpy() function calls... if you want to be very messy you can take advantage of the fact that IP addresses are 4 bytes, i.e., a uint32_t, so if you cast to that you can assign directly instead of using memcpy, i.e.

union IP {
    uint8_t  ip[4];
    uint32_t ipAsInt;
};

now just copy ipAsInt instead of memcpying.

[To be honest I can't guarantee that the compiler won't optimize that for you anyway]

Joel Spolsky
  • 33,372
  • 17
  • 89
  • 105
1

There is no way to change data of two or more structures at a time. But, you can attach them with one specific structure, so that whenever the data of that specific structure or we can say Base Structure changes, it will affect all other structures. Overall, just call Base Structure in all other structures. For example, refer the code below:

struct o{
    int data=3;
    char name='c';
}one;
struct t{
    int data=one.data;
    char name=one.name;
}two;

Now, when you call structure two, it will print data from structure one.

Jake Peralta
  • 408
  • 3
  • 9