use pins like an array??

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

hi everybody

 

I'm going to use 12 xmega pins to control 12 valves on a known time table(for example:fixed timeout).

 

I want to assign a name for each pin and use the names in a loop to turn the valves on and off  one by one.

but I'm not sure about the right way to implement this.

 

first of all i took a look at xmega header file for addresses and extracted what is written below.
 

 

#define valve1             _SFR_MEM8(0x0605) <<0

#define valve2             _SFR_MEM8(0x0605) <<2

.

.

#define valve8             _SFR_MEM8(0x0605) <<7

 

#define valve9             _SFR_MEM8(0x0624) <<0

#define valve10             _SFR_MEM8(0x0624) <<1

#define valve11             _SFR_MEM8(0x0624) <<2

#define valve12             _SFR_MEM8(0x0624) <<3

 

x=12

while(x)

    {

             valve[x] = true;                                   //     turn out register of selected pin high

             _delay_s(time_out);                           //     wait timer

            valve[x] = fulse;                                  //     turn out register of selected pin Low

             x--;

}

 

 

any suggestions?

how could it be practical?

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

I'd just write a single accessor function that takes pin number (0..11) and pin state (0/1) then put all the complexity of mapping 0..11 to the right ports/pins within that.

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

dear clawson

thanks for your super fast answer

I'm not sure how to do that, so please let me try the code here and have your advise if possible?

thank you

;-)

 

#define on 1

#define off 0

void valve_act(unsigned char pin , bool stat)

 {

              //    I'm not sure how to assign pins to valves because every port has 8 members and i have 12 valves

             //  maybe(struct method?):

         

           if (pin<7){

           porta-> pin= stat; 

           }

         

          else if (pin>7)

          {

            portb->(pin-7)=stat;

           }

          else

          { return 0;}     

       

};

 

main{

x=12

while(x){

     valve_act(x, on);

     _delay_s(timeout);    

     valve_act(x,off);

     x--;       

}

}

 

does these make sense?

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

19:37 I think I did something like that some time ago... hold on.

19:42 Found it.

 

It is from a project where an AVR with a RS485 interface controlls a bunch of relays.

I wanted a general interface to send commands over RS485 to turn the relays on / of.

 

It starts with defining the the locations of all I/O pins.

I do this always in the same manner for all I/O pins in each project.

Those definitions are always in the file "main.h", and they look like:

//===========================================================================
// User definitions for: main.h
//===========================================================================
#if (PROJECT_NUMBER == PROJECT_SWITCH_MULTI)	|| \
	(PROJECT_NUMBER == PROJECT_SWITCH_DESK_LIGHT)
#define PORT_REL_A		PORTB
#define DDR_REL_A		DDRB
#define PIN_REL_A		PINB
#define BIT_REL_A		(1<<0)

#define PORT_REL_B		PORTB
#define DDR_REL_B		DDRB
#define PIN_REL_B		PINB
#define BIT_REL_B		(1<<1)

#define PORT_REL_C		PORTB
#define DDR_REL_C		DDRB
#define PIN_REL_C		PINB
#define BIT_REL_C		(1<<2)

#define PORT_REL_D		PORTB
#define DDR_REL_D		DDRB
#define PIN_REL_D		PINB
#define BIT_REL_D		(1<<3)

...

#else
# ...
#endif

Note: with the "PROJECT_NUMBER" macro I can use different I/O pin locations for different projects.

Note: Bit numbers are not shifted in the default AVR libraries, but I like to shift those numbers right in main.h, to make the rest of the code look cleaner.

 

In the command interpreter I use single lower case letters to identify the relays: a...h.

I have made an enum to give them all a name:

