Falsche Ergebnisse mit Modulo

[tg (Gast)]

Hallo,

ich habe an einem ATMEGA1284 einen Drehgeber per HCTL2016 angeschlossen, 
der die Position mit 16 Bit zählt, den Istwert lese ich in den ersten 
Zeilen in ra_val ein. Da kommt an, was erwartet wird.

Eine volle Umdrehung der Achse liefert 12000 Inkremente, die sollen auf 
0 … 24h umgerechnet werden (Stundenachse am Teleskop). In positiver 
Zählrichtung kein Problem, in negativer muss ich einen Offset verrechen, 
weil 2 hoch 16  / 12000 nicht ohne Rest aufgeht. Das funktioniert auch. 
Dass die Berechnung nur innerhalb von reichlich ±2 Umdrehungen richtig 
funktioniert, ist mir klar, da ich soweit gar nicht komme, stört das 
nicht.

Ich habe die Zeile unten gekennzeichnet, welche falsch rechnet … 
übersehe ich da irgendwas? Ich hab aus der Vermutung heraus, dass eine 
interne Umwandlung stattfindet, zur Kontrolle mit / ohne Vorzeichen 
ausgeben lassen, da kommen die gleichen Werte raus.

Die Ausgaben auf UART0 sind nur zur Fehlersuche drin. Ich habe dort 
jeweils die erwarteten bzw. falschen Werte im Kommentar vermerkt.

encoder_resolution wird mit 12000 aufgerufen, kommt auch richtig an. Ich 
habe die deklaration der Encoderauflösung versuchsweise auf uint32_t 
geändert - ist wirkungslos. cli() am Anfang und sei() am Ende der 
Funktion bringt auch nichts. Hat jemand eine Idee?

Grüße
tg

//globale Deklarationen:

uint16_t resolution_r = 12000;              // Startwert Auflösung Stundenwinkel


//------------------------------------------[]-------------------------------------------
//Quadraturencoder RA auslesen und aufbereiten, Übergabe in Sekunden
uint32_t get_ra_seconds (uint16_t encoder_resolution)
{
    uint32_t ra_val = 0;

    char Buffer[10];                                                // Test

    HCTL_CONTROL_PORT &= ~((1 << HCTL_SEL) | (1 << HCTL_OE_RA));    //SEL=L; OE=L; set inhibit
    asm volatile ("nop");                                           //Reaktionszeit für HCTL erforderlich
    ra_val = (HCTL_DATA_PORT << 8);                                 //high-Byte lesen + schieben
    HCTL_CONTROL_PORT |= (1 << HCTL_SEL);                           //SEL=H; OE=L; start inhibit reset
    asm volatile ("nop");                                           //Reaktionszeit für HCTL erforderlich
    ra_val |= HCTL_DATA_PORT;                                       //low-Byte lesen + addieren
    HCTL_CONTROL_PORT |= (1 << HCTL_OE_RA);                         //SEL=H; OE=H; complete inhibit reset

    uart0_puts ("RA: ");
    utoa( ra_val, Buffer, 10 );
    uart0_puts (Buffer);           //Beispiel: 65291
    uart0_puts (" | ");

    if (ra_val >= 65536/2)         //negative Zählrichtung: Werte umrechnen
    {
        ra_val = ra_val - (65535 % encoder_resolution);             //Offset bei negativer Zählrichtung
    }

    utoa( ra_val, Buffer, 10 );
    uart0_puts (Buffer);           //Beispiel: 59756 … ok
    uart0_puts (" | ");

    ra_val = ra_val % encoder_resolution;        // RECHNET FALSCH auf eine Umdrehung reduzieren

    utoa( ra_val, Buffer, 10 );
    uart0_puts (Buffer);           //Beispiel: 5516 kommt, 11756 erwartet
    uart0_puts (" u|i ");
    itoa( ra_val, Buffer, 10 );
    uart0_puts (Buffer);           //Beispiel: 5516 kommt, 11756 erwartet

    ra_val = (ra_val * (uint32_t) 86400) / encoder_resolution;  //in Sekunden, umskalieren auf 24h

//...

    return ra_val;
}

Der Vollständigkeit halber noch der Aufruf dieser Funktion:

   ha_seconds = get_ra_seconds(resolution_r);

[Stefan F. (Gast)]

Gebe mal ein Beispiel an, mit welchen konkreten zahlen man das Problem 
(auf einem PC) reproduzieren kann:

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

int main()
{
    uint32_t ra_val = 20000;
    uint16_t encoder_resolution = 180;
    printf("ra_val=%d, encoder_resolution=%d, modulo=%d\n",
        ra_val, encoder_resolution, ra_val%encoder_resolution);
    return 0;
}

