bas knippels
bas knippels

Reputation: 53

Can I turn this requirement into a macro?

I have C programs with decrementing software counters. If for instance I want to blink an led every 2 seconds I can do:

if(!ledT) { 
    ledT = 200;
    // code
    // code
    // code 
}

Because I always do the exact same combination with every counter, I tend to type it one line.

if(!ledT) { ledT = 200;
    // code 
    // code 
    // code  
}

For the entire if-line I'd like to use a macro instead. So the code would look something like:

expired(ledT, 200) {
    // code 
    // code 
    // code 
}

I use something similar in my state machine code for the entry state.

if(runOnce) { runOnce = false;
    // code 
    // code 
    // code 

Desired syntax:

entryState {
    // code 
    // code 
    // code 

.

#define entryState if(runOnce) { runOnce = false; // this ofcourse cannot work But something like this is what I want.

I've made several attempts but I got nowhere. The problem is that the { is somewhere in the middle of the macro and I want to type a { behind the macro because as we all know, no code editor can live with an unequal number of { and }.

expired(ledT, 200); // expired is macro, not function
    // code
    // code
    // code
}

So this is out of the question.

Whilst reading about macros, I've read something interesting about using: do ... while(0). This 'trick' abuses the compiler's optimization feature to create a certain macro, which would otherwise be impossible.

This site

sheds some light about this manner.

Is there a way to use some kind of 'macro trick' to achieve what I want?

So again, that is transforming:

// this
if(runOnce) {
    runOnce = false;
    // code
    // code
    // code

// into this
entryState {
    // code
    // code
    // code

// and this:
if(!someTimer) {
    someTimer = someInterval;
    // code
    // code
    // code

// must be transformed into:
timeExpired(someTimer, someInterval) {
    // code
    // code
    // code

And an answer like "No, it simply cannot be done" will also be accepted (providing you know what you are talking about)

EDIT: I need to add an addition because not everybody seems to know what I want, the last given answer is not even aimed at the specific problem at hand. Somehow toggling IO suddenly became important? Therfor I altered my code examples to better illustrate what the problem is.

EDIT2: I agree that the the timeExpired macro does not improve readability at all

To show that some macros can improve readabilty I'll give a snippet of a state and a state machine. This is how a generated state looks like in my code:

State(stateName) {
    entryState {
        // one time only stuff
    }
    onState {
        // continous stuff
        exitFlag = true; // setting this, exits the state
    }
    exitState {
        // one time only stuff upon exit
        return true;
    }
}

Currently in place with these macros:

#define State(x) static bool x##F(void)
#define entryState if(runOnce) 
#define onState runOnce = false;
#define exitState if(!exitFlag) return false; else

I think I should I should exchange return true; in the states by EXIT or something prittier. And the state machine which calls these States looks like:

#undef State

#define State(x) break; case x: if(x##F())
extern bit weatherStates(void) {
    if(enabled) switch(state){
        default: case weatherStatesIDLE: return true;

        State(morning) {
            if(random(0,1)) nextState(afternoon, 0);
            else            nextState(rain, 0); }

        State(afternoon) {
            nextState(evening, 0); }

        State(evening) {
            if(random(0,1)) nextState(night, 0);
            else            nextState(thunder, 0); }

        State(night) {
            nextState(morning, 0); }

        State(rain) {
            nextState(evening, 0); }

        State(thunder) {
            nextState(morning, 0); }

        break; }
    else if(!weatherStatesT) enabled = true; 
    return false; }
#undef State

The only thing which is not generated are the 'if' and 'else' before the 'nextState()' functions. These 'flow conditions' need filling in.

If a user is provided with a small example or an explanation, he should have no difficulty at all with filling in the states. He should also be able to add states manually.

I'd even like to exchange this by macros:

extern bit weatherStates(void) {
        if(enabled) switch(state){
            default: case weatherStatesIDLE: return true;

and

break;} }
    else if(!weatherStatesT) enabled = true;
    return false;}

Why would I do this? To hide irrelevant information out of your display. To remove alot of tabs in the state machine. To increase overal readability by using a simple syntax. Like with 3rd library functions you need to know how to use the code rather to know how the function does the trick.

You don't need to know, how a state signals that it is ready. It is more important to know that the function in question is used as a state function than to know that it returns a bit variable.

Also I test macros before using. So I don't provide somebody with state machines that may show strange behavior.

Upvotes: 0

Views: 138

Answers (4)

Swedgin
Swedgin

Reputation: 893

DISCLAIMER: I don't recommend using this solution.

I had a go at trying to make this into macro's. It is indeed possible but if it's faster, that's another question. As you make a new variable each time you call the macro.

#include <stdio.h>

#define entryState(runOnce) int temp_state = runOnce; if (runOnce) runOnce = 0; if (temp_state)
#define timeExpired(someTimer, someInterval) int temp_expired = someTimer; if (!someTimer) someTimer = someInterval; if (!temp_expired)

int main(int argc, const char* argv[]) {

    int runOnce = 1;
    int someTimer = 0;
    int someInterval = 200;
    timeExpired(someTimer, someInterval) {
        printf("someTimer is Expired\n");
    }

    printf("someTimer: %i\n\n", someTimer);

    entryState(runOnce) {
        printf("this is running once\n");
    }

    printf("runOnce: %i\n", runOnce);

}

Compiling and running:

c:/repo $ gcc test.c -o test
c:/repo $ ./test.exe 
someTimer is Expired
someTimer: 200

this is running once
runOnce: 0

I don't have a C51 compiler at hand now, so I let the testing on the 8051 over to you.

Upvotes: 0

Lundin
Lundin

Reputation: 213721

Given that this is for some old 8051 legacy project, it is extremely unlikely that you need to create abstraction layer macros for pin I/O handling. You'll only have just so many pins. Your original code is most likely the best and clearest one.

If you for some reason worry about code repetition, because you have multiple combinations of the product/support multiple PCB with different routing etc, and you are stuck with your current code base... then as a last resort you could use macros to avoid code repetition. This also assuming that you are a seasoned C programmer - otherwise stop reading here.

What you will be looking at in that rare scenario is probably something that's known as "X macros", which is about declaring a whole list of pre-processor constants. Then whenever you need to do something repetitive, you call upon that list and use the constants inside it that you are interested for that specific call. Each call is done by specifying what the macro "X" should do in that particular call, then undefined the macro afterwards.

For example if you have ports A, B, C, you have LEDs on port A:0, B:1 and C:2 respectively and wish to use different delays per pin, you can declare a list like this:

#define LED_LIST           \
/*  port   pin   delay */  \
  X(A,     0,    100)      \
  X(B,     1,    200)      \
  X(C,     2,    300)      \

Then you can call upon this list when you need to do repetitive tasks. For example if these ports have data direction registers you need to set accordingly and those registers are called DDRA, DDRB, DDRC (using Motorola/AVR naming as example):

/* set data direction registers */
#define X(port, pin, delay) DDR##port |= 1u<<pin;
  LED_LIST
#undef X

This will expand to:

DDRA |= 1u<<0;
DDRB |= 1u<<1;
DDRC |= 1u<<2;

Similarly, you can initialize the counters as:

/* declare counters */
#define X(port, delay) static uint16_t count##port = delay;
  LED_LIST
#undef X


...

/* check if counters elapsed */
#define X(port, delay) if(count##port == 0) { count##port = delay; PORT##port ^= 1u << pin; }
  LED_LIST
#undef X

(I replaced the toggle macro with a simple bitwise XOR)

Which will expand to:

static uint16_t countA = 100;
static uint16_t countB = 200;
static uint16_t countC = 300;

...

if(countA == 0) 
{ 
  countA = 100; 
  PORTA ^= 1u << 0; 
}
if(countB == 0) 
{ 
  countB = 200; 
  PORTB ^= 1u << 1; 
}
if(countC == 0) 
{ 
  countC = 300; 
  PORTC ^= 1u << 2; 
}

And of course avoid using 16 bit counters like done here unless you must, since you are working with a crappy 8-bitter.

Upvotes: 4

0___________
0___________

Reputation: 67476

#define LL(ledT) do {if(!ledT) { ledT = 200; TOG(ledPin); }}while(0)

Whilst reading about macros, I've read something interesting about using: do ... while(0). This 'trick' abuses the compiler's optimization feature to create a certain macro, which would otherwise be impossible.

Most of the opinions there are actually wrong. There is nothing about optimizations.

The main reason is to make macros using curled braces to compile at all.

This one will not compile

#define A(x) {foo(x);bar(x);}

void foo1(int x)
{
    if (x) A(1);
       else B(0);
}

but this one will compile

#define A(x) do{foo(x);bar(x);}while(0)

void foo1(int x)
{
    if (x) A(1);
       else B(0);
}

https://godbolt.org/z/4jH2jP

Upvotes: 1

Konrad Rudolph
Konrad Rudolph

Reputation: 545588

There’s no need to employ macros here, and doing so leads to highly un-idiomatic C code that doesn’t really have any advantages over proper C code.

Use a function instead:

int toggle_if_unset(int time, int pin, int interval) {
    if (time == 0) {
        time = 200;
        TOG(pin);
    }
    return time;
}
ledT = toggle_if_unset(ledT, ledPin, 200);

(I’m guessing appropriate parameter names based on your example; adjust as appropriate.)

What’s more, it looks as if ledT and ledPin are always paired and belong together, in which case you should consider putting them into a struct:

struct led {
    pin_t pin;
    int interval;
};

void toggle_if_unset(struct led *led, int new_interval);

Or something along these lines.

Upvotes: 5

Related Questions