Pointer to Register?

[mbeals 74]

Say i'm storing configuration data for a set of motor drivers in a struct.  I want to be able to define the timer register as a variable in the structure so that I don't have to mess with duplicate functions or lots of if statements when it comes time to update the value.  I'm pretty sure this is an application for pointers, but it's been a long while since I've had to play with them.

 

 

 

typedef struct{
    unsigned int speed;
    unsigned char direction;  //1 = forward, 0 = reverse
    unsigned char decay;   //1 = fast, 0 = slow
    unsigned char error;  //Motor error flag
 
    unsigned char *TCCR;
    unsigned char *PORT;
    unsigned char PIN;
} motor;

 
 
 
void changeSpeed(unsigned int* motors)
{
    motors[0].TCCR = motors[0].speed;
    motors[1].TCCR = motors[1].speed;
    
}
 
 
 
//Motor 1
motor motor1;
motor1.TCCR = &TA0CCR1;
 
//Motor 2
motor motor2;
motor2.TCCR = &TA1CCR1;
 
unsigned int* motors[2];
motors[0] = &motor1;
motors[1] = &motor2;

motor1.speed = 50;

changeSpeed(motors); 

 

 

Is this even the right approach?  

[mbeals 74]

I spent some quality time with google and thing I have things sorted out.  At least it now compiles with no errors, but I'm curious to hear feed back.  Here is some important code (pardon the length)

 

main.c

#include "msp430g2553.h"
#include "hardware_description.h"
#include "motors.h"


//Control Registers
unsigned char initialized = 0;

//MCU - globals
unsigned char debug = 1;

//Motor structs
motor motors[2];


//Main state machine
void main(void) {
	if (!initialized)
	{
		config_motor_timers();

		//Set speed to 0 and set motor direction to initialize driver input channels
		motors[0].speed = 0;
		motors[0].TCCR[0] = &MOTOR1_in1CCR;
		motors[0].TCCR[1] = &MOTOR1_in2CCR;
		motors[0].PORT[0] = MOTOR1_in1PORT;
	    motors[0].PORT[1] = MOTOR1_in2PORT;
	    motors[0].PIN[0]  = MOTOR1_in1PIN;
	    motors[0].PIN[1]  = MOTOR1_in2PIN;

		motors[1].speed = 0;
		motors[1].TCCR[0] = &MOTOR2_in1CCR;
		motors[1].TCCR[1] = &MOTOR2_in2CCR;
		motors[1].PORT[0] = MOTOR2_in1PORT;
		motors[1].PORT[1] = MOTOR2_in2PORT;
		motors[1].PIN[0]  = MOTOR2_in1PIN;
	    motors[1].PIN[1]  = MOTOR2_in2PIN;

	    changeDirection(&motors[0],1);
	    changeDirection(&motors[1],1);

		initialized = 1;
	}
	
}

 

 

 

 

motors.h

//Motors
typedef struct{
	unsigned int speed;
	unsigned char direction;  //1 = forward, 0 = reverse
	unsigned char decay;   //1 = fast, 0 = slow
    unsigned char error;  //Motor error flag
    unsigned int  max_ramp;  //Cycles to wait between speed updates

    volatile unsigned int  *TCCR[2];
    unsigned char  PORT[2];
    unsigned char   PIN[2];
} motor;

void config_motor_timers(void);
void changeSpeed(motor* Motor, unsigned int speed);
void changeDirection(motor* Motor, char direction);
void set_pin_OUT(unsigned char port, unsigned char pin, unsigned char value);
void set_pin_TIMER(unsigned char port, unsigned char pin);
void motorState(unsigned char state);

 

 

 

motors.c

#include "motors.h"
#include "msp430g2553.h"

unsigned int PWMPeriod = 100-1; //in uS, aka clock ticks for SMCLK
unsigned char motor_enable = 0;

void config_motor_timers(void)
{
	TA1CTL = TASSEL_2 + MC_1;   // SMCLK, up mode
	TA0CTL = TASSEL_2 + MC_1;   // SMCLK, up mode

	TA1CCTL1 = OUTMOD_7;
	TA0CCTL1 = OUTMOD_7;// CCR1 reset/set

	TA0CCR0 = PWMPeriod;
	TA1CCR0 = PWMPeriod;
}

//Function to smoothly change the speed of a motor observing the max_ramp rate
void changeSpeed(motor* Motor, unsigned int speed)
{
	unsigned char i;
	if (speed > Motor->speed) //if we are speeding up
	{
		for(i=Motor->speed; i<=speed; i++)
		{
			*Motor->TCCR[1] = i;
			//_delay_cycles(Motor->max_ramp);
		}
	}else{  //otherwise we are slowing down
		for(i=Motor->speed; i>=speed; i--)
				{
					*Motor->TCCR[1] = i;
					//_delay_cycles(Motor->max_ramp);
				}
	}

	Motor->speed = speed;
}


void set_pin_OUT(unsigned char port, unsigned char pin, unsigned char value)
{

	unsigned volatile char *DIR;
	unsigned volatile char *POUT;
	unsigned volatile char *REN;
	unsigned volatile char *SEL;


	switch (port)
	{
	case 1 :
	   DIR = &P1DIR;
	   POUT = &P1OUT;
	   REN = &P1REN;
	   SEL = &P1SEL;
	   break;
	case 2 :
		DIR = &P2DIR;
		POUT = &P2OUT;
		REN = &P2REN;
		SEL = &P2SEL;
		break;
	}

	*SEL &= ~pin;    //Turn off special feature (turn into GPIO)
	*REN |= pin;		//Enable pull up resistor
	*DIR |= pin;		//Set pin as output

	//Set pin as 0 or 1
	switch (value)
	{
	case 0:
		*POUT &= ~pin;
		break;
	case 1:
		*POUT |= pin;
		break;
	}
}


void set_pin_TIMER(unsigned char port, unsigned char pin)
{
	unsigned volatile char *DIR;
	//int *OUT;
	//int *REN;
	unsigned volatile char *SEL;


	switch (port)
	{
	case 1 :
	   DIR = &P1DIR;
	  // OUT = &P1OUT;
	  // REN = &P1REN;
	   SEL = &P1SEL;
	   break;
	case 2 :
		DIR = &P2DIR;
		//OUT = &P2OUT;
		//REN = &P2REN;
		SEL = &P2SEL;
		break;
	}

	*SEL |= pin;
	*DIR |= pin;

}

//Function to change the direction a motor is turning.
void changeDirection(motor* Motor, char direction)
{
	    Motor->direction = direction;  //store direction in struct
	    int old_speed = Motor->speed;

	    changeSpeed(Motor,0);		   //Bring motor to zero speed safely before changing dir

	   	if (Motor->direction)   //if going forward
	   	{
	   		set_pin_OUT(Motor->PORT[0], Motor->PIN[0], 1);
	   		set_pin_TIMER(Motor->PORT[1], Motor->PIN[1]);

	   	}else{
	   		set_pin_OUT(Motor->PORT[0], Motor->PIN[0], 0);
	   		set_pin_TIMER(Motor->PORT[1], Motor->PIN[1]);
	}

	   	changeSpeed(Motor,old_speed);

}

void motorState(unsigned char state)
{
	P2SEL &= ~BIT5;
	P2DIR |= BIT5;
	P2REN |= BIT5;

	if(state)
	{
	   P2OUT |= BIT5;
	}else{
	   P2OUT &= ~BIT5;
	}

}

 

 

 

 

 

 

 

 

[roadrunner84 466]

When I saw this code

		motors[1].speed = 0; 		motors[1].TCCR[0] = &MOTOR2_in1CCR; 		motors[1].TCCR[1] = &MOTOR2_in2CCR; 		motors[1].PORT[0] = MOTOR2_in1PORT; 		motors[1].PORT[1] = MOTOR2_in2PORT; 		motors[1].PIN[0] = MOTOR2_in1PIN; 	 motors[1].PIN[1] = MOTOR2_in2PIN;

The first thing I thought was "why are TCCR pointers, but PORT not?"

You use this switch on much places to determine the port registers depending on the value of a variable. Instead you could use a similar mechanism here.

You wouldn't need to store all port related pointers, just a base pointer; the relative position of the port related registers is similar for every port. This means you could determine the position of one from another.

If PIN is your base, then the position of POUT would be as POUT = PIN + (P1OUT-P1IN).

Say what again? If P1OUT is X bytes further in memory than P1IN, than any POUT would be 1 byte further in memory relative to the corresponding PIN. So (P1OUT-P1IN) would be X, we add this X to the pointer PIN to get any POUT.

[yyrkoon 250]

A couple of things to consider.

 

Table driven state machine.

 

http://brettbeauregard.com/blog/2011/04/improving-the-beginners-pid-introduction/'>Improving the Beginner

[yyrkoon 250]

mbeals, not sure how you feel about C++, but at first look, it seems almost a perfect fit for a C++ template class. Granted, I have been using C++ template classes a lot myself lately so I am kind of always looking for way to make things "fit" into template classes . . .

 

Something like this: Nokia 5110 C++ Template Class The specific device is not all that interesting for your case, but look at how pointers to ports, etc are being passed around at object instance time. For that specific case, the values passed in ( by reference ) take on no extra overhead.

 

What I was thinking in your case however, was about creating an object instance for each motor, with its own configuration, like port, pins, and various helper methods ( ChangeSpeed(), ChangeDirection() , etc ) for each object.

 

Perhaps . . . I have not really thought it thru 100%.

[roadrunner84 466]

Templatized registers would work quite nicely in this case, it needs some getting used to, but it can make your programming luge a lot more fun.

By the way, that register trick I told you does not work for interrupts related to gpio. (pin change interrupts to be more specific)

[mbeals 74]

 

When I saw this code

		motors[1].speed = 0; 		motors[1].TCCR[0] = &MOTOR2_in1CCR; 		motors[1].TCCR[1] = &MOTOR2_in2CCR; 		motors[1].PORT[0] = MOTOR2_in1PORT; 		motors[1].PORT[1] = MOTOR2_in2PORT; 		motors[1].PIN[0] = MOTOR2_in1PIN; 	 motors[1].PIN[1] = MOTOR2_in2PIN;

