Code kürzen?

International Deutsch
1

Hallo Zusammen,

ich möchte meine Beleuchtung+Steckdosen gerne über eine Fernbedienung steuern.
Die Sache mit der Beleuchtung kommt noch.
Die Steckdosen lassen sich wie gewollt über ein IR Fernbedienung steuern.

Ich würde gerne wissen wie ich die switch case Sache eventuell verbessern, bzw sachgerecht benutzen könnte und ja ich weiß ist etwas schlampig geschrieben.

#include <TimerOne.h>
#include <MeetAndroid.h>
#include <IRremote.h>
//---------------Bluetooth----------------------//
MeetAndroid meetAndroid;

//--------------Funk----------------------------//
#include <RCSwitch.h>

RCSwitch mySwitch = RCSwitch();
// --------------Infrarot------------------------//
int  IRLed = 30; //IR Led
IRrecv irrecv(IRLed);
decode_results results;

//--------------RGB LED Konstanten-----------//
int ledr1 = 2;
int ledg1 = 3;
int ledb1 = 4;
int ledr2 = 5;
int ledg2 = 6;
int ledb2 = 7;
int ledr3 = 8;
int ledg3 = 9;
int ledb3 = 10;
int ledr4 = 11;
int ledg4 = 12;
int ledb4 = 13;


//------------------Variablen-----------------//
boolean Dose1, Dose2, Dose3, Dose4, Dose5, Dose6; //Steckdose//

int RGB1, RGB2, RGB3, RGB4;   // Led Stripe//
//------------------------------------------//

//------Funktionen---------//
//--------------Dose1--------------------//
void dose1 ()
{
 if (Dose1 == 0)
 {
mySwitch.switchOn("11111", 1);
Dose1++;
  Serial.println("Dose1 AN");
  
}
else
{
  mySwitch.switchOff("11111", 1);
  Dose1--;
  Serial.println("Dose1 AUS");
  
}
}
//-------------------Dose2-----------------------//
void dose2 ()
{
  if (Dose2 == 0)
  {
    mySwitch.switchOn("11111", 2);
  Dose2++;
  Serial.println("Dose 2 An");
    }
    else
    {
      mySwitch.switchOff("11111", 2);
    Dose2--;
    Serial.println (" Dose2 Aus");
    }
}
//-------------------Dose3------------------------//
void dose3 ()
{
  if (Dose3 == 0)
  {
    mySwitch.switchOn("11111", 3);
  Dose3++;
  Serial.println("Dose3 AN");
  }
  else
  {
    mySwitch.switchOff("11111", 3);
    Dose3--;
    Serial.println("Dose3 Aus");
  }
}
//---------------Dose4---------------------------------------//
void dose4 ()
{
  if (Dose4 == 0)
  {
  mySwitch.switchOn("11111", 4);
  Dose4++;
  Serial.println("Dose4 AN");
}
else
{
  mySwitch.switchOff("11111", 4);
  Dose4--;
  Serial.println("Dose4 AUS");
}
}
//-----------------------Alle Dosen aus-------------------------------------//
void aus()
{
  if(Dose1 || Dose2|| Dose3|| Dose4|| Dose5 || Dose6 ==1)     // Abfrage ob eine Dose an ist
  {
    mySwitch.switchOff("11111", 1);
        mySwitch.switchOff("11111", 2);
            mySwitch.switchOff("11111", 3);
                mySwitch.switchOff("11111", 4);
                    mySwitch.switchOff("11111", 5);
                        mySwitch.switchOff("11111", 6);
    Dose1 = 0, Dose2 = 0, Dose3 = 0, Dose4 = 0, Dose5 = 0, Dose6 = 0;
    Serial.println("Alle Dosen aus");
    
  }
}
//------------------------LED Stripe 1-----------------------//
// Platzhalter
//------------------------------------------------------
void setup()
{Serial.begin(9600);  //Start Serial//
irrecv.enableIRIn();  //Start IR Reciver//
mySwitch.enableTransmit(31); // Funk Empfänger start Pin 31//
}
void loop()
//---------Infrarot Eingabe auslesn-----------------//
{ int result;
if (irrecv.decode(&results))
 {  result = results.value, 0x00ff;      
   Serial.println(results.value, HEX);      
         
    
 
    //----------------Funksteckdosen------------------------//
    switch (result)
    {
    case 0x00F2E30C4: dose1(); break;
    case 0x003A7DB5B8: dose2(); break;
    case 0x0067D88E78: dose3(); break;
    case 0x009328136C: dose4(); break;
    //case 0x00: dose5(); break;
    //case 0x00: dose6(); break;
    case 0x00CE75E914: aus(); break;
  }
  irrecv.resume(); // Nexte Eingabe//
}
}

Besten Dank schon mal im voraus.

2

Wenn Du schon weisst, daß es schlampig geschrieben ist, warum räumst Du nicht zuerst einmal auf?

Absolulte Minimalanforderung ist, daß die Einrückungen sauber sind. Weiterhin würde ich dafür plädieren Konstante auch mit "const" zu deklarieren.

if(Dose1 || Dose2|| Dose3|| Dose4|| Dose5 || Dose6 ==1)

funktioniert zwar wohl zufällig, aber so wie es dasteht ist es vermutlich nicht gemeint.

Was ist Dein Problem mit dem "switch"?

3

Hallo Udo,

