Framework zur Ansteuerung von LEDs

[Bernhard (Gast)]

Hallo Zusammen!

Vor einiger Zeit habe ich einen Code entwickelt mit dem man LEDs 
verschieden ansteuern kann. Funktionen wie EIN, AUS, BLINKEN und BLITZEN 
werden unterstützt. Compiliert belegt dieser aktuell 374 Bytes.
Die LED's liegen an einem Port an Px0 bis Px3

Die Funktion led_process sollte in einem Timer z.B. jede 10ms aufgerufen 
werden.

Trotz der kleinen Größe finde ich ihn immernoch etwas umständlich.
Ich suche noch etwas "Intelligenz" wie z.B. der Code von Peter Dannegger 
(ich erinnere mich an die kleinen und schnellen Debounce-Routinen)

Vielleicht hat jemand noch eine Idee was man besser machen kann :)

Grüße,
Bernhard

[Karl H. (kbuchegg)]

Das hier

  for (i = 0; i < LED_COUNT; i++) {
    if (ledctx[i].state == LED_OFF) {
      out &= ~_BV(i);

sind so typische Operationen, die du nicht haben willst, wenn der µC 
keinen Barrelshifter hat. Das _BV(i) muss dann vom Compiler in eine 
Schleife aufgelöst werden

   result = 0x01;
   for( j = 0; j < i; j++ )
     result <<= 1;

und das willst du eigentlich nicht. Vor allen Dingen, wenn es sich 
leicht vermeiden lässt

  uint8_t bitMask;

  bitMask = 0x01;

  for (i = 0; i < LED_COUNT; i++) {


    if (ledctx[i].state == LED_OFF) {
      out &= ~bitMask;

      ....
    }
    ....

    bitMask <<= 1;
  }

Deine Schreibweise ist furchtbar. Deine else hab ich erst im dritten 
Anlauf gesehen. Es ist nichts falsch an

   if ( ..... ) {

     ....
   }

   else if( ...... ) {
     ....
   }

   else if( ...... ) {
     ....
   }

   else {
     ....
   }

ausser, dass so auch ein halbblinder Blindenhund auf 5 Meter Entfernung 
sehen kann, dass es sich um mehrere Alternativen handelt, von denen die 
erste genommen wird.

Bei

   if ( ..... ) {

     ....
   } else

   if( ...... ) {
     ....
   } else

   if( ...... ) {
     ....
   }

   else {
     ....
   }

sieht man das weit nicht so gut, weil jeder automatisch immer auf den 
Zeilenafang schaut und nicht eine Zeile drüber, ob da vor dem if noch 
ein else steht. Beim schnellen drüber lesen sind das 3 voneinander 
unabhängige if. Und erst im dritten Anlauf entdeckt man dann noch die 
else und stellt die richtige Assoziation her, dass die 3 if dann doch 
nicht so unabhängig voneinander sind, wie es erst den Anschein hatte.

[Bernhard (Gast)]

@Karl Heinz Buchegger:

Vielen Dank, jetzt ist der Code nur noch 322 Bytes lang.
Die _BV(i) Operationen bzw. 1 << i haben mir auch noch nicht gefallen. 
Deine Lösung ist super, ich sitze wahrscheinlich heute schon zu lang 
vorm Rechner, sonst wäre mir das auch gleich aufgefallen :)

Die Schreibweise habe ich angepasst, mit der Zeit gewöhnt man sich in 
der Tat ein paar Undinge an

Jetzt sieht das ganze so aus:

void led_process(void)
{
        static uint8_t out = 0;
        uint8_t i, mask;

        for (i = 0, mask = 1; i < LED_COUNT; i++, mask <<= 1) {
                if (ledctx[i].state == LED_OFF) {
                        out &= ~mask;
                } else if (ledctx[i].state == LED_ON) {
                        out |= mask;
                } else if (!ledctx[i].timer) {
                        if (ledctx[i].state == LED_BLINK) ledctx[i].timer = BLINK_TIME;
                        if (ledctx[i].flag) {
                                if (ledctx[i].state == LED_FLASHON) ledctx[i].timer = FLASH1_TIME;
                                if (ledctx[i].state == LED_FLASHOFF) ledctx[i].timer = FLASH2_TIME;
                                ledctx[i].flag = 0;
                                out &= ~mask;
                        } else {
                                if (ledctx[i].state == LED_FLASHON) ledctx[i].timer = FLASH2_TIME;
                                if (ledctx[i].state == LED_FLASHOFF) ledctx[i].timer = FLASH1_TIME;
                                ledctx[i].flag = 1;
                                out |= mask;
                        }
                } else {
                        ledctx[i].timer--;
                        continue;
                }

        }
        LED_PORT = (LED_PORT & 0xF0) | out;
}

[Bernhard (Gast)]

Gerade festgestellt, wenn man folgenden Code mit einem else ergänzt, 
dann spart man jeweils 10 Bytes:

Davor

if (ledctx[i].state == LED_FLASHON) ledctx[i].timer = FLASH1_TIME;
if (ledctx[i].state == LED_FLASHOFF) ledctx[i].timer = FLASH2_TIME;

danach

if (ledctx[i].state == LED_FLASHON) ledctx[i].timer = FLASH1_TIME; else
if (ledctx[i].state == LED_FLASHOFF) ledctx[i].timer = FLASH2_TIME;

Ich vergleiche gleich mal die lst Files, der Unterschied ist schon 
beträchtlich und interessiert mich grad bennend

[Bernhard (Gast)]

Sieht so aus als ob der Compiler ohne "else" den Wert von state komplett 
neu aus dem Speicher lädt.
Mit dem else wird der Wert im Register wiederverwendet.

Also macht es auch Sinn den Wert von z.B. ledctx[i].state in eine lokale 
Variable zu packen da diese recht oft verwendet wird.

Jetzt sind wir schon bei 278 Bytes - es funktioniert alles noch 
einwandfrei.

void led_process(void)
{
        static uint8_t out = 0;
        uint8_t i, mask, state;

        for (i = 0, mask = 1; i < LED_COUNT; i++, mask <<= 1) {

                state = ledctx[i].state;

                if (state == LED_OFF) {
                        out &= ~mask;
                } else if (state == LED_ON) {
                        out |= mask;
                } else if (!ledctx[i].timer) {
                        if (state == LED_BLINK) ledctx[i].timer = BLINK_TIME;
                        if (ledctx[i].flag) {
                                if (state == LED_FLASHON) ledctx[i].timer = FLASH1_TIME; else
                                if (state == LED_FLASHOFF) ledctx[i].timer = FLASH2_TIME;
                                ledctx[i].flag = 0;
                                out &= ~mask;
                        } else {
                                if (state == LED_FLASHON) ledctx[i].timer = FLASH2_TIME; else
                                if (state == LED_FLASHOFF) ledctx[i].timer = FLASH1_TIME;
                                ledctx[i].flag = 1;
                                out |= mask;
                        }
                } else {
                        ledctx[i].timer--;
                        continue;
                }
        }

        LED_PORT = (LED_PORT & 0xF0) | out;
}