Macro trouble for AVR128da code

Go To Last Post
14 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I am reading an input pin. Tried to make a macro like this

#define IRSensorBlocked PORTD.IN & PIN5_bm

The code works without a macro like this

in main ...
		if (IRSensorBlocked())
		{
			IRDeactivate;
			Activate_Emergency_Stop();
		}
		else
		{
			Release_Emergency_Stop();
		}
...

uint8_t IRSensorBlocked(void)
{
	if(PORTD.IN & IRSensor)
	{
         return True;
        };
	return False;
}

Can i make it a Macro? I get error expected ')' before token

 

Full Program Listing

#define F_CPU 1000000UL  // 1mhz

#include <avr/io.h>
#include <util/delay.h>
#include <avr/interrupt.h>

#define False 0
#define True  1

// Button input used for debugging code
#define BUTTON_PIN     PIN7_bm

// Emergency Stop Relay Output Pin LOW = STOP (turns off relay)
#define AirCylinderPin PIN6_bm
#define AirCylinderRelayON PORTD.OUTSET  = AirCylinderPin
#define AirCylinderRelayOFF PORTD_OUTCLR = AirCylinderPin

// IRLed and IR led Macros
#define IRPowerPin PIN0_bm
#define IRActivate PORTC.OUTSET = PIN0_bm
#define IRDeactivate PORTC.OUTCLR = PIN0_bm

// IRSensor Macros
#define IRSensor PIN5_bm
//#define IRSensorBlocked PORTD.IN & PIN5_bm

void CLK_init(void);
void PORT_init(void);
void Activate_Emergency_Stop(void);
void Release_Emergency_Stop(void);

uint8_t IRSensorBlocked(void);

uint8_t IRSensorStatus = 0;
uint8_t volatile button_event = False;

int main(void)
{
	cli();
	CLK_init();
	PORT_init();
	IRDeactivate;
	AirCylinderRelayOFF;
	AirCylinderRelayON;
	AirCylinderRelayOFF;
	sei();

	while (1)
	{
		IRActivate;
		IRActivate;
		_delay_ms(5); // Allow Rise time

		if (IRSensorBlocked())
		{
			IRDeactivate;
			Activate_Emergency_Stop();
		}
		else
		{
			Release_Emergency_Stop();
		}

		IRDeactivate;
		_delay_ms(50);
	}
}

ISR(PORTD_PORT_vect)
{
	button_event = True;
	IRDeactivate;
	PORTD.INTFLAGS = BUTTON_PIN;
}

void PORT_init(void)
{
	// INPUT PINS
	// Button Input
	PORTD.DIRCLR = BUTTON_PIN;
	// IRSensor Input Pin INPUT
	PORTD.DIRCLR = IRSensor;
	PORTD.PIN5CTRL = PORT_ISC_RISING_gc;

	// OUTPUT PINS
	// Emergency Stop Pin OUTPUT
	PORTD.DIRSET = AirCylinderPin;
	// IR led Pin Output
	PORTC.DIRSET = IRPowerPin;

	// INTERRUPTS
	// clear button interrupts
	PORTD.INTFLAGS = BUTTON_PIN;
	// Configure PC7 interrupt as rising
	PORTD.PIN7CTRL = PORT_ISC_FALLING_gc | PORT_PULLUPEN_bm;

}

void CLK_init(void)
{
	// Set clock source and speed to 1mhz
	_PROTECTED_WRITE (CLKCTRL.OSCHFCTRLA, CLKCTRL_FREQSEL_1M_gc);
}

uint8_t IRSensorBlocked(void)
{
	if(PORTD.IN & IRSensor)
	{
	  return True;
        };
	return False;
}

void Activate_Emergency_Stop()
{
	AirCylinderRelayOFF;

}

void Release_Emergency_Stop()
{
	AirCylinderRelayON;
}

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I found a bug while testing the input, when I run the program with the input pin grounded it runs fine

When i set the pin high it still runs but setting back to low it does something unexpected

The input is on Port C

That last line I deleted PORTD.PIN5CTRL = PORT_ISC_RISING_gc

It somehow made it so PORTC Pin0 became an input, debugging setting the pin high did not work but program ran. Hmmmm..

Not really sure that line is only for interrupts perhaps?

 

Edit: Portc Pin 0 stays as OUTPUT but the code to set the pin high no longer works

 

Port pin setup for sensor input

 

	// IRSensor Input Pin INPUT
	PORTD.DIRCLR = IRSensor;
	PORTD.PIN5CTRL = PORT_ISC_RISING_gc;
Last Edited: Mon. Feb 1, 2021 - 08:22 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
#define IRSensorBlocked PORTD.IN & PIN5_bm
		if (IRSensorBlocked())

 

Remember that macros are just text substitution.   Your macro is defined with no arguments , so the if statement expands to "if (PORTD.IN & PIN5_bm())" which doesn't make sense.

Try using:

#define IRSensorBlocked() (PORTD.IN & PIN5_bm)

(The extra parens around the "result" part aren't involved in your current problem, but they're a good idea!)

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

If the purpose of making a macro is to eliminate subroutine calls, you can make it an inline function.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

kabasan wrote:
you can make it an inline function.
+1

 

Doing these things as static inline also allows for parameter type checking - not relevant here but on the whole you should get into the habit of doing this kind of thing as static inline functions.

 