der zitierte Codeschnippsel ist mir auch gleich aufgefallen. Allerdings ist die Sache mit den Konstanten wohl eher eine Glaubensfrage, oder ?
Ich selber deklariere auch immer so....
Konkret zum Codeschnippsel: Das letze Vergleichen mit 1 (==1) kannst Du Dir tatsächlich sparen weil Du alle vorhergehenden Variablen ebenfalls im Unterbewusssein mit 1 vergleichst :slight_smile:

M.E.: Im Vergleich zu anderen Posts sieht Dein Code schon sehr sauber aus.

Viele Grüße

Jörg

4

Bestimmte Regeln (einrücken, sinnvolle Variablennamen, verständliche und erklärende Komentare) soll man sich immer angewöhnen egal ob der Sketch ein 20 Zeiler wird oder es am Ende ein paar Tausend sein werden. Das Einrücken hilft bei der Leserlichkeit und darum bei der Fehlersuche. Aus meiner PHP Erfahrung wird eine Klammer zuwenig immer in der letzten Zeile angezeigt.
Grüße Uwe

5

Allerdings ist die Sache mit den Konstanten wohl eher eine Glaubensfrage, oder ?

Wenn Du glaubst, daß Compiler immer ohne Hilfe richtig optimieren und wenn Du glaubst, daß Du nie aus versehen eine Konstante überschreibst, und wenn Du glaubst, daß Du immer alleine programmierst, dann ist es eine Glaubensfrage.

Ansonsten sollte "const" eher großzügig verwendet werden. Ich verwende const mittlerweile auch häufig bei Variablen. DAS ist dann schon eher eine Glaubensfrage. Meiner Meinung nach hätte man das Schlüsselwort auch besser "immuteable" genannt. Das wäre nicht so sugestiv. Und noch besser sollten eigentlich alle Variablen immer "immutable" oder "final" sein außer man sagt explizit etwas anderes. Wie gesagt, DAS ist eine Glaubensfrage. Aber Konstanten mit "const" zu deklarieren ist eigentlich standard.

6

Nachtrag: 2 Dinge die am Code strukturell falsch sind.

  1. Die Verwendung von "int" für die Dosen wo ein boolean eher angebracht wäre.
  2. Die Verwendung von n im Funktionsnamen statt im Argument. D.h. ich würde nur ein Funktion
void dose(const uint8_t n)
Serial.print("Dose");
Serial.print(n);
{
 if (Dose[n])
{
  mySwitch.switchOn("11111", 1);
  Serial.println(" AN");
}
else
{
  mySwitch.switchOff("11111", 1);
  Serial.println(" AUS");
}
Dose[n] = !Dose[n];
}

Und natürlich die Dosen als Array deklarieren.

boolean Dose[6] = {false, false, false, false, false, false};

Danach können die Dosen auch in Schleifen verarbeitet werden.

7

Hey Udo,

du kannst es mir bestimmt erklären: wieso verwendest du (verwendet man) "uint8_t" anstatt "byte"? Bin schon öfters darüber gestolpert, hab aber keinen unterschied zwischen den beiden Typen feststellen können?

8

Wahrscheinlich aus Gewohnheit.

"uint8_t" heißt Unsigned INTeger 8 Bit (Rest weiß ich nicht). Das definiert auf ALLEN Systemen auf denen der Code compiliert wird, eine Variable zu 1 Byte (8Bit). Bezeichnungen wie INT sind Systemabhängig Arduino hat 2 Byte, PCs 4 Byte usw.
Mit "uint8_t" wießt Du einfach genau wie Deine Variable auf allen Systemen sein wird.

wenn Du noch weiterlesen willst: Unterschied zw. uint8_t und unsigned int - Mikrocontroller.net

Grüße Uwe

9

Naja, technisch sind die auf den Atmels gleich.

Ein Byte ist ein byte Byte – Wikipedia. Ein uint8_t sind 8 Bits. Auf dem Atmel ist das beides 8 Bits lang, aber auf anderen Maschinen nicht. Wenn ich also "byte" deklariere, dann sage ich "das kleinste Häppchen was sich gut in einem Prozessorregister ausrichten lässt". Wenn ich "uint8_t" schreibe, dann sage ich "8 Bit". Nun sind beim AVR die beiden Typen gleich. Die Bedeutung ist aber eine andere. Wenn ich "Byte" schreibe, dann sage ich eigentlich "kleinste und hoffentlich schnellstens berechenbare Einheit". Wenn ich "uint8_t" schreibe, dann sage ich "wenigstens 8 Bits", egal ob das jetzt maximal schnell geht oder nicht.

Auf dem AVR aber eher Geschmackssache. Ich bin zu faul mir zu merken wieviele Bits die Maschine mit der ich gerade arbeite gerade hat. Das Problem tritt heutzutage aber eher bei den langen Typen auf. D.h. 32bit vs. 64bit Integers.

Wenn man es super richtig machen will sollte man überhaupt keine solchen Typen verwenden sondern ein Dictionary mit "semantischen" Typen verwenden. D.h. niemals technisch sondern immer semantisch typen. Also nicht "uint8_t" oder "int" sondern

typedef uint8_t key_code;
typedef uint16_t counter;
...

key_code my_key;
counter   n;

In C mache ich mir nicht immer die Mühe. In höheren Sprachen halte ich das aber für sehr angebracht. Und nein, ECMAskript ist keine höhere Sprache, das ist eine Mischung aus Unfall und Klebstoff :wink:

10

Man frägt sich was eingentlich ein C Compiler aus einem Byte für eine PDP 10 macht :wink: