[AVR GCC] Error called object is not a function or function pointer

[Simon]

Just starting to play with some code and the below line gets me: Error called object is not a function or function pointer.

Code: [Select]

#define TCB0_CLKSEL_CLK_PER      TCB0.CTRLA = ((TCB0.CTRLA & 0b11111001) | 0b00000000) // use peripheral clock.


[Simon]

ok, turns out it was caused by a lack of a colon on a similar statement on a different register several lines before. Hm, that's shit! an error in one place throws up an error sowhere else totally unrelated or maybe not as my error was on TCB0.CTRLA and both registers appear to be struct variables in the same group

[T3sl4co1l]

Also,



Tim

[PhilipPeake]

That is what using preprocessor macro functions wins you. :-)

[Simon]

That is what using preprocessor macro functions wins you. :-)

Not really. Had i used the raw math and forgotten the colon on the same statement it would have thrown an error on the same other statement!

All the define did was tell me what the heck i was doing and hiding the meaningless gibberish that will be copy and pasted in at compile time.

[Mechatrommer]

"| 0b00000000"? ok thats charming...

[andyturk]

Code: [Select]

#define TCB0_CLKSEL_CLK_PER      TCB0.CTRLA = ((TCB0.CTRLA & 0b11111001) | 0b00000000) // use peripheral clock.


Yuck!

1. Most people avoid putting comments "in" macro definitions
2. "| 0b00000000" ... why? (already mentioned by Mechatrommer)
3. You're defining an "object-like" macro there, but it's got an assignment in it? Dangerous!
4. Why isn't "0b11111001" a named constant?
5. A more readable name for the macro would be helpful. What does "_CLK_PER" mean?

[NorthGuy]

1. Most people avoid putting comments "in" macro definitions

The compiler strips out comments before processing macros. Thus it doesn't matter where you put the comments.

[Simon]

Code: [Select]

#define TCB0_CLKSEL_CLK_PER      TCB0.CTRLA = ((TCB0.CTRLA & 0b11111001) | 0b00000000) // use peripheral clock.


Yuck!

1. Most people avoid putting comments "in" macro definitions
2. "| 0b00000000" ... why? (already mentioned by Mechatrommer)
3. You're defining an "object-like" macro there, but it's got an assignment in it? Dangerous!
4. Why isn't "0b11111001" a named constant?
5. A more readable name for the macro would be helpful. What does "_CLK_PER" mean?

1. Why?! comments are throw away text and useful in any code.
2. Yes it is pointless in this statement but this is the first line in a series of statements where that operation changes number, it's simply a template:

// CNTMODE[2:0] Timer Mode Writing these bits selects the Timer mode
#define TCB0_MODE_PERIODIC_INTERRUPT  TCB0.CTRLB = ((TCB0.CTRLB & 0b11111000) | 0b00000000)
#define TCB0_MODE_TIME_OUT_CHECK      TCB0.CTRLB = ((TCB0.CTRLB & 0b11111000) | 0b00000001)
#define TCB0_MODE_EVENT               TCB0.CTRLB = ((TCB0.CTRLB & 0b11111000) | 0b00000010)
#define TCB0_MODE_FREQ_MEASUREMENT    TCB0.CTRLB = ((TCB0.CTRLB & 0b11111000) | 0b00000011)
#define TCB0_MODE_PW_MEASUREMENT      TCB0.CTRLB = ((TCB0.CTRLB & 0b11111000) | 0b00000100)
#define TCB0_MODE_FREQ_PW_MEASUREMENT TCB0.CTRLB = ((TCB0.CTRLB & 0b11111000) | 0b00000101)
#define TCB0_MODE_SINGLE_SHOT         TCB0.CTRLB = ((TCB0.CTRLB & 0b11111000) | 0b00000110)
#define TCB0_MODE_8BIT_PWM            TCB0.CTRLB = ((TCB0.CTRLB & 0b11111000) | 0b00000111)

4. A named constant. OK if I name one I have to name them all, that is 2^8 permutations........ why does it need naming and further obfuscating? at some point register bits need setting to 0 and 1. Do you make up constants for this stuff when you put such code inline in your main program? making it a constant technically requires an additional .c file.
5. "CLK_PER is THE name of the periphery clock in the datasheet, I can't think of a better name myself.

[Mechatrommer]

1. Most people avoid putting comments "in" macro definitions

The compiler strips out comments before processing macros. Thus it doesn't matter where you put the comments.

agree, and it helps readability. when we know it doesnt impact the generated code negatively, and on the other side it can give positive impact, we'll do it.

4. A named constant. OK if I name one I have to name them all, that is 2^8 permutations........

you dont have to. only name what are "useful". when you see people try to define all the permutations, then he got an off the chart intelligence

why does it need naming and further obfuscating?

readability. one may think its for the purpose of others, but its not just for others, its also for the programmer himself (you) few months or years from now.

one may think why the heck is "| 0b00000000"? when in fact there is something in your mind, only you know it, hence not readable to others. and in few years when you have forgotten it, it will not be readable to you as well. btw, i did occasionally this kind of charming code because sometime i hate to see one line of code is shorter than the other, so adding a meaningless code may add to the "charmingness", but some people wont give a rat ass and further obfuscated his mind.

now i dont know whats going on in your code (semantically), but for readability sake may i suggest...

Code: [Select]

#define INTR_BIT 0b00000000
#define INTR_MSK 0b11111000

#define CHCK_BIT 0b00000001
#define CHCK_MSK 0b11111000

#define EVNT_BIT 0b00000010
#define EVNT_MSK 0b11111000

...

#define TCB0_MODE_PERIODIC_INTERRUPT  TCB0.CTRLB = ((TCB0.CTRLB & INTR_MSK) | INTR_BIT)
#define TCB0_MODE_TIME_OUT_CHECK      TCB0.CTRLB = ((TCB0.CTRLB & CHCK_MSK) | CHCK_BIT)
#define TCB0_MODE_EVENT               TCB0.CTRLB = ((TCB0.CTRLB & EVNT_MSK) | EVNT_BIT)

