SpenceKonde / megaTinyCore

Arduino core for the tinyAVR 0/1/2-series - Ones's digit 2,4,5,7 (pincount, 8,14,20,24), tens digit 0, 1, or 2 (featureset), preceded by flash in kb. Library maintainers: porting help available!
Other
551 stars 142 forks source link

RTC_init() will only work in setup() , loop forever if run in main() #420

Closed cpainchaud closed 3 years ago

cpainchaud commented 3 years ago
void RTC_init()
{
  /* Initialize RTC: */
  while (RTC.STATUS > 0  )
  {
    ;                                 /* Wait for all register to be synchronized */
  }
  RTC.CLKSEL = RTC_CLKSEL_INT1K_gc;   

  RTC.PITINTCTRL = RTC_PI_bm;           /* PIT Interrupt: enabled */

  RTC.PITCTRLA = RTC_PERIOD_CYC4096_gc 
                 | RTC_PITEN_bm;
}

while (RTC.STATUS > 0) is never ending loop when RTC_init() is run from main() instead of setup();

SpenceKonde commented 3 years ago

And what value does RTC.STATUS have when it's getting hung up like that? Is that the first time RTC_init() is called, or was it also called in setup?

SpenceKonde commented 3 years ago

Do you mean setup() and loop() function? I imagine so, since under the hood, main() calls the core's internal init(), as well as setup(), and finally while(1) loop(); And if you define main yourself (it's weakly defined so you can override it), setup() isn't called unless you call it.

cpainchaud commented 3 years ago

And what value does RTC.STATUS have when it's getting hung up like that? Is that the first time RTC_init() is called, or was it also called in setup?

it's not called in setup, STATUS remains stuck at value=2

cpainchaud commented 3 years ago

Do you mean setup() and loop() function? I imagine so, since under the hood, main() calls the core's internal init(), as well as setup(), and finally while(1) loop(); And if you define main yourself (it's weakly defined so you can override it), setup() isn't called unless you call it.

loop() function yes sorry

cpainchaud commented 3 years ago

Apparently it's happening on second call of RTC_init(), not the first one

cpainchaud commented 3 years ago
void RTC_init()
{
  static int call_count = 0;

  call_count++;

  /* Initialize RTC: */
  while (RTC.STATUS > 0 ||RTC.PITSTATUS )
  {
    Serial.print(call_count);
    Serial.print(" ");
    Serial.println(RTC.STATUS);                                   /* Wait for all register to be synchronized */
  }
  RTC.CLKSEL = RTC_CLKSEL_INT1K_gc;  

  RTC.PITINTCTRL = RTC_PI_bm;           /* PIT Interrupt: enabled */

  RTC.PITCTRLA = RTC_PERIOD_CYC4096_gc
                 | RTC_PITEN_bm;
}

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

void loop() {

  // do stuff

  RTC_init();  // <-- the idea here is to have a variable sleep period that will be passed as a parameter of the function later in the project
  set_sleep_mode(SLEEP_MODE_PWR_DOWN);
  sleep_enable();
  sleep_cpu();

}
SpenceKonde commented 3 years ago

What part is this again?

I see that you are trying to use the PIT without enabling the RTC. Most tinyAVR 0-series and 1-series are impacted by silicon errata, see the applicable errata sheets for the part you're working with. The Rev. C silicon that is purportedly available in new 3216 and 3217 parts corrects this. But I don't think it's fixed on anything else. Issue not listed on 2-series errata, so should work there.

Disabling the RTC Stops the PIT
Writing RTC.CTRLA.RTCEN to ‘0’ will stop the PIT.
Writing RTC.PITCTRLA.PITEN to ‘0’ will stop the RTC.
Work Around
Do not disable the RTC or the PIT if any of the modules are used.

