Please suggest a code which will produce square waves of 7 different frequencies at a particular pin depending on input at a different port.Its URGENT.Need to complete work by tomorrow.
radbuz 
Square wave generation avr.
Author
Message
radbuz MBedder New forum guidelines do not allow us to answer such questions, sorry :lol:
radbuz If thats the case then i've made a code.Can you please help in correcting it because once i put an input at a particular pin the same square wave keeps on running even if i give input at some other pin
radbuz #define F_CPU 1000000 #include#include #include #include void A880(void){ TIMSK|=(1<<OCIE1A); TCNT1=0; OCR1A=142; } void G784(void){ TIMSK|=(1<<OCIE1A); TCNT1=0; OCR1A=160;} void B988(void){ TIMSK|=(1<<OCIE1A); TCNT1=0; OCR1A=126;} void C523(void){ TIMSK|=(1<<OCIE1A); TCNT1=0; OCR1A=238;} void D587(void){ TIMSK|=(1<<OCIE1A); TCNT1=0; OCR1A=212;} void E659(void){ TIMSK|=(1<<OCIE1A); TCNT1=0; OCR1A=190;} void F698(void) {PORTB=0xFF; TIMSK|=(1<<OCIE1A); TCNT1=0; OCR1A=178;} void AB(void) { TIMSK|=(1<<OCIE1A); TCNT1=0; OCR1A=134;} void GA(void) { TIMSK|=(1<<OCIE1A); TCNT1=0; OCR1A=150;} void FG(void) { TIMSK|=(1<<OCIE1A); TCNT1=0; OCR1A=158;} void DE(void) {PORTB=0xFF; TIMSK|=(1<<OCIE1A); TCNT1=0; OCR1A=200;} void CD(void) { TIMSK|=(1<<OCIE1A); TCNT1=0; OCR1A=226;} int main(){ DDRB|=0xFF; DDRA|=0x00; PORTA=0x00; PORTB=0XFF; PINA=0X00; TCCR1A|=(1<<COM1A1); TCCR1B|=(1<<WGM12)|(1<<CS11); if (PINA & 0b00000001){A880();} else if(PINA & 0b00000010){B988();} else if(PINA & 0b00000100){C523();} else if(PINA & 0b00001000){D587();} else if(PINA & 0b00010000){E659();} else if(PINA & 0b00100000){F698();} else if(PINA & 0b01000000){G784();} else if(PINA & 0b01100000){AB();} else if(PINA & 0b01000001){GA();} else if(PINA & 0b00000011){FG();} else if(PINA & 0b00001100){DE();} else if(PINA & 0b00011000){CD();} sei(); while(1); return 0; } ISR (TIMER1_COMPA_vect){ PORTB =~ PORTB; }
MBedder Please do not cross post. Thank you so much.
radbuz I'm frustrated as my code is not working.Thats the reason for the cross post.I'm Sorry.Please help me.
Kartman
Encase your if (pina ...... ) else .... In the while(1) loop. Your code is currently working as you wrote it- it reads the pins, sets the frequency then sits in the while(1) loop forever.
MartinM57
...and two minor points for extra marks:
DDRB|=0xFF;
DDRA|=0x00;
should really be
DDRB=0xFF;
DDRA=0x00;
(OR'ing something with 0xFF sets it to 0xFF; OR'ing something with 0x00 leaves it unchanged. You want to explicitly set the registers to 0xFF and 0x00 respectively, so just do that)
Delete
return 0;
(where are you trying to returning to?)
(how do you think you will get to that line with the preceding while (1) statement?)
david.prentice #define F_CPU 1000000 #include#include #include #include void A880(void) { TIMSK |= (1<<OCIE1A); TCNT1 = 0; OCR1A = 142; } void G784(void) { TIMSK |= (1<<OCIE1A); TCNT1 = 0; OCR1A = 160; } void B988(void) { TIMSK |= (1<<OCIE1A); TCNT1 = 0; OCR1A = 126; } void C523(void) { TIMSK |= (1<<OCIE1A); TCNT1 = 0; OCR1A = 238; } void D587(void) { TIMSK |= (1<<OCIE1A); TCNT1 = 0; OCR1A = 212; } void E659(void) { TIMSK |= (1<<OCIE1A); TCNT1 = 0; OCR1A = 190; } void F698(void) { PORTB = 0xFF; TIMSK |= (1<<OCIE1A); TCNT1 = 0; OCR1A = 178; } void AB(void) { TIMSK |= (1<<OCIE1A); TCNT1 = 0; OCR1A = 134; } void GA(void) { TIMSK |= (1<<OCIE1A); TCNT1 = 0; OCR1A = 150; } void FG(void) { TIMSK |= (1<<OCIE1A); TCNT1 = 0; OCR1A = 158; } void DE(void) { PORTB = 0xFF; TIMSK |= (1<<OCIE1A); TCNT1 = 0; OCR1A = 200; } void CD(void) { TIMSK |= (1<<OCIE1A); TCNT1 = 0; OCR1A = 226; } int main() { DDRB |= 0xFF; DDRA |= 0x00; PORTA = 0x00; PORTB = 0XFF; PINA = 0X00; TCCR1A |= (1<<COM1A1); TCCR1B |= (1<<WGM12)|(1<<CS11); if (PINA & 0b00000001) { A880(); } else if (PINA & 0b00000010) { B988(); } else if (PINA & 0b00000100) { C523(); } else if (PINA & 0b00001000) { D587(); } else if (PINA & 0b00010000) { E659(); } else if (PINA & 0b00100000) { F698(); } else if (PINA & 0b01000000) { G784(); } else if (PINA & 0b01100000) { AB(); } else if (PINA & 0b01000001) { GA(); } else if (PINA & 0b00000011) { FG(); } else if (PINA & 0b00001100) { DE(); } else if (PINA & 0b00011000) { CD(); } sei(); while (1); return 0; } ISR(TIMER1_COMPA_vect) { PORTB = ~PORTB; }
Others have pointed out some minor points. You understand how a hardware timer works.
I have indented your code for readability. You should be able to see your program structure and flow, and make appropriate corrections.
You might also look for 'similarities' in your functions. Can you see how to make things simpler and tidier?
I understand what your program is doing. Can your teacher ?
If you add some brief explanatory comments and present attractive code, you will go to the top of your class.
You are clearly in a different league to the idle students who do not make any effort at all.
David.
bobgardner Gee, I answered that question already, but I don't see my answer here? Maybe the sysops thought it was US politics?
david.prentice Bob,
Didn't you notice that those mysterious function names GA(), FG(), CD(), ... are all part of a secret cipher ?
The conspiracy continues.
David.
radbuz I have tried correcting all the mistakes suggested by kartman and martin.However after burning the code on hardware when i run it it runs ok till i give input from 1 sensor but as soon as i give input from a different sensor the sound doesn't change at all.What could be the problem.The code seems to be alright.
david.prentice Have you really corrected the mistake suggested by kartman?
Post only the main() function when you have checked it.
David.
bobgardner If you read the PINA in the while loop, you can stick a 0 in the TCCR1B to shut of the timer if the PINA == 0xff
radbuz
int main(){
DDRB=0xFF;
DDRA=0x00;
PORTA=0x00;
PINA=0x00;
TCCR1A|=(1<<COM1A1);
TCCR1B|=(1<<WGM12)|(1<<CS11);
sei();
while(1){
if (PINA & 0b00000001){A880();}
else if(PINA & 0b00000010){B988();}
else if(PINA & 0b00000100){C523();}
else if(PINA & 0b00001000){D587();}
else if(PINA & 0b00010000){E659();}
else if(PINA & 0b00100000){F698();}
else if(PINA & 0b01000000){G784();}
else if(PINA & 0b01100000){AB();}
else if(PINA & 0b01000001){GA();}
else if(PINA & 0b00000011){FG();}
else if(PINA & 0b00001100){DE();}
else if(PINA & 0b00011000){CD();}
else if (PINA & 0b00000000){TIMSK=(0<<OCIE1A);}
}
}
MartinM57
So maybe you have to debug in stages rather than try the whole thing first time and not know what works and what doesn't?
So take out the PIN testing for a moment - what does this do?
int main(){
DDRB=0xFF;
DDRA=0x00;
PORTA=0x00;
PINA=0x00;
TCCR1A|=(1<<COM1A1);
TCCR1B|=(1<<WGM12)|(1<<CS11);
sei();
while(1){
A880();
delay_ms (500);
B988();}
delay_ms (500);
C523();}
delay_ms (500);
D587();}
delay_ms (500);
E659();}
delay_ms (500);
F698();}
delay_ms (500);
G784();}
delay_ms (500);
AB();}
delay_ms (500);
GA();}
delay_ms (500);
FG();}
delay_ms (500);
DE();}
delay_ms (500);
CD();}
delay_ms (500);
}
}
..then build the code back up from a working version of this.
Last Edited: Sat. Feb 18, 2012 - 04:46 PM
bobgardner I think the check for all inputs off doesnt use the AND. More like if(PINA == 0xFF). And it looks like you are are trying to turn off the interrupt by multiplying a 0 by something. This setting bits by shifting is just a confusing distraction for beginners. There is no sense in using it until 6 months from now when you wonder what bits are set when you store a 0x37 in the register. If you want to turn off the timer, I'd stick a 0x00 in the timer control register. See where I said that in my message? I said it because I meant it. I usually don't talk just to hear my head roar.
Koshchi
else if(PINA & 0b01100000){AB();}
else if(PINA & 0b01000001){GA();}
else if(PINA & 0b00000011){FG();}
else if(PINA & 0b00001100){DE();}
else if(PINA & 0b00011000){CD();}
The functions in these ifs will never be run as one of the previous ifs will have been true.
else if (PINA & 0b00000000){TIMSK=(0<<OCIE1A);}
This if can never be true under any circumstances.
You seriously need a refresher course in Bit Manipulation.
theusch 
It is an interesting assignment. Our comments now are probably too late. ;)
Quote:
Was the assignment really only given like yesterday? :twisted:
URGENT.Need to complete work by tomorrow.
Anyway Steve pointed out why comparing the masks must be done properly. While perhaps not critical to this assignment, buttons are going to bounce so there is going to be some stuttering operation from the "keyboard". Also, turning of the interrupt mask will leave the output pins either all high or all low. (Why is OP toggling all of PORTB when the assignment calls for a pin?)
This is a very cute assignment for an AVR. Here is how I would do it, using two timers. Hmm, we don't know which AVR model...
-- Set up one 8-bit timer with about a 10ms heartbeat.
-- Set up a 16-bit timer in CTC mode with output on OC1A.
-- Every 10ms, gather the 8 inputs from PINA and debounce in parallel.
-- Then do a switch() on the 12/15 tones desired using the mask values.
Set OCR1A accordingly.
Now, the only tricky part is "no key pressed". I might even handle that outside the switch(). Pseudo code:
if (switch_current == 0)
{
// no keys pressed
turn off timer1
disconnect the pin from the timer
set output pin low
}
else
{
connect the output pin to the timer
start the timer
do the switch() on the key-bank value, setting OCR1A
** in the default: case, it is an invalid switch combination
turn off the timer as if there was no key pressed
}
radbuz @Martin:The part of the code you suggested is working perfectly.Please tell me what to do next.
bobgardner Lee, forget about switch bounce when helping a new guy. Just put a darn 10ms delay in the loop and it will never be a problem. You cant peck a button faster than about 20ms, and you couldnt hear the 1ms bounce for 1 or 2 bounces over a bounce period of a couple ms anyway. Its an organ! The multi $1000 Kurzweil synthesizers laboriously try to reproduce the switch click of the Hammond B3 using multimegahertz digital signal processors. Simple is good usually. Push button, hear tone.
radbuz
int main(){
DDRB=0xFF;
DDRA=0b10000000;
PORTA=0b01111111;
PORTB=0xFF;
TCCR1A|=(1<<COM1A1);
TCCR1B|=(1<<WGM12)|(1<<CS11);
sei();
if (PINA==0b00000001){A880();}
else if (PINA==0b00000010){B988();}
else if(PINA==0b00000100){C523();}
else if(PINA==0b00001000){D587();}
else if (PINA==0b00010000){E659();}
else if(PINA==0b00100000){F698();}
else if(PINA==0b01000000){G784();}
else if(PINA==0b00000011){AB();}
else if(PINA==0b01000001){GA();}
else if(PINA==0b01100000){FG();}
else if(PINA==0b00011000){DE();}
else if(PINA==0b00001100){CD();}
while(1);
}
ISR (TIMER1_COMPA_vect){
PORTB =~ PORTB;
}
This is my modified main function but it still isn't working.Sorry to bother you all but i'm just a beginner at the moment so kindly cooperate with me :)
MartinM57
Until you move the IF into the WHILE loop (again :() you will only get one frequency. So if you leave the code as is, and restart the program with different logic levels on Port A, what happens?
How are you wiring Port A? What is connected to it and how?
david.prentice I suggest that you start by indenting your code and adding explanatory comments.
You can then refer to the statements or behaviour that you don't understand.
Your original code implied that you had a very good knowledge. Perhaps you just pasted blocks of code !
Your last post shows that you haven't a clue.
We are happy to help out those people who show willing.
Incidentally, in real life you normally wire switches active-low. i.e. you use internal pull-up resistors and when you press the switch, it makes to GND. Can we assume you are using Proteus? And the project is 'virtual'.
You might look in your data sheet for 'OC1A pin', 'COM1A1'.
David.
radbuz Thats jst a slightly old piece of code i had pasted.Actually everything is inside the while loop only.I have 7 ir sensors which are connected to the 7 pins of portA from PA0-PA6.Whenevr i bring my finger infront of a sensor the appropriate tune should play on PORTB.This is basically my project
radbuz Yes,David i am using proteus but i am really confused regarding input-output pins and these internal pull up resistors.Thats why i'm making a mess in the code i guess.
I didn't paste any code. :(
david.prentice If your sensors are active-high, then you were reading correctly in your very first post.
Seriously, it takes zero brain-power to indent your code neatly. It makes a massive difference to your readability.
Likewise, comments are also for your benefit. However your teacher will be very impressed.
You will make a good impression here too !
No-one will write your code for your school project. But if you show effort, you will get good assistance.
David.
radbuz Yes my sensors are active high ones.Ok,then i will be reverting back to the original code and testing.Thank you.
bobgardner try this
//file piano.c #define F_CPU 1000000 #include#include #include #include #define A880 142 #define G784 160 #define B988 126 #define C523 238 #define D587 212 #define E659 190 #define F698 178 #define AB 134 #define GA 150 #define FG 158 #define DE 200 #define CD 226 //----------------------------- void initports(void){ DDRB |= 0xFF; DDRA |= 0x00; PORTA = 0x00; PORTB = 0XFF; PINA = 0X00; TCCR1A |= (1<<COM1A1); TCCR1B |= (1<<WGM12)|(1<<CS11); TIMSK |= (1<<OCIE1A); } char inputdat[13]={0,1,2,3,4,0x10,0x20,0x40,0x60,0x41,0x03,0x0c,0x18}; //---------------------------- int main(void) { char pina; initports(); while(1){ pina=PINA; switch(pina){ case 0x00: OCR1A=0; break; case 0x01: OCR1A=A880; break; case 0x02: OCR1A=B988; break; case 0x03: OCR1A=C523; break; case 0x04: OCR1A=D587; break; case 0x10: OCR1A=E659; break; case 0x20: OCR1A=F698; break; case 0x40: OCR1A=G784; break; case 0x60: OCR1A=AB; break; case 0x41: OCR1A=GA; break; case 0x03: OCR1A=FG; break; case 0x0c: OCR1A=DE; break; case 0x18: OCR1A=CD; break; }//switch delay_ms(10); }//while return 0; } ISR(TIMER1_COMPA_vect) { PORTB = ~PORTB; } //----------------------eof---------------------------
theusch Bob, how are you going to service a compare interrupt every two AVR clock cycles when OCR1A is 0?
bobgardner OK, I'll keep working on it. Maybe I should follow my own advice and just stuff a 00 in the timer control register. Or do a CLI if nothing is pressed.
radbuz @Bob:Thank you very much for your fruitful help.I couldn't have completed this project without your help.I would be very grateful if you could do me a last favour.Can you tell me the code to write when no input is there.Stuffing a 00 in TCCR 1B is not working and it makes this annoying noise when no input is connected.Thank you again :)
bobgardner If you set the timer up to toggle the output pin, you'll always get a 50% duty on the OCR pin. If you hook the RC to PD5 (thats OC1A on a mega32), you dont need the interrupt enable or handler. Then I think setting the OCR1A to 0x0000 might actually work as expected. That example I posted up there was not run on hardware I'm sorry to say. Notice I didn't use that table I created. I guess you might also notice I forgot to enable interrupts, so if its working without them, I guess we don't need them.
radbuz ok i'll try that.Thanks.
radbuz So many people have helped me so its my duty to help students who will be doing such kinds of projects in future.So here is the final working code.
#define F_CPU 1000000 #include#include #include #include #define A880 142 #define G784 160 #define B988 126 #define C523 238 #define D587 212 #define E659 190 #define F698 178 #define AB 134 #define GA 150 #define FG 158 #define DE 200 #define CD 226 //----------------------------- void initports(void){ DDRB |= 0xFF; DDRA |= 0x00; PORTA = 0x00; PORTB = 0XFF; PINA = 0X00; TCCR1A |= (1<<COM1A1); TCCR1B |= (1<<WGM12)|(1<<CS11); TIMSK |= (1<<OCIE1A); } //---------------------------- int main(void) { char pina; sei(); initports(); while(1){ pina=PINA; switch(pina){ case 0x00: TIMSK=(0<<OCIE1A);break; case 0x01: OCR1A=A880; break; case 0x02: OCR1A=B988; break; case 0x04: OCR1A=C523; break; case 0x08: OCR1A=D587; break; case 0x10: OCR1A=E659; break; case 0x20: OCR1A=F698; break; case 0x40: OCR1A=G784; break; case 0x03: OCR1A=AB; break; case 0x41: OCR1A=GA; break; case 0x60: OCR1A=FG; break; case 0x18: OCR1A=DE; break; case 0x0C: OCR1A=CD; break; }//switch _delay_ms(10); }//while return 0; } ISR(TIMER1_COMPA_vect) { PORTB = ~PORTB; } //----------------------eof---------------------------
david.prentice Let us know how many marks your teacher gave you for Bob's work.
David.
bobgardner I guess that 0 << OCIE1A works by accident, but shifting a zero is a fairly vague mathematical concept. To clear a bit one must AND the register with a mask of all 1s and a 0 in the bit position of the bit to be reset/cleared. Something like TIMSK &= 0b11101111 if you happen to remember that OCIE1A is this bit -> 0001 0000. If you havent memorized all the bit positions in all the registers, you can use the shift trick -> TIMSK &= ~(1 << OCIE1A). See how it shifts the one over to the correct bit, then flips the whole byte upside down, then ANDs it into the register?
John_A_Brown
Level: Raving Lunatic
Joined: Sat. Sep 20, 2003
Posts: 6203 View posts
Location: Underneath the Cotswolds escarpment, England.
What happens if Bob's code is failed?
Does he get kicked off the forum?
MartinM57
...still OR'ing in initport() rather than assigning and not a (useful) comment in sight...
...and a sledgehammer approach to treating the whole of PORTB at once rather than the original requirement of "...produce square waves of 7 different frequencies at a particular pin..."