Need help with interrupt

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

I am basically new to all this AVR stuff, I have been working with the Arduino for quite some time and I understand how to use that, but proper AVR stuff is a lot harder for me. What I am trying to do is read an IR code from specialised air conditioning remote. These codes are quite complex and not like TV codes or anything. Their length can be anything from 80 bytes to 800 bytes. The codes I am fine with and understand perfectly how they work. I built this project originally on the Arduino and tried porting it over, losing all the Arduino specific stuff and it didn't work. The timing was dodgy and it sometimes missed bits, or even whole sections. Originally it used while loops and delays to time how long the pin was either high or low.

After spending the last month trying to get it working on my own, I decided to try another route and use interrupts instead. I figure this will be more accurate and reliable method. Now it doesn´t work at all. What happens now is as soon as the device is powered the interrupt fires and send out loads of blank data. I´m sure it just a simple mistake, but I cannot see it. This project will be more complicated later. For now I just want to check that I am saving the correct data, the sendData function will be deleted once I know the data is good. I am using an ATMEGA328P for now. So here is the code I have I done. I would really appreciate it if someone can point out where I am going wrong. It´s not a hardware problem, I have verified that codes are been sent clean with my oscilloscope. Thanks.

#define F_CPU 8000000UL 
#include 
#include 
#include 

//Define Ports/Pins
#define StatusLED_PORT PORTB
#define StatusLED PORTB5
#define irREC_PORT PORTD
#define irREC PORTD2
#define irLED_PORT PORTD
#define irLED PORTD3

//Global Defines
#define MaxPulseLength 6500  //The Maximum Pulse length is 65mSec

//Prototypes
void error (uint8_t errorCode);
void sendData (void);

//Global Vars
uint16_t CurrentpulseLength = 0;  //The length of the current pulse
uint16_t data[800];			//temporary Storage of the data
uint16_t bitNumber = 0;	//The current bit number when rec/send code
uint16_t lengthOfData = 0;	//The length of the code


int main (void)
{
	DDRB |= (1 << StatusLED);	// Set LED as output
	PORTD |= (1 << PORTD2);	// set PD2 to high (internal Pull-up on)
	
	StatusLED_PORT |= (1 << StatusLED);	// Turn On status LED
	_delay_ms(500);
	StatusLED_PORT &= ~(1 << StatusLED);// Turn Off status LED
	
	TCCR1B |= (1 << WGM12);		// Configure timer 1 for CTC mode

	TIMSK1 |= (1 << OCIE1A);	// Enable CTC interrupt
	
	EIMSK |= (1<< INT0);		//Enable INT0
	
	EICRA |= (1<< ISC00);	//Set interrupt to trigger of any logic change on INT0

	sei();	//  Enable global interrupts

	OCR1A   = 79;				// Set CTC compare value to 100KHz  

	TCCR1B |= (1 << CS10) ;		// Start timer at Fcpu/1
	
	for (;;)
	{

	}
}

ISR(TIMER1_COMPA_vect)
{
	CurrentpulseLength++; // Add 1 to the pulse length timer
	
	if (CurrentpulseLength >= MaxPulseLength) //If the pulse is longer the maximum defined pulse then we have got all the code 
	{
		EIMSK &= ~(1<< INT0);		//Disable INT0
		TIMSK1 &= ~(1 << OCIE1A);	//Disable CTC interrupt
		lengthOfData = bitNumber;	//Save the length of the code
		//sendData();					//Send the data out (DELETE THIS)
	}
}	

ISR(INT0_vect)
{
	data[bitNumber] = CurrentpulseLength;	//Save the pulse length
	CurrentpulseLength = 0;					//Clear the pulse length
	bitNumber++;							//Go to the next bit
	StatusLED_PORT ^= 1<<StatusLED;			//Toggle the Status LED
	
	if (bitNumber >= 800)					//If we try save too much data stop getting data and display the error
	{
		EIMSK &= ~(1<< INT0);				//Disable INT0
		TIMSK1 &= ~(1 << OCIE1A);			//Disable CTC interrupt
		StatusLED_PORT &= ~(1<<StatusLED);	//Turn off the Status LED
		error(1);							//Send Error Code 1
	}
	
}

