Bytes auf PROGMEM lesen - was geht hier schief?

[Nico B. (vegetico)]

Moin...
In Anlehnung an die Wordclock hatte ich in einer anderen Bastelei die 
PWM übernommen.
Aktuell läuft sie so - ohne Probleme:

uint8_t pwm_table[64] PROGMEM =
  {
    0,   1,   2,   3,   4,   5,   6,   7,   
    8,   9,  10,  11,  12,  13,  14,  15,  
   16,  17,  18,  19,  20,  21,  22,  23,  
   24,  25,  26,  27,  28,  29,  30,  31,  
   32,  33,  34,  35,  36,  37,  38,  39,  
   40,  41,  42,  43,  48,  52,  57,  62,  
   68,  74,  81,  89,  97, 106, 116, 126, 
  138, 150, 164, 179, 196, 214, 234, 255   };

Bzw.

extern void set_pwm_values(uint8_t red, uint8_t green, uint8_t blue)
{
  uint8_t value = 0;
  if (red>=0 && red<64)
  {
    value = pgm_read_word(pwm_table+red);
    PWM_RED = pgm_read_word(pwm_table+red);
  }  

  if (green>=0 && green<64)
  {
    value = pgm_read_word(pwm_table+green);
    PWM_GREEN = pgm_read_word(pwm_table+green);
  }

  if (blue>=0 && blue<64)
  {
    value = pgm_read_word(pwm_table+blue);
    PWM_BLUE = pgm_read_word(pwm_table+blue);
  }
}

Zwar konnte man sich 'value' im Debugger nie anschauen (das war der 
eigentliche Grund dafür) aber es hat funktioniert.
Jetzt wurde das Ganze um die Helligkeit erweitert:

extern void set_pwm_values(unsigned char red, unsigned char green, unsigned char blue, unsigned char brightness)
{
  unsigned char brightness_pwm_val = pgm_read_byte (pwm_table + brightness);
  uint16_t brightnessFactor =  ((uint16_t) brightness_pwm_val);
  
  if ((brightness>=0) && (brightness<=63))
  {
    if (red>=0 && red<255)
    {
      PWM_RED = ((brightnessFactor * red)/256);
    }  

    if (green>=0 && green<MAX_PWM_VALUE)
    {
      PWM_GREEN = ((brightnessFactor * green)/256);
    }

    if (blue>=0 && blue<MAX_PWM_VALUE)
    {
      PWM_BLUE = ((brightnessFactor * blue)/256);
    }  
  }
}

Und ich seh im Debugger, das zwar die erste Zeile (brightness_pwm_val) 
aufgerufen wird, er aber sofort weiter auf die if Bedingung springt und 
der brightnessFactor 0 ist. Heißt für mich, dass er den Wert ausm 
Speicher nicht lesen kann.
Ich versteh nicht, wieso das der Fall ist - wie gesagt ist der Code 
mittlerweile zu 99% identisch mit dem WordClock Beispiel - aber trotzdem 
geht was schief... Außerdem läuft die erste Version ja auch...
Kann mich einer mal drauf hindeuten...

Achso, testen tu ich aktuell mit red/ green/ blue = 100 und brightness = 
20, also alles im 'normalen' Bereich

Danke
Nico

[Johann L. (gjlayde)]

Warum benutzt du read_word wenn doch nur Bytes in der Tabelle stehen?

Ein uint8_t ist immer >= 0, den Test kannste also sparen → Warnungen 
aktivieren und beachten.

[Nico B. (vegetico)]

Warum ich damals read_word genommen hatte weiß ich ehrlich gesagt nicht 
mehr - da es funktioniert hatte hab ich mir nie Gedanken drüber gemacht.
Compiler Warnungen sind aber alle an (-Wall) - allerdings sind die auf 
0.

[Johann L. (gjlayde)]

-Wall gibt nicht alle Warnungen, zB nicht -Wtype-limits. Evtl. ist also 
auch -Wextra angebracht.

[Nico B. (vegetico)]

