ATmega 328 - fehlerhaftes Rechenergebnis

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.

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.

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...

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.

>> 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!

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:

1

char mjd_txt[11]; // uint32_t hat max. 10 Dezimalstellen  + term. 0

2

uint32_t mjd;

3

ultoa(mjd , mjd_txt,  10); // man beachte das extra l im Funktionsnamen

draus. Ist dann mjd immer noch gleich?

>> 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. 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.

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 :)

1

#include <stdio.h>

2

#include <stdlib.h>

3

#include <stdint.h>

4

5

int main (int argc, char **argv)

6

{

7

  uint32_t mjd = 80614;

8

  uint8_t year_x;

9

  uint8_t month_x;

10

11

  uint16_t year;

12

  uint8_t month, day, k;

13

14

  /*

15

   * 15079 is lowest value for 1.1.1900

16

   * 88127 is highes value for 28.02.2100 

17

   *

18

   * As a result year_x is between 0 and 200

19

   * year_x * 365.25 is 73050 --> use (uint32_t) casts

20

   */

21

22

  year_x   = (uint8_t) ((mjd - 15078.2) / 365.25);

23

  month_x  = (uint8_t) ((mjd - 14956.1 - (uint32_t)(year_x * 365.25)) / 30.6001);

24

  day      = (uint8_t) (mjd - 14956 - (uint32_t)(year_x * 365.25) - (uint32_t)(month_x * 30.6001));

25

26

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

27

  else k = 0;

28

29

  year  = (uint16_t)(year_x + k + 1900);

30

  month = (uint8_t)(month_x - 1 - (k * 12));

31

32

  printf( "mjd = %u\r\n", mjd);

33

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

34

35

  return 0;

36

}

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.