unineffizienter Code in Arduino Beispiel "Blink without Delay" ?

[Karli (Gast)]

In der Arduino Software findet sich im Beispiel "Blink without Delay" 
folgender Codeschnipsel:

    // if the LED is off turn it on and vice-versa:
    if (ledState == LOW) {
      ledState = HIGH;
    } else {
      ledState = LOW;
    }

Wäre es nicht sinnvoll, diesen durch

    ledState = !ledstate;

zu ersetzen?

[Karli (Gast)]

Karli schrieb:
> ledState = !ledstate;
Es hätte
ledState = !ledState;
heißen müssen.

[Einer K. (Gast)]

Karli schrieb:
> Wäre es nicht sinnvoll, diesen durch    ledState = !ledstate;
> zu ersetzen?

Ja.
Alleine schon wegen der Schreibfaulheit.

[H.Joachim S. (crazyhorse)]

Woran machst du fest, was "uneffizienter code" ist? Am Schreibaufwand? 
Am vermuteten erzeugten Code? An der Lesbarkeit?

Es wird am Ende derselbe Maschinencode herauskommen :-)

[Einer (Gast)]

unineffizienter Code
ergibt effizienter Code

Aber zum Thema.
Das ist ein Beispiel. Das soll Verständlich für Anfänger sein.

[holger (Gast)]

>Es wird am Ende derselbe Maschinencode herauskommen :-)

Das ist nicht richtig.
Wenn LOW = 0 und HIGH = 1 wird ledState 0 oder 1.
Im zweiten Beispiel wird ledState 0 oder 0xFF wenn ledState uint8_t ist.

[loeti2 (Gast)]

holger schrieb:
> Das ist nicht richtig.
> Wenn LOW = 0 und HIGH = 1 wird ledState 0 oder 1.
> Im zweiten Beispiel wird ledState 0 oder 0xFF wenn ledState uint8_t ist.

Wohl kaum, mal in einem C-Buch den Unterschied zwischen logischer und 
Bit-Negierung nachlesen.

[foobar (Gast)]

Die "Verbesserung" funktioniert nur identisch, wenn LOW 0 ist und HIGH 1 
oder umgekehrt.  Bei allen anderen Werten (evtl gibt es ja OFF, LOW, 
MEDIUM, HIGH?) klappt das "!" nicht.

Der Sinn und Zweck von benamten Konstanten ist es ja, dass einem der 
konkrete Wert nicht interessiert.  Durch das "!" machst du genau das 
Gegenteil, du verlässt dich auf konkrete Werte.

Bei numerischem LOW/HIGH (und nur die beiden) ginge:

ledState = LOW+HIGH - ledState;
// oder
ledState ^= LOW^HIGH;

Ob das aber, insbesondere für Anfänger, lesbarer wäre?

[googoo (Gast)]

wie wär's mit led = led ^ 1 ? :)
oder auch led ^=1

[A. S. (Gast)]

Arduino Fanboy D. schrieb im Beitrag #5865391:
> Es ist int.

Dann poste bitte den relevanten Code.

[Matthias S. (da_user)]

Karli schrieb:
> Wäre es nicht sinnvoll, diesen durch    ledState = !ledstate;
> zu ersetzen?

Die Frage ist eher, was der bezweckte Sinn ist. Ist es, möglichst 
"effizienten" Code zu schreiben, oder Anfängern die Sprache 
beizubringen?

[Andre (Gast)]

Karli schrieb:
> Wäre es nicht sinnvoll,

Komplett auf die Variable zu verzichten und das Bit im Ausgangsregister 
zu toggeln?

[GEKU (Gast)]

googoo schrieb:
> wie wär's mit led = led ^ 1 ? :)
> oder auch led ^=1

Ich würde bit-weise invertieren, dann kann man die anderen PortPin's 
noch für andere Zwecke verwenden.

#define PP0 = 0x01;
#define PP1 = 0x02;
#define PP2 = 0x04;
#define PP3 = 0x08;
#define PP4 = 0x10;
#define PP5 = 0x20;
#define PP6 = 0x04,;
#define PP7 = 0x08;

port ~= PP5;   // für PortPin 5