//==================================================================== Enums.
enum eRelay{
	RELAY_NR_A		= 'a',
	RELAY_NR_B,
	RELAY_NR_C,
	RELAY_NR_D,
	RELAY_NR_E,
	RELAY_NR_F,
	RELAY_NR_G,
	RELAY_NR_H,
	RELAY_STATE_ON	= '+',
	RELAY_STATE_OFF	= '-',
	RESPONSE_TIMED	= 'T',
	RESPONSE_MUL	= '*',
	RESPONSE_TIMED_LOAD = 'E',
};

The other letters are commands. '+' to turn a relay on, '-' to turn a relay off, etc.

Multiple commands can be combined. A string like "+a+c-f" turns the relays a and c on, and turns relay f off.

 

With preparation of the above #defined macro's and the enum the missing link is between the ascii commands and the actual (re) setting of the relays.

This is implemented in a simple switch, with some extra sanity checks before it.

 

//===========================================================================
// Turn one of the output relays on or off.
// retval: true = Command is executed.
// retval: false = Some kind of error occured.
// Not static because it is also used for delayed functions.
bool RelayOutput( uint8_t OnOff, uint8_t Relay, uint8_t Parse) {

	Relay |= 0x20;		// Turn to lower case Bug: Ugly 2015-10-17.
	OnOff |= 0x20;
	if (Relay < RELAY_NR_A)
		return false;
	if (Relay > RELAY_NR_H)
		return false;
	if (!Parse)
		return false;

	if( OnOff == CMD_TOGGLE) {
		if( RelayState( Relay) == RELAY_STATE_ON) {
			OnOff = RELAY_STATE_OFF;
		}
		else {
			OnOff = RELAY_STATE_ON;
		}
	}

	if( OnOff == CMD_ON) {
		switch( Relay) {
		case RELAY_NR_A:	PORT_REL_A |= BIT_REL_A;	break;
		case RELAY_NR_B:	PORT_REL_B |= BIT_REL_B;	break;
		case RELAY_NR_C:	PORT_REL_C |= BIT_REL_C;	break;
		case RELAY_NR_D:	PORT_REL_D |= BIT_REL_D;	break;
		case RELAY_NR_E:	PORT_REL_E |= BIT_REL_E;	break;
		case RELAY_NR_F:	PORT_REL_F |= BIT_REL_F;	break;
		case RELAY_NR_G:	PORT_REL_G |= BIT_REL_G;	break;
		case RELAY_NR_H:	PORT_REL_H |= BIT_REL_H;	break;
		}
	} else if( OnOff == CMD_OFF) {
		switch( Relay) {
		case RELAY_NR_A:	PORT_REL_A &= ~BIT_REL_A;	break;
		case RELAY_NR_B:	PORT_REL_B &= ~BIT_REL_B;	break;
		case RELAY_NR_C:	PORT_REL_C &= ~BIT_REL_C;	break;
		case RELAY_NR_D:	PORT_REL_D &= ~BIT_REL_D;	break;
		case RELAY_NR_E:	PORT_REL_E &= ~BIT_REL_E;	break;
		case RELAY_NR_F:	PORT_REL_F &= ~BIT_REL_F;	break;
		case RELAY_NR_G:	PORT_REL_G &= ~BIT_REL_G;	break;
		case RELAY_NR_H:	PORT_REL_H &= ~BIT_REL_H;	break;
		}
	}
	return true;
}

 

Another way would be to use the adresses of the I/O register names and work with pointers, but then you will also need a lookup table somewhere.

Instead of using a switch() you could also put the names in an array and use that.

 

Alternatively you could look at the "arduino" library code. They use pin numbers to identify I/O pins, but they do a lot of extra checking in an attempt to make those libraries fool proof.

For example, if you set an output high that can also be used for PWM, then the library will turn PWM off for that pin.

And it does that every time you set/reset an I/O pin.

This results in a tremendous amount of overhead. Acceptible if you just want to turn a relay on or off, but if you do software SPI with the "arduino" functions you are lucky to get a few kbps out of it.

Doing magic with a USD 7 Logic Analyser: https://www.avrfreaks.net/comment/2421756#comment-2421756

