pa-pa / AskSinPP

104 stars 71 forks source link

IrqInternalBatt funktioniert nicht nach hal.sleep<>(); #198

Closed jp112sdl closed 4 years ago

jp112sdl commented 4 years ago

Hi,

vielleicht habe ich nur einen groben Denkfehler. Folgendes kurzes Beispiel:

#define EI_NOTEXTERNAL
#include <EnableInterrupt.h>
#include <AskSinPP.h>
#include <LowPower.h>

#include <MultiChannelDevice.h>

#define LED_PIN           4
#define CONFIG_BUTTON_PIN 8

#define PEERS_PER_CHANNEL 6

// all library classes are placed in the namespace 'as'
using namespace as;

// define all device properties
const struct DeviceInfo PROGMEM devinfo = {
  {0xca, 0xfe, 0xde},     // Device ID
  "0101010101",           // Device Serial
  {0xca, 0xfe},           // Device Model Indoor
  0x10,                   // Firmware Version
  as::DeviceType::THSensor, // Device Type
  {0x01, 0x00}            // Info Bytes
};

typedef AvrSPI<10, 11, 12, 13> SPIType;
typedef Radio<SPIType, 2> RadioType;
typedef StatusLed<LED_PIN> LedType;
typedef AskSin<LedType, IrqInternalBatt, RadioType> Hal;
Hal hal;

class WeatherChannel : public Channel<Hal, List1, EmptyList, List4, PEERS_PER_CHANNEL, List0>, public Alarm {
  public:
    WeatherChannel () : Channel(), Alarm(5) {}
    virtual ~WeatherChannel () {}

    virtual void trigger (__attribute__ ((unused)) AlarmClock& clock) {
      tick = seconds2ticks(5);
      DPRINT("ALARM V= ");
      DDECLN(device().battery().current());
      clock.add(*this);
    }

    void setup(Device<Hal, List0>* dev, uint8_t number, uint16_t addr) {
      Channel::setup(dev, number, addr);
      sysclock.add(*this);
    }

    uint8_t status () const { return 0; }

    uint8_t flags  () const { return 0; }
};

typedef MultiChannelDevice<Hal, WeatherChannel, 1> WeatherType;
WeatherType sdev(devinfo, 0x20);
ConfigButton<WeatherType> cfgBtn(sdev);

void setup () {
  DINIT(57600, ASKSIN_PLUS_PLUS_IDENTIFIER);
  sdev.init(hal);
  hal.initBattery(60UL * 60, 24, 19);
  buttonISR(cfgBtn, CONFIG_BUTTON_PIN);
  sdev.initDone();
  while( hal.battery.current() == 0 ) ;
  DPRINTLN("INIT DONE");
}

void loop() {
  bool worked = hal.runready();
  bool poll = sdev.pollRadio();
  if ( worked == false && poll == false ) {
    hal.sleep<>();
  }
}

Alle 5 Sekunden wacht die AlarmClock auf und gibt die aktuelle Spannung aus.

ALARM V= 31
ALARM V= 31
...

Reduziere ich langsam am Labornetzgerät die Spannung, verändert sich nichts. :x: Der initial beim Start gemessene Wert wird fortlaufend ausgegeben. :white_check_mark:Entferne ich hal.sleep<>();, dann funktioniert es und der ausgegebene Spannungswert sinkt.

Nun die spannende Frage: Ist das nur bei mir so? Kann das jemand reproduzieren?

pa-pa commented 4 years ago

Mach bitte mal in den loop noch etwas "Arbeit" rein - _delay_ms(10) oder so. Die ersten 10 Interruptwerte werden weggeworfen und dann bist Du wahrscheinlich schon wieder im sleep().

jp112sdl commented 4 years ago

Damit geht es. Es reichen auch schon 5ms.

Selbst mit Temperaturmessung und Senden hatte ich jedoch immer die selben Spannungswerte erhalten, auch über zig Alarm-Trigger hinweg... dann geht das wohl alles insgesamt zu schnell?

Jetzt mache ich mir natürlich Gedanken, ob meine ganzen auf IrqInternalBatt umgeflashten TH-Sensoren irgendwann mal einen korrekten LowBat-Status liefern werden.

Wäre es vielleicht sinnvoll, den Delay noch mit in den sleep () zu packen?

jp112sdl commented 4 years ago

Die ersten 10 Interruptwerte werden weggeworfen und dann bist Du wahrscheinlich schon wieder im sleep().

Ich hatte mir hier nach Zeile 378 https://github.com/pa-pa/AskSinPP/blob/6d66a8c691cf9b2647e5034eb7355adc39799b80/BatterySensor.h#L377-L379 ein DPRINT eingebaut und auch eine Reihe von (jedoch immer veralteten) Werten bekommen. Also es wurden schon deutlich mehr als 10 Werte erfasst.

Das _delay_ms scheint noch irgendwas anderes zu "reparieren".

Dann werd ich sicherheitshalber mal meine Geräte neu flashen.

pa-pa commented 4 years ago

