Forum: Mikrocontroller und Digitale Elektronik Bitmanipulationen, Übungsaufgabe


Announcement: there is an English version of this forum on EmbDev.net. Posts you create there will be displayed on Mikrocontroller.net and EmbDev.net.
von Georg M. (g_m)


Angehängte Dateien:

Lesenswert?

Hallo zusammen,
folgende Übungsaufgabe zum Thema "Bitmanipulationen".
Diese Übungsaufgabe hat keine praktische Bedeutung, aber man kann das 
Ergebnis sofort beobachten - an der blinkenden LED.
Die Aufgabe lautet:
1
blink-blink-blink-blink----pause----blink-blink-blink-blink----pause----....
1
#include <avr/io.h>
2
3
uint8_t cnt = 0xFF;          // SW counter
4
uint8_t avar;                // auxiliary variable
5
6
int main(void)
7
{
8
  PORTA.DIRSET = PIN3_bm;                               // PA3 output (LED)
9
10
  TCB0.CCMP = 0xFE4F;                                   // timeout value
11
  TCB0.CTRLA = TCB_CLKSEL_CLKDIV2_gc | TCB_ENABLE_bm;   // select clock and enable TCB
12
13
  while(1)
14
  {
15
    if(TCB0.INTFLAGS)
16
    {
17
      TCB0.INTFLAGS = TCB_CAPT_bm;                                // clear TCB0 interrupt flag
18
      avar = (cnt >> 2) & (cnt >> 3) & (cnt << 1) & (cnt << 2);   // combine
19
      avar &= 0x08;                                               // clear all bits except bit_3
20
      PORTA.OUT = (PORTA.OUT & 0xF7) | avar;                      // update bit_3
21
      cnt--;                                                      // proceed
22
    }
23
  }
24
}


Meine Frage ist nun, ob man diesen Code irgendwie 
verbessern/optimieren/vereinfachen bzw. komplett anders schreiben kann.

von Lothar M. (Firma: Titel) (lkmiller) (Moderator) Benutzerseite


Lesenswert?

Georg M. schrieb:
> Meine Frage ist nun, ob man diesen Code irgendwie
> verbessern/optimieren/vereinfachen bzw. komplett anders schreiben kann.
In welche Richtung soll er denn verbessert/optimiert/vereinfacht oder 
gar ganz umgeschrieben werden?

Dein Code ist derzeit insofern effizient, als er optimal auf genau diese 
Hardware zugeschnitten ist. Er ist aber völlig unverständlich. Denn auch 
wenn die Lösung relativ einfach ist, kapiert in akzeptabler Zeit kein 
Mensch, was da passiert, wenn du da nicht "Viermalblinker mit Pause auf 
Bit 3" dazuschreibst.

Besser verständlich wäre es, das Blinken auf dem LSB des Zählers zu 
machen und dieses Bit dann auf den LED-Ausgang zu schieben.

Insofern würde ich zumindest den Kommentar "combine" wesentlich 
ausführlicher gestalten.

: Bearbeitet durch Moderator
von Mario M. (thelonging)


Lesenswert?

Im Endeffekt prüft das Programm, ob die Bits 1, 2, 5 und 6 gleichzeitig 
gesetzt sind.
1
avar = ((cnt & 0x66) == 0x66) ? 0x08 : 0;

von Georg M. (g_m)


Lesenswert?

Mario M. schrieb:
>
1
avar = ((cnt & 0x66) == 0x66) ? 0x08 : 0;

Wie? Es funktioniert auch ganz ohne bitweise Verschiebungen?

Dann ist die Aufgabe natürlich komplett falsch. Dann muss man sich etwas 
anderes ausdenken.

von Rolf M. (rmagnus)


Lesenswert?

Mario M. schrieb:
> avar = ((cnt & 0x66) == 0x66) ? 0x08 : 0;

oder avar komplett weglassen und dann so:
1
if (cnt & 0x66 == 0x66)
2
    PORTA.OUT |= 0x08;
3
else
4
    PORTA.OUT &= ~0x08;

von Mario M. (thelonging)


Lesenswert?

Sischa dat.

Rolf M. schrieb:
> if (cnt & 0x66 == 0x66)

Macht was anderes.

von Rolf M. (rmagnus)


Lesenswert?

Mario M. schrieb:
> Macht was anderes.

Ja, Klammern vergessen. Bitte dazudenken ;-)

von Bruno V. (bruno_v)


Lesenswert?

Georg M. schrieb:
> Meine Frage ist nun, ob man diesen Code irgendwie
> verbessern/optimieren/vereinfachen bzw. komplett anders schreiben kann.

Ich kann Lothar da nur zustimmen: Der Code ist Horror für jeden im 
beruflichen Umfeld.

Ich bin ganz sicher kein Freund von langen Variablennamen, ausführlichen 
Kommentaren oder unnützen Anweisungen. Aber am Ende sollte der Code 
erkennen lassen, was da passiert. Statt


Georg M. schrieb:
1
  avar = (cnt >> 2) & (cnt >> 3) & (cnt << 1) & (cnt << 2);   // combine
2
  avar &= 0x08;                                               // clear all bits except bit_3
