technoblogy / ulisp

A version of the Lisp programming language for ATmega-based Arduino boards.
http://www.ulisp.com/
MIT License
377 stars 45 forks source link

Problems with sleep on 1284p #64

Closed asmagill closed 4 months ago

asmagill commented 5 months ago

I'm using the latest ulisp 4.4b (as of this posting) on an ATMega1284P with the MightyCore 2.2.2 bootloader and compiling with the Arduino IDE.

This may be related to using the Arduino IDE, but I found 2 issues with using the sleep function...

The relavent code is at https://github.com/technoblogy/ulisp/blob/596440d95c56a4bc48b9f112c1e2f36beddaa366/ulisp-avr.ino#L1983-L2010

First, when compiling I get a redefinition of void sleep() from .../avr/2.2.2/cores/MCUdude_corefiles/wiring_extras.h. Simple enough, just rename the function to something like ulisp_sleep and change it in the two places it's called, and that cleared up.

But when I invoke (sleep 10) it would go into sleep mode and never wake up. I tweaked the code a bit to the following and it works again as I expect (I also took out some unnecessary #if... #endif stuff since it was checking the same defines as the one wrapping the whole function within doze):

void ulisp_sleep () {
#if defined(CPU_ATmega2560) || defined(CPU_ATmega1284P)
  ADCSRA = ADCSRA & ~(1<<ADEN); // Turn off ADC
  delay(100);  // Give serial time to settle
  sleep_enable();
  sleep_cpu();
  ADCSRA = ADCSRA | 1<<ADEN; // Turn on ADC
#endif
}

void doze (int secs) {
#if defined(CPU_ATmega2560) || defined(CPU_ATmega1284P)
  // Set up Watchdog timer for 1 Hz interrupt
  WDTCSR = 1<<WDCE | 1<<WDE;
  WDTCSR = 1<<WDIE | 6<<WDP0;     // 1 sec interrupt
  delay(100);  // Give serial time to settle
  ADCSRA = ADCSRA & ~(1<<ADEN); // Turn off ADC
  PRR0 = PRR0 | 1<<PRTIM0;
  while (secs > 0) {
    sleep_enable();
    sleep_cpu();
    secs--;
  }
  WDTCSR = 1<<WDCE | 1<<WDE;     // Disable watchdog
  WDTCSR = 0;
  ADCSRA = ADCSRA | 1<<ADEN; // Turn on ADC
  PRR0 = PRR0 & ~(1<<PRTIM0);
#else
  delay(1000*secs);
#endif
}

My understanding of the low level timers and registers is weak, but I'm thinking it's because we disable TIMER0 in doze, but then call delay in sleep (ulisp_sleep)...

Also, I noticed that it is possible to invoke (sleep) with no arguments and it will call sleep (ulisp_sleep) directly, but I don't discern any way to wake back up... am I missing something somewhere?

Have I sussed this out correctly, and should I submit a PR?

technoblogy commented 5 months ago

To wake up from (sleep) with no timeout you have to generate an interrupt, such as a Reset. For an example of using that see:

http://www.technoblogy.com/show?2AMW

I'll check your other point on my ATMega1284P board; won't be able to do that until next week though.

Thanks for the report! David

technoblogy commented 5 months ago

I've had another look at your report and I think you're correct; The statements:

  PRR0 = PRR0 | 1<<PRTIM0;

and

  PRR0 = PRR0 & ~(1<<PRTIM0);

should be moved to either side of:

  sleep_enable();
  sleep_cpu();

in sleep(). You should then be able to keep the rest of my original code (apart from the unnecessary #if... #endif stuff.

I'll verify that when I get to my computer.

Regards, David

asmagill commented 5 months ago

I can confirm moving those into my ulisp_sleep function does in fact work.

technoblogy commented 5 months ago

Great!

technoblogy commented 4 months ago

I've fixed this on Release 4.6; thanks for pointing it out.