ATmega 328 - fehlerhaftes Rechenergebnis

[Daniel H. (danyag)]

Hey,

ich nutze einen ATmega 328, ein Display und ein RDA5807 Radiomodul, das 
über I²C angesprochen wird. Soweit klappt alles ganz gut mit den 
Einstellmöglichkeiten und dem Auslesen von RDS Informationen

Ich habe allerdings ein Problem bei dem ich leider nicht weiter weiß.
Um das Datum aus den RDS Informationen auszuwerten, wird eine 17bit Zahl 
erzeugt. Aus dieser werden dann nach gegebenen "Rechenvorschriften" 3 
Integerwerte für Tag, Monat und Jahr berechnet.
Für die Erzeugung der 17bit Zahl habe ich 4 Methoden ausprobiert, die 
alle das richtige Ergebnis (mjd = 57390 für den 03.01.2016) erzeugen.
Die folgende Berechnung von Tag, Monat und Jahr funktioniert allerdings 
nur bei einer der 4 Methoden korrekt.

-Weiß vielleicht jemand weiter, wo das Problem liegen könnte oder wo ich 
einen Denkfehler habe? Stimmt was nicht mit dem Datentypen? Fehlt ein 
"cast"? Ist der µC "überfordert?

Den entsprechenden Abschnitt des Codes habe ich angehängt. Dort sind 
alle 4 Varianten und die Umrechnung aufgeführt.


Mit freundlichen Grüßen
Daniel H.

[Tassilo H. (tassilo_h)]

Irgendwie glaube ich nicht, daß mjd korrekt berechnet wird bei 2-4. Da 
sind soviele (1<<(16-i)) drin, mit i das 0 sein kann.
Das ist doch nicht definiert auf einer 8/16bit-Maschine. Mach da mal 
(1ul << (16-i)) draus.

[Daniel H. (danyag)]

Tassilo H. schrieb:
> Irgendwie glaube ich nicht, daß mjd korrekt berechnet wird bei 2-4.

Ich beurteile es nur anhand des Wertes, den ich auf dem Display ablesen 
kann. Diser wird durch utoa() erzeugt.

Tassilo H. schrieb:
> Mach da mal
> (1ul << (16-i)) draus.
Im angehängten Beispiel habe ich Variante 3 mit deiner Ergänzung 
versucht.

for (i=0;i<17;i++)
{
if(i>=0 && i<= 1 && byte4[i+14] == '1') mjd += (int)(1ul << (16-i));
if(i>=2 && i<=16 && byte5[i- 2] == '1')  mjd += (int)(1ul << (16-i));
}

Leider stimmt wieder lediglich der Wert von mjd.

[Tassilo H. (tassilo_h)]

Wenn mjd wirklich immer gleich ist, kann das doch eingentlich nicht 
sein. Wie gibst du mjd aus, d.h. wie wird mjd in einen string gewandelt? 
Der Ausgabe fuer year zufolge muss mjd ja sehr gross sein. Wenn Du da 
eine 16-bit Funktion verwendest fuer die Anzeige, wuerde man die oberen 
2 byte von mjt nicht sehen... Dem geposteten Code nach kann das 
eigentlich auch nicht sein, also liegt der Fehler wohl in den nicht 
geposteten Teilen...

[Otto Normalverteilung (Gast)]

RAM voll? Dann gibt es komische Effekte...

[Lothar (Gast)]

Da musst du nur systematisch vorgehen.

Du hast eine Konstruktion die ein korrektes Ergebnis liefert. Bei der 
würde ich erst einmal den if-else Moloch auflösen. Danach Schritt für 
Schritt die anderen Teile.

[Ulrich P. (uprinz)]

Also ein paar Anmerkungen, die vielleicht helfen:

die Deklaration von i ist nicht zu sehen. Da Du aber Bits zählst wäre 
nur eine uint8_t sinnvoll.

Dann ist
if ( i >= 0 ...
immer wahr und damit überflüssig. Damit würde
... i <= 1)
der Entscheidungsteil sein und der nachfolgende
if(i>=2 && i<=16) { ... }
wäre ein einfaches
else { ... }

Jegliche Verwendung von
(int)
in einem unsigned int bekannter Breite ist eigentlich ein no-go, denn 
int ist ein signed und nur relativ bekannter Breite... int ist definiert 
als "so breit, wie die Architektur, aber mindestens 16 Bit". Auf einem 
Cortex hat man hier also eine Vorzeichen behaftete 32 Bit Variable, auf 
einem AVR eine Vorzeichen behaftete 16 Bit Variable.
Also würde ich
1. alle Variablen und Type-Castings mit präzisen Angaben machen
2. printf( " ... %u", ...) verwenden, wenn ich uintXX_t deklarierte 
Variablen ausgeben will.

Ich bin mir zudem sicher, dass man auf float Umwege komplett verzichten 
kann. Aber ich habe meine RDA Module noch irgendwo eingepackt herum 
liegen. Vielleicht schaue ich mir das die Tage mal an.