Durch Addition, z.B.  (PP3+PP5+PP7), können mehrere PortPin's 
angesteuert werden.

[GEKU (Gast)]

GEKU schrieb:
> #define PP6 = 0x04,;
> #define PP7 = 0x08;

natürlich:

#define PP6 = 0x40;
#define PP7 = 0x80;

[The Mule (Gast)]

#define in dem Zusammenhang ist Rückschritt. Dann lieber den originalen 
Syntax.

[GEKU (Gast)]

The Mule schrieb:
> #define in dem Zusammenhang ist Rückschritt.

Dient nur zur Erklärung. Man kann natürlich auch eine Konstante 
definieren.
Reine Geschmackssache!

[GEKU (Gast)]

The Mule schrieb:
> Dann lieber den originalen
> Syntax.

Und wie wurde LOW & HIGH definiert?

[Christopher B. (chrimbo)]

GEKU schrieb:
> port ~= PP5;   // für PortPin 5

was soll das tun?
das wäre ja ausgeschrieben

port = port ~0x20; ->
port = port 0xDF;

Ich würde sowas wie missing operator erwarten. Oder ist ~ in CPP anders 
definiert?

--- Probieren ---
GCC (C) sagt:
error: expected ';' before '~' token

[Einer K. (Gast)]

Liebe Leute, aller Arduino Quellcode liegt im Internet öffentlich aus.

GEKU schrieb:
> Und wie wurde LOW & HIGH definiert?
https://github.com/arduino/ArduinoCore-avr/blob/master/cores/arduino/Arduino.h

und:
A. S. schrieb:
> Arduino Fanboy D. schrieb im Beitrag #5865391:
>> Es ist int.
>
> Dann poste bitte den relevanten Code.
https://github.com/arduino/Arduino/tree/ee1967cd530ceb9a1d638875e385157e90d532e8/build/shared/examples/02.Digital/BlinkWithoutDelay

----------

Ich finde das Beispiel übrigens auch nicht sonderlich schön.
Und hätte sicherlich bool als Datentype für die Variable verwendet.

[Einer K. (Gast)]

GEKU schrieb:
> natürlich:#define PP6 = 0x40;
> #define PP7 = 0x80;

? ? ?
Was soll das = da, oder das ;
Welche Sprache ist das?

[Roland P. (pram)]

Andre schrieb:
> Karli schrieb:
>> Wäre es nicht sinnvoll,
>
> Komplett auf die Variable zu verzichten und das Bit im Ausgangsregister
> zu toggeln?

Oder noch effizienter: Die LED direkt an einen Timer-PWM hängen, dann 
blinkt die sogar ohne Code (und der TO lernt, wie man einen Timer 
aufsetzt)

VG
Roland

[Matthias S. (Firma: matzetronics) (mschoeldgen)]

Sowas geht auch

// toggle ledstate
ledstate = 1 - ledstate;

[Philipp_K59 (Gast)]

Oder ganz easy..

ledState = !ledState?HIGH:LOW;

wäre nach dem Beispiel gekürzt:

digitalWrite(ledPin, (!ledState?HIGH:LOW));

[gurgl (Gast)]

GEKU schrieb:
> Ich würde bit-weise invertieren

GEKU schrieb:
> port ~= PP5;   // für PortPin 5

Was du meinst ist XOR, ^

Die Tilde die du verwendest ist Bitweise-Invertieren und ein unärer 
Operator, dh. er erwartet EINEN Operanden

zB
~ 00 ergibt FF
~ 88 ergibt 77  usw

Theoretisch müsste "port~=;" funktionieren ;) kann das aber grad nicht 
testen;)

[Philipp K. (philipp_k59)]

Philipp_K59 schrieb:
> wäre nach dem Beispiel gekürzt:
>
> digitalWrite(ledPin, (!ledState?HIGH:LOW));

Eher so.. bin da nicht so der Short-If Profi.

digitalWrite(ledPin, (ledState=!ledState?HIGH:LOW));

[Sebastian S. (amateur)]

Irgendwie komme ich mit dem Hingucken einfach nicht nach!

Ich assoziiere "Blink" mit etwas, was man sehen kann.

Die Code-Fragmente flackern mir zu schnell.