Ich bin mittlerweile auf dem AVR Studio 5 - hier kann ich leider die 
Parameter nicht händisch ändern. Und -Wextra ist bei den Warnings nicht 
dabei... Aber auch pedantisch bringt keine anderen/ neuen Warnings 
(naja, zwei Stück - die haben aber hiermit nix zu tun).
Ich hab oben nebenbei mittlerweile auch auf pgm_read_byte gewechselt - 
macht aber keinen Unterschied...

[Nico B. (vegetico)]

Ah, das pgm_read_word kaum ausm 'LED Fading' Tutorial.
http://www.mikrocontroller.net/articles/LED-Fading
Da warens auch uint16_t und keine uint8_t... Also damals falsch 
abgeschrieben :)

[Nico B. (vegetico)]

Ich seh das Problem einfach nicht... Das muß doch was triviales sein...

[Rolf Magnus (Gast)]

Nico B. schrieb:
> Jetzt wurde das Ganze um die Helligkeit erweitert:
> Und ich seh im Debugger, das zwar die erste Zeile (brightness_pwm_val)
> aufgerufen wird, er aber sofort weiter auf die if Bedingung springt und
> der brightnessFactor 0 ist. Heißt für mich, dass er den Wert ausm
> Speicher nicht lesen kann.

Es kann genauso heißen, daß er brightnessFactor wegoptimiert und 
stattdessen direkt brightness_pwm_val nimmt, weil das eh den gleichen 
Wert hat.
Wie sehen denn die Werte PWM_RED u.s.w aus? passen die zu den 
übergebenen Parameterwerten?

[Markus C. (ljmarkus)]

value = pgm_read_byte(&pwm_table[green])

[Nico B. (vegetico)]

Ja, die übergebenen Werte passen alle...
Wie gesagt, mit der alten Version (ohne Helligkeit und dem pgm_read_word 
Fehler) lief es ja, deswegen hatte ich mir nie Gedanken drum gemacht... 
Bezüglich des wegoptimieren - das hatte ich auch schon mal überlegt und 
deswegen schonmal einfach um 1 inkrementiert etc - mit dem gleichen 
Resultat.

[Rolf Magnus (Gast)]

Markus C. schrieb:
> value = pgm_read_byte(&pwm_table[green])

Das nur eine andere Schreibweise, macht aber genau das gleiche wie:

     value = pgm_read_byte(pwm_table+green);

Das folgt daraus, daß pwm_table[green] das gleiche ist wie 
*(pwm_table+green).

Nico B. schrieb:
> Ja, die übergebenen Werte passen alle...

Meine Frage war aber, ob die Ergebniswerte zu diesen passen, oder ob 
trotz sinnvoller Eingabewerte ein falsches Ergebnis rauskommt.

> Bezüglich des wegoptimieren - das hatte ich auch schon mal überlegt und
> deswegen schonmal einfach um 1 inkrementiert etc - mit dem gleichen
> Resultat.

Nun, so leicht kannst du den Optimizer nicht austricksen. Der Compiler 
hat ja keine Notwendigkeit, den alten Wert zu behalten, da er danach 
nirgends mehr benutzt wird.
Du kannst brightnessFactor zum Testen mal volatile machen. Dann muß er 
die Variable in realem Speicher anlegen.

[Johann L. (gjlayde)]

Anstatt weig mit einem Debugger rumzufutscheln wo man nur sieht, daß 
irgendwas schief läuft, tut es ofmals ein Blick in den erzeugten Code. 
Da sieht man dann, warum es nicht so läuft wie andedacht, und die 
Fehlerquelle ist rasch beseitigt.

[Nico B. (vegetico)]

Ich verstehs immer noch nicht...
Habe Rolfs Rat befolt und mal ziemlich viel als volatile deklariert - 
mit dem Ergebniss, das es läuft:

  volatile unsigned char brightnessPWMVAL = pgm_read_byte (pwm_table + brightness);
  volatile uint16_t brightnessFactor =  ((uint16_t) brightnessPWMVAL) + 1;
  volatile uint8_t value_red =  ((brightnessFactor * red)/256);
  volatile uint8_t value_green =  ((brightnessFactor * green)/256);
  uint8_t value_blue =  ((brightnessFactor * blue)/256);
  if (brightness<=63)
  {
    if (red<255)
    {
      PWM_RED = value_red;
    }  

    if (green<255)
    {
      PWM_GREEN = value_green;
    }

    if (blue<255)
    {
      PWM_BLUE = value_blue;
    }
  }