(it would also make MISRA happy!)

 

I'd go for something like:

__attribute__((always_inline)) static inline bool IRSensorBlocked(void)
{
    return (PORTD.IN & PIN5_bm) ? true : false;
}

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Can’t believe I’m up at 7 after going to sleep at 3am. That in-line code is very cool. Where do you learn stuff like that, is there a guru encyclopedia of macros I should add to my bookshelf? There are so many things I don’t even know exist. When I programmed the Commodore 64 6502 all I needed was one book “programming the 6502”

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

And a book with IO and memory layout etc. of the C64 ;) 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

metron9 wrote:
Where do you learn stuff like that, is there a guru encyclopedia of macros I should add to my bookshelf?

Not sure which bit you mean:

static inline

is just standard C. You should have already learned about that when you first learned C. "inline" is simply a hint to the compiler to say "it could be more efficient to put the code of this at the point of call rather than holding it separately and CALLing it". The static means "this is very localised - it won't be used anywhere else so you don't actually need to create a copy of this to be CALLd if you are only going to inline it"

__attribute__((always_inline))

This is very specifically a GCC thing that only applies to the GCC compiler. It has lot of Function Attributes and Variable Attributes . As I say the core C keyword "inline" is simply a hint saying "this might be a good idea" but this attribute is a firm instruction to say "it's not just a suggestion, you must inline this code"

return (PORTD.IN & PIN5_bm) ? true : false;

If you didn't recognise this it's simply known as the "tenary" operator in C. You might often use it in something like:

var = (count < 64) ? count | 64;

That says  "test count and see if it is less than 64, if it is then use the value in count to assign a value to "var" but if it was >=64 then set it to 64. In effect (used like this) it is limiting count to something tha tis <= 64. If count is 96 for example it's out of range and 64 will be assigned.

 

In my use I'm just using it as a short form of:

bool retval;
if (PORTD.IN & PIN5_bm) {
    retval = true;
}
else {
    retval = false;
}
return retval;

but I put it all into the return statement. In:

__attribute__((always_inline)) static inline bool IRSensorBlocked(void)
{
    return (PORTD.IN & PIN5_bm) ? true : false;
}

it's just saying read PORTD.IN, test bit 5, if set return true, else return false. Then the stuff on top just says, insert this read/test stuff inline, don't CALL it and don't create a separate copy.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

metron9 wrote:
Where do you learn stuff like that

inline functions have been a standard part of 'C' for many years - so should be covered in any standard 'C' reference.

 

https://en.wikipedia.org/wiki/Inline_function#Standard_support

 

compiler-specific details like the "attribute" stuff would be in the compiler documentation; in this case:

 

https://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html

 

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

As the return type is bool, the return statement is overly complicated.

Even a cast to bool would be better than the ternary operator.

 

The ternary operator is such a joy to read,

that about the only time one should use it

is when one needs a single expression,

e.g. when initializing a const.

Moderation in all things. -- ancient proverb

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

For a static function (means it can only be seen by the c file it is in, approximately speaking), you don't really need the inline.

Depending on optimisation settings, the compiler is quite likely to inline it anyway.

So you can just do (simplest way ususally is to define it above where it is called, then you don't need a separate declaration)

 

static uint8_t IRSensorBlocked(void)
{
    return PORTD.IN & PIN5_bm ? 1 : 0;
}

int main(void)
{
}

If you are using bool types, then include <stdbool.h>

This also defines true and false for you (note all lower case)

In this case, unlike if using uint8_t, you don't actually need the ternary returning true or false,

(a bool by definition can only ever hold 0 or 1, any non zero is converted to 1)

 so you can write it as

#include <stdbool.h>

static bool IRSensorBlocked(void)
{
    return PORTD.IN & PIN5_bm;
}

int main(void)
{
}

but using return PORTD.IN & PIN5_bm ? true : false;

will do exactly the same, it will be the same code produced either way, so take your pick.

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

skeeve wrote:
As the return type is bool, the return statement is overly complicated.

+1

 

return (PORTD.IN & PIN5_bm) ? true : false;

The expression (PORTD.IN & PIN5_bm) is evaluated as a boolean expression - so no need to force it to one of the bool constants.

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

awneil wrote:
The expression (PORTD.IN & PIN5_bm) is evaluated as a boolean expression
How can an expression that generates 0x00 or 0x20 be considered as being evaluated as a bool? As other have said you can use the fact that false=0 / true=!false so that the 0x20 is later interpreted as boolean but this only happens when you are returning this as "bool" and so there is an implied casting conversion at the moment of return - but not at the point that the parenthesized expression is evaluated.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If it was && (logical AND) then it would be a 'boolean' value, can only be 0 or 1 (but note, in C, this 0 or 1 result is actually of type int).

With & (bitwise AND) it is not intrinsically 'boolean'.

If function is returning bool, however, then the conversion from the (non boolean) result of the 'bitwise and' to 'bool' happens implicitly, because you have defined the retun type as bool.

That conversion is exactly as if you wrote

return !!(PORTD.IN & PIN5_bm);

or

return (PORTD.IN & PIN5_bm) != 0;

or

return PORTD.PIN & PIN5_bm ? true : false;

just that the compiler will do it for you.

If you do write it explcitly, I expect it makes absolutely no difference to the generated code.