Der Ansatz: "Ohne Delay" ist ein Witz, wenn dieses einfach, ersatzlos 
weggelassen wird.

[Mark B. (markbrandis)]

Karli schrieb:
> In der Arduino Software findet sich im Beispiel "Blink without Delay"
> folgender Codeschnipsel:
>

>     // if the LED is off turn it on and vice-versa:
>     if (ledState == LOW) {
>       ledState = HIGH;
>     } else {
>       ledState = LOW;
>     }
>

> Wäre es nicht sinnvoll, diesen durch
>

>     ledState = !ledstate;
>

> zu ersetzen?

Was genau wäre dadurch gewonnen?

Der generierte Maschinencode wird genau der gleiche sein.

[gurgl (Gast)]

Sebastian S. schrieb:
> Der Ansatz: "Ohne Delay" ist ein Witz, wenn dieses einfach, ersatzlos
> weggelassen wird.

Die Idee dahinter ist, den Code für´s Bit-Toggeln so ineffizient langsam 
zu machen, daß ein extra Delay unnötig wird :P

[gurgl (Gast)]

Mark B. schrieb:
> Der generierte Maschinencode wird genau der gleiche sein.

Es geht grundsätzlich um eine Designentscheidung: 
Kurz-und-knapp-mathematisch oder eher in Prosaform.

Spätestens wenn man ein Programm mit sagen wir 300 Zeilen durcharbeiten 
(zB Bugs finden) muss, macht es nen Unterschied ob es 300 oder 1500 
Zeilen sind

(sprich 3000, 30000 Zeilen usw obwohl das dann schon pervers ist)

[Mark B. (markbrandis)]

gurgl schrieb:
> Mark B. schrieb:
>> Der generierte Maschinencode wird genau der gleiche sein.
>
> Es geht grundsätzlich um eine Designentscheidung:
> Kurz-und-knapp-mathematisch oder eher in Prosaform.
>
> Spätestens wenn man ein Programm mit sagen wir 300 Zeilen durcharbeiten
> (zB Bugs finden) muss, macht es nen Unterschied ob es 300 oder 1500
> Zeilen sind

Wenn die einzelnen Funktionen kurz und übersichtlich sind, weil man 
vernünftig programmiert hat, ist es relativ egal ob die gesamte Software 
nun aus 300 oder 3.000 Zeilen Code besteht.

Code wird jedenfalls nicht automatisch besser, wenn man mehr davon in 
eine Zeile packt. In dem Beispiel oben mag es okay sein, aber oft genug 
führt "Zeilenquetscherei" eher zu schlechter lesbarem und wartbarem 
Code.

[Einer K. (Gast)]

Philipp_K59 schrieb:
> Oder ganz easy..
>
> ledState = !ledState?HIGH:LOW;

? ?

 ledState = !(ledState?HIGH:LOW);
 ledState = (!ledState)?HIGH:LOW;

 ledState = ledState?LOW:HIGH);

 ledState = !ledState;

Naja, ob der Trenäre Operator in einem "Blink without delay" gut 
aufgehoben ist, möchte ich bezweifeln.

[vodoo (Gast)]

Mark B. schrieb:
> aber oft genug
> führt "Zeilenquetscherei" eher zu schlechter lesbarem und wartbarem
> Code.

Das liegt immer am Programmierer selbst.

[vodoo (Gast)]

Arduino Fanboy D. schrieb:
> Trenäre Operator

ledState = (ledState == HIGH) ?  LOW : HIGH ;

da müsste ich jetzt auch 2 mal hinsehen, schön isses nicht aber wenn 
schon so dann so.

[Einer K. (Gast)]

vodoo schrieb:
> (ledState == HIGH)

vodoo schrieb:
> aber wenn schon so dann so.

Da bin ich gegen...
Ein Vergleich mit HIGH ist unsinnig, wenn die Variable sowieso nur HIGH 
und LOW werden kann.

(ledState == HIGH)
Würde zu
(HIGH == HIGH)
oder
(LOW == HIGH)

Einfach nur (ledState) ist an der Stelle völlig ausreichend
(Die Klammern sind auch noch über)

[imKeller (Gast)]

Arduino erregt die Geister

Ah , die kostbare Zeit !!!