Im Beispiel verhalten sich rot und grün richtig - blau macht Murks...

Bitte nicht über das viele Gerümpel motzen - das war rein für mich zum 
testen. Trotzdem bleibt: via volatile bekomm ich endlich meinen Debugger 
Output und die Werte passen. Ohne funzt es nicht - zumindest ist das 
Verhalten an der TestLED nicht das 'erwartete' (weiß net, wie ichs 
anders beschreiben soll).

@Rolf: damit auch die Antwort auf die falsch verstandene Frage: nein, 
das setzen der PWM hatte nicht funktioniert. Die Eingabeparameter waren 
korrekt - aber das Leuchtverhalten aka Pulsbreite der PWM war falsch. 
Deswegen hatte ich mitm debuggen angefangen.

@Johann: ich hab beim AVR Studio 5 mal nachm ASM Output gesucht - bisher 
aber nicht gefunden. Weißt du ausm Stehgreif, wo ich die Settings dafür 
finde (hab noch nicht nach gegoogelt).

[Johann L. (gjlayde)]

Nico B. schrieb:
> @Johann: ich hab beim AVR Studio 5 mal nachm ASM Output gesucht - bisher
> aber nicht gefunden. Weißt du ausm Stehgreif, wo ich die Settings dafür
> finde (hab noch nicht nach gegoogelt).

Ich verwende kein AStudio. Braucht kein Mensch ;-)

Compiler-Optionen: -save-temps -fverbose-asm -g0

[Werner B. (werner-b)]

Mag ja nix damit zu tun haben, aber ändere mal

uint8_t pwm_table[64] PROGMEM = ...

in

const uint8_t pwm_table[64] PROGMEM = ...

Falls aus Versehen als C++ compiliert wird.
Zudem "könnte" sich er avr-gcc 4.5.1 aus dem Studio5 hier etwas anders 
verhalten.

[Ralf G. (ralg)]

Nico B. schrieb:
> volatile uint8_t value_green =  ((brightnessFactor * green)/256);
> uint8_t value_blue =  ((brightnessFactor * blue)/256);

Nico B. schrieb:
> Im Beispiel verhalten sich rot und grün richtig - blau macht Murks...

Ich hab' mir nicht alles durchgelesen. Aber warum muss value_blue nicht 
als volatile deklariert werden?

[Karl H. (kbuchegg)]

Ralf G. schrieb:
> Nico B. schrieb:
>> volatile uint8_t value_green =  ((brightnessFactor * green)/256);
>> uint8_t value_blue =  ((brightnessFactor * blue)/256);
>
> Nico B. schrieb:
>> Im Beispiel verhalten sich rot und grün richtig - blau macht Murks...
>
> Ich hab' mir nicht alles durchgelesen. Aber warum muss value_blue nicht
> als volatile deklariert werden?

Im Grunde muss gar nichts davon als volatile definiert werden, ess ei 
denn man möchte die Variblen unbegindt im Debugger sehen und daher den 
Compiler daran hindern sie einfach wegzuoptimieren.

Entweder das ist ein sauberer Bug im Compiler, oder aber da ist im 
Programm noch ganz was anderes faul, was man aber an den Ausschnitten 
nicht erkennen kann. Ein komplettes Programm wäre gut, damit das mal wer 
nachvollziehen kann.

[Nico B. (vegetico)]

@Ralf: Das hatte ich nur fürs testen so gemacht, ums einfacher 
unterscheiden zu können bzw. um zu gucken wie sichs im Debugger verhält.
Eigentlich wars setzen des Timers via pgm_read, via volatile 
Variablenzuweisung und einmal wie vorher (blau). So wars für mich 
einfacher, die Unterschiede zu sehen...

@Werner: Ich check mal die Compilersettings bezüglich C++ - und änder 
das array zu const. Macht ja auch Sinn...

Super, danke generell für die vielen Hinweise...