Hier bitte die beiden Zahlen ersetzen.

[c-hater (Gast)]

tg schrieb:

> Eine volle Umdrehung der Achse liefert 12000 Inkremente, die sollen auf
> 0 … 24h umgerechnet werden (Stundenachse am Teleskop). In positiver
> Zählrichtung kein Problem, in negativer muss ich einen Offset verrechen,
> weil 2 hoch 16  / 12000 nicht ohne Rest aufgeht.

Häh? Wenn das in positiver Richtung geht, muss es auch in negativer 
Richtung gehen. Wenn irgendwelche Teilbarkeiten tatsächlich eine Rolle 
spielen würden, täten sie das in beiden Richtungen gleichermaßen. Das 
kann man sogar wissen, ohne Programmieren zu können, dazu braucht man 
einfach nur bis zur vierten Klasse in Mathe mitgehalten zu haben...

Kurzfassung: du hast keine Ahnung von dem, was du da tust. Lerne.

[Stefan F. (Gast)]

c-hater schrieb:
> du hast keine Ahnung von dem, was du da tust. Lerne.

Bitte ...

[Oliver S. (oliverso)]

Auch wenn es wie üblich grenzwertig deutlich formuliert ist, hat c-hater 
im Prinzip wohl recht.

Der Wurm steckt nicht im Modulo-Operator.

Oliver

[tg (Gast)]

Stefanus F. schrieb:

Hallo Stefanus

mit den eingetragenen Werten

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

int main()
{
    uint32_t ra_val = 59753;
    uint16_t encoder_resolution = 12000;
    printf("ra_val=%d, encoder_resolution=%d, modulo=%d\n",
        ra_val, encoder_resolution, ra_val%encoder_resolution);
    return 0;
}

sollte 11753 raus kommen, tatsächlich erscheinen bei mir 5513.

Grüße
tg

[Egon D. (Gast)]

tg schrieb:

> In positiver Zählrichtung kein Problem, [...]

Tatsächlich?

Das Teleskop läuft genau 5 Tage lang vorwärts; dann
steht der Zähler auf 5*12000 = 60'000. Was passiert
im Verlaufe des sechsten Tages?

[Thomas Z. (usbman)]

Ich werfe mal %d in den Raum...
Du verwendest eine 8 Bit CPU ein int ist nur 16 Bit groß. Das gibt 
natürlich Probleme wenn du printf auf einmal eine 32bit Zahl vorwirfst.

Thomas

[c-hater (Gast)]

Oliver S. schrieb:

> Auch wenn es wie üblich grenzwertig deutlich formuliert ist, hat c-hater
> im Prinzip wohl recht.
>
> Der Wurm steckt nicht im Modulo-Operator.

So ist es. Aber wollen wir ihn mal nicht völlig dumm sterben lassen...

@tg (Gast):

Der Trick ist einfach: zyklische Sachverhalte (nämlich z.B. die 
Erdumdrehung bzw. unsere 24h-Abbildung selbiger) auch eben als Zyklus zu 
sehen und so im Code umzusetzen.

Du kannst das an zwei Stellen tun. Sinnvoller wird sein, es zu tun, 
bevor du die Schritte in deine Funktion einfüllst. Allerdings musst du 
dann ggf. einen zusätzlichen Parameter mitschleppen, nämlich einen 
Tages-Offset. Nötig wird das, wenn du im Endergebnis nicht nur die 
Uhrzeit sehen willst (die es ja jeden Tag gleichermassen gibt), sondern 
tatsächlich eine konkrete, eindeutige Zeit.

[Stefan F. (Gast)]

tg schrieb:
> sollte 11753 raus kommen

Tut es bei mir auch.

[tg (Gast)]

Egon D. schrieb:
> tg schrieb:
>
>> In positiver Zählrichtung kein Problem, [...]
>
> Tatsächlich?
>
> Das Teleskop läuft genau 5 Tage lang vorwärts; dann
> steht der Zähler auf 5*12000 = 60'000. Was passiert
> im Verlaufe des sechsten Tages?

Das Teleskop soll sich ja nicht tagelang kontinuierlich drehen. Schon um 
die Kabel nicht aufzuwickeln, wird das immer wieder zurückgedreht, so 
dass ich sicher in dem Limit von ±2 richtig verarbeiteten Umdrehungen 
bleibe.

[Stefan F. (Gast)]

Thomas Z. schrieb:
> Ich werfe mal %d in den Raum...
> Du verwendest eine 8 Bit CPU ein int ist nur 16 Bit groß. Das gibt
> natürlich Probleme wenn du printf auf einmal eine 32bit Zahl vorwirfst.