[Daniel H. (danyag)]

>> Tassilo H. schrieb:
> Wenn mjd wirklich immer gleich ist, kann das doch eingentlich nicht
> sein. Wie gibst du mjd aus, d.h. wie wird mjd in einen string gewandelt?

char mjd_txt[10];
uint32_t mjd;
utoa(mjd , mjd_txt,  10);

Der string mjd_txt wird dann auf dem Display ausgegeben.

>> Otto Normalverteilung schrieb:
> RAM voll? Dann gibt es komische Effekte...

..an sowas hatte ich auch gedacht, aber da kenne ich mich nicht aus. 
Nach dem Kompilieren bekomme ich diese Info:
Device: atmega328p

Program:   19056 bytes (58.2% Full)
(.text + .data + .bootloader)

Data:       1044 bytes (51.0% Full)
(.data + .bss + .noinit)

EEPROM:        2 bytes (0.2% Full)
(.eeprom)

Zu fehlerhaften Berechnungen kam es auch als month_x direkt in einer 
Zeile berechnet werden sollte.

>> Ulrich P. schrieb:
> die Deklaration von i ist nicht zu sehen. Da Du aber Bits zählst wäre
> nur eine uint8_t sinnvoll.

i ist tatsächlich als uint8_t deklariert, auch wenn ich nicht daran 
gedacht habe, dass eine andere Deklaration problematisch sein könnte.

>> Ulrich P. schrieb:
> Dann ist
> if ( i >= 0 ...
> immer wahr und damit überflüssig. Damit würde
> ... i <= 1)
> der Entscheidungsteil sein und der nachfolgende
> if(i>=2 && i<=16) { ... }
> wäre ein einfaches
> else { ... }

Das stimmt natürlich! Käme sowas denn als Fehlerquelle in Betracht? Ich 
hätte gedacht dass der Compiler das eh optimiert und so war es für mich 
logisch. Ich werde das aber gleich mal wie von dir vorgeschlagen kürzen.

>> Ulrich P. schrieb:
> Jegliche Verwendung von
> (int)
> in einem unsigned int bekannter Breite ist eigentlich ein no-go..

Gut zu wissen. Ich hatte die Casts zu Beginn präzise angegeben und diese 
geändert, um zu schauen ob es einen Unterschied machen würde.

>> Ulrich P. schrieb:
> 2. printf( " ... %u", ...) verwenden, wenn ich uintXX_t deklarierte
> Variablen ausgeben will.

Ich dachte dass printf zu speicherintensiv sein könnte. Werde ich aber 
auch mal versuchen.
Danke erst mal für die vielen guten Tipps und Ratschläge!

[Tassilo H. (tassilo_h)]

Daniel H. schrieb:
> char mjd_txt[10];
> uint32_t mjd;
> utoa(mjd , mjd_txt,  10);
>
> Der string mjd_txt wird dann auf dem Display ausgegeben.

Mach da mal:

char mjd_txt[11]; // uint32_t hat max. 10 Dezimalstellen  + term. 0
uint32_t mjd;
ultoa(mjd , mjd_txt,  10); // man beachte das extra l im Funktionsnamen

draus. Ist dann mjd immer noch gleich?

[formel (Gast)]

Woher ist die Formel für die Berechnung des mjd (die letzten paar 
zeilen), bitte mal eine Quelle angeben. Die Formel ergibt bei mir für 
53005 , 1.1.2004 den 32.12.2003.

[formel (Gast)]

Und bitte mal angeben wie byte4 und byte5 definiert sind.

[Daniel H. (danyag)]

>> Tassilo H. schrieb:
> Mach da mal:char mjd_txt[11]; // uint32_t hat max. 10 Dezimalstellen  +
> term. 0
> uint32_t mjd;
> ultoa(mjd , mjd_txt,  10); // man beachte das extra l im Funktionsnamen
> draus. Ist dann mjd immer noch gleich?

das hat tatsächlich das angezeigte Ergebnis verändert!
Nachdem ich dann noch die casts spezifiziert habe, wie von
Ulrich P. im Beitrag #4412255 angemerkt, kommt nun bei der Berechnung 
des Datums der korrekte Wert raus.

Welche der 4 Varianten ergibt denn unter verschiedenen Gesichtspunkten 
am meisten Sinn? - Geschwindigkeit, Speicher, "Eleganz"
- Ich sehe natürlich die Größe des Codes nach dem Kompilieren, aber wenn 
mir jemand erklären könnte wie aufwendig die Operationen für den µC sind 
bzw. wie man zur effizientesten Lösung kommt, würde ich mich freuen.

>> formel schrieb:
> Die Formel ergibt bei mir für
> 53005 , 1.1.2004 den 32.12.2003.

Ja, du hast recht.. für das aktuelle Datum stimmt alles, aber bei mjd = 
53005 passt es nicht. Für 53006 stimmt das Datum wieder.

