Ext Interrupt Driven Data Collector Troubbles

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

Hello all, I have a project on the go and while it *kinda* works its terribly non modular and not scalable at all.

I have an external 90kHz clock source that is horribly inaccurate sending me packets of data 24bits long. I decided to use PCINT0.PA0 as the clock input.

My current code reads my fake 8bit input and displays it on the LCD in both its binary form as well as its decimal equiv. The data is 24bits signed. In my final version I only want to know the Dec output with a +/- sign.

itoa seemed like a good choice of functions as it will automatically add the sign for me and I need to turn my int into a string for the LCD to output.

Is their a better way to capture each bit until all 24 are in and then call an LCD update? As if I have a 24bit array I don't know of any functions to convert it into a doubble for easy manipulation.

and is their a built in function by chance that will convert a signed binary value to decimal easily?

// Project Includes
#include 
#include 
#ifndef F_CPU
	#define F_CPU 1000000UL
#endif
#include 
#include 
#include 

// LCD Pin Definintions
#define LCD_DATA_PORT PORTD
#define LCD_DATA_DDR  DDRD

#define LCD_CTRL_PORT PORTB
#define LCD_CTRL_DDR  DDRB
#define LCD_RS 0
#define LCD_RW 1
#define LCD_EN 2

// LCD Display Settings
#define Cursor        0
#define Blink         0

// Function Declarations
void LCD_init();
void LCD_enable();
void LCD_nibble(unsigned char nibble);
void LCD_command(unsigned char command);
void LCD_putc(unsigned char ascii);
void LCD_puts(unsigned char *string);
void LCD_goto_xy(unsigned int x, unsigned int y);
#define LCD_puts_xy(x,y,string) {LCD_goto_xy(x,y);LCD_puts(string);}

// Global Variable Declarations
volatile int buffer[8];
volatile int update = 0;
volatile int step = 0;
int tmp_buff = 0;
char str_buff[11]; // 24bit binary + sign & decimal

// Pin change 0-7 interrupt service routine
ISR(PCINT0_vect) 
{
	cli();
	
	// Fake input, Generates 11110000
	if(step < 4) buffer[step] = 1;
	else buffer[step] = 0;

	if(step < 8)	step++;
	if(step == 8)
		{
			update = 1;
			step = 0;
		}
	if(step > 8) step = 0;

	sei();	
}

int main()
{ 
	LCD_init(); 
	  
	// External Interrupt(s) initialization
	EICRA=0x2A;
	EIMSK=0x01;
	PCMSK0=0x01; // Listin On PCINT0
	PCICR=0x01;  // Config For PCINT0:7
	PCIFR=0x01;  // Interrupt Flag Register

	// turn on interrupts!
  sei();

	char *message = "Reading Data Test";
	LCD_puts_xy(0,0,message);

	for(;;)
	{
		if(update == 1)
		{
			itoa (buffer,str_buff,2);  // Display Binary Data
			LCD_puts_xy(0,2,str_buff);

			tmp_buff = ((buffer[0]*1)+(buffer[1]*2)+(buffer[2]*4)+(buffer[3]*8)+(buffer[4]*16)+(buffer[5]*32)+(buffer[6]*64)+(buffer[7]*128));
			itoa (tmp_buff,str_buff,10);   // Display Dec Equiv
			LCD_puts_xy(0,3,str_buff);

			update = 0;
		}
	}                 
return 0;
} 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Rather than use one int per bit, you can shift bits in this way. Note this is MSB first. Declare bit_bucket as a long if you want 24bits.

bit_bucket<<=1; //shift bits along
if (bit) //if new bit == 1
{
bit_bucket |= 1; //'or' in a '1' bit
}

You also should choose your storage sizes better - don't use ints when a char is sufficient.

What is the problem with using itoa()? It converts a signed integer to ascii which is what you want it seems.