(note - yes, I know this is not what you describe. They also say "Workaround: None" for the MUXPOS bug on the AVR128DA, which is one of the easiest bugs to workaround ever, and release hardware with errata that have were present on parts they released 4 years prior, hide the errata in a separate document, and give no indication in the datasheet that a peripheral doesn't work the way they say, never has, and likely never will; They're not exactly models of candor and humility when it comes to silicon bugs,. At least Atmel put the errata in the same document! (Atmel also produced far less of it by using the same old obsolete peripherals they had for well over a decade)

cpainchaud commented 3 years ago

ATtiny412 , i can't tell for the revision.

SpenceKonde commented 3 years ago

Doesn't matter what revision it is for a 412,all versions of the silicon are bugged. Like I said, it's just the 321x they fixed.

2.7 RTC - Real-Time Counter
2.7.1 Any Write to the RTC.CTRLA Register Resets the RTC and PIT Prescaler
Any write to the RTC.CTRLA register resets the 15-bit prescaler resulting in a longer period on the current count or
period.
Work Around
None.
Affected Silicon Revisions
Rev. A Rev. B Rev. C
X X X
2.7.2 Disabling the RTC Stops the PIT
Writing RTC.CTRLA.RTCEN to ‘0’ will stop the PIT.
Writing RTC.PITCTRLA.PITEN to ‘0’ will stop the RTC.
Work Around
Do not disable the RTC or the PIT if any of the modules are used.
Affected Silicon Revisions
Rev. A Rev. B Rev. C
X X X

Try setting RTCEN bit to 1 in RTC.CTRLA (the first time you call RTC_init() I mean. I would take a guess that what "triggers" it is when you write the RTC.CLKSEL with the RTC disabled, the CNTBUSY bit is set and never gets cleared because it never can be synchronized to the RTC clock domain because the RTC clock isn't running.

Does it really work if you set it up like that but call it just from setup O_o And it uses the correct clock and all?

Oh: And you can always get the revision from the chip, just Serial.println(SYSCFG.REVID);

cpainchaud commented 3 years ago

Thx that made it ! RTC.CTRLA |= RTC_RTCEN_bm;

cpainchaud commented 3 years ago

yet now pause/delays are shortened by a factor of x2 or x4 .... weird!

SpenceKonde commented 3 years ago

Glad to hear it!

Well, to the extent that I can be "glad" when the matter is a godamned silicon bug they designed into the entire 0-series and 1-series and have fixed in a grand total of 2 parts since then....

And these are pauses and delays generated how?

Are you sure this isn't something caused during investigations of the other issue that you forgot to undo? (happens to me all the time. Occasionally it's even gotten into releases!)

cpainchaud commented 3 years ago

nope, commenting RTC.CTRLA |= RTC_RTCEN_bm; fixes the issue, all values are divided by 2, 8seconds become 4, 4 becomes 2

cpainchaud commented 3 years ago
#include <Arduino.h>
#include "SHTC3.h"
#include <TinyMegaI2CMaster.h>
#include <avr/sleep.h>

#define SHTC3_ADDR_7BIT 0b1110000

bool i2cOpResult;
int32_t temperatureFromSensor;
int32_t humidityFromSensor;

void RTC_init()
{
  static int call_count = 0;
  call_count++;

  if( call_count > 1 )
    return;

  RTC.CTRLA |= RTC_RTCEN_bm;

  /* Initialize RTC: */
  while (RTC.STATUS > 0 )
  {
    //Serial.print(call_count);
    //Serial.print(" ");
    //Serial.println(RTC.STATUS);                                   /* Wait for all register to be synchronized */
  }
  RTC.CLKSEL = RTC_CLKSEL_INT1K_gc;   

  RTC.PITINTCTRL = RTC_PI_bm;           /* PIT Interrupt: enabled */

  RTC.PITCTRLA = RTC_PERIOD_CYC8192_gc 
                 | RTC_PITEN_bm;
}

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

void setup() {
  TinyMegaI2C.init();
  Serial.begin(9600);
  delay(10);

  Serial.print("Chip Revision:");
  Serial.println(SYSCFG.REVID);

}

void radioTransmitData() {

  uint8_t i;
  uint8_t bitNumber;
  uint8_t mask;

  bool toggle = true;

  //preamble
  for(i=0; i<8; i++) {
    if(toggle)
      digitalWrite(1, HIGH);
    else
      digitalWrite(1, LOW);
    delayMicroseconds(500);
    toggle = !toggle;
  }
  // Preamble is supposed to finish in LOW state

  // Now transmitting data
  for(i=0; i<24; i++) {
    for(bitNumber=0; bitNumber <8; bitNumber++){
      mask = 1;
      mask = mask << bitNumber;
      if( (sendBuffer[i] & mask) == 0 ) {
        digitalWrite(1, LOW);
        delayMicroseconds(290);
        digitalWrite(1, HIGH);
        delayMicroseconds(690);
      }
      else{
        digitalWrite(1, LOW);
        delayMicroseconds(690);
        digitalWrite(1, HIGH);
        delayMicroseconds(290);
      }
    }
  }

  // we need to make sure PIN is LOW when we exit the loop
  digitalWrite(1, LOW);

};

void loop() {

  Serial.println("beg loop 5");

  TinyMegaI2C.start(SHTC3_ADDR_7BIT, 0);
  if(TinyMegaI2C.write(SHTC3_WAKEUP/256) && TinyMegaI2C.write(SHTC3_WAKEUP & 0xFF)) {
    Serial.println("sent wake up");
  }
  delayMicroseconds(SHTC3_RESET_DELAY_US);
  TinyMegaI2C.stop();

  TinyMegaI2C.start(SHTC3_ADDR_7BIT, 0);
  if(TinyMegaI2C.write(SHTC3_READ_LP/256) && TinyMegaI2C.write(SHTC3_READ_LP & 0xFF)) {
    Serial.println("sent measure request");
  }
  TinyMegaI2C.restart(SHTC3_ADDR_7BIT, 6);
  temperatureFromSensor = ((int32_t)TinyMegaI2C.read()) << 8;
  temperatureFromSensor += TinyMegaI2C.read();

  TinyMegaI2C.read(); // CRC

  //humidityFromSensor = 0;
  humidityFromSensor = ((int32_t)TinyMegaI2C.read()) << 8;
  humidityFromSensor += TinyMegaI2C.read();

  TinyMegaI2C.readLast(); // CRC

  TinyMegaI2C.stop();

  TinyMegaI2C.start(SHTC3_ADDR_7BIT, 0);
  if(TinyMegaI2C.write(SHTC3_SLEEP/256) && TinyMegaI2C.write(SHTC3_SLEEP & 0xFF)) {
    Serial.println("sent sleep");
  }
  TinyMegaI2C.stop();

  //(175.0f / 65536.0f) * val - 45.0f
  Serial.println( (temperatureFromSensor*175*10)/65536 - 45*10 );

  //(100.0f/65536.0f) * Val
  Serial.println( humidityFromSensor*100/65536 );

  Serial.flush();

  RTC_init();
  set_sleep_mode(SLEEP_MODE_PWR_DOWN);
  sleep_enable();
  sleep_cpu();

  //delay(500);
}
SpenceKonde commented 3 years ago

Can you tell me on what lines this delay is? I don't see where the delay you're talking about is.....

cpainchaud commented 3 years ago

I meant Sleep time is reduced by x2

cpainchaud commented 3 years ago

more updates : it happens when RTC.CTRLA |= RTC_RTCEN_bm; is called a second time! so a workaround for the workaround ;) is to use this one only once

SpenceKonde commented 3 years ago

That is fucked up.... Almost certainly part of the same errata that you're working around.... so thoughtful of them to warn us about that in the detailed errata description Unless they are so utterly incompetent that despite knowing there was something seriously wrong there, and that it was kind of wacky they didn't poke around enough to spot it. That would be consistent with errata as unsubtle as a lot of the errata here is being in the product in the first place (I'm particularly thinking of the Dx-series, though the "D type latch is non-functional" is a gem too (

sigh. I like most of what Microchip does... the new AVRs are great, AVRxt is a huge improvement. but the way they handle errata is straight up disrespectful to their customers.

SpenceKonde commented 3 years ago

Closing. no defect in the core.

cpainchaud commented 3 years ago

@SpenceKonde I can only second what you said .... I think I found a new bug : in SLEEP_POWER_DOWN mode, my multimeter shows 0.02uA when board_build.f_cpu = 20000000L to 5000000L if I use 1Mhz-4Mhz clock in board build then I get 10x power consumption: 0.15uA

And this is happening with whatever Fuse i put to effectively drive the crystal to 1-20Mhz range. That could be an issue in your code potentially?

SpenceKonde commented 3 years ago

Is there a typo there? The numbers I see are 0.2us and 0.15us, and neither of those is 10x the other.


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 Tue, May 11, 2021, 12:20 Christophe Painchaud @.***> wrote:

I can only second what you said .... I think I found a new bug : in SLEEP_POWER_DOWN mode, my multimeter shows 0.2uA when board_build.f_cpu = 20000000L to 5000000L if I use 1Mhz-4Mhz clock in board build then I get 10x power consumption: 0.15uA

And this is happening with whatever Fuse i put to effectively drive the crystal to 1-20Mhz range. That could be an issue in your code potentially?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/SpenceKonde/megaTinyCore/issues/420#issuecomment-838775507, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABTXEW6H2HJQ7PLTUUCFS6DTNFKMNANCNFSM44VKEZ6Q .

SpenceKonde commented 3 years ago

Anything depending on F_CPU is my department though


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 Tue, May 11, 2021, 12:20 Christophe Painchaud @.***> wrote:

I can only second what you said .... I think I found a new bug : in SLEEP_POWER_DOWN mode, my multimeter shows 0.2uA when board_build.f_cpu = 20000000L to 5000000L if I use 1Mhz-4Mhz clock in board build then I get 10x power consumption: 0.15uA

And this is happening with whatever Fuse i put to effectively drive the crystal to 1-20Mhz range. That could be an issue in your code potentially?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/SpenceKonde/megaTinyCore/issues/420#issuecomment-838775507, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABTXEW6H2HJQ7PLTUUCFS6DTNFKMNANCNFSM44VKEZ6Q .

cpainchaud commented 3 years ago

Is there a typo there? The numbers I see are 0.2us and 0.15us, and neither of those is 10x the other.

0.02uA at 20-5Mhz range , 0.15uA at 4Mhz or less

SpenceKonde commented 3 years ago

Oh yuck... that would definitely be a bug in the core I reckon....

You have checked that it is okay with 8 or `16 MHz, - the "16 MHz derived" speeds, not just 5/10/20, right? (I am not really set up to measure such small amounts of current)

Note that since 2.2.0 we set OSCCFG fuse on every UPDI upload, not just burn bootloader. So burning bootloader at 16 but then selecting 20 ,MHz and uploading will run at 20.

Thanks for holding off with the bug report until I had already released 2.3.2, though ;-)

cpainchaud commented 3 years ago

Oh yuck... that would definitely be a bug in the core I reckon You have checked that it is okay with 8 or `16 MHz, - the "16 MHz derived" speeds, not just 5/10/20, right? (I am not really set up to measure such small amounts of current)

I checked all possible speeds and the mess is in the range 1-4Mhz

Note that since 2.2.0 we set OSCCFG fuse on every UPDI upload, not just burn bootloader.

with PIO and pyudpi it never burns these fuses. I am waiting for my Nano to arrive to test the other method ....

SpenceKonde commented 3 years ago

Huh.... Well that's interesting.

Oh you're on PIO? I would not expect any of the IDE integration features like burning fuses on upload to work if it's not able to parse boards.txt and platform.txt to generate correct upload and bootload commands. You are using 2.3.2 of the core not the.. I don't use the alternate IDEs because I spend so much time on core maintenance that I never get to do any projects big enough to see the need for PIO. AFAIK it's is probably using straight pyupdi rather than what we use (I don't know which is faster though, actually()

What settings are you using for millis timing source, does changing that make a difference?

Since I can't go and investigate this myself - it's below the noise floor on my multimeter, all I can do is look through the code and wonder, and ask questions. The fact that it's not okay at 4MHz or 1 MHz but is okay at 5 MHz and 8 MHz excludes all the things I'd suspect...

cpainchaud commented 3 years ago

Hi,

I do use Arduino IDE to burn fuses/bootloader.

I will do the following whenever I can:

  1. compile the code sample on Arduino rather than PIO
  2. confirm the readings on 2 different ampmeters of 2 different brands
  3. yet you never know with chinese Ampmeters at these scales so I will also try to run a very large capacitor discharge test and see how long it takes to deplete in both configurations. I can even add more capas if it's too short to measure but given that in sleep time it seems to be under 0.2uA it should work.
cpainchaud commented 3 years ago

well Arduino IDE + 2.3.2 spits this error straigtht:

exec: "C:\Users\christophe.painchaud\AppData\Local\Arduino15\packages\DxCore\tools\avr-gcc\7.3.0-atmel3.6.1-azduino4/bin/avr-g++": file does not exist Error compiling for board ATtiny412/402/212/202.

The right path should be C:\Users\christophe.painchaud\AppData\Local\Arduino15\packages\DxCore\tools\avr-gcc\7.3.0-atmel3.6.1-azduino4\avr\bin

See? It's missing the 'avr' sub directly at the end

SpenceKonde commented 3 years ago

No, i think it is installing a defective toolchain package.


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 Wed, May 12, 2021, 08:17 Christophe Painchaud @.***> wrote:

well Arduino IDE + 2.3.2 spits this error straigtht:

exec: "C:\Users\christophe.painchaud\AppData\Local\Arduino15\packages\DxCore\tools\avr-gcc\7.3.0-atmel3.6.1-azduino4/bin/avr-g++": file does not exist Error compiling for board ATtiny412/402/212/202.

The right path should be C:\Users\christophe.painchaud\AppData\Local\Arduino15\packages\DxCore\tools\avr-gcc\7.3.0-atmel3.6.1-azduino4\avr\bin

See? It's missing the 'avr' sub directly at the end

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/SpenceKonde/megaTinyCore/issues/420#issuecomment-839723415, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABTXEW5YLZWWJHLD72SRPIDTNJWUXANCNFSM44VKEZ6Q .

SpenceKonde commented 3 years ago

Reinstalling 2.3.2 will fix it. It was puilling in version 4 instead of 4b (aka 4a on linux 4b windows). The script I use to do the board manager release was pointed to the wrong version for some reason.

SpenceKonde commented 3 years ago

This was part of the debacle that led to there being multiple versions of the last DxCore release too.

I'm using a semi-automated means of generating new toolchain files, because arduino folks have just straight up dropped the ball there. I runthe build script on a C5XL amazon instance in the cloud, but the binaries are suspect, and it only solves the problem for x64 linux and I couldn't figure out how to get the cross-compiling setup together to do proper builds for the rest of the platforms. So instead I take the result of that. For everything except windows, i copy it to WSL ubuntu and it gets gets extracted, the files related to support for new devices copied ontop of an the Arduino7 toolchain., bringing the new precompiled objects device specs and io headers (taking care not to bring the new linker scripts which don;t work for reasons unclear), and then recompress them. I have a sequence of commands I copy-paste from a text file, and regexes I pass the output through to generate the descriptions of it for board manager (with size and sha256 hash). On windows I do the same thing manually.

With the last round, I screwed up BOTH of those batches. Linux was missing half of the files, and on windows, well, there was an extra layer of directories.. But I forgot about updating the megaTinyCore build script to use the 4b vertsion with both sets of fixes, instead of 4, which doesn't work on either platform. And it didn't show up on CI because that odesn't test board manager stuff.

cpainchaud commented 3 years ago

Reinstalling 2.3.2 will fix it. It was puilling in version 4 instead of 4b (aka 4a on linux 4b windows). The script I use to do the board manager release was pointed to the wrong version for some reason.

Arduino IDE just fails to downgrade or uninstall. Super helpful message :)

image

It feels like I will need to reinstall Arduino IDE itself

SpenceKonde commented 3 years ago

Argh, seriously.....? no reinstalling ide won't fix it, you need to delete your arduino15 folder, it's in c:/users/yourusername/AppData(hidden)/local... because thats where board package state is stored... it persists across ide reinstall (and reinstall wont fix it )


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 Fri, May 14, 2021, 03:02 Christophe Painchaud @.***> wrote:

Reinstalling 2.3.2 will fix it. It was puilling in version 4 instead of 4b (aka 4a on linux 4b windows). The script I use to do the board manager release was pointed to the wrong version for some reason.

Arduino IDE just fails to downgrade or uninstall. Super helpful message :)