[MaWin (Gast)]

Arduino ist gut! Ebenso Bascom!

[vodoo (Gast)]

Arduino Fanboy D. schrieb:
> (Die Klammern sind auch noch über)

klammern waren ja wegen dem vergleich und an dem würde ich unbedingt 
festhalten da sich ja high, low verändern könnten in anderen 
arduinoversionen, so ist dat sicher!



Nehmt den Arduino, er ist sehr gut!

[Einer K. (Gast)]

vodoo schrieb:
> so ist dat sicher!

Ob du hier
ledState = (ledState == HIGH) ?  LOW : HIGH ;
oder
ledState = ledState == HIGH ?  LOW : HIGH ;
oder
ledState = ledState ?  LOW : HIGH ;
oder
ledState = not ledState;
schreibst ist völlig irrelvant.
Komme was wolle

[Einer K. (Gast)]

Karl Max schrieb im Beitrag #5865771:
> und/oder in der Variablendeklaration

Wenn das auf meinen Mist gewachsen wäre, würde sicherlich sowas da 
stehen:
> bool ledState;

[Farim (Gast)]

Man sollte sich den ganzen Code ansehen:

// constants won't change. Used here to set a pin number:
const int ledPin =  LED_BUILTIN;// the number of the LED pin

// Variables will change:
int ledState = LOW;             // ledState used to set the LED

// Generally, you should use "unsigned long" for variables that hold time
// The value will quickly become too large for an int to store
unsigned long previousMillis = 0;        // will store last time LED was updated

// constants won't change:
const long interval = 1000;           // interval at which to blink (milliseconds)

void setup() {
  // set the digital pin as output:
  pinMode(ledPin, OUTPUT);
}

void loop() {
  // here is where you'd put code that needs to be running all the time.

  // check to see if it's time to blink the LED; that is, if the difference
  // between the current time and last time you blinked the LED is bigger than
  // the interval at which you want to blink the LED.
  unsigned long currentMillis = millis();

  if (currentMillis - previousMillis >= interval) {
    // save the last time you blinked the LED
    previousMillis = currentMillis;

    // if the LED is off turn it on and vice-versa:
    if (ledState == LOW) {
      ledState = HIGH;
    } else {
      ledState = LOW;
    }

    // set the LED with the ledState of the variable:
    digitalWrite(ledPin, ledState);
  }
}

[Maxim B. (max182)]

Karli schrieb:
> Wäre es nicht sinnvoll, diesen durch    ledState = !ledstate;
> zu ersetzen?

Am besten kuckt man Assembler-Code von beiden übersetzten Varianten.
Ich denke, deine Variante erzeugt mehr Code, da zuerst Pin-Zustand 
gelesen sein muß, danach invertiert und wieder in Port geschrieben.
Etwa so:

in r16, PORTB
ldi r17, BITMASK
eor r16, r17
out PORTB, r16

insgesamt 4 Word 4 Cycles

Wenn einfach auf 0 oder 1 setzen, dann wird für die meisten ATMega-Ports 
kürzere Behehl von Compiler gewählt. Z.B.

clr PORTB, PB5

oder

setb PORTB, PB5

Jeweils 1 Word 2 Cycles.

Das ist immer so: wenn zu Compilieren-Zeit alle Daten bekannt sind, 
bekommt man kürzere Maschinencode als mit Variablen.

[Einer K. (Gast)]

Maxim B. schrieb:
> Ich denke, deine Variante erzeugt mehr Code, da zuerst Pin-Zustand
> gelesen sein muß, danach invertiert und wieder in Port geschrieben.

Da ist nix mit Port!
Eine einfache Variable, es ist.

[Maxim B. (max182)]

Ist ledState eine Variable in RAM, die nichts außen macht?
Wenn man damit LED ansteuern will, dann ist das bestimmt Portpin.

Es ist besser, keine Variablen dort zu benutzen, wo es um im voraus 
bekannte Ports geht. So wird Programm schneller und kürzer.

Sonst bekommen wir 2000 Worte in Code, wo auch 5 genügt.

[Einer K. (Gast)]

Farim schrieb:
> Man sollte sich den ganzen Code ansehen:

Oer es gleich anders machen....

    class SimpleTimer
    {
      private:
      uint32_t timeStamp;      // Zeitmerker
      bool reached;            // default Status: timer abgelaufen
    
      public:
      SimpleTimer():timeStamp(0),reached(true){}
      
      void start()
      {
        timeStamp   = millis();
        reached     = false;
      }
      
      void reset()
      {
         reached     = true;
      }
    
      bool operator()(const uint32_t interval) 
      {
        if(!reached) reached = millis() - timeStamp >= interval;
        return reached;
      }
    };

const byte led  =  LED_BUILTIN;
const unsigned long interval = 500UL;
SimpleTimer timer;

void setup() 
{
 pinMode(led,OUTPUT);
 // timer.start(); // wenn der Timer mit Wartezeit beginnen soll
}

void loop() 
{
  if(timer(interval))
  {
    digitalWrite(led,!digitalRead(led));
    timer.start();
  }

  // hier tue anderes Zeugs
}

Natürlich kann die Ausgabe durch direkte Registerzugriffe deutlich 
beschleunigt werden.
> PORTB = _BV(PB5); // toggelt den Pin in einem Takt

[Stefan F. (Gast)]

Arduino Fanboy D. schrieb:
> Natürlich kann die Ausgabe durch direkte Registerzugriffe deutlich
> beschleunigt werden.
>> PORTB = _BV(PB5); // toggelt den Pin in einem Takt

Möp. Dazu musst du das PINB Register beschreiben. Aber erkläre das mal 
einem Prinzipienreiter (wenn man einen Output machen will, soll das auch 
so heissen ...).

[Einer K. (Gast)]

Stefanus F. schrieb:
> Möp. Dazu musst du das PINB Register beschreiben.

Da hast du wahr!

Arduino Fanboy D. schrieb:
>> PORTB = _BV(PB5); // toggelt den Pin in einem Takt

PINB = _BV(PB5); // toggelt den Pin in einem Takt
Muss es heißen.

[GEKU (Gast)]

gurgl schrieb:
> Was du meinst ist XOR, ^

Der Unterschied zwischen ^ und ~ ist:

^ ändert das ganz Port (XOR) und
~ nur ein Bit (NOT)

https://de.m.wikibooks.org/wiki/C-Programmierung:_Liste_der_Operatoren_nach_Priorität

[leo (Gast)]

GEKU schrieb:
> Der Unterschied zwischen ^ und ~ ist:
>
> ^ ändert das ganz Port (XOR) und
> ~ nur ein Bit (NOT)

Bitte poste keinen Unsinn.
leo

[GEKU (Gast)]

Sorry die Hitze

leo schrieb:
> Bitte poste keinen Unsinn.

[Hummelfan (Gast)]

Wenn ich beim Arduino schnell nen einfachen Blinker brauche, nehm ich 
folgende Zeile:
digitalWrite(LED_BUILTIN, millis() & 0x80); // Blinken mit 4Hz Takt

[Matthias S. (da_user)]

Hummelfan schrieb:
> Wenn ich beim Arduino schnell nen einfachen Blinker brauche, nehm ich
> folgende Zeile:
> digitalWrite(LED_BUILTIN, millis() & 0x80); // Blinken mit 4Hz Takt

Das erinnert mich an den "Blink-Merker" bei der S7 ;-)

[Maxim B. (max182)]

Hummelfan schrieb:
> Wenn ich beim Arduino schnell nen einfachen Blinker brauche, nehm ich
> folgende Zeile:
> digitalWrite(LED_BUILTIN, millis() & 0x80); // Blinken mit 4Hz Takt

Klar: man kann auch mit einem Mikroskop statt Hammer die Nägel 
einschlagen...

[A. S. (Gast)]

Mark B. schrieb:
> Wenn die einzelnen Funktionen kurz und übersichtlich sind, weil man
> vernünftig programmiert hat, ist es relativ egal ob die gesamte Software
> nun aus 300 oder 3.000 Zeilen Code besteht.

Das sehe ich genauso. Auch 30.000 Zeilen kann man noch nach Lehrbuch 
coden, so mit McCabe einstellig und nur eine Bildschirm-Seite. Skalieren 
tut es aber (leider) nicht.