Hi, I need some hellp with my code structure.

Hi Cris,

The following rules extremely strong guidelines I think apply to your specific scenarios:

(1) For every .c file apart from main.c, create a header file with the same name. Otherwise, it's extremely confusing, and causes the types of issues that you are seeing. So, for lamps.c, there should also be a lamps.h; anything else is definitely unorthodox. The same name header file (with .c replaced with .h) is the standard way. If you feel that will result in too many files for your volume of code, then that's a sign that you may have too many source files, e.g. all lamp related functions may be better in a single file than several (this is just an example).

(2) For every header file, stick in there the function prototypes, that you want to be accessible elsewhere. So, your lamps.h file should contain void lampTest( int state ); and your lamps.c file should not contain this prototype.

(3) Each .c file should contain a #include with the same name header file. So, your lamps.c file should contain #include lamps.h

(4) Each header file should contain something like the following:

#ifndef __LAMPS_HEADER_FILE__
#define __LAMPS_HEADER_FILE__
/* the rest of the file */
#endif // __LAMPS_HEADER_FILE__

This too is standard, always to be done without question. If it isn't done, then it's again extremely unorthodox.

(5) Now you are free to liberally use #include "lamps.h" in any file you like. There's no issue with excessively using it, because the linker will only use it once, regardless of how many places it is included, so you're not bloating the firmware. For readability, sure it should only be used where needed. So, your main code will contain #include "lamps.h" and you will be free to use the lampTest function in the main function.

(6) Absolutely important (same level as feeding gremlins after midnight! this is defcon 1 level): Never define variables in your header files. Always in .c source files, otherwise again it's in unorthodox territory where it will sometimes work but will often cause issues. If you need to access that variable from a different file (for instance, if (say) you have a lampState variable defined in lamps.c that you also wish to use in main.c, then you must use an extern declaration, and ideally in the header file. So, your lamps.h file would contain extern int lampState; and then your main.c file needs to contain nothing except the #include "lamps.h".

There are plenty of other guidelines, but the ones above are most directly related to the types of symptoms you are seeing, and it's a guaranteed approach. The concepts are solid.

If you want to be sure that any of the above is normal, it's good to examine trusted code, for example peek at the Linux source code, e.g. this header file. You'll see examples of extern in there for instance, but you could browse other files from that link too, to follow how others coded it.