[image: image] https://user-images.githubusercontent.com/6696638/118233971-f8f59100-b492-11eb-8ff2-d77dbbffec66.png

It feels like I will need to reinstall Arduino IDE itself

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/SpenceKonde/megaTinyCore/issues/420#issuecomment-841056176, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABTXEW2N4FRKV523O26MGGTTNTDHRANCNFSM44VKEZ6Q .

cpainchaud commented 3 years ago

I am an Arduino IDE noob :p Anyways I found my way through it and got it working.

I am getting even weirder behavior with the following test code: it will do sleep POWERDOWN for 4 secs then delay 4 secs in a loop. the POWERDOWN sleep will be ignored/return immediatly if I keep the delay in code. If I comment the delay(), POWERDOWN sleep will work fine.

#include <Arduino.h>
#include <avr/sleep.h>

void RTC_init()
{
  static int call_count = 0;
  call_count++;

  //if( call_count > 1 )
  //  return;

  /* Initialize RTC: */
  while (RTC.STATUS > 0 )
  {
    //Serial.print(call_count);
    //Serial.print(" ");
    //Serial.println(RTC.STATUS);                                   /* Wait for all register to be synchronized */
  }
  RTC.CLKSEL = RTC_CLKSEL_INT1K_gc; 

  RTC.PITINTCTRL = RTC_PI_bm;           /* PIT Interrupt: enabled */

  RTC.PITCTRLA = RTC_PERIOD_CYC4096_gc
                 | RTC_PITEN_bm;
}

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