Bunch of old projects with AVR's: http://www.hoevendesign.com

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

Wow !!!

Amazing !

i appreciate your help and thankful for time you put on this answer.quite handsome.

 

but maybe not suitable to my project because i use 4 timers and RTC plus 2 UART's for communication and debug ,I2C for time backup,

12 port pin's to control and EEPROM for data backup and so on.

 

so i need an optimized code with lowest time for control valves while reliable, and I'm looking for a method to address pins directly

because i don't like to loose data in communication while turning valves on/of.

I have two Interrupts with high priority, two with medium priority and one with low.

and all of this should done with 8MHZ sys clock.

I didn't measure the difference between methods yet, but I'm curios to find most optimized method regardless of time.

I ask for your help if you know how to behave pins like an array(or assign them to an array)  :-)

best regards

ad.h

 

 

P.S :

I forgot to mention that i used the valves in a special order to turn them on and off in sequence to decrease the time and code.

Last Edited: Mon. Oct 1, 2018 - 11:50 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

What do you think is sub-optimal about Paul's code then? It's pretty much exactly what I had in mind for post #2. Something like:

void digitalWrite(int pin_num, int pin_state) {
    switch (pin_state) {
        case 0:
        switch(pin_num) {
            case 0: PORTB &= ~(1 << 3); break;
            case 1: PORTA &= ~(1 << 2); break;
            case 2: PORTD &= ~(1 << 7); break;
            case 3: PORTC &= ~(1 << 5); break;
            etc.
        }
        break;
        case 1:
        switch(pin_num) {
            case 0: PORTB |= (1 << 3); break;
            case 1: PORTA |= (1 << 2); break;
            case 2: PORTD |= (1 << 7); break;
            case 3: PORTC |= (1 << 5); break;
            etc.
        }
        
    }
}

Is your concern that if it is the 12th pin (pin_number == 11) that the switch() may have to process 11 prior not= tests before it matches to "case 11" ? I think you'll find that some implementation of switch/case optimize for this kind of thing (and especially when the cases are consecutive).

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

I suppose another approach might be:

typedef struct {
    volatile uint8_t * port;
    uint8_t pin;
} port_pin_t;

port_pin_t pinarray[] = {
    { &PORTB, (1 << 3) },
    { &PORTD, (1 << 2) },
    { &PORTA, (1 << 5) },
    { &PORTC, (1 << 7) },
    etc.
}

static inline set_pin(int n) {
    pinarray[n].port |= pinarray[n].pin;
}

static inline clr_pin(int n) {
    pinarray[n].port &= ~pinarray[n].pin;
}

then later:

set_pin(5);
clr_pin(11);

In all cases there is going to be some overhead for trying to give the port/pin groups a virtual ID (0.. 11). Far better is just to say:

PORTB &= ~(1 << 3);

exactly where you need that bit in that port clear. If you want to "dress it up":

#define WIDGET_VALVE_PORT PORTB
#define WIDGET_VALVE_PIN (1 << 3)

WIDGET_VALVE_PORT &= ~WIDGET_VALVE_PIN;

it's true that this does not lend itself to:

for (int i = 0; i < 12; i++) {
    clr_valve(i);
}

which is what making an "array" out of the combinations - but exactly how important and required is that anyway?

Last Edited: Mon. Oct 1, 2018 - 12:04 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Dear paul & clawson

both ways were excellent!

based on what i know (if true) case statement uses one clock(maybe more) to compare the case with every existing option there is,so with my 12 cases (think about 12 repetition) it

will waste about 124 sys clocks to do the last valve.

 

i was trying to write somthing like this:

 

x=10;

while(x){

if (x>7){

portb.out |= 1<<(x-7);

}

else{

porta.out |= 1<<(x);

}

x--;

if(!x) x=10;

}

 

what's your idea about ?

 

thanks for sharing your experiences.

 

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