By the way, there may be "more standard" ways to do things (for instance, there are conventions on naming of things, which I have not followed above, since I'm not a C developer (any more). Also, there are more modern techniques, for instance, the C++ concept of setters and getters reduces the need to share variables across files, and such concepts can be coded in C too. But that's for another day.

Finally, if you upload your code to (say) replit, then others can inspect and optionally edit your code (if you give them permission : ). 

 shabaz  yess. I agree with most. but 4 is well a toss-up.  in a large production codebase, I agree because I use it for debugging.  ifndef -endif is very good where you have two or versions in the same package. you just the define the on you wish to use. But this is not the case!  So I decided not to use that "structure"

Nope, Nada, Old wifes tail. #6 First its a great place to store all of your vars, defs , structs, etc. You just should not put active code segments there here are few that break that rule too:

definition of PI, and we don't have one. so we can get lazy by doing something like this: const int PI = 3.14 ( but it needs 16 decimals there). now question why the const its not needed. It just tells the compiler that wait you can't change this value, and I normally don't use it. you could  use atan2() a math function. like this: const double pi = atan2(1,1) but why?  #define PI atan2(1,1) and let the compiler figure it out. and btw once it is defined you can not change the value. 

and I have one more for you how about the square of a number, nope C does not have one either. to square a number you have to multiply it by itself. so you could end up here. square( n )  but wait you need to declare what type is n. so that's not going to work again here is the solution #define SQ (a)*(a)  but we can change the SQ to something more useable. what about the @ symbol? or ƒ the symbol so  A² + B² = C²would become A² + B² = C². Aƒ  + Bƒ = Cƒ   I kinda ike it. So the #define is this: 
#define  ƒ (n)*(n) and there is no rule agaist it. beauty rains.

~~CAH

Hi Cris,

This is what's causing the problems, because there will be corner-cases (sometimes more often than not) where things will break down and not compile (as you have seen) unless those guidelines are followed. There's not going to be a single place in the entire Linux source code which includes variables inside the header file.

As further corroboration: I don't have any old C books at hand, but if you're willing to accept the writings of Bjarne Stroustrup (a C++ guru, he has many computing-specific awards as a result of his career), this is what he wrote regarding topic #6. You can see in red he explicitly calls out variable definitions (by variable definition I mean literally the example I gave, int lampState. Consts and externs are fine; but that's not part of what is forbidden in guideline #6. 

Similarly with topic #4. Again, this is what Stroustrup wrote:

 (photos are from The C++ Programming Language by Bjarne Stroustrup).

It is fine to deviate away from convention, but that's a more advanced thing. The whole purpose of good guidelines/rules is to help prevent getting into trouble later. You can see that Stroustrup mentions the #ifndef/#ifdef thing works (and that's a guarantee, there's no condition on it).

That's why I say these are rock-solid guidelines. One can bend the rules and still achieve successful compilation and operation eventually, but these techniques mentioned above work 100%.

I’ve always considered header files to be like the index cards in a library card catalogue - it tells you what’s in the actual books, but you’d never put a book directly into the card catalogue drawers.

Basically, header files give references to things that are available in the .c files.

I have to agree with shabaz - stay away from having anything “real” in the header files and you’ll be far less likely to run into the issues you are asking about.

This is why I really don't care for C++ as you see most of what I write is in C not C++  where things that work don't.  case pinpoint  I have a valid typedeft struct which is initialized when I defined it: (landingGear.h) 

typedef struct lamps{
      char name[15];
      char color[10];
       int pin;
}Lamps[10] = {
{"RIGHT", "RED", 22}, {"NOSE", "RED", 23}, {"LEFT", "RED", 24},
{"RIGHT", "GREEN", 25}, {"NOSE", "GREEN", 26}, {"LEFT", "GREEN", 27},
{"WARNnose", "GREEN", 28}, {"WARNgear", "ORANGE", 29}, {"WARNnoseDis", "BLUE", 30}, {"WARNskid", "RED", 31}};

now i have used this type of code for a long time and I works.

and I include it in (lamps.h)

#include "include/landingGear.h"

void lampTest( int state );
void lamp( Lamps.pin[count], int state );
void blink( Lamps.pin[count] );

and again in lamps.c

#include "include/landingGear.h"
#include "include/lamps.h"

void lampTest( int state ){
for( int count = 0; count < 10; count++) {
digitalWrite( Lamps.pin[count] ); }
}

void lamp( Lamps.pin[count], int state ){
digitalWrite( Lamps.pin[count] );
}

void blink( Lamps.pin[count] ){
//TBD
};

and I get this crap: 

In file included from sketch/lamps.c:2:0:
landingGear.h:8:1: error: unknown type name ‘lamps’
lamps Lamps[] = {
^
In file included from sketch/include/lamps.h:1:0,
from sketch/lamps.c:3:
landingGear.h:7:1: error: conflicting types for ‘lamps’
lamps;
^
In file included from sketch/lamps.c:2:0:
sketch/include/landingGear.h:7:1: note: previous declaration of ‘lamps’ was here
lamps;
^
In file included from sketch/include/lamps.h:1:0,
from sketch/lamps.c:3:
landingGear.h:8:1: error: unknown type name ‘lamps’
lamps Lamps[] = {
^
landingGear.h:8:7: error: redefinition of ‘Lamps’
lamps Lamps[] = {
^
In file included from sketch/lamps.c:2:0:
sketch/include/landingGear.h:8:7: note: previous definition of ‘Lamps’ was here
lamps Lamps[] = {
^
In file included from sketch/lamps.c:3:0:
lamps.h:4:17: error: expected ‘)’ before ‘.’ token
void lamp( Lamps.pin[count], int state );
^
lamps.h:5:18: error: expected ‘)’ before ‘.’ token
void blink( Lamps.pin[count] );
^
sketch/lamps.c: In function ‘lampTest’:
lamps.c:7:22: error: request for member ‘pin’ in something not a structure or union
digitalWrite( Lamps.pin[count] ); }
^
sketch/lamps.c: At top level:
lamps.c:10:17: error: expected ‘)’ before ‘.’ token
void lamp( Lamps.pin[count], int state ){
^
lamps.c:14:18: error: expected ‘)’ before ‘.’ token
void blink( Lamps.pin[count] ){
^
exit status 1
unknown type name ‘lamps’

This report would have more information with
"Show verbose output during compilation"
option enabled in File -> Preferences.

The way you’re doing it, every time a file includes that .h file, it tries to create a new instance of the lamps[] array.

That’s fine if you only include it once from one file, but as soon as other files need to know about the array or the struct, you either don’t get the definition or you redefine the array.

It gets even more tricky to keep track of it all when files include header files that include other header files.

I’m guessing some compilers just don’t allow that in header files due to those reasons. You might be able to get away with it by adding some compiler flags, but why not just follow the best practices and have code that plays nice everywhere.

The way you’re doing it, every time a file includes that .h file, it tries to create a new instance of the lamps[] array.

That’s fine if you only include it once from one file, but as soon as other files need to know about the array or the struct, you either don’t get the definition or you redefine the array.

It gets even more tricky to keep track of it all when files include header files that include other header files.

I’m guessing some compilers just don’t allow that in header files due to those reasons. You might be able to get away with it by adding some compiler flags, but why not just follow the best practices and have code that plays nice everywhere.

 ntewinkel then you end up with a mess of unreadable code (toilet roll).