void setup() {
  RTC.CTRLA |= RTC_RTCEN_bm; // to fix bug in the chip hardware https://github.com/SpenceKonde/megaTinyCore/issues/420
  Serial.begin(9600);
  Serial.println("\n\n   START !!!\n\n");
}

void loop() {

  static int count = 0;

  count++;

  Serial.print("PING ");
  Serial.println(count);

  Serial.println("POWER_DOWN SLEEP 4secs ");
  Serial.flush();

  RTC_init();
  set_sleep_mode(SLEEP_MODE_PWR_DOWN);
  sleep_enable();
  sleep_cpu();

  Serial.println("DELAY() 4secs ");
  delay(4000)

;}
SpenceKonde commented 3 years ago

Does not reproduce for me. I see 3.5 seconds in power down sleep, then 4sec delaty... so something appears to be different, but not sure what, What settings do youhave for millis/micros timer, system clock. any other non-default menu options?

SpenceKonde commented 3 years ago

Oh - huh - the FIRST one returns instantly, yeah. I wonder why...

SpenceKonde commented 3 years ago

Aaaaha! Found a better description of how these timers are imoplemented in the datasheet of newer parts. The timing of the first PIT interrupt not guaranteed. This behavior is normal and correct - the PIT is counting whenever The RTC is enabled. Once the RTC is enabled AND the pit is configured, it generates an interrupt every 4 seconds whether you want

#include <Arduino.h>
#include <avr/sleep.h>

