Modellflugzeug Beleuchtung mit PPM Signal

Moin ihr lieben,

mal eine kleine Spielerei von meiner Flugzeugbeleuchtung:

//Plane_Lights v0a UnoArduSim
/******** preprocessor ********/
/******** pin config ********/
const uint8_t PPM_pin = 2;
//4=bnc, 5-6=strobe wing, 7-8=nav, 9=land, 10=strobe tail
const uint8_t Light_pins[7] = {4, 5, 6, 7, 8, 9, 10};
/******** global constants ********/
//switch states 1000 - 2000us (+100/-100 safety space)
const uint32_t Beacon_PPM = 1100, Strobe_PPM = 1300, Nav_PPM = 1500;
const uint32_t Landing_PPM = 1700, Landing_only_PPM = 1900;
/******** global variables ********/
volatile uint32_t PPM_start = 0, PPM_stop = 0, PPM_us = 0;
/******** functions ********/
void get_PPM()
{
  if (digitalRead(PPM_pin))
    //if (bitRead(PIND, PD2)) //Pin 2 UNO
  {
    PPM_start = micros();
  } else {
    PPM_stop = micros();
  }
  if (PPM_start < PPM_stop) PPM_us = PPM_stop - PPM_start;
}//void get_PPM() END [ISR]
/******** function prototypes ********/

void setup()
{
  for (uint8_t id = 0; id < sizeof(Light_pins); id++)
  {
    pinMode(Light_pins[id], OUTPUT);
  }
  //attachInterrupt(digitalPinToInterrupt(PPM_pin), get_PPM, CHANGE);
  attachInterrupt(0, get_PPM, CHANGE);
}//void setup() END

void loop()
{
  static uint32_t total_interval_ms;
  uint32_t interval_ms, act_ms = millis();

  //interval update
  if (act_ms - total_interval_ms >= 2000) total_interval_ms = act_ms;
  interval_ms = act_ms - total_interval_ms;

  //Nav
  if (PPM_us >= Nav_PPM && PPM_us < Landing_only_PPM)
  {
    digitalWrite(Light_pins[3], HIGH);
    digitalWrite(Light_pins[4], HIGH);
    //bitSet(PORTD, PD7);
    //bitSet(PORTB, PB0);
  } else {
    digitalWrite(Light_pins[3], LOW);
    digitalWrite(Light_pins[4], LOW);
    //bitClear(PORTD, PD7);
    //bitClear(PORTB, PB0);
  }
  //Landing
  if (PPM_us >= Landing_PPM)
  {
    digitalWrite(Light_pins[5], HIGH);
    //bitSet(PORTB, PB1);
  } else {
    digitalWrite(Light_pins[5], LOW);
    //bitClear(PORTB, PB1);
  }
  //Strobe:
  if (PPM_us >= Strobe_PPM && PPM_us < Landing_only_PPM)
  {
    //Strobe wing
    if ((interval_ms > 0 && interval_ms < 50) ||
        (interval_ms >= 100 && interval_ms < 150))
    {
      digitalWrite(Light_pins[1], HIGH);
      digitalWrite(Light_pins[2], HIGH);
      //bitSet(PORTD, PD5);
      //bitSet(PORTD, PD6);
    } else {
      digitalWrite(Light_pins[1], LOW);
      digitalWrite(Light_pins[2], LOW);
      //bitClear(PORTD, PD5);
      //bitClear(PORTD, PD6);
    }
    //Strobe tail
    if (interval_ms > 0 && interval_ms < 100)
    {
      digitalWrite(Light_pins[6], HIGH);
      //bitSet(PORTB, PB2);
    } else {
      digitalWrite(Light_pins[6], LOW);
      //bitSet(PORTB, PB2);
    }
  }
  //Beacon
  if (((interval_ms >= 500 && interval_ms < 600) ||
       (interval_ms >= 1500 && interval_ms < 1600)) &&
      (PPM_us >= Beacon_PPM && PPM_us < Landing_only_PPM))
  {
    digitalWrite(Light_pins[0], HIGH);
    //bitSet(PORTD, PD4);
  } else {
    digitalWrite(Light_pins[0], LOW);
    //bitClear(PORTD, PD4);
  }
}//void loop() END

:slight_smile:

UAS_IO.txt (1.15 KB)

Sowas habe ich auch mal gebastelt..

Das habe ich allerdings direkt über i2c in die Arducopter Firmware integriert.

void get_PPM()
{
  if (digitalRead(PPM_pin))
    //if (bitRead(PIND, PD2)) //Pin 2 UNO
  {
    PPM_start = micros();
  } else {
    PPM_stop = micros();
  }
  if (PPM_start < PPM_stop) PPM_us = PPM_stop - PPM_start;
}//void get_PPM() END [ISR]

Warum nicht einfacher?