...

few things to note...

1) there are few duplicate define (*_MSK), some one may argue what the fuck, why dont just declare it once and use in all? well as you said, they have certain purpose and meaning. being none duplicated is not the point, readability is. and in the future, the value may change, so you only change the define, you dont have to touch the rest.
2) i dont put parenthesis in the define because its a simple number, i imagine there's very little (from experience) can go wrong with that. if you want extra taboo, you can put parenthesis, it may help a "good practice" habit, it wont sacrifice readability imho.

Code: [Select]

#define INTR_BIT (0b00000000)

3) i put some cryptic shortcut such as CHCK instead of CHECK, MSK instead of MASK, because i'm one of the charming coder its good when everything are line up nicely (same character count as the next lines) but sometime it will put us in hard decision because readability is sacrificed, so the charmingness is sometime not advisable. but when we think other reader can assume the meaning correctly, we tend to do so, like in whatsapp texting, we know (or hope) that everybody will know what a INTR means, dont you? further we usually and keep seeing super extra space separators placed in the code just to line up above and bottom code in many places and coders, just as in your example, its not so obvious if you dont put it in code quotation (thats why coders have been using and will always be using i suspect, archaic font such as courier or lucida (fixed width font) instead of times new roman or arial) so i believe i'm not the only charming coder here. but if we go too far, or meaningless without comment or explanation, it can go problematic.

fwiw...

[Nusa]

If I had a dollar for every time I've fixed someones code simply by moving the // comment in a #define to the line before.

One of my rules for writing portable code is never put a comment on the same line as a #define. True, it won't bite you 99.9% of the time, but finding those 0.1% problems makes up for it!

[Mechatrommer]

inline comment wont even pass the compiler gate. try:

Code: [Select]

#define MYNUM 10 // my number

void main() {
    int i  = MYNUM + 1;
};

it wont hurt and value is just correct, at least to the compilers i'm using here. its not like it will get expanded to...

Code: [Select]

    int i  = 10 // my number + 1;


[Ian.M]

1. Why?! comments are throw away text and useful in any code.
2. Yes it is pointless in this statement but this is the first line in a series of statements where that operation changes number, it's simply a template:

Code: [Select]

// CNTMODE[2:0] Timer Mode Writing these bits selects the Timer mode
#define TCB0_MODE_PERIODIC_INTERRUPT  TCB0.CTRLB = ((TCB0.CTRLB & 0b11111000) | 0b00000000)
#define TCB0_MODE_TIME_OUT_CHECK      TCB0.CTRLB = ((TCB0.CTRLB & 0b11111000) | 0b00000001)
#define TCB0_MODE_EVENT               TCB0.CTRLB = ((TCB0.CTRLB & 0b11111000) | 0b00000010)
#define TCB0_MODE_FREQ_MEASUREMENT    TCB0.CTRLB = ((TCB0.CTRLB & 0b11111000) | 0b00000011)
#define TCB0_MODE_PW_MEASUREMENT      TCB0.CTRLB = ((TCB0.CTRLB & 0b11111000) | 0b00000100)
#define TCB0_MODE_FREQ_PW_MEASUREMENT TCB0.CTRLB = ((TCB0.CTRLB & 0b11111000) | 0b00000101)
#define TCB0_MODE_SINGLE_SHOT         TCB0.CTRLB = ((TCB0.CTRLB & 0b11111000) | 0b00000110)
#define TCB0_MODE_8BIT_PWM            TCB0.CTRLB = ((TCB0.CTRLB & 0b11111000) | 0b00000111)
4. A named constant. OK if I name one I have to name them all, that is 2^8 permutations........ why does it need naming and further obfuscating? at some point register bits need setting to 0 and 1. Do you make up constants for this stuff when you put such code inline in your main program? making it a constant technically requires an additional .c file.
5. "CLK_PER is THE name of the periphery clock in the datasheet, I can't think of a better name myself.

That's not how I'd do it.   

Code: [Select]

// Assign bits in x defined by mask to reg, LSB at bit pozn. Other bits of reg left unchanged.
#define WRITEBITS(reg, pozn, mask, x) (reg^=(reg^((x)<<(pozn)))&((mask)<<(pozn)))
//...

// CNTMODE[2:0] selects Timer Mode
#define TCB0_MODE_POZN 0
#define TCB0_MODE_MASK 0b111
// Timer modes in consecutive ascending order
enum {  TCB0_MODE_PERIODIC_INTERRUPT=0,
        TCB0_MODE_TIME_OUT_CHECK,
        TCB0_MODE_EVENT,
        TCB0_MODE_FREQ_MEASUREMENT,
        TCB0_MODE_PW_MEASUREMENT,
        TCB0_MODE_FREQ_PW_MEASUREMENT,
        TCB0_MODE_SINGLE_SHOT,
        TCB0_MODE_8BIT_PWM };

#define SET_TCB0_MODE(mode) WRITEBITS(TCB0.CTRLB, TCB0_MODE_POZN, TCB0_MODE_MASK, (mode))
then invoke with a function-like macro call. e.g:

Code: [Select]

SET_TCB0_MODE(TCB0_MODE_FREQ_MEASUREMENT);
Note that both the bit constants to be assigned and the mask for them are shifted 'up' the register by pozn so that enum can be used to declare them even for groups of bits that don't include the LSB of the register.  If you need to skip a bit value in the enum, e.g. 0b101, declare it as ..._RESERVED101

[T3sl4co1l]

I still don't get why you're going and inventing your own thing that's not adding functionality to the built in libraries, and is different from industry standards so will therefore bite some poor programmer who has to maintain it in some years.  Or some weeks, since IIRC you said this is supposed to be very portable right out the door or something?

Tim

[Mechatrommer]

Code: [Select]

SET_TCB0_MODE(TCB0_MODE_FREQ_MEASUREMENT);

i dont like this "still the machine/register code" naming. in the end, few months from now we have to reopen datasheet to see what its all about. i prefer something like human friendly meaning such as...

Code: [Select]

TIMER_MODE(TCB0_MODE_FREQ_MEASUREMENT);or

Code: [Select]

SET_PWM_TIMER_MODE(TCB0_MODE_FREQ_MEASUREMENT);whatever... so our code can become self documenting re explaining what the datasheet tries to do. but as i said, i dont know what the register actually do i may be mistaken in the naming and you can do whatever you think most readable to you. we are here just to advice and sharing thoughts but ymmv.

[Ian.M]

In general, I'm mostly with you on the naming thing, but for this usage case, I was only going on what Simon provided without reference to the datasheet. 

Unless you want to write yet another thin hardware abstraction layer for supposed portability, with constants for all possible modes of all possible timers that will be translated on a per timer basis to the correct mode bits for that timer,  (which IMHO is *NOT* a good idea:

https://xkcd.com/927/

as the hardware abstraction layer documentation is almost invariably significantly poorer than the datasheet), the psuedofunction macro name cannot sensibly be detached from the register name, as that provides the vital visual check that you are using the right group of constants with it.   

I suppose you could name the constants TIMER0_MODE_.... and the function TIMER0_MODE(), but as you will definitely need the datasheet open anyway, there's little to be gained.

[T3sl4co1l]

That's not how I'd do it.

...

then invoke with a function-like macro call. e.g:

Code: [Select]

SET_TCB0_MODE(TCB0_MODE_FREQ_MEASUREMENT);
Note that both the bit constants to be assigned and the mask for them are shifted 'up' the register by pozn so that enum can be used to declare them even for groups of bits that don't include the LSB of the register.  If you need to skip a bit value in the enum, e.g. 0b101, declare it as ..._RESERVED101

This is good, and only a modest variation on the way Atmel did it (at least for the MEGA family) already.  A random example from an xmega header:

Code: [Select]

/* Quadrature Decoder Index Recognition Mode */
typedef enum EVSYS_QDIRM_enum
{
    EVSYS_QDIRM_00_gc = (0x00<<5),  /* QDPH0 = 0, QDPH90 = 0 */
    EVSYS_QDIRM_01_gc = (0x01<<5),  /* QDPH0 = 0, QDPH90 = 1 */
    EVSYS_QDIRM_10_gc = (0x02<<5),  /* QDPH0 = 1, QDPH90 = 0 */
    EVSYS_QDIRM_11_gc = (0x03<<5),  /* QDPH0 = 1, QDPH90 = 1 */
} EVSYS_QDIRM_t;

"_gc" stands for "group configuration", so it is the pattern of bits to set for the desired configuration.  The register is masked with the "_gm" (group mask), and the individual bits with "_bm" (bit mask).

Therefore, a basic modify statement has the form:

Code: [Select]

