3
votes

I'm developing code for Atmel, which defines consistent register names to program port pins. For instance:

  • PORTC is traditionally used to set (high or low) any pin on port C
  • PINC is used to read the state of a particular pin on port C
  • DDRC is used to read/write the direction (0=input, 1=output) of any pin on port C

So I found some code — which I fully understand — that defines a PIN macro like this:

#define LED_PIN    C,7

Then the following macros (restricted to the use case I'm talking about here):

#define _BV(bit)                        (1 << bit)
...
#define _SET(type, name, bit)           type ## name |= _BV(bit)
#define _CLEAR(type, name, bit)         type ## name &= ~ _BV(bit)
#define _TEST(type, name, bit)          ( type ## name  & _BV(bit) )
...
#define OUTPUT(pin)         _SET(DDR, pin)
#define INPUT(pin)          _CLEAR(DDR, pin)
...
#define HIGH(pin)           _SET(PORT, pin)
#define LOW(pin)            _CLEAR(PORT, pin)

So I can write a function main() as follows:

main() {
  OUTPUT(LED_PIN);
  HIGH(LED_PIN);
}

While it is very convenient and prevents from defining three macros (e.g. PINC7, PORTC7 and DDRC7) for just one LED, my main concern about this is that is imposes to also redefine AVR macros to account for that kind of notation, bearing in mind AVR macros use SFR registers as arguments (from sfr_defs.h):

#define bit_is_set(sfr, bit) (_SFR_BYTE(sfr) & _BV(bit))

Also, AVR defines macros to convert from [what I'd call] bare registry names to actual memory-mapped special-function register names (sfr) for avr-gcc:

#define _SFR_BYTE(sfr) _MMIO_BYTE(_SFR_ADDR(sfr))

and _SFR_ADDR() adds an offset 0 or 20 according to the target Atmel CPU. So in order to enhance compatibility with those AVR library functions, which take an SFR register plus an optional argument, I decided to rewrite the initial code and tried the following:

#define _SFR_BIT(type, name, bit)   type ## name, bit
#define _PORT(pin)                  _SFR_BIT(PORT, pin)
#define _PIN(pin)                   _SFR_BIT(PIN, pin)
#define _DDR(pin)                   _SFR_BIT(DDR, pin)

#define set_bit(sfr, bit)           _SFR_BYTE(sfr) |= _BV(bit)
#define set_output(pin)             set_bit(_PIN(pin))

but then I face a compiler error message as soon as I write something like this in function main():

set_output(LED_PIN);

main.cpp:72:19: error: macro "set_bit" requires 2 arguments, but only 1 given

I get the same error if I try this, too:

#define set_output(pin)             set_bit(_SFR_BIT(DDR, pin))

Why is that macro OUTPUT(), which passes only two out of of three arguments to _SET() compiles fine and my macro doesn't?


As Jens Gustedt pointed to, the generic explanation lies in the order of which the compiler resolves macros.

To transform an arbitrary number of arguments to be passed to a function with a fixed number of arguments using a macro, the function name can be made an argument to the macro:

#define EXPAND(f, ...)              f(__VA_ARGS__)
#define _SFR_BIT(type, name, bit)   type ## name, bit
...
#define set_bit(sfr, bit)           _SFR_BYTE(sfr) |= _BV(bit)
...
#define set_output(pin)             EXPAND(set_bit, _SFR_BIT(PIN, pin))

How many and which arguments are passed to the function (first argument) is resolved without any "argument missing" compiler error.

A refined version:

#define _SFR_X(f, type, name, bit)  f(type ## name, bit)
...
#define set_bit(sfr, bit)           _SFR_BYTE(sfr) |= _BV(bit)
...
#define set_output(pin)             _SFR_X(set_bit, PIN, pin)

DISCLAIMER

The code is not yet in production. The code may be ugly, of course. It's mostly some base work under construction. Hence not yet finalized.

1
Names starting with underscores are reserved for the implementation. Don't use them in application code.too honest for this site
Is that the only name starting with an underscore?? What in myy comment is unclear? Oh, and your code can invoke undefined behaviour, depending on the shift count. In general shifting signed integers an arbitrary number of bits should be avoided. Use unsigned integers and fixed-width types!too honest for this site
And by the way, what I want here is understand why the code doesn't work. Let's talk about refinement later on, shall we? Eliminating non-working code won't tell me why it doesn't work anyway, so...user4113344
@Olaf: can you please give time instead of jumping on your high horse? In case you don't know: I'm busy, working, typing a question, answering comments and yet have other work to do in spite of addressing your... requirements. I will reword my question but don't give a damn if I'm not prompt enough for you — especially given the fact that someone already gave me the appropriate answer. And you're off: it's not all the code that I found on the internet. Only the part I wrote to rewrite it is in question!user4113344
Well, as I'm busy, too, I don't intend to do your job (which includes writing maintainable code). It is your project after all - unless you'd work for me, in that case I'd check your whole project first and - depneding on that analysis - we might have a talk (well, we had definitively for the _ names already).too honest for this site

1 Answers

3
votes

The decomposition of the arguments for set_bit into several takes place before the _SFR_BIT macro is expandend. So when it then sees the comma that resulted from the expansion of the latter, it is already too late.

A common trick is to expand the argument once more

#define set_bit0(...)   set_bit(__VA_ARGS__)
#define set_output(pin) set_bit0(_PIN(pin))

Here _PIN(pin) is expanded and passt to set_bit0. When passing the arguments from set_bit0 to set_bit it sees the already expandend sequence, including the comma.