void get_PPM() {
  if (digitalRead(PPM_pin)) {
    PPM_start = micros();
  } else {
    PPM_stop = micros();
    PPM_us = PPM_stop - PPM_start;
  }
}

Warum kommentierst du die bessere Lösung aus?

  //attachInterrupt(digitalPinToInterrupt(PPM_pin), get_PPM, CHANGE);
  attachInterrupt(0, get_PPM, CHANGE);

Wozu dienen die ganzen auskommentierten alternativen Portzugriffe?

Speziell im Kopfteil empfinde ich deine ****** Kommentare als nicht hilfreich
da sie einen unübersichtlichen Block von Text erzeugen und Offensichtlichkeiten erklären.
Gleiches gilt für z.B. //void get_PPM() END [ISR]

volatile uint32_t PPM_start = 0, PPM_stop = 0, PPM_us = 0;

Nur PPM_us muss volatile sein, falls ich nichts übersehen habe.

Guten Morgen,

ich danke Dir für dein Feedback, welches ich gerne in Kauf nehme.
Der Teil mit den // auskommentierten Bereichen LowLevel Funktionen oder eben Portzugriff, ist leider nötig, da diese Version für UnoArduSim erstellt wurde. (Siehe Änderungsgrund & erste Zeile)

Dein Tipp mit der PPM_us Berechnung in get_PPM() finde ich gut. Es kann jedoch nich ein bischen eingespaart werden: die Variable PPM_stop kann man sich komplett sparen. :slight_smile:
Meines Wissens nach sollte jede Variable, welche in einer Interruptroutine aufgerufen/bearbeitet wird als volatile deklariert sein.

Das mit den Kommentaren finde ich hilfreich, da ich somit schnell zwischen den verschiedenen Variablen unterscheiden kann. Die Kommentare an den Enden einer Funktion dienen mir als klarer Indiz für den Abschluss einer Funktion. (das Ganze macht erst in grösseren Projekten)

Ich danke Dir, dass Du dir die Zeit genommen hast mal über meinen Sketch schon blicken.
Gruß grillgemuese

Whandall:

void get_PPM()

{
  if (digitalRead(PPM_pin))
    //if (bitRead(PIND, PD2)) //Pin 2 UNO
  {
    PPM_start = micros();
  } else {
    PPM_stop = micros();
  }
  if (PPM_start < PPM_stop) PPM_us = PPM_stop - PPM_start;
}//void get_PPM() END [ISR]



Warum nicht einfacher?


void get_PPM() {
  if (digitalRead(PPM_pin)) {
    PPM_start = micros();
  } else {
    PPM_stop = micros();
    PPM_us = PPM_stop - PPM_start;
  }
}



Warum kommentierst du die bessere Lösung aus?


//attachInterrupt(digitalPinToInterrupt(PPM_pin), get_PPM, CHANGE);
  attachInterrupt(0, get_PPM, CHANGE);



Wozu dienen die ganzen auskommentierten alternativen Portzugriffe?

Speziell im Kopfteil empfinde ich deine ****** Kommentare als nicht hilfreich
da sie einen unübersichtlichen Block von Text erzeugen und Offensichtlichkeiten erklären.
Gleiches gilt für z.B. //void get_PPM() END [ISR]



volatile uint32_t PPM_start = 0, PPM_stop = 0, PPM_us = 0;



Nur PPM_us muss volatile sein, falls ich nichts übersehen habe.

grillgemuese:
Der Teil mit den // auskommentierten Bereichen LowLevel Funktionen oder eben Portzugriff, ist leider nötig, da diese Version für UnoArduSim erstellt wurde. (Siehe Änderungsgrund & erste Zeile)

Das ist für mich kein Grund für auskommentierten Kram.

grillgemuese:
Meines Wissens nach sollte jede Variable, welche in einer Interruptroutine aufgerufen/bearbeitet wird als volatile deklariert sein.

Das ist falsches 'Wissen'.

volatile ist nötig für Variablen die in einer ISR und in nicht-ISR Kode benutzt werden.

Zugriffe auf PPM_us aus dem nicht-ISR Kode müssen atomar ausgeführt werden, das ist noch ein Fehler in deinem Kode.

Whandall:
Das ist für mich kein Grund für auskommentierten Kram.

Gut, für mich aber schon. :confused:

leicht angepasste Version:

//Plane_Lights v0 for UnoArduSim
/******** pin config ********/
const uint8_t PPM_pin = 2;
//4=bnc, 5-6=strobe wing, 7-8=nav, 9=land, 10=strobe tail
const uint8_t Light_pins[7] = {4, 5, 6, 7, 8, 9, 10};
/******** global constants ********/
//switch states 1000 - 2000us
const uint32_t Beacon_PPM = 1100, Strobe_PPM = 1300, Nav_PPM = 1500;
const uint32_t Landing_PPM = 1700, Landing_only_PPM = 1900;
/******** global variables ********/
volatile uint32_t PPM_us = 0;
/******** functions ********/
void get_PPM()
{
  static uint32_t PPM_start;
  if (digitalRead(PPM_pin))
  //if (bitRead(PIND, PD2)) //Pin 2 UNO
  {
    PPM_start = micros();
  } else {
    PPM_us = micros() - PPM_start;
  }
}//void get_PPM() END

void setup()
{
  for (uint8_t id = 0; id < sizeof(Light_pins); id++)
  {
    pinMode(Light_pins[id], OUTPUT);
  }
  attachInterrupt(0, get_PPM, CHANGE);
  //attachInterrupt(digitalPinToInterrupt(PPM_pin), get_PPM, CHANGE);
}//void setup() END

void loop()
{
  static uint32_t total_interval_ms;
  uint32_t interval_ms = 0, act_ms = millis();
  uint32_t safe_PPM_us = PPM_us;
  //interval update
  if (act_ms - total_interval_ms >= 2000) total_interval_ms = act_ms;
  interval_ms = act_ms - total_interval_ms;

  //Nav
  if (safe_PPM_us >= Nav_PPM && safe_PPM_us < Landing_only_PPM)
  {
    digitalWrite(Light_pins[3], HIGH);
    digitalWrite(Light_pins[4], HIGH);
    //bitSet(PORTD, PD7);
    //bitSet(PORTB, PB0);
  } else {
    digitalWrite(Light_pins[3], LOW);
    digitalWrite(Light_pins[4], LOW);
    //bitClear(PORTD, PD7);
    //bitClear(PORTB, PB0);
  }
  //Landing
  if (safe_PPM_us >= Landing_PPM)
  {
    digitalWrite(Light_pins[5], HIGH);
    //bitSet(PORTB, PB1);
  } else {
    digitalWrite(Light_pins[5], LOW);
    //bitClear(PORTB, PB1);
  }
  //Strobe:
  if (safe_PPM_us >= Strobe_PPM && safe_PPM_us < Landing_only_PPM)
  {
    //Strobe wing
    if ((interval_ms > 0 && interval_ms < 50) ||
        (interval_ms >= 100 && interval_ms < 150))
    {
      digitalWrite(Light_pins[1], HIGH);
      digitalWrite(Light_pins[2], HIGH);
      //bitSet(PORTD, PD5);
      //bitSet(PORTD, PD6);
    } else {
      digitalWrite(Light_pins[1], LOW);
      digitalWrite(Light_pins[2], LOW);
      //bitClear(PORTD, PD5);
      //bitClear(PORTD, PD6);
    }
    //Strobe tail
    if (interval_ms > 0 && interval_ms < 100)
    {
      digitalWrite(Light_pins[6], HIGH);
      //bitSet(PORTB, PB2);
    } else {
      digitalWrite(Light_pins[6], LOW);
      //bitClear(PORTB, PB2);
    }
  }
  //Beacon
  if (((interval_ms >= 500 && interval_ms < 600) ||
       (interval_ms >= 1500 && interval_ms < 1600)) &&
      (safe_PPM_us >= Beacon_PPM && safe_PPM_us < Landing_only_PPM))
  {
    digitalWrite(Light_pins[0], HIGH);
    //bitSet(PORTD, PD4);
  } else {
    digitalWrite(Light_pins[0], LOW);
    //bitClear(PORTD, PD4);
  }
}//void loop() END

Dir fehlt immer noch der atomare Zugriff auf PPM_us.

  noInterrupts();
uint32_t PPM_us_copy = PPM_us;
  Interrupts();

  //Nav
  if (PPM_us_copy >= Nav_PPM && PPM_us < Landing_only_PPM)

Whandall:
Dir fehlt immer noch der atomare Zugriff auf PPM_us.

  noInterrupts();

uint32_t PPM_us_copy = PPM_us;
  Interrupts();

//Nav
  if (PPM_us_copy >= Nav_PPM && PPM_us < Landing_only_PPM)

Dessen Funktion ist doch vergleichbar mit einer Laufzeit synchronen / asynchronen Variablen Aktualisierung, oder?

Keine Ahnung zu was das gleich ist, es ist jedenfalls notwendig um einen konsistenten Wert sicherzustellen.

Die Variable hat mehr als ein Byte und ist volatile, ergo muss sie mit ausgeschalteten Interrupts gelesen werden.
An der Kopie darf sich der Optimizer wieder austoben, er könnte also hier durchaus kürzeren/schnelleren Kode erzeugen,
auch wenn die Unterschiede eher marginal sein sollten.

Deine PPM Zeiten und Konstanten würden auch in einen uint16_t passen,
was den Kode auch wieder verkleinert und beschleunigt.
Oder du erstellst die Arbeitskopie in einem uint16_t.

  noInterrupts();