void sendData ()
{
	TIMSK1 |= (1 << OCIE1A);
	
	while (CurrentpulseLength < data[bitNumber])  //Do nothing until the pulse length == to the saved data value
	CurrentpulseLength = 0;		//Reset the current pulse
	irLED_PORT ^=(1<<irLED);	//Toggle the IR LED
	bitNumber++;				//go to the next bit
	
	if (bitNumber >= lengthOfData)
	{
		TIMSK1 |= (1 << OCIE1A);  //Disable the timer interrupt
		for (uint16_t i=0; i
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You haven't said what it is that is supposed to happen and what actually does happen?

I could be wrong but it looks like you may be sharing variables between two threads of execution without them being volatile. If so that's a no-no.

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

Your code has a lot of comments but nowhere does it tell me what the code is expected to do. I can only guess there is some sort of incoming bitstream you are wantibg to decode. Without knowing much more i could suggest that maybe you want to use the timer input capture feature and structure your code so that you avoid delays in the isrs.

Do you have any information on the structure of the bitstream? IR codes are normally manchester encoded or pulse position encoded. You would normally measure then bits, classify the bits ( fat/skinny) with some tolerance and in the case of manchester encoding, feed the bits into a state machine to extract the data and clock. Then you would have some form of preamble that you search for to synchronise the bits - a long stream of ones or zeroes . For long data streams you would most likely have some form of error check code.

Looking more closely at your code, you have the timer set up to interrupt at 100kHz - that gives you 160 clocks to do your work. I think you're pushing your luck. If you time your isrs in the simulator you might find you might be running out of time. Also, sampling at that rate you could eliminate the external interrupt and just read the input pin state in the timer isr. I would suspect even doing this you may run out of time. Input capture takes care of some of the grunt work. When the input changes state, it captures the timer count and interrupts. Your isr can read the capture value and based on a previous capture, calc the bit width with good precision.

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

Thanks. What is the input capture feature, that sounds promising? I can´t find anything with that name in the datasheet.

What I was trying to with the code is have 2 ISRs. The first is the timer, which I wanted to trigger every 10usec and add 1 to a counter. The other to check for changes on the ir receiver pin, when the pin changes it stores the time since the last change and resets the timer and waits (while timing) for another pin change. If the pin does not change after 65msec, then I take what been stored so far and send it out to my oscilloscope, for checking.

Quote:

I could be wrong but it looks like you may be sharing variables between two threads of execution without them being volatile. If so that's a no-no.

I don't know what you mean by this. It´s the threads of execution bit I don't know what that means.

What happens now is that as soon as the device is powered, irrespective if there is a signal or not, it outputs a load of blank data, i.e. very quick on/off pulses.

The data is mixed types, it depends which device I am working with and which option. Generally the low pulses are constant and there are 2 types of high pulses (1 and 0), but this isn´t always the case and the start bits differ as well. That´s why I need something universal-ish to store the codes.

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

I could see the technique you were using. As such, interrupts won't make a bad algorithm any better, so if you couldn't get it working without interrupts, then adding them most likely won't help at all. I suggest you run the code in the simulator and do some timing measurements. Unless you want the reception to run in the background, avoid using interrupts - especially with your current technique.

Input capture - it's in the timer1 section of the datasheet. The register name is ICR1.

Regarding 'volatile' see Clawson's footer - FAQ#1

I've also written a tutorial on 'the traps when using interrupts'.

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

Thanks, I found the ICR1 stuff in the datasheet. If I have understood it correctly, then that does pretty everything my code does, but in hardware. So it measures the duration between 2 logic changes and saves the time as a 16-bit value in the ICR register, then creates an interrupt. All i would have to do is save the timed event in the data array I already have, add one to the counter (to go to the next value in the array), and then clear 16-bit timer resigister. Is that correct? I would not need any of the interrupts that I already have, but do I create a new ISR for the ICR1 or do I poll the register until it changes. I don´t quite get how to do that bit. And also i am not sure how to find out when the ir code has finished transmitting. With the method I am using now, (granted it doesn´t work) but i can measure if the current pulse has been longer than 65mSec and then take that as the end of the code. How do I do that with the ICR method?

I read the FAQ for volatile, I see what I did wrong now. What I should do is change any global variable that modified by the ISR to volatile, correct?

I couldn´t find the other tutorial on the traps when using interrupts, could you tell me where I can find it.

Thanks

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

Regards,
Steve A.

The Board helps those that help themselves.

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

Strictly speaking, the input capture triggers on a single event, it doesn't time between two. For instance, to measure the width of a pulse, you may setup the capture to look for the rising edge and then for the falling edge.

As far as finding the edge of a code, it would be easy enough to configure a timer compare interrupt that is continually advanced by what ever timeout makes sense, i.e. 65ms.

Martin Jay McKee

As with most things in engineering, the answer is an unabashed, "It depends."

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

Thanks, that is a really good tutorial.

I'm lost again now though. If I measure the width of a pulse, ie the rising edge until the falling edge, then I would only get half the code, i wouldn't have the falling edge to rising edge. Or is there a way to keep inverting the trigger quick enough to catch the pulses? I reckon that I need 20uSec resolution to capture the codes reliably enough. I had it at 10uSec in my code, just to be sure.

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

Quote:

Or is there a way to keep inverting the trigger quick enough to catch the pulses? I reckon that I need 20uSec resolution to capture the codes ...

Resolution isn't going to be your problem--ICP resolution can be down to a single AVR clock cycle.

But you need to describe a bit more the high/low times of these pulses, especially the min.

As mentioned earlier, I'd think the protocol would be straightforward--like the "fat/skinny" mentioned. So what is the "frequency"--the normal period of the signal?

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

With input capture you really don't want to touch the timer as this introduces timing errors. If you do your calcs in unsigned int, it works out correctly. Do the calc in binary or hex with a pencil and paper to see how it works. From my experience,the pulse widths are usually 50us or greater, so 10us resolution is adequate and the overall bit rate is around 1000 bits/sec unless it is IRDA but that is very short distance.

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

The problem is there isn't a "normal" pulse. I am dealing with several different types of coding, which all differ. From 100+ codes I have tested so far the smallest pulse I have found is 200uSec. For example, that particular code sends 5 200uSec pulses (200 on, 200 off) followed by 32mSec off, then 3.6mSec on, 400uSec off, then the data which is 200uSec off and either 1.6mSec, 1.2mSec or 450uSec on. The data is 300 bits and at the end there is a 1.6mSec on, presumably a stop signal. But this is just one particular code, the next one could be nothing like this at all and could be more similar to a TV code, just longer. Some of the more simpler codes just send a long on/off then 30-40 bits of data, which is the one I was testing with, as its fairly simple. That is also the one I had working on the Arduino, I could control the air conditioning unit with the Arduino with that code.

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

It should be easy to port the arduino code. I gather the code would be testing port bits and using delays. Anyway, input capture can measure the pulses.

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

Yes the arduino used delays and tested the ports.

I have been having a go with the input capture and found some examples online. One of the examples I found uses the following to work out the length of the pulse.

uint32_t length = (((uint32_t)(ICR1)) + 0xFFFF - last_timestamp) & 0xFFFF;

How does that work? Why can't you just simply do length - last_timestamp? And why is it cast to a 32 bit value when the timer is only 16bit?

the other bit I am stuck on is how to find out when the code has stopped. My idea at the minute is set up an interrupt to wait for a low pulse (the IR receiver is normally high), when that occurs, start the timer and input capture with overflow. If I figure out the length of the longest possible code and set the prescaler so the timer overflows a bit after that and use the interrupt on the overflow to stop getting the code. Would that work or is there a better way? i know the first measured pulse would be a little bit out because of the time taken to start the timers etc, but I can make up for that in software, no? Everything else I am OK on I think, it´s just these few things which I am stuck on.

Thanks to anyone who can help me.

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

I think I have it working, finally. I still need to test it against more codes to make sure, but so far it looks bang on to within 1uSec. I found some code on the internet for using the ICR and based my code heavily around that. The only bit I don´t understand is the 32bit stuff in my last post, everything else I get.

Here is the code I have, I would appreciate any comments anybody may have on it.

#define F_CPU 8000000UL
#include  
#include  
#include  

// pin definitions
#define IRpin_PIN PINB
#define IRpin 0
#define irLED_PORT PORTD
#define irLED PORTD3
#define statusLED_PORT PORTB
#define statusLED PORTB5

// global variables, volatile so they can be safely used inside interrupt routines
static volatile uint16_t last_timestamp;
static volatile char has_overflowed;
volatile uint16_t data[400];	//Storage for the data
volatile uint16_t bitnumber;	//counter for the data bit number (array number)
volatile uint16_t lengthOfData;  //How many bits the data is

//Prototypes
void timerCapture_init(void);
void sendCode (void);

void delayMicroseconds(uint16_t count) {
	while(count--) {
		_delay_us(1);
	}
}

//IRPIN has gone LOW and caused the interrupt to start getting the code
ISR (PCINT0_vect)
{
	cli();  //Stop all interrupts
	
	PCICR &= ~(1<<PCIE0);  //Disable interrupt on PCIE0 ports
	
	TCCR1B = (1<<ICNC1) | (1<<ICES1) | (1<<CS11) ; // start timer, enable noise canceler, trigger on rising edge, prescale 8
	TIMSK1 = 1<<ICIE1 | 1<<TOIE1; // enable timer capture interrupt and timer overflow interrupt
	
	sei();  //Restart Interrupts
	
	//RESETS
	statusLED_PORT &= ~(1<<statusLED);  //Turn off the status LED
	has_overflowed = 0;
	last_timestamp = 0;
	bitnumber = 0;
}

// output has changed, thus initiating a capture, causing this interrupt
ISR(TIMER1_CAPT_vect)
{
	// calculate using last timestamp
	uint32_t length = (((uint32_t)(ICR1)) + 0xFFFF - last_timestamp) & 0xFFFF;
	last_timestamp = ICR1;
	has_overflowed = 0; // reset timer overflow flag
	
	// output the pulse width
	if (IRpin_PIN & (1 << IRpin))
	{
		TCCR1B &= ~(1<<(ICES1)); // switch triggering edge for next capture
		data[bitnumber] = (uint16_t)length;
		bitnumber++;	//go to the next bit
	}
	else
	{
		TCCR1B |= 1<<ICES1; // switch triggering edge for next capture
		data[bitnumber] = (uint16_t)length; //go to the next bit
		bitnumber++;	//go to the next bit
	}
	if (bitnumber >= 400)
	{
		lengthOfData = 400;
		sendCode();	
	}
	
}

// overflow event, two overflows in a row means the remote control is off
ISR(TIMER1_OVF_vect)
{
	if (has_overflowed == 1)
	{
		lengthOfData = bitnumber;  //Save how long the data is
		sendCode();
		has_overflowed = 2; // send only once
	}
	else
	{
		if (has_overflowed != 2)
		{
			has_overflowed = 1;
		}
	}
}


int main()
{
	DDRB |= 1<<statusLED; //Set the status LED as an output
	statusLED_PORT |= 1<<statusLED; //Switch on the status LED
	
	DDRD |= 1<<irLED;  // Set the IR LED output to be an output
	irLED_PORT |= (1<<irLED);  //Set the IR Output to be high, to match the receiver 
		
	PCICR |= 1<<PCIE0;  //Enable interrupt on PCIE0 ports
	PCMSK0 |= 1<<PCINT0;  //Set PCINT0 (PB.0) as the interrupt pin
	sei();	// enable interrupts
	while (1); // infinite loop
}

void sendCode()  //TEMPORY FUNCTION FOR TESTING DATA
{
	
	for (int i=0;i

Thanks to everyone who has helped me. I should've just asked a month ago instead of struggling on my own.

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

The person who wrote the code didn't realise that doing the calc as a uint16_t would work just the same. I think your overflow logic is a bit flawed. Consider the timer keeps on running and you might start capturing at any count of the timer so your overflow could happen in 65000 ticks or 1 tick and any inbetween. If you want a timeout after the last transition, use the output compare feature to give an interrupt X ticks from your last capture.
Eg: OCR1A = ICR1A+ TIMEOUT_TICKS

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

The way I understand it, with the code I posted, the timer must overflow twice in succession to end the code. So maximum pulse length is anything between 65535 and 131070 ticks, which is fine for me. However, If I use output compare like suggested, then I only need that one line of code and my other ISRs will be quicker, as there is less code in them. Have I understood that correctly?

Quote:

The person who wrote the code didn't realise that doing the calc as a uint16_t would work just the same.

So I can just do length=ICR1 - last_timestamp; instead?

Thanks

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

I didn't pick the two overflows, so if you're happy with that mechanism, run with it. Using the output compare is just a bit more explicit - you might save some cycles but I doubt if will make a big difference. Your call.

If length and last_timestamp are uint16_t, then the calc will work. Try it and you'll see.

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

I am stuck again. What I am trying to do now is make the codes that I save smaller so I can save at least 2 codes into the EEPROM. My idea was to loop through all the saved data and pick out the 1´s and 0´s and save them as an 8 bit value, therefore significantly reducing the codes. If for example the first 8 pulses were all 1´s, then I would save it as 0xFF, and if say it was four 1´s and then four 0´s I would save it as 0xF0, and so on. Anything that does not match a 1 or 0, I just save that as it´s raw timing.

It doesn´t work, so now I am just focusing on one particular code until I can get that to work, then I will figure out how to make more universal. This particular code sends 5 short pulses high of 550uSec (which are 0´s). The low pulses are all 330uSecs, which is common through the code. Then it sends a 30mSec high followed by 12mSec low, followed by 3mSec high, then the first bit of the code, either 1´s (1.2mSec) or 0 (550uSec). After that it sends the same sort of thing 2 more times, starting from the 30mSec high. The data is not the same all three times though, it does change a little bit each time.

With my code It just saves everything as a RawCode and does not recognise any 1´s or 0´s and I have no idea why.

Any help greatly appreciated.

Thanks.

void decodeData()  //Decoded from the remote code to our own code
{
cli();
lowbit = data[2];
dataZero = data[1];
uint8_t j= 0; //used for the low raw code array
uint8_t k= 0; //used for the high raw code array
uint8_t bitCounter = 0; //Used to count which bit we are in for the code hex
uint8_t byteCounter = 0; //Used to count which byte we are in for the code hex
uint16_t i = 16;


		//We already know what 0 is as we guess its the data[1] value, so the next thing is top find out what a 1 is.
		//To do that we are going to start going through the data until we find a value that does not match the zero
		//value, and call that our 1 value.  We start at data[16] to miss all the odd values at the start of the code.
		
		while (dataOne == 0)
		{
		  if ((data[i] > (dataZero - 50)) && (data[i] < (dataZero - 50 )))
		  {
			  i++;
		  }
		  else 
		  {
		  	dataOne = data[i];
		  }		 	  
		}
		
		//Now we know what a 1 and 0 is, 
		i=0;	//Start from the begging of the code
		//Loop through the data and save all the values as either 1´s or 0´s and also the low pulse
		for (byteCounter = 0; byteCounter<44;byteCounter++)
		{
			for (bitCounter = 0; bitCounter<7;bitCounter++)
			{
				//First start with the low pulses and only save value which do not match our known low pulse value and are not the start pulse
				if (!((data[i] > (lowbit - 50)) && (data[i] < (lowbit + 50))))
				{
					lowDataRaw[j] = data[i]; //Save what the raw timing is
					lowDataRawPos[j] = i; //Save the position in the code so we know where to put it when recode the data
					j++; //increase the counter 
					if (j>19)
					{
						setMode = 0;
						LEDblink(38);  // blink the LED 38/2 times so we know we have an error
						setMode = 1;
						resetData();
						return;
					}					
				}
				
				i++;	//increase the i counter
			
				if (data[i] == 0) //if the data i is blank, then end this function and move on to save the code
				{
					reCodeData();
					return;
				}
			
				//Now we can move on to the high pulses
				//if this is a 1, save it as a 1 in our hex code
				if ((data[i] > (dataOne - 200)) && (data[i] < (dataOne + 200)))
				{
					dataHex[byteCounter] |= (1<<bitCounter);
				}
							
				//If it is not a 0, 1, the mark or first pulse then save its raw value and position in the code
				else if (!((data[i] > (dataZero - 50)) && (data[i] < (dataZero + 50))))
				{
					highDataRaw[k] = data[i]; //Save what the raw timing is
					highDataRawPos[k] = i; //Save the position in the code so we know where to put it when recode the data				
					k++;
					if (k>19)
					{
						setMode = 0;
						LEDblink(40);
						setMode = 1;
						resetData();
						return;
					}				
				}			
				//increase the counter
				i++;	
						
				if (data[i] == 0) //if the data i is 0, then end this function and move on to save the code
				{
					reCodeData();
					return;
				}
			
				
			}
		
		}							
	
}
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

#define irREC PORTD2
#define irLED_PORT PORTD
#define irLED PORTD3

two symbols with the same definition?

Simple stuff first. Does your IR transmitter 'chop' the IR signal into 38KHz pulses? Is the IR receiver on the external device expecting to see IR 'chopped'?

Are you blasting the IR light pulses with a current booster like a ULN2003 chip? IR usually pulses 100+ milliamps through the IR LED during the brief ON period.

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

A useful tool for debugging is the soundcard oscilloscope. Google this. The waveforms look bad due to the ac coupling, but the timing is very good. Great for this type of work as you can grab a near infinite amount of samples.

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

Simonetta wrote:
#define irREC PORTD2
#define irLED_PORT PORTD
#define irLED PORTD3

two symbols with the same definition?


What do you mean, they all look different to me?

There is no problem getting the ir codes, the problem is doing stuff with them. I could just quit now and save the raw timing values, but I want to be able to save more values later on so need to compress them a bit.

I have an oscilloscope which grabs the data, and that is how I know it´s not working. If i just pass out the raw data after the AVR has received it, then the waveform is the same on the scope as compared to remote control, so I know that bit works, it´s just when I try to mess with the data it doesn´t work. I should've mentioned that the low pulses part works. My code correctly recognises a "normal" low pulse and the few that are different, it´s just the high pulses that it doesn´t.

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

The key to packing 8 bits into a single byte is simply << or >>. You say it does not work but I see the code doing:

            if ((data[i] > (dataOne - 200)) && (data[i] < (dataOne + 200)))
            {
               dataHex[byteCounter] |= (1<<bitCounter);
            }

with bitcounter=0..7. This "looks" like it ought to work so what do you mean when you say "It doesn´t work, so now I am just focusing..". In what way does it not work?

Another approach would be to do the packing afterwards. So say you had:

uint8_t bits[] = { 0,1,1,1,0,1,0,1,  0,1,0,0,0,1,1,0};
uint8_t packed[2];
for (uint8_t bytes=0; bytes < 2; bytes++) {
  packed[bytes] = 0;
  for (uint8_t bit_count=0; bit_count < 8; bit_count++) {
     packed[bytes] <<= 1;
     if (bits[(bytes*8) + bit_count] == 1) {
       packed[bytes] |= 1;
     }
  }
}

That's untested but hopefully conveys the general idea of packing groups of 8 bits at a time into packed bytes? The key thing is that for each group of 8 you <<=1 the result each time (it starts with 0 so first shift does nothing) then add a 1 (OR) into the bottom bit if the corresponding unpacked byte is a 0x01.

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

clawson wrote:

This "looks" like it ought to work so what do you mean when you say "It doesn´t work, so now I am just focusing..". In what way does it not work?

The status LED blinks 20 times, which indicates that I have tried to save more than 20 "raw values". Looking at the code on the scope, there should be no more than 6 values that are not a regular 1 or 0, hence 6 raw values should be saved and rest should be either 1 or 0. I would therefore assume (which I probably shouldn´t) that it must something wrong in this section of the code,

if ((data[i] > (dataOne - 200)) && (data[i] < (dataOne + 200)))
{
 dataHex[byteCounter] |= (1<<bitCounter);
}

Or I am not finding what the "1" value is correctly with this code,

		while (dataOne == 0)
		{
		  if ((data[i] > (dataZero - 50)) && (data[i] < (dataZero - 50 )))
		  {
			  i++;
		  }
		  else 
		  {
		  	dataOne = data[i];
		  }		 	  
		}

Is there anything inherently wrong with what I have done here? data[i] is the raw timing code captured from the remote control earlier on. It is definitely correct, as if I just pass that straight out to the scope , it matches the code being inputted.

Quote:
That's untested but hopefully conveys the general idea of packing groups of 8 bits at a time into packed bytes? The key thing is that for each group of 8 you <<=1 the result each time (it starts with 0 so first shift does nothing) then add a 1 (OR) into the bottom bit if the corresponding unpacked byte is a 0x01.

Would you mind explaining this a little more please. I don't quite understand it.

Many thanks for the help.

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

Is a flashing LED your only debug aid? It's far easier if you have a real debugWire debugger or failing that at least output text message to an LCD or UART (if this is an Arduino that last one should be easy). Then you can print out key variables such as the count of bits received and count of packed bytes created etc.

Quote:

Would you mind explaining this a little more please. I don't quite understand it.

Sure, no problem. as I say this is just something I sketched out to show as an example so there could be an error there but what I'm showing is a test program as an example that has bits[] as an input where there are 16 bytes each holding either 0 or 1. The intention is to pack those 16 bytes into just 2 bytes of output (packed[]). The code treats the input as 2 lots of 8 bits. So the outer counter (bytes) counts the output bytes (packed[0] and packed[1]) and the inner counter counts the input bits (bit_count) in groups of 8.

Each time 8 bits are to be packed into one byte the code starts by setting the output byte (packed[bytes]) to be 0. It then enters the inner loop to process the next 8 bytes of input. It does:

packed[bytes] <<= 1;

so that any bits that are already "packed" within that byte are moved one place left towards the most significant bit. On the first iteration of the loop (because packed[bytes] was just set to 0 outside the loop) nothing happens because packed[bytes] holds 0 and shifting that one position left therefore does nothing.

I then have an if() test to see if the next bit of the input is 0 or 1. I just test bits[(bytes*8) + bit_count] because for the first group of 8 bits bytes=0 so it's simply bits[0..7]. Then when bytes=1 while packing the second byte it will be bits[8 + 0..7] that is considered. If the tested input byte is found to hold 1 then the lowest byte of the output that is currently being built is set to 1 too with:

packed[bytes] |= 1;

The bit_count loop now increments to 1 and the loop continues. Supposing that first bit tested was 1 then packed[bytes] will hold 0x01. It then does the <<=1 again and this time that bit will be moved one place to the left so it becomes 0x02. This leaves bit 0 free again so if the next input byte encountered is 1 then another 1 bit can be |='d into that lowest bit position. At the end of the loop, if that initial bit was 1 it will now have shifted right across the byte to be 0x80 with the possibility that other 1 bits have been OR'd in to the right of this.

Hope that helps.

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

clawson wrote:
Is a flashing LED your only debug aid? It's far easier if you have a real debugWire debugger or failing that at least output text message to an LCD or UART (if this is an Arduino that last one should be easy). Then you can print out key variables such as the count of bits received and count of packed bytes created etc.

I am afraid so. I agree though I definitely need a debugger, but finances are not good at the minute, so for now I need to struggle on with what I have. I am not using the Arduino, I am using a breadboarded ATMEGA328P at 8MhZ (external oscillator). That frequency works great and if I use the Arduino (at 16 MhZ) instead the timings get messed up and I end up with less optimal timings. I could however use the Arduino board without a microcontroller in and just use the USB part, so I can get serial data out and actually see what is going on. I will try that next, good idea, well done.

I think I understand what your code does. Basically I think you are doing the same as I was trying to do, but in a much more elegant way. One more question though, am I right in thinking that

packed[bytes] <<= 1;

Is the same as writing,

packed[bytes] = packed[bytes] << 1;

Thank you for the help. I will see if I can the serial data thing to work, and get back to you when I have some data.

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

Quote:

Is the same as writing,

It is. On the whole for all binary ops in C the construct

foo = foo op bar;

can be shortened to:

foo op= bar;

you are probably already familiar with:

foo = foo + 7;

being

foo += 7;

Similarly

foo = foo << 3;

is

foo <<= 3;

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

Thanks, I didn´t know that.

I am still looking in to the serial data thing, but something I have just thought about. Does it matter that this function is called by an ISR? I use the Timer overflow to tell when the remote code is finished, when it´s overflowed twice then it runs this decode function. Does that matter? If a function is called from within an ISR is it then part of the ISR and possibly why I am having problems?

// overflow event, two overflows in a row means the remote control is off
ISR(TIMER1_OVF_vect)
{
	if (has_overflowed == 1)
	{
		lengthOfData = bitnumber;  //Save how long the data is
		has_overflowed = 2; // send only once
		if (lengthOfData > 60) //If the data is longer than 60 (2 x 30 bits on/off) Send the code
		{ 
			decodeData();
		}  
		else //Otherwise clear the data and start again
		{
			resetData();
		}					
	}
	else
	{
		if (has_overflowed != 2){ has_overflowed = 1;}
	}
}
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ISRs should be lean and mean. For one thing when you call a function from one then the compiler cannot know what registers the called function may be using so the ISR prologue/epilogue has no choice but to stack/restore all the important registers adding about 35-30 cycles to the operation of the ISR.

Only put time critical stuff in the ISR - for the rest just set a global (volatile!) flag to say "stuff_read_to_decode" then back in main() loop processing monitor the flag and when it's seen to be set do that decoding.

(software is often better designed on paper then later implemented to design spec rather than just bolting everything on in your text editor and hoping the timing and everything might work out OK ;-))

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

Thanks so I should do it something like this instead?

volatile boolean run_decode_data;

ISR(TIMER1_OVF_vect) 
{ 
   if (has_overflowed == 1) 
   { 
      lengthOfData = bitnumber;  //Save how long the data is 
      has_overflowed = 2; // send only once 
      if (lengthOfData > 60) //If the data is longer than 60 (2 x 30 bits on/off) Send the code 
      { 
         run_decode_data = 1; 
      }  
      else //Otherwise clear the data and start again 
      { 
         resetData(); 
      }                
   } 
   else 
   { 
      if (has_overflowed != 2){ has_overflowed = 1;} 
   } 
}

void main()
{
some other code that does stuff

while(1)
 {
  some more code that does stuff

  if(run_decode_data == 1)
    {
      decodedata();
    }
 }
}

Then do the same with the reset function as well?

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

Quote:

if(run_decode_data == 1)
{
decodedata();
}


Don't forget to clear the flag when you act upon it.

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

I am still stuck but I have tracked down to where the bug is, I just don´t know why.

lowbit = data[2];
dataZero = data[1];
uint8_t j= 0; //used for the low raw code array
uint8_t k= 0; //used for the high raw code array
uint8_t bitCounter = 0; //Used to count which bit we are in for the code hex
uint8_t byteCounter = 0; //Used to count which byte we are in for the code hex
uint16_t i = 17;
				
		//We already know what 0 is as we guess its the data[1] value, so the next thing is to find out what a 1 is.
		//To do that we are going to start going through the data until we find a value that does not match the zero
		//value, and call that our 1 value.  We start at data[16] to miss all the odd values at the start of the code.
		
		while (dataOne == 0)
		{
		  if ((data[i] > (dataZero - 200)) && (data[i] < (dataZero - 200 )))
		  {
			  i+=2;
		  }
		  else 
		  {
		  	dataOne = data[i];
		  }		 	  
		}

The data array holds the timing data from the previously recorded remote data. The even number (i.e data[0], data[2], data[4], etc) are the low pulses and the odd numbers are the high pulses. What I have found is that no matter what I set i to, i becomes the dataOne value. So in other words the if statement fails no matter what. Up to i=21 should all be the same dataZero (which they are). I am sorry if that doesn´t make sense, but basically the array holds this data,

....data[2] = 380
....data[17] = 380
[18] = 380
[19] = 380
[20] = 380
[21] = 1200

So dataOne should be i=21=1200, but it´s not, it´s stopping on whatever the first check is, whatever I set i to.

I hope that makes sense and hope even more that it is just something stupid that I have done and is easy to fix.

Thanks

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

I figured it out, it was just a stupid mistake.

< (dataZero - 200 )))

should have been

< (dataZero + 200 )))

