-2

I am in a position in which I have got an anonymous structure containing several elements. In order to access them by index I have placed them in a union, like this:

union
{
    struct
    {
        unsigned char COMMAND;      //STRUCT_ARRAY[0]
        unsigned char ADDR_H;       //STRUCT_ARRAY[1]
        unsigned char ADDR_M;       //STRUCT_ARRAY[2]
        unsigned char ADDR_L;       //STRUCT_ARRAY[3]
        unsigned char DATA;         //STRUCT_ARRAY[4]
        unsigned char CHECKSUM;     //STRUCT_ARRAY[5]
    };
    unsigned char STRUCT_ARRAY[6];
    //all of the struct members can be accessed from STRUCT_ARRAY by index
}MY_UNION;

This union currently resides inside a file source.c. I need to access it from main.c. I have a header which both files include, lets call it header.h.

How can I read the value of, for instance, ADDR_H and ADDR_M in main.c while modifying it periodically from source.c?

The code works a bit like this:

source.c:

#include "header.h"

union
{
    struct
    {
        unsigned char COMMAND;      //STRUCT_ARRAY[0]
        unsigned char ADDR_H;       //STRUCT_ARRAY[1]
        unsigned char ADDR_M;       //STRUCT_ARRAY[2]
        unsigned char ADDR_L;       //STRUCT_ARRAY[3]
        unsigned char DATA;         //STRUCT_ARRAY[4]
        unsigned char CHECKSUM;     //STRUCT_ARRAY[5]
    };
    unsigned char STRUCT_ARRAY[6];
    //all of the struct members can be accessed from STRUCT_ARRAY by index
}MY_UNION;

void modify(void)
{
    MY_UNION.ADDR_H = somevalue;
    MY_UNION.ADDR_M = somevalue;
    MY_UNION.ADDR_L = somevalue;
}

In main.c:

#include "header.h"

void main(void)
{
    modify();
    print(MY_UNION.ADDR_H);    //custom function to print values to a screen
    print(MY_UNION.ADDR_M);
    print(MY_UNION.ADDR_L);
}
too honest for this site
  • 12,050
  • 4
  • 30
  • 52
hat
  • 781
  • 2
  • 14
  • 25

2 Answers2

1

Fundamental program design:

  • Never declare variables in header files.
  • Never use spaghetti programming with extern.
  • Don't directly expose internals such as this protocol struct outside the translation unit that handles the protocol. You need to have an abstraction layer in between.

Quick & dirty solution:

  • Change the union definition in the h file to be a typedef:

    typedef union 
{
    struct
    {
        unsigned char COMMAND;      //STRUCT_ARRAY[0]
        unsigned char ADDR_H;       //STRUCT_ARRAY[1]
        unsigned char ADDR_M;       //STRUCT_ARRAY[2]
        unsigned char ADDR_L;       //STRUCT_ARRAY[3]
        unsigned char DATA;         //STRUCT_ARRAY[4]
        unsigned char CHECKSUM;     //STRUCT_ARRAY[5]
    };
    unsigned char STRUCT_ARRAY[6];
    //all of the struct members can be accessed from STRUCT_ARRAY by index
} MY_UNION;

  • Declare the actual variable locally in the .c file: static MY_UNION my_union;.

  • Access the variable with setter/getters, example:

    uint8_t get_address_h (void)
{
  return my_union.ADDR_H;
}

void set_address_h (uint8_t addr_h)
{
  my_union.ADDR_H = addr_h;
}

Proper solution:

In a proper program you should be hiding the internals of this protocol entirely from other files, including the typedef union.

Nobody but the protocol converter should access this union. You will have functions like set_address, set_data etc which the caller can know without knowing the protocol internals.

Lundin
  • 195,001
  • 40
  • 254
  • 396
  • In the OP's code `MY_UNION` is the variable name. It would be easier to keep the same convention. – Rishikesh Raje Dec 05 '18 at 10:14
  • @RishikeshRaje They shouldn't declare variables in the header, so the original code can't be preserved. – Lundin Dec 05 '18 at 10:37
  • I haven't declared any variables in the header, which is why I didn't give any code for `header.h` in my question. Does the code I wrote here imply this? – hat Dec 05 '18 at 10:45
  • @Jǝssǝ `union{ ... }MY_UNION;` declares a variable called MY_UNION. How it compiles without a union tag or typedef, you'll have to ask the wonderful PIC compiler. – Lundin Dec 05 '18 at 10:47
  • I accepted the other answer because it seemed logical, it worked and looked 'correct'. Your answer provides better background and advice. (I'm not saying that your answer is bad @RishikeshRaje). However, I don't like to remove an accept once I have given it, so I have just upvoted. I apologise for being too hasty. – hat Dec 05 '18 at 11:36
  • @Jǝssǝ Please note that there are a whole lot of people in embedded who have "lots of experience", while they are really just EE who have picked up dabbling with C programming and actually don't know what they are doing. Creating global variables all over the place would be a strong indicator that the original programmer is a quack (with the exception of constants and MCU registers, which isn't the case here). In addition, I doubt that the PIC crapilers follow C11 but maybe they do? – Lundin Dec 05 '18 at 11:50
  • In my limited experience they don't. PIC hardware is great, but the compilers are terrible. C18 at least follows the standard most of the time, while others like CCS throw it all out the window and ask you to learn a new C. – hat Dec 05 '18 at 11:54
  • @Jǝssǝ Ok I just downloaded the C18 manual and they admit it being a PoS. They partially follow the "X3-159" ANSI C draft from the 1980s. Their anonymous struct is a non-standard extension to that draft standard, but as it happens it is compatible with standard C11. Just as the compiler, the PIC hardware is also ancient 1980s stuff, but that's another story. – Lundin Dec 05 '18 at 12:04
-1

The easiest way is to typedef the union

in header.h

typedef union
{
    struct
    {
        ...
    };
    unsigned char STRUCT_ARRAY[6];
}MyUnionType;

extern MyUnionType MY_UNION;

Then in source.c define the variable

MyUnionType MY_UNION;

This variable now can be used in any source file. (main.c etc)

Rishikesh Raje
  • 8,556
  • 2
  • 16
  • 31