SpenceKonde / DxCore

Arduino core for AVR DA, DB, DD, EA and future DU-series parts - Microchip's latest and greatest AVRs. Library maintainers: Porting help and adviccee is available.
Other
189 stars 50 forks source link

Different behaviour Serial0.available() between tinyMegaCore and DxCore #149

Closed mrWheel closed 3 years ago

mrWheel commented 3 years ago

Is there an explanation why this code worked fine on the tinyMegaCore (T3216) but not so with the DxCore (AVR128DB28)?

setup()
{
  Serial0.begin(9600);
  .
  .
  //-- sleep sleepMinutes minuten
  for(int i=0; i<((sleepMinutes*60)*2); i++)
  {
    if (Serial0.available())
    {
      startScan = false;   
    }
    else
    {
      sleep_cpu();  // ~0.5 sec.
      startScan = true;
    }
  }
  Serial0.println(F("\r\nI'm awake again ..\r\n"));
SpenceKonde commented 3 years ago

What does it do differently?

mrWheel commented 3 years ago

Let me elaborate: On the T3216 I can interrupt (exit) the for loop by keying in characters (that I can then parse and act upon). With the AVR-DB the characters are not “catched” and so Serial.available() is always “0” and the code executes an other sleep_cpu().

The idea is that the AVR sleeps a long time (or actual multiple shorter times in the for-loop) but can be woken-up if it receives a character (s).

Maybe due to eratta 2,12? Or should I use a different sleep-mode?

SpenceKonde commented 3 years ago

Please post the full test sketch, I need to see all the configuration, particularly where you set things up to make it wake up.

mrWheel commented 3 years ago

Please post the full test sketch, I need to see all the configuration, particularly where you set things up to make it wake up.

The code is still "Work in progress". Sleep seems to work, but I cannot exit the for .. sleep .. loop with Serial input! There are many functions is separate files, but they have no influence on the main setup() and loop()

Apriciate your help!!

/*
***************************************************************************  
**  Program   : ZDD_Controller 
**  Based on  : t3216_muxTester_v05
**  Processor : AVR128DB32
*/
#define _FW_VERSION "v1.0 (01-10-2021)"
/*
**  Copyright (c) 2021 Willem Aandewiel
**
**  TERMS OF USE: MIT License. See bottom of file.                                                            
***************************************************************************      
*      
  Arduino-IDE settings for AVR128DB28:

    - Chip: "AVR128DB28"
    - Clock Speed: "24MHz internal"
    - millis()/micros() timer (TCB2 recomended): "TCB2"
    - BOD level if enabled (Bootload burn req'd): "1.9V"
    - BOD Mode Active/Sleeping (Bootload burn req'd): "Disabled/Disabled"
    - Save EEPROM (Bootload burn req'd): "EEPROM retained"
    - Reset pin function (Bootload burn req'd): "Hardware Reset (recomended)"
    - Startup Time (Bootload burn req'd): "8ms"
    - Port: <select correct port>
    - Programmer:"jtag2updi"

    ==================================
    ==> DxCore version 1.3.6 <==
    ==================================
*/

#define HAS_COMM_MODULE
#define USE_SORT
//-- activate only ONE of the below!!
 #define DEBUG1
// #define DEBUG2
// #define DEBUG3

#include "macros.h"
#include <avr/sleep.h>

#define SCommMod          Serial2
#define _NUM_SENSORS        3
#define _MAX_DISTANCE     299   //-- between 100 and 500
#define _MAX_MEASURES       8   //-- minimal 5!
#define _SLEEP_MINUTES      5   //-- in minutes
#define _POWER_CM_ON      HIGH  //-- inverse next 2 lines for
#define _POWER_CM_OFF      LOW  //-- other chip
#define _MAX_REPLY_BUFF 200

#ifdef USE_SORT
  //-- prototype --
  void printArray(int ID, uint32_t aVal[_MAX_MEASURES], int);
  void printArray(int ID, uint32_t aVal[_MAX_MEASURES]);
#endif

//-- AVR128DB28
//                  O-----------+
//         LED  PA7 [1]      [28] PA6
// SCAN SWITCH  PC0 [2]      [27] PA5   POWER - ZDD_CommModule
//    Power S0  PC1 [3]      [26] PA4   
//  Trigger S0  PC2 [4]      [25] PA3   
//     Echo S0  PC3 [5]      [24] PA2   UPDI
//           VDDIO2 [6]      [23] PA1   RXD0  - Monitor
//    Power S1  PD1 [7]      [22] PA0   TXD0  - Monitor
//  Trigger S1  PD2 [8]      [21] GND  
//     Echo S1  PD3 [9]      [20] VCC
//    Power S2  PD4 [10]     [19] UPDI
//  Trigger S2  PD5 [11]     [18] PF6   RESET (system)
//     Echo S2  PD6 [12]     [17] PF1   RXD2  - ZDD_CommModule
//              PD7 [13]     [16] PF0   TXD2  - ZDD_CommModule
//             AVCC [14]     [15] GND
//                  +-----------+
//

//-- define pins numbers
//--         -- PORTA --
//const uint8_t TXD0        = PIN_PA0;  //-- system 
//const uint8_t RXD0        = PIN_PA1;  //-- system
//const uint8_t FREE        = PIN_PA2;
//const uint8_t UPDI        = PIN_PA3;  //-- system
//const uint8_t FREE        = PIN_PA4;
  const uint8_t POWER_CM    = PIN_PA5;
  const uint8_t RESET_SW    = PIN_PA6;
  const uint8_t STATUS_LED  = PIN_PA7;  //-- default

//--         -- PORTC --
  const uint8_t SCAN_SW     = PIN_PC0;
  const uint8_t S0_POWER    = PIN_PC1;
  const uint8_t S0_TRIGGER  = PIN_PC2;
  const uint8_t S0_ECHO     = PIN_PC3;

//--         -- PORTD --
  const uint8_t S1_POWER    = PIN_PD1;
  const uint8_t S1_TRIGGER  = PIN_PD2;
  const uint8_t S1_ECHO     = PIN_PD3;
  const uint8_t S2_POWER    = PIN_PD4;
  const uint8_t S2_TRIGGER  = PIN_PD5;
  const uint8_t S2_ECHO     = PIN_PD6;
  const uint8_t FREE        = PIN_PD7;

//--         -- PORTF --
//const uint8_t TXD2        = PIN_PF0;  //-- system
//const uint8_t RXD2        = PIN_PF1;  //-- system
//const uint8_t RESET       = PIN_PF6;  //-- system

//--    PIN_PA0, PIN_PA1
#define Monitor         Serial0

struct structSensor {
  int Power;
  int Trigger;
  int Echo;
};

// defines variables
structSensor  Sensor[6];
int16_t       distanceInt, distanceNorm;
uint8_t       testAddress = 0;
unsigned char data[4]={};
uint32_t      toggleTimer, scanTimer, blinkLedTimer, powerTimer, timeOutTimer;
uint32_t      startTime, elapsTime;
uint32_t      sendTimer;
uint8_t       dummyDist = 0;
int           sample;
char          cmdBuff[50];
char          tmpBuff[200];
bool          runAutonomous = true;
bool          startScan = false;
volatile bool startRunning  = true;
uint8_t       swNewState = 0, swLastState = 1;
uint8_t       testState, testSave;
uint16_t      aDist[8];
uint8_t       commCount;
int8_t        commResponse;
bool          gotRDYresponse, modemReady=false;
bool          IC2found = false, IC3found = false, IC4found = false;
int16_t       sleepMinutes = _SLEEP_MINUTES;
char          lastReply[_MAX_REPLY_BUFF+1];

//=============================================================
#define resetWDT() __asm__ __volatile__ ("wdr"::)

//=============================================================
void enableWDT()
{
  Monitor.println(F("enable WDT .. "));
  Monitor.flush();

//----------------------------------------------------------
//   Timeout  WDT period name       WDT Window name
//   -------- --------------------  ------------------------
//    0.008s  WDT_PERIOD_8CLK_gc    WDT_WINDOW_8CLK_gc
//    0.016s  WDT_PERIOD_16CLK_gc   WDT_WINDOW_16CLK_gc
//    0.032s  WDT_PERIOD_32CLK_gc   WDT_WINDOW_32CLK_gc
//    0.064s  WDT_PERIOD_64CLK_gc   WDT_WINDOW_64CLK_gc
//    0.128s  WDT_PERIOD_128CLK_gc  WDT_WINDOW_128CLK_gc
//    0.256s  WDT_PERIOD_256CLK_gc  WDT_WINDOW_256CLK_gc
//    0.512s  WDT_PERIOD_512CLK_gc  WDT_WINDOW_512CLK_gc
//    1.024s  WDT_PERIOD_1KCLK_gc   WDT_WINDOW_1KCLK_gc
//    2.048s  WDT_PERIOD_2KCLK_gc   WDT_WINDOW_2KCLK_gc
//    4.096s  WDT_PERIOD_4KCLK_gc   WDT_WINDOW_4KCLK_gc
//    8.192s  WDT_PERIOD_8KCLK_gc   WDT_WINDOW_8KCLK_gc
//----------------------------------------------------------

//_PROTECTED_WRITE(WDT.CTRLA, WDT_PERIOD_8KCLK_gc | WDT_WINDOW_16CLK_gc );
  _PROTECTED_WRITE(WDT.CTRLA, WDT_PERIOD_8KCLK_gc);

} //  enableWDT()

//=============================================================
void disableWDT()
{
  Monitor.println(F("disable WDT .."));
  Monitor.flush();
  _PROTECTED_WRITE(WDT.CTRLA, 0); 

} //  disableWDT()

//=============================================================
void RTC_init(void)
{
  /* Initialize RTC: */
  while (RTC.STATUS > 0)
  {
    ;                                   /* Wait for all register to be synchronized */
  }
  //RTC.CLKSEL = RTC_CLKSEL_INT32K_gc;    /* 32.768kHz Internal Ultra-Low-Power Oscillator (OSCULP32K) */
  //RTC.CLKSEL = RTC_CLKSEL_OSC1K_gc;
  RTC.CLKSEL = RTC_CLKSEL_OSC32K_gc;
  RTC.PITINTCTRL = RTC_PI_bm;           /* PIT Interrupt: enabled */

  RTC.PITCTRLA = RTC_PERIOD_CYC16384_gc /* RTC Clock Cycles 16384, resulting in 32.768kHz/16384 = 2Hz */
  | RTC_PITEN_bm;                       /* Enable PIT counter: enabled */

} //  RTC_init()

//=============================================================
ISR(RTC_PIT_vect)
{
  RTC.PITINTFLAGS = RTC_PI_bm;          /* Clear interrupt flag by writing '1' (required) */

} // ISR()

//---------------------------------------------------------
void checkCommMod(int32_t timeOutTime = 500)
{
  uint32_t waitTimer = millis();
  while ((int)(millis()-waitTimer) < timeOutTime)
  {
    while(SCommMod.available())
    {
      char c = (char)SCommMod.read();
      waitTimer = millis(); // reset timer
      if (c >= ' ' || c == '\n')
      {
        Monitor.print(c);  
      }
    }
  }

} //  checkCommMod()

//---------------------------------------------------------
void setup()
{
  pinMode(STATUS_LED, OUTPUT);
  pinMode(POWER_CM,   OUTPUT);  
//pinMode(RESET_SW, INPUT_PULLUP);  //-- system default
  pinMode(SCAN_SW,  INPUT_PULLUP);

  //-- Power DOWN ZDD_CommModule
  digitalWriteFast(POWER_CM, _POWER_CM_OFF);  // TPS2052B

  //-- SoftwareSerial 
  Monitor.begin(9600); // PIN_PA5 (RX) / PIN_PA4 (TX)
  while(!Monitor) 
  { 
    delay(50); 
    digitalWriteFast(STATUS_LED, CHANGE);
  }

  //delay(500);
  digitalWriteFast(STATUS_LED, HIGH);

  //----- setup OpenColector for SoftwareSerial port -----------
  SCommMod.begin(9600); // PIN_PB2 (TX) / PIN_PB3 (RX)
  while(!SCommMod) { delay(10); }

  Monitor.println(F("\r\n\n\nAnd than it all starts ..."));
  Monitor.print(F("ZDD_Controller Version: "));
  Monitor.println(_FW_VERSION);
  Monitor.println();

  Monitor.println(F("Blink the STATUS LED (slow)...\r\n"));
  blinkStatusLed(2, 500);

  //-- setup timer and sleep mode
  RTC_init();
  //-- Set sleep mode to STANDBY 
  //set_sleep_mode(SLEEP_MODE_STANDBY); // seems to work, no Serial.available()
  //-- Set sleep mode to POWER DOWN mode
  set_sleep_mode(SLEEP_MODE_PWR_DOWN);  // seems to work
  //-- Set sleep mode to IDLE mode
  //set_sleep_mode(SLEEP_MODE_IDLE);  // does not work
  sleep_enable();
  interrupts();

  enableWDT();

  //-- now initialize Sensor array -----
  Sensor[0].Power   = S0_POWER;
  Sensor[0].Trigger = S0_TRIGGER;
  Sensor[0].Echo    = S0_ECHO;

  Sensor[1].Power   = S1_POWER;
  Sensor[1].Trigger = S1_TRIGGER;
  Sensor[1].Echo    = S1_ECHO;

  Sensor[2].Power   = S2_POWER;
  Sensor[2].Trigger = S2_TRIGGER;
  Sensor[2].Echo    = S2_ECHO;

  for (int s=0; s<_NUM_SENSORS; s++)
  {
    pinMode(Sensor[s].Power,   OUTPUT);
    pinMode(Sensor[s].Trigger, OUTPUT);
    pinMode(Sensor[s].Echo,    INPUT);
  }

  digitalWriteFast(POWER_CM, _POWER_CM_OFF);  //-- PUT ZDD_CommModule to sleep
  blinkStatusLed(5, 100);
  disableWDT();

  Monitor.println(F("\r\nOK! Ready to go ..\r\n"));
  showMenu();
  Monitor.println(F("Now going to sleep for five minutes .."));
  Monitor.flush();

  //-- 75*4=300 seconden := 5 minuten
  //for(int s=0; s<75; s++)
  Monitor.print("Start sleep ..");
  Monitor.flush();
  //RTC.CLKSEL = RTC_CLKSEL_OSC1K_gc; ~ 16 seconden
  //RTC.CLKSEL = RTC_CLKSEL_OSC32K_gc;  ~0.5 seconden
  uint32_t sleepStart = millis();
  for(int i=0; i<5; i++)
  //for(int i=0; i<((5*60)*2); i++)  
  {
    if (Monitor.available())
    {
      startScan = false;   
    }
    else
    {
      Monitor.print("."); Monitor.flush();
      sleep_cpu();  // 
      startScan = true;
    }
  }
  Monitor.printf("\r\nI'm awake again after %d minutes..\r\n"
                                    , (millis()-sleepStart)/60);
  Monitor.flush();

  toggleTimer = millis();
  sendTimer   = millis()+500;
  startScan   = true;
  Monitor.flush();

} //  setup()

//---------------------------------------------------------
void loop()
{  
  checkCommMod();

  if (Monitor.available())
  {
    memset(cmdBuff, 0, sizeof(cmdBuff));
    Monitor.readBytesUntil('\n', cmdBuff, sizeof(cmdBuff));
    //removeCntrFromBuff();
    if (strncasecmp(cmdBuff, "SCA", 3)==0)
    {
      startScan = true;
    }
    else if (strncasecmp(cmdBuff, "POW", 3)==0)
    {
      Monitor.println(F("power UP ZDD_CommModule .."));
      digitalWriteFast(POWER_CM, _POWER_CM_ON);  //-- TPS2052B
    }
    else if (strncasecmp(cmdBuff, "RES", 3)==0)
    {
      Monitor.println(F("Reset ZDD_Controller .."));
      Monitor.flush();
      disableWDT();
      delay(100);
    }
    else if (strncasecmp(cmdBuff, "IN+", 3)==0)
    {
      sleepMinutes += 5;
      if (sleepMinutes > 60) sleepMinutes = 60;
      Monitor.print(F("Sleep time is set to ["));
      Monitor.print(sleepMinutes);
      Monitor.println(F("] minutes\r\n"));
      startScan = false;
    }
    else if (strncasecmp(cmdBuff, "IN-", 3)==0)
    {
      sleepMinutes -= 5;
      if (sleepMinutes < 1) sleepMinutes = 1;
      Monitor.print(F("Sleep time is set to ["));
      Monitor.print(sleepMinutes);
      Monitor.println(F("] minutes\r\n"));
      startScan = false;
    }
    else
    {
      //-- send cmdBuff to ZDD_CommModule
      SCommMod.println(cmdBuff);
    }
  }

  if (startScan)
  {
    startTime   = millis();
    scanTimer   = millis();
    startScan   = false;
    modemReady  = false;
    blinkStatusLed(2, 200);

    //-- power up the ZDD_CommModule
    Monitor.println(F("power UP ZDD_CommModule .."));

    digitalWriteFast(POWER_CM, _POWER_CM_ON);  // Power Up ZDD_CommModule
    SCommMod.println("RESET");
    enableWDT();

    Monitor.println(F("\r\nScan sensoren .."));
    //-- it takes some time to get a stable reading
    //-- so power sensor 0 in advance
    digitalWrite(Sensor[0].Power, HIGH);
    Monitor.println(F("Sw0 powered up!"));
    Monitor.flush();
    delay(100);

    //-- now read every sensor one-by-one
    for (int s=0; s< _NUM_SENSORS; s++)
    {
      resetWDT();
      aDist[s] = 0;
      //-- power UP next sensor so it has time to stabilize
      digitalWrite(Sensor[s+1]. Power, HIGH);
      //-- read this sensor
      aDist[s] = readDistance(s);
      //-- switch OFF power for this sensor
      digitalWrite(Sensor[s].Power, LOW);
      //delay(100); //-- test zou weg moeten mogen
    }
    Monitor.println();
    disableWDT();
    blinkStatusLed(4, 100);

#ifdef HAS_COMM_MODULE
    sendDataToCommMod();

#else  //-- HAS NO COMM_MODULE
    Monitor.println(F("compiled with HAS_COMM_MODULE not defined!"));
    Monitor.println(F("Skip communication with Modem.\r\n"));

#endif  //-- HAS_COMM_MODULE

    Monitor.println(F("power DOWN Sensors and ZDD_CommModule .."));
    //-- power down the ZDD_CommModule
    digitalWriteFast(POWER_CM, _POWER_CM_OFF);  // Power Down TPS2052B

    for (int s=0; s<_NUM_SENSORS; s++)
    {
      Monitor.printf("%d ",s);
      digitalWrite(Sensor[s].Power, LOW);
    }
    Monitor.println();

    elapsTime = millis() -startTime;
    Monitor.printf("\r\nElapse time is %d.%d sec.\r\n"
                            , (elapsTime / 1000)
                            , (elapsTime - ((elapsTime / 1000)*1000)));

  } // if startScan ..

  showMenu();

  Monitor.print(F("Now going to sleep for about "));
  Monitor.print(sleepMinutes);
  Monitor.println(F(" minutes .."));
  while (Monitor.available())
  {
    Monitor.read();
  }
  Monitor.flush();

  digitalWriteFast(POWER_CM, _POWER_CM_OFF);  // Power Down ZDD_CommModule
  //-- sleep sleepMinutes minuten
  enableWDT();
  uint32_t sleepStart = millis();
  //for(int i=0; i<((sleepMinutes*60)*2); i++)
  for(int i=0; i<((sleepMinutes*60)*1); i++)  // temp half time
  {
    if (Monitor.available())
    {
      startScan = false;   
    }
    else if (!digitalRead(SCAN_SW)) //-- SCAN switch pushed?
    {
      Monitor.println(F("Scan Switch Pushed!!"));
      //-- wait for switch release 
      while(!digitalRead(SCAN_SW)) { delay(5); }
      startScan = true;
      break;
    }
    else
    {
      sleep_cpu();  // ~0.5 sec.
      startScan = true;
    }
    resetWDT();
  }
  Monitor.print("\r\nI'm awake again after ");
  printMinuten((millis()-sleepStart));
  Monitor.println(" minutes..");
  disableWDT();

} //  loop()
SpenceKonde commented 3 years ago

Thanks I will have a look tonight.

I don't think the USART errata is going to be part of this puzzle: The t3216 and DB have all the same USART bugs and unless I'm mistaken you didn't enable anything that was the subject of the errata - no ODME bit nor start of frame detection interrupt here.... They made the USART module for the tinies back in the early days of modern AVR era, it had only a couple of bugs, and it has not undergone any revisions since other than having lost one of wacky rs485 modes, and I wouldn't be surprised to be find that the change was only made on paper. That would be pretty typical - you decide the behavior is too fiddly, or too weird, or it interacts poorly with too many use cases, but you want the rest of the module in a new product? You can fix that by just removing it from the datasheet without expensive silicon changes. After all, if someone uses it anyway, even if it makes demons fly out of their nostrils, "Well, that's a reserved bit, 'reserved bits should always be set to 0'"

There's a lot of copy+paste in microcontroller design, which is also why we can copy+paste large amounts of code between these parts. Which of course is what makes these sort of issues valuable as checks on the core design - I'm not sure what is different yet, but I think it's most likely to be a difference in the core that shouldn't be there.

SpenceKonde commented 3 years ago

So you have two loops that fit that descrioption?

  uint32_t sleepStart = millis();
  for(int i=0; i<5; i++)
  //for(int i=0; i<((5*60)*2); i++)  
  {
    if (Monitor.available())
    {
      startScan = false;   
    }
    else
    {
      Monitor.print("."); Monitor.flush();
      sleep_cpu();  // 
      startScan = true;
    }
  }

and

  enableWDT();
  uint32_t sleepStart = millis();
  //for(int i=0; i<((sleepMinutes*60)*2); i++)
  for(int i=0; i<((sleepMinutes*60)*1); i++)  // temp half time
  {
    if (Monitor.available())
    {
      startScan = false;   
    }
    else if (!digitalRead(SCAN_SW)) //-- SCAN switch pushed?
    {
      Monitor.println(F("Scan Switch Pushed!!"));
      //-- wait for switch release 
      while(!digitalRead(SCAN_SW)) { delay(5); }
      startScan = true;
      break;
    }
    else
    {
      sleep_cpu();  // ~0.5 sec.
      startScan = true;
    }
    resetWDT();
  }
  Monitor.print("\r\nI'm awake again after ");
  printMinuten((millis()-sleepStart));
  Monitor.println(" minutes..");
  disableWDT();

But in both cases, what we're doing is waking at 2 Hz based on the PIT, and looking at the serial port. The serial port should never receive anything on either device, though, because it's asleep and has not been configured to either wake on start of frame nor to stay on in standby (which would consume a large enough amount of power to defeat the point of sleep, because it would force the oscillator to stay on).

Are you using software serial on the tiny3216 as the comments imply? Because that will wake the device (I have zero faith in it's ability to get the first character right, and little faith in it's ability to keep framing straight after that - it's a godawful library: it pulls in attachInterrupt() which in turn gloms onto every single pin interrupt vector making manual pin interrupt configuration impossible. And the attachInterrupt interrupt is SLOW because it's written for the general case, and the compiler has no way to know that there's only one interrupt on one port that will ever fire, so it has to check all pins of all ports); somehow the mess of a library is actually able to generate correct results despite having such a timing critical routine as the RX isr written in C for the compiler to make hash of, an excruciatingly long time between the the ISR firing and execution getting to the point where it looks for the 8 bits of the character (this is bad because only the time between the falling edge of the start bit and the baud rate specify when it should sample the pin for each bit, so you need to know exactly how long the prologue and interrupt overheat takes on your system, including checking all those interrupt flags to find out which one got called (and hey, whaddyaknow, it's the only one we ever set up )!, it's not the same on different parts, because they have different numbers of interrupts) and that it functions at all ever is a miracle or an atrocity....

If you were, that would probably receive characters during sleep, because the interrupts would wake it up, though it's hard to imagine that it gets the first byte right, nor not-wrong enough for the one after it to not be misframed;

I really hate both attachInterrupt() and softwareSerial. The latter because it relies on guesses that were made for different parts under different conditions to remain valid, and because it relies on the former, which. IMO is either the most evil or second most evil part of the Arduino API (in terms of that API demanding that implementations kneecap* themselves to support it, and kneecapping any sketch that includes anything from the WInterruppts.c translation unit, because the ISRs are then defined (hence not usable if you need, for example a fast pin interrupt without all that godawful overhead) for all ports... Oh, and it needs to create storage for pointers to the attached functions for every pin in RAM, organized as an array.... meaning that even though a 28 pin part may only have 4 pins on port C and no port B/E, since it does have a port F.... bringing in attachInterrupt bringsin 8 (A) + 8 (B) + 8 (C) + 8 (D) + 8 (E) + 7 (F - no PF7 on 28-pin parts until the DD which can turn UPDI into an I/O pin) = 47 pointers, or 94 bytes, the majority of which will never be read or written.... The two of those together commit a level of heinous atrocity that only the likes of the String class could hope to rival....

Finally, what are you doing with millis while the device is asleep? Millis is never tracked while sleeping. A library to change that is is in progress though and in extrremely high demand.

mrWheel commented 3 years ago

Short answer: Yes, SoftwareSerial on the t3216 (as it only has one UART). The two same routines are a bit overdone and should be places in a function but “it is work in progress”. The millis() around these for-sleep-loop’s are an attempt to find out how long the total for-sleep-loop took. There not part of the necessary code. I know your horror about the attachInterrupt() function. I wish I had the knowledge to do it on a lower level. On the other hand, most of my programs are not time critical .. except now where I try to catch an incoming character ;-)

So great you are looking in to all this!

SpenceKonde commented 3 years ago

Is the software serial the one that you're checking during that time on the t3216? Because if so, that explains why it works there.

mrWheel commented 3 years ago

Is the software serial the one that you're checking during that time on the t3216? Because if so, that explains why it works there.

Oh, I thought that was clear. Yes! The SoftwareSerial is the one I check in the t3216.

To elaborate a little more: The device is going to be potted to make it waterproof. Connection to Serial and sensors will be with water-proof connectors. In the production setting there is no way to press the [reset] (or [scan]) button (also potted on the PCB). The only time I need to “break out of the for-sleep-loop” is for debugging. The sleep-time will be about 20 minutes or so and for debugging it would be nice to break out of it.

Oh .. the [scan] button does work in the sketch (well, come to think of it: of-course! Nothing to do with interrupts).

SpenceKonde commented 3 years ago

I have learned from experience that when comments and code do not match (eg, the software serial in comments hardware in code) it's very hard to know what the right assuptions to make are. But that explains things then.

And the first character actually makes it through the wake from standby -> software serial okay? Why I say this is - if you don't care what character it sees - if it sees one at all you could send something like 0x255 (I pick this because the serial port is least likely to realize it's gotten a character at all, while the interrupt will be triggered either way, though that's not a very easy character to type if you're not using a powerful serial monitor, as opposed to.... that thing arduino ships with (that thing is just crap. The only nice thing about it is that it closes and reopens connections when uploading over the same port (which is real nice when the console is the same port that it's programmed over), which is more or less the absolute raw bare minimum excuse for a serial console - I use the swiss army knife of consoles, hTerm. I think windows only. Freeware (oddly not open source) but it is difficult to imagine a feature it lacks. Person who wrote is from arduino circles, which is why it seems custom made for this sort of thing....)

Anyway

I'd try something like

ISR(PORTA_PORT_vect) {
PORTA.PIN1CTRL &= ~(PORT_ISC_gm); // turn off interrupt
VPORTA.INTFLAGS |= 1<<1; // clear the flag too, otherwise it will fire the instant it's turned on.
}
// to enable before entering sleep: 
PORTA.PIN1CTRL = (PORTA.PIN1CTRL & ~(PORT_ISC_gm)) | PORT_ISC_BOTHEDGES_gc;

You can't wake on rise or fall only because it's pin 1 or pin 5 of port A. Only pins 2 and 6 of ports are "fully async" and can see if an edge is rising or falling while the clock is stopped (they can also trigger off pulses shorter than one system clock... including noise, unfortunately. But with a line that stays idle until you send something, that's fine. I'm thinking best to have it turn itself off, and then make sure you never went to sleep without turning it on, just because you'll otherwise generate tons of unnecessary interrupts which can only cause problems.

Oh, and if you're going to be putting this into sleep for battery use, you need to make sure that you don't have any floating pins. Those pins randomly transitioning all the time can burn a surprising amount of power - you either set them output, or turn on the pullup, or disable the input buffer (or have them connected to something that puts them into one state or the other. Some people swear this was always a problem, and while I don't deny that i was a problem on classic AVRs, i don't recall it being quite so dramatic there.

mrWheel commented 3 years ago

And the first character actually makes it through the wake from standby -> software serial okay?

Yes, thats in the t3216 sketch and that works!

Why I say this is - if you don't care what character it sees -

I don't care!

if it sees one at all you could send something like 0x255 (I pick this because the serial port is least likely to realize it's gotten a character at all, while the interrupt will be triggered either way, though that's not a very easy character to type if you're not using a powerful serial monitor, as opposed to.... that thing arduino ships with (that thing is just crap.

I use CoolTerm but see no option to send 0x255

Also: typing anything does NOT escape the for-sleep-loop (I have added a "break;" after startScan = false;

    if (Monitor.available())
    {
      startScan = false;
      break;   
    }

I includes the ISR() before

#define resetWDT() __asm__ __volatile__ ("wdr"::)

.. and

    .
    .
    else
    {
      // to enable before entering sleep: 
      PORTA.PIN1CTRL = (PORTA.PIN1CTRL & ~(PORT_ISC_gm)) | PORT_ISC_BOTHEDGES_gc;
      sleep_cpu();  // ~0.5 sec.
      startScan = true;
    }

But it has no effect (not breaking out of the for-sleep-loop) :-(

Oh, and if you're going to be putting this into sleep for battery use, you need to make sure that you don't have any floating pins. Those pins randomly transitioning all the time can burn a surprising amount of power - you either set them output, or turn on the pullup

I know (work in progress ;-)

SpenceKonde commented 3 years ago

Oh, I wasnt thinking about the fact that you have it going back to sleep after every wake, so the pin interrupt just needs to set a globally scoped volatile variable to something. Or even a bit in one of the general purpose registers (there are 4, analogous to the 3 classic avrs have except all are in the low io space, while the classic GPIORs are usually, but it all depends where on the board those darts landed during the design process. Microchip doesn't play darts. they're like a normal volatile byte, except that |= and &= are single clock and atomic, and tests that and them with a value consisting of a 1 bit and 7 0s or 0 and 7 1s ) are similarly blazing fast. Not that I think they are necessary here


Spence Konde Azzy’S Electronics

New products! Check them out at tindie.com/stores/DrAzzy GitHub: github.com/SpenceKonde ATTinyCore: Arduino support for almost every ATTiny microcontroller Contact: @.***

On Sat, Oct 2, 2021, 06:09 mrWheel @.***> wrote:

And the first character actually makes it through the wake from standby -> software serial okay?

Yes, thats in the t3216 sketch and that works!

Why I say this is - if you don't care what character it sees -

I don't care!

if it sees one at all you could send something like 0x255 (I pick this because the serial port is least likely to realize it's gotten a character at all, while the interrupt will be triggered either way, though that's not a very easy character to type if you're not using a powerful serial monitor, as opposed to.... that thing arduino ships with (that thing is just crap.

I use CoolTerm but see no option to send 0x255

Also: typing anything does NOT escape the for-sleep-loop (I have added a " break;" after startScan = false;

if (Monitor.available())
{
  startScan = false;
  break;
}

I includes the ISR() before

define resetWDT() asm volatile ("wdr"::)

.. and

.
.
else
{
  // to enable before entering sleep:
  PORTA.PIN1CTRL = (PORTA.PIN1CTRL & ~(PORT_ISC_gm)) | PORT_ISC_BOTHEDGES_gc;
  sleep_cpu();  // ~0.5 sec.
  startScan = true;
}

But it has no effect (not breaking out of the for-sleep-loop) :-(

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/SpenceKonde/DxCore/issues/149#issuecomment-932726719, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABTXEW7FMEVJ3D34PA5ISTTUE3K4BANCNFSM5FCSP3ZA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

mrWheel commented 3 years ago

Oh, I wasnt thinking about the fact that you have it going back to sleep after every wake, so the pin interrupt just needs to set a globally scoped volatile variable to something. Or even a bit in one of the general purpose registers (there are 4, analogous to the 3 classic avrs have except all are in the low io space, while the classic GPIORs are usually, but it all depends where on the board those darts landed during the design process. Microchip doesn't play darts. they're like a normal volatile byte, except that |= and &= are single clock and atomic, and tests that and them with a value consisting of a 1 bit and 7 0s or 0 and 7 1s ) are similarly blazing fast. Not that I think they are necessary here

Pfff.. the above is all gibberish to me :-( (remember: “end-user”)

mrWheel commented 3 years ago

As a side note: received your AVR128DB64 board! I’m not in the office right now so I can not read the silkscreen but assume it is tip-top designed! Thanks

SpenceKonde commented 3 years ago

Thanks - you would have gotten one of the pre-release boards so you're silkscreen is only "okay" anyway - The biggest changes between prerelease and release version was to improve the silkscreen and add a whole bunch of information that even I can barely read unaided, including stuff like a table of which port has which CCL on it, the ports named on the front (including "has ADC" where appropriate). Yours does have what I consider to be the most dramatic change I made to the silk though, the pin names in Pxn notation around the outside, in hope of driving more people away from referring to pins by number. x_x That convention is only sensible with very homogeneous product - when everyone's using a '328p, it's fine. On classic tinyAVR, it wasn't terrible aside from the fact that most parts have multiple numbering schemes, because NOTHING was in common between parts with different pincount. But on modern parts, the sequential numbering is not ideal in terms of usefulness.

And unlike my older yellow-soldermask designs, with appropriate magnification, you should be able to read the silkscreen. It's small, but readable (the yellow boards were almost unreadable, even under good conditions with good eyesight - white on light yellow. I think the board house switched solder mask formulations, because when I first started using those colors, it wasn't a problem, or the place I order through switched board houses; I do wish they were more careful in alignment between silkscreen and other board features - but this is the only place that doesn't charge a crazy surcharge for panelizing and won't complain about being asked to make perfboard (many board houses don't like that, I think it wears out the drill bits)

Oh - The important thing I was saying is that I think you want to set a volatile variable defined at global scope, which you would set when the ISR was called, and clear before entering the sleep loop, and then test that instead of Serial0.available. (ie, declare it as volatile uint8_t SerialWake = 0; set it to 1 in the ISR, and check that instead of available. (volatile is required since non-volatile variable accesses will often be optimized away - the compiler doesn't consider interrupts to be reachable when doing optimization. Access to a volatile variable cannot be optimized away, and all volatile variables must be accessed in the order the program specifies. (The compiler must assume that both reading and writing them may have sideffects, and that their value may change on it's own)

I always was convinced that a key design tool at Atmel was a dartboard and bucket of darts for choosing register addresses and pin mappings. There's no other explanation for how some of those decisions came out. Since microchip took over, everything is more systematic. As you'd expect from a company that makes microchips named Microchip.

SpenceKonde commented 3 years ago

I believe this is solved - reopen and explain what the outstanding issue is if not.