void RTC_init()
{
  static int call_count = 0;
  call_count++;

  //if( call_count > 1 )
  //  return;

  /* Initialize RTC: */
  while (RTC.STATUS > 0 )
  {
    //Serial.print(call_count);
    //Serial.print(" ");
    //Serial.println(RTC.STATUS);                                   /* Wait for all register to be synchronized */
  }
  RTC.CLKSEL = RTC_CLKSEL_INT1K_gc; 

  RTC.PITINTCTRL = RTC_PI_bm;           /* PIT Interrupt: enabled */

  RTC.PITCTRLA = RTC_PERIOD_CYC4096_gc
                 | RTC_PITEN_bm;
}

ISR(RTC_PIT_vect)
{
  RTC.PITINTFLAGS = RTC_PI_bm;          /* Clear interrupt flag by writing '1' (required) */
  Serial.write('*'); // Don;t print to serial from within an ISR, it's very poor practice.... but sometimes you have to. 
// minimize badness with the cleanest, smallest thing you can write - a single character,=via .write() bypassing bloat of print
}

void setup() {
  RTC.CTRLA |= RTC_RTCEN_bm; // to fix bug in the chip hardware https://github.com/SpenceKonde/megaTinyCore/issues/420
  Serial.begin115200);
  Serial.println("\n\n   
}I'm convinced this is actually safe (though bad practice) at 9600 baud, but moving to 1125200 baud (57600 is f running at 1 MHz ) Basically, you want to have serial ruinning FAST  reason writing to serial ffrom the interrupt is bad is because of the buffer, not because data is ciming out of the serial port. get data the arduino TX buffer to the /USAERR). MMinimize the amout of that being done by  high baud rates and slow bloated arduino API functions

void loop() {

  static int count = 0;

  count++;

  Serial.print("PING ");
  Serial.println(count);

  Serial.println("POWER_DOWN SLEEP 4secs ");
  Serial.flush();

  RTC_init();
  set_sleep_mode(SLEEP_MODE_PWR_DOWN);
  sleep_enable();
  sleep_cpu();

  Serial.println("DELAY() 4secs ");
  delay(4000);

}

But I think you are totally misunderstanding what the PIT does, 
and by virtue uf tyour comments, you were carrying me along for the ride... 

Once you set up the PIT() it generates a *periodic* interrupt In your case, you set it to a period of 4 seconds. At some point in time between 0 and 4 second - anywhere in that range is vali. 