Ich habe mal im setIdle() etwas eingebaut. Probier mal jetzt. Wenn 10mal der Wert nicht ermittelt werden konnte, wird so lange weiter gelesen, bis ein gültiger Wert ermittelt wurde.

jp112sdl commented 4 years ago

Sitze gerade dran und hab's direkt ausprobiert - funktioniert nun auch ohne Delay! Danke

Eine Frage noch zu: v = (__gb_BatCurrent + v) / 2; Welcher Sinn steckt dahinter, den Mittelwert aus der aktuellen und der vorherigen Messung zu bilden, statt direkt den Vergleich

if( v < __gb_BatCurrent ) {...

zu machen?

pa-pa commented 4 years ago

Hm - gute Frage. Wollte ich wohl Messwerte mitteln. Und habe dann bestimmt den Mindestcounter eingebaut. Kannst ja mal ohne probieren. Sparen wir ein paar Byte.

jp112sdl commented 4 years ago

Oh weh, ich hatte eigentlich schon die IrqExternalBatt Klasse fertig :sweat_smile: Aber die Umbauten in der Globals.cpp werfen mich grad wieder zurück.... Können die

uint16_t __gb_BatCurrent = 0;
uint16_t __gb_BatCount = 0;
void (*__gb_BatIrq)() = 0;

wieder global werden und aus der IrqInternalBatt:: raus? Sonst müsste ich das für IrqExternalBatt:: noch mal definieren.

Oder docch lieber eine einzige IrqBatt bauen, die über template-Parameter external oder internal misst?

pa-pa commented 4 years ago

Könnten auch wieder raus - aber ich habe lieber alles in Klassen. Da weiss ich dann wenigstens, dass der Linker das Zeug auch weg läßt, wenn es nicht gebraucht wird. Habe ja auch den ADC-Vector-Routine über die Klasse angebunden. Könntest Du nicht auch ableiten und nur das init() und irq() anpassen ?

pa-pa commented 4 years ago

IrqBatt mit Template wäre auch cool :-)

jp112sdl commented 4 years ago

Und kann das static void vecfunc() __asm__("__vector_21") __attribute__((__signal__, __used__, __externally_visible__)); wieder raus und auf die vorherige ISR(ADC_vect) {... zurück gebaut werden?

Der _ADCvect ist beim 328P = __vector_21, beim ATMega1284P ist das __vector_24

IrqBat mit Template wäre auch cool :-)

Letztendlich unterscheiden sich die Messungen (intern/extern) nur in der Berechnung und dem Setzen des zu messenden Channels im ADMUX (sowie dem ACTIVATIONPIN).

Ich mache morgen mal einen Vorschlag fertig.

pa-pa commented 4 years ago

Wenn der ADC_vect beim 1284 anders ist, müssen wir noch eine eintsprechendes Define machen. Vielleicht gibt es ja auch schon eines irgendwo. Ich will alle IRQ-Vector-Routinen so umbauen, da sie dann nur ins System kommen, wenn die Klasse genutzt wird. ISR(ÁDC_vect) wird halt immer eingetragen und damit sind dann auch die aufgerufenen Klassen immer mit drin.

jp112sdl commented 4 years ago

Wenn der ADC_vect beim 1284 anders ist, müssen wir noch eine eintsprechendes Define machen.

Ja dann so

===

Das Problem mit den falschen/veralteten Messwerten habe ich bei externer Messung jedoch immer noch :/ Ich habe batSkip mal auf > 50 gesetzt, aber auch das hilft nur manchmal.

A0 klemmt über Spannungsteiler an +5V vom FTDI. Gemessen wird alle 3 Sekunden.

screenshot

Die LowPower schaltet im Sleep den _ADCOFF. Das Wiedereinschalten dauert einige Clock Cycles... vielleicht hat es damit was zu tun. In jedem Fall hilft weiterhin ein _delay_ms() unmittelbar vor hal.sleep<>();

pa-pa commented 4 years ago

Du musst batCount hoch setzen - das wird im irq & im setIdle geprüft - könnte vielleicht besser batCountToIgnore heissen

jp112sdl commented 4 years ago

Hatte ich zwischenzeitlich auch probiert. Aufgefallen ist mir, dass nach dem Sleep ein paar krumme Messwerte erfasst werden. Da reicht es aus, wenn von hunderten Folge-Messungen > 10 (oder 20, ...) auch nur 1 Wert aus der Reihe tanzt. Der erfüllt dann die Bedingung

if( v < __gb_BatCurrent ) {
          __gb_BatCurrent = v;
        }

Ich werde mal messen, wie hoch der zusätzliche Ruhestromverbrauch ist, wenn man den _ADCON lässt. Dann gibt es nämlich keinerlei Probleme.

jp112sdl commented 4 years ago

Ich hab mal einen Vorschlag als PR gemacht https://github.com/pa-pa/AskSinPP/pull/199

Ohne Template; dafür mit initExternal Methode. Entweder es wird wie gehabt init() aufgerufen, dann wird intern gemessen, oder man nimmt initExternal(sens,activate).