bitmanipulation zu kompliziert?

[marten (Gast)]

Hallo,

ich bin grade dabei folgende Aufgabe zu loesen:

Write a C Program that will read a two-bit value via Port 2.0, 2.1 when 
a push button is momentarily pressed.  The push button is connected to 
Port 1.1   LEDs are connected to Port 1 bits 6 and 7.
a. The routine should send a high output to Port 2 bit 6 if the value 
read in is greater than the last value read in.  The value should remain 
high for approx 1 second.

b. The routine should add one to a memory location SAME, when the value 
is equal to the previous value read in.

c. The routine should send a low output to Port 1 bit 7 if it is less 
than the previous value read in.  The light should extinguish for 
approximately 1 second.

The routine should be initialised with the last value read in as 0, port 
1.6 low and port 1.7 high.


Ich habe etwas geschrieben und wuerde gerne wissen ob es einen 
einfacheren weg gibt. Es geht mir im wesentlichen um den kommentierten 
Teil. Wir benutzen microC und einen At89S8253. Der Compiler bringt keine 
Fehlermeldung. Leider habe ich erst wieder naechste Woche die 
moeglichkeit es am Board zu testen. Gibt es einen besseren weg z.B. ohne 
die Variable "buffer"?

sbit lowbit at P2.B0;
sbit highbit at P2.B1;
sbit button_0 at P1.B1;
sbit led_1 at P1.B6;
sbit led_2 at P1.B7;

void main() {

unsigned char buffer = 0x00;
unsigned char val_new = 0x00;
unsigned char val_old = 0x00;
unsigned char same;

lowbit = 1;         //input
highbit = 1;        //input
button_0 = 1;       //input
led_1 = 0;          //output
led_2 = 0;          //output

     while(1){
     
     if (button_0 == 0){                  //is the button pressed?
        while (button_0 == 0);            //is the button still pressed?

        buffer = highbit;               // if highbit 1 --> buffer = 00000001
        buffer <<= 1;                   // buffer = 00000010
        
        val_new = buffer;               // val_new = 00000010
        
        buffer = lowbit;                // if lowbit 1 --> buffer = 00000001
        
        val_new |= buffer;              // val_new = 00000011
        
        if (val_new > val_old){
           led_1 = 1;
           delay_ms (1000);
           led_1 = 0;
           }
           else if (val_new < val_old){
           led_2 = 0;
           delay_ms (1000);
           led_2 = 1;
           }
           else {
           same++;
           }

        val_old = val_new;

        }
     }

}

[Karl H. (kbuchegg)]

Das

        buffer = highbit;               // if highbit 1 --> buffer = 00000001
        buffer <<= 1;                   // buffer = 00000010
        
        val_new = buffer;               // val_new = 00000010
        
        buffer = lowbit;                // if lowbit 1 --> buffer = 00000001

        val_new |= buffer;              // val_new = 00000011

erscheint mir ein wenig kompliziert.

Nichts gegen Zwischenvariablen, aber man darf ruhig auch in einem 
einfachen Ausdruck ruhig ein wenig rechnen ...

    val_new  = ( highbit << 1 ) + lowbit;

... und muss nicht alles in x Zeilen auseinanderdröseln. Irgendwann 
dröselt man dann nämlich alles so weit auseinander, dass man überhaupt 
nichts mehr sieht, weil man sich die 'Absicht' des Programmierers über 
viele Codezeilen hinweg erst mal mühsam zusammensuchen muss.

Eine alternative Schreibweise wäre

    val_new = ( 2 * highbit ) + lowbit;

die meiner Meinung nach die Wertigkeit der beiden Bits etwas besser 
rausbringt. Jeder halbwegs vernünftige Compiler wird allerdings sowieso 
beides in gleichen Code umsetzen, so dass die Multiplikation jetzt erst 
mal schlimmer aussieht, als es tatsächlich ist.

[Stefan F. (Gast)]

Wenn du die Ports als Bytes betrachtest und die gewünschten Bits mit 
Logischen Operatoren herausfilterst, wird der Code sicher deutlich 
kürzer und vermutlich auch effizienter.

[marten (Gast)]

Super,

vielen Dank fuer eure Hilfe.

Greetz