uint16_t PPM_us_copy = PPM_us;
  Interrupts();

  //Nav
  if (PPM_us_copy >= Nav_PPM && PPM_us < Landing_only_PPM)

aktualisierte Version:

//Plane_Lights v0 for UnoArduSim
/******** pin config ********/
const uint8_t PPM_pin = 2;
//4=bnc, 5-6=strobe wing, 7-8=nav, 9=land, 10=strobe tail
const uint8_t Light_pins[7] = {4, 5, 6, 7, 8, 9, 10};
/******** global constants ********/
//switch states 1000 - 2000us
const uint16_t Beacon_PPM = 1100, Strobe_PPM = 1300, Nav_PPM = 1500;
const uint16_t Landing_PPM = 1700, Landing_only_PPM = 1900;
/******** global variables ********/
volatile uint32_t PPM_us = 0;
/******** functions ********/
void get_PPM()
{
  static uint32_t PPM_start;
  if (digitalRead(PPM_pin))
  //if (bitRead(PIND, PD2)) //Pin 2 UNO
  {
    PPM_start = micros();
  } else {
    PPM_us = micros() - PPM_start;
  }
}//void get_PPM() END

void setup()
{
  for (uint8_t id = 0; id < sizeof(Light_pins); id++)
  {
    pinMode(Light_pins[id], OUTPUT);
  }
  attachInterrupt(0, get_PPM, CHANGE);
  //attachInterrupt(digitalPinToInterrupt(PPM_pin), get_PPM, CHANGE);
}//void setup() END

void loop()
{
  static uint32_t total_interval_ms;
  uint32_t interval_ms = 0, act_ms = millis();
  noInterrupts();
  uint16_t safe_PPM_us = (uint16_t)PPM_us;
  interrupts();
  //interval update
  if (act_ms - total_interval_ms >= 2000) total_interval_ms = act_ms;
  interval_ms = act_ms - total_interval_ms;

  //Nav
  if (safe_PPM_us >= Nav_PPM && safe_PPM_us < Landing_only_PPM)
  {
    digitalWrite(Light_pins[3], HIGH);
    digitalWrite(Light_pins[4], HIGH);
    //bitSet(PORTD, PD7);
    //bitSet(PORTB, PB0);
  } else {
    digitalWrite(Light_pins[3], LOW);
    digitalWrite(Light_pins[4], LOW);
    //bitClear(PORTD, PD7);
    //bitClear(PORTB, PB0);
  }
  //Landing
  if (safe_PPM_us >= Landing_PPM)
  {
    digitalWrite(Light_pins[5], HIGH);
    //bitSet(PORTB, PB1);
  } else {
    digitalWrite(Light_pins[5], LOW);
    //bitClear(PORTB, PB1);
  }
  //Strobe:
  if (safe_PPM_us >= Strobe_PPM && safe_PPM_us < Landing_only_PPM)
  {
    //Strobe wing
    if ((interval_ms > 0 && interval_ms < 50) ||
        (interval_ms >= 100 && interval_ms < 150))
    {
      digitalWrite(Light_pins[1], HIGH);
      digitalWrite(Light_pins[2], HIGH);
      //bitSet(PORTD, PD5);
      //bitSet(PORTD, PD6);
    } else {
      digitalWrite(Light_pins[1], LOW);
      digitalWrite(Light_pins[2], LOW);
      //bitClear(PORTD, PD5);
      //bitClear(PORTD, PD6);
    }
    //Strobe tail
    if (interval_ms > 0 && interval_ms < 100)
    {
      digitalWrite(Light_pins[6], HIGH);
      //bitSet(PORTB, PB2);
    } else {
      digitalWrite(Light_pins[6], LOW);
      //bitClear(PORTB, PB2);
    }
  }
  //Beacon
  if (((interval_ms >= 500 && interval_ms < 600) ||
       (interval_ms >= 1500 && interval_ms < 1600)) &&
      (safe_PPM_us >= Beacon_PPM && safe_PPM_us < Landing_only_PPM))
  {
    digitalWrite(Light_pins[0], HIGH);
    //bitSet(PORTD, PD4);
  } else {
    digitalWrite(Light_pins[0], LOW);
    //bitClear(PORTD, PD4);
  }
}//void loop() END

Whandall:
Deine PPM Zeiten und Konstanten würden auch in einen uint16_t passen,
was den Kode auch wieder verkleinert und beschleunigt.
Oder du erstellst die Arbeitskopie in einem uint16_t.

Danke für diesen Tipp!
Gruß
grillgemuese

Das sieht vom Kode her besser aus. :wink:

Mit den Kommentaren kann ich mich nicht anfreunden,
die stören mich mehr als sie mir helfen.