amirdelta wrote:
uses one clock(maybe more)
Most definitely more - the AVR is a RISC processor so each opcode can only achieve one small step at a time.
amirdelta wrote:
what's your idea about ?
Not enough detail - I've no idea what you are trying to achieve but it does seem pretty clear that 8 steps of that are setting all of bits 0..7 in PORTA. If speed is of the essence why would you encode the pointless loop? Why wouldn't you simply say:

PORTB = 0xFF;

That is probably one LDI and one STS so about 3..4 cycles to set all 8 bits at once.

 

Seems you are focusing too much on making this 0..11 pin numbering thing that you have lost sight of what it is you are actually trying to do! if that entire loop is about setting 8 bits in one port and 2..3 in another then two statements:

PORTA = 0xFF;
PORTB = 0x07;

(or whatever) achieve this without a lot of pointless pin mapping.

 

Is this all because one day the thing on PORTB bit 3 might move to PORTC bit 5 or something? Does hardware really get up and shuffle itself about during the night then?

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

amirdelta wrote:
i need an optimized code with lowest time for control valves while reliable, and I'm looking for a method to address pins directly because i don't like to loose data in communication while turning valves on/of

 

Err You're on Xmega according to your 1st post. So execution time should really be a consideration at all. Plus you get nice atomic access features like OUTSET, OUTCLR & the less usefull OUTTGL.

 

So you can to an extent treat ports like an array assuming you have actually wired them as an array.

 

PORTB.OUTSET = (1 << relayNo);  // set a pin using std port
PORTB.OUTCLR = (1 << relayNo);  // reset a pin using std port

 

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

In your post #8 you are shifting bits in software, which is a relatively expensive operation for an AVR.

 

The execution speed of a switch is pretty much a guess without trying it out. The compiler is free to do a whole lot of optimisations here.

Sometimes a simple lookup table may be used.

If numbers are consecutive, then an offset calculation with a jump table may be used.

Sometimes a binary tree search may be implemented in the background.

The array method in clawson's #7 is probably the fastest method.

Once the array index " n " is known, the AVR only has to fetch some values form memory and do the |= or &= to set a bit of an IO register.

Normally an " |= " or an " &= ~ " is translated to a single sbi or cbi instruction, but the compiler probably can not do that here because of the loading of values from an array.

 

The only way to do it in C which might be faster is to make functions for each bit set or clear operation, make an array of these functions and use a function pointer to execute the right function from the array.

 

But it is all too much work to bother about.

You need to sit back, scratch yourself behind the ear, and think about what kind of speed you need. The switch() method probably takes a few micro seconds. Is that good enough for you? How much performance do you need? 1us, 10us, Is 100us acceptable? The Xmega's are also quite a bit faster than the AVR's i'm used to.

The software is very likely already faster than the time to turn on a power transistor, the time needed for a magnetic field in the valves to build up, and the time needed for mechanical things to move in your valves / relays, whatever. Those combined can very easily be several milli seconds.

 

I also thing you have made the beginner mistake of assuming that the amount of text in a program is correlated to the execution speed of a compiled program.

There is no straightforward correlation. And on top of that the compiler has a big bag full of tricks to optimize code and those tricks may or may not kick in depending on somtimes obfuscated details.

If you want to go down to that level, you should study the .LSS output of the compiler. GCC uses assembly as an intermediate step to create a binary and this listing file has the assembly output of the compiler combined with the C source code as comment. So you can see from each line of C source code what machine code is acutally generated.

Hint:

The shift in your example #8 has a variable as rvalue, which means the shift has to be done at execution time.

The shift in other examples has a constant  as rvalue, which means the shift is calculated by te preprocessor and loaded as a constant in the final binary.

 

https://www.xkcd.com/1691/

Doing magic with a USD 7 Logic Analyser: https://www.avrfreaks.net/comment/2421756#comment-2421756

Bunch of old projects with AVR's: http://www.hoevendesign.com

Last Edited: Mon. Oct 1, 2018 - 04:54 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Paulvdh wrote:
The array method in clawson's #7 is probably the fastest method.
And you'll notice I deliberately pre-calculated the bit shifts at array initialization time so that they'd be done by the compiler, not at run time. Far better to use whole masks, not bit numbers!

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

+1 on premature optimization.

 

More important than any of this is to make sure you and anyone else

who comes back to the code later can understand what it's doing.

 

I'd much prefer to find something like:

 

open_valve (HYDRAULIC_LIFT_RAISE_VALVE);
...
close_valve (PNEUMATIC_TOOL_AIR_SUPPLY_VALVE);

 

instead of a bunch of shifting, |= and &=.

 

--Mike

 

EDIT: Of course like most everybody else here I enjoy low level bit

flipping and optimizing even when unnecessary just for the fun of it!

 

Last Edited: Mon. Oct 1, 2018 - 08:12 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If you use my code in #7 (I thought that used to be #8?) then it could just as easily be

set(HYDRAULIC_LIFT_RAISE_VALVE);
clr(PNEUMATIC_VALVE)
etc.

 

(Probably use an enum)

 

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

My comment was toward the OP.  Apologies if it sounded harsh, was not

intended to.  Sometimes I get a little... "excited" about interface issues.

 

--Mike

 

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

dear friends

I REALLY appreciate your helps and efforts.

all of them was helpful.

I've just wanted to reply them in details but changed my mind and decided to advance and explain more with a conclusion.

 

as i said before there is 12 valves that should be turned on and off on consecutive order (because of design pin mapping) with

a given timeout and this process needs to be done four times.and on eventual failure(for example power failure) continue at the

last valve.

so i decided to write a function to call during normal operation and after failure and do the coding inside.

some detail:

after every valve completion i want to write a byte to EEPROM to save the last successful valve.and on restart load the last state and continue.

 

Now based on you friends said upper:

I think the best idea is to ...

Wait!!

I forgot to say that,my first idea was to override the header file definitions and do my own def's for two ports as arrays and

act the pins as array members.

 

and write something like what clawson said at the end of  post #7 :

 

 

for (int i = 0; i < 12; i++) {
    clr_valve(i);
}

I even didn't use for loop because i think it uses more clock to run in compare with descending while loop :-)

devillaughsmileysmiley

 

 

 

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

seems i have a lot to learn about bit manipulation because i thought they are very affordable when considering sys clock.

 

beetween us --unfortunately I'm a newbie in planet of C , Xmega and AS7.and my basic studies are not in electronic while (do not read this section-- do serius projects for hobby or self enjoymentangel)

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

This is a XMEGA so set or clr don't need |=  or  &=~    that is (can also be) handled in a different addr. with same "mask"

 

add:

so make a global array of pointer's and mask (perhaps two pointer's each a set or clear addr. , but the easy way is to use an offset).

 

 

Last Edited: Tue. Oct 2, 2018 - 06:35 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

may i ask you to write your propose

i think every idea will help me to decide.

thank you.

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

What they are saying about OUTSET/OUTCLR is that it is a "better" way (atomic) to set or reset bits. So my functions above:

static inline set_pin(int n) {
    pinarray[n].port |= pinarray[n].pin;
}

static inline clr_pin(int n) {
    pinarray[n].port &= ~pinarray[n].pin;
}

would effectively be:

static inline set_pin(int n) {
    pinarray[n].port->OUTSET = pinarray[n].pin;
}

static inline clr_pin(int n) {
    pinarray[n].port->OUTCLR = pinarray[n].pin;
}

In tiny/mega you just had individual registers like:

DDRB
PORTB
PINB

but in Xmega it is more like:

typedef struct PORT_struct
{
    register8_t DIR;  /* I/O Port Data Direction */
    register8_t DIRSET;  /* I/O Port Data Direction Set */
    register8_t DIRCLR;  /* I/O Port Data Direction Clear */
    register8_t DIRTGL;  /* I/O Port Data Direction Toggle */
    register8_t OUT;  /* I/O Port Output */
    register8_t OUTSET;  /* I/O Port Output Set */
    register8_t OUTCLR;  /* I/O Port Output Clear */
    register8_t OUTTGL;  /* I/O Port Output Toggle */
etc.
} port_t;

#define PORTB                (*(PORT_t *) 0x0620) /* I/O Ports */

so "PORTB" is not just a register but a pointer to a block of registers (grouped as a struct) and "OUTSET" and "OUTCLR" are registers withing that. So now when you use:

typedef struct {
    volatile port_t * port;
    uint8_t pin;
} port_pin_t;

port_pin_t pinarray[] = {
    { &PORTB, (1 << 3) },
    { &PORTD, (1 << 2) },
    { &PORTA, (1 << 5) },
    { &PORTC, (1 << 7) },
    etc.
}

then the first member of each array entry here is a pointer to port_t struct. That's why in:

static inline set_pin(int n) {
    pinarray[n].port->OUTSET = pinarray[n].pin;
}

I use the '->' operator.

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

OK, so I had to try this for real...

/*
 * test1.c
 *
 * Created: 24/09/2018 13:23:01
 * Author : uid23021
 */ 
#include <avr/io.h>

typedef enum {
	HYDRAULIC = 0,
	PNEUMATIC,
	WIDGET,
	SPLINE_SHAFT
} valve_e;

typedef struct {
	volatile PORT_t * port;
	uint8_t pin;
} port_pin_t;

port_pin_t pinarray[] = {
	{ &PORTB, (1 << 3) },
	{ &PORTD, (1 << 2) },
	{ &PORTA, (1 << 5) },
	{ &PORTC, (1 << 7) }
};

static inline void set(int n) {
	pinarray[n].port->OUTSET = pinarray[n].pin;
}

static inline void clr(int n) {
	pinarray[n].port->OUTCLR = pinarray[n].pin;
}

int main(void)
{
	set(HYDRAULIC);
	clr(PNEUMATIC);
	clr(WIDGET);
	set(SPLINE_SHAFT);
}

The first invocation "set(HYDRAULIC)" yields:

	pinarray[n].port->OUTSET = pinarray[n].pin;
 23c:	e0 e0       	ldi	r30, 0x00	; 0
 23e:	f0 e2       	ldi	r31, 0x20	; 32
 240:	a0 81       	ld	r26, Z
 242:	b1 81       	ldd	r27, Z+1	; 0x01
 244:	82 81       	ldd	r24, Z+2	; 0x02
 246:	15 96       	adiw	r26, 0x05	; 5
 248:	8c 93       	st	X, r24

The array is at 0x2000 in RAM. That loads the pin number into R24 and the pointer to the PORT_t into R27:R26 (aka X). It then adds an offset of 5 because OUTSET is the 6th element in the port structure. Then it writes the pin mask to the register. The next code "clr(PNEUMATIC)" yields:

    pinarray[n].port->OUTCLR = pinarray[n].pin;
 24a:	a3 81       	ldd	r26, Z+3	; 0x03
 24c:	b4 81       	ldd	r27, Z+4	; 0x04
 24e:	85 81       	ldd	r24, Z+5	; 0x05
 250:	16 96       	adiw	r26, 0x06	; 6
 252:	8c 93       	st	X, r24

this is relying on the fact that Z is already set so now it loads the pinarray[1] details with pin mask in R24 and the PORT_t address in X. This time the OUTCLR is at offset 6 in the port structure.

 

Just to compare, this code:

	PORTB.OUTSET = (1 << 3);
	PORTD.OUTCLR = (1 << 2);
	PORTA.OUTCLR = (1 << 5);
	PORTC.OUTSET = (1 << 7);

which is the most direct way to set and clear the bits yields:

	PORTB.OUTSET = (1 << 3);
 268:	88 e0       	ldi	r24, 0x08	; 8
 26a:	80 93 25 06 	sts	0x0625, r24	; 0x800625 <__TEXT_REGION_LENGTH__+0x700625>
	PORTD.OUTCLR = (1 << 2);
 26e:	84 e0       	ldi	r24, 0x04	; 4
 270:	80 93 66 06 	sts	0x0666, r24	; 0x800666 <__TEXT_REGION_LENGTH__+0x700666>
	PORTA.OUTCLR = (1 << 5);
 274:	80 e2       	ldi	r24, 0x20	; 32
 276:	80 93 06 06 	sts	0x0606, r24	; 0x800606 <__TEXT_REGION_LENGTH__+0x700606>
	PORTC.OUTSET = (1 << 7);
 27a:	80 e8       	ldi	r24, 0x80	; 128
 27c:	80 93 45 06 	sts	0x0645, r24	; 0x800645 <__TEXT_REGION_LENGTH__+0x700645>

So there is a "cost" for doing this indirectly using the array access thing simply so you can enumerate the pins. Of course what this latter code does not cater for is:

#define NUM_ELEMENTS(x) sizeof(x)/sizeof(x[0])
	for(int i = 0; i < NUM_ELEMENTS(pinarray); i++) {
		set(i);
	}

where that code yields:

	for(int i = 0; i < NUM_ELEMENTS(pinarray); i++) {
 290:	80 e2       	ldi	r24, 0x20	; 32
 292:	ec 30       	cpi	r30, 0x0C	; 12
 294:	f8 07       	cpc	r31, r24
 296:	39 f0       	breq	.+14     	; 0x2a6 <main+0x6a>
	{ &PORTA, (1 << 5) },
	{ &PORTC, (1 << 7) }
};

static inline void set(int n) {
	pinarray[n].port->OUTSET = pinarray[n].pin;
 298:	a0 81       	ld	r26, Z
 29a:	b1 81       	ldd	r27, Z+1	; 0x01
 29c:	82 81       	ldd	r24, Z+2	; 0x02
 29e:	15 96       	adiw	r26, 0x05	; 5
 2a0:	8c 93       	st	X, r24
 2a2:	33 96       	adiw	r30, 0x03	; 3
 2a4:	f5 cf       	rjmp	.-22     	; 0x290 <main+0x54>

 

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

you know what !

i cant understand this part:

 

port_pin_t pinarray[] = {
    { &PORTB, (1 << 3) },
    { &PORTD, (1 << 2) },
    { &PORTA, (1 << 5) },
    { &PORTC, (1 << 7) },
    etc.
}

As i know every pin has its own address.and i want to build an array of pin addresses and pull them high or push them low by writing directly to the address ,efficient while reliable.

if i understand correctly

&PORTB, (1 << 3)

this means pin2  of portB as member 0 of pinarray[]

correct?

 

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

Yes, What I am showing is a way to make an arbitrary list of Ports and Pins. So my list above sets pinarry[0] to PORTB.3, [1] to PORTD.2, [2] to PORTA.5, [3] to PORTC.7

 

It could just as easily have been:

port_pin_t pinarray[] = {
	{ &PORTB, (1 << 0) },
	{ &PORTB, (1 << 1) },
	{ &PORTB, (1 << 2) },
	{ &PORTB, (1 << 5) }
};

or something like that where it's almost all consecutive pins in PORTB with just PORTB.5 being the out of sequence exception. That is PB0, PB1, PB2 and PB5.

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

Dear clawson

I saw your posts(#21 , 21)  after i wrote the post #22

perfect !

I appreciate.

Beyond beyond my expectation ! satisfying all my needs and questions i had.

I'll try to simulate my own code and count the cycles it take (of course for my advance :-) )

and post the results here at the right time ASAP.

 

best regards

ad.h

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

Dear clawson

thanks to your help i made up my mind and wrote something like this:
 

// mcu : Atxmega32a4u
// F_cpu 8000000

    typedef struct {
        volatile PORT_t *port;
        unsigned char pin;
        }port_pin_t;
    port_pin_t valveArray[]={
        {&PORTA, (1<<0)},           // valve1
        {&PORTA, (1<<1)},            // valve2
        {&PORTA, (1<<2)},            // valve3
        {&PORTA, (1<<3)},            // valve4
        {&PORTA, (1<<4)},            // valve5
        {&PORTA, (1<<5)},            // valve6
        {&PORTA, (1<<6)},            // valve7
        {&PORTA, (1<<7)},            // valve8
        {&PORTB, (1<<0)},            // valve9
        {&PORTB, (1<<1)},            // valve10
        {&PORTB, (1<<2)},            // valve11
        {&PORTB, (1<<3)}            // valve12
    };

static inline void clr(int n){
		valveArray[n].port->OUTCLR = valveArray[n].pin;
	};

static inline void set(int n){
		valveArray[n].port->OUTSET = valveArray[n].pin;
	};

void move(void){
	static int x=12;
				while (x)
				{
					set(x-1);
					my_delay(timeout);
					clr(x-1);
					x--;

				}
        x=12;

	};

int main(void){
    PORTA_DIR=0XFF;
    PORTB_DIR=0XFF;
    move();
}

 

All process (forgetting delays) takes about 38 sys clocks under simulation.

A full 12 valve on/off process takes less than 500 sys clocks and about 57us !!!

quite acceptable by me :-) (cause i know port pins are a lot slower than this numbers to toggle).

 

tons of respects :-)

&

best regards

ad.h

 

Last Edited: Sun. Oct 7, 2018 - 06:20 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Dear paul

thanks to your waning and after a few simulations i figured out bit shifting is not most affordable way to manipulate bits.

 

look at some of assembler details

you were right.

 

    PORTB_DIR=0XFF;

    
00001A52  STS 0x0620,R24        Store direct to data space

    PORTD_DIRSET|=PIN0_bm|PIN1_bm|PIN3_bm;

00001A54  LDI R30,0x61        Load immediate
00001A55  LDI R31,0x06        Load immediate
00001A56  LDD R24,Z+0        Load indirect with displacement
00001A57  ORI R24,0x0B        Logical OR with immediate
00001A58  STD Z+0,R24        Store indirect with displacement

    PORTD_OUTSET|=PIN0_bm;

00001A59  LDI R30,0x65        Load immediate
00001A5A  LDI R31,0x06        Load immediate
00001A5B  LDD R24,Z+0        Load indirect with displacement
00001A5C  ORI R24,0x01        Logical OR with immediate
00001A5D  STD Z+0,R24        Store indirect with displacement

 

 

thanks for your smart advise.

Best wishes

ad.h

 

Last Edited: Sun. Oct 7, 2018 - 06:22 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I don't total get what you want ! 

I would say that if you want an array of "valves" you should also have the state of the valve as a parameter (no separate set clr ) so you would have a valve_update(valve_nn, x) where x is 0 for off and 1 for on.

That would give a simpler programming of what to set the valve to (at least the way I programme).

 

That would make it simple to make a structure so valves can be set to default, all_closed  pump_mode_1 etc.

 

 

 

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

let me explain in another words

I've had needed  an array of pins to assign to valves and control them in code efficient way,in addition the number of valves was more than 8

so i had to assign them to more than one port without distraction and focusing on other parts of program.

the main cause to what i said upper was complicated set of communication in line with control,so as the communication must not interrupted

the timing of valves should not run out of limits.

so i thought to control valves in most efficient way trough writing directly to their registers(stable and reliable in line with code efficiency).

and i wasn't sure about proper way of addressing pins.

after i investigated the way proposed by mr clawson in assembler, i Convinced to do the code in his way.