| Author |
Message |
|
|
 Posted: Jul 08, 2012 - 07:58 AM |
|
Joined: Jul 08, 2012
Posts: 8
|
|
I am working on an LED. I am now able to control the marquee of the LED. But when i want use switches to control the LED ,it doesnt respond according to the switches.
this is my code:-
Quote:
#include <avr/io.h>
int main( void )
{
DDRD = 0x00;
DDRB = 0xFF;
switch (PIND)
{
case '0x01':
PORTB = 0x01;
case '0xfe':
PORTB = 0x02;
case '0x04':
PORTB = 0x04;
case '0x08':
PORTB = 0x08;
case '0x10':
PORTB = 0x10;
case '0x20':
PORTB = 0x20;
case '0x40':
PORTB = 0x40;
case '0x80':
PORTB = 0x80;
default:
PORTB = 0x00;
}
return 0;
}
Can anyone help to figure out what is wrong with the code please?
i am using STK500 and ATMEGA168 to do the work. |
|
|
| |
|
|
|
|
|
|
Posted: Jul 08, 2012 - 08:21 AM |
|
Joined: Dec 06, 2007
Posts: 2512
Location: Redmond, WA USA
|
|
| Your program falls right out the bottom with the return 0; statement and terminates. You need to enclose your switch and case statements within a while loop. Even then it will not work as coded. |
_________________
Larry
Those afraid to embrace the future will quickly fade into the past. - larryvc
Last edited by larryvc on Jul 08, 2012 - 08:26 AM; edited 1 time in total
|
| |
|
|
|
|
|
|
Posted: Jul 08, 2012 - 08:24 AM |
|
Joined: Feb 12, 2005
Posts: 16254
Location: Wormshill, England
|
|
|
mochi321 wrote:
I am working on an LED. I am now able to control the marquee of the LED. But when i want use switches to control the LED ,it doesnt respond according to the switches.
this is my code:-
Quote:
Code:
#include <avr/io.h>
int main( void )
{
DDRD = 0x00;
DDRB = 0xFF;
switch (PIND)
{
case '0x01':
PORTB = 0x01;
case '0xfe':
PORTB = 0x02;
case '0x04':
PORTB = 0x04;
case '0x08':
PORTB = 0x08;
case '0x10':
PORTB = 0x10;
case '0x20':
PORTB = 0x20;
case '0x40':
PORTB = 0x40;
case '0x80':
PORTB = 0x80;
default:
PORTB = 0x00;
}
return 0;
}
Can anyone help to figure out what is wrong with the code please?
i am using STK500 and ATMEGA168 to do the work.
The switch() statement normally uses a break; between every case clause.
The case takes a numeric value like 0x41 or 'A'.
'0x41' is not a valid expression.
It you trace your program by hand, you will see that for any value of PIND it will end up with the default case. i.e. all LEDs on.
Also note that your fingers will struggle to achieve any value like 0x80. You need to press all the switches SW0..SW6 while starting your program.
A pressed switch is 0 and not 1.
You normally use 'internal pull-ups' on a pin that is connected to a switch. The STK500 already has some external pull-ups (I think).
If you put the switch inside a while () loop, it will work continuously. Your program just reads the value of PIND at program start-up.
David. |
|
|
| |
|
|
|
|
|
|
Posted: Jul 09, 2012 - 08:47 AM |
|
Joined: Jul 08, 2012
Posts: 8
|
|
Hi David, I follow your suggestion to write this code is
Code:
#include <avr/io.h>
int main( void )
{
DDRD = 0x00;
DDRB = 0xFF;
while(1)
{
switch (PIND)
{
case 'FE':
PORTB = 0x01;
break;
case 'FD':
PORTB = 0x02;
break;
case 'FA':
PORTB = 0x04;
break;
case 'F8':
PORTB = 0x08;
break;
case 'EF':
PORTB = 0x10;
break;
case 'DF':
PORTB = 0x20;
break;
case 'AF':
PORTB = 0x40;
break;
case '8F':
PORTB = 0x80;
break;
default:
PORTB = 0xff;
break;
}
}
}
$ Fixed CODE tags - JS $
but it still doesn't respond according to the switches.
Can you help to figure out what is wrong with the code or would it be possible for me to have some sample code from you please? |
|
|
| |
|
|
|
|
|
|
Posted: Jul 09, 2012 - 08:55 AM |
|
Joined: Jan 08, 2009
Posts: 1148
Location: Lund, Sweden
|
|
Did you get any warnings?
Try something like this:
Code:
switch (PIND)
{
case 0xFE:
PORTB = 0x01;
break;
case 0xFD:
PORTB = 0x02;
break;
...
|
|
|
| |
|
|
|
|
|
|
Posted: Jul 09, 2012 - 09:06 AM |
|
Joined: Mar 28, 2001
Posts: 20311
Location: Sydney, Australia (Gum trees, Koalas and Kangaroos, No Edelweiss)
|
|
| mochi321 please use the CODE button when posting code, I have fixed the post above. |
_________________
John Samperi
Ampertronics Pty. Ltd.
www.ampertronics.com.au
* Electronic Design * Custom Products * Contract Assembly
|
| |
|
|
|
|
|
|
Posted: Jul 09, 2012 - 09:23 AM |
|
Joined: Jul 08, 2012
Posts: 8
|
|
For snigelen
I get the warnings to following:
../LED_switch.c:12:9: warning: character constant too long for its type
../LED_switch.c: In function 'main':
../LED_switch.c:12: warning: case label value exceeds maximum value for type
../LED_switch.c:15:9: warning: character constant too long for its type
../LED_switch.c:15: warning: case label value exceeds maximum value for type
I try your suggestion ,but i get the same result. |
|
|
| |
|
|
|
|
|
|
Posted: Jul 09, 2012 - 09:31 AM |
|
Joined: Sep 19, 2005
Posts: 765
Location: Belgium
|
|
|
Quote:
For snigelen
I get the warnings to following:
../LED_switch.c:12:9: warning: character constant too long for its type
../LED_switch.c: In function 'main':
../LED_switch.c:12: warning: case label value exceeds maximum value for type
../LED_switch.c:15:9: warning: character constant too long for its type
../LED_switch.c:15: warning: case label value exceeds maximum value for type
I try your suggestion ,but i get the same result.
From those warnings it is clear you did not remove the quotes.
Lose the quotes, you cannot compare strings in a switch statement anyway.
single quotes are used for single characters, double quotes for strings. You want a numeric value that you represent in hexadecimal, so only use the 0x prefix.
change
Code:
case 'FE':
to
Code:
case 0xFE:
as was explained by David, and repeated by snigelen.
david.prentice wrote:
The case takes a numeric value like 0x41 or 'A'.
'0x41' is not a valid expression.
|
|
|
| |
|
|
|
|
|
|
Posted: Jul 09, 2012 - 10:01 AM |
|
Joined: Jul 18, 2005
Posts: 62209
Location: (using avr-gcc in) Finchingfield, Essex, England
|
|
|
Quote:
For snigelen
I get the warnings to following:
../LED_switch.c:12:9: warning: character constant too long for its type
Did that not make you wonder "why is it telling me that the character constant is too long?". Did it not prompt you to take a look at your C manual and find out how to specify integer constants. All these are valid (in GCC)
1
0b1011
037
0x4F
'A'
'\r'
While none of these are
4F
'hello'
'FA'
'037'
'0x47'
If you don't understand why this is it's time to hit the books and brush up on some fundamentals of C programming.
Anyway it seems like madness to try and act on all 8 bits of PIND at once. You are likely only interested in whether one button (one bit) is set or clear at any one time so I'd do something like:
uint8_t switch;
Code:
switch = PIND;
if (switch & (1<<0)) {
// do stuff for button 0 being pressed
}
if (switch & (1<<1)) {
// do stuff for button 1 being pressed
}
if (switch & (1<<2)) {
// do stuff for button 2 being pressed
}
if (switch & (1<<3)) {
// do stuff for button 3 being pressed
}
etc.
For the STK500 where the buttons are active low then you would actually invert that using C's NOT operator:
Code:
switch = PIND;
if (!(switch & (1<<0))) {
// do stuff for button 0 being pressed
}
if (!(switch & (1<<1))) {
// do stuff for button 1 being pressed
}
if (!(switch & (1<<2))) {
// do stuff for button 2 being pressed
}
if (!(switch & (1<<3))) {
// do stuff for button 3 being pressed
}
etc.
I wouldn't have said that either a switch()/case or if()/else if()/else if() construct would be appropriate here as you might want to act on the fact that both button 0 and button 3 are both pressed. Those two constructs tend to imply you are only willing to consider one of the 8 buttons being pressed. |
_________________
|
| |
|
|
|
|
|
|
Posted: Jul 09, 2012 - 11:00 AM |
|
Joined: Jul 08, 2012
Posts: 8
|
|
My revised code:
#include <avr/io.h>
Code:
int main( void )
{
DDRD = 0x00;
DDRB = 0xFF;
while(1)
{
switch (PIND)
{
case 0xFE:
PORTB = 0x01;
break;
case 0xFD:
PORTB = 0x02;
break;
case 0xFA:
PORTB = 0x04;
break;
case 0xF8:
PORTB = 0x08;
break;
case 0xEF:
PORTB = 0x10;
break;
case 0xDF:
PORTB = 0x20;
break;
case 0xAF:
PORTB = 0x40;
break;
case 0x8F:
PORTB = 0x80;
break;
default:
PORTB = 0xff;
break;
}
}
}
the code don't has any error and warning,
but it doesn't respond according to the switches. |
|
|
| |
|
|
|
|
|
|
Posted: Jul 09, 2012 - 11:20 AM |
|
Joined: Feb 12, 2005
Posts: 16254
Location: Wormshill, England
|
|
|
Code:
#include <avr/io.h>
int main(void)
{
DDRD = 0x00; // switches are input
PORTD = 0xFF; // internal pull-ups on switches.
DDRB = 0xFF; // LEDs are output
while (1) // loop forever
{
switch (~PIND) // human readable switches
{
case 0x01:
PORTB = ~0x01; // active-low LEDs
break;
default:
PORTB = ~0x00; // active-low
break;
}
}
}
It takes a little while to get your head around active-low. Personally, I just work in human, and then invert the result when I write to the PORT.
Likewise, I read the switches and turn them into human (with the ~ operator).
Note that you can read several switches at a time as a separate case. i.e. all 256 possible finger combinations of the 8 push-buttons.
If you just want to consider switches as 8 different combinations, you use the if ((PIND & 0x01) == 0) style.
Again, I would use a human_switch = ~PIND variable.
Then you can use if (human_switch & 0x01) ...
David. |
|
|
| |
|
|
|
|
|
|
Posted: Jul 09, 2012 - 12:09 PM |
|
Joined: Sep 05, 2001
Posts: 2496
|
|
|
clawson wrote:
I wouldn't have said that either a switch()/case or if()/else if()/else if() construct would be appropriate here as you might want to act on the fact that both button 0 and button 3 are both pressed. Those two constructs tend to imply you are only willing to consider one of the 8 buttons being pressed.
But it's an impossible task to press two keys simultaneous from the CPU view 
Since no human was able to press both within the same microsecond or so.
Thus CPU see always a single press at first.
Also testing only a single bit would be smaller and faster code, since then the compiler can use the bit instructions (SBIS, CBIS).
Peter |
|
|
| |
|
|
|
|
|
|
Posted: Jul 09, 2012 - 12:13 PM |
|
Joined: Jul 18, 2005
Posts: 62209
Location: (using avr-gcc in) Finchingfield, Essex, England
|
|
|
Quote:
But it's an impossible task to press two keys simultaneous from the CPU view Exclamation
Since no human was able to press both within the same microsecond or so.
Eh? On my STK500 I can happily hold down 3/4/5 of the buttons at the same time? In fact if I really tried I reckon I could hold down all 8 at the same time.
EDIT: no I tried it about 7 is the most I can press at once as need a finger/thumb to grip the STK500 board itself while I'm trying to do this. |
_________________
|
| |
|
|
|
|
|
|
Posted: Jul 09, 2012 - 01:01 PM |
|
Joined: Sep 05, 2001
Posts: 2496
|
|
|
clawson wrote:
Eh? On my STK500 I can happily hold down 3/4/5 of the buttons at the same time?
I meant not the pressed state.
I meant the moment, when the key was closed.
The CPU see at first only a single key press, then two, then three and so on.
Since you can not press all keys at the same time.
There are several milliseconds difference.
Peter |
|
|
| |
|
|
|
|
|
|
Posted: Jul 09, 2012 - 01:03 PM |
|
Joined: Jul 18, 2005
Posts: 62209
Location: (using avr-gcc in) Finchingfield, Essex, England
|
|
| Yeah but OP is sitting in a while() loop polling PIND. My contention is that on any of the reads of PIND either 0 or 1 or 2 or 3 or 4 or 5 or 6 or 7 or 8 buttons may be found to be pressed. If the code processing that PIND reading just handled the first button it found pressed and stopped it would miss the fact that others were also being held. |
_________________
|
| |
|
|
|
|
|
|
Posted: Jul 09, 2012 - 01:11 PM |
|
Joined: Jul 08, 2012
Posts: 8
|
|
For David
I try your code,but it still doesn't respond according to the switches.
so I maybe to try "if" to revised the code |
|
|
| |
|
|
|
|
|
|
Posted: Jul 09, 2012 - 01:41 PM |
|
Joined: Feb 12, 2005
Posts: 16254
Location: Wormshill, England
|
|
Oops. I should have tested the code that I posted.
Code:
switch (~PIND & 0xFF) // human readable switches
What happens with a straight ~PIND is that you get PIND extended to an int before the NOT is applied. Hence 0xFE is extended to 0x00FE and when inverted you get 0xFF01.
I am sorry for complicating matters. If you use an 'unsigned char' for the state of the switches:
David.
Code:
int main(void)
{
unsigned char human;
DDRD = 0x00; // switches are input
PORTD = 0xFF; // internal pull-ups on switches.
DDRB = 0xFF; // LEDs are output
while (1) // loop forever
{
human = ~PIND; // don't need to worry about int extension.
switch (human) // human readable switches
{
case 0x01:
PORTB = ~0x01; // active-low LEDs
break;
default:
PORTB = ~0x00; // active-low
break;
}
}
}
|
|
|
| |
|
|
|
|
|
|
Posted: Jul 09, 2012 - 03:03 PM |
|
Joined: Sep 05, 2001
Posts: 2496
|
|
|
mochi321 wrote:
For David
I try your code,but it still doesn't respond according to the switches.
so I maybe to try "if" to revised the code
That's why you still not follow the suggestions to test only a single pin instead the whole byte. 
Especially on the ATmega168 PORTB may be no fully accessible.
And if accessible, then on the STK500 PB6,7 are routed to the XTAL1,2 connector.
Thus checking the bit was always the favoured way to read a key.
Peter |
|
|
| |
|
|
|
|
|
|
Posted: Jul 10, 2012 - 08:37 AM |
|
Joined: Jul 08, 2012
Posts: 8
|
|
My code finally works!
Thanks everyone for your kind help, I greatly appreciate it.
My code is
Code:
#include <avr/io.h>
int main(void)
{
unsigned char human;
DDRD = 0x00; // switches are input
PORTD = 0xFF; // internal pull-ups on switches.
DDRB = 0xFF; // LEDs are output
while (1) // loop forever
{
human = ~PIND; // don't need to worry about int extension.
if (human == 0x01)
PORTB = ~0x01;
else if (human == 0x02)
PORTB = ~0x02;
else if (human == 0x04)
PORTB = ~0x04;
else if (human == 0x08)
PORTB = ~0x08;
else if (human == 0x10)
PORTB = ~0x10;
else if (human == 0x20)
PORTB = ~0x20;
else if (human == 0x40)
PORTB = ~0x0F;
else if (human == 0x80)
PORTB = ~0x30;
else
PORTB = ~0x3f;
}
}
|
|
|
| |
|
|
|
|
|
|
Posted: Jul 10, 2012 - 09:00 AM |
|
Joined: Feb 12, 2005
Posts: 16254
Location: Wormshill, England
|
|
Your if else-if structure where you test for specific values with == is exactly the same as using switch - case - default.
If you use another variable, it means you can keep your human brain at room temperature:
Code:
int main(void)
{
unsigned char human, leds;
DDRD = 0x00; // switches are input
PORTD = 0xFF; // internal pull-ups on switches.
DDRB = 0xFF; // LEDs are output
while (1) // loop forever
{
human = ~PIND; // don't need to worry about int extension.
switch (human) // human readable switches
{
case 0x01:
leds = 0x01; // keep regular logic
break;
case 0x02:
leds = 0x02; // keep regular logic
break;
case 0x04:
leds = 0x04; // keep regular logic
break;
case 0x08:
leds = 0x08; // keep regular logic
break;
case 0x10:
leds = 0x10; // keep regular logic
break;
case 0x40:
leds = 0x0F; // keep regular logic
break;
case 0x80:
leds = 0x30; // keep regular logic
break;
default:
leds = 0x3F; // light 7 leds
break;
}
PORTB = ~leds; // active-low for LED to light.
}
}
Note that you can easily change your PORT or active-high / active-low sense. You might also prefer showing which LED in a more intuitive way:
Code:
case 0x80:
leds = 0b00110000; // keep regular logic
break;
The 0b notation for binary constants is supported by avr-gcc but is not regular C.
David. |
|
|
| |
|
|
|
|
|
|
Posted: Jul 10, 2012 - 09:09 AM |
|
Joined: Jul 08, 2012
Posts: 8
|
|
David wrote:
Quote:
Your if else-if structure where you test for specific values with == is exactly the same as using switch - case - default.
If you use another variable, it means you can keep your human brain at room temperature:
I try this code,but it doesn't works.
I don't know what's wrong happen |
|
|
| |
|
|
|
|
|
|
Posted: Jul 10, 2012 - 09:23 AM |
|
Joined: Feb 12, 2005
Posts: 16254
Location: Wormshill, England
|
|
Just saying "I try this code,but it doesn't works" is not very helpful to you or your readers.
Please describe what you expected, and what you got.
I confess. I just wrote the example code, and never tested it in real-life.
I have just tried it, and it behaves as I expected. The STK500 has PORTB.6 and PORTB.7 on the PORTE/AUX header.
David.
Oops. I forgot the 0x20 case. I leave this as an exercise for the reader. |
|
|
| |
|
|
|
|
|
|
Posted: Jul 10, 2012 - 09:55 AM |
|
Joined: Jul 08, 2012
Posts: 8
|
|
sorry, i want expected when i try your code by using switch - case - default.
Code:
#include <avr/io.h>
int main(void)
{
unsigned char human, leds;
DDRD = 0x00; // switches are input
PORTD = 0xFF; // internal pull-ups on switches.
DDRB = 0xFF; // LEDs are output
while (1) // loop forever
{
human = ~PIND; // don't need to worry about int extension.
switch (human) // human readable switches
{
case 0x01:
leds = 0x01; // keep regular logic
break;
case 0x02:
leds = 0x02; // keep regular logic
break;
case 0x04:
leds = 0x04; // keep regular logic
break;
case 0x08:
leds = 0x08; // keep regular logic
break;
case 0x10:
leds = 0x10; // keep regular logic
break;
case 0x40:
leds = 0x0F; // keep regular logic
break;
case 0x80:
leds = 0x30; // keep regular logic
break;
default:
leds = 0x3F; // light 7 leds
break;
}
PORTB = ~leds; // active-low for LED to light.
}
}
but it doesn't light up when i press the swithches |
|
|
| |
|
|
|
|
|
|
Posted: Jul 10, 2012 - 10:18 AM |
|
Joined: Feb 12, 2005
Posts: 16254
Location: Wormshill, England
|
|
Well, I connected PORTD to switches and PORTB to LEDs with the two 10-way ribbons.
With no buttons pressed, all 6 LEDs light up. LED0..LED5
Press SW0 and just LED0 lights
ditto for SW1 to SW5.
Press SW6 and LED0..LED3 light.
Press SW7 and LED4,LED5 light.
Quote:
but it doesn't light up when i press the swithches
Do the switches light with no buttons pressed?
Did you copy-paste my code, or type it by hand?
David. |
|
|
| |
|
|
|
|
|
FAQ
Search
Register
Log in to check your private messages
Log in
Maximize
RSS