Vielleicht musst du beim AVR "%ld" schreiben. Wenn du alle Warnungen vom 
Compiler einschaltest (-Wall) sollte er sich ggf. dazu melden.

Allerdings: 11753 würde in einen 16bit Integer noch rein passen.

[Egon D. (Gast)]

tg schrieb:

> Egon D. schrieb:
>> tg schrieb:
>>
>>> In positiver Zählrichtung kein Problem, [...]
>>
>> Tatsächlich?
>>
>> Das Teleskop läuft genau 5 Tage lang vorwärts; dann
>> steht der Zähler auf 5*12000 = 60'000. Was passiert
>> im Verlaufe des sechsten Tages?
>
> Das Teleskop soll sich ja nicht tagelang kontinuierlich
> drehen.

Herrgott. Beantworte doch einfach meine Frage.

[tg (Gast)]

Stefanus F. schrieb:
> tg schrieb:
>> sollte 11753 raus kommen
>
> Tut es bei mir auch.

Naja, hatte ich auch erwartet ;-)
Ich hab ja die kompletten Berechnungen, die da mal laufen sollen, in 
einer Tabellenkalkulation parallel als "normale" Berechnung und in 
Ganzzahlarithmetik vorbereitet, da funktioniert auch alles.

Ich hatte wie gesagt die Hoffnung, das jemand was auffällt, was ich 
konstant übersehe, z.B. hinsichtlich Deklarationen, implizite Casts 
usw., was den Effekt verursacht. Am Modulo-Operator hab ich eigentlich 
keine Zweifel.

Danke dir!
Grüße
tg

[Egon D. (Gast)]

tg schrieb:

> Ich hatte wie gesagt die Hoffnung, das jemand was
> auffällt, was ich konstant übersehe, [...]

Ist ja auch geschehen -- aber die Leute ignorierst
Du ja.

Nun gut.

[Klaus (Gast)]

tg schrieb:
> übersehe ich da irgendwas?

> Die Modulo-Berechnung ergibt in C grundsätzlich nur mit positiven
> Werten sinnvolle Ergebnisse. Wenn der zweite Operand negativ ist,
> so wird dieses Vorzeichen von C schlichtweg ignoriert. Wenn der erste
> Operand negativ ist, so wird die Berechnung ausgeführt, als ob der
> Operand positiv wäre und schlussendlich dem Resultat das Vorzeichen
> wieder angehängt.

http://manderc.com/operators/modoperator/index.php

Man muß also für Fälle, in denen ein negatives Verzeichen vorkommt, eine 
Sonderbehandlung einbauen.

MfG Klaus

[Oliver S. (oliverso)]

Müsste man, spielt aber beim TO keine Rolle.

Ansonsten scheint die Aufgabe zu sein, den Wertebereich 0-12000 auf den 
Bereich 0-24 abzubilden. Sollte machbar sein ;)

Oliver

[tg (Gast)]

Klaus schrieb:
> Man muß also für Fälle, in denen ein negatives Verzeichen vorkommt, eine
> Sonderbehandlung einbauen.

Die tatsächlich auftretenden Werte schließen aus, dass die nicht 
funktionierende Modulo-Operation eine negative Zahl zu sehen bekommt. 
Ich habe trotzdem mal

    if (ra_val < 0)
        uart0_puts (" <0 ");

davor gesetzt - es wird keine Meldung ausgegeben, wenn danach falsche 
Werte berechnet werden. Ein impliziter Cast findet also offensichtlich 
nicht statt.

Ich habe inzwischen auch unterschiedliche Optimierungseinstellungen 
getestet .. macht keinen Unterschied beim Ergebnis. Es ist nach wie vor 
diese Zeile

   ra_val = ra_val % encoder_resolution;

die nicht mitspielt. Wenn ich die Auflösung "hart" eintrage, stimmt das 
Ergebnis ebenfalls nicht:

   ra_val = ra_val % 12000;

Beide Fällen führen z.B. zu 59980 % 12000 = 5740. Wenn ich aber die 
59980 fest eintrage

   ra_val = 59980 % encoder_resolution;

kommen wie zu erwarten 11980 raus. Bei vorangegangenen Tests wurden mir 
die Werte von ra_val korrekt angezeigt, irgendwas geht daneben, wenn 
diese Zeile losarbeitet ...

Grüße
tg

[tg (Gast)]

Oliver S. schrieb:
> Ansonsten scheint die Aufgabe zu sein, den Wertebereich 0-12000 auf den
> Bereich 0-24 abzubilden. Sollte machbar sein ;)