port.register = (port.register & ~port_bitgroup_gm) | port_bitgroup_state_gc;
The "& ~" reads the existing register value and zeroes the desired bits; the | puts back in the desired bits.  (The bitmasking can be omitted in the initialization routine, where you don't need to worry about the initial state of other bits in the register.)

The only reason you should need/want to get fancier, is to be able to change all "port.register" and bitgroup/state references, to a more general definition, in your own headers.

This is more or less what Ian is doing: putting the whole assignment into a macro function.

By encapsulating the assignment in a more general macro, you only have to change, say, a few dozens of register, bit and group names.

You definitely don't want to maintain exhaustive lists of bits, that will accumulate errors faster than you can say "refactor"!

At the very least, if you insist on building such lists, write a tool to generate them!  That's how the pros make their headers (well, at least I would hope so..).  That's also what their headers are for.  Use them!

With this additional level of naming indirection, you can -- if nothing else -- use them in the same way as the XMEGA style headers, even for platforms that don't use that format (e.g., original MEGA).  You can get consistent naming conventions across families, or sub-families anyway.  It's probably a fair method even to support completely different platforms, too (ARM?)!  Though, that's probably optimistic, and  you may need more (or fewer) statements, on platforms quite that different (i.e., wider-bit devices probably use fewer registers to hold all their control/status bits).

Generality is the key here.  You don't want to go more specific, less general.  You must only go more general.  To go more specific, means more code refactoring.  More work, more frustration!

It also means this: if you don't know, in what directions you should generalize -- if you aren't very familiar with the ecosystems of microcontrollers in general -- you probably shouldn't do it at all, and just leave it at the family level.

Make it obvious to the reader, which functions/defines need to be checked/changed, to migrate to a different device/family, and leave it at that.

Better still, if it's an open project: let other, more experienced programmers fork and improve it.  If you don't have that kind of breadth of knowledge, don't make it up as you go -- take advantage of those more experienced!

Tim

[Siwastaja]

Didn't see anyone mention this yet, but as a general note, when you want to create a function-like macro in C preprocessor, you need to do it like this:

#define do_something() do{ your_actual_code } while(0)

This way, the macro is actually usable like any function would be. do { } while(0); always runs once (and doesn't generate any extra instructions). It's syntactical boilerplate.

This looks absolutely stupid and totally illogical (google for it to find reasons and explanation why you need this), but C preprocessor just is this way, and you need to live with this. C is far from perfect and many details are slightly broken, but many alternatives proposed over the years have had more fundamental problems.

Edit: removed excess semicolon.

[andersm]

Didn't see anyone mention this yet, but as a general note, when you want to create a function-like macro in C preprocessor, you need to do it like this:

#define do_something() do{ your_actual_code } while(0);

You want to drop the final semicolon though.

[Ian.M]

Yes.  However *some* compilers are borked and don't optimise away the traditional do{ .... }while(0) wrapper.  Finger of shame pointed at older versions of Microchip XC8 in FREE mode! 

That's why I wrapped it in () and made sure its contents are a valid expression.  It serves the same purpose of making the macro act as if it was a single native C statement to surrounding blocks and flow control, returning the register value, which is discarded when its invoked.   

I suppose one could try a ((void)( ... )) wrapper to discard the return value.

Another trap is parameters that have side effects when evaluated.   A C function evaluates them once when its called, but the macro evaluates them each time they are used, so if it use any of them more than once, it wont behave like a function.

[Simon]

If I had a dollar for every time I've fixed someones code simply by moving the // comment in a #define to the line before.

One of my rules for writing portable code is never put a comment on the same line as a #define. True, it won't bite you 99.9% of the time, but finding those 0.1% problems makes up for it!

Oh so I am allowed a comment in the header file. yes if not all preprocessors ignore // and everything after it sure.

[Simon]

I'm sorry but for years people have put in their code some register anded and ored to set certain bits with a numerical value. I put it behind a macro and now that number has to be a constant??? what the fuck are you all on about? jesus christ! you want me to either create a constant whoes name if generic will be erm... let me think.... the number it represents and if i do them specific I'll just be making multiple constants that have the same values....

Indeed, coding is a sodding religion!!!!!!!!!!!!!!!!!!

[andyturk]

Indeed, coding is a sodding religion!!!!!!!!!!!!!!!!!!

Isn't it amazing how much discussion just a single line of code can generate? We are Artists. (*cough*) And coding styles are like snowflakes... 

If Kernighan and/or Ritchie posted code on the internet, it would probably be ripped to shreds. So don't feel too bad about the volume of discussion.

There have been lots of good suggestions in this thread. Now it's up to you to soak that all in and decide whether to go back and refactor¹ working code to make it cleaner, safer, or whatever. None of this stuff will matter if you're at the, "I just want fscking thing to work!" stage. But if you're trying to convince other programmers to use your shiny new library of goodness, removing some of the objections listed here might be a good idea.

-----------

1. Real programmers don't change code, they "refactor" it, which sounds a lot more impressive.

[NorthGuy]

If I had a dollar for every time I've fixed someones code simply by moving the // comment in a #define to the line before.

You wouldn't earn a single dollar.

One of my rules for writing portable code is never put a comment on the same line as a #define. True, it won't bite you 99.9% of the time, but finding those 0.1% problems makes up for it!

Could you show a single example of how this can cause a problem?

[mikerj]

I'm sorry but for years people have put in their code some register anded and ored to set certain bits with a numerical value. I put it behind a macro and now that number has to be a constant??? what the fuck are you all on about? jesus christ! you want me to either create a constant whoes name if generic will be erm... let me think.... the number it represents and if i do them specific I'll just be making multiple constants that have the same values....

Indeed, coding is a sodding religion!!!!!!!!!!!!!!!!!!

You have demonstrated numerous times that you don't yet fully understand some pretty fundamental concepts in C (e.g. integer promotion, const values etc.), so why do you feel qualified to criticise both the language and peoples advice on how to use it?

ORing a value with zero is pointless.  Even though the optimiser should strip it out it adds nothing to the readability of your code and arguably detracts from it, so why do it?

You said you'd need to define 2^8 symbols for every register value but this isn't true for typical peripherals that use one or more bits per register to control specific functions.  You define symbols for each function in the register and then OR them together.  Usually these are already defined for you in a target specific header that comes with the compiler.  This way someone reading your code can see what it's doing without having to decode "magic" numbers.

Also using binary notation is not a good idea, this is a specific GCC extension and not supported by many/most compilers.  Use hex notation in these cases, it's very easy to mentally convert this to a binary sequence and is standard C.

[sokoloff]

If I had a dollar for every time I've fixed someones code simply by moving the // comment in a #define to the line before.

You wouldn't earn a single dollar.

On any C99-or-later conforming compiler... C99 is the first version of the C standard that clarified how those comments where to be handled in the pre-processor phase.

Before that, I could certainly believe that early cfront implementations (the original C++ to C preprocessor) or any C pre-processor that didn't understand "//" but where the subsequent compiler did could have exhibited such behavior.

[sokoloff]

I'm sorry but for years people have put in their code some register anded and ored to set certain bits with a numerical value. I put it behind a macro and now that number has to be a constant??? what the fuck are you all on about? jesus christ! you want me to either create a constant whoes name if generic will be erm... let me think.... the number it represents and if i do them specific I'll just be making multiple constants that have the same values....

Indeed, coding is a sodding religion!!!!!!!!!!!!!!!!!!

You have demonstrated numerous times that you don't yet fully understand some pretty fundamental concepts in C (e.g. integer promotion, const values etc.), so why do you feel qualified to criticise both the language and peoples advice on how to use it?

ORing a value with zero is pointless.  Even though the optimiser should strip it out it adds nothing to the readability of your code and arguably detracts from it, so why do it?

I disagree. It tells me, as a reader of the code, that the author consciously is not setting those bits (as opposed to simply having forgotten or made a copy/paste error).

I'd be inclined towards a variant of the code as below, assuming you are using a C++14 or later compiler (which supports the 0b/0B notation as part of the language spec).

Code: [Select]

// CNTMODE[2:0] Timer Mode Writing these bits selects the Timer mode
#define TCB0_TIMER_MODE_MASK 0b00000111
#define TCB0_SET_TIMER_MODE_PERIODIC_INTERRUPT   TCB0.CTRLB = ((TCB0.CTRLB & (^TCB0_TIMER_MODE_MASK)) | 0b000)
#define TCB0_SET_TIMER_MODE_TIME_OUT_CHECK       TCB0.CTRLB = ((TCB0.CTRLB & (^TCB0_TIMER_MODE_MASK)) | 0b001)
#define TCB0_SET_TIMER_MODE_EVENT                TCB0.CTRLB = ((TCB0.CTRLB & (^TCB0_TIMER_MODE_MASK)) | 0b010)
#define TCB0_SET_TIMER_MODE_FREQ_MEASUREMENT     TCB0.CTRLB = ((TCB0.CTRLB & (^TCB0_TIMER_MODE_MASK)) | 0b011)
#define TCB0_SET_TIMER_MODE_PW_MEASUREMENT       TCB0.CTRLB = ((TCB0.CTRLB & (^TCB0_TIMER_MODE_MASK)) | 0b100)
#define TCB0_SET_TIMER_MODE_FREQ_PW_MEASUREMENT  TCB0.CTRLB = ((TCB0.CTRLB & (^TCB0_TIMER_MODE_MASK)) | 0b101)
#define TCB0_SET_TIMER_MODE_SINGLE_SHOT          TCB0.CTRLB = ((TCB0.CTRLB & (^TCB0_TIMER_MODE_MASK)) | 0b110)
#define TCB0_SET_TIMER_MODE_8BIT_PWM             TCB0.CTRLB = ((TCB0.CTRLB & (^TCB0_TIMER_MODE_MASK)) | 0b111)

[sokoloff]

C++14 compiler support for 0b literals is fairly common nowadays:

https://en.cppreference.com/w/cpp/compiler_support#cpp14

[Mechatrommer]

so why do you feel qualified to criticise both the language and peoples advice on how to use it?

what a "too clever" kid will answer to his father's advice? well in this case the OP is not a kid and i guess its just because he doesnt need it, i should have sticked with my 1st reply, instead i got trolled by the later poster let the OP play with his new toy peacefully. (ps: i lately tend to delete my posts before someone else do, which are not relevant to the topic or pose less interest, it will not add value to my post count and hence is a shame )

[Simon]

I'm sorry but for years people have put in their code some register anded and ored to set certain bits with a numerical value. I put it behind a macro and now that number has to be a constant??? what the fuck are you all on about? jesus christ! you want me to either create a constant whoes name if generic will be erm... let me think.... the number it represents and if i do them specific I'll just be making multiple constants that have the same values....

Indeed, coding is a sodding religion!!!!!!!!!!!!!!!!!!

You have demonstrated numerous times that you don't yet fully understand some pretty fundamental concepts in C (e.g. integer promotion, const values etc.), so why do you feel qualified to criticise both the language and peoples advice on how to use it?

ORing a value with zero is pointless.  Even though the optimiser should strip it out it adds nothing to the readability of your code and arguably detracts from it, so why do it?

You said you'd need to define 2^8 symbols for every register value but this isn't true for typical peripherals that use one or more bits per register to control specific functions.  You define symbols for each function in the register and then OR them together.  Usually these are already defined for you in a target specific header that comes with the compiler.  This way someone reading your code can see what it's doing without having to decode "magic" numbers.

Also using binary notation is not a good idea, this is a specific GCC extension and not supported by many/most compilers.  Use hex notation in these cases, it's very easy to mentally convert this to a binary sequence and is standard C.

Granted i am a novice but DDRB = a load of math is commonly seen, why do I suddenly need to make these values into indescribable variables?. I need to write several bits in a register at one time, these values of interest can be several bits long starting and ending anywhere and of and any combination therefor there are numerous combinations. I really don't get it, what I am being told makes no sense in this regard and I have never seen it done before.

I was not aware that binary was GCC specific. Given that we are talking about individual bit positions in a register binary is the mos readable format that relates directly to the datasheet. All i am doing is trying to convert a statement like "TCB0.CTRLB = ((TCB0.CTRLB & 0b11111000) | 0b00000010)" into something meaningful, it is being done by simple copy and pasting by the preprocessor and is the only way i can think of programming without restudying the datasheet every time I need to change something ore write something new. I am even using the datasheet's own definitions and names to make it even easier to find what the hell i was on about in the datasheet. Yes the first statement in the list is semi nonsensical but does no harm and works, the problem exactly was ?

[NorthGuy]

I personally don't like copy and paste, so I try to avoid it. Otherwise I misspell something and then I need to correct the mistake in every re-incarnation of whatever I copied and pasted. It's not always possible to avoid copying and pasting, but it is possible, to some extent, here. So, if I ever wanted to do the same as OP, I would do:

Code: [Select]

#define TCB0_MODE(v) TCB0.CTRLB = ((TCB0.CTRLB & 0xf8) | (v))

#define TCB0_MODE_PERIODIC_INTERRUPT TCB0_MODE(0)
#define TCB0_MODE_TIME_OUT_CHECK TCB0_MODE(1)
Also, if I ever wanted to change what I've done, I would need to change it in only one place, for example

Code: [Select]

#define TCB0_MODE(v) TCB0.CTRLB ^= (TCB0.CTRLB ^ (v)) & 0x07

#define TCB0_MODE_PERIODIC_INTERRUPT TCB0_MODE(0)
#define TCB0_MODE_TIME_OUT_CHECK TCB0_MODE(1)

[sokoloff]

I was not aware that binary was GCC specific.

It's not. It was previously an extension to the language, supported early on by gcc and others, but became part of the C++14 standard.

[Simon]

So I define it once in my standard header file, make sure it is correct and then copy and paste my human readable defines into the code. This is easier to get right. If i make a code error I correct the code in one place and the correction is carried through by the next preprocessor.

Ultimately you can correct mistakes with find and replace in most programs, atmel studio will do it on the entire project/solution if needs be.

[Simon]

I was not aware that binary was GCC specific.

It's not. It was previously an extension to the language, supported early on by gcc and others, but became part of the C++14 standard.

So I am doing good after all!

[Nusa]

If I had a dollar for every time I've fixed someones code simply by moving the // comment in a #define to the line before.

You wouldn't earn a single dollar.

One of my rules for writing portable code is never put a comment on the same line as a #define. True, it won't bite you 99.9% of the time, but finding those 0.1% problems makes up for it!

Could you show a single example of how this can cause a problem?

Sure. Here's a link to a real-live example that broke when the Arduino preprocessor was changed in 2016 (I'm known as Eric on that forum).
http://forum.seemecnc.com/viewtopic.php?f=99&t=10268&p=91408&hilit=comment#p91407

The define in question is:
#define Z_PROBE_PIN 16 // mini-rambo ext pins on P3 ext next to LCD header
The fix was:
// mini-rambo ext pins on P3 ext next to LCD header
#define Z_PROBE_PIN 16
Technically this would work too, but would get broken the next time someone OCD about formatting touched the code:
#define Z_PROBE_PIN 16// mini-rambo ext pins on P3 ext next to LCD header

You have to remember these are strings, not numbers. Does it expand to "16" or "16 "? Most of the time it doesn't matter. This is one of those cases where it does. The rest of the story is found in the macros that translate the arduino logical pin number to the physical pin identifier. The macro _READ() is invoked by the macro READ(Z_PROBE_PIN) and is found in a file named fastio.h:

Code: [Select]

/*
This code contibuted by Triffid_Hunter and modified by Kliment
why double up on these macros? see http://gcc.gnu.org/onlinedocs/cpp/Stringification.html
        2012/3/10 AT90USB128x modified by lincomatic to match Teensyduino
*/
#ifndef _ARDUINO_H
#define _ARDUINO_H

#include <avr/io.h>

/*
utility functions
*/

#ifndef MASK
/// MASKING- returns \f$2^PIN\f$
#define MASK(PIN) (1 << PIN)
#endif

/*
magic I/O routines
now you can simply SET_OUTPUT(STEP); WRITE(STEP, 1); WRITE(STEP, 0);
*/

/// Read a pin
#define _READ(IO) ((bool)(DIO ## IO ## _RPORT & MASK(DIO ## IO ## _PIN)))
/// write to a pin
#define _WRITE(IO, v) do { if (v) {DIO ##  IO ## _WPORT |= MASK(DIO ## IO ## _PIN); } else {DIO ##  IO ## _WPORT &= ~MASK(DIO ## IO ## _PIN); }; } while (0)
/// toggle a pin
#define _TOGGLE(IO) do {DIO ##  IO ## _RPORT = MASK(DIO ## IO ## _PIN); } while (0)

/// set pin as input
#define _SET_INPUT(IO) do {DIO ##  IO ## _DDR &= ~MASK(DIO ## IO ## _PIN); } while (0)
/// set pin as output
#define _SET_OUTPUT(IO) do {DIO ##  IO ## _DDR |=  MASK(DIO ## IO ## _PIN); } while (0)

/// check if pin is an input
#define _GET_INPUT(IO) ((DIO ## IO ## _DDR & MASK(DIO ## IO ## _PIN)) == 0)
/// check if pin is an output
#define _GET_OUTPUT(IO) ((DIO ## IO ## _DDR & MASK(DIO ## IO ## _PIN)) != 0)

// why double up on these macros? see http://gcc.gnu.org/onlinedocs/cpp/Stringification.html

/// Read a pin wrapper
#define READ(IO) _READ(IO)
/// Write to a pin wrapper
#define WRITE(IO, v) _WRITE(IO, v)
#define     PULLUP(IO,v)            _WRITE(IO, v)
/// toggle a pin wrapper
#define TOGGLE(IO) _TOGGLE(IO)

/// set pin as input wrapper
#define SET_INPUT(IO) _SET_INPUT(IO)
/// set pin as output wrapper
#define SET_OUTPUT(IO) _SET_OUTPUT(IO)

/// check if pin is an input wrapper
#define GET_INPUT(IO) _GET_INPUT(IO)
/// check if pin is an output wrapper
#define GET_OUTPUT(IO) _GET_OUTPUT(IO)

/*
ports and functions
added as necessary or if I feel like it- not a comprehensive list!
*/

#if defined (__AVR_ATmega168__) || defined (__AVR_ATmega328__) || defined (__AVR_ATmega328P__)
// UART
#define RXD DIO0
#define TXD DIO1

// SPI
#define SCK 13
#define MISO DIO12
#define MOSI DIO11
#define SS 10

// TWI (I2C)
#define SCL AIO5
#define SDA AIO4

// timers and PWM
#define OC0A DIO6
#define OC0B DIO5
#define OC1A DIO9
#define OC1B DIO10
#define OC2A DIO11
#define OC2B DIO3

#define DEBUG_LED AIO5

/*
pins
*/

#define DIO0_PIN PIND0
#define DIO0_RPORT PIND
#define DIO0_WPORT PORTD
#define DIO0_DDR DDRD
#define DIO0_PWM NULL

#define DIO1_PIN PIND1
#define DIO1_RPORT PIND
#define DIO1_WPORT PORTD
#define DIO1_DDR DDRD
#define DIO1_PWM NULL

#define DIO2_PIN PIND2
#define DIO2_RPORT PIND
#define DIO2_WPORT PORTD
#define DIO2_DDR DDRD
#define DIO2_PWM NULL

#define DIO3_PIN PIND3
#define DIO3_RPORT PIND
#define DIO3_WPORT PORTD
#define DIO3_DDR DDRD
#define DIO3_PWM &OCR2B

///* SNIP....you get the idea, and the file is way too long to copy here anyway */



[Simon]

except I do not try to write code with macros or setup any conditions. I simply use them as text replacement tools to make code more readable or to define a constant where the "cosnt" assignment fails.

If you took all of the code behind my macros and put that in the c code instead it would be the same thing.

[Nusa]

If I had a dollar for every time I've fixed someones code simply by moving the // comment in a #define to the line before.

You wouldn't earn a single dollar.

On any C99-or-later conforming compiler... C99 is the first version of the C standard that clarified how those comments where to be handled in the pre-processor phase.

Before that, I could certainly believe that early cfront implementations (the original C++ to C preprocessor) or any C pre-processor that didn't understand "//" but where the subsequent compiler did could have exhibited such behavior.

I've been getting paid for code since 1973, so I've lived the entire era where the preprocessor didn't support it, while the compiler did. And yes, it's certainly possible some of my personal rules are a bit obsolete, but they have served me well.

This particular rule still has some value, as I just demonstrated in my previous response.

[sokoloff]

If I had a dollar for every time I've fixed someones code simply by moving the // comment in a #define to the line before.

You wouldn't earn a single dollar.

On any C99-or-later conforming compiler... C99 is the first version of the C standard that clarified how those comments where to be handled in the pre-processor phase.

Before that, I could certainly believe that early cfront implementations (the original C++ to C preprocessor) or any C pre-processor that didn't understand "//" but where the subsequent compiler did could have exhibited such behavior.

I've been getting paid for code since 1973, so I've lived the entire era where the preprocessor didn't support it, while the compiler did. And yes, it's certainly possible some of my personal rules are a bit obsolete, but they have served me well.

This particular rule still has some value, as I just demonstrated in my previous response.

Completely understand.
I was trying to support your point. It seems like I may not have succeeded...

[NorthGuy]

You have to remember these are strings, not numbers. Does it expand to "16" or "16 "? Most of the time it doesn't matter. This is one of those cases where it does.

It expands to "16". It is tokenized before pre-processing. Pre-processor operates on tokens.

The problem here is not the use of comments, but the use of buggy pre-processors/compilers.

Either this

Code: [Select]

#define Z_PROBE_PIN 16// mini-rambo ext pins on P3 ext next to LCD header
#define MASK(PIN) (1 << PIN)
#define _READ(IO) ((bool)(DIO ## IO ## _RPORT & MASK(DIO ## IO ## _PIN)))
#define READ(IO) _READ(IO)

READ(Z_PROBE_PIN)
or this

Code: [Select]

#define Z_PROBE_PIN 16 // mini-rambo ext pins on P3 ext next to LCD header
#define MASK(PIN) (1 << PIN)
#define _READ(IO) ((bool)(DIO ## IO ## _RPORT & MASK(DIO ## IO ## _PIN)))
#define READ(IO) _READ(IO)

READ(Z_PROBE_PIN)
expands to this:

Code: [Select]

((bool)(DIO16_RPORT & (1 << DIO16_PIN)))
If your tools don't have bugs that is.

If you have open source tools, rather than adhering to their bugs, you better fix the tools.

[cv007]

#define TCB0_MODE(v) TCB0.CTRLB = ((TCB0.CTRLB & 0xf8) | (v))

you can even take it further since you have more than one of the same timer-
#define TCB_MODE(n,v) TCB##n##.CTRLB = ((TCB##n##.CTRLB & 0xf8) | (v))

now you have a generic for every timer b
TCB_MODE(0, 0)
TCB_MODE(1, 0)

(you would even get an error if n was an invalid number)


but it is endless when it comes to defines/macros, and you end up with another language to maintain where you use the manufacturers defines/macros, your own defines/macros, and ultimately the functions which will use a combination of both

not trying to throw wrenches into gears, but maybe consider just using functions to do the work
a simple example using the avr defines already provided, and turning the mode setting into a function-

Code: [Select]

//just to make sure using a valid timer number
//(there could already be something in the avr include for this, I don't know)
typedef enum { TCBn0, TCBn1, TCBn2, TCBn3 } TCBn_t;

//simple function
//if we want to know what some of these defines are- look in one file- iom4809.h
//this function and iom4809.h are all we need to see how this function works
TCBmode(TCBn_t n, TCB_CNTMODE_t mode){
     TCB0[n].CTRLB = (TCB0[n].CTRLB & (TCB_CNTMODE_gm<<TCB_CNTMODE_gp)) | mode;
}//usage- it looks like a function, and it is a function
TCBmode( TCBn0 ,TCB_CNTMODE_TIMEOUT_gc );

//compare to
TCB0_MODE_TIME_OUT_CHECK;

//or
SET_TCB0_MODE(TCB0_MODE_TIME_OUT_CHECK);

//or
TCB_MODE(0, TCB0_MODE_TIME_OUT_CHECK);

//or any other type of macro/define
//AND_IT_ENDS_UP_LOOKING_LIKE_THIS_EVERYWHERE_FOR_EVERYTHING & WHICH_IS_HARD_TO_READ & DECIPHER


I know every last byte wants to be squeezed out of a micro, but compilers are good at removing unused code, in-lining, etc.,  and in many cases the defines/macros are not worth the trouble when a function will do the job with little cost and where readability/reliability becomes much better (in my opinion).

I'll go crawl back into my hole in the ground.

[andyturk]

If you have open source crappy proprietary tools, rather than adhering to their bugs, you better fix the tools (or find a better mcu).

FTFY.

[NorthGuy]

If you have open source crappy proprietary tools, rather than adhering to their bugs, you better fix the tools (or find a better mcu).

FTFY.

I don't understand your communication, but I do consider it impolite to present your own stuff as if it was said by me. I didn't say what you have quoted, nor anything remotely similar.

[Nusa]

If you have open source tools, rather than adhering to their bugs, you better fix the tools.

Nonetheless, I provided you with the ONE example you asked for. Just because it worked it in your own personal environment doesn't mean it wasn't a real problem for real people in real lives that needed real solutions right now.

Remember, I said this is one of my "rules for writing portable code". I have to write for the world that exists, not the world that is perfect. This is a detail that, in the real world, has never been implemented consistently between implementations. I've already proved that. A simple habit avoids the problem entirely, so that's the way I go. And if someone takes some of my code to a new platform 10 years from now and it still works, I've done my job correctly.

P.S. No charge, but it would be nice if you sent me a dollar anyway.

[Simon]

#define TCB0_MODE(v) TCB0.CTRLB = ((TCB0.CTRLB & 0xf8) | (v))

you can even take it further since you have more than one of the same timer-
#define TCB_MODE(n,v) TCB##n##.CTRLB = ((TCB##n##.CTRLB & 0xf8) | (v))

now you have a generic for every timer b
TCB_MODE(0, 0)
TCB_MODE(1, 0)

(you would even get an error if n was an invalid number)


but it is endless when it comes to defines/macros, and you end up with another language to maintain where you use the manufacturers defines/macros, your own defines/macros, and ultimately the functions which will use a combination of both

not trying to throw wrenches into gears, but maybe consider just using functions to do the work
a simple example using the avr defines already provided, and turning the mode setting into a function-

Code: [Select]

//just to make sure using a valid timer number
//(there could already be something in the avr include for this, I don't know)
typedef enum { TCBn0, TCBn1, TCBn2, TCBn3 } TCBn_t;

//simple function
//if we want to know what some of these defines are- look in one file- iom4809.h
//this function and iom4809.h are all we need to see how this function works
TCBmode(TCBn_t n, TCB_CNTMODE_t mode){
     TCB0[n].CTRLB = (TCB0[n].CTRLB & (TCB_CNTMODE_gm<<TCB_CNTMODE_gp)) | mode;
}//usage- it looks like a function, and it is a function
TCBmode( TCBn0 ,TCB_CNTMODE_TIMEOUT_gc );

//compare to
TCB0_MODE_TIME_OUT_CHECK;

//or
SET_TCB0_MODE(TCB0_MODE_TIME_OUT_CHECK);

//or
TCB_MODE(0, TCB0_MODE_TIME_OUT_CHECK);

//or any other type of macro/define
//AND_IT_ENDS_UP_LOOKING_LIKE_THIS_EVERYWHERE_FOR_EVERYTHING & WHICH_IS_HARD_TO_READ & DECIPHER


I know every last byte wants to be squeezed out of a micro, but compilers are good at removing unused code, in-lining, etc.,  and in many cases the defines/macros are not worth the trouble when a function will do the job with little cost and where readability/reliability becomes much better (in my opinion).

I'll go crawl back into my hole in the ground.

And I have no intention of going to such extremes, why bother? it becomes a new language. I write everything for TCB0 and then do a text replacement for the other timers, simple, fast. That is all more robust and I'm not using macro's to create code but simply replace text making my code readable. I am indeed considering writing some functions but at the end of the day they will still have the macros in them so that if i ever want to be reminded of what that code does it's easy.

[NorthGuy]

If you have open source tools, rather than adhering to their bugs, you better fix the tools.

Nonetheless, I provided you with the ONE example you asked for. Just because it worked it in your own personal environment doesn't mean it wasn't a real problem for real people in real lives that needed real solutions right now.

Remember, I said this is one of my "rules for writing portable code". I have to write for the world that exists, not the world that is perfect. This is a detail that, in the real world, has never been implemented consistently between implementations. I've already proved that. A simple habit avoids the problem entirely, so that's the way I go. And if someone takes some of my code to a new platform 10 years from now and it still works, I've done my job correctly.

You cannot predict what compiler bugs you may encounter. No matter what you rules are, you cannot make sure that future bugs don't affect you.

I have encountered compiler bugs in the past. This is certainly not an easy thing to deal with. However, it would be silly if I decided to change my programming habits to work around compiler bugs.

P.S. No charge, but it would be nice if you sent me a dollar anyway.

Why me?

[sokoloff]

P.S. No charge, but it would be nice if you sent me a dollar anyway.

Why me?

Presumably referencing this:

If I had a dollar for every time I've fixed someones code simply by moving the // comment in a #define to the line before.

You wouldn't earn a single dollar.

One of my rules for writing portable code is never put a comment on the same line as a #define. True, it won't bite you 99.9% of the time, but finding those 0.1% problems makes up for it!

Could you show a single example of how this can cause a problem?

[andyturk]

If you have open source crappy proprietary tools, rather than adhering to their bugs, you better fix the tools (or find a better mcu).

FTFY.

I don't understand your communication, but I do consider it impolite to present your own stuff as if it was said by me. I didn't say what you have quoted, nor anything remotely similar.

FTFY means "Fixed That For You". I.e., I disagreed with your statement and made a snarky edit as a demonstration. I don't consider that impolite really, so perhaps we disagree on that as well.

Welcome to the internet.

[NorthGuy]

FTFY means "Fixed That For You". I.e., I disagreed with your statement and made a snarky edit as a demonstration. I don't consider that impolite really, so perhaps we disagree on that as well.

You didn't disagree with my statement. You disagreed with your personal interpretation of my statement. That is exactly why quoting should be exact. Here's what I said:

If you have open source tools, rather than adhering to their bugs, you better fix the tools.

If you want to make this into comparison with proprietary tools (which it is not), it's much simpler:

- If you have proprietary tools you have to live with their bugs.
- If you have open source tools, rather than adhering to their bugs, you better fix the tools.

Welcome to the internet.

What a lame excuse. Internet is not in the parallel virtual universe where nothing matters any more. Other participants are not robots, but human beings.

[andyturk]

You didn't disagree with my statement. You disagreed with your personal interpretation of my statement.

What? Are you trying to start a food fight? 

That is exactly why quoting should be exact.

Well sure, if it's an important conversation about technical stuff. But you made a (possibly snarky?) comment about open source tools and I made a (definitely snarky!) reply. Sigh.

Internet is not in the parallel virtual universe where nothing matters any more. Other participants are not robots, but human beings.

Agreed. Sorry if my posts offended.

FWIW, you do know your stuff and I actually look for threads where you participate, since there's often good material to be found... especially when it comes to the C preprocessor!

EDIT: "posts"

[NorthGuy]

What? Are you trying to start a food fight? 

Of course not. Bad peace is better than a good war.

[Mechatrommer]

What? Are you trying to start a food fight? 

Of course not. Bad peace is better than a good war.

but good peace is better than a bad peace... no? 

[Bruce Abbott]

Also using binary notation is not a good idea, this is a specific GCC extension and not supported by many/most compilers.

This is one of the reasons I prefer assembler. Don't have to worry about language Nazis telling you what compiler extensions you should or should not take advantage of.

C++ integer literal

binary-literal is the character sequence 0b or the character sequence 0B followed by one or more binary digits (0, 1)