It works properly now, except that the first bit (data[0]) is wrong, so that´s my next mission find out why that is.

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

Ok some progress. I sort of "fixed" the issue of the data[0] problem. Basically w3hat was happening is that the the first bit sent was way too long and not the data[0] value. If you set it so that it just sent data[0] instead of incremanting (data[0], data[1], data[2], etc) then all every bit was correct except the first bit. Now it works from changing this,

ISR (TIMER1_COMPA_vect)
{
	if(setMode)
	{
		OCR1A = data[bitnumber]; // Set CTC compare value to the next data bit length
 		irLED_PORT ^= (1<<irLED);  //Toggle the ir led
 		bitnumber++;	 //Go to the next bit
	}
	else
	{
		qtrSecTimer++;  //Add 1 to the quarter second timer.	
	}		
}
void sendCode()  //TEMPORY FUNCTION FOR TESTING DATA
{
	
	setMode = 1;	//enable Set Mode
	
	bitnumber = 0;  //Start at the second bit, the first bit is sent manually, outside the ISR

	TCCR1B = 0;
	TIMSK1 = 0;
			
	TCCR1B = (1 << WGM12); // Configure timer 1 for CTC mode
	TIMSK1 = (1 << OCIE1A); // Enable CTC interrupt
	
	OCR1A   = data[0]; // Set CTC compare value to the first data bit length
	TCCR1B |= (1 << CS11); // Start timer at F_cpu/8
	
	sei(); //  Enable global interrupts
	
	setMode = 1;	//Enable Set Mode
	
	while (bitnumber <= lengthOfData);  //Do nothing until all the data is sent
	
	cli();
		
	resetData();
	
	statusLED_PORT |= 1<<statusLED; //Switch on the status LED
}
void LEDblink(uint8_t errorCode)
{
	qtrSecTimer = 0;
	
	TCCR1B |= (1 << WGM12); //Configure timer 1 for CTC mode
	TIMSK1 |= (1 << OCIE1A); //Enable CTC interrupt
	sei(); //Enable global interrupts
	OCR1A   = 31249; //Set CTC compare value to 0.25 seconds
	TCCR1B |= (1 << CS11) | (1 << CS10); //Start timer at F_cpu/64
	
	for (uint8_t i=0; i

And changing the sendCode()to this,

void sendCode()  //TEMPORY FUNCTION FOR TESTING DATA
{
	
	setMode = 0;	//disable Set Mode
	
	LEDblink(2);
	
	bitnumber = 0;  //Start at the second bit, the first bit is sent manually, outside the ISR

	TCCR1B = 0;
	TIMSK1 = 0;
			
	TCCR1B = (1 << WGM12); // Configure timer 1 for CTC mode
	TIMSK1 = (1 << OCIE1A); // Enable CTC interrupt
	
	OCR1A   = data[0]; // Set CTC compare value to the first data bit length
	TCCR1B |= (1 << CS11); // Start timer at F_cpu/8
	
	sei(); //  Enable global interrupts
	
	setMode = 1;	//Enable Set Mode
	
	while (bitnumber <= lengthOfData);  //Do nothing until all the data is sent
	
	cli();
		
	resetData();
	
	statusLED_PORT |= 1<<statusLED; //Switch on the status LED
}

the thing is I don´t know why this makes any difference and why it now works.

I have just tried a different remote control to the one I have been testing so far, to see if that works and I get the same problem with the first bit with the modified code. Except this time the first bit is way too short, the rest of the code is perfect, like on the other remote. This remote sends a completely different style of code from the one i've been using before.

Is there some trick with the timers that I am missing or something?

EDIT: I was wrong and got my traces mixed up on the scope. The second remote does exactly the same as the first remote. The first pulse is too long. It should be a 3.2mSec pulse, but it is outputting a 48mSec pulse. Every other pulse is correct.

Please, if anybody can help me I would really appreciate it. I don't know what else to do now, where to look for the error or anything.