You would also need some form of interlock between the isr and the main line code with your buffer[] data - if this is getting changed in the background, what gets displayed will also change based on the current values. You might want to have an intermediate variable for this.

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

Thanks for the quick reply.

I was thinking about bit shifting the data in but as its MSB first I would then need to reverse the whole string. Though in thought this might still be less code and more scalable even with the reversal.

I choose storage sizes randomly during debug as half the time I have no Idea the scope of the variable. In the debug stage I follow variables and downsize my variables as well as sign/unsign them.

I quite like itoa() its just that it outputs to an array and my LCD function takes a string so I either need to write a function to convert an array to a string or re-write the LCD routine to use both.

Interlock between the ISR and Main? I will have the buffer as volatile and I dump it into a temp variable when pass that to my LCD routine so the buffer can be updated while the LCD is busy. Do you think I need more separation?

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

Don't you mean lsb first?
In which case the following will do the job for 24 bits.

bit_bucket>>=1; //shift bits along
if (bit) //if new bit == 1
{
bit_bucket |= 0x800000L; //'or' in a '1' bit
}

Quote:
Interlock between the ISR and Main? I will have the buffer as volatile and I dump it into a temp variable when pass that to my LCD routine so the buffer can be updated while the LCD is busy. Do you think I need more separation?

Where you copy it into the temp variable is the problem. If you look at the generated assembler you'll see that this operation takes many machine instructions and an interrupt can occur at any time. Therefore, you may read some of one sequence and some of the following sequence which may cause problems.
Whilst declaring the variable volatile is necessary, the compiler doesn't fix the problem for you.

I wrote some words on this:
https://www.avrfreaks.net/index.p...

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

Kartman wrote:

bit_bucket>>=1; //shift bits along
if (bit) //if new bit == 1
{
bit_bucket |= 0x800000L; //'or' in a '1' bit
}

Oh wow I cant believe I didn't think of shifting it in like that.. I didn't use it because I was thinking "If I take my input and shift it right it will be removed from the variable (10110 >>2 = 00101)" but it makes perfect sense to add the input to the top and shift right...

This is why I ask these things, I took a year long break from programming and I forgot ALL my linear algebra and c/c++ syntax, It makes me sad.

I took a peek at the asm and it does generate quite a bit more code than I thought to shuffle variables around, asm makes my head hurt but I will read through your tutorial tonight. Thanks.

One thing I forgot to mention is my PCINT0 triggers an interrupt on the rising and falling edge currently, I have it set in EICRA for falling edge and even tried setting all three to falling edge (0x2A) but it still triggers on both edges. I tested it with a simple push button and its not switch bounce.

// External Interrupt(s) initialization
   EICRA=0x02;
   EIMSK=0x01;
   PCMSK0=0x01; // Listin On PCINT0
   PCICR=0x01;  // Config For PCINT0:7
   PCIFR=0x01;  // Interrupt Flag Register 

(Im running a mega164p)

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

Pin-change interrupts trigger on both edges--see the datasheet.

Use an "external interrupt" if you want edge configuration.

Lee

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

theusch wrote:
..if you want edge configuration.

Thanks for making me re-re-read the datasheet, I thought I had to set all of the registers to make it work but I removed it and now have this:

	// External Interrupt(s) initialization
	EICRA  |= (1<<ISC01);  // Falling Edge Trigger
	EIMSK  |= (1<<INT0);   // ISR On INT0 Pin (PD2)

And it works just as I wanted, I didn't even realize their was a difference between PCINT and INT they use so many acronyms in these bloody datasheets.

Kartman - Im now right shifting my data in and it works beautifully. Now I need to work on how to format the bloody string. I think im going to write my own signed binary to int routine but then I found sprintf.

sprintf(str_buff,"%#+4.4Lf",buffer);

I cant get printf to work yet due to a linker issue I need to address but I cant find any documentation on weather this function can convert signed binary I found out that itoa does not.

Thanks!