And yourhalf time bug when youi write RTC.CTRLA? Well, that resets the the couPIT counter, too. If the interrupt is generated precisely at the midpoint (ie, if it's counting from -2048 to 2047, and srtarts at0 and triggers the interrupt when the timer "rolls over" from 2037 to -2948 - that would produce the behavior your see. 

You should initialize only when you have changed the configuration - not every time you want another delay from it. It is going to keep generionterrupts every 4 seconds, whether youn need them to waker up or not. it's a qMaking the RTC wait a specificed amount of time before triggering an interrupt requires the RTC, not the PIT, and cannot be used in power down sleep, as only the PIT can be used then.
You need to adapt  your code togenerate behavior that you are happy with fromn interrupt that fires in the background at a constant frequency, regardless of whether your are sleeping or not, and which, at initialization
cpainchaud commented 3 years ago

Thank you for digging this. You are absolutely right when you say that I don't know much about how the PIT works, thank you for the details!

Based on what you just wrote I am still a bit lost on the what should be approach for the following scenario: I am making a low-power-long-lasting battery power sensor. I had in mind the following sequence:

  1. Send I2C sensor read request
  2. Sleep (POWERDOWN if possible) 20 ms which is response time for the I2C sensor
  3. read I2C response, compute data, prepare buffer for radio transmission
  4. send data via radio (bascially a pin go LOW/HIGH alternatively every 400-700us). This is where is need another sleep period
  5. sleep at least 45 seconds
  6. go back to 1.

Based on these requirement, I will apparently have to change the registers a few times to match the 20ms(is it achievable to sleep that small amount of time?), 400-800u (is it achievable to sleep that small amount of time?) and 45seconds (~32+16secs with the 1Khz clock) periods

cpainchaud commented 3 years ago

So I just modified the code to call RTC_init() only once and changed sleep time to 4secs, delay() to 7 seconds. The problem persists with sleep() returning almost instantly, all time except for the first trigger. I chose 4 seconds and 7 seconds to ensure that the clock/end of delay() would never happen around the same time and that their difference would make the shift a different value every time.

#include <Arduino.h>
#include <avr/sleep.h>

void RTC_init()
{
  static int call_count = 0;
  call_count++;

  //if( call_count > 1 )
  //  return;

  /* Initialize RTC: */
  while (RTC.STATUS > 0 )
  {
    //Serial.print(call_count);
    //Serial.print(" ");
    //Serial.println(RTC.STATUS);                                   /* Wait for all register to be synchronized */
  }
  RTC.CLKSEL = RTC_CLKSEL_INT1K_gc;    

  RTC.PITINTCTRL = RTC_PI_bm;           /* PIT Interrupt: enabled */

  RTC.PITCTRLA = RTC_PERIOD_CYC4096_gc 
                 | RTC_PITEN_bm;
}

ISR(RTC_PIT_vect)
{
  RTC.PITINTFLAGS = RTC_PI_bm;          /* Clear interrupt flag by writing '1' (required) */
  Serial.println("INT terminated");
}

void setup() {
  Serial.begin(9600);
  Serial.println("\n\n   START !!!\n\n");

  RTC.CTRLA |= RTC_RTCEN_bm; // to fix bug in the chip hardware https://github.com/SpenceKonde/megaTinyCore/issues/420
  RTC_init();

  set_sleep_mode(SLEEP_MODE_PWR_DOWN);
  sleep_enable();
}

void loop() {

  static int count = 0;
  count++;

  Serial.print("PING ");
  Serial.println(count);

  Serial.println("POWER_DOWN SLEEP 4secs ");
  Serial.flush();

  sleep_cpu();

  Serial.println("DELAY() 7secs ");
  Serial.flush();
  delay(7000);
}
SpenceKonde commented 3 years ago

I think the best you could do for your requirements is getting the would probably be standby sleep with the RTC interrupts, which give you a lot of freedom to set when they happen (you can set the compare value towhat you want and reset count to 0 - though you do need to wait for count to sync.

I'l try to reproduce your issue with the the RTC if I have time, but I don;t really have any experience with the RTC or sleep modes because figuring out how those work and playing around with them has been getting deferred for over a year and I don't expect to get to work on it until mid-late summer.

freemovers commented 3 years ago

I can take a look tonight, but make sure to check the PITSTATUS while the registers are synchronized, not just the RTC.STATUS:


while (RTC.STATUS > 0 || RTC.PITSTATUS);
freemovers commented 3 years ago

Here is some more information of what we found last time: https://github.com/SpenceKonde/megaTinyCore/issues/298#issuecomment-761519827

freemovers commented 3 years ago

Looks like your PIT is working correctly and interrupting your code every 4 seconds:


21:15:40.562 ->    START !!!
21:15:40.562 -> 
21:15:40.562 -> 
21:15:40.562 -> PING 1
21:15:40.562 -> POWER_DOWN SLEEP 4secs 
21:15:42.526 -> INT terminated   // First interrupt
21:15:42.526 -> DELAY() 7secs 
21:15:46.491 -> INT terminated   //  -> 4 seconds after last interrupt
21:15:49.516 -> PING 2
21:15:49.516 -> POWER_DOWN SLEEP 4secs 
21:15:50.447 -> INT terminated   //  -> 4 seconds after last interrupt
21:15:50.447 -> DELAY() 7secs 
21:15:54.349 -> INT terminated   //  -> 4 seconds after last interrupt
21:15:57.375 -> PING 3
21:15:57.375 -> POWER_DOWN SLEEP 4secs 
21:15:58.309 -> INT terminated   //  -> 4 seconds after last interrupt
21:15:58.309 -> DELAY() 7secs 
21:16:02.229 -> INT terminated   //  -> 4 seconds after last interrupt
21:16:05.265 -> PING 4
21:16:05.265 -> POWER_DOWN SLEEP 4secs 
21:16:06.200 -> INT terminated   //  -> 4 seconds after last interrupt
21:16:06.200 -> DELAY() 7secs 
21:16:10.105 -> INT terminated   //  -> 4 seconds after last interrupt
21:16:13.134 -> PING 5
21:16:13.134 -> POWER_DOWN SLEEP 4secs 
21:16:14.062 -> INT terminated   //  -> 4 seconds after last interrupt
21:16:14.062 -> DELAY() 7secs 
21:16:18.017 -> INT terminated   //  -> 4 seconds after last interrupt

I have used TCB0 in single-shot mode for my last project where I wanted a short pulse period after waking up from the PIT. On the other hand, 20ms is long enough for the RTC counter. The RTC does not run in SLEEP_MODE_PWR_DOWN, so you can just switch between the different sleep modes to run the different timers. 
cpainchaud commented 3 years ago

Looks like your PIT is working correctly and interrupting your code every 4 seconds:

21:15:40.562 ->    START !!!
21:15:40.562 -> 
21:15:40.562 -> 
21:15:40.562 -> PING 1
21:15:40.562 -> POWER_DOWN SLEEP 4secs 
21:15:42.526 -> INT terminated   // First interrupt
21:15:42.526 -> DELAY() 7secs 
21:15:46.491 -> INT terminated   //  -> 4 seconds after last interrupt
21:15:49.516 -> PING 2
21:15:49.516 -> POWER_DOWN SLEEP 4secs 
21:15:50.447 -> INT terminated   //  -> 4 seconds after last interrupt
21:15:50.447 -> DELAY() 7secs 
21:15:54.349 -> INT terminated   //  -> 4 seconds after last interrupt
21:15:57.375 -> PING 3
21:15:57.375 -> POWER_DOWN SLEEP 4secs 
21:15:58.309 -> INT terminated   //  -> 4 seconds after last interrupt
21:15:58.309 -> DELAY() 7secs 
21:16:02.229 -> INT terminated   //  -> 4 seconds after last interrupt
21:16:05.265 -> PING 4
21:16:05.265 -> POWER_DOWN SLEEP 4secs 
21:16:06.200 -> INT terminated   //  -> 4 seconds after last interrupt
21:16:06.200 -> DELAY() 7secs 
21:16:10.105 -> INT terminated   //  -> 4 seconds after last interrupt
21:16:13.134 -> PING 5
21:16:13.134 -> POWER_DOWN SLEEP 4secs 
21:16:14.062 -> INT terminated   //  -> 4 seconds after last interrupt
21:16:14.062 -> DELAY() 7secs 
21:16:18.017 -> INT terminated   //  -> 4 seconds after last interrupt

I have used TCB0 in single-shot mode for my last project where I wanted a short pulse period after waking up from the PIT. On the other hand, 20ms is long enough for the RTC counter. The RTC does not run in SLEEP_MODE_PWR_DOWN, so you can just switch between the different sleep modes to run the different timers. 

Hi @freemovers,

If I look at your logs, we see that between message "POWER_DOWN SLEEP 4secs" and "DELAY() 7sec" there is always 0.8-1.2seconds while it is requested to sleep for 4 seconds.

cpainchaud commented 3 years ago

Here is some more information of what we found last time: #298 (comment)

I will have a look. I suppose we need rock solid interfaces in this project to avoid newcomers to shoot themselves in the foot like me :)

SpenceKonde commented 3 years ago

first one in the logs is 2 seconds.... the combination of the interrupt and the reasonably close periods of the delay vs What's throwing you off is that the interrupt doesn't actually end the non-sleep pauses, but it does end sleep.... so the fact that the periods of the two was selected to be way so different? Well, uh, not exactly!

Actually in @freemovers' demo, the system of the two pauses en effect forms a 8-seconds delay! NOT the 11 second delay that both of you are expecting. Actually, as I think about it, this has probably been a key point of confusion this whole time...

I only just now realized what could happen and what in this case, clearly is happening.: imagine you're sleeping. At time t = 0 This other device wakes you up with some sort of alarm,You turn on, service the ISR and see that next on your schedule is to twiddle your thumbs for 7 seconds. 4 seconds into this activity, at t = 4, the interrupt fires again. ISR fires you service it and then proceed. after 3 seconds, at t = 7. you've finished the delay. now it says you sleep for 4 seconds. So you start sleeping., but in another second, the isr fires. Now you're just woke up from sleep. You're back where you were at time t = 0 so this has a period of 8 seconds, not 11.

cpainchaud commented 3 years ago

@SpenceKonde do you mean that I need to reset the periodic counter to 0 before I go to sleep if I want to run sleep_cpu in sync with my loop? if so what would be the appropriate way of doing this?

May be my bad assumption was that sleep_cpu() would take care of that but now it makes sense to think it's not!

thank you

SpenceKonde commented 3 years ago

No, I didn't say that; I didn't say anything about how to achieve the goal you have in mind. I was only saying that your claim that his logs demonstrate unexpected behavior is not correct. The behavior shown inthe logs is correct and expected, if you look at what the hardware is expecting.

On currently available silicon (but not on new parts if/when they fix the errata!) I think you can reset the PIT prescaler by doing RTC.CTRLA = RTC.CTRLA (and probably waiting for it to sync); that is a serious defect though; it certainly shouldn't have that sideffect, but the errata sheet clearly indicates that it does.I think that this was actually a key factor explaining some of the earlier issues you were looking at;.... Otherwise you can't really do that, and you are using the PIT here in a manner inconsistent with thei designers' intent. you should be counting the times it wakes you and having it wake you several times, and not trying to use it to time a single PTC cycle accurately. The PIT should never be used when you need to time something on the scale of one PIT cycle.

cpainchaud commented 3 years ago

Ok thank you, I will experiment a bit. I think many people will be interested in a "sleep_power_down(int seconds)" like function which will avoid all the issues I am having right now.

SpenceKonde commented 3 years ago

Yeah, that's one of the things on the list of plans for the future sleep library, that's exactly the sort of thing that I think would be super useful. I was planning to not attempt t do it with the PIT with powerdown sleep and instead use the the RTC interupt in standby sleep, so we could get exactly the length of time we want.

On modern AVRs, power-down sleep doesn't save much power comp[ared to power down.