Quelle: Seite 82/132
http://www.interactive-radio-system.com/docs/EN50067_RDS_Standard.pdf
Ich hoffe ich darf den Link hier so einfach angeben?!


Vielen Dank nochmal für die bisherige Hilfe ;)

[Daniel H. (danyag)]

Daniel H. schrieb:
>>> formel schrieb:
>> Die Formel ergibt bei mir für
>> 53005 , 1.1.2004 den 32.12.2003.
>
> Ja, du hast recht.. für das aktuelle Datum stimmt alles, aber bei mjd =
> 53005 passt es nicht. Für 53006 stimmt das Datum wieder.

Bei der Fehlersuche und dem Hin- und Herbauen des Codes hatten sich 
offenbar neue Fehler eingeschlichen.

mjd = 53005;
year_x   = (uint16_t) ((mjd - 15078.2) / 365.25);
month_x  = (uint16_t) ((mjd - 14956.1 - (uint16_t) (year_x * 365.25)) /
           30.6001);
day      = (mjd - 14956 - (uint16_t)(year_x * 365.25) - (uint16_t)
           (month_x * 30.6001));
if (month_x == 14 || month_x == 15) k = 1;
else k = 0;
year  = year_x + k + 1900;
month = month_x - 1 - (k * 12);

ergibt mit 1.1.2004 das gewünschte Datum.

[Ulrich P. (uprinz)]

Das rechnet zwar jetzt richtig, aber nur durch Zufall...
Genauer: Nur im Bereich ab dem 1.3.1900 bis zum 3.8.2079

Es darf mit unsigned gerechnet werden, weil die Spec sagt, dass die 
Formel erst ab dem 1.3.1900 (mjd = 15079) gilt. Vorher würde year_x = 
mjd - 15078.2 einen underflow erzeugen, wenn man wegen unsigned den 
negativen Zahlenraum ausschlägt.

year_x, month_x und day dürfen uint8_t sein, denn sie liegen alle unter 
256, wenn man 88127 als höchstes erlaubtes mjd einsetzt, denn year_x 
wird dann max. 200.

Zwischendrin wird (uint16_t)(year_x * 365.25) verwendet und das ist ein 
Problem, denn am 04.08.2079 ist dann year_x * 365.25 > 65535 und erzeugt 
einen overflow.

Hier muss man also nach (uint32_t) casten, damit es noch stimmt. Ich bin 
mir aber nicht sicher, ob man 2079 überhaupt noch analoges UKW Radio 
hören kann, ob man überhaupt noch Funk-Radio hört oder ob überhaupt noch 
jemand da ist, der Radio hören kann.

Kleine Optimierung:

Geht man davon aus, dass wir das Jahr 2000 hinter uns haben, dann kann 
man die globale Variable year als uint8_t nutzen und rechnet
year  = (uint8_t)(year_x + k - 100);
Dann nutzt man printf( "20%02u", year); wenn es 4-Stellig angezeigt 
werden soll.

Wäre noch mal eine nette Theoriestunde, ob man die ganze Berechnung 
nicht in den glatten Integer Raum verlegen könnte :)

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>

int main (int argc, char **argv)
{
  uint32_t mjd = 80614;
  uint8_t year_x;
  uint8_t month_x;

  uint16_t year;
  uint8_t month, day, k;

  /*
   * 15079 is lowest value for 1.1.1900
   * 88127 is highes value for 28.02.2100 
   *
   * As a result year_x is between 0 and 200
   * year_x * 365.25 is 73050 --> use (uint32_t) casts
   */
  
  year_x   = (uint8_t) ((mjd - 15078.2) / 365.25);
  month_x  = (uint8_t) ((mjd - 14956.1 - (uint32_t)(year_x * 365.25)) / 30.6001);
  day      = (uint8_t) (mjd - 14956 - (uint32_t)(year_x * 365.25) - (uint32_t)(month_x * 30.6001));

  if (month_x == 14 || month_x == 15) k = 1;
  else k = 0;

  year  = (uint16_t)(year_x + k + 1900);
  month = (uint8_t)(month_x - 1 - (k * 12));

  printf( "mjd = %u\r\n", mjd);
  printf( "d.m.y %u.%u.%u\r\n", day, month,year);

  return 0;
}

[Ulrich P. (uprinz)]

Kleiner Nachtrag für die, die Lust dazu haben. Im Anhang ein kleines C 
Programm, dass das Datum mal mit float, mal komplett als integer 
berechnet. Ist ein wenig ausführlicher, damit man besser vergleichen 
kann.

Wahrscheinlich wird der Weg über Integer ohne Float den kleineren und 
schnelleren Code erzeugen... habe es ab lediglich mal flott auf der 
Kommandozeile meines Linux ausprobiert und nicht auf einem AVR.

> gcc rds.c -o rds
> ./rds

Viel Spaß damit.