The first thing I thought was "why are TCCR pointers, but PORT not?"

You use this switch on much places to determine the port registers depending on the value of a variable. Instead you could use a similar mechanism here.

You wouldn't need to store all port related pointers, just a base pointer; the relative position of the port related registers is similar for every port. This means you could determine the position of one from another.

If PIN is your base, then the position of POUT would be as POUT = PIN + (P1OUT-P1IN).

Say what again? If P1OUT is X bytes further in memory than P1IN, than any POUT would be 1 byte further in memory relative to the corresponding PIN. So (P1OUT-P1IN) would be X, we add this X to the pointer PIN to get any POUT.

 

This was actually how I started off writing the code.  Then I figured that since this chip only has 2 ports, that it was worth paying a small memory penalty for the sake of readability.  If this was an ARM chip with its 50 billion ports, then I would most definitely do it like this.  I may switch back....I'm not sure yet....I'm just happy to have sort of remembered how pointers work.

 

 

 

mbeals, not sure how you feel about C++, but at first look, it seems almost a perfect fit for a C++ template class. Granted, I have been using C++ template classes a lot myself lately so I am kind of always looking for way to make things "fit" into template classes . . .

 

Something like this: Nokia 5110 C++ Template Class The specific device is not all that interesting for your case, but look at how pointers to ports, etc are being passed around at object instance time. For that specific case, the values passed in ( by reference ) take on no extra overhead.

 

What I was thinking in your case however, was about creating an object instance for each motor, with its own configuration, like port, pins, and various helper methods ( ChangeSpeed(), ChangeDirection() , etc ) for each object.

 

Perhaps . . . I have not really thought it thru 100%.

 

I don't have strong feelings for C++ either way.  I have a lot of time invested in OO matlab code, so that's the way I tend to approach problems now anyway, but when I started looking into C++ objects, I realized I should probably try a simpler solution until I got up to speed.    I'll have to sit down and dig into that code to figure out what all is going on.

 

One of the other issues I'm having is thinking of an elegant way to keep the functions from blocking.  I'm planning on using a set of global registers that are updated through the I2C and UART interrupt vectors (a new command comes in/something changes, an interrupt updates the value and sets a 'update me' flag), then just leaving the main loop to continually poll for new update flags.  However, I have some functions, like the change_speed function that can take multiple cycles to execute (e.g. the soft start where it incrementally increases speed).  There is no reason to have that function blocking execution on the stack just to update the CCR1 register a few times.  I need to somehow have the change_speed function tell the main loop to execute a series of commands, one per loop cycle....but I'm not sure how to pull that off in a generalized manner.

[RobG 1,891]

Here's something you might be able to use (it's for port registers, but you'll get the idea.)

 

// register lookup table
volatile u_char * const registerPointer[] = { &P1IN, &P1OUT, &P1DIR, &P1REN, &P1SEL, &P1SEL2, &P1IE, &P1IES, &P1IFG };
// to set the new value
*registerPointer[registerToSet] = newValue;
// or use struct
typedef struct {
    u_char register1;
    u_char value1;
    u_char register2;
    u_char value2;
} TimerValues;
// then 
*registerPointer[myNewValues->register1] = myNewValues->value1;
*registerPointer[myNewValues->register2] = myNewValues->value2;

[yyrkoon 250]

mbeals, so something like this ? Simple two state (state machine?)

 

Granted, what I did for that post is extremely simple, just to illustrate the concept. But basically, the WDT in interval mode sets a bit flag within any type of variable, and the main loop once awake, checks the flag, see's that its set, does something, then clears the flag before entering back into sleep. This is also part of how my reflow oven controller "firmware" works.

 

One thing that might be more of interest to you though, in this context( going by your latest post ). Perhaps using a ring buffer to push, and pop "flags" or method calls into the buffer that will be acted upon at some later point. This is not something I had really considered much myself, until Rickta59 brought it up in a way that made total sense to me on IRC. In the end though, it kind of ends up looking ( at least in my own mind ) a lot like the table driven state machine example link I posted above. Just circular.

[mbeals 74]

Perhaps using a ring buffer to push, and pop "flags" or method calls into the buffer that will be acted upon at some later point. 

 

 

Exactly what I want to do....but I'm not sure to store function calls and their arguments in a ring buffer.  Although it may be somewhat moot.  I seem to remember the driver chip handling soft start internally...but it would still be nice to have a dynamic execution stack like that

[yyrkoon 250]

function pointers. Look at that table driven state machine link to look at one example of how it is done. Well actually, you can search the web for "function pointers in C/C++" and come up with some fairly decent examples. Atleast when I recently did a search, I ran into a couple of good examples. let me see if I can find one quickly.

 

Oh, hmm yeah my bad I misunderstood your question. Honestly I am not very familiar with ring buffers in this context, perhaps one of the more advanced C/C++  people on the forums will speak up here.