3
  PORTA.OUT = (PORTA.OUT & 0xF7) | avar;                      // update bit_3
4
  cnt--;                                                      // proceed
wäre plain code das mittel der Wahl (hier mit cnt++ und 32 Takte 
insgesamt)
1
   
2
   const unsigned char mask = 0x08;         
3
4
      cnt++;
5
      switch(cnt%32)
6
      {
7
      case 1:
8
      case 3:
9
      case 5:
10
      case 7:  PORTA.OUT |= mask;
11
               break;
12
      default: PORTA.OUT &= ~mask;
13
               break;
14
      
15
      }

von Obelix X. (obelix)


Lesenswert?

Georg M. schrieb:
> avar = ((cnt & 0x66) == 0x66) ? 0x08 : 0;

oder
1
avar = (!(cnt ^ 0x66)) ? 0x08 : 0;

von Lothar M. (Firma: Titel) (lkmiller) (Moderator) Benutzerseite


Angehängte Dateien:

Lesenswert?

Georg M. schrieb:
> Dann muss man sich etwas anderes ausdenken.
Warum brauchst du eigentlich so viele Bits für diese Aufgabe?

Mein Ansatz wäre sowas mit 4 Bits:
1
Bit     LED
2
3210
3
----    -
4
---1    1
5
--1-    -
6
--11    1
7
-1--    -
8
-1-1    1
9
-11-    -
10
-111    1
11
1---    -
12
1--1    -
13
1-1-    -
14
1-11    -
15
11--    -
16
11-1    -
17
111-    -
18
1111    -
Und dann ist die LED an, wenn Bit 3 = 0 und Bit 0 = 1 ist:
1
    if(TCB0.INTFLAGS)
2
    {
3
      TCB0.INTFLAGS = TCB_CAPT_bm;                // clear TCB0 interrupt flag
4
      if (~cnt&8 && cnt&1)    PORTA.OUT |= 0x08;  // LED an wenn Bit3=0 und Bit0=1
5
      else                    PORTA.OUT &= ~0x08; // LED aus
6
      cnt++;
7
    }

https://onlinegdb.com/8EY-dN1ca

: Bearbeitet durch Moderator
von Bruno V. (bruno_v)


Lesenswert?

Nachtrag: Bei einer Taktanzahl, die nicht 2^n ist, z.B. 77, einfach ein 
"case 77: cnt = 0; break;" hinzu. Und % im switch weglassen

: Bearbeitet durch User
von Mario M. (thelonging)


Lesenswert?

Ist ja alles schön und gut, aber eure Programme machen was anderes. So 
sieht es im Original aus:
1
1100000011000000
2
1100000011000000
3
0000000000000000
4
0000000000000000
5
0000000000000000
6
0000000000000000
7
0000000000000000
8
0000000000000000
9
1100000011000000
10
1100000011000000
11
0000000000000000
12
0000000000000000
13
0000000000000000
14
0000000000000000
15
0000000000000000
16
0000000000000000

von Lothar M. (Firma: Titel) (lkmiller) (Moderator) Benutzerseite


Angehängte Dateien:

Lesenswert?

Mario M. schrieb:
> Ist ja alles schön und gut, aber eure Programme machen was anderes.
Meins macht das, was die Spec verlangt: vier mal blinken, dann eine 
Pause, dann von vorn...  ;-)


> So sieht es im Original aus
Es macht das, was die Spec verlangt: vier mal blinken, dann eine Pause, 
dann von vorn.

https://onlinegdb.com/_FIuA6gpPO

: Bearbeitet durch Moderator
von Mario M. (thelonging)


Lesenswert?

Lothar M. schrieb:
> Es macht das, was die Spec verlangt: vier mal blinken, dann eine Pause,
> dann von vorn.
Ein Auto ist alles, was vier Räder hat und sich von selbst bewegt. 😂
Noch kürzer (und noch kryptischer) wäre:
1
if (~cnt & 0x66)
2
    PORTA.OUT &= ~0x08;
3
else
4
    PORTA.OUT |= 0x08;

von Lothar M. (Firma: Titel) (lkmiller) (Moderator) Benutzerseite



Lesenswert?

Obelix X. schrieb:
> Georg M. schrieb:
>> avar = ((cnt & 0x66) == 0x66) ? 0x08 : 0;
> oder
> avar = (!(cnt ^ 0x66)) ? 0x08 : 0;
Hast du das ausprobiert oder nur vermutet?

Mario M. schrieb:
> Noch kürzer (und noch kryptischer) wäre
Auf jeden Fall weniger kryptisch als das Original...  ;-)

: Bearbeitet durch Moderator
von Mario M. (thelonging)


Lesenswert?

Man könnte auch noch "cnt" aufwärts zählen lassen, dann fiele das 
Negieren weg:
1
for (int cnt = 0; cnt < 256; cnt++)
2
{
3
    if (cnt & 0x66) printf("-");
4
    else printf ("#");
5
}

von Obelix X. (obelix)


Lesenswert?

Lothar M. schrieb:
> Obelix X. schrieb:
>> Georg M. schrieb:
>>> avar = ((cnt & 0x66) == 0x66) ? 0x08 : 0;
>> oder
>> avar = (!(cnt ^ 0x66)) ? 0x08 : 0;
> Hast du das ausprobiert oder nur vermutet?

Sorry, ich hatte da was verwechselt.

von Peter D. (peda)


Lesenswert?

Georg M. schrieb:
> Meine Frage ist nun, ob man diesen Code irgendwie
> verbessern/optimieren/vereinfachen bzw. komplett anders schreiben kann.

Der ist doch perfekt als Beispiel für Code Obfuscation, besser geht es 
nicht.

https://de.wikipedia.org/wiki/Obfuskation_(Software)

von Peter D. (peda)


Lesenswert?

Georg M. schrieb:
> Wie? Es funktioniert auch ganz ohne bitweise Verschiebungen?

Schiebeoperationen nimmt man typisch nur für seriell parallel Wandlung.
Oder um Bitmasken an die gewünschte Stelle zu schieben.

Planloses und mehrfaches Schieben ist ungebräuchlich, da es eben die 
Lesbarkeit massiv erschwert.

Zählvariablen wertet man einfacher über Arrays oder Switch aus. Dann 
sind auch Änderungen leicht möglich, z.B. 5 Pulse.
Daher sind auch LUTs in FPGAs so beliebt.

von Gregor J. (Firma: Jasinski) (gregor_jasinski)


Lesenswert?

Die richtigen Übungen machen den Meister, die falschen bewirken in der 
Regel leider nur, dass man sich immer weiter in der eigenen Suppe, die 
nie über den Tellerrand hinausragen wird, weil sie es gar nicht kann, 
dreht. Das gleiche gilt z.B. auch für Liegestützen, die man falsch – 
indem man z.B. nur den Popo auf und ab bewegt – macht, sofern man eine 
Veranschaulichung in diesem Kontext verwenden möchte. Alle anderen, die 
diese Übung so ähnlich ausführen, werden natürlich begeistert sein und 
sich ermuntern, ähnliche Übungen auszuführen – eine Gruppe der 
gegenseitigen Adoration, Begeisterung und des Klatschens entsteht.

von Veit D. (devil-elec)


Lesenswert?

Hallo,

diese Zeile jemanden Neuen zu erklären
1
avar = (cnt >> 2) & (cnt >> 3) & (cnt << 1) & (cnt << 2);
wird der Knackpunkt sein. Auf den ersten Blick sieht das wie sinnfreies 
hin- und herschieben aus. Bevor du deine Leute damit von Beginn an 
überforderst, schiebe ein einziges Bit durch und zeige das mit Lauflicht 
am Port wie sich dadurch die Led bewegt. Dann noch invertiert. Kannste 
auch mit schöner formatierter serieller Ausgabe machen. Wenn das klar 
ist kannste tiefer einsteigen. Wäre so mein Ansatz.

: Bearbeitet durch User
von Johann L. (gjlayde) Benutzerseite


Lesenswert?

Georg M. schrieb:
> Meine Frage ist nun, ob man diesen Code irgendwie verbessern...
1
> uint8_t avar;                // auxiliary variable

Das sollte keinesfalls eine globale Variable sein (und auch keine im 
Static Storage).  Einfach eine lokale Variable in der entsprechenden 
Funktion machen / block machen (while Loop von main).

Gleeiches gile für cnt; diese Variable ist lokal in main.
1
> TCB0.CCMP = 0xFE4F;         // timeout value

Magische Werte erschweren es den Code nachzuvollziehen (warum nicht 
0xFE4E? etc.) und auch anzupassen, etwa auf eine andere Taktfrequenz 
oder Event-Rate.  Oft lassen sich AVR Timerwerte berechnen gemäß
1
SFR = (uint32_t) F_CPU / PRESCALE / IRQs - 1;
wobei:

F_CPU = Taktrate (des Quatzes, R/C-Oszillators) in Hz

PRESCALE = Eingestellter Prescaler-Wert (Kommentare schweigen dazu)

IRQs = Anzahl Ereignisse pro Sekunde

Es wäre wünschenswert, dass die Kommentare genau und nachvollziehbar 
erklärt, wie man auf exakt diesen Wert kommt (oder eben eine Formel wie 
oben, die übrigens bereits vom Compiler ausgewertet wird.

Aber dem steht die 2-spaltige Formatierung entgegen, die einen dazu 
drängt, knappe und schlampige Kommentare zu schreiben statt hilfreicher. 
Kommentare sollten einem Wert genug sein, ihnen eine eigene Zeile zu 
gönnen, statt sie dem Ziel unterzuordnen, möglichst viele Zeilen ins 
Fenster zu quetschen.

Auch die anderen Kommentare sind mangelhaft, egal ob es sich "nur" um 
eine Übungsaufgabe handelt, um ein Tutrial, oder um produktiven Code.

Bitte melde dich an um einen Beitrag zu schreiben. Anmeldung ist kostenlos und dauert nur eine Minute.
Bestehender Account
Schon ein Account bei Google/GoogleMail? Keine Anmeldung erforderlich!
Mit Google-Account einloggen
Noch kein Account? Hier anmelden.