Ja, das geht gerade noch so ;-). Ich hab das oben vielleicht etwas 
unklar beschrieben.

Pro kompletter Umdrehung der Achse bekomme ich 12000 Inkremente. Die 
werden von einem 16bit-Zähler-IC erfasst. Der zählt also in die eine 
Richtung 0, 1, ... 12000 (nach einer Umdrehung), 24000 (2. Umdr.) ... 
36000 ... usw.

In die andere Richtung zählt der 0, 65535, .. 53535 (1. Umdr.) ... usw. 
Je nachdem, wie herum ich drehe, bekomme ich also für den gleichen 
Winkel unterschiedliche Zählerstände. Deshalb wird die zuletzt genannte 
drehrichtung per Offset so korrigiert, dass der Wert richtungsunabhängig 
den Winkel eindeutig abbildet (nach der letzten Modulo-12000-Operation).

Mit einem umfangsbegrenzten Ringzähler hätte ich's einfacher, aber da an 
dem Controller noch 2 Stepper, 2 Drehgeber und mehrere I²C-Teile (incl. 
Display) hängen, wollte ich mir keine Timingproblem einhandeln.

Fehler würden bei meiner Berechnung erst entstehen, wenn ich 2^16 / 2 
überschreite, d.h, mehr als ca. 2,7 Umdrehungen in eine Richtung mache. 
Das kann ich aber ausschließen.

Aber wie gesagt, das ist alles nicht das Problem .. das besteht "nur" 
darin, die Ursache für die eine falsch rechnende Zeile zu finden.

Grüße
Thilo

[Thomas Z. (usbman)]

Was passiert bei

ra_val = ra_val % 12000L;

Stimmt dann der Wert?

Thomas

[Oliver S. (oliverso)]

tg schrieb:
> In die andere Richtung zählt der 0, 65535, .. 53535 (1. Umdr.) ... usw.
> Je nachdem, wie herum ich drehe, bekomme ich also für den gleichen
> Winkel unterschiedliche Zählerstände.

Je nun, eigentlich bekommst du 0, -1, -2, usw. (bei positiver und 
negativer Drehrichtung ist ein signed integer ganz passend), was durch 
Addition von 12000  bei negativen Werten dann dazu führt, daß du doch 
die selben Werte für eine Winkelstellung erhältst, unabhängig von der 
Drehrichtung.

Oliver

[tg (Gast)]

Thomas Z. schrieb:
> Was passiert bei
>
> ra_val = ra_val % 12000L;
>
> Stimmt dann der Wert?

Nein, bleibt falsch. Ich hatte die Deklaration für encoder_resolution 
schon versuchsweise auf uint32_t geändert, ebenfalls ohne Erfolg.

Ich versuchs morgen mal mit dem Vorschlag von Oliver, scheint so, als 
könnte man damit die Modulo-Rechnerei ganz vermeiden ... ich melde mich 
wieder.

tg

[Peter D. (peda)]

Dein Fehler ist, daß 65536 (uint16_t) nicht ohne Rest durch 12000 
teilbar ist.
Bilde daher die Differenz zu einer Variable, die Du nachführst, sobald 
der Wert außerhalb 0..11999 ist.
Differenzen stimmen immer, auch über einen Überlauf hinweg.

[Peter D. (peda)]

uint16_t modulo12000(uint16_t val)
{
  static uint16_t old = 0;
  while( (val - old) >= 12000)
  {
    if ((int16_t)(val - old) < 0)
      old += 12000;
    else
      old -= 12000;
  }
  return val - old;
}

[Stefan F. (Gast)]

Peter D. schrieb:
> Dein Fehler ist, daß 65536 (uint16_t)

für 16 bit genau um 1 zu groß ist. Aber egal, wir haben trotzdem 
verstanden, was du meintest.

[Peter D. (peda)]

Da war noch ein Fehler drin.
old wird ja abgezogen, d.h. wenn Differenz negativ, muß old kleiner 
werden
So ist es richtig rum:

uint16_t modulo12000(uint16_t val)
{
  static uint16_t old = 0;
  while( (val - old) >= 12000)
  {
    if ((int16_t)(val - old) < 0)
      old -= 12000;
    else
      old += 12000;
  }
  return val - old;
}

[Andreas M. (amesser)]

Ich würde gar nicht versuchen direkt auf der 16 Bit Variable des Zählers 
zu arbeiten. Stattdessen würde ich mir in der MCU den letzten 16 Bit 
Zählerstand merken.

Mit der Differenz des aktuellen Stand lässts sich das problemlos auf 
jede beliebige Darstellung, also auch zyklisch mit 12000 Schritten 
abbilden. Und zwar ganz ohne Modulo. (Ist vermutlich sogar schneller)

uint16_t last_enc;

int16_t pos;

void update_pos()
{
  uint16t_t enc = get_enc();

  /* max 3 iterations */
  while(enc != last_enc)
  {
    int16_t delta = (int16_t)(enc - last_enc);
    int16_t tmp;

    if (delta > 11999)
      delta = 11999;
    else if (delta < -12000)
      delta = -12000;

    last_enc += (uint16_t)delta;

    tmp = delta + pos;

    if (tmp >= 12000)
      pos = tmp - 12000
    else if (tmp < 0)
      pos = tmp + 12000;
    else
      pos = tmp;
  }
}

[Samuel C. (neoexacun)]

Peter D. schrieb:
>

> uint16_t modulo12000(uint16_t val)
> {
>   static uint16_t old = 0;
>   while( (val - old) >= 12000)
>   {
>     if ((int16_t)(val - old) < 0)
>       old += 12000;
>     else
>       old -= 12000;
>   }
>   return val - old;
> }
>

Warum derart aufwendig?

uint16_t modulo12000(uint16_t val)
{
  while( val >= 12000)
  {
      val -= 12000;
  }
  return val;
}

oder ohne Schleife:

uint16_t modulo12000(uint16_t val)
{
  uint16_t mul = val / 12000;
  val -= mul*12000;
  return val;
}

oder als Einzeiler:

uint16_t modulo12000(uint16_t val)
{
  return val - ((val / 12000) * 12000);
}

[Andreas M. (amesser)]

Etwas nachdenken hilft manchmal :-).

Andreas M. schrieb:
>

> ... // Umständlicher C-Code
>

uint16_t last_enc;

int16_t pos;

/** Update encoder position
 *
 *  @note Must be called once at least within two full rotations
 */
void update_pos()
{
  uint16t_t enc = get_enc();
  int16_t delta = 0;

  /* Modulo full rotations:
   * Works for max 2 full rotations in either
   * pos or neg direction */
  do
  {
    delta = (int16_t)(enc - last_enc)

    if (delta > 12000)
      delta = 12000;
    else if (delta < -12000)
      delta = -12000;

    last_enc += (uint16_t)delta;
  } while(enc != last_enc)


  /* Update current position with remaining
   * fraction of rotation */
  tmp = delta + pos;
 
  if (tmp >= 12000)
    pos = tmp - 12000
  else if (tmp < 0)
    pos = tmp + 12000;
  else
    pos = tmp;
}

[tg (Gast)]

So, wie gestern Abend angekündigt, habe ich mir nach Olivers Anregung, 
den Encoderwert als int statt uint zu behandeln, die Funktion 
umgestrickt:

//Quadraturencoder RA auslesen und aufbereiten, Übergabe in Sekunden
uint32_t get_ra_seconds (uint16_t encoder_resolution)
{
    uint32_t ra_val;
    int16_t  ra_enc;

    HCTL_CONTROL_PORT &= ~((1 << HCTL_SEL) | (1 << HCTL_OE_RA));    //SEL=L; OE=L; set inhibit
    asm volatile ("nop");                                           //Reaktionszeit für HCTL erforderlich
    ra_enc = (HCTL_DATA_PORT << 8);                                 //high-Byte lesen + schieben
    HCTL_CONTROL_PORT |= (1 << HCTL_SEL);                           //SEL=H; OE=L; start inhibit reset
    asm volatile ("nop");                                           //Reaktionszeit für HCTL erforderlich
    ra_enc |= HCTL_DATA_PORT;                                       //low-Byte lesen + addieren
    HCTL_CONTROL_PORT |= (1 << HCTL_OE_RA);                         //SEL=H; OE=H; complete inhibit reset

    while (ra_enc < 0)
        ra_enc += encoder_resolution;

    while (ra_enc >= encoder_resolution)
        ra_enc -= encoder_resolution;

    ra_val = (ra_enc * (uint32_t) 86400) / encoder_resolution;     //auf 24h umskaliert in Sekunden

    return ra_val;
}

Funktioniert wie geplant. Die Unterscheidung zwischen positiven und 
negativen Zählerständen habe ich mir gespart, da ja die kopfgesteuerten 
Schleifen eigentlich die selbe Funktion erfüllen, d.h., satt if oder 
else wird hier nur die zutreffende while-Schleife abgearbeitet. Ich 
nehem mal an, dass in der Bearbeitungszeit kein relevanter Unterschied 
besteht.

Ich danke euch nochmal für eure Mühe und Vorschläge!

Grüße
Thilo