TomMajor / SmartHome

Various SmartHome projects, devices, information and examples including AskSinPP usage
86 stars 28 forks source link

Kurioser Fehler, wenn ADC6/7 als SENSPIN verwendet wird #45

Closed ChMatsch closed 3 years ago

ChMatsch commented 3 years ago

Habe heute mein blaues Wunder erlebt, denn mein AskSinPP-System, mit dem ich schon viele Tage gut arbeite, funktionierte plötzlich nicht mehr. Es basiert auf dem Projekt HB-UNI-Sensor1 und läuft also auf ATmega328P . Plötzlich Errormeldungen beim Initialisieren des CC1101 beim Setzen des Frequenzregisters. Anschließend dann auch überhaupt keine Radio-Kommunikation mehr. Als Ursache fand ich dann heraus, dass es die Initialisierung der Batteriemessung in tmBattery war. Dort wird in init() der Klasse tmBatteryResDiv der SENSPIN auf INPUT gesetzt.

    void init(uint32_t period, AlarmClock& clock)
    {
        pinMode(ACTIVATIONPIN, INPUT);    // disable resistor divider
        pinMode(SENSPIN, INPUT);          // input
        digitalWrite(SENSPIN, LOW);       // pull-up off
        tmBattery::init(period, clock);
    }

In meinem Fall hatte ich die Portbelegung geändert, seitdem liegt der SENSPIN auf Eingang ADC7 - und genau das ist das Problem. Die beiden Ports ADC6 und ADC7 sind per se reine analoge Eingänge und benötigen deshalb die beiden mittleren Konfigurationsbefehle nicht. Im Gegenteil, allein die Ausführung des Befehls

pinMode(SENSPIN,INPUT);

mit #define SENS_PIN A7 führt zum Versagen des kompletten Systems. Was genau da passiert, das weiß ich nicht. Vielleicht beschreibt die Methode pinMode ja in diesem Fall ein nicht existierendes Register). Kurios, dass dadurch die CC1101-Initialisierung schief geht! Gefixt habe ich das, indem ich die beiden Ports von der Behandlung ausgeschlossen habe:

    void init(uint32_t period, AlarmClock& clock)
    {
        pinMode(ACTIVATIONPIN, INPUT);    // disable resistor divider
        if ((SENSPIN!=A6) && (SENSPIN!=A7))
        {   // ADC6 + 7 sind per se reine ADC-Eingänge
          pinMode(SENSPIN, INPUT);     // input 
          digitalWrite(SENSPIN, LOW);       // pull-up off
        }
        tmBattery::init(period, clock);
    }

Nun funktioniert's wieder.

TomMajor commented 3 years ago

Sehr interessanter Punkt, vielen Dank. ich hatte mir nie Gedanken speziell über A6/A7 gemacht, aber wenn ich jetzt drüber nachdenke sollte eigentlich pinMode(SENSPIN,INPUT); von der Arduino Lib nur für Prozessor valide dig. IO ausgeführt werden, oder? Frage mich was da genau bei A7 passiert damit das ganze System versagt. Muss ich bei Gelegenheit mal reinschauen.

Dein Vorschlag macht auf jeden Fall Sinn. Würde nur bei den if mit #if arbeiten da es zur Compile Zeit reicht, zur Laufzeit nicht nötig. ich ändere das die nächsten Tage. Danke!

ChMatsch commented 3 years ago

Ich stimme dir zu, dass das, was wir/du jetzt änderst, nichts ist als ein workaround für einen Bug in der Methode pinMode(), denn dort müßte dafür gesorgt sein, dass ein nicht möglicher Befehl zu keinerlei schädlichen Wirkungen führt. Man kann nur vermuten, dass dort die Sonderstellung des 6. und 7. analogen Kanals nicht beachtet wurde (PC6 ist übrigens auch das Reset-Pin). (ich gebe zu, auch ich bin schon darüber gestolpert, als ich die Hardware so ausgelegt hatte, dass Port ADC7 zunächst als OUTPUT verwendet werden sollte).

Das Preprozessor-IF werde ich auch gleich einarbeiten, das spart ein paar Byte Flash, ich hab nur noch 88 davon!

TomMajor commented 3 years ago

ist gefixt: https://github.com/TomMajor/SmartHome/commit/8788f70f40d05cb61dda3cc77ebc05e0d5d39c5b Preprozessor If konnte ich doch nicht machen da SensPin template Parameter

TomMajor commented 3 years ago

Erklärung des Bugs in der Arduino Lib:

digitalPinToPort / digitalPinToBitMask holen die Werte für das Portregister und das Value dafür. Sie machen aber nur ein simples program memory read von den Tabellen. Diese gehen aber nur bis Index 19. A6 ist Index 20. D.h. es werden Tabellenwerte für A6/A7 geholt die "out of bounds" in den Tabellen sind, was auch immer da im program memory hinter der Tabelle steht. Eigentlich müssten die Arduino Makros digitalPinToPort / digitalPinToBitMask auf Index Range checken bevor sie den program memory lesen.

#define PIN_A7   (21)

#define digitalPinToPort(P) ( pgm_read_byte( digital_pin_to_port_PGM + (P) ) )
#define digitalPinToBitMask(P) ( pgm_read_byte( digital_pin_to_bit_mask_PGM + (P) ) )

const uint8_t PROGMEM digital_pin_to_port_PGM[] = {
    PD, /* 0 */
    PD,
    PD,
    PD,
    PD,
    PD,
    PD,
    PD,
    PB, /* 8 */
    PB,
    PB,
    PB,
    PB,
    PB,
    PC, /* 14 */
    PC,
    PC,
    PC,
    PC,
    PC,
};

const uint8_t PROGMEM digital_pin_to_bit_mask_PGM[] = {
    _BV(0), /* 0, port D */
    _BV(1),
    _BV(2),
    _BV(3),
    _BV(4),
    _BV(5),
    _BV(6),
    _BV(7),
    _BV(0), /* 8, port B */
    _BV(1),
    _BV(2),
    _BV(3),
    _BV(4),
    _BV(5),
    _BV(0), /* 14, port C */
    _BV(1),
    _BV(2),
    _BV(3),
    _BV(4),
    _BV(5),
};
jp112sdl commented 3 years ago

Hi, bin hier zufällig drüber gestolpert... Betrifft das A6/A7-Spezial-Handling nur digitalRead auf diese Pins?

TomMajor commented 3 years ago

In erster Linie den pinMode

TE schrieb:

Im Gegenteil, allein die Ausführung des Befehls pinMode(SENSPIN,INPUT); mit #define SENS_PIN A7 führt zum Versagen des kompletten Systems.

digitalRead ist dann aber glaub ich genau so undefiniert wenn die angesprochenen Makros verwendet werden,

jp112sdl commented 3 years ago

Hmm, ich überleg grad, warum es beim Bodenfeuchtesensor nie Probleme gab. Dort werden sie auch auf INPUT gesetzt, aber dann mit analogRead gelesen.

Oder ist das hier behandelte Problem abhängig von einer bestimmten Board-Version? Ich kann mich erinnern, dass z.B. bei der 1.8.2 irgendwas komisch war.

TomMajor commented 3 years ago

schau dir pinMode() in wiring_digital.c an. Was auch auch immer nach der jeweiligen Tabelle steht, das bestimmt der linker auf dem user PC. OS, Board-Version, Compiler Flags, alles wird Einfluss haben. Teilweise wird es ja mit if (port == NOT_A_PIN) abgefangen, aber es sind Fälle denkbar wo das nicht reicht.

ChMatsch commented 3 years ago

Hallo ihr beiden,

mir fällt jetzt nichts ein, was mit dem Board zu tun haben könnte – außer der Pinbelegung.

In meinem Fall geht es um ein eigenes Board. Es ist ein “Sonnensensor”, der sowohl die Helligkeit wie auch die Wärmeleistung der Sonne (Pyranometer) liefern soll. Derzeit bin ich am testen des Helligkeitssensors. Seine Besonderheit ist, dass ich 6 MAX44009 einsetze, die alle horizontal und vertikal um je 60° versetzt angeordnet sind. Durch Interpolation zwischen den Sensoren unter Berücksichtigung der Empfindlichkeitskurven erhalte ich so einen Helligkeitswert, der weitestgehend unabhängig ist vom Einfallswinkel der Sonne, horizontal in einem Bereich 60...300° und vertikal von 15...75°. Das Pyranometer besteht aus zwei Thermistoren und ist eher der einfache Part.

Ich hatte zunächst nur die Ports A0...A5 benutzt, mußte dann aber aus Portknappheit nochmal umdisponieren und auch A6 und A7 mit benutzen. Dazu habe ich 2 A-Ports (PC3 und PC5) auf digitale Outputs umgeschaltet und dafür A6 und A7 als analoge Eingänge. Von da ab hatte ich das Problem. Und ja, es ist allein der pinMode-Befehl, der das Problem auslöst.

TomMajor commented 3 years ago

interessanter use case :smile_cat: ich denke wir haben das Problem erst mal gelöst und ich schließe mal die Issue.