View previous topic :: View next topic |
Author |
Message |
mayai
Joined: 19 Dec 2009 Posts: 8
|
logic operation |
Posted: Mon Jan 11, 2010 3:11 am |
|
|
Hello everybody... I have this code... basically it based on logic operation to get the output... the actual code has 5 logic so it has 32 possibilities.
My main problem is the simulation is working well but when the actual circuit is being tested it does not produce the output as desired.
Code: |
#include <16f877a.h>
#device adc=8
#use delay(clock=20000000)
#fuses hs, noprotect,nowdt,nolvp
#define 1 PIN_C0
#define 2 PIN_C1
#byte PORTB=6
#byte PORTC=7
void fwd()
{
output_high(pin_b0);
output_high(pin_b1);
delay_ms(5000);
output_low(pin_b0);
output_low(pin_b1);
}
void stp()
{
delay_ms(5000);
output_low(pin_b0);
output_low(pin_b1);
}
void rgt()
{
output_high(pin_b1);
delay_ms(5000);
output_low(pin_b1);
}
void lft()
{
output_high(pin_b0);
delay_ms(5000);
output_low(pin_b0);
}
void main()
{
int16 adc_value0,adc_value1;
int16 i;
setup_adc_ports(ALL_ANALOG);
setup_adc(ADC_CLOCK_DIV_32);
set_tris_b(0b00000000);
set_tris_c(0b00000011);
set_tris_a(0b00000011);
portc=0;
portb=0;
while(1)
{
for(i=0;i<2;i++)
{
set_adc_channel(0); //set to read channel a0 next
delay_us(20);
adc_value0 = read_adc(); //get 10 bit readig for channel 0
if (adc_value0<128) output_low(pin_c0);
if (adc_value0>128) output_high(pin_c0);
delay_ms(500);
set_adc_channel(1); //set to read channel a1 next
delay_us(20);
adc_value1 = read_adc(); //get 10 bit readig for channel 1
if (adc_value1<128) output_low(pin_c1);
if (adc_value1>128) output_high(pin_c1);
delay_ms(500);
{
if ((input_state(PIN_C0) == 0) && (input_state(PIN_C1) == 0))
{
fwd();
}
else if ((input_state(PIN_C0) == 0) && (input_state(PIN_C1) == 1))
{
rgt();
}
else if ((input_state(PIN_C0) == 1) && (input_state(PIN_C1) == 0))
{
lft();
}
else if ((input_state(PIN_C0) == 1) && (input_state(PIN_C1) == 1))
{
stp();
}
delay_ms(20);
}
}
}
} |
|
|
|
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Mon Jan 11, 2010 12:42 pm |
|
|
Quote: |
#define 1 PIN_C0
#define 2 PIN_C1
|
You are trying to redefine the numbers 1 and 2. You're not allowed to
do that. The compiler should display an error message, but it doesn't.
It just ignores those statements.
Quote: | #include <16f877a.h>
#device adc=8
int16adc_value0,adc_value1;
|
Here, you have told the compiler to return an 8-bit result from the
read_adc() function. But then you have declared your adc result
variables as 16-bit. That's not necessary. You could declare
them as 8 bit, using 'int8'. You would only need them as 'int16' if
you used #device adc=10.
Quote: | adc_value0 = read_adc(); //get 10 bit readig for channel 0 |
The comment says 10-bit, but you have the compiler set for 8-bit readings.
Quote: | set_tris_b(0b00000000);
set_tris_c(0b00000011);
set_tris_a(0b00000011); |
You are setting the TRIS, but you have not told the compiler to use
"fast io" mode. Therefore, the compiler will use "standard i/o" mode.
In that mode, the compiler sets the TRIS when it does any input or
output operations on ports or pins. It will overwrite your TRIS settings
as needed, depending on the CCS function.
Quote: |
int16 i;
for(i=0;i<2;i++)
|
You have declared your counter variable 'i' as 16-bit, but you are only
counting from 0 to 1. You could use 'int8', and make the generated code
be smaller and more efficient.
Quote: | if (adc_value0<128) output_low(pin_c0);
if (adc_value0>128) output_high(pin_c0); |
Here you are doing two tests. But what if the adc value is exactly 128 ?
What will your code do ? It would be better to do one test (the first line)
and then use an 'else' statement to handle all cases that are not satisfied
by the first test.
Quote: | if ((input_state(PIN_C0) == 0) && (input_state(PIN_C1) == 0))
{
fwd();
} |
Here you are reading the pins (while leaving them in output mode),
to find what values you wrote to them, I suppose. What external
circuits do you have on these pins ? What if the electrical load is
so high, that the pin's output drivers are not able to hold the output
logic level ? Then you would write a '1' to the pin, but read back a '0'.
Is that what you want ? I don't know. If that's not what you want
to happen, then it would be better to keep the adc results in a variable.
Then you could easily test it, and it would not be affected by any external
circuits. |
|
|
mayai
Joined: 19 Dec 2009 Posts: 8
|
|
Posted: Fri Jan 15, 2010 3:25 am |
|
|
i know that this program is actually "stupid" so i tried to use different approach... here's the idea:
1. there are 5 sensors in the program (basically 4 is digitally triggered and only 1 use ADC)
2. the 5 sensors is define by sensor1(analog),sensor2,sensor3,sensor4 and sensor5...
3.all the sensor reading will be combine by the or oepration exp sensor=sensor1 or sensor2 or sensor3 or sensor4 or sensor5... by this we will get sensor=0b00011111 if all the sensor is high...
4. then we go to the famous if operation.. say : if sensor=0b00000000 {foward()} else if sensor=0b00000001{backward} and so on...
5. the movement void will choose the movement of the motor base on the logic operation...
6. i have develop the program but it seems i have come to a dead end... please help me my dear mentor....:-)
here's the code:
Code: |
#include <16f877a.h>
#device adc=8
#use delay(clock=20000000)
#fuses hs, noprotect,nowdt,nolvp
#byte PORTB=6
#byte PORTC=7
#byte PORTD=8
#byte PORTA=9
void fwd()
{
output_high(pin_b0);
output_high(pin_b1);
delay_ms(5000);
output_low(pin_b0);
output_low(pin_b1);}
void left()
{
output_high(pin_b1);
delay_ms(2000);
output_low(pin_b1);
}
void right()
{
output_high(pin_b0);
delay_ms(2000);
output_low(pin_b0);
}
void main()
{
int adc_value0;
int i;
int sensor,sensor1,sensor2,sensor3,sensor4,sensor5;
setup_adc_ports(ALL_ANALOG);
setup_adc(ADC_CLOCK_DIV_32);
portc=0;
portb=0;
portd=0;
while(1)
{
for(i=0;i<1;i++)
{
set_adc_channel(0); //set to read channel a0 next
delay_us(20);
adc_value0 = read_adc(); //get 10 bit readig for channel 0
if (adc_value0<128)
{
sensor1=0b00000100;
}
if (adc_value0>128)
{
sensor1=0b00000000;
}
delay_ms(10);
{
if(input(pin_c1)==0)
{
sensor2=0b00000000;
}
else if(input(pin_c1)==1)
{
sensor2=0b00000001;
}
}
{
if(input(pin_c2)==0)
{
sensor3=0b00000000;
}
else if(input(pin_c2)==1)
{
sensor3=0b00000010;
}
}
{
if(input(pin_c3)==0)
{
sensor4=0b00000000;
}
else if(input(pin_c3)==1)
{
sensor2=0b00001000;
}
}
{
if(input(pin_c4)==0)
{
sensor2=0b00000000;
}
else if(input(pin_c4)==1)
{
sensor2=0b00010000;
}
}
sensor=sensor1|sensor2|sensor3|sensor4|sensor5;
if (sensor=0b00000000)
{
fwd();
}
else if (sensor=0b00000001)
{
left();
}
delay_ms(20);
}
}
}
|
please do not hestitate... thanks in advance... |
|
|
FvM
Joined: 27 Aug 2008 Posts: 2337 Location: Germany
|
|
Posted: Fri Jan 15, 2010 3:37 am |
|
|
I wonder, if it can be done more complicated?
C has logical operators for similar purposes and CCS C has an int1 (bit) variable. It's the recommended high
level programming way to make readable code for logical operations. |
|
|
Wayne_
Joined: 10 Oct 2007 Posts: 681
|
|
Posted: Fri Jan 15, 2010 3:48 am |
|
|
First you have a copy and paste error :-
if(input(pin_c3)==0)
{
sensor4=0b00000000;
}
else if(input(pin_c3)==1)
{
sensor2=0b00001000;
You specify sensor2 should that be sensor4 ?
Although what you are doing should work, you have a lot of redundant code. One way I may do it is :-
Code: |
int sensor;
// in your for loop
for(i=0;i<1;i++)
{
sensor = 0;
set_adc_channel(0); //set to read channel a0 next
delay_us(20);
if (read_adc() < 128)
sensor |= 0b00000100;
sensor |= input(pin_c1);
sensor |= input(pin_c2) << 1;
sensor |= input(pin_c3) << 3;
sensor |= input(pin_c4) << 4;
switch(sensor)
{
case 0:
fwd();
break;
case 0b00000001:
left();
break;
}
|
And I know I am doing it for you here but it may help you learn something.
I did say may do it as it does depend on what else you are doing.
For instance. It may be quicker and easier to just read in port C and mask of unused pins. This will mean your bitmap is different. |
|
|
mayai
Joined: 19 Dec 2009 Posts: 8
|
|
Posted: Fri Jan 15, 2010 6:58 am |
|
|
thanks for your suggestion, may i ask
Code: | sensor = 0;
set_adc_channel(0); //set to read channel a0 next
delay_us(20);
if (read_adc() < 128)
sensor |= 0b00000100;
sensor |= input(pin_c1);
sensor |= input(pin_c2) << 1;
sensor |= input(pin_c3) << 3;
sensor |= input(pin_c4) << 4; |
is it << mean the position of the bit? i assume that if <<1 meaning 0b00000010... it is right? and may i presume that |= meaning the or operation of the code? thank you very much wayne... |
|
|
Wayne_
Joined: 10 Oct 2007 Posts: 681
|
|
Posted: Fri Jan 15, 2010 7:30 am |
|
|
|= means to OR the value on the right with the variable on the left and assign it to the variable on the left.
sensor |= number;
Will or number with sensor and put the value in sensor.
<< means to shift left the value by a number of bits so
0b00000001 << 2 == 0b00000100
I use it to place the bit at the correct position based on your code.
If the input is 0 then it means nothing as shifting 0 equals 0.
If the input is 1 then it moves it to the correct bit position then ors it with sensor. |
|
|
mayai
Joined: 19 Dec 2009 Posts: 8
|
|
Posted: Fri Jan 15, 2010 11:27 am |
|
|
ok thanks... here's the test code that i've upgrade thanks to wayne......
Code: | #include <16f877a.h>
#device adc=8
#use delay(clock=20000000)
#fuses hs, noprotect,nowdt,nolvp
#byte PORTB=6
#byte PORTC=7
#byte PORTD=8
#byte PORTA=9
void fwd()
{
output_high(pin_b0);
output_high(pin_b1);
delay_ms(5000);
output_low(pin_b0);
output_low(pin_b1);}
void left()
{
output_high(pin_b1);
delay_ms(2000);
output_low(pin_b1);
}
void right()
{
output_high(pin_b0);
delay_ms(2000);
output_low(pin_b0);
}
void main()
{
int i;
int sensor;
setup_adc_ports(ALL_ANALOG);
setup_adc(ADC_CLOCK_DIV_8);
set_tris_b(0b00000000);
portc=0;
portb=0;
portd=0;
while(1)
{
for(i=0;i<1;i++)
{
sensor = 0;
set_adc_channel(0); //set to read channel a0 next
delay_us(20);
if (read_adc() < 128)
sensor |= 0b00000100;
else if (read_adc() > 128)
sensor |= 0b00000000;
sensor |= input(pin_c1) << 0;
sensor |= input(pin_c2) << 1;
sensor |= input(pin_c3) << 3;
sensor |= input(pin_c4) << 4;
switch(sensor)
{
case 0b00000000:
fwd();
break;
case 0b00000001:
left();
break;
}
}
}
} |
the question now is why does the program not running in the simulation... honestly i dont see anything that needs to be change... excuse my ignorance but i think this is the best way for me to improve my programming.. |
|
|
|