Output Pins festlegen und verwenden (Anfänger)

[Kaan A. (deranfaenger)]

Hi Leute,

vor ab will ich sagen, dass ich schon etwas herumgegooglet habe und es 
nach meinem jetzigen Stand der Verständnis so funktionieren müsste.

Folgendes Problem:

Ich muss einen Mikrocontroller programmieren, der je nach 
Eingangssignalen PWM-Signale erzeugt und diese an bestimmten Pins 
ausgibt.
Die Ausgangspins habe ich global in einem Array definiert sodass ich 
einfachen zugriff drauf habe:

static const unsigned char *OUTPUT_ARRAY[] = {
  (unsigned char *) 0x05, (unsigned char *) 6, // PB6
  (unsigned char *) 0x05, (unsigned char *) 7, // PB7
  (unsigned char *) 0x0B, (unsigned char *) 5, // PD5
  (unsigned char *) 0x0B, (unsigned char *) 6, // PD6
  (unsigned char *) 0x0B, (unsigned char *) 7, // PD7
  (unsigned char *) 0x05, (unsigned char *) 0  // PB0
};


Nun muss ich in der Timerinterruptroutine nach gewissen Bedingungen den 
Ausgang an diesen Pins 0 bzw. 1 setzen:

(*OUTPUT_ARRAY[i*2]) |= (1 << (int)OUTPUT_ARRAY[i*2 + 1]);

Das funktioniert aber nicht ich bekomme folgende Fehlermeldung:
assignment of read-only location '*OUTPUT_ARRAY[(int)i * 2]'


Kann jemand weiterhelfen?
Is bestimmt ein einfaches Problem :/

[Tom Thomsen (Gast)]

Kaan Ayhan schrieb:
> Kann jemand weiterhelfen?
> Is bestimmt ein einfaches Problem :/

Nun mal ganz langsam. An welchem Mikrocontroller versuchst du dich 
überhaupt? Vielleicht kann der das gar nicht.

[tictactoe (Gast)]

Ich erklär dir hier mal nur die Fehlermeldung.

Kaan Ayhan schrieb:
> Die Ausgangspins habe ich global in einem Array definiert sodass ich
> einfachen zugriff drauf habe:
>
> static const unsigned char *OUTPUT_ARRAY[] = {
> ...
> };
>
> (*OUTPUT_ARRAY[i*2]) |= (1 << (int)OUTPUT_ARRAY[i*2 + 1]);
>
> Das funktioniert aber nicht ich bekomme folgende Fehlermeldung:
> assignment of read-only location '*OUTPUT_ARRAY[(int)i * 2]'

Schauen wir mal, was du hier hast:

static const unsigned char *OUTPUT_ARRAY[] = { ... }
                            ^OUTPUT_ARRAY ist ein
                                        ^Array von
                           ^Pointern auf
       ^nur-lesbare
             ^unsigned char

Also, das, worauf die Pointer zeigen, ist "const" -- nur lesbar. Jetzt 
müsstest die Fehlermeldung eigentlich klipp und klar sein.

Am einfachsten ist sicherlich, das "const" wegzulassen. Das 
funktioniert, weil die "Pointer", die du zuweist, eh alle non-const 
sind:

(unsigned char *) 0x05, (unsigned char *) 6, // PB6

Vielleicht hast du aber auch folgendes gemeint:

static unsigned char * const OUTPUT_ARRAY[] = { ... }
                             ^OUTPUT_ARRAY ist ein
                       ^nur-lesbares
                                         ^Array von
                     ^Pointern auf
       ^unsigned char

Denn du hast sicher nicht vor, nach dem Initialisieren OUTPUT_ARRAY 
selbst zu verändern. Daher kannst du es "const" machen. Aber das muss 
man ganz in der Nähe des Namens hinschreiben. Aber das, worauf die 
Pointer zeigen, bleibt beschreibbar -- genau was du brauchst.

Apropos Namen: Es wäre gut, wenn du dir angewöhnst, Namen aus nur 
Grossbuchstaben für Macros zu reservieren.

[Peter D. (peda)]

Du kannst ganz massiv Code und Zeit sparen, wenn Du bei nur 6 Pins ein 
switch/case nimmst.
Dann kann der Compiler CBI/SBI draus machen und muß nicht erst 
umständlich Pointer aufdröseln und Bitmasken shiften.
Insbesondere im Interrupt spart man damit auch Push/Pop-Orgien.

[Eric B. (beric)]

Kaan Ayhan schrieb:
> Is bestimmt ein einfaches Problem :/

typedef struct
{
  unsigned char * port_address;
  unsigned char   pin_bit;
} tPinConfig;

static const tPinConfig output_pin[] =
{
  { (unsigned char *) 0x05, 1u << 6u },
  /* ... insert other ports here ... */
  { (unsigned char *) 0x05, 1u << 0u },
};

/* ON */
*(output_pin[X].port_address) |= output_pin[X].pin_bit;

/* OFF */
*(output_pin[X].port_address) &= ~output_pin[X].pin_bit;

/* TOGGLE */
*(output_pin[X].port_address) ^= output_pin[X].pin_bit;

[Kaan A. (deranfaenger)]

Hi,

erstmal danke für die schnellen Antworten.

tom, sry dachte habe das dazugeschrieben..  es handelt sich um einen 
ATMega88pa

tictactoe, danke für die super Erklärung. Jetzt funktioniert es genau so 
wie ich es haben möchte :)

Peter, ja du hast wohl Recht... die Interruptroutine dauert zu lange, 
sodass der nächste Interrupt zu früh kommt. Wird wohl an meinem 
Amateurcode liegen.
Wie meinst du das mit der switch-Anweisung? Ich kann mir das nicht so 
genau vorstellen wie das aussehen soll.

Eric, wäre deine schreibweise performanter, oder ist das einfach nur die 
schönere/professionellere Variante?
So muss ich das ja trotzdem durch eine for-Schleife jagen... die sind 
aber wie es ausschaut zu zeitintensiv.

Die Funktion des µC sieht so aus, dass ich 6 digitale In- und Outputpins 
und 2 analoge Inputpins habe. An den analogen Eingängen wird die Phase 
und das Tastverhältnis eingestellt. Über die digitalen Inputpins kann 
der Benutzer festlegen für welche PWM-Ausgänge die Einstellungen 
übernommen werden sollen und in meinen 2 Arrays speichere ich mir eben 
diese Adressen.

Ich hab hier mal eine leicht abgespeckte Version des Codes (Die Funktion 
chooseMag wird hier zwar nicht verwendet aber vielleicht wird für euch 
dadurch deutlicher was ich meine)

#include <avr/io.h>
#include <avr/interrupt.h>

#define NR_OF_MAGS  6

// Define new Type for Magnetron
struct Magnetron {
  float frequency;
  float threshold;
  float phase;
  int currentTime;  // PWM-Time in Timer-frequency
};
// Declare Array of Type Magnetron
struct Magnetron mag[NR_OF_MAGS];
// initialize Magnetrons
void initMag(struct Magnetron *mag){
  mag->frequency = 500;
  mag->threshold = 300;
  mag->phase = 0;
  mag->currentTime = 0;
}

// Magnetron PWM Output Pin addresses
static unsigned char * const output_array[] = {
  (unsigned char *) 0x25, (unsigned char *) 6, // PB6
  (unsigned char *) 0x25, (unsigned char *) 7, // PB7
  (unsigned char *) 0x2B, (unsigned char *) 5, // PD5
  (unsigned char *) 0x2B, (unsigned char *) 6, // PD6
  (unsigned char *) 0x2B, (unsigned char *) 7, // PD7
  (unsigned char *) 0x25, (unsigned char *) 0  // PB0
};

// Magnetron Enable Pin addresses
static const unsigned char * ENABLE_ARRAY[] = {
  (unsigned char *) 0x25, (unsigned char *) 1, // PB1
  (unsigned char *) 0x25, (unsigned char *) 2, // PB2
  (unsigned char *) 0x25, (unsigned char *) 3, // PB3
  (unsigned char *) 0x2B, (unsigned char *) 2, // PD2
  (unsigned char *) 0x2B, (unsigned char *) 3, // PD3
  (unsigned char *) 0x2B, (unsigned char *) 4  // PD4
};

// check witch Magnetrons are choosen to be updated
void chooseMag(char magEnabled[]){
  for (unsigned char i = 0; i < sizeof(magEnabled); i++){
    magEnabled[i] = (*ENABLE_ARRAY[i*2]) & (1 << (int)ENABLE_ARRAY[i*2 + 1]);
  }
}

void setOutput(int scaledTime, unsigned char i){
  if((mag[i].phase + mag[i].threshold) > mag[i].frequency){
    if(scaledTime <= ((mag[i].phase + mag[i].threshold) - mag[i].frequency)) {*output_array[i*2] |= (1<<(int)output_array[i*2+1]);};
  }else{
    (((scaledTime - mag[i].phase) >= 0)&&((scaledTime - mag[i].phase) < mag[i].threshold)) ? (*output_array[i*2] |= (1<<(int)output_array[i*2+1])) : (*output_array[i*2] &= ~(1<<(int)output_array[i*2+1]));
  }
}

//
void checkPWM(){
  for (unsigned char i=0; i<NR_OF_MAGS; i++){
    int scaledTime;
    scaledTime = mag[i].currentTime / (float) 3906.25 * mag[i].frequency; // scale currentTime from Timer frequency to desired PWM  frequency
    if(scaledTime > mag[i].frequency) {mag[i].currentTime = 0;};
    setOutput(scaledTime, i);
  }
}

// for each Magnetron increase PWM-Time
void stepPWM(){
  for (unsigned char i=0; i<NR_OF_MAGS; i++){
    mag[i].currentTime += 1;
  }
}

//Timer Interrupt
ISR(TIMER0_OVF_vect){
  stepPWM();
  checkPWM();
}

// Initialize Data Direction Registers
void initDDR(){
  DDRB = 0b11000001;
  DDRC = 0b00000000;
  DDRD = 0b11100011;
}

int main(void){

  // Initialize Analog Digital Converter
  PRR &= 0xfe; // Power reduction off -> ADC enable
  ADMUX &= ~(1<<ADLAR); // result is right adjusted
  
  // Initialize Pin directions
  initDDR();
  
  // initialize Magnetron
  for (unsigned char i=0; i<NR_OF_MAGS; i++){
    initMag(&mag[i]);
  }
  
  // Initialize Timer Interrupt
  // System Clock runs in default with 1MHz --> 0.000001 seconds per Tick
  // Interrupt at Overflow --> 0.000001 * 256 = 0.000256 seconds per Interrupt  =>  3.90625 kHz
  sei(); // enable global interrupts
  TCCR0A = 0x00; // disable Compare Output Mode,  set Timer Mode of Operation to Normal
  TCCR0B = 0x01; // disable Force Output Mode, set Timer Mode of Operation to Normal, set Clock Source to I/O Clock without prescaler
  TIMSK0 |= 1 << TOIE0; // enable Timer Interrupt at overflow
  
    while(1){}; // es wird im Moment nur die Timerinterrupt Routine getestet
    
  return 0;
}

Wenn das ganze funktioniert, werde ich die Funktionen noch etwas 
verschönern und aufteilen, aber erstmal ist mir natürlich die 
Funktionalität wichtig.

Ich hoffe ihr könnt mir helfen, den Code zu optimieren.

MFG der Anfänger

[Peter D. (peda)]

Kaan Ayhan schrieb:
> Wie meinst du das mit der switch-Anweisung?

#include "sbit.h"

void pin_out( uint8_t pinno, uint8_t level )
{
  if( level )
    switch( pinno ){
      case 0: PORT_B6 = 1; break;
      case 1: PORT_B7 = 1; break;
      case 2: PORT_D5 = 1; break;
      case 3: PORT_D6 = 1; break;
      case 4: PORT_D7 = 1; break;
      case 5: PORT_B0 = 1; break;
    }
  else
    switch( pinno ){
      case 0: PORT_B6 = 0; break;
      case 1: PORT_B7 = 0; break;
      case 2: PORT_D5 = 0; break;
      case 3: PORT_D6 = 0; break;
      case 4: PORT_D7 = 0; break;
      case 5: PORT_B0 = 0; break;
    }
}

Sollte schneller gehen, als mit Pointer.

Einfacher wärs natürlich, wenn alle Outputpins auf dem selben Port 
wären.

[Kaan A. (deranfaenger)]

Danke jetzt scheint es mir logisch..

Aber ich kann die bib nicht einbinden. Wenn ich #include <sbit.h> mache, 
kommt die Fehlermeldung: No such file or directory
Ich hab die Datei im gleichen Ordner, wie das Projekt.

Achja und ich benutze AtmelStudio 6.2.

[Eric B. (beric)]

Kaan Ayhan schrieb:
> Eric, wäre deine schreibweise performanter, oder ist das einfach nur die
> schönere/professionellere Variante?

* Es spart ein paar bytes,
* Es ist besser lesbar,
* Es braucht keine unnötige casts von Pointer auf normale ints,
* Der von dir beschriebene Fehler tritt nicht auf.

[Markus F. (mfro)]

Kaan Ayhan schrieb:
> Wenn ich #include <sbit.h> mache,
...
> Ich hab die Datei im gleichen Ordner, wie das Projekt.

Dann brauchst Du

#include "sbit.h"

Sonst wird im System-Include-Pfad gesucht.

[Peter D. (peda)]

Kaan Ayhan schrieb:

scaledTime = mag[i].currentTime / (float) 3906.25 * mag[i].frequency;

Der Pinzugriff ist jetzt nicht mehr das Problem.
Du hast float im Interrupt und dazu noch float Division.
Wenn das ne PWM werden soll, dann aber nur gaanz laangsaam.

[Kaan A. (deranfaenger)]

Danke für den Tipp, Markus.

Und Peter, ja ich habe auch gemerkt dass das Problem in dieser Zeile 
liegt, doch wie kann ich das Problem lösen? Einfach einen Integer daraus 
zu machen wird das Problem ja nicht lösen denke ich... das Problem ist, 
dass es ganz ohne Division auch nicht geht. Denn es werden 6 PWM-Signale 
erzeugt. Alle werden in der Timer-Frequenz(Timer Interrupt bei Overflow 
mit 8bit Timer und 1MHz für eine höhere Frequenz könnte ich natürlich 
auch den Interrupt früher setzen) hochgezählt. Nun muss ich ja um die 
PWM zu berechnen das Ganze umskalieren. Sprich geteilt durch aktuelle 
Frequenz mal erwünschte Frequenz.

Um mit dem Interrupt kein Problem mehr zu haben, hab ich die Funktion 
checkPWM() in die main gezogen. Im Interrupt wird nur noch die Variable 
currentTime pro Ausgang hochgezählt ( stepPWM() ).

Weiß jemand wie ich das besser machen kann?

Wichtig ist, dass Frequenz und Tastverhältnis für alle Ausgänge 
unabhängig von einander variabel sein sollten.
Wenn ich die Frequenz für alle Ausgänge gleich hätte, wäre das ja kein 
Problem.

[Peter D. (peda)]

Schau Dir mal andere SW-PWM Varianten an, die machen überhaupt keine 
Berechnungen im Interrupt.
Nur Arrayzugriffe, um den nächsten Zählwert laden.
Die Berechnung eines neuen PWM-Wertes erfolgt einmalig in der Mainloop.

Kaan Ayhan schrieb:
> Wichtig ist, dass Frequenz und Tastverhältnis für alle Ausgänge
> unabhängig von einander variabel sein sollten.

Das kling recht crazy.
Mags Du mal die Anwendung schildern, warum die Frequenz sich ändern muß.
Allgemein ist PWM ja nur Änderung des Mittelwertes.

[Kaan A. (deranfaenger)]

Danke für den Tipp ich werde es mir morgen anschauen.

Ja der Mikrocontroller soll die Ansteuerung der Netzteile von Magnetrons 
übernehmen um den Erhitzungsprozess in einem Mikrowellenofen zu 
optimieren. So grob erklärt.

Meint ihr, wenn ich die Einstellung der Frequenz auf "vor den Start" 
begrenze, wäre es besser? Wenn man denn keinen anderen Weg findet